Skip to content

Commit

Permalink
fix(cli): getting credentials via SSO fails when the region is set in…
Browse files Browse the repository at this point in the history
… the profile (#32520)

We were reading the region from the config file and passing it to the credential providers. 
However, in the case of SSO, this makes the credential provider use that region to do the 
SSO flow, which is incorrect. The region that should be used for that is the one set in the
`sso_session` section of the config file.

The long term solution is for all the logic for handling regions in the SDK itself, without
forcing consumers to know all the intricacies of all the use cases. As a mitigation for now,
we are using the non-public `parentClientConfig` while we wait for an SDK update.

Fixes #32510.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
otaviomacedo authored Dec 14, 2024
1 parent 27619cc commit bf026bd
Showing 1 changed file with 15 additions and 0 deletions.
15 changes: 15 additions & 0 deletions packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ export class AwsCliCompatible {
requestHandler: AwsCliCompatible.requestHandlerBuilder(options.httpOptions),
customUserAgent: 'aws-cdk',
logger: options.logger,
};

// Super hacky solution to https://github.com/aws/aws-cdk/issues/32510, proposed by the SDK team.
//
// Summary of the problem: we were reading the region from the config file and passing it to
// the credential providers. However, in the case of SSO, this makes the credential provider
// use that region to do the SSO flow, which is incorrect. The region that should be used for
// that is the one set in the sso_session section of the config file.
//
// The idea here: the "clientConfig" is for configuring the inner auth client directly,
// and has the highest priority, whereas "parentClientConfig" is the upper data client
// and has lower priority than the sso_region but still higher priority than STS global region.
const parentClientConfig = {
region: await this.region(options.profile),
};
/**
Expand All @@ -51,6 +64,7 @@ export class AwsCliCompatible {
ignoreCache: true,
mfaCodeProvider: tokenCodeFn,
clientConfig,
parentClientConfig,
logger: options.logger,
}));
}
Expand Down Expand Up @@ -83,6 +97,7 @@ export class AwsCliCompatible {
const nodeProviderChain = fromNodeProviderChain({
profile: envProfile,
clientConfig,
parentClientConfig,
logger: options.logger,
mfaCodeProvider: tokenCodeFn,
ignoreCache: true,
Expand Down

0 comments on commit bf026bd

Please sign in to comment.