Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ACCT-4459: Support for domain scoped roles #1095

Merged
merged 26 commits into from Oct 13, 2022
Merged

ACCT-4459: Support for domain scoped roles #1095

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1972534
ACCT-4459: Add initial implementation of Policy structs
Sep 21, 2022
3ee7457
ACCT-4459: Add support to create members with policies
Sep 23, 2022
1028adb
ACCT-4459: Add Permission Group APIs and move permission/resource gro…
Sep 23, 2022
61f081f
ACCT-4459: Add resource group utility methods
Sep 23, 2022
caec0ba
ACCT-4459: Added create account member with policies utility method
Sep 23, 2022
8a83a0f
ACCT-4593: Add WIP script to call relevant endpoints for zola/nonzola…
Sep 28, 2022
3c87263
ACCT-4593: Refactor to reduce clutter, list out more endpoints to test
Sep 29, 2022
402e980
ACCT-4459: Add account_members_test
Sep 29, 2022
4e222dc
ACCT-4459: Add validation to Update and test
Sep 29, 2022
cf6756d
ACCT-4459: Rename methods for backwards-compat
Sep 29, 2022
53c41b8
ACCT-4459: Add resource group documentation and testing
Sep 29, 2022
026ece7
ACCT-4459: Add permission group testing and update documentation
Sep 29, 2022
09ca723
ACCT-4593: Keep policies and roles methods, clean up others
Sep 29, 2022
b29d54e
ACCT-4593: Remove internal testing script
Sep 29, 2022
aad7838
ACCT-4459: Resolve lint errors and file structure
Sep 30, 2022
5e812bd
ACCT-4459: gofmt
Sep 30, 2022
bd5a17f
ACCT-4459: end comments in periods
Sep 30, 2022
c4f8f2e
ACCT-4459: Move CreateAccountMember methods to new experimental format
Sep 30, 2022
3a517bb
ACCT-4459: Remove utility methods
Oct 12, 2022
78850c0
simplify method usage and params
jacobbednarz Oct 12, 2022
44f1170
permission_group: simplify `Get`/`List` methods and reuse
jacobbednarz Oct 12, 2022
0c76040
test cleanup
jacobbednarz Oct 12, 2022
e4be156
permission_groups: more validation
jacobbednarz Oct 12, 2022
5f45d74
update changelog to reflect method param changes
jacobbednarz Oct 12, 2022
06f2fc7
add missing `errors`
jacobbednarz Oct 12, 2022
6f4b50f
Merge branch 'master' into imobbs/ACCT-4459-domain-scoped-roles
jacobbednarz Oct 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
114 changes: 91 additions & 23 deletions account_members.go
View file
Open in desktop
Expand Up @@ -9,11 +9,12 @@ import (

// AccountMember is the definition of a member of an account.
type AccountMember struct {
ID string `json:"id"`
Code string `json:"code"`
User AccountMemberUserDetails `json:"user"`
Status string `json:"status"`
Roles []AccountRole `json:"roles"`
ID string `json:"id"`
Code string `json:"code"`
User AccountMemberUserDetails `json:"user"`
Status string `json:"status"`
Roles []AccountRole `json:"roles,omitempty"`
Policies []Policy `json:"policies,omitempty"`
}

// AccountMemberUserDetails outlines all the personal information about
Expand Down Expand Up @@ -46,9 +47,10 @@ type AccountMemberDetailResponse struct {
// AccountMemberInvitation represents the invitation for a new member to
// the account.
type AccountMemberInvitation struct {
Email string `json:"email"`
Roles []string `json:"roles"`
Status string `json:"status,omitempty"`
Email string `json:"email"`
Roles []string `json:"roles,omitempty"`
Policies []Policy `json:"policies,omitempty"`
Status string `json:"status,omitempty"`
}

// AccountMembers returns all members of an account.
Expand Down Expand Up @@ -81,18 +83,73 @@ func (api *API) AccountMembers(ctx context.Context, accountID string, pageOpts P
//
// API reference: https://api.cloudflare.com/#account-members-add-member
func (api *API) CreateAccountMemberWithStatus(ctx context.Context, accountID string, emailAddress string, roles []string, status string) (AccountMember, error) {
invite := AccountMemberInvitation{
Email: emailAddress,
Roles: roles,
Policies: nil,
Status: status,
}
return api.CreateAccountMemberInternal(ctx, accountID, invite)
}

// CreateAccountMember invites a new member to join an account with roles.
// The member will be placed into "pending" status and receive an email confirmation.
// NOTE: If you are currently enrolled in Domain Scoped Roles, your roles will be converted to policies
// upon member invitation. We recommend upgrading to CreateAccountMemberWithPolicies to use policies.
//
// API reference: https://api.cloudflare.com/#account-members-add-member
func (api *API) CreateAccountMember(ctx context.Context, accountID string, emailAddress string, roles []string) (AccountMember, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're overhauling the invitations, let's update CreateAccountMember method signature to match the experimental client. this will also allow you to cut down on some of the boilerplate code we're repeating and putting into CreateAccountMemberInternal.

(incomplete but hopefully enough to get you started example)

type CreateAccountMemberParams struct {
	Email    string   `json:"email"`
	Roles    []string `json:"roles,omitempty"`
	Policies []Policy `json:"policies,omitempty"`
	Status   string   `json:"status,omitempty"`
}

func (api *API) CreateAccountMember(ctx context.Context, rc *ResourceContainer, params CreateAccountMemberParams) (AccountMember, error) {
	invite := AccountMemberInvitation{
		Email: params.Email,
	}

	if len(params.Roles) > 0 {
		invite.Roles = params.Roles
	}

	// ... etc
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, thanks! Can you confirm that this is the right way to go about things?

  • Re-write CreateAccountMember to match the experimental client
  • Remove the CreateAccountMemberInternal method
  • Re-factor utility methods (CreateAccountMemberWithStatus, CreateAccountMemberWithRoles, etc) to just call CreateAccountMember - the alternative I supposed is just removing the utility methods?

Here's my current CreateAccountMember impl, let me know if it looks okay and the above is correct and I'll push :)

func (api *API) CreateAccountMember(ctx context.Context, rc *ResourceContainer, params CreateAccountMemberParams) (AccountMember, error) {
	if rc.Level != AccountRouteLevel {
		return AccountMember{}, fmt.Errorf(errInvalidResourceContainerAccess, rc.Level)
	}

	invite := AccountMemberInvitation{
		Email:  params.EmailAddress,
		Status: params.Status,
	}

	roles := []AccountRole{}
	for i := 0; i < len(params.Roles); i++ {
		roles = append(roles, AccountRole{ID: invite.Roles[i]})
	}
	err := validateRolesAndPolicies(roles, params.Policies)
	if err != nil {
		return AccountMember{}, err
	}

	if params.Roles != nil {
		invite.Roles = params.Roles
	} else if params.Policies != nil {
		invite.Policies = params.Policies
	}

	uri := fmt.Sprintf("/accounts/%s/members", rc.Identifier)
	res, err := api.makeRequestContext(ctx, http.MethodPost, uri, invite)
	if err != nil {
		return AccountMember{}, err
	}

	var accountMemberListResponse AccountMemberDetailResponse
	err = json.Unmarshal(res, &accountMemberListResponse)
	if err != nil {
		return AccountMember{}, fmt.Errorf("%s: %w", errUnmarshalError, err)
	}

	return accountMemberListResponse.Result, nil
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks spot on! nice one.

invite := AccountMemberInvitation{
Email: emailAddress,
Roles: roles,
Policies: nil,
Status: "",
}
return api.CreateAccountMemberInternal(ctx, accountID, invite)
}

// CreateAccountMemberWithRoles is a terse wrapper around the CreateAccountMember method
// for clarity on what permissions you're granting an AccountMember.
//
// API reference: https://api.cloudflare.com/#account-members-add-member
func (api *API) CreateAccountMemberWithRoles(ctx context.Context, accountID string, emailAddress string, roles []string) (AccountMember, error) {
return api.CreateAccountMember(ctx, accountID, emailAddress, roles)
}

// CreateAccountMemberWithPolicies invites a new member to join your account with policies.
// Policies are the replacement to legacy "roles", which enables the newest feature Domain Scoped Roles.
//
// API documentation will be coming shortly. Blog post: https://blog.cloudflare.com/domain-scoped-roles-ga/
func (api *API) CreateAccountMemberWithPolicies(ctx context.Context, accountID string, emailAddress string, policies []Policy) (AccountMember, error) {
invite := AccountMemberInvitation{
Email: emailAddress,
Roles: nil,
Policies: policies,
Status: "",
}
return api.CreateAccountMemberInternal(ctx, accountID, invite)
}

// CreateAccountMemberInternal allows you to provide a raw AccountMemberInvitation to be processed
// and contains the logic for other CreateAccountMember* methods
func (api *API) CreateAccountMemberInternal(ctx context.Context, accountID string, invite AccountMemberInvitation) (AccountMember, error) {
// make sure we have account
ianmobbs marked this conversation as resolved.
Show resolved Hide resolved
if accountID == "" {
return AccountMember{}, ErrMissingAccountID
}

uri := fmt.Sprintf("/accounts/%s/members", accountID)

newMember := AccountMemberInvitation{
Email: emailAddress,
Roles: roles,
Status: status,
// make sure we have roles OR policies
roles := []AccountRole{}
for i := 0; i < len(invite.Roles); i++ {
roles = append(roles, AccountRole{ID: invite.Roles[i]})
}
res, err := api.makeRequestContext(ctx, http.MethodPost, uri, newMember)
err := validateRolesAndPolicies(roles, invite.Policies)
if err != nil {
return AccountMember{}, err
}

uri := fmt.Sprintf("/accounts/%s/members", accountID)
res, err := api.makeRequestContext(ctx, http.MethodPost, uri, invite)
if err != nil {
return AccountMember{}, err
}
Expand All @@ -106,14 +163,6 @@ func (api *API) CreateAccountMemberWithStatus(ctx context.Context, accountID str
return accountMemberListResponse.Result, nil
}

// CreateAccountMember invites a new member to join an account.
// The member will be placed into "pending" status and receive an email confirmation.
//
// API reference: https://api.cloudflare.com/#account-members-add-member
func (api *API) CreateAccountMember(ctx context.Context, accountID string, emailAddress string, roles []string) (AccountMember, error) {
return api.CreateAccountMemberWithStatus(ctx, accountID, emailAddress, roles, "")
}

// DeleteAccountMember removes a member from an account.
//
// API reference: https://api.cloudflare.com/#account-members-remove-member
Expand All @@ -140,6 +189,11 @@ func (api *API) UpdateAccountMember(ctx context.Context, accountID string, userI
return AccountMember{}, ErrMissingAccountID
}

err := validateRolesAndPolicies(member.Roles, member.Policies)
if err != nil {
return AccountMember{}, err
}

uri := fmt.Sprintf("/accounts/%s/members/%s", accountID, userID)

res, err := api.makeRequestContext(ctx, http.MethodPut, uri, member)
Expand Down Expand Up @@ -183,3 +237,17 @@ func (api *API) AccountMember(ctx context.Context, accountID string, memberID st

return accountMemberResponse.Result, nil
}

// validateRolesAndPolicies ensures either roles or policies are provided in
// CreateAccountMember requests, but not both
func validateRolesAndPolicies(roles []AccountRole, policies []Policy) error {
hasRoles := roles != nil && len(roles) > 0
hasPolicies := policies != nil && len(policies) > 0
hasRolesOrPolicies := hasRoles || hasPolicies
hasRolesAndPolicies := hasRoles && hasPolicies
hasCorrectPermissions := hasRolesOrPolicies && !hasRolesAndPolicies
if !hasCorrectPermissions {
return ErrMissingMemberRolesOrPolicies
}
return nil
}
194 changes: 194 additions & 0 deletions account_members_test.go
View file
Open in desktop
Expand Up @@ -99,6 +99,43 @@ var newUpdatedAccountMemberStruct = AccountMember{
},
}

var mockPolicy = Policy{
ID: "mock-policy-id",
PermissionGroups: []PermissionGroup{PermissionGroup{
ID: "mock-permission-group-id",
Name: "mock-permission-group-name",
Permissions: []Permission{Permission{
ID: "mock-permission-id",
Key: "mock-permission-key",
}},
}},
ResourceGroups: []ResourceGroup{ResourceGroup{
ID: "mock-resource-group-id",
Name: "mock-resource-group-name",
Scope: Scope{
Key: "mock-resource-group-name",
ScopeObjects: []ScopeObject{ScopeObject{
Key: "*",
}},
},
}},
Access: "allow",
}

var expectedNewAccountMemberWithPoliciesStruct = AccountMember{
ID: "new-member-with-polcies-id",
Code: "new-member-with-policies-code",
User: AccountMemberUserDetails{
ID: "new-member-with-policies-user-id",
FirstName: "John",
LastName: "Appleseed",
Email: "user@example.com",
TwoFactorAuthenticationEnabled: false,
},
Status: "accepted",
Policies: []Policy{mockPolicy},
}

func TestAccountMembers(t *testing.T) {
setup()
defer teardown()
Expand Down Expand Up @@ -274,6 +311,80 @@ func TestCreateAccountMember(t *testing.T) {
}
}

func TestCreateAccountMemberWithPolicies(t *testing.T) {
setup()
defer teardown()

handler := func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, http.MethodPost, r.Method, "Expected method 'POST', got %s", r.Method)
w.Header().Set("content-type", "application/json")
fmt.Fprintf(w, `{
"success": true,
"errors": [],
"messages": [],
"result": {
"id": "new-member-with-polcies-id",
"code": "new-member-with-policies-code",
"user": {
"id": "new-member-with-policies-user-id",
"first_name": "John",
"last_name": "Appleseed",
"email": "user@example.com",
"two_factor_authentication_enabled": false
},
"status": "accepted",
"policies": [{
"id": "mock-policy-id",
"permission_groups": [{
"id": "mock-permission-group-id",
"name": "mock-permission-group-name",
"permissions": [{
"id": "mock-permission-id",
"key": "mock-permission-key"
}]
}],
"resource_groups": [{
"id": "mock-resource-group-id",
"name": "mock-resource-group-name",
"scope": {
"key": "mock-resource-group-name",
"objects": [{
"key": "*"
}]
}
}],
"access": "allow"
}]
}
}
`)
}

mux.HandleFunc("/accounts/01a7362d577a6c3019a474fd6f485823/members", handler)

actual, err := client.CreateAccountMemberWithPolicies(context.Background(), "01a7362d577a6c3019a474fd6f485823", "user@example.com", []Policy{mockPolicy})

if assert.NoError(t, err) {
assert.Equal(t, expectedNewAccountMemberWithPoliciesStruct, actual)
}
}

func TestCreateAccountMemberWithRolesAndPoliciesErr(t *testing.T) {
setup()
defer teardown()

_, err := client.CreateAccountMemberInternal(context.Background(), "fake", AccountMemberInvitation{
Email: "user@example.com",
Roles: []string{"fake-role-id"},
Policies: []Policy{mockPolicy},
Status: "active",
})

if assert.Error(t, err) {
assert.Equal(t, err, ErrMissingMemberRolesOrPolicies)
}
}

func TestCreateAccountMemberWithoutAccountID(t *testing.T) {
setup()
defer teardown()
Expand Down Expand Up @@ -342,6 +453,89 @@ func TestUpdateAccountMember(t *testing.T) {
}
}

func TestUpdateAccountMemberWithPolicies(t *testing.T) {
setup()
defer teardown()

handler := func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, http.MethodPut, r.Method, "Expected method 'PUT', got %s", r.Method)
w.Header().Set("content-type", "application/json")
fmt.Fprintf(w, `{
"success": true,
"errors": [],
"messages": [],
"result": {
"id": "new-member-with-polcies-id",
"code": "new-member-with-policies-code",
"user": {
"id": "new-member-with-policies-user-id",
"first_name": "John",
"last_name": "Appleseed",
"email": "user@example.com",
"two_factor_authentication_enabled": false
},
"status": "accepted",
"policies": [{
"id": "mock-policy-id",
"permission_groups": [{
"id": "mock-permission-group-id",
"name": "mock-permission-group-name",
"permissions": [{
"id": "mock-permission-id",
"key": "mock-permission-key"
}]
}],
"resource_groups": [{
"id": "mock-resource-group-id",
"name": "mock-resource-group-name",
"scope": {
"key": "mock-resource-group-name",
"objects": [{
"key": "*"
}]
}
}],
"access": "allow"
}]
},
"result_info": {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
}
}`)
}

mux.HandleFunc("/accounts/01a7362d577a6c3019a474fd6f485823/members/new-member-with-polcies-id", handler)

actual, err := client.UpdateAccountMember(context.Background(), "01a7362d577a6c3019a474fd6f485823", "new-member-with-polcies-id", expectedNewAccountMemberWithPoliciesStruct)

if assert.NoError(t, err) {
assert.Equal(t, expectedNewAccountMemberWithPoliciesStruct, actual)
}
}

func UpdateAccountMemberWithRolesAndPoliciesErr(t *testing.T) {
setup()
defer teardown()

_, err := client.UpdateAccountMember(context.Background(),
"01a7362d577a6c3019a474fd6f485823",
"",
AccountMember{
Roles: []AccountRole{AccountRole{
ID: "some-role",
}},
Policies: []Policy{mockPolicy},
},
)

if assert.Error(t, err) {
assert.Equal(t, err, ErrMissingMemberRolesOrPolicies)
}
}

func TestUpdateAccountMemberWithoutAccountID(t *testing.T) {
setup()
defer teardown()
Expand Down
2 changes: 2 additions & 0 deletions errors.go
View file
Open in desktop
Expand Up @@ -17,6 +17,7 @@ const (
errUnmarshalErrorBody = "error unmarshalling the JSON response error body"
errRequestNotSuccessful = "error reported by API"
errMissingAccountID = "required missing account ID"
errMissingMemberRolesOrPolicies = "account member must be created with roles or policies (not both)"
ianmobbs marked this conversation as resolved.
Show resolved Hide resolved
errMissingZoneID = "required missing zone ID"
errMissingAccountOrZoneID = "either account ID or zone ID must be provided"
errAccountIDAndZoneIDAreMutuallyExclusive = "account ID and zone ID are mutually exclusive"
Expand All @@ -41,6 +42,7 @@ var (
ErrAccountIDOrZoneIDAreRequired = errors.New(errMissingAccountOrZoneID)
ErrAccountIDAndZoneIDAreMutuallyExclusive = errors.New(errAccountIDAndZoneIDAreMutuallyExclusive)
ErrMissingResourceIdentifier = errors.New(errMissingResourceIdentifier)
ErrMissingMemberRolesOrPolicies = errors.New(errMissingMemberRolesOrPolicies)
)

type ErrorType string
Expand Down