Merge pull request #36 from ConductorOne/lauren/account-member-roll-all

Paginate through account members, store member id on profile

Author: laurenleach

.github/workflows/ci.yaml

@@ -52,6 +52,8 @@ jobs:
         CONNECTOR_ENTITLEMENT: 'role:1963e6e3aca5ac9a7a91609a0040ab02:member'
         CONNECTOR_PRINCIPAL: '9d9a62a5b834a8c9c5cf43cd234dfd4a'
         CONNECTOR_PRINCIPAL_TYPE: 'user'
+        # Fake ID for failure testing
+        FAKE_PRINCIPAL_ID: 'fake-id-that-does-not-exist-12345'
     steps:
       - name: Install Go
         uses: actions/setup-go@v5
@@ -74,6 +76,19 @@ jobs:
         run: |
          ./baton-cloudflare
           baton grants --entitlement="${{ env.CONNECTOR_ENTITLEMENT }}" --output-format=json | jq --exit-status ".grants[].principal.id.resource == \"${{ env.CONNECTOR_PRINCIPAL }}\""
+      - name: Test grant failure with fake ID
+        run: |
+          set +e  # Don't exit on error
+          ./baton-cloudflare --grant-entitlement ${{ env.CONNECTOR_ENTITLEMENT }} --grant-principal ${{ env.FAKE_PRINCIPAL_ID }} --grant-principal-type ${{ env.CONNECTOR_PRINCIPAL_TYPE }}
+          exit_code=$?
+          set -e  # Re-enable exit on error
+
+          if [ $exit_code -eq 0 ]; then
+            echo "ERROR: Grant with fake ID should have failed but succeeded"
+            exit 1
+          else
+            echo "SUCCESS: Grant with fake ID failed as expected (exit code: $exit_code)"
+          fi
       - name: Revoke grants
         run: |
           ./baton-cloudflare

.golangci.yml

@@ -71,7 +71,6 @@ linters:
     - durationcheck # check for two durations multiplied together
     - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
     - exhaustive # check exhaustiveness of enum switch statements
-    - exportloopref # checks for pointers to enclosing loop variables
     - forbidigo # Forbids identifiers
     - gochecknoinits # Checks that no init functions are present in Go code
     - goconst # Finds repeated strings that could be replaced by a constant

pkg/connector/role.go

@@ -228,18 +228,28 @@ func (r *roleResourceType) Grant(ctx context.Context, principal *v2.Resource, en
 		return nil, fmt.Errorf("baton-cloudflare: only users can be granted role membership")
 	}
 
-	memberId, err := getMemberId(ctx, r, userId)
+	userTrait, err := rs.GetUserTrait(principal)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("baton-cloudflare: user trait not found on principal")
+	}
+
+	memberId, found := rs.GetProfileStringValue(userTrait.GetProfile(), memberIdProfileKey)
+	if !found || memberId == "" {
+		memberId, err = getMemberId(ctx, r, userId)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	account, err := r.GetAccountMember(ctx, r.accountId, memberId)
 	if err != nil {
 		return nil, fmt.Errorf("error: %s", err.Error())
 	}
 
-	roles := []cloudflare.AccountRole{{
-		ID: roleId},
+	roles := []cloudflare.AccountRole{
+		{
+			ID: roleId,
+		},
 	}
 	for _, role := range account.Result.Roles {
 		if role.ID == roleId {
@@ -248,7 +258,7 @@ func (r *roleResourceType) Grant(ctx context.Context, principal *v2.Resource, en
 				zap.String("principal_id", principal.Id.String()),
 				zap.String("principal_type", principal.Id.ResourceType),
 			)
-			return nil, fmt.Errorf("cloudflare-connector: user %s already has this role", principal.DisplayName)
+			return annotations.New(&v2.GrantAlreadyExists{}), nil
 		}
 
 		roles = append(roles, cloudflare.AccountRole{
@@ -350,18 +360,34 @@ func (r *roleResourceType) UpdateAccountMember(ctx context.Context, accountID, m
 }
 
 func getMemberId(ctx context.Context, r *roleResourceType, userId string) (string, error) {
-	memberUsers, _, err := r.client.AccountMembers(ctx, r.accountId, cloudflare.PaginationOptions{})
-	if err != nil {
-		return "", wrapError(err, "failed to list user members")
-	}
+	processedMemberCount := 0
+	perPage := 50
+	page := 1
+
+	for {
+		memberUsers, resp, err := r.client.AccountMembers(ctx, r.accountId, cloudflare.PaginationOptions{
+			Page:    page,
+			PerPage: perPage,
+		})
+		if err != nil {
+			return "", wrapError(err, "failed to list user members")
+		}
 
-	for _, memberUser := range memberUsers {
-		if memberUser.User.ID == userId {
-			return memberUser.ID, nil
+		for _, memberUser := range memberUsers {
+			if memberUser.User.ID == userId {
+				return memberUser.ID, nil
+			}
+		}
+
+		processedMemberCount += perPage
+		if processedMemberCount >= resp.Total {
+			break
 		}
+
+		page++
 	}
 
-	return "", nil
+	return "", fmt.Errorf("cloudflare-connector: account member not found for user with id: %s", userId)
 }
 
 func (r *roleResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotations.Annotations, error) {
@@ -379,9 +405,18 @@ func (r *roleResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotat
 
 	userId := principal.Id.Resource
 	roleId := entitlement.Resource.Id.Resource
-	memberId, err := getMemberId(ctx, r, userId)
+
+	userTrait, err := rs.GetUserTrait(principal)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("baton-cloudflare: user trait not found on principal")
+	}
+
+	memberId, found := rs.GetProfileStringValue(userTrait.GetProfile(), memberIdProfileKey)
+	if !found || memberId == "" {
+		memberId, err = getMemberId(ctx, r, userId)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	account, err := r.GetAccountMember(ctx, r.accountId, memberId)
@@ -407,7 +442,7 @@ func (r *roleResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotat
 			zap.String("principal_id", principal.Id.String()),
 			zap.String("principal_type", principal.Id.ResourceType),
 		)
-		return nil, fmt.Errorf("cloudflare-connector: user %s does not have this role", principal.DisplayName)
+		return annotations.New(&v2.GrantAlreadyRevoked{}), nil
 	}
 
 	member, err := r.UpdateAccountMember(ctx, r.accountId, memberId, cloudflare.AccountMember{

pkg/connector/user.go

@@ -11,6 +11,8 @@ import (
 	rs "github.com/conductorone/baton-sdk/pkg/types/resource"
 )
 
+const memberIdProfileKey = "member_id"
+
 type UserResourceType struct {
 	resourceType *v2.ResourceType
 	client       *cloudflare.API
@@ -26,10 +28,11 @@ func userResource(member cloudflare.AccountMember) (*v2.Resource, error) {
 	firstName := user.FirstName
 	lastName := user.LastName
 	profile := map[string]interface{}{
-		"login":      user.Email,
-		"first_name": firstName,
-		"last_name":  lastName,
-		"email":      user.Email,
+		"login":            user.Email,
+		"first_name":       firstName,
+		"last_name":        lastName,
+		"email":            user.Email,
+		memberIdProfileKey: member.ID,
 	}
 
 	userTraits := []rs.UserTraitOption{