From 5171907e7a6f68a4a492eedee7cabb058630ad5d Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Wed, 17 Jun 2026 06:27:01 +0000 Subject: [PATCH 1/5] =?UTF-8?q?feat:=20Sparse=20ACLs=20Phase=201=20?= =?UTF-8?q?=E2=80=94=20permission-set/account=20scope=20bindings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshape Identity Center permission-set assignments into the c1 Sparse-ACL contract: a permission_set role type, a permission_set_assignment binding type carrying TRAIT_SCOPE_BINDING with ScopeBindingTrait{role,scope}, an "assigned" entitlement, account ChildResourceType wiring, and trait-reading Grant/Revoke that reuse the existing Create/DeleteAccountAssignment core (HYBRID coexistence: the legacy account-entitlement path is retained). Co-authored-by: c1-squire-dev[bot] --- pkg/connector/account.go | 82 +++++---- pkg/connector/connector.go | 11 +- pkg/connector/permission_set.go | 113 +++++++++++++ pkg/connector/permission_set_assignment.go | 186 +++++++++++++++++++++ pkg/connector/resource_types.go | 46 +++++ 5 files changed, 401 insertions(+), 37 deletions(-) create mode 100644 pkg/connector/permission_set.go create mode 100644 pkg/connector/permission_set_assignment.go diff --git a/pkg/connector/account.go b/pkg/connector/account.go index f4aca4cd..a2520736 100644 --- a/pkg/connector/account.go +++ b/pkg/connector/account.go @@ -183,6 +183,11 @@ func (o *accountResourceType) List(ctx context.Context, _ *v2.ResourceId, opts r accountId, []resourceSdk.AppTraitOption{resourceSdk.WithAppProfile(profile)}, resourceSdk.WithAnnotation(annos), + // Sparse ACLs: advertise the scope-binding type as a child so the SDK + // crawls per-(account, permission set) bindings under each account. + resourceSdk.WithAnnotation(&v2.ChildResourceType{ + ResourceTypeId: resourceTypePermissionSetAssignment.Id, + }), ) if err != nil { return nil, nil, err @@ -551,32 +556,48 @@ func (o *accountResourceType) verifyAccountStatus(ctx context.Context, accountID return false, nil } -func (o *accountResourceType) Grant(ctx context.Context, principal *v2.Resource, entitlement *v2.Entitlement) (annotations.Annotations, error) { - principalType := awsSsoAdminTypes.PrincipalType("") - principalId := "" +// resolveSSOPrincipal maps an sso_user / sso_group principal resource to the +// AWS PrincipalType + native principal id used by Create/DeleteAccountAssignment. +func resolveSSOPrincipal(principal *v2.Resource) (awsSsoAdminTypes.PrincipalType, string, error) { switch principal.Id.ResourceType { case resourceTypeSSOUser.Id: - principalType = awsSsoAdminTypes.PrincipalTypeUser ssoUserID, err := ssoUserIdFromARN(principal.Id.Resource) if err != nil { - return nil, err + return "", "", err } - principalId = ssoUserID + return awsSsoAdminTypes.PrincipalTypeUser, ssoUserID, nil case resourceTypeSSOGroup.Id: - principalType = awsSsoAdminTypes.PrincipalTypeGroup ssoGroupID, err := ssoGroupIdFromARN(principal.Id.Resource) if err != nil { - return nil, err + return "", "", err } - principalId = ssoGroupID + return awsSsoAdminTypes.PrincipalTypeGroup, ssoGroupID, nil default: - return nil, fmt.Errorf("baton-aws: invalid principal resource type: %s", principal.Id.ResourceType) + return "", "", fmt.Errorf("baton-aws: invalid principal resource type: %s", principal.Id.ResourceType) } +} +func (o *accountResourceType) Grant(ctx context.Context, principal *v2.Resource, entitlement *v2.Entitlement) (annotations.Annotations, error) { binding := &PermissionSetBinding{} if err := binding.UnmarshalText([]byte(entitlement.Id)); err != nil { return nil, err } + return o.provisionAssignment(ctx, binding.AccountID, binding.PermissionSetId, principal) +} + +// provisionAssignment creates an Identity Center account assignment for the given +// (account, permission set, principal). It is the shared core of the account-entitlement +// Grant path and the scope-binding Grant path; both recover (accountID, permissionSetArn) +// from their own source (entitlement id vs ScopeBindingTrait) and call this with identical +// AWS semantics (status verification, CreateAccountAssignment at TargetType=AWS_ACCOUNT, +// and status polling to terminal state). +func (o *accountResourceType) provisionAssignment(ctx context.Context, accountID string, permissionSetArn string, principal *v2.Resource) (annotations.Annotations, error) { + principalType, principalId, err := resolveSSOPrincipal(principal) + if err != nil { + return nil, err + } + + binding := &PermissionSetBinding{AccountID: accountID, PermissionSetId: permissionSetArn} // try to verify the account status before creating the assignment // if we have permissions and the account is suspended, fail to avoid ConflictException @@ -725,37 +746,26 @@ func (o *accountResourceType) checkDeleteAccountAssignmentStatus(ctx context.Con } func (o *accountResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotations.Annotations, error) { - principal := grant.Principal - entitlement := grant.Entitlement - principalType := awsSsoAdminTypes.PrincipalType("") - principalId := "" - switch principal.Id.ResourceType { - case resourceTypeSSOUser.Id: - principalType = awsSsoAdminTypes.PrincipalTypeUser - ssoUserID, err := ssoUserIdFromARN(principal.Id.Resource) - if err != nil { - return nil, err - } - - principalId = ssoUserID - - case resourceTypeSSOGroup.Id: - principalType = awsSsoAdminTypes.PrincipalTypeGroup - ssoGroupID, err := ssoGroupIdFromARN(principal.Id.Resource) - if err != nil { - return nil, err - } - - principalId = ssoGroupID - default: - return nil, fmt.Errorf("baton-aws: invalid principal resource type: %s", principal.Id.ResourceType) + binding := &PermissionSetBinding{} + if err := binding.UnmarshalText([]byte(grant.Entitlement.Id)); err != nil { + return nil, err } + return o.deprovisionAssignment(ctx, binding.AccountID, binding.PermissionSetId, grant.Principal) +} - binding := &PermissionSetBinding{} - if err := binding.UnmarshalText([]byte(entitlement.Id)); err != nil { +// deprovisionAssignment deletes an Identity Center account assignment for the given +// (account, permission set, principal). It is the shared core of the account-entitlement +// Revoke path and the scope-binding Revoke path, with identical AWS semantics +// (status verification, DeleteAccountAssignment at TargetType=AWS_ACCOUNT, status polling, +// and GrantAlreadyRevoked idempotency on 404). +func (o *accountResourceType) deprovisionAssignment(ctx context.Context, accountID string, permissionSetArn string, principal *v2.Resource) (annotations.Annotations, error) { + principalType, principalId, err := resolveSSOPrincipal(principal) + if err != nil { return nil, err } + binding := &PermissionSetBinding{AccountID: accountID, PermissionSetId: permissionSetArn} + // try to verify the account status before deleting the assignment // if we have permissions and the account is suspended, fail to avoid ConflictException // if we don't have permissions, warn but continue (backward compatibility) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index f3716ca2..efba0d80 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -451,7 +451,14 @@ func (c *AWS) ResourceSyncers(ctx context.Context) []connectorbuilder.ResourceSy if c.orgsEnabled && c.ssoEnabled { l.Debug("orgsEnabled. creating accountBuilder") - rs = append(rs, accountBuilder(c.orgClient, c.roleARN, c.ssoAdminClient, c.identityInstance, c.ssoRegion, c.identityStoreClient)) + acct := accountBuilder(c.orgClient, c.roleARN, c.ssoAdminClient, c.identityInstance, c.ssoRegion, c.identityStoreClient) + rs = append(rs, + acct, + // Sparse ACLs (Cloud Infrastructure Access): permission set as role, and the + // per-(account, permission set) scope-binding. Both are OptInRequired. + permissionSetBuilder(c.ssoAdminClient, c.identityInstance), + permissionSetAssignmentBuilder(acct), + ) } if c.syncSecrets { @@ -485,6 +492,8 @@ func (d *defaultCapabilitiesBuilder) ResourceSyncers(_ context.Context) []connec ssoUserBuilder("", nil, nil, nil, nil), ssoGroupBuilder("", nil, nil, nil), accountBuilder(nil, "", nil, nil, "", nil), + permissionSetBuilder(nil, nil), + permissionSetAssignmentBuilder(accountBuilder(nil, "", nil, nil, "", nil)), accountIAMBuilder(nil, nil, nil), secretBuilder(nil, nil), } diff --git a/pkg/connector/permission_set.go b/pkg/connector/permission_set.go new file mode 100644 index 00000000..06030fa6 --- /dev/null +++ b/pkg/connector/permission_set.go @@ -0,0 +1,113 @@ +package connector + +import ( + "context" + "fmt" + + awsSdk "github.com/aws/aws-sdk-go-v2/aws" + awsSsoAdmin "github.com/aws/aws-sdk-go-v2/service/ssoadmin" + awsSsoAdminTypes "github.com/aws/aws-sdk-go-v2/service/ssoadmin/types" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/pagination" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" +) + +// permissionSetRoleID is the SINGLE place an Identity Center permission-set role id is +// constructed. It is called by both the permission_set role-resource builder (its resource +// id) and the scope-binding trait's role_id, so the two cannot drift — a drift would make +// c1 ingest a dangling role reference and silently drop the RoleScopeBindingRelationship. +// The id is the bare permission-set ARN. Round-trips through c1's ParseV2ExternalID +// (SplitN("::",2)) unchanged, since the ARN is carried whole in the resource half. +func permissionSetRoleID(permissionSetArn string) string { + return permissionSetArn +} + +type permissionSetResourceType struct { + resourceType *v2.ResourceType + ssoAdminClient *awsSsoAdmin.Client + identityInstance *awsSsoAdminTypes.InstanceMetadata +} + +func (o *permissionSetResourceType) ResourceType(_ context.Context) *v2.ResourceType { + return o.resourceType +} + +func (o *permissionSetResourceType) List(ctx context.Context, _ *v2.ResourceId, opts resourceSdk.SyncOpAttrs) ([]*v2.Resource, *resourceSdk.SyncOpResults, error) { + bag := &pagination.Bag{} + if err := bag.Unmarshal(opts.PageToken.Token); err != nil { + return nil, nil, err + } + if bag.Current() == nil { + bag.Push(pagination.PageState{ResourceTypeID: resourceTypePermissionSet.Id}) + } + + input := &awsSsoAdmin.ListPermissionSetsInput{ + InstanceArn: o.identityInstance.InstanceArn, + } + if bag.PageToken() != "" { + input.NextToken = awsSdk.String(bag.PageToken()) + } + + resp, err := o.ssoAdminClient.ListPermissionSets(ctx, input) + if err != nil { + return nil, nil, wrapAWSError(fmt.Errorf("baton-aws: ssoadmin.ListPermissionSets failed: %w", err)) + } + + rv := make([]*v2.Resource, 0, len(resp.PermissionSets)) + for _, psArn := range resp.PermissionSets { + descResp, err := o.ssoAdminClient.DescribePermissionSet(ctx, &awsSsoAdmin.DescribePermissionSetInput{ + InstanceArn: o.identityInstance.InstanceArn, + PermissionSetArn: awsSdk.String(psArn), + }) + if err != nil { + return nil, nil, wrapAWSError(fmt.Errorf("baton-aws: ssoadmin.DescribePermissionSet failed: %w", err)) + } + resource, err := permissionSetResource(descResp.PermissionSet) + if err != nil { + return nil, nil, err + } + rv = append(rv, resource) + } + + if resp.NextToken != nil { + token, err := bag.NextToken(*resp.NextToken) + if err != nil { + return rv, nil, err + } + return rv, &resourceSdk.SyncOpResults{NextPageToken: token}, nil + } + return rv, nil, nil +} + +// permissionSetResource builds the role resource for a permission set. Its resource id is +// permissionSetRoleID(arn) so it byte-matches the scope-binding trait's role_id. +func permissionSetResource(ps *awsSsoAdminTypes.PermissionSet) (*v2.Resource, error) { + arnStr := awsSdk.ToString(ps.PermissionSetArn) + return resourceSdk.NewRoleResource( + awsSdk.ToString(ps.Name), + resourceTypePermissionSet, + permissionSetRoleID(arnStr), + nil, + resourceSdk.WithAnnotation(&v2.V1Identifier{Id: arnStr}), + resourceSdk.WithDescription(awsSdk.ToString(ps.Description)), + ) +} + +// Entitlements is a no-op: permission_set is a pure role catalog node +// (SkipEntitlementsAndGrants). The "assigned" entitlement lives on the binding. +func (o *permissionSetResourceType) Entitlements(_ context.Context, _ *v2.Resource, _ resourceSdk.SyncOpAttrs) ([]*v2.Entitlement, *resourceSdk.SyncOpResults, error) { + return nil, nil, nil +} + +// Grants is a no-op: grants are emitted on the binding's "assigned" entitlement. +func (o *permissionSetResourceType) Grants(_ context.Context, _ *v2.Resource, _ resourceSdk.SyncOpAttrs) ([]*v2.Grant, *resourceSdk.SyncOpResults, error) { + return nil, nil, nil +} + +func permissionSetBuilder(ssoAdminClient *awsSsoAdmin.Client, identityInstance *awsSsoAdminTypes.InstanceMetadata) *permissionSetResourceType { + return &permissionSetResourceType{ + resourceType: resourceTypePermissionSet, + ssoAdminClient: ssoAdminClient, + identityInstance: identityInstance, + } +} diff --git a/pkg/connector/permission_set_assignment.go b/pkg/connector/permission_set_assignment.go new file mode 100644 index 00000000..e56fa54d --- /dev/null +++ b/pkg/connector/permission_set_assignment.go @@ -0,0 +1,186 @@ +package connector + +import ( + "context" + "fmt" + + awsSdk "github.com/aws/aws-sdk-go-v2/aws" + awsSsoAdmin "github.com/aws/aws-sdk-go-v2/service/ssoadmin" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/annotations" + entitlementSdk "github.com/conductorone/baton-sdk/pkg/types/entitlement" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" +) + +// permissionSetAssignmentEntitlement is the binding's assignment entitlement slug. It MUST +// be exactly "assigned" — c1's JIT and reverse-resolution paths hardcode +// ScopeRoleAssignedEntitlementSlug = "assigned". +const permissionSetAssignmentEntitlement = "assigned" + +// permissionSetAssignmentObjectID is the SINGLE place the binding object id is constructed. +// Shape is "-", byte-identical to c1's JIT-fabricated id +// (scope_role_jit.go scopeRoleBindingExternalID = role + "-" + scope), so a JIT'd binding +// and the later sync-discovered binding reconcile to one RoleScopeBindingRelationship row. +// The embedded '-', ':' and '/' in the ARN are harmless: c1 never splits this id back into +// (role, scope) — it is an opaque external identity; role/scope are always recovered from +// the ScopeBindingTrait. +func permissionSetAssignmentObjectID(permissionSetArn string, accountID string) string { + return permissionSetArn + "-" + accountID +} + +type permissionSetAssignmentResourceType struct { + resourceType *v2.ResourceType + // account is reused for AWS clients, session-cached permission-set lookups, status + // polling, grant construction, and the shared provision/deprovision core — keeping the + // scope-binding provisioning path byte-identical to the legacy account-entitlement path. + account *accountResourceType +} + +func (o *permissionSetAssignmentResourceType) ResourceType(_ context.Context) *v2.ResourceType { + return o.resourceType +} + +// permissionSetAssignmentResource builds the (permission set → account) scope-binding +// resource. The trait's role_id byte-matches the permission_set builder's resource id and +// scope_resource_id byte-matches the account builder's resource id (both load-bearing for +// c1 reference resolution). +func permissionSetAssignmentResource(permissionSetArn string, permissionSetName string, accountID string, accountResourceID *v2.ResourceId) (*v2.Resource, error) { + roleScopeRoleID := &v2.ResourceId{ + ResourceType: resourceTypePermissionSet.Id, + Resource: permissionSetRoleID(permissionSetArn), + } + scopeResourceID := &v2.ResourceId{ + ResourceType: resourceTypeAccount.Id, + Resource: accountID, + } + return resourceSdk.NewScopeBindingResource( + fmt.Sprintf("%s on %s", permissionSetName, accountID), + resourceTypePermissionSetAssignment, + permissionSetAssignmentObjectID(permissionSetArn, accountID), + []resourceSdk.ScopeBindingTraitOption{ + resourceSdk.WithRoleScopeRoleId(roleScopeRoleID), + resourceSdk.WithRoleScopeResourceId(scopeResourceID), + }, + resourceSdk.WithParentResourceID(accountResourceID), + ) +} + +func (o *permissionSetAssignmentResourceType) List(ctx context.Context, parentResourceID *v2.ResourceId, opts resourceSdk.SyncOpAttrs) ([]*v2.Resource, *resourceSdk.SyncOpResults, error) { + // Bindings live under each account; only crawl when listed as a child of an account. + if parentResourceID == nil || parentResourceID.ResourceType != resourceTypeAccount.Id { + return nil, nil, nil + } + accountID := parentResourceID.Resource + + permissionSetIDs, err := o.account.getOrFetchPermissionSetIDs(ctx, opts.Session, accountID) + if err != nil { + return nil, nil, err + } + + rv := make([]*v2.Resource, 0, len(permissionSetIDs)) + for _, psArn := range permissionSetIDs { + ps, err := o.account.getPermissionSetWithCache(ctx, opts.Session, psArn) + if err != nil { + return nil, nil, err + } + resource, err := permissionSetAssignmentResource( + awsSdk.ToString(ps.PermissionSetArn), + awsSdk.ToString(ps.Name), + accountID, + parentResourceID, + ) + if err != nil { + return nil, nil, err + } + rv = append(rv, resource) + } + + return rv, nil, nil +} + +// assignedEntitlement returns the binding's "assigned" entitlement. Grantable to SSO users +// and groups — AWS CreateAccountAssignment accepts PrincipalType=GROUP natively. +func assignedEntitlement(resource *v2.Resource) *v2.Entitlement { + return entitlementSdk.NewAssignmentEntitlement( + resource, + permissionSetAssignmentEntitlement, + entitlementSdk.WithGrantableTo(resourceTypeSSOUser, resourceTypeSSOGroup), + ) +} + +func (o *permissionSetAssignmentResourceType) Entitlements(_ context.Context, resource *v2.Resource, _ resourceSdk.SyncOpAttrs) ([]*v2.Entitlement, *resourceSdk.SyncOpResults, error) { + return []*v2.Entitlement{assignedEntitlement(resource)}, nil, nil +} + +func (o *permissionSetAssignmentResourceType) Grants(ctx context.Context, resource *v2.Resource, opts resourceSdk.SyncOpAttrs) ([]*v2.Grant, *resourceSdk.SyncOpResults, error) { + scope, err := resourceSdk.GetScopeBindingTrait(resource) + if err != nil { + return nil, nil, fmt.Errorf("baton-aws: failed to read scope binding trait: %w", err) + } + accountID := scope.GetScopeResourceId().GetResource() + permissionSetArn := scope.GetRoleId().GetResource() + + entitlement := assignedEntitlement(resource) + + input := &awsSsoAdmin.ListAccountAssignmentsInput{ + AccountId: awsSdk.String(accountID), + InstanceArn: o.account.identityInstance.InstanceArn, + PermissionSetArn: awsSdk.String(permissionSetArn), + } + if opts.PageToken.Token != "" { + input.NextToken = awsSdk.String(opts.PageToken.Token) + } + + resp, err := o.account.ssoAdminClient.ListAccountAssignments(ctx, input) + if err != nil { + return nil, nil, wrapAWSError(fmt.Errorf("baton-aws: ssoadmin.ListAccountAssignments failed: %w", err)) + } + + rv := make([]*v2.Grant, 0, len(resp.AccountAssignments)) + for _, assignment := range resp.AccountAssignments { + // Reuses the account builder's grant construction: direct grant for users, plus + // GrantExpandable{sso_group::member} for groups. Sparse and grant-expansion + // coexist on the same grant. + grant := o.account.buildGrantFromAssignment(entitlement, assignment) + if grant != nil { + rv = append(rv, grant) + } + } + + if resp.NextToken != nil && *resp.NextToken != "" { + return rv, &resourceSdk.SyncOpResults{NextPageToken: *resp.NextToken}, nil + } + return rv, nil, nil +} + +// Grant reads (account, permission set) from the ScopeBindingTrait on the entitlement's +// resource — never by parsing the binding object id — then provisions via the shared +// account-assignment core. +func (o *permissionSetAssignmentResourceType) Grant(ctx context.Context, principal *v2.Resource, entitlement *v2.Entitlement) (annotations.Annotations, error) { + scope, err := resourceSdk.GetScopeBindingTrait(entitlement.GetResource()) + if err != nil { + return nil, fmt.Errorf("baton-aws: failed to read scope binding trait: %w", err) + } + accountID := scope.GetScopeResourceId().GetResource() + permissionSetArn := scope.GetRoleId().GetResource() + return o.account.provisionAssignment(ctx, accountID, permissionSetArn, principal) +} + +// Revoke reads (account, permission set) from the ScopeBindingTrait on the granted +// entitlement's resource, then deprovisions via the shared account-assignment core. +func (o *permissionSetAssignmentResourceType) Revoke(ctx context.Context, grant *v2.Grant) (annotations.Annotations, error) { + scope, err := resourceSdk.GetScopeBindingTrait(grant.GetEntitlement().GetResource()) + if err != nil { + return nil, fmt.Errorf("baton-aws: failed to read scope binding trait: %w", err) + } + accountID := scope.GetScopeResourceId().GetResource() + permissionSetArn := scope.GetRoleId().GetResource() + return o.account.deprovisionAssignment(ctx, accountID, permissionSetArn, grant.GetPrincipal()) +} + +func permissionSetAssignmentBuilder(account *accountResourceType) *permissionSetAssignmentResourceType { + return &permissionSetAssignmentResourceType{ + resourceType: resourceTypePermissionSetAssignment, + account: account, + } +} diff --git a/pkg/connector/resource_types.go b/pkg/connector/resource_types.go index c221ec3b..bbe955d1 100644 --- a/pkg/connector/resource_types.go +++ b/pkg/connector/resource_types.go @@ -170,4 +170,50 @@ var ( ), ), } + + // resourceTypePermissionSet is the Identity Center permission set modeled as a + // role (Sparse ACLs / Cloud Infrastructure Access). It is a pure role catalog + // node: its id is the bare permission-set ARN (see permissionSetRoleID), which + // the scope-binding trait's role_id must byte-match. + resourceTypePermissionSet = &v2.ResourceType{ + Id: "permission_set", + DisplayName: "Permission Set", + Traits: []v2.ResourceType_Trait{v2.ResourceType_TRAIT_ROLE}, + Annotations: annotations.New( + &v2.SkipEntitlementsAndGrants{}, + &v2.OptInRequired{}, + capabilityPermissions( + "sso:ListInstances", + "sso:ListPermissionSets", + "sso:DescribePermissionSet", + ), + ), + } + + // resourceTypePermissionSetAssignment is the (permission set → account) binding + // carrying TRAIT_SCOPE_BINDING. This single trait is what makes the AWS app + // ingest as SPARSE/HYBRID and lights up the RoleScopeBindingRelationship / JIT / + // UAR / JML pipelines. Provisioning rides the existing Create/DeleteAccountAssignment + // path at the account scope. + resourceTypePermissionSetAssignment = &v2.ResourceType{ + Id: "permission_set_assignment", + DisplayName: "Permission Set Assignment", + Traits: []v2.ResourceType_Trait{v2.ResourceType_TRAIT_SCOPE_BINDING}, + Annotations: annotations.New( + &v2.OptInRequired{}, + capabilityPermissions( + // Read + "sso:ListInstances", + "sso:ListPermissionSetsProvisionedToAccount", + "sso:DescribePermissionSet", + "sso:ListAccountAssignments", + // Provision + "sso:CreateAccountAssignment", + "sso:DeleteAccountAssignment", + "sso:DescribeAccountAssignmentCreationStatus", + "sso:DescribeAccountAssignmentDeletionStatus", + "organizations:DescribeAccount", + ), + ), + } ) From fe357da424facf0bdbc92dbe248e1ed2d382bb2e Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Wed, 17 Jun 2026 06:30:05 +0000 Subject: [PATCH 2/5] test: scope-binding id round-trip, drift guard, slug; regen capabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add unit tests for the §3.6 gate invariants: object-id shape with a real permission-set ARN, byte-match to c1's JIT fabrication, role-id drift guard between the role resource and the trait, trait byte-match, the "assigned" slug, and the ResourceIDToString/ParseV2ExternalID round-trip. Regenerate baton_capabilities.json to declare permission_set (TRAIT_ROLE) and permission_set_assignment (TRAIT_SCOPE_BINDING + CAPABILITY_PROVISION, optInRequired). Co-authored-by: c1-squire-dev[bot] --- baton_capabilities.json | 130 ++++++++++++++++ .../permission_set_assignment_test.go | 146 ++++++++++++++++++ 2 files changed, 276 insertions(+) create mode 100644 pkg/connector/permission_set_assignment_test.go diff --git a/baton_capabilities.json b/baton_capabilities.json index 9b83a829..50ce45a5 100644 --- a/baton_capabilities.json +++ b/baton_capabilities.json @@ -463,6 +463,136 @@ ] } }, + { + "resourceType": { + "id": "permission_set", + "displayName": "Permission Set", + "traits": [ + "TRAIT_ROLE" + ], + "annotations": [ + { + "@type": "type.googleapis.com/c1.connector.v2.SkipEntitlementsAndGrants" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.OptInRequired" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.CapabilityPermissions", + "permissions": [ + { + "permission": "sso:ListInstances" + }, + { + "permission": "sso:ListPermissionSets" + }, + { + "permission": "sso:DescribePermissionSet" + } + ] + } + ] + }, + "capabilities": [ + "CAPABILITY_SYNC" + ], + "permissions": { + "permissions": [ + { + "permission": "sso:ListInstances" + }, + { + "permission": "sso:ListPermissionSets" + }, + { + "permission": "sso:DescribePermissionSet" + } + ] + }, + "optInRequired": true + }, + { + "resourceType": { + "id": "permission_set_assignment", + "displayName": "Permission Set Assignment", + "traits": [ + "TRAIT_SCOPE_BINDING" + ], + "annotations": [ + { + "@type": "type.googleapis.com/c1.connector.v2.OptInRequired" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.CapabilityPermissions", + "permissions": [ + { + "permission": "sso:ListInstances" + }, + { + "permission": "sso:ListPermissionSetsProvisionedToAccount" + }, + { + "permission": "sso:DescribePermissionSet" + }, + { + "permission": "sso:ListAccountAssignments" + }, + { + "permission": "sso:CreateAccountAssignment" + }, + { + "permission": "sso:DeleteAccountAssignment" + }, + { + "permission": "sso:DescribeAccountAssignmentCreationStatus" + }, + { + "permission": "sso:DescribeAccountAssignmentDeletionStatus" + }, + { + "permission": "organizations:DescribeAccount" + } + ] + } + ] + }, + "capabilities": [ + "CAPABILITY_SYNC", + "CAPABILITY_PROVISION" + ], + "permissions": { + "permissions": [ + { + "permission": "sso:ListInstances" + }, + { + "permission": "sso:ListPermissionSetsProvisionedToAccount" + }, + { + "permission": "sso:DescribePermissionSet" + }, + { + "permission": "sso:ListAccountAssignments" + }, + { + "permission": "sso:CreateAccountAssignment" + }, + { + "permission": "sso:DeleteAccountAssignment" + }, + { + "permission": "sso:DescribeAccountAssignmentCreationStatus" + }, + { + "permission": "sso:DescribeAccountAssignmentDeletionStatus" + }, + { + "permission": "organizations:DescribeAccount" + } + ] + }, + "optInRequired": true + }, { "resourceType": { "id": "role", diff --git a/pkg/connector/permission_set_assignment_test.go b/pkg/connector/permission_set_assignment_test.go new file mode 100644 index 00000000..d937141e --- /dev/null +++ b/pkg/connector/permission_set_assignment_test.go @@ -0,0 +1,146 @@ +package connector + +import ( + "strings" + "testing" + + awsSdk "github.com/aws/aws-sdk-go-v2/aws" + awsSsoAdminTypes "github.com/aws/aws-sdk-go-v2/service/ssoadmin/types" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + entitlementSdk "github.com/conductorone/baton-sdk/pkg/types/entitlement" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A realistic permission-set ARN contains '-', ':' (including the empty region/account +// ":::" run) and '/'. These are the characters the §3.6 gate worried about. +const ( + testPermissionSetArn = "arn:aws:sso:::permissionSet/ssoins-7204abcd1234abcd/ps-0123456789abcdef" + testAccountID = "123456789012" + testPermissionSetID = testPermissionSetArn // ListPermissionSets returns ARNs +) + +// resourceIDToString and parseV2ExternalID mirror c1's pkg/connector/v2.ResourceIDToString +// (type + "::" + resource) and baton-sdk's baton.ParseV2ExternalID (SplitN("::", 2)). They +// are replicated here so the test pins the exact reconcile-key contract independently of +// the c1 source tree. +func resourceIDToString(rt, resource string) string { return rt + "::" + resource } + +func parseV2ExternalID(t *testing.T, s string) *v2.ResourceId { + t.Helper() + parts := strings.SplitN(s, "::", 2) + require.Len(t, parts, 2, "external id must split into type::resource") + return &v2.ResourceId{ResourceType: parts[0], Resource: parts[1]} +} + +// scopeRoleBindingExternalID replicates c1's scope_role_jit.go helper exactly. +func scopeRoleBindingExternalID(roleResource, scopeResource string) string { + return roleResource + "-" + scopeResource +} + +func TestPermissionSetRoleID_BareARN(t *testing.T) { + assert.Equal(t, testPermissionSetArn, permissionSetRoleID(testPermissionSetArn)) +} + +func TestPermissionSetAssignmentObjectID_Shape(t *testing.T) { + got := permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID) + assert.Equal(t, testPermissionSetArn+"-"+testAccountID, got) +} + +// The connector-emitted binding object id MUST byte-match c1's JIT-fabricated id so a +// JIT'd-then-synced binding reconciles to one RoleScopeBindingRelationship row. +func TestPermissionSetAssignmentObjectID_MatchesJITFabrication(t *testing.T) { + jit := scopeRoleBindingExternalID(permissionSetRoleID(testPermissionSetArn), testAccountID) + connector := permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID) + assert.Equal(t, jit, connector, "connector emission must equal c1 JIT fabrication") +} + +// Drift guard (§2.1.1): the permission_set role resource id and the binding's trait +// role_id MUST be byte-identical — both flow through permissionSetRoleID. +func TestRoleID_NoDriftBetweenRoleResourceAndTrait(t *testing.T) { + ps := &awsSsoAdminTypes.PermissionSet{ + PermissionSetArn: awsSdk.String(testPermissionSetArn), + Name: awsSdk.String("PowerUserAccess"), + } + roleResource, err := permissionSetResource(ps) + require.NoError(t, err) + + binding, err := permissionSetAssignmentResource(testPermissionSetArn, "PowerUserAccess", testAccountID, + &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID}) + require.NoError(t, err) + + trait, err := resourceSdk.GetScopeBindingTrait(binding) + require.NoError(t, err) + + assert.Equal(t, roleResource.Id.Resource, trait.GetRoleId().GetResource(), + "role resource id and trait role_id must byte-match") + assert.Equal(t, resourceTypePermissionSet.Id, roleResource.Id.ResourceType) + assert.Equal(t, resourceTypePermissionSet.Id, trait.GetRoleId().GetResourceType()) +} + +// The scope-binding trait must carry non-nil role_id and scope_resource_id that byte-match +// the permission_set and account builder ids (load-bearing; mismatch silently drops the RSBR). +func TestPermissionSetAssignmentResource_TraitByteMatch(t *testing.T) { + binding, err := permissionSetAssignmentResource(testPermissionSetArn, "PowerUserAccess", testAccountID, + &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID}) + require.NoError(t, err) + + assert.Equal(t, resourceTypePermissionSetAssignment.Id, binding.Id.ResourceType) + assert.Equal(t, permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID), binding.Id.Resource) + + trait, err := resourceSdk.GetScopeBindingTrait(binding) + require.NoError(t, err) + require.NotNil(t, trait.GetRoleId()) + require.NotNil(t, trait.GetScopeResourceId()) + + assert.Equal(t, testPermissionSetArn, trait.GetRoleId().GetResource()) + assert.Equal(t, resourceTypePermissionSet.Id, trait.GetRoleId().GetResourceType()) + assert.Equal(t, testAccountID, trait.GetScopeResourceId().GetResource()) + assert.Equal(t, resourceTypeAccount.Id, trait.GetScopeResourceId().GetResourceType()) + + // Binding's parent is the account (hierarchy edge for c1's by-inheritance walk). + require.NotNil(t, binding.ParentResourceId) + assert.Equal(t, resourceTypeAccount.Id, binding.ParentResourceId.ResourceType) + assert.Equal(t, testAccountID, binding.ParentResourceId.Resource) +} + +// Entitlement slug MUST be exactly "assigned"; its id must match the SDK's NewEntitlementID +// shape that c1 keys the JIT reconcile on. +func TestAssignedEntitlement_Slug(t *testing.T) { + binding, err := permissionSetAssignmentResource(testPermissionSetArn, "PowerUserAccess", testAccountID, + &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID}) + require.NoError(t, err) + + ent := assignedEntitlement(binding) + assert.Equal(t, "assigned", ent.Slug) + assert.Equal(t, v2.Entitlement_PURPOSE_VALUE_ASSIGNMENT, ent.Purpose) + + // c1 fabricates the entitlement external id as NewEntitlementID(binding, "assigned"). + wantID := entitlementSdk.NewEntitlementID(binding, "assigned") + assert.Equal(t, wantID, ent.Id) + assert.Equal(t, + resourceTypePermissionSetAssignment.Id+":"+permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID)+":assigned", + ent.Id) +} + +// The full reconcile-key round-trip: ResourceIDToString -> ParseV2ExternalID recovers the +// binding's (type, resource) verbatim despite the ARN's embedded "::"/":::" — proving the +// gate decision (raw concatenation, no encoding) holds for real ARNs. +func TestBindingSourceID_RoundTripsThroughParseV2ExternalID(t *testing.T) { + binding, err := permissionSetAssignmentResource(testPermissionSetArn, "PowerUserAccess", testAccountID, + &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID}) + require.NoError(t, err) + + sourceID := resourceIDToString(binding.Id.ResourceType, binding.Id.Resource) + parsed := parseV2ExternalID(t, sourceID) + + assert.Equal(t, resourceTypePermissionSetAssignment.Id, parsed.ResourceType) + assert.Equal(t, permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID), parsed.Resource) + + // The role resource id (a bare ARN with ":::") also round-trips intact. + roleSourceID := resourceIDToString(resourceTypePermissionSet.Id, permissionSetRoleID(testPermissionSetArn)) + parsedRole := parseV2ExternalID(t, roleSourceID) + assert.Equal(t, resourceTypePermissionSet.Id, parsedRole.ResourceType) + assert.Equal(t, testPermissionSetArn, parsedRole.Resource) +} From 553cdde31ff8d1f86deb0f199e215583928a2902 Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Wed, 17 Jun 2026 06:57:49 +0000 Subject: [PATCH 3/5] =?UTF-8?q?fix(sparse-acls):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20drop=20V1Identifier,=20Grant=20idempotency,=20behav?= =?UTF-8?q?ioral=20tests,=20real-SDK=20reconcile=20keys?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R1: remove V1Identifier from the new permission_set role resource (brand-new sparse-ACL type has no v1 predecessor; matches confluence/azure exemplars and the sibling binding resource). T3: provisionAssignment now pre-checks ListAccountAssignments and emits GrantAlreadyExists (mirroring the existing GrantAlreadyRevoked-on-404 path) instead of relying on implicit CreateAccountAssignment idempotency. T1: add behavioral unit tests via a new ssoAdminAPI/orgsAPI client seam and an in-memory fake — binding List emits the scope-binding resource+trait, Grant calls CreateAccountAssignment with the right scope/role/principal, Revoke deletes the right assignment, and both idempotency annotations fire. T2: the reconcile-key test now pins against the REAL baton-sdk constructors (resource.NewResourceID + entitlement.NewEntitlementID); removed the circular local ResourceIDToString/ParseV2ExternalID replica (those are c1 platform internals, not in the connector module graph — end-to-end "::" round-trip is covered by the .c1z grep step). Pagination: confirmed binding List paginates fully via the SDK paginator in getOrFetchPermissionSetIDs (no single-page truncation); no change needed. Co-authored-by: c1-squire-dev[bot] --- pkg/connector/account.go | 49 ++- pkg/connector/permission_set.go | 9 +- ...permission_set_assignment_behavior_test.go | 287 ++++++++++++++++++ .../permission_set_assignment_test.go | 58 ++-- pkg/connector/ssoadmin_api.go | 49 +++ 5 files changed, 416 insertions(+), 36 deletions(-) create mode 100644 pkg/connector/permission_set_assignment_behavior_test.go create mode 100644 pkg/connector/ssoadmin_api.go diff --git a/pkg/connector/account.go b/pkg/connector/account.go index a2520736..14178350 100644 --- a/pkg/connector/account.go +++ b/pkg/connector/account.go @@ -123,8 +123,8 @@ type entitlementsPageState struct { type accountResourceType struct { resourceType *v2.ResourceType - orgClient *awsOrgs.Client - ssoAdminClient *awsSsoAdmin.Client + orgClient orgsAPI + ssoAdminClient ssoAdminAPI roleArn string identityInstance *awsSsoAdminTypes.InstanceMetadata identityClient client.IdentityStoreClient @@ -585,6 +585,33 @@ func (o *accountResourceType) Grant(ctx context.Context, principal *v2.Resource, return o.provisionAssignment(ctx, binding.AccountID, binding.PermissionSetId, principal) } +// accountAssignmentExists reports whether (account, permissionSet, principal) is already an +// Identity Center account assignment. It paginates ListAccountAssignments fully so a binding +// on a later page is not missed. Used by provisionAssignment to emit GrantAlreadyExists. +func (o *accountResourceType) accountAssignmentExists(ctx context.Context, accountID string, permissionSetArn string, principalType awsSsoAdminTypes.PrincipalType, principalId string) (bool, error) { + var nextToken *string + for { + resp, err := o.ssoAdminClient.ListAccountAssignments(ctx, &awsSsoAdmin.ListAccountAssignmentsInput{ + AccountId: awsSdk.String(accountID), + InstanceArn: o.identityInstance.InstanceArn, + PermissionSetArn: awsSdk.String(permissionSetArn), + NextToken: nextToken, + }) + if err != nil { + return false, wrapAWSError(fmt.Errorf("baton-aws: ssoadmin.ListAccountAssignments failed: %w", err)) + } + for _, a := range resp.AccountAssignments { + if a.PrincipalType == principalType && awsSdk.ToString(a.PrincipalId) == principalId { + return true, nil + } + } + if resp.NextToken == nil || *resp.NextToken == "" { + return false, nil + } + nextToken = resp.NextToken + } +} + // provisionAssignment creates an Identity Center account assignment for the given // (account, permission set, principal). It is the shared core of the account-entitlement // Grant path and the scope-binding Grant path; both recover (accountID, permissionSetArn) @@ -597,6 +624,20 @@ func (o *accountResourceType) provisionAssignment(ctx context.Context, accountID return nil, err } + // Idempotency: if the assignment already exists, emit GrantAlreadyExists and skip the + // create+status-poll (and the DescribeAccount status check below). CreateAccountAssignment + // is natively idempotent, so re-creating would also succeed, but emitting the annotation is + // the baton-sdk idiom and mirrors the GrantAlreadyRevoked path in deprovisionAssignment. + exists, err := o.accountAssignmentExists(ctx, accountID, permissionSetArn, principalType, principalId) + if err != nil { + return nil, err + } + if exists { + annos := annotations.New() + annos.Append(&v2.GrantAlreadyExists{}) + return annos, nil + } + binding := &PermissionSetBinding{AccountID: accountID, PermissionSetId: permissionSetArn} // try to verify the account status before creating the assignment @@ -881,9 +922,9 @@ func (o *accountResourceType) fetchPermissionSetFromAPI(ctx context.Context, per } func accountBuilder( - orgClient *awsOrgs.Client, + orgClient orgsAPI, roleArn string, - ssoAdminClient *awsSsoAdmin.Client, + ssoAdminClient ssoAdminAPI, identityInstance *awsSsoAdminTypes.InstanceMetadata, region string, identityClient client.IdentityStoreClient, diff --git a/pkg/connector/permission_set.go b/pkg/connector/permission_set.go index 06030fa6..3b301fc0 100644 --- a/pkg/connector/permission_set.go +++ b/pkg/connector/permission_set.go @@ -24,7 +24,7 @@ func permissionSetRoleID(permissionSetArn string) string { type permissionSetResourceType struct { resourceType *v2.ResourceType - ssoAdminClient *awsSsoAdmin.Client + ssoAdminClient ssoAdminAPI identityInstance *awsSsoAdminTypes.InstanceMetadata } @@ -88,7 +88,10 @@ func permissionSetResource(ps *awsSsoAdminTypes.PermissionSet) (*v2.Resource, er resourceTypePermissionSet, permissionSetRoleID(arnStr), nil, - resourceSdk.WithAnnotation(&v2.V1Identifier{Id: arnStr}), + // No V1Identifier: permission_set is a brand-new sparse-ACL resource type with no + // v1 predecessor entity, so there is no legacy id to preserve. Both proven exemplars + // (baton-confluence space_role, baton-azure-infrastructure role_assignment) omit it on + // their new role/binding types; the binding resource here omits it for the same reason. resourceSdk.WithDescription(awsSdk.ToString(ps.Description)), ) } @@ -104,7 +107,7 @@ func (o *permissionSetResourceType) Grants(_ context.Context, _ *v2.Resource, _ return nil, nil, nil } -func permissionSetBuilder(ssoAdminClient *awsSsoAdmin.Client, identityInstance *awsSsoAdminTypes.InstanceMetadata) *permissionSetResourceType { +func permissionSetBuilder(ssoAdminClient ssoAdminAPI, identityInstance *awsSsoAdminTypes.InstanceMetadata) *permissionSetResourceType { return &permissionSetResourceType{ resourceType: resourceTypePermissionSet, ssoAdminClient: ssoAdminClient, diff --git a/pkg/connector/permission_set_assignment_behavior_test.go b/pkg/connector/permission_set_assignment_behavior_test.go new file mode 100644 index 00000000..28f879aa --- /dev/null +++ b/pkg/connector/permission_set_assignment_behavior_test.go @@ -0,0 +1,287 @@ +package connector + +import ( + "context" + "errors" + "testing" + + awsSdk "github.com/aws/aws-sdk-go-v2/aws" + awsOrgs "github.com/aws/aws-sdk-go-v2/service/organizations" + awsOrgsTypes "github.com/aws/aws-sdk-go-v2/service/organizations/types" + awsSsoAdmin "github.com/aws/aws-sdk-go-v2/service/ssoadmin" + awsSsoAdminTypes "github.com/aws/aws-sdk-go-v2/service/ssoadmin/types" + "github.com/conductorone/baton-aws/test" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeSSOAdmin is an in-memory ssoAdminAPI for behavioral tests. Each method delegates to a +// pluggable func; nil funcs return an empty, successful response. Create/Delete inputs are +// recorded so tests can assert the connector calls AWS with the right scope/role/principal. +type fakeSSOAdmin struct { + listPermissionSetsProvisionedToAccountFn func(*awsSsoAdmin.ListPermissionSetsProvisionedToAccountInput) (*awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput, error) + describePermissionSetFn func(*awsSsoAdmin.DescribePermissionSetInput) (*awsSsoAdmin.DescribePermissionSetOutput, error) + listAccountAssignmentsFn func(*awsSsoAdmin.ListAccountAssignmentsInput) (*awsSsoAdmin.ListAccountAssignmentsOutput, error) + createAccountAssignmentFn func(*awsSsoAdmin.CreateAccountAssignmentInput) (*awsSsoAdmin.CreateAccountAssignmentOutput, error) + deleteAccountAssignmentFn func(*awsSsoAdmin.DeleteAccountAssignmentInput) (*awsSsoAdmin.DeleteAccountAssignmentOutput, error) + describeCreationStatusFn func(*awsSsoAdmin.DescribeAccountAssignmentCreationStatusInput) (*awsSsoAdmin.DescribeAccountAssignmentCreationStatusOutput, error) + describeDeletionStatusFn func(*awsSsoAdmin.DescribeAccountAssignmentDeletionStatusInput) (*awsSsoAdmin.DescribeAccountAssignmentDeletionStatusOutput, error) + + createInputs []*awsSsoAdmin.CreateAccountAssignmentInput + deleteInputs []*awsSsoAdmin.DeleteAccountAssignmentInput +} + +func (f *fakeSSOAdmin) ListPermissionSets(_ context.Context, _ *awsSsoAdmin.ListPermissionSetsInput, _ ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.ListPermissionSetsOutput, error) { + return &awsSsoAdmin.ListPermissionSetsOutput{}, nil +} + +func (f *fakeSSOAdmin) ListPermissionSetsProvisionedToAccount( + _ context.Context, + in *awsSsoAdmin.ListPermissionSetsProvisionedToAccountInput, + _ ...func(*awsSsoAdmin.Options), +) (*awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput, error) { + if f.listPermissionSetsProvisionedToAccountFn != nil { + return f.listPermissionSetsProvisionedToAccountFn(in) + } + return &awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput{}, nil +} + +func (f *fakeSSOAdmin) DescribePermissionSet(_ context.Context, in *awsSsoAdmin.DescribePermissionSetInput, _ ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.DescribePermissionSetOutput, error) { + if f.describePermissionSetFn != nil { + return f.describePermissionSetFn(in) + } + return &awsSsoAdmin.DescribePermissionSetOutput{PermissionSet: &awsSsoAdminTypes.PermissionSet{PermissionSetArn: in.PermissionSetArn}}, nil +} + +func (f *fakeSSOAdmin) ListAccountAssignments(_ context.Context, in *awsSsoAdmin.ListAccountAssignmentsInput, _ ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.ListAccountAssignmentsOutput, error) { + if f.listAccountAssignmentsFn != nil { + return f.listAccountAssignmentsFn(in) + } + return &awsSsoAdmin.ListAccountAssignmentsOutput{}, nil +} + +func (f *fakeSSOAdmin) CreateAccountAssignment(_ context.Context, in *awsSsoAdmin.CreateAccountAssignmentInput, _ ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.CreateAccountAssignmentOutput, error) { + f.createInputs = append(f.createInputs, in) + if f.createAccountAssignmentFn != nil { + return f.createAccountAssignmentFn(in) + } + return &awsSsoAdmin.CreateAccountAssignmentOutput{ + AccountAssignmentCreationStatus: &awsSsoAdminTypes.AccountAssignmentOperationStatus{ + RequestId: awsSdk.String("create-req"), + Status: awsSsoAdminTypes.StatusValuesInProgress, + }, + }, nil +} + +func (f *fakeSSOAdmin) DeleteAccountAssignment(_ context.Context, in *awsSsoAdmin.DeleteAccountAssignmentInput, _ ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.DeleteAccountAssignmentOutput, error) { + f.deleteInputs = append(f.deleteInputs, in) + if f.deleteAccountAssignmentFn != nil { + return f.deleteAccountAssignmentFn(in) + } + return &awsSsoAdmin.DeleteAccountAssignmentOutput{ + AccountAssignmentDeletionStatus: &awsSsoAdminTypes.AccountAssignmentOperationStatus{ + RequestId: awsSdk.String("delete-req"), + Status: awsSsoAdminTypes.StatusValuesInProgress, + }, + }, nil +} + +func (f *fakeSSOAdmin) DescribeAccountAssignmentCreationStatus( + _ context.Context, + in *awsSsoAdmin.DescribeAccountAssignmentCreationStatusInput, + _ ...func(*awsSsoAdmin.Options), +) (*awsSsoAdmin.DescribeAccountAssignmentCreationStatusOutput, error) { + if f.describeCreationStatusFn != nil { + return f.describeCreationStatusFn(in) + } + return &awsSsoAdmin.DescribeAccountAssignmentCreationStatusOutput{ + AccountAssignmentCreationStatus: &awsSsoAdminTypes.AccountAssignmentOperationStatus{Status: awsSsoAdminTypes.StatusValuesSucceeded}, + }, nil +} + +func (f *fakeSSOAdmin) DescribeAccountAssignmentDeletionStatus( + _ context.Context, + in *awsSsoAdmin.DescribeAccountAssignmentDeletionStatusInput, + _ ...func(*awsSsoAdmin.Options), +) (*awsSsoAdmin.DescribeAccountAssignmentDeletionStatusOutput, error) { + if f.describeDeletionStatusFn != nil { + return f.describeDeletionStatusFn(in) + } + return &awsSsoAdmin.DescribeAccountAssignmentDeletionStatusOutput{ + AccountAssignmentDeletionStatus: &awsSsoAdminTypes.AccountAssignmentOperationStatus{Status: awsSsoAdminTypes.StatusValuesSucceeded}, + }, nil +} + +// fakeOrgs is an orgsAPI that reports every account active, so the provision/deprovision +// status-verification step passes without a real AWS Organizations call. +type fakeOrgs struct{} + +func (f *fakeOrgs) DescribeAccount(_ context.Context, in *awsOrgs.DescribeAccountInput, _ ...func(*awsOrgs.Options)) (*awsOrgs.DescribeAccountOutput, error) { + return &awsOrgs.DescribeAccountOutput{Account: &awsOrgsTypes.Account{Id: in.AccountId, Status: awsOrgsTypes.AccountStatusActive}}, nil +} + +func (f *fakeOrgs) ListAccounts(_ context.Context, _ *awsOrgs.ListAccountsInput, _ ...func(*awsOrgs.Options)) (*awsOrgs.ListAccountsOutput, error) { + return &awsOrgs.ListAccountsOutput{}, nil +} + +const ( + behaviorRegion = "us-east-1" + behaviorIdentityStoreID = "d-1234567890" + behaviorInstanceArn = "arn:aws:sso:::instance/ssoins-7204abcd1234abcd" + behaviorUserNativeID = "11111111-2222-3333-4444-555555555555" +) + +func newBehaviorAccount(sso *fakeSSOAdmin) *accountResourceType { + identityInstance := &awsSsoAdminTypes.InstanceMetadata{ + InstanceArn: awsSdk.String(behaviorInstanceArn), + IdentityStoreId: awsSdk.String(behaviorIdentityStoreID), + } + return accountBuilder(&fakeOrgs{}, "", sso, identityInstance, behaviorRegion, &test.MockedIdentityStoreClient{}) +} + +func behaviorBinding(t *testing.T) (*v2.Resource, *v2.Entitlement) { + t.Helper() + binding, err := permissionSetAssignmentResource(testPermissionSetArn, "PowerUserAccess", testAccountID, + &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID}) + require.NoError(t, err) + return binding, assignedEntitlement(binding) +} + +func behaviorUserPrincipal() *v2.Resource { + return &v2.Resource{Id: &v2.ResourceId{ + ResourceType: resourceTypeSSOUser.Id, + Resource: ssoUserToARN(behaviorRegion, behaviorIdentityStoreID, behaviorUserNativeID), + }} +} + +// List on the binding resource type emits one scope-binding resource per permission set +// provisioned to the account, each carrying a ScopeBindingTrait whose role_id/scope_resource_id +// byte-match the permission_set and account builder ids. +func TestPermissionSetAssignmentList_EmitsScopeBinding(t *testing.T) { + ctx := context.Background() + sso := &fakeSSOAdmin{ + listPermissionSetsProvisionedToAccountFn: func(in *awsSsoAdmin.ListPermissionSetsProvisionedToAccountInput) (*awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput, error) { + assert.Equal(t, testAccountID, awsSdk.ToString(in.AccountId)) + return &awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput{PermissionSets: []string{testPermissionSetArn}}, nil + }, + describePermissionSetFn: func(in *awsSsoAdmin.DescribePermissionSetInput) (*awsSsoAdmin.DescribePermissionSetOutput, error) { + return &awsSsoAdmin.DescribePermissionSetOutput{PermissionSet: &awsSsoAdminTypes.PermissionSet{ + PermissionSetArn: in.PermissionSetArn, + Name: awsSdk.String("PowerUserAccess"), + }}, nil + }, + } + psa := permissionSetAssignmentBuilder(newBehaviorAccount(sso)) + + parentID := &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID} + resources, _, err := psa.List(ctx, parentID, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, resources, 1) + + trait, err := resourceSdk.GetScopeBindingTrait(resources[0]) + require.NoError(t, err) + assert.Equal(t, testPermissionSetArn, trait.GetRoleId().GetResource()) + assert.Equal(t, resourceTypePermissionSet.Id, trait.GetRoleId().GetResourceType()) + assert.Equal(t, testAccountID, trait.GetScopeResourceId().GetResource()) + assert.Equal(t, resourceTypeAccount.Id, trait.GetScopeResourceId().GetResourceType()) + require.NotNil(t, resources[0].ParentResourceId) + assert.Equal(t, testAccountID, resources[0].ParentResourceId.Resource) +} + +// List only crawls bindings as a child of an account; any other parent yields nothing. +func TestPermissionSetAssignmentList_GatedOnAccountParent(t *testing.T) { + ctx := context.Background() + psa := permissionSetAssignmentBuilder(newBehaviorAccount(&fakeSSOAdmin{})) + + resources, _, err := psa.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + assert.Empty(t, resources) + + resources, _, err = psa.List(ctx, &v2.ResourceId{ResourceType: resourceTypePermissionSet.Id, Resource: testPermissionSetArn}, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + assert.Empty(t, resources) +} + +// Grant resolves (account, permission set) from the trait and calls CreateAccountAssignment +// with the right scope (account), role (permission set ARN), and principal. +func TestPermissionSetAssignmentGrant_CallsCreateWithScopeAndRole(t *testing.T) { + ctx := context.Background() + sso := &fakeSSOAdmin{} // ListAccountAssignments default empty => not already assigned + psa := permissionSetAssignmentBuilder(newBehaviorAccount(sso)) + + _, ent := behaviorBinding(t) + annos, err := psa.Grant(ctx, behaviorUserPrincipal(), ent) + require.NoError(t, err) + + require.Len(t, sso.createInputs, 1) + in := sso.createInputs[0] + assert.Equal(t, testPermissionSetArn, awsSdk.ToString(in.PermissionSetArn)) + assert.Equal(t, testAccountID, awsSdk.ToString(in.TargetId)) + assert.Equal(t, awsSsoAdminTypes.TargetTypeAwsAccount, in.TargetType) + assert.Equal(t, awsSsoAdminTypes.PrincipalTypeUser, in.PrincipalType) + assert.Equal(t, behaviorUserNativeID, awsSdk.ToString(in.PrincipalId)) + assert.False(t, annos.Contains(&v2.GrantAlreadyExists{}), "fresh grant must not report already-exists") +} + +// Grant is idempotent: when the assignment already exists, it emits GrantAlreadyExists and +// does not call CreateAccountAssignment. +func TestPermissionSetAssignmentGrant_IdempotentAlreadyExists(t *testing.T) { + ctx := context.Background() + sso := &fakeSSOAdmin{ + listAccountAssignmentsFn: func(in *awsSsoAdmin.ListAccountAssignmentsInput) (*awsSsoAdmin.ListAccountAssignmentsOutput, error) { + return &awsSsoAdmin.ListAccountAssignmentsOutput{AccountAssignments: []awsSsoAdminTypes.AccountAssignment{{ + AccountId: in.AccountId, + PermissionSetArn: in.PermissionSetArn, + PrincipalType: awsSsoAdminTypes.PrincipalTypeUser, + PrincipalId: awsSdk.String(behaviorUserNativeID), + }}}, nil + }, + } + psa := permissionSetAssignmentBuilder(newBehaviorAccount(sso)) + + _, ent := behaviorBinding(t) + annos, err := psa.Grant(ctx, behaviorUserPrincipal(), ent) + require.NoError(t, err) + assert.True(t, annos.Contains(&v2.GrantAlreadyExists{}), "existing assignment must report already-exists") + assert.Empty(t, sso.createInputs, "must not call CreateAccountAssignment when already assigned") +} + +// Revoke resolves (account, permission set) from the trait and calls DeleteAccountAssignment +// with the right scope, role, and principal. +func TestPermissionSetAssignmentRevoke_CallsDelete(t *testing.T) { + ctx := context.Background() + sso := &fakeSSOAdmin{} + psa := permissionSetAssignmentBuilder(newBehaviorAccount(sso)) + + _, ent := behaviorBinding(t) + grant := &v2.Grant{Entitlement: ent, Principal: behaviorUserPrincipal()} + annos, err := psa.Revoke(ctx, grant) + require.NoError(t, err) + + require.Len(t, sso.deleteInputs, 1) + in := sso.deleteInputs[0] + assert.Equal(t, testPermissionSetArn, awsSdk.ToString(in.PermissionSetArn)) + assert.Equal(t, testAccountID, awsSdk.ToString(in.TargetId)) + assert.Equal(t, awsSsoAdminTypes.PrincipalTypeUser, in.PrincipalType) + assert.Equal(t, behaviorUserNativeID, awsSdk.ToString(in.PrincipalId)) + assert.False(t, annos.Contains(&v2.GrantAlreadyRevoked{})) +} + +// Revoke is idempotent: a 404 from the deletion-status check yields GrantAlreadyRevoked. +func TestPermissionSetAssignmentRevoke_IdempotentAlreadyRevoked(t *testing.T) { + ctx := context.Background() + sso := &fakeSSOAdmin{ + describeDeletionStatusFn: func(_ *awsSsoAdmin.DescribeAccountAssignmentDeletionStatusInput) (*awsSsoAdmin.DescribeAccountAssignmentDeletionStatusOutput, error) { + return nil, errors.New("operation error SSO Admin: DescribeAccountAssignmentDeletionStatus, https response error StatusCode: 404, Received a 404 status error") + }, + } + psa := permissionSetAssignmentBuilder(newBehaviorAccount(sso)) + + _, ent := behaviorBinding(t) + grant := &v2.Grant{Entitlement: ent, Principal: behaviorUserPrincipal()} + annos, err := psa.Revoke(ctx, grant) + require.NoError(t, err) + assert.True(t, annos.Contains(&v2.GrantAlreadyRevoked{}), "404 on delete must report already-revoked") +} diff --git a/pkg/connector/permission_set_assignment_test.go b/pkg/connector/permission_set_assignment_test.go index d937141e..1d9c2dbb 100644 --- a/pkg/connector/permission_set_assignment_test.go +++ b/pkg/connector/permission_set_assignment_test.go @@ -1,7 +1,6 @@ package connector import ( - "strings" "testing" awsSdk "github.com/aws/aws-sdk-go-v2/aws" @@ -21,20 +20,11 @@ const ( testPermissionSetID = testPermissionSetArn // ListPermissionSets returns ARNs ) -// resourceIDToString and parseV2ExternalID mirror c1's pkg/connector/v2.ResourceIDToString -// (type + "::" + resource) and baton-sdk's baton.ParseV2ExternalID (SplitN("::", 2)). They -// are replicated here so the test pins the exact reconcile-key contract independently of -// the c1 source tree. -func resourceIDToString(rt, resource string) string { return rt + "::" + resource } - -func parseV2ExternalID(t *testing.T, s string) *v2.ResourceId { - t.Helper() - parts := strings.SplitN(s, "::", 2) - require.Len(t, parts, 2, "external id must split into type::resource") - return &v2.ResourceId{ResourceType: parts[0], Resource: parts[1]} -} - -// scopeRoleBindingExternalID replicates c1's scope_role_jit.go helper exactly. +// scopeRoleBindingExternalID mirrors c1's scope_role_jit.go JIT fabrication +// (scopeRoleBindingExternalID = role + "-" + scope). c1 is the platform repo and is not in +// this connector's module graph, so this one-line shape — the JIT id the connector emission +// must byte-match — is pinned here. The end-to-end byte-match against a live sync is the .c1z +// grep-verification step (enablement checklist / data-model §7.1). func scopeRoleBindingExternalID(roleResource, scopeResource string) string { return roleResource + "-" + scopeResource } @@ -124,23 +114,33 @@ func TestAssignedEntitlement_Slug(t *testing.T) { ent.Id) } -// The full reconcile-key round-trip: ResourceIDToString -> ParseV2ExternalID recovers the -// binding's (type, resource) verbatim despite the ARN's embedded "::"/":::" — proving the -// gate decision (raw concatenation, no encoding) holds for real ARNs. -func TestBindingSourceID_RoundTripsThroughParseV2ExternalID(t *testing.T) { +// The reconcile-key contract is pinned against the REAL baton-sdk id constructors, not local +// replicas. baton-sdk does NOT export the c1-side type-prefixed external-id codec +// (ResourceIDToString uses "::" / ParseV2ExternalID splits on it) — those live in the c1 +// platform repo, which is intentionally absent from a connector's module graph — so the two +// reconcile keys we CAN verify with real SDK functions are pinned here, and the end-to-end +// "::" round-trip is covered by the .c1z grep-verification step (G6) against a live sync. +func TestBindingReconcileKeys_RealSDK(t *testing.T) { binding, err := permissionSetAssignmentResource(testPermissionSetArn, "PowerUserAccess", testAccountID, &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID}) require.NoError(t, err) - sourceID := resourceIDToString(binding.Id.ResourceType, binding.Id.Resource) - parsed := parseV2ExternalID(t, sourceID) - - assert.Equal(t, resourceTypePermissionSetAssignment.Id, parsed.ResourceType) - assert.Equal(t, permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID), parsed.Resource) + // 1) The binding's resource id is exactly what the SDK's canonical NewResourceID produces + // for the same (type, object id) — so the connector emits the SDK-canonical id, with the + // ARN's embedded '-', ':' and '/' carried verbatim into Resource (no lossy encoding). + wantID, err := resourceSdk.NewResourceID(resourceTypePermissionSetAssignment, + permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID)) + require.NoError(t, err) + assert.Equal(t, wantID.ResourceType, binding.Id.ResourceType) + assert.Equal(t, wantID.Resource, binding.Id.Resource) + assert.Equal(t, permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID), binding.Id.Resource) - // The role resource id (a bare ARN with ":::") also round-trips intact. - roleSourceID := resourceIDToString(resourceTypePermissionSet.Id, permissionSetRoleID(testPermissionSetArn)) - parsedRole := parseV2ExternalID(t, roleSourceID) - assert.Equal(t, resourceTypePermissionSet.Id, parsedRole.ResourceType) - assert.Equal(t, testPermissionSetArn, parsedRole.Resource) + // 2) The entitlement external id — c1's canonical reconcile key — is exactly the real SDK + // NewEntitlementID(binding, "assigned"); the realistic ARN's special chars survive intact. + gotEnt := assignedEntitlement(binding) + wantEntID := entitlementSdk.NewEntitlementID(binding, permissionSetAssignmentEntitlement) + assert.Equal(t, wantEntID, gotEnt.Id) + assert.Equal(t, + resourceTypePermissionSetAssignment.Id+":"+permissionSetAssignmentObjectID(testPermissionSetArn, testAccountID)+":assigned", + gotEnt.Id) } diff --git a/pkg/connector/ssoadmin_api.go b/pkg/connector/ssoadmin_api.go new file mode 100644 index 00000000..786a17eb --- /dev/null +++ b/pkg/connector/ssoadmin_api.go @@ -0,0 +1,49 @@ +package connector + +import ( + "context" + + awsOrgs "github.com/aws/aws-sdk-go-v2/service/organizations" + awsSsoAdmin "github.com/aws/aws-sdk-go-v2/service/ssoadmin" +) + +// ssoAdminAPI is the subset of *ssoadmin.Client that the account and permission-set builders +// call. Declaring it as an interface (rather than depending on the concrete *ssoadmin.Client) +// gives unit tests a seam to substitute a fake without standing up real AWS clients or a mock +// HTTP server. *ssoadmin.Client satisfies this interface structurally, so production wiring is +// unchanged. +// +// The two List* methods double as the API clients for the SDK's +// NewListPermissionSetsPaginator / NewListPermissionSetsProvisionedToAccountPaginator +// constructors — an ssoAdminAPI value is assignable to those constructors' narrower client +// interfaces because its method set is a superset. +type ssoAdminAPI interface { + ListPermissionSets(ctx context.Context, params *awsSsoAdmin.ListPermissionSetsInput, optFns ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.ListPermissionSetsOutput, error) + ListPermissionSetsProvisionedToAccount( + ctx context.Context, + params *awsSsoAdmin.ListPermissionSetsProvisionedToAccountInput, + optFns ...func(*awsSsoAdmin.Options), + ) (*awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput, error) + DescribePermissionSet(ctx context.Context, params *awsSsoAdmin.DescribePermissionSetInput, optFns ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.DescribePermissionSetOutput, error) + ListAccountAssignments(ctx context.Context, params *awsSsoAdmin.ListAccountAssignmentsInput, optFns ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.ListAccountAssignmentsOutput, error) + CreateAccountAssignment(ctx context.Context, params *awsSsoAdmin.CreateAccountAssignmentInput, optFns ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.CreateAccountAssignmentOutput, error) + DeleteAccountAssignment(ctx context.Context, params *awsSsoAdmin.DeleteAccountAssignmentInput, optFns ...func(*awsSsoAdmin.Options)) (*awsSsoAdmin.DeleteAccountAssignmentOutput, error) + DescribeAccountAssignmentCreationStatus( + ctx context.Context, + params *awsSsoAdmin.DescribeAccountAssignmentCreationStatusInput, + optFns ...func(*awsSsoAdmin.Options), + ) (*awsSsoAdmin.DescribeAccountAssignmentCreationStatusOutput, error) + DescribeAccountAssignmentDeletionStatus( + ctx context.Context, + params *awsSsoAdmin.DescribeAccountAssignmentDeletionStatusInput, + optFns ...func(*awsSsoAdmin.Options), + ) (*awsSsoAdmin.DescribeAccountAssignmentDeletionStatusOutput, error) +} + +// orgsAPI is the subset of *organizations.Client the account builder calls. Same rationale and +// structural-satisfaction property as ssoAdminAPI; ListAccounts also doubles as the API client +// for NewListAccountsPaginator. +type orgsAPI interface { + DescribeAccount(ctx context.Context, params *awsOrgs.DescribeAccountInput, optFns ...func(*awsOrgs.Options)) (*awsOrgs.DescribeAccountOutput, error) + ListAccounts(ctx context.Context, params *awsOrgs.ListAccountsInput, optFns ...func(*awsOrgs.Options)) (*awsOrgs.ListAccountsOutput, error) +} From ce8e0f61b1b58add27678613d294fd2a60b61cf9 Mon Sep 17 00:00:00 2001 From: "c1-squire-dev[bot]" Date: Wed, 17 Jun 2026 12:52:37 +0000 Subject: [PATCH 4/5] =?UTF-8?q?feat(sparse-acls):=20Phase=202=20=E2=80=94?= =?UTF-8?q?=20AWS=20Organizations=20Root=E2=86=92OU=E2=86=92Account=20hier?= =?UTF-8?q?archy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the AWS Organizations org tree as Sparse ACLs scope context so c1's by-inheritance review walks Account → OU → Root with the role pinned. - New `organization` (root) and `organizational_unit` resource types: scope tiers only (SkipEntitlementsAndGrants + OptInRequired), no bindings — AWS Identity Center assigns permission sets to ACCOUNTS only, so there is no native OU/Root-level assignment. The tiers are hierarchy/review context. - New org/OU builders walk ListRoots → ListOrganizationalUnitsForParent (recursive, via ChildResourceType) with pagination.Bag, parenting each OU to its crawl seed. - account.List re-parents each account onto its Root/OU via ListParents, wiring the hierarchy edge the by-inheritance ancestor walk needs. - Fail-soft: missing organizations:* read permission degrades to flat accounts with a WARN rather than aborting the sync (errors otherwise propagate — no false-revocation). New org-read perms declared on the account / org / OU capability sets; baton_capabilities.json regenerated. - Behavioral tests via the existing client-mock seam: root/OU emission, nested-OU recursion, parent-type gating, account re-parenting, and the access-denied fail-soft paths. Scope-bindings stay account-level and the TRAIT_SCOPE_BINDING contract is unchanged; this commit only adds hierarchy context above the binding. Co-authored-by: c1-squire-dev[bot] --- baton_capabilities.json | 86 ++++++ pkg/connector/account.go | 33 ++- pkg/connector/connector.go | 6 + pkg/connector/organization.go | 248 ++++++++++++++++++ pkg/connector/organization_test.go | 239 +++++++++++++++++ ...permission_set_assignment_behavior_test.go | 44 +++- pkg/connector/resource_types.go | 40 +++ pkg/connector/ssoadmin_api.go | 17 +- 8 files changed, 702 insertions(+), 11 deletions(-) create mode 100644 pkg/connector/organization.go create mode 100644 pkg/connector/organization_test.go diff --git a/baton_capabilities.json b/baton_capabilities.json index 50ce45a5..9d94cb26 100644 --- a/baton_capabilities.json +++ b/baton_capabilities.json @@ -60,6 +60,9 @@ { "permission": "organizations:DescribeOrganization" }, + { + "permission": "organizations:ListParents" + }, { "permission": "sso:ListPermissionSets" }, @@ -142,6 +145,9 @@ { "permission": "organizations:DescribeOrganization" }, + { + "permission": "organizations:ListParents" + }, { "permission": "sso:ListPermissionSets" }, @@ -463,6 +469,86 @@ ] } }, + { + "resourceType": { + "id": "organization", + "displayName": "Organization Root", + "annotations": [ + { + "@type": "type.googleapis.com/c1.connector.v2.SkipEntitlementsAndGrants" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.OptInRequired" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.ChildResourceType", + "resourceTypeId": "organizational_unit" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.CapabilityPermissions", + "permissions": [ + { + "permission": "organizations:ListRoots" + }, + { + "permission": "organizations:ListOrganizationalUnitsForParent" + } + ] + } + ] + }, + "capabilities": [ + "CAPABILITY_SYNC" + ], + "permissions": { + "permissions": [ + { + "permission": "organizations:ListRoots" + }, + { + "permission": "organizations:ListOrganizationalUnitsForParent" + } + ] + }, + "optInRequired": true + }, + { + "resourceType": { + "id": "organizational_unit", + "displayName": "Organizational Unit", + "annotations": [ + { + "@type": "type.googleapis.com/c1.connector.v2.SkipEntitlementsAndGrants" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.OptInRequired" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.ChildResourceType", + "resourceTypeId": "organizational_unit" + }, + { + "@type": "type.googleapis.com/c1.connector.v2.CapabilityPermissions", + "permissions": [ + { + "permission": "organizations:ListOrganizationalUnitsForParent" + } + ] + } + ] + }, + "capabilities": [ + "CAPABILITY_SYNC" + ], + "permissions": { + "permissions": [ + { + "permission": "organizations:ListOrganizationalUnitsForParent" + } + ] + }, + "optInRequired": true + }, { "resourceType": { "id": "permission_set", diff --git a/pkg/connector/account.go b/pkg/connector/account.go index 14178350..36509be6 100644 --- a/pkg/connector/account.go +++ b/pkg/connector/account.go @@ -161,6 +161,7 @@ func (o *accountResourceType) List(ctx context.Context, _ *v2.ResourceId, opts r } rv := make([]*v2.Resource, 0, len(resp.Accounts)) + orgReadDenied := false for _, account := range resp.Accounts { annos := &v2.V1Identifier{ Id: awsSdk.ToString(account.Id), @@ -177,23 +178,45 @@ func (o *accountResourceType) List(ctx context.Context, _ *v2.ResourceId, opts r l.Debug("baton-aws: account found", zap.String("name", name), zap.String("account_id", accountId), zap.String("account_status", string(status))) profile := accountProfile(ctx, account) - userResource, err := resourceSdk.NewAppResource( - name, - resourceTypeAccount, - accountId, - []resourceSdk.AppTraitOption{resourceSdk.WithAppProfile(profile)}, + resourceOpts := []resourceSdk.ResourceOption{ resourceSdk.WithAnnotation(annos), // Sparse ACLs: advertise the scope-binding type as a child so the SDK // crawls per-(account, permission set) bindings under each account. resourceSdk.WithAnnotation(&v2.ChildResourceType{ ResourceTypeId: resourceTypePermissionSetAssignment.Id, }), + } + + // Sparse ACLs hierarchy (Phase 2): re-parent the account under its Root/OU so c1's + // by-inheritance review can walk Account → OU → Root with the role pinned. Fail-soft: + // without organizations:ListParents the account stays flat (parentless) and we WARN once. + parentID, accessDenied, err := accountParentResourceID(ctx, o.orgClient, accountId) + if err != nil { + return nil, nil, err + } + if accessDenied { + orgReadDenied = true + } + if parentID != nil { + resourceOpts = append(resourceOpts, resourceSdk.WithParentResourceID(parentID)) + } + + userResource, err := resourceSdk.NewAppResource( + name, + resourceTypeAccount, + accountId, + []resourceSdk.AppTraitOption{resourceSdk.WithAppProfile(profile)}, + resourceOpts..., ) if err != nil { return nil, nil, err } rv = append(rv, userResource) } + if orgReadDenied { + l.Warn("baton-aws: missing organizations:ListParents permission; accounts synced flat (no Root/OU hierarchy). " + + "Add organizations:ListParents to enable by-inheritance review across the org tree.") + } if resp.NextToken != nil { token, err := bag.NextToken(*resp.NextToken) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index efba0d80..3bee6bf3 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -458,6 +458,10 @@ func (c *AWS) ResourceSyncers(ctx context.Context) []connectorbuilder.ResourceSy // per-(account, permission set) scope-binding. Both are OptInRequired. permissionSetBuilder(c.ssoAdminClient, c.identityInstance), permissionSetAssignmentBuilder(acct), + // Sparse ACLs hierarchy: Organization Root → OU scope tiers (account re-parenting + // happens in accountBuilder.List). Hierarchy/review context only — no bindings. + organizationBuilder(c.orgClient), + organizationalUnitBuilder(c.orgClient), ) } @@ -494,6 +498,8 @@ func (d *defaultCapabilitiesBuilder) ResourceSyncers(_ context.Context) []connec accountBuilder(nil, "", nil, nil, "", nil), permissionSetBuilder(nil, nil), permissionSetAssignmentBuilder(accountBuilder(nil, "", nil, nil, "", nil)), + organizationBuilder(nil), + organizationalUnitBuilder(nil), accountIAMBuilder(nil, nil, nil), secretBuilder(nil, nil), } diff --git a/pkg/connector/organization.go b/pkg/connector/organization.go new file mode 100644 index 00000000..0bdd552b --- /dev/null +++ b/pkg/connector/organization.go @@ -0,0 +1,248 @@ +package connector + +import ( + "context" + "errors" + "fmt" + + awsSdk "github.com/aws/aws-sdk-go-v2/aws" + awsOrgs "github.com/aws/aws-sdk-go-v2/service/organizations" + awsOrgsTypes "github.com/aws/aws-sdk-go-v2/service/organizations/types" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/pagination" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" + "go.uber.org/zap" +) + +// isOrgAccessDenied reports whether err is an AWS Organizations AccessDeniedException — +// i.e. the caller lacks the organizations:* read permission. The Sparse ACLs hierarchy is +// best-effort context (bindings always live at the account leaf), so a missing org-tree read +// permission degrades gracefully to flat accounts with a WARN rather than aborting the sync. +// This is checked on the RAW error before wrapAWSError (which converts throttles to a gRPC +// status and breaks the errors.As chain). +func isOrgAccessDenied(err error) bool { + var accessDenied *awsOrgsTypes.AccessDeniedException + return errors.As(err, &accessDenied) +} + +// organizationResourceType syncs AWS Organizations roots as the top tier of the Sparse ACLs +// hierarchy (Root → OU → Account). Roots hold no binding; they are navigation / by-inheritance +// review context only. +type organizationResourceType struct { + resourceType *v2.ResourceType + orgClient orgsAPI +} + +func (o *organizationResourceType) ResourceType(_ context.Context) *v2.ResourceType { + return o.resourceType +} + +// organizationResource builds the org-root scope resource. Its id is the root id (r-xxxx), +// which an account's ListParents result (ParentTypeRoot) byte-matches when re-parenting. +func organizationResource(root awsOrgsTypes.Root) (*v2.Resource, error) { + name := awsSdk.ToString(root.Name) + if name == "" { + name = awsSdk.ToString(root.Id) + } + return resourceSdk.NewResource( + name, + resourceTypeOrganization, + awsSdk.ToString(root.Id), + ) +} + +func (o *organizationResourceType) List(ctx context.Context, parentResourceID *v2.ResourceId, opts resourceSdk.SyncOpAttrs) ([]*v2.Resource, *resourceSdk.SyncOpResults, error) { + // Roots are the top tier — only enumerated at the top level, never as a child. + if parentResourceID != nil { + return nil, nil, nil + } + + bag := &pagination.Bag{} + if err := bag.Unmarshal(opts.PageToken.Token); err != nil { + return nil, nil, err + } + if bag.Current() == nil { + bag.Push(pagination.PageState{ResourceTypeID: resourceTypeOrganization.Id}) + } + + input := &awsOrgs.ListRootsInput{} + if bag.PageToken() != "" { + input.NextToken = awsSdk.String(bag.PageToken()) + } + + resp, err := o.orgClient.ListRoots(ctx, input) + if err != nil { + if isOrgAccessDenied(err) { + ctxzap.Extract(ctx).Warn("baton-aws: missing organizations:ListRoots permission; skipping organization hierarchy (accounts remain flat)", zap.Error(err)) + return nil, nil, nil + } + return nil, nil, wrapAWSError(fmt.Errorf("baton-aws: organizations.ListRoots failed: %w", err)) + } + + rv := make([]*v2.Resource, 0, len(resp.Roots)) + for _, root := range resp.Roots { + resource, err := organizationResource(root) + if err != nil { + return nil, nil, err + } + rv = append(rv, resource) + } + + if resp.NextToken != nil { + token, err := bag.NextToken(*resp.NextToken) + if err != nil { + return rv, nil, err + } + return rv, &resourceSdk.SyncOpResults{NextPageToken: token}, nil + } + return rv, nil, nil +} + +func (o *organizationResourceType) Entitlements(_ context.Context, _ *v2.Resource, _ resourceSdk.SyncOpAttrs) ([]*v2.Entitlement, *resourceSdk.SyncOpResults, error) { + return nil, nil, nil +} + +func (o *organizationResourceType) Grants(_ context.Context, _ *v2.Resource, _ resourceSdk.SyncOpAttrs) ([]*v2.Grant, *resourceSdk.SyncOpResults, error) { + return nil, nil, nil +} + +func organizationBuilder(orgClient orgsAPI) *organizationResourceType { + return &organizationResourceType{ + resourceType: resourceTypeOrganization, + orgClient: orgClient, + } +} + +// organizationalUnitResourceType syncs AWS Organizations OUs as the intermediate hierarchy +// tiers. It is crawled top-down: as a child of an organization (root) and, recursively, of +// itself (nested OUs). OUs hold no binding — they are review/navigation context only. +type organizationalUnitResourceType struct { + resourceType *v2.ResourceType + orgClient orgsAPI +} + +func (o *organizationalUnitResourceType) ResourceType(_ context.Context) *v2.ResourceType { + return o.resourceType +} + +// organizationalUnitResource builds an OU scope resource parented to the root or parent OU it +// was crawled under. Its id is the OU id (ou-xxxx), which an account's ListParents result +// (ParentTypeOrganizationalUnit) byte-matches when re-parenting. +func organizationalUnitResource(ou awsOrgsTypes.OrganizationalUnit, parentResourceID *v2.ResourceId) (*v2.Resource, error) { + name := awsSdk.ToString(ou.Name) + if name == "" { + name = awsSdk.ToString(ou.Id) + } + return resourceSdk.NewResource( + name, + resourceTypeOrganizationalUnit, + awsSdk.ToString(ou.Id), + resourceSdk.WithParentResourceID(parentResourceID), + ) +} + +func (o *organizationalUnitResourceType) List(ctx context.Context, parentResourceID *v2.ResourceId, opts resourceSdk.SyncOpAttrs) ([]*v2.Resource, *resourceSdk.SyncOpResults, error) { + // OUs only exist beneath a root or another OU; they are never a top-level resource. Crawl + // only when listed as a child of one of those tiers (the SDK drives this via the + // ChildResourceType annotations on organization / organizational_unit). + if parentResourceID == nil || + (parentResourceID.ResourceType != resourceTypeOrganization.Id && + parentResourceID.ResourceType != resourceTypeOrganizationalUnit.Id) { + return nil, nil, nil + } + + bag := &pagination.Bag{} + if err := bag.Unmarshal(opts.PageToken.Token); err != nil { + return nil, nil, err + } + if bag.Current() == nil { + bag.Push(pagination.PageState{ResourceTypeID: resourceTypeOrganizationalUnit.Id}) + } + + input := &awsOrgs.ListOrganizationalUnitsForParentInput{ + ParentId: awsSdk.String(parentResourceID.Resource), + } + if bag.PageToken() != "" { + input.NextToken = awsSdk.String(bag.PageToken()) + } + + resp, err := o.orgClient.ListOrganizationalUnitsForParent(ctx, input) + if err != nil { + if isOrgAccessDenied(err) { + ctxzap.Extract(ctx).Warn("baton-aws: missing organizations:ListOrganizationalUnitsForParent permission; skipping OU subtree", + zap.String("parent_id", parentResourceID.Resource), zap.Error(err)) + return nil, nil, nil + } + return nil, nil, wrapAWSError(fmt.Errorf("baton-aws: organizations.ListOrganizationalUnitsForParent failed: %w", err)) + } + + rv := make([]*v2.Resource, 0, len(resp.OrganizationalUnits)) + for _, ou := range resp.OrganizationalUnits { + resource, err := organizationalUnitResource(ou, parentResourceID) + if err != nil { + return nil, nil, err + } + rv = append(rv, resource) + } + + if resp.NextToken != nil { + token, err := bag.NextToken(*resp.NextToken) + if err != nil { + return rv, nil, err + } + return rv, &resourceSdk.SyncOpResults{NextPageToken: token}, nil + } + return rv, nil, nil +} + +func (o *organizationalUnitResourceType) Entitlements(_ context.Context, _ *v2.Resource, _ resourceSdk.SyncOpAttrs) ([]*v2.Entitlement, *resourceSdk.SyncOpResults, error) { + return nil, nil, nil +} + +func (o *organizationalUnitResourceType) Grants(_ context.Context, _ *v2.Resource, _ resourceSdk.SyncOpAttrs) ([]*v2.Grant, *resourceSdk.SyncOpResults, error) { + return nil, nil, nil +} + +func organizationalUnitBuilder(orgClient orgsAPI) *organizationalUnitResourceType { + return &organizationalUnitResourceType{ + resourceType: resourceTypeOrganizationalUnit, + orgClient: orgClient, + } +} + +// accountParentResourceID resolves an account's immediate Organizations parent (its Root or OU) +// into the matching scope-resource id, so the account's parent pointer connects it to the +// Sparse ACLs hierarchy for c1's by-inheritance ancestor walk. +// +// An account has exactly one direct parent in AWS Organizations, so ListParents returns a single +// entry (no pagination needed). Returns (nil, true, nil) — accessDenied — when the caller lacks +// organizations:ListParents, so the account builder degrades to a flat (parentless) account with +// a single WARN rather than failing the sync. Any other error is propagated (never swallowed). +func accountParentResourceID(ctx context.Context, orgClient orgsAPI, accountID string) (*v2.ResourceId, bool, error) { + resp, err := orgClient.ListParents(ctx, &awsOrgs.ListParentsInput{ + ChildId: awsSdk.String(accountID), + }) + if err != nil { + if isOrgAccessDenied(err) { + return nil, true, nil + } + return nil, false, wrapAWSError(fmt.Errorf("baton-aws: organizations.ListParents failed: %w", err)) + } + if len(resp.Parents) == 0 { + return nil, false, nil + } + + p := resp.Parents[0] + parentID := awsSdk.ToString(p.Id) + switch p.Type { + case awsOrgsTypes.ParentTypeRoot: + return &v2.ResourceId{ResourceType: resourceTypeOrganization.Id, Resource: parentID}, false, nil + case awsOrgsTypes.ParentTypeOrganizationalUnit: + return &v2.ResourceId{ResourceType: resourceTypeOrganizationalUnit.Id, Resource: parentID}, false, nil + default: + ctxzap.Extract(ctx).Warn("baton-aws: unexpected Organizations parent type; leaving account unparented", + zap.String("account_id", accountID), zap.String("parent_type", string(p.Type))) + return nil, false, nil + } +} diff --git a/pkg/connector/organization_test.go b/pkg/connector/organization_test.go new file mode 100644 index 00000000..ca2b1fe4 --- /dev/null +++ b/pkg/connector/organization_test.go @@ -0,0 +1,239 @@ +package connector + +import ( + "context" + "testing" + + awsSdk "github.com/aws/aws-sdk-go-v2/aws" + awsOrgs "github.com/aws/aws-sdk-go-v2/service/organizations" + awsOrgsTypes "github.com/aws/aws-sdk-go-v2/service/organizations/types" + awsSsoAdminTypes "github.com/aws/aws-sdk-go-v2/service/ssoadmin/types" + v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testRootID = "r-abc1" + testOUID = "ou-abc1-11111111" + testOUID2 = "ou-abc1-22222222" +) + +// organization.List emits one organization resource per Organizations root, id=root id. +func TestOrganizationList_EmitsRoots(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listRootsFn: func(_ *awsOrgs.ListRootsInput) (*awsOrgs.ListRootsOutput, error) { + return &awsOrgs.ListRootsOutput{Roots: []awsOrgsTypes.Root{{ + Id: awsSdk.String(testRootID), + Name: awsSdk.String("Root"), + }}}, nil + }, + } + builder := organizationBuilder(orgs) + + resources, res, err := builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Nil(t, res) + require.Len(t, resources, 1) + assert.Equal(t, resourceTypeOrganization.Id, resources[0].Id.ResourceType) + assert.Equal(t, testRootID, resources[0].Id.Resource) + assert.Nil(t, resources[0].ParentResourceId, "root is the top tier; no parent") +} + +// organization.List degrades to no hierarchy (no error) when org-read permission is missing. +func TestOrganizationList_FailSoftAccessDenied(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listRootsFn: func(_ *awsOrgs.ListRootsInput) (*awsOrgs.ListRootsOutput, error) { + return nil, &awsOrgsTypes.AccessDeniedException{Message: awsSdk.String("no perms")} + }, + } + resources, res, err := organizationBuilder(orgs).List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err, "missing org-read permission must not abort the sync") + assert.Nil(t, res) + assert.Empty(t, resources) +} + +// organizationalUnit.List crawls child OUs under a root and parents them to the crawl seed. +func TestOrganizationalUnitList_CrawlsUnderRoot(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listOUsFn: func(in *awsOrgs.ListOrganizationalUnitsForParentInput) (*awsOrgs.ListOrganizationalUnitsForParentOutput, error) { + assert.Equal(t, testRootID, awsSdk.ToString(in.ParentId)) + return &awsOrgs.ListOrganizationalUnitsForParentOutput{OrganizationalUnits: []awsOrgsTypes.OrganizationalUnit{{ + Id: awsSdk.String(testOUID), + Name: awsSdk.String("Engineering"), + }}}, nil + }, + } + parent := &v2.ResourceId{ResourceType: resourceTypeOrganization.Id, Resource: testRootID} + resources, _, err := organizationalUnitBuilder(orgs).List(ctx, parent, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, resources, 1) + assert.Equal(t, resourceTypeOrganizationalUnit.Id, resources[0].Id.ResourceType) + assert.Equal(t, testOUID, resources[0].Id.Resource) + require.NotNil(t, resources[0].ParentResourceId) + assert.Equal(t, resourceTypeOrganization.Id, resources[0].ParentResourceId.ResourceType) + assert.Equal(t, testRootID, resources[0].ParentResourceId.Resource) +} + +// organizationalUnit.List recurses into nested OUs (parent is another OU). +func TestOrganizationalUnitList_CrawlsNestedUnderOU(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listOUsFn: func(in *awsOrgs.ListOrganizationalUnitsForParentInput) (*awsOrgs.ListOrganizationalUnitsForParentOutput, error) { + assert.Equal(t, testOUID, awsSdk.ToString(in.ParentId)) + return &awsOrgs.ListOrganizationalUnitsForParentOutput{OrganizationalUnits: []awsOrgsTypes.OrganizationalUnit{{ + Id: awsSdk.String(testOUID2), + Name: awsSdk.String("Platform"), + }}}, nil + }, + } + parent := &v2.ResourceId{ResourceType: resourceTypeOrganizationalUnit.Id, Resource: testOUID} + resources, _, err := organizationalUnitBuilder(orgs).List(ctx, parent, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, resources, 1) + assert.Equal(t, testOUID2, resources[0].Id.Resource) + assert.Equal(t, resourceTypeOrganizationalUnit.Id, resources[0].ParentResourceId.ResourceType) + assert.Equal(t, testOUID, resources[0].ParentResourceId.Resource) +} + +// organizationalUnit.List is gated: it only crawls under a root or another OU, never at the +// top level or under an unrelated parent type. +func TestOrganizationalUnitList_GatedOnParentType(t *testing.T) { + ctx := context.Background() + called := false + orgs := &fakeOrgs{ + listOUsFn: func(_ *awsOrgs.ListOrganizationalUnitsForParentInput) (*awsOrgs.ListOrganizationalUnitsForParentOutput, error) { + called = true + return &awsOrgs.ListOrganizationalUnitsForParentOutput{}, nil + }, + } + builder := organizationalUnitBuilder(orgs) + + resources, _, err := builder.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + assert.Empty(t, resources) + + resources, _, err = builder.List(ctx, &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID}, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + assert.Empty(t, resources) + assert.False(t, called, "must not call ListOrganizationalUnitsForParent for a non-root/OU parent") +} + +// accountParentResourceID maps an Organizations ROOT parent to an organization scope id. +func TestAccountParentResourceID_Root(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listParentsFn: func(in *awsOrgs.ListParentsInput) (*awsOrgs.ListParentsOutput, error) { + assert.Equal(t, testAccountID, awsSdk.ToString(in.ChildId)) + return &awsOrgs.ListParentsOutput{Parents: []awsOrgsTypes.Parent{{ + Id: awsSdk.String(testRootID), + Type: awsOrgsTypes.ParentTypeRoot, + }}}, nil + }, + } + parent, accessDenied, err := accountParentResourceID(ctx, orgs, testAccountID) + require.NoError(t, err) + assert.False(t, accessDenied) + require.NotNil(t, parent) + assert.Equal(t, resourceTypeOrganization.Id, parent.ResourceType) + assert.Equal(t, testRootID, parent.Resource) +} + +// accountParentResourceID maps an Organizations OU parent to an organizational_unit scope id. +func TestAccountParentResourceID_OU(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listParentsFn: func(_ *awsOrgs.ListParentsInput) (*awsOrgs.ListParentsOutput, error) { + return &awsOrgs.ListParentsOutput{Parents: []awsOrgsTypes.Parent{{ + Id: awsSdk.String(testOUID), + Type: awsOrgsTypes.ParentTypeOrganizationalUnit, + }}}, nil + }, + } + parent, accessDenied, err := accountParentResourceID(ctx, orgs, testAccountID) + require.NoError(t, err) + assert.False(t, accessDenied) + require.NotNil(t, parent) + assert.Equal(t, resourceTypeOrganizationalUnit.Id, parent.ResourceType) + assert.Equal(t, testOUID, parent.Resource) +} + +// accountParentResourceID reports accessDenied (not an error) when org-read perms are missing, +// so the account builder degrades to a flat account. +func TestAccountParentResourceID_FailSoftAccessDenied(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listParentsFn: func(_ *awsOrgs.ListParentsInput) (*awsOrgs.ListParentsOutput, error) { + return nil, &awsOrgsTypes.AccessDeniedException{Message: awsSdk.String("no perms")} + }, + } + parent, accessDenied, err := accountParentResourceID(ctx, orgs, testAccountID) + require.NoError(t, err) + assert.True(t, accessDenied) + assert.Nil(t, parent) +} + +// account.List re-parents each active account under its Root/OU via ListParents. +func TestAccountList_ReParentsUnderHierarchy(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listAccountsFn: func(_ *awsOrgs.ListAccountsInput) (*awsOrgs.ListAccountsOutput, error) { + return &awsOrgs.ListAccountsOutput{Accounts: []awsOrgsTypes.Account{{ + Id: awsSdk.String(testAccountID), + Name: awsSdk.String("prod"), + Status: awsOrgsTypes.AccountStatusActive, + }}}, nil + }, + listParentsFn: func(_ *awsOrgs.ListParentsInput) (*awsOrgs.ListParentsOutput, error) { + return &awsOrgs.ListParentsOutput{Parents: []awsOrgsTypes.Parent{{ + Id: awsSdk.String(testOUID), + Type: awsOrgsTypes.ParentTypeOrganizationalUnit, + }}}, nil + }, + } + acct := newOrgAccount(orgs) + + resources, _, err := acct.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err) + require.Len(t, resources, 1) + require.NotNil(t, resources[0].ParentResourceId, "account must be parented to its OU") + assert.Equal(t, resourceTypeOrganizationalUnit.Id, resources[0].ParentResourceId.ResourceType) + assert.Equal(t, testOUID, resources[0].ParentResourceId.Resource) +} + +// account.List degrades to a flat (parentless) account when ListParents is denied — no error. +func TestAccountList_FlatWhenOrgReadDenied(t *testing.T) { + ctx := context.Background() + orgs := &fakeOrgs{ + listAccountsFn: func(_ *awsOrgs.ListAccountsInput) (*awsOrgs.ListAccountsOutput, error) { + return &awsOrgs.ListAccountsOutput{Accounts: []awsOrgsTypes.Account{{ + Id: awsSdk.String(testAccountID), + Name: awsSdk.String("prod"), + Status: awsOrgsTypes.AccountStatusActive, + }}}, nil + }, + listParentsFn: func(_ *awsOrgs.ListParentsInput) (*awsOrgs.ListParentsOutput, error) { + return nil, &awsOrgsTypes.AccessDeniedException{Message: awsSdk.String("no perms")} + }, + } + acct := newOrgAccount(orgs) + + resources, _, err := acct.List(ctx, nil, resourceSdk.SyncOpAttrs{}) + require.NoError(t, err, "missing org-read permission must not abort account sync") + require.Len(t, resources, 1) + assert.Nil(t, resources[0].ParentResourceId, "account stays flat when re-parenting is denied") +} + +// newOrgAccount builds an accountResourceType backed by the given fakeOrgs (SSO client unused +// by the List/re-parent path under test). +func newOrgAccount(orgs *fakeOrgs) *accountResourceType { + identityInstance := &awsSsoAdminTypes.InstanceMetadata{ + InstanceArn: awsSdk.String(behaviorInstanceArn), + IdentityStoreId: awsSdk.String(behaviorIdentityStoreID), + } + return accountBuilder(orgs, "", &fakeSSOAdmin{}, identityInstance, behaviorRegion, nil) +} diff --git a/pkg/connector/permission_set_assignment_behavior_test.go b/pkg/connector/permission_set_assignment_behavior_test.go index 28f879aa..05c960d4 100644 --- a/pkg/connector/permission_set_assignment_behavior_test.go +++ b/pkg/connector/permission_set_assignment_behavior_test.go @@ -115,17 +115,55 @@ func (f *fakeSSOAdmin) DescribeAccountAssignmentDeletionStatus( } // fakeOrgs is an orgsAPI that reports every account active, so the provision/deprovision -// status-verification step passes without a real AWS Organizations call. -type fakeOrgs struct{} +// status-verification step passes without a real AWS Organizations call. The org-tree methods +// delegate to pluggable funcs; nil funcs return an empty, successful response. +type fakeOrgs struct { + listAccountsFn func(*awsOrgs.ListAccountsInput) (*awsOrgs.ListAccountsOutput, error) + listRootsFn func(*awsOrgs.ListRootsInput) (*awsOrgs.ListRootsOutput, error) + listOUsFn func(*awsOrgs.ListOrganizationalUnitsForParentInput) (*awsOrgs.ListOrganizationalUnitsForParentOutput, error) + listParentsFn func(*awsOrgs.ListParentsInput) (*awsOrgs.ListParentsOutput, error) + describeAccountFn func(*awsOrgs.DescribeAccountInput) (*awsOrgs.DescribeAccountOutput, error) +} func (f *fakeOrgs) DescribeAccount(_ context.Context, in *awsOrgs.DescribeAccountInput, _ ...func(*awsOrgs.Options)) (*awsOrgs.DescribeAccountOutput, error) { + if f.describeAccountFn != nil { + return f.describeAccountFn(in) + } return &awsOrgs.DescribeAccountOutput{Account: &awsOrgsTypes.Account{Id: in.AccountId, Status: awsOrgsTypes.AccountStatusActive}}, nil } -func (f *fakeOrgs) ListAccounts(_ context.Context, _ *awsOrgs.ListAccountsInput, _ ...func(*awsOrgs.Options)) (*awsOrgs.ListAccountsOutput, error) { +func (f *fakeOrgs) ListAccounts(_ context.Context, in *awsOrgs.ListAccountsInput, _ ...func(*awsOrgs.Options)) (*awsOrgs.ListAccountsOutput, error) { + if f.listAccountsFn != nil { + return f.listAccountsFn(in) + } return &awsOrgs.ListAccountsOutput{}, nil } +func (f *fakeOrgs) ListRoots(_ context.Context, in *awsOrgs.ListRootsInput, _ ...func(*awsOrgs.Options)) (*awsOrgs.ListRootsOutput, error) { + if f.listRootsFn != nil { + return f.listRootsFn(in) + } + return &awsOrgs.ListRootsOutput{}, nil +} + +func (f *fakeOrgs) ListOrganizationalUnitsForParent( + _ context.Context, + in *awsOrgs.ListOrganizationalUnitsForParentInput, + _ ...func(*awsOrgs.Options), +) (*awsOrgs.ListOrganizationalUnitsForParentOutput, error) { + if f.listOUsFn != nil { + return f.listOUsFn(in) + } + return &awsOrgs.ListOrganizationalUnitsForParentOutput{}, nil +} + +func (f *fakeOrgs) ListParents(_ context.Context, in *awsOrgs.ListParentsInput, _ ...func(*awsOrgs.Options)) (*awsOrgs.ListParentsOutput, error) { + if f.listParentsFn != nil { + return f.listParentsFn(in) + } + return &awsOrgs.ListParentsOutput{}, nil +} + const ( behaviorRegion = "us-east-1" behaviorIdentityStoreID = "d-1234567890" diff --git a/pkg/connector/resource_types.go b/pkg/connector/resource_types.go index bbe955d1..7ffb4352 100644 --- a/pkg/connector/resource_types.go +++ b/pkg/connector/resource_types.go @@ -64,6 +64,9 @@ var ( // Read "organizations:ListAccounts", "organizations:DescribeOrganization", + // Sparse ACLs hierarchy (Phase 2): resolve each account's parent (Root/OU) so + // c1's by-inheritance review can walk the org tree. Fail-soft if absent. + "organizations:ListParents", "sso:ListPermissionSets", "sso:DescribePermissionSet", "sso:ListPermissionSetsProvisionedToAccount", @@ -216,4 +219,41 @@ var ( ), ), } + + // resourceTypeOrganization is the AWS Organizations root, modeled as the top scope tier + // of the Sparse ACLs hierarchy (Root → OU → Account). It holds no binding — AWS has no + // native Root-level permission-set assignment — and exists purely as navigation / + // by-inheritance review context. SkipEntitlementsAndGrants + OptInRequired, like Azure's + // management-group tier. + resourceTypeOrganization = &v2.ResourceType{ + Id: "organization", + DisplayName: "Organization Root", + Annotations: annotations.New( + &v2.SkipEntitlementsAndGrants{}, + &v2.OptInRequired{}, + // The root is the crawl seed for the OU tree. + &v2.ChildResourceType{ResourceTypeId: "organizational_unit"}, + capabilityPermissions( + "organizations:ListRoots", + "organizations:ListOrganizationalUnitsForParent", + ), + ), + } + + // resourceTypeOrganizationalUnit is an AWS Organizations OU, an intermediate scope tier + // between the root and accounts. Like the root it carries no binding (no native OU-level + // assignment) and is hierarchy/review context only. It declares itself as a child type so + // the SDK recurses into nested OUs. SkipEntitlementsAndGrants + OptInRequired. + resourceTypeOrganizationalUnit = &v2.ResourceType{ + Id: "organizational_unit", + DisplayName: "Organizational Unit", + Annotations: annotations.New( + &v2.SkipEntitlementsAndGrants{}, + &v2.OptInRequired{}, + &v2.ChildResourceType{ResourceTypeId: "organizational_unit"}, + capabilityPermissions( + "organizations:ListOrganizationalUnitsForParent", + ), + ), + } ) diff --git a/pkg/connector/ssoadmin_api.go b/pkg/connector/ssoadmin_api.go index 786a17eb..6bf7dd4e 100644 --- a/pkg/connector/ssoadmin_api.go +++ b/pkg/connector/ssoadmin_api.go @@ -40,10 +40,21 @@ type ssoAdminAPI interface { ) (*awsSsoAdmin.DescribeAccountAssignmentDeletionStatusOutput, error) } -// orgsAPI is the subset of *organizations.Client the account builder calls. Same rationale and -// structural-satisfaction property as ssoAdminAPI; ListAccounts also doubles as the API client -// for NewListAccountsPaginator. +// orgsAPI is the subset of *organizations.Client the account and org-hierarchy builders call. +// Same rationale and structural-satisfaction property as ssoAdminAPI; ListAccounts also doubles +// as the API client for NewListAccountsPaginator. +// +// The ListRoots / ListOrganizationalUnitsForParent / ListParents methods back the Sparse ACLs +// Root → OU → Account hierarchy (Phase 2): the org / OU builders walk the tree top-down, and +// the account builder re-parents each account onto its Root/OU via ListParents. type orgsAPI interface { DescribeAccount(ctx context.Context, params *awsOrgs.DescribeAccountInput, optFns ...func(*awsOrgs.Options)) (*awsOrgs.DescribeAccountOutput, error) ListAccounts(ctx context.Context, params *awsOrgs.ListAccountsInput, optFns ...func(*awsOrgs.Options)) (*awsOrgs.ListAccountsOutput, error) + ListRoots(ctx context.Context, params *awsOrgs.ListRootsInput, optFns ...func(*awsOrgs.Options)) (*awsOrgs.ListRootsOutput, error) + ListOrganizationalUnitsForParent( + ctx context.Context, + params *awsOrgs.ListOrganizationalUnitsForParentInput, + optFns ...func(*awsOrgs.Options), + ) (*awsOrgs.ListOrganizationalUnitsForParentOutput, error) + ListParents(ctx context.Context, params *awsOrgs.ListParentsInput, optFns ...func(*awsOrgs.Options)) (*awsOrgs.ListParentsOutput, error) } From cefa7cda3e51bd3b926719016f21dcf4e6065b60 Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Mon, 22 Jun 2026 19:49:41 +0000 Subject: [PATCH 5/5] Paginate permission set assignment List in batches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit permissionSetAssignmentResourceType.List buffered every provisioned permission set for an account into a single response with no page token, doing one DescribePermissionSet per set synchronously — unlike account.Entitlements, which batches via entitlementsBatchSize. For accounts with many provisioned permission sets this is a large synchronous fan-out per call. Reuse the existing account-entitlement batching mechanism verbatim (entitlementsPageState + entitlementsBatchSize + encode/decodePageToken) so binding List paginates identically to account.Entitlements: at most one batch of bindings per call, with a page token resuming at the next index. Keeps the rate-limit blast radius small enough to resume from a checkpoint (D-SACL-007 fidelity — no new paging mechanism). Co-authored-by: c1-squire-dev[bot] --- pkg/connector/permission_set_assignment.go | 43 ++++++++++++- ...permission_set_assignment_behavior_test.go | 60 +++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/pkg/connector/permission_set_assignment.go b/pkg/connector/permission_set_assignment.go index e56fa54d..c8792b9b 100644 --- a/pkg/connector/permission_set_assignment.go +++ b/pkg/connector/permission_set_assignment.go @@ -72,14 +72,40 @@ func (o *permissionSetAssignmentResourceType) List(ctx context.Context, parentRe } accountID := parentResourceID.Resource + // Reuse the account-entitlement batching mechanism verbatim (entitlementsPageState + + // entitlementsBatchSize + encode/decodePageToken) so binding List paginates identically + // to account.Entitlements: one DescribePermissionSet fan-out is bounded to a batch per + // call instead of buffering every provisioned permission set at once, keeping the + // blast radius on a rate-limit error small enough to resume from a checkpoint. + pageState, err := decodePageToken[entitlementsPageState](opts.PageToken.Token) + if err != nil { + return nil, nil, fmt.Errorf("baton-aws: failed to decode page token: %w", err) + } + permissionSetIDs, err := o.account.getOrFetchPermissionSetIDs(ctx, opts.Session, accountID) if err != nil { return nil, nil, err } - rv := make([]*v2.Resource, 0, len(permissionSetIDs)) - for _, psArn := range permissionSetIDs { - ps, err := o.account.getPermissionSetWithCache(ctx, opts.Session, psArn) + // If no permission sets, we're done. + if len(permissionSetIDs) == 0 { + return nil, nil, nil + } + + // If we've processed all permission sets, we're done. + if pageState.PermissionSetIndex >= len(permissionSetIDs) { + return nil, nil, nil + } + + // Calculate the end of the current batch. + batchEnd := pageState.PermissionSetIndex + entitlementsBatchSize + if batchEnd > len(permissionSetIDs) { + batchEnd = len(permissionSetIDs) + } + + rv := make([]*v2.Resource, 0, batchEnd-pageState.PermissionSetIndex) + for i := pageState.PermissionSetIndex; i < batchEnd; i++ { + ps, err := o.account.getPermissionSetWithCache(ctx, opts.Session, permissionSetIDs[i]) if err != nil { return nil, nil, err } @@ -95,6 +121,17 @@ func (o *permissionSetAssignmentResourceType) List(ctx context.Context, parentRe rv = append(rv, resource) } + // Determine next page state; an empty token signals completion. + if batchEnd < len(permissionSetIDs) { + nextPageToken, err := encodePageToken(entitlementsPageState{ + PermissionSetIndex: batchEnd, + }) + if err != nil { + return nil, nil, fmt.Errorf("baton-aws: failed to encode page token: %w", err) + } + return rv, &resourceSdk.SyncOpResults{NextPageToken: nextPageToken}, nil + } + return rv, nil, nil } diff --git a/pkg/connector/permission_set_assignment_behavior_test.go b/pkg/connector/permission_set_assignment_behavior_test.go index 05c960d4..29430f9f 100644 --- a/pkg/connector/permission_set_assignment_behavior_test.go +++ b/pkg/connector/permission_set_assignment_behavior_test.go @@ -3,6 +3,7 @@ package connector import ( "context" "errors" + "fmt" "testing" awsSdk "github.com/aws/aws-sdk-go-v2/aws" @@ -12,6 +13,7 @@ import ( awsSsoAdminTypes "github.com/aws/aws-sdk-go-v2/service/ssoadmin/types" "github.com/conductorone/baton-aws/test" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" + "github.com/conductorone/baton-sdk/pkg/pagination" resourceSdk "github.com/conductorone/baton-sdk/pkg/types/resource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -228,6 +230,64 @@ func TestPermissionSetAssignmentList_EmitsScopeBinding(t *testing.T) { assert.Equal(t, testAccountID, resources[0].ParentResourceId.Resource) } +// List paginates the per-account binding fan-out in entitlementsBatchSize batches (mirroring +// account.Entitlements): each call emits at most one batch and returns a page token round-trip +// that resumes at the next index, and across all pages every provisioned permission set yields +// exactly one binding — no duplicates, none dropped. +func TestPermissionSetAssignmentList_PaginatesInBatches(t *testing.T) { + ctx := context.Background() + + // More than two full batches so we exercise a middle page plus a short final page. + const total = entitlementsBatchSize*2 + 10 + allArns := make([]string, 0, total) + for i := 0; i < total; i++ { + allArns = append(allArns, fmt.Sprintf("%s-%03d", testPermissionSetArn, i)) + } + + sso := &fakeSSOAdmin{ + listPermissionSetsProvisionedToAccountFn: func(in *awsSsoAdmin.ListPermissionSetsProvisionedToAccountInput) (*awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput, error) { + assert.Equal(t, testAccountID, awsSdk.ToString(in.AccountId)) + return &awsSsoAdmin.ListPermissionSetsProvisionedToAccountOutput{PermissionSets: allArns}, nil + }, + } + psa := permissionSetAssignmentBuilder(newBehaviorAccount(sso)) + parentID := &v2.ResourceId{ResourceType: resourceTypeAccount.Id, Resource: testAccountID} + + seen := make(map[string]int) + pageSizes := []int{} + token := "" + pages := 0 + for { + resources, res, err := psa.List(ctx, parentID, resourceSdk.SyncOpAttrs{PageToken: pagination.Token{Token: token}}) + require.NoError(t, err) + pageSizes = append(pageSizes, len(resources)) + for _, r := range resources { + seen[r.Id.Resource]++ + } + + if res == nil || res.NextPageToken == "" { + break + } + // The page token round-trips to the next batch boundary. + decoded, err := decodePageToken[entitlementsPageState](res.NextPageToken) + require.NoError(t, err) + assert.Equal(t, len(seen), decoded.PermissionSetIndex, "page token must resume exactly where this page left off") + token = res.NextPageToken + + pages++ + require.Less(t, pages, total, "pagination must terminate") + } + + // Batches are 25, 25, 10 — each call emits at most one batch. + assert.Equal(t, []int{entitlementsBatchSize, entitlementsBatchSize, 10}, pageSizes) + // Every provisioned permission set produced exactly one binding across all pages. + require.Len(t, seen, total) + for _, arn := range allArns { + wantID := permissionSetAssignmentObjectID(arn, testAccountID) + assert.Equal(t, 1, seen[wantID], "binding %s must be emitted exactly once", wantID) + } +} + // List only crawls bindings as a child of an account; any other parent yields nothing. func TestPermissionSetAssignmentList_GatedOnAccountParent(t *testing.T) { ctx := context.Background()