diff --git a/pkg/connector/aws_client_factory.go b/pkg/connector/aws_client_factory.go index fd74ea7b..e60525cc 100644 --- a/pkg/connector/aws_client_factory.go +++ b/pkg/connector/aws_client_factory.go @@ -5,16 +5,29 @@ import ( "fmt" "net/http" "sync" + "time" awsSdk "github.com/aws/aws-sdk-go-v2/aws" awsConfig "github.com/aws/aws-sdk-go-v2/config" + "github.com/aws/aws-sdk-go-v2/credentials/stscreds" "github.com/aws/aws-sdk-go-v2/service/iam" awsOrgs "github.com/aws/aws-sdk-go-v2/service/organizations" - "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "go.uber.org/zap" ) +// crossAccountRoleSessionName identifies baton's cross-account sessions in the +// target account's CloudTrail. +const crossAccountRoleSessionName = "BatonCrossAccountSession" + +// crossAccountSessionDuration is explicit because stscreds defaults to 15 minutes rather +// than inheriting the STS API's 1 hour; 1h is also the maximum role chaining permits. +const crossAccountSessionDuration = time.Hour + +// crossAccountCredentialRefreshWindow re-assumes this far before real expiry, so no request +// is signed with credentials that expire in flight (ExpiredToken is not retried). +const crossAccountCredentialRefreshWindow = 5 * time.Minute + type AWSClientFactory struct { mutex sync.Mutex @@ -22,6 +35,10 @@ type AWSClientFactory struct { baseClient *http.Client aws *AWS + // stsClientFn resolves the STS client used to assume into child accounts. It is a + // field so tests can inject a fake; production always uses (*AWS).getSTSClient. + stsClientFn func(ctx context.Context) (stscreds.AssumeRoleAPIClient, error) + // Map for accountId iamClientMap map[string]*iam.Client orgClientMap map[string]*awsOrgs.Client @@ -35,30 +52,56 @@ func NewAWSClientFactory(config Config, aws *AWS, baseClient *http.Client) *AWSC iamClientMap: make(map[string]*iam.Client), orgClientMap: make(map[string]*awsOrgs.Client), aws: aws, + stsClientFn: func(ctx context.Context) (stscreds.AssumeRoleAPIClient, error) { + return aws.getSTSClient(ctx) + }, } } +// getConfig builds an aws config for a child account backed by auto-refreshing +// assumed-role credentials. +// +// The credentials MUST be refreshing rather than a one-shot AssumeRole wrapped in a +// static provider: assumed-role sessions here last at most 1 hour (see +// crossAccountSessionDuration), while clients built here are cached for the lifetime of +// the process and created eagerly at account-listing time. Any sync where more than an +// hour elapsed between creation and use died mid-sync with `ExpiredToken`, and the cached +// client then failed every subsequent sync until the process restarted. +// aws.NewCredentialsCache re-assumes on expiry, which makes caching the client safe. This +// mirrors the connector's own credentials (see (*AWS).getCallingConfig). +// +// LoadDefaultConfig is retained deliberately: it is what lets child-account clients pick +// up ambient AWS settings (retry mode/attempts, FIPS and dualstack via ConfigSources, +// endpoint mode, app id). Hand-building an awsSdk.Config here would silently drop them. func (f *AWSClientFactory) getConfig(ctx context.Context, accountId string) (awsSdk.Config, error) { l := ctxzap.Extract(ctx) roleArn := fmt.Sprintf("arn:aws:iam::%s:role/%s", accountId, f.config.IamAssumeRoleName) - stsClient, err := f.aws.getSTSClient(ctx) + stsClient, err := f.stsClientFn(ctx) if err != nil { return awsSdk.Config{}, fmt.Errorf("baton-aws: getSTSClient failed: %w", err) } - output, err := stsClient.AssumeRole(ctx, &sts.AssumeRoleInput{ - RoleArn: awsSdk.String(roleArn), - RoleSessionName: awsSdk.String("BatonCrossAccountSession"), - }) - - if err != nil { + creds := awsSdk.NewCredentialsCache( + stscreds.NewAssumeRoleProvider(stsClient, roleArn, func(aro *stscreds.AssumeRoleOptions) { + aro.RoleSessionName = crossAccountRoleSessionName + aro.Duration = crossAccountSessionDuration + }), + func(o *awsSdk.CredentialsCacheOptions) { + o.ExpiryWindow = crossAccountCredentialRefreshWindow + }, + ) + + // stscreds providers are lazy — constructing one issues no API call. Retrieve once so + // this still reports whether the role is assumable at all, which callers rely on to + // skip inaccessible accounts (see accountIAMResourceType.parseAssumeRole). + if _, err := creds.Retrieve(ctx); err != nil { l.Warn("Failed to assume role", zap.Error(err), zap.String("roleArn", roleArn)) return awsSdk.Config{}, err } - opts := GetAwsConfigOptionsForAssumeRole(output, f.baseClient, f.config) + opts := GetAwsConfigOptionsForCredentials(creds, f.baseClient, f.config) baseConfig, err := awsConfig.LoadDefaultConfig(ctx, opts...) if err != nil { diff --git a/pkg/connector/aws_client_factory_test.go b/pkg/connector/aws_client_factory_test.go new file mode 100644 index 00000000..00e87eef --- /dev/null +++ b/pkg/connector/aws_client_factory_test.go @@ -0,0 +1,354 @@ +package connector + +import ( + "context" + "errors" + "fmt" + "net/http" + "os" + "sync" + "testing" + "time" + + awsSdk "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/credentials/stscreds" + awsIam "github.com/aws/aws-sdk-go-v2/service/iam" + awsOrgs "github.com/aws/aws-sdk-go-v2/service/organizations" + awsSsoAdminTypes "github.com/aws/aws-sdk-go-v2/service/ssoadmin/types" + "github.com/aws/aws-sdk-go-v2/service/sts" + stsTypes "github.com/aws/aws-sdk-go-v2/service/sts/types" + smithy "github.com/aws/smithy-go" + "github.com/stretchr/testify/require" +) + +// fakeSTSClient records AssumeRole calls and returns queued responses. Credentials +// expire after expiresIn so a CredentialsCache is forced to re-assume. +type fakeSTSClient struct { + mu sync.Mutex + calls int + expiresIn time.Duration + // errAfter, when > 0, makes every call at or beyond that ordinal fail with err. + errAfter int + err error +} + +func (f *fakeSTSClient) AssumeRole(ctx context.Context, in *sts.AssumeRoleInput, _ ...func(*sts.Options)) (*sts.AssumeRoleOutput, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.calls++ + + if f.errAfter > 0 && f.calls >= f.errAfter { + return nil, f.err + } + + return &sts.AssumeRoleOutput{Credentials: fakeCredentials(f.expiresIn)}, nil +} + +func (f *fakeSTSClient) callCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.calls +} + +func fakeCredentials(expiresIn time.Duration) *stsTypes.Credentials { + return &stsTypes.Credentials{ + AccessKeyId: awsSdk.String("AKIAFAKE"), + SecretAccessKey: awsSdk.String("secret"), + SessionToken: awsSdk.String("token"), + Expiration: awsSdk.Time(time.Now().Add(expiresIn)), + } +} + +// isolateAWSEnv detaches the process's ambient AWS configuration for the duration of a +// test. getConfig goes through awsConfig.LoadDefaultConfig, which reads the environment +// and the shared config files, so without this a developer machine with AWS_PROFILE set +// fails these tests with "failed to get shared config profile" instead of exercising the +// code under test. Only profile/credential-file resolution is neutralized — tests that +// assert on ambient settings set their own AWS_* vars on top of this. +func isolateAWSEnv(t *testing.T) { + t.Helper() + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_DEFAULT_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", os.DevNull) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", os.DevNull) + // Credentials are always supplied explicitly here, so a stray IMDS probe would only + // add latency and a dependency on the host. + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") +} + +func newTestFactory(t *testing.T, stsClient stscreds.AssumeRoleAPIClient) *AWSClientFactory { + t.Helper() + isolateAWSEnv(t) + + return &AWSClientFactory{ + mutex: sync.Mutex{}, + config: Config{GlobalRegion: "us-east-1", IamAssumeRoleName: "BatonRole"}, + baseClient: http.DefaultClient, + iamClientMap: make(map[string]*awsIam.Client), + orgClientMap: make(map[string]*awsOrgs.Client), + stsClientFn: func(ctx context.Context) (stscreds.AssumeRoleAPIClient, error) { + return stsClient, nil + }, + } +} + +// TestGetConfigCredentialsRefreshOnExpiry is the regression test for CXP-801: the +// credentials a cross-account client is built with must re-assume once the session +// expires, instead of being frozen at creation time by a static provider. +func TestGetConfigCredentialsRefreshOnExpiry(t *testing.T) { + ctx := context.Background() + // Already expired on arrival, so the second Retrieve must trigger a fresh AssumeRole. + fake := &fakeSTSClient{expiresIn: -time.Minute} + f := newTestFactory(t, fake) + + cfg, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + require.Equal(t, 1, fake.callCount(), "getConfig should probe assumability exactly once") + + _, err = cfg.Credentials.Retrieve(ctx) + require.NoError(t, err) + require.Equal(t, 2, fake.callCount(), "expired credentials must trigger a re-assume, not be served stale") +} + +// TestGetConfigCredentialsCachedWhileValid guards the other direction: an unexpired +// session must not re-assume on every request. +func TestGetConfigCredentialsCachedWhileValid(t *testing.T) { + ctx := context.Background() + fake := &fakeSTSClient{expiresIn: time.Hour} + f := newTestFactory(t, fake) + + cfg, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + + for range 5 { + _, err = cfg.Credentials.Retrieve(ctx) + require.NoError(t, err) + } + require.Equal(t, 1, fake.callCount(), "valid credentials should be served from cache") +} + +// TestGetConfigRefreshesBeforeExpiry pins the refresh window. Credentials whose remaining +// life falls inside crossAccountCredentialRefreshWindow must be re-assumed rather than +// served, because a request signed with them can reach AWS after they expire and come back +// 403 ExpiredToken — which is absent from the SDK's retryable error codes. +func TestGetConfigRefreshesBeforeExpiry(t *testing.T) { + ctx := context.Background() + // Still valid, but well inside the refresh window. + fake := &fakeSTSClient{expiresIn: crossAccountCredentialRefreshWindow / 2} + f := newTestFactory(t, fake) + + cfg, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + + before := fake.callCount() + _, err = cfg.Credentials.Retrieve(ctx) + require.NoError(t, err) + require.Greater(t, fake.callCount(), before, + "credentials expiring inside the refresh window must be re-assumed, not signed with") +} + +// TestGetConfigProbesAssumability covers what accountIAMResourceType.parseAssumeRole +// relies on: an unassumable role must surface as an error from getConfig. stscreds +// providers are lazy, so without the eager Retrieve this would silently succeed and the +// account would fail later mid-crawl instead of being skipped. +func TestGetConfigProbesAssumability(t *testing.T) { + ctx := context.Background() + fake := &fakeSTSClient{ + expiresIn: time.Hour, + errAfter: 1, + err: stsAccessDenied(), + } + f := newTestFactory(t, fake) + + _, err := f.getConfig(ctx, "123456789012") + require.Error(t, err, "getConfig must report a role it cannot assume") + require.Equal(t, 1, fake.callCount()) +} + +// TestGetConfigUsesStableSessionName keeps the CloudTrail session name intact. +func TestGetConfigUsesStableSessionName(t *testing.T) { + ctx := context.Background() + rec := &sessionNameRecorder{expiresIn: time.Hour} + f := newTestFactory(t, rec) + + _, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + require.Equal(t, crossAccountRoleSessionName, rec.sessionName) +} + +// TestGetConfigResolvesAmbientAWSSettings pins the behavior that makes getConfig go +// through awsConfig.LoadDefaultConfig rather than hand-building an awsSdk.Config: +// child-account clients must keep resolving ambient AWS settings. ConfigSources is the +// load-bearing one — service clients read FIPS/dualstack and endpoint settings out of it +// at construction, so an empty ConfigSources silently disables AWS_USE_FIPS_ENDPOINT for +// cross-account clients. +func TestGetConfigResolvesAmbientAWSSettings(t *testing.T) { + t.Setenv("AWS_RETRY_MODE", "adaptive") + t.Setenv("AWS_MAX_ATTEMPTS", "7") + t.Setenv("AWS_SDK_UA_APP_ID", "baton-aws-probe") + + ctx := context.Background() + f := newTestFactory(t, &fakeSTSClient{expiresIn: time.Hour}) + + cfg, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + + require.Equal(t, awsSdk.RetryModeAdaptive, cfg.RetryMode, "AWS_RETRY_MODE must reach child-account clients") + require.Equal(t, 7, cfg.RetryMaxAttempts, "AWS_MAX_ATTEMPTS must reach child-account clients") + require.Equal(t, "baton-aws-probe", cfg.AppID) + require.NotEmpty(t, cfg.ConfigSources, "ConfigSources must be populated so FIPS/dualstack/endpoint settings resolve") + + // The pinned fields must still win over anything ambient. + require.Equal(t, "us-east-1", cfg.Region) + require.Equal(t, http.DefaultClient, cfg.HTTPClient) +} + +// TestGetConfigPreservesRefreshingCredentialsThroughLoadDefaultConfig guards the +// interaction between the two halves of the fix: LoadDefaultConfig must hand back the +// *awsSdk.CredentialsCache it was given rather than re-wrapping or replacing it, or the +// refresh behavior would be silently lost on the way out. +func TestGetConfigPreservesRefreshingCredentialsThroughLoadDefaultConfig(t *testing.T) { + ctx := context.Background() + fake := &fakeSTSClient{expiresIn: -time.Minute} + f := newTestFactory(t, fake) + + cfg, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + require.IsType(t, &awsSdk.CredentialsCache{}, cfg.Credentials, + "LoadDefaultConfig must pass the CredentialsCache through untouched") + + before := fake.callCount() + _, err = cfg.Credentials.Retrieve(ctx) + require.NoError(t, err) + require.Greater(t, fake.callCount(), before, + "credentials returned by getConfig must still re-assume on expiry") +} + +// TestGetConfigRoleARN verifies the assumed role ARN is composed from the account id +// and the configured role name. +func TestGetConfigRoleARN(t *testing.T) { + ctx := context.Background() + rec := &sessionNameRecorder{expiresIn: time.Hour} + f := newTestFactory(t, rec) + + _, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + require.Equal(t, "arn:aws:iam::123456789012:role/BatonRole", rec.roleARN) +} + +// TestGetConfigRequestsOneHourSessions pins the session length actually sent to STS. +// stscreds does not inherit the STS API's 1-hour default: AssumeRoleProvider.Retrieve +// back-fills an unset Duration with stscreds.DefaultDuration (15 minutes) and always sends +// DurationSeconds. Dropping aro.Duration would therefore quarter the session length and +// quadruple AssumeRole volume across every child account, silently and with no test +// failure — hence this assertion on the wire value rather than on the constant. +func TestGetConfigRequestsOneHourSessions(t *testing.T) { + ctx := context.Background() + rec := &sessionNameRecorder{expiresIn: time.Hour} + f := newTestFactory(t, rec) + + _, err := f.getConfig(ctx, "123456789012") + require.NoError(t, err) + require.Equal(t, int32(3600), awsSdk.ToInt32(rec.durationSeconds), + "cross-account sessions must request 3600s: the maximum role chaining permits, "+ + "and what the pre-refresh code received from the STS API default") +} + +type sessionNameRecorder struct { + expiresIn time.Duration + sessionName string + roleARN string + durationSeconds *int32 +} + +func (r *sessionNameRecorder) AssumeRole(ctx context.Context, in *sts.AssumeRoleInput, _ ...func(*sts.Options)) (*sts.AssumeRoleOutput, error) { + r.sessionName = awsSdk.ToString(in.RoleSessionName) + r.roleARN = awsSdk.ToString(in.RoleArn) + r.durationSeconds = in.DurationSeconds + return &sts.AssumeRoleOutput{Credentials: fakeCredentials(r.expiresIn)}, nil +} + +// TestIsAccessDeniedErrorSSOAdmin covers the second half of CXP-801: SSO Admin returns a +// typed AccessDeniedException whose code is "AccessDeniedException", which the old +// "AccessDenied"-only comparison missed, leaving the fail-soft skips in permission_set.go +// and inline_policy.go dead. +func TestIsAccessDeniedErrorSSOAdmin(t *testing.T) { + t.Run("iam unmodeled AccessDenied", func(t *testing.T) { + require.True(t, isAccessDeniedError(iamAccessDenied())) + }) + + t.Run("sso admin typed AccessDeniedException", func(t *testing.T) { + err := &awsSsoAdminTypes.AccessDeniedException{Message: awsSdk.String("nope")} + require.True(t, isAccessDeniedError(wrapAsOperationError("SSO Admin", "ListManagedPoliciesInPermissionSet", err))) + }) + + t.Run("unrelated error", func(t *testing.T) { + require.False(t, isAccessDeniedError(errors.New("boom"))) + }) + + t.Run("different api error code", func(t *testing.T) { + err := &smithy.GenericAPIError{Code: "ValidationException", Message: "bad input"} + require.False(t, isAccessDeniedError(wrapAsOperationError("IAM", "ListGroups", err))) + }) + + t.Run("nil", func(t *testing.T) { + require.False(t, isAccessDeniedError(nil)) + }) +} + +// TestCredentialsFailureIsNotTreatedAsResourceDenial is the guard against the regression +// the refreshing-credentials fix would otherwise introduce. Once credentials re-assume +// lazily, an STS AccessDenied surfaces at the IAM call site through the credentials +// chain. If isAccessDeniedError matched it, every fail-soft skip would swallow a +// credentials outage and the sync would report success with grants silently missing — +// which reads downstream as revoked access. +func TestCredentialsFailureIsNotTreatedAsResourceDenial(t *testing.T) { + // Mirrors the real wrap chain: + // IAM OperationError → "get identity: %w" → "get credentials: %w" → STS OperationError + stsErr := wrapAsOperationError(sts.ServiceID, "AssumeRole", &smithy.GenericAPIError{ + Code: errCodeAccessDenied, + Message: "User is not authorized to perform: sts:AssumeRole", + }) + credsErr := fmt.Errorf("get identity: %w", fmt.Errorf("get credentials: %w", stsErr)) + iamErr := wrapAsOperationError(awsIam.ServiceID, "ListAttachedUserPolicies", credsErr) + + require.True(t, isCredentialsRetrievalError(iamErr), + "an STS error nested under an IAM operation must be recognized as a credentials failure") + require.False(t, isAccessDeniedError(iamErr), + "a credentials failure must NOT be skippable — it has to fail the sync loudly") +} + +// TestIsCredentialsRetrievalErrorIgnoresResourceDenials ensures a genuine IAM denial is +// still skippable, i.e. the new guard is not over-broad. +func TestIsCredentialsRetrievalErrorIgnoresResourceDenials(t *testing.T) { + err := iamAccessDenied() + require.False(t, isCredentialsRetrievalError(err)) + require.True(t, isAccessDeniedError(err)) +} + +// TestIsCredentialsRetrievalErrorDirectSTSCall documents that a denial raised by a direct +// STS call (not through the credentials chain) is also classified as a credentials +// failure. baton-aws only calls STS to obtain credentials, so this is the correct read. +func TestIsCredentialsRetrievalErrorDirectSTSCall(t *testing.T) { + err := stsAccessDenied() + require.True(t, isCredentialsRetrievalError(err)) + require.False(t, isAccessDeniedError(err)) +} + +func iamAccessDenied() error { + return wrapAsOperationError(awsIam.ServiceID, "ListAttachedUserPolicies", &smithy.GenericAPIError{ + Code: errCodeAccessDenied, + Message: "not authorized", + }) +} + +func stsAccessDenied() error { + return wrapAsOperationError(sts.ServiceID, "AssumeRole", &smithy.GenericAPIError{ + Code: errCodeAccessDenied, + Message: "not authorized to assume role", + }) +} + +func wrapAsOperationError(serviceID, op string, err error) error { + return &smithy.OperationError{ServiceID: serviceID, OperationName: op, Err: err} +} diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index af79a8e7..8599d415 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -611,18 +611,23 @@ func GetAwsConfigOptions(httpClient *http.Client, config Config) []func(*awsConf return opts } -func GetAwsConfigOptionsForAssumeRole(output *sts.AssumeRoleOutput, httpClient *http.Client, config Config) []func(*awsConfig.LoadOptions) error { +// GetAwsConfigOptionsForCredentials builds load options for a config backed by an +// explicit credentials provider. +// +// These are passed to awsConfig.LoadDefaultConfig rather than used to hand-build an +// awsSdk.Config so that cross-account clients keep resolving ambient AWS settings — +// AWS_RETRY_MODE, AWS_MAX_ATTEMPTS, AWS_SDK_UA_APP_ID, request-compression settings, +// AccountIDEndpointMode, and (via ConfigSources) FIPS / dualstack / endpoint resolution. +// Only credentials, region, HTTP client, and defaults mode are pinned here. +// +// The provider is expected to be an *awsSdk.CredentialsCache; LoadDefaultConfig passes +// one through untouched instead of re-wrapping it, preserving its refresh behavior. +func GetAwsConfigOptionsForCredentials(creds awsSdk.CredentialsProvider, httpClient *http.Client, config Config) []func(*awsConfig.LoadOptions) error { opts := []func(*awsConfig.LoadOptions) error{ awsConfig.WithHTTPClient(httpClient), awsConfig.WithRegion(config.GlobalRegion), awsConfig.WithDefaultsMode(awsSdk.DefaultsModeInRegion), - awsConfig.WithCredentialsProvider( - credentials.NewStaticCredentialsProvider( - *output.Credentials.AccessKeyId, - *output.Credentials.SecretAccessKey, - *output.Credentials.SessionToken, - ), - ), + awsConfig.WithCredentialsProvider(creds), } return opts diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index f5101189..528dca0e 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -14,6 +14,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws/arn" aws_middleware "github.com/aws/aws-sdk-go-v2/aws/middleware" awsIdentityStoreTypes "github.com/aws/aws-sdk-go-v2/service/identitystore/types" + "github.com/aws/aws-sdk-go-v2/service/sts" smithy "github.com/aws/smithy-go" "github.com/aws/smithy-go/middleware" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" @@ -23,6 +24,14 @@ import ( "google.golang.org/protobuf/proto" ) +const ( + // errCodeAccessDenied is the unmodeled code IAM (and STS) return for an authorization + // failure; errCodeAccessDeniedException is the code carried by service-modeled + // AccessDeniedException types, e.g. SSO Admin's. + errCodeAccessDenied = "AccessDenied" + errCodeAccessDeniedException = "AccessDeniedException" +) + const ( MembershipEntitlementIDTemplate = "%s:%s:member" V1MembershipEntitlementIDTemplate = "membership:%s" @@ -428,13 +437,62 @@ func wrapAWSError(err error) error { return err } +// isCredentialsRetrievalError reports whether err originated in an STS credential +// retrieval rather than in the API call the caller actually made. +// +// Cross-account clients hold auto-refreshing assumed-role credentials, so when a +// re-assume fails mid-sync (role deleted, trust policy edited, SCP change) STS returns +// AccessDenied and the SDK surfaces it at the call site — the whole chain is %w-wrapped +// and reachable by errors.As: +// +// iam. err → smithy.OperationError{ServiceID:"IAM"} → "get identity: %w" +// → "get credentials: %w" → CredentialsCache → AssumeRoleProvider +// → smithy.OperationError{ServiceID:"STS"} → GenericAPIError{Code:"AccessDenied"} +// +// Without this check isAccessDeniedError matches that error and the fail-soft skips +// treat a credentials outage as "this resource denied us", producing a sync that reports +// success with grants silently missing. Absent grants read as revoked access downstream, +// so a partial sync is worse than a hard failure: credential problems must be loud. +// +// A plain errors.As is not enough: it matches the OUTERMOST *smithy.OperationError, which +// is the caller's own service (IAM), and would never see the STS one nested beneath it. +// So each match that is not STS is stepped past, and the search resumes underneath it. +func isCredentialsRetrievalError(err error) bool { + for e := err; e != nil; { + var opErr *smithy.OperationError + if !errors.As(e, &opErr) { + return false + } + if opErr.ServiceID == sts.ServiceID { + return true + } + e = errors.Unwrap(opErr) + } + return false +} + +// isAccessDeniedError reports whether err is a resource-level authorization denial that +// the caller may safely skip. +// +// Both spellings are matched: IAM models no AccessDenied* error type and returns an +// unmodeled GenericAPIError with code "AccessDenied", while SSO Admin returns a typed +// *ssoadmin.AccessDeniedException whose ErrorCode() is "AccessDeniedException". Matching +// only the former left the fail-soft skips in permission_set.go and inline_policy.go dead +// for SSO Admin, failing the sync instead of degrading. +// +// Credential-retrieval failures are explicitly excluded — see isCredentialsRetrievalError. func isAccessDeniedError(err error) bool { + if isCredentialsRetrievalError(err) { + return false + } + var apiErr smithy.APIError if !errors.As(err, &apiErr) { return false } + switch apiErr.ErrorCode() { - case "AccessDenied", "AccessDeniedException": + case errCodeAccessDenied, errCodeAccessDeniedException: return true default: return false