From 01fec04ea8c0e33a406e6727801f8bc133a21196 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Sat, 14 Dec 2024 08:05:53 +0000 Subject: [PATCH 1/5] fix(cli): getting credentials via SSO fails when the region is set in 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 https://github.com/aws/aws-cdk/issues/32510. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-cdk/lib/api/aws-auth/awscli-compatible.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts b/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts index 319e75e3bdb79..3c1fec2604abd 100644 --- a/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts +++ b/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts @@ -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), }; /** @@ -51,6 +64,7 @@ export class AwsCliCompatible { ignoreCache: true, mfaCodeProvider: tokenCodeFn, clientConfig, + parentClientConfig, logger: options.logger, })); } @@ -83,6 +97,7 @@ export class AwsCliCompatible { const nodeProviderChain = fromNodeProviderChain({ profile: envProfile, clientConfig, + parentClientConfig, logger: options.logger, mfaCodeProvider: tokenCodeFn, ignoreCache: true, From f350aeb422f051ff49471259dc34a3f946b99a34 Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Sat, 14 Dec 2024 08:27:30 +0000 Subject: [PATCH 2/5] chore(release): 2.173.1 --- CHANGELOG.v2.alpha.md | 2 ++ CHANGELOG.v2.md | 7 +++++++ version.v2.json | 4 ++-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.v2.alpha.md b/CHANGELOG.v2.alpha.md index 20a61ed47ec77..4280ad430bc56 100644 --- a/CHANGELOG.v2.alpha.md +++ b/CHANGELOG.v2.alpha.md @@ -2,6 +2,8 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [2.173.1-alpha.0](https://github.com/aws/aws-cdk/compare/v2.173.0-alpha.0...v2.173.1-alpha.0) (2024-12-14) + ## [2.173.0-alpha.0](https://github.com/aws/aws-cdk/compare/v2.172.0-alpha.0...v2.173.0-alpha.0) (2024-12-11) diff --git a/CHANGELOG.v2.md b/CHANGELOG.v2.md index 0fab8ff3b811a..c188a43ac5618 100644 --- a/CHANGELOG.v2.md +++ b/CHANGELOG.v2.md @@ -2,6 +2,13 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [2.173.1](https://github.com/aws/aws-cdk/compare/v2.173.0...v2.173.1) (2024-12-14) + + +### Bug Fixes + +* **cli:** getting credentials via SSO fails when the region is set in the profile ([#32520](https://github.com/aws/aws-cdk/issues/32520)) ([01fec04](https://github.com/aws/aws-cdk/commit/01fec04ea8c0e33a406e6727801f8bc133a21196)) + ## [2.173.0](https://github.com/aws/aws-cdk/compare/v2.172.0...v2.173.0) (2024-12-11) diff --git a/version.v2.json b/version.v2.json index 4e5d312a6668b..31d52cf7acfd4 100644 --- a/version.v2.json +++ b/version.v2.json @@ -1,4 +1,4 @@ { - "version": "2.173.0", - "alphaVersion": "2.173.0-alpha.0" + "version": "2.173.1", + "alphaVersion": "2.173.1-alpha.0" } \ No newline at end of file From 7ee9b909695aca317a11aad16ca983dcc6d6f85a Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Tue, 17 Dec 2024 11:35:09 +0100 Subject: [PATCH 3/5] fix(cli): doesn't support plugins that return initially empty credentials (#32552) Plugins that return V2 credentials objects, return credentials that are self-refreshing. Those credentials can start out empty, which is perfectly valid. We shouldn't reject them if they are. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/api/aws-auth/credential-plugins.ts | 2 +- .../test/api/plugin/credential-plugin.test.ts | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts b/packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts index c587e0fc20040..b2047bd3fbbfb 100644 --- a/packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts +++ b/packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts @@ -165,7 +165,7 @@ function isV3Provider(x: PluginProviderResult): x is SDKv3CompatibleCredentialPr } function isV2Credentials(x: PluginProviderResult): x is SDKv2CompatibleCredentials { - return !!(x && typeof x === 'object' && x.accessKeyId && (x as SDKv2CompatibleCredentials).getPromise); + return !!(x && typeof x === 'object' && (x as SDKv2CompatibleCredentials).getPromise); } function isV3Credentials(x: PluginProviderResult): x is SDKv3CompatibleCredentials { diff --git a/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts b/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts index f665dc179db9c..77fc97731e58d 100644 --- a/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts +++ b/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts @@ -104,6 +104,26 @@ test('plugin can return V2 compatible credential-provider', async () => { expect(getPromise).toHaveBeenCalled(); }); +test('plugin can return V2 compatible credential-provider with initially empty keys', async () => { + // GIVEN + mockCredentialFunction(() => Promise.resolve({ + accessKeyId: '', + secretAccessKey: '', + expired: false, + getPromise() { + this.accessKeyId = 'keyid'; + return Promise.resolve({}); + }, + })); + + // WHEN + const creds = await fetchNow(); + + await expect(creds).toEqual(expect.objectContaining({ + accessKeyId: 'keyid', + })); +}); + test('plugin must not return something that is not a credential', async () => { // GIVEN mockCredentialFunction(() => Promise.resolve({ From e59b1db4d8da5fc11d0e3beeb136593440100325 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Tue, 17 Dec 2024 17:51:31 +0100 Subject: [PATCH 4/5] fix(cli): allow credential plugins to return `null` for `expiration` (#32554) According to the type definitions, the `expiration` field of V3 AWS credentials must be `undefined` or `Date`, but we are running into situations in reality where the value is `null`, leading to the error: ``` TypeError: Cannot read properties of null (reading 'getTime') ``` Survive that specific case. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk/lib/api/aws-auth/provider-caching.ts | 2 +- .../aws-cdk/test/api/plugin/credential-plugin.test.ts | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/aws-auth/provider-caching.ts b/packages/aws-cdk/lib/api/aws-auth/provider-caching.ts index 0fe68a1a637c6..22e4b357e191d 100644 --- a/packages/aws-cdk/lib/api/aws-auth/provider-caching.ts +++ b/packages/aws-cdk/lib/api/aws-auth/provider-caching.ts @@ -20,5 +20,5 @@ export function makeCachingProvider(provider: AwsCredentialIdentityProvider): Aw export function credentialsAboutToExpire(token: AwsCredentialIdentity) { const expiryMarginSecs = 5; - return token.expiration !== undefined && token.expiration.getTime() - Date.now() < expiryMarginSecs * 1000; + return !!token.expiration && token.expiration.getTime() - Date.now() < expiryMarginSecs * 1000; } diff --git a/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts b/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts index 77fc97731e58d..af5f6012ed09d 100644 --- a/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts +++ b/packages/aws-cdk/test/api/plugin/credential-plugin.test.ts @@ -1,4 +1,5 @@ import { CredentialPlugins } from '../../../lib/api/aws-auth/credential-plugins'; +import { credentialsAboutToExpire } from '../../../lib/api/aws-auth/provider-caching'; import { CredentialProviderSource, Mode, SDKv3CompatibleCredentials } from '../../../lib/api/plugin/credential-provider-source'; import { PluginHost, markTesting } from '../../../lib/api/plugin/plugin'; @@ -134,6 +135,15 @@ test('plugin must not return something that is not a credential', async () => { await expect(fetchNow()).rejects.toThrow(/Plugin returned a value that/); }); +test('token expiration is allowed to be null', () => { + expect(credentialsAboutToExpire({ + accessKeyId: 'key', + secretAccessKey: 'secret', + // This is not allowed according to the `.d.ts` contract, but it can happen in reality + expiration: null as any, + })).toEqual(false); +}); + function mockCredentialFunction(p: CredentialProviderSource['getProvider']) { mockCredentialPlugin({ name: 'test', From 522334fd63c8a90ffb3612cdba6748d1f18d383c Mon Sep 17 00:00:00 2001 From: HBobertz Date: Tue, 17 Dec 2024 14:22:54 -0500 Subject: [PATCH 5/5] chore(release): 2.173.2 --- CHANGELOG.v2.alpha.md | 2 ++ CHANGELOG.v2.md | 8 ++++++++ version.v2.json | 4 ++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.v2.alpha.md b/CHANGELOG.v2.alpha.md index 4280ad430bc56..c937b3a65832b 100644 --- a/CHANGELOG.v2.alpha.md +++ b/CHANGELOG.v2.alpha.md @@ -2,6 +2,8 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [2.173.2-alpha.0](https://github.com/aws/aws-cdk/compare/v2.173.1-alpha.0...v2.173.2-alpha.0) (2024-12-17) + ## [2.173.1-alpha.0](https://github.com/aws/aws-cdk/compare/v2.173.0-alpha.0...v2.173.1-alpha.0) (2024-12-14) ## [2.173.0-alpha.0](https://github.com/aws/aws-cdk/compare/v2.172.0-alpha.0...v2.173.0-alpha.0) (2024-12-11) diff --git a/CHANGELOG.v2.md b/CHANGELOG.v2.md index c188a43ac5618..bfe3129111fe4 100644 --- a/CHANGELOG.v2.md +++ b/CHANGELOG.v2.md @@ -2,6 +2,14 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [2.173.2](https://github.com/aws/aws-cdk/compare/v2.173.1...v2.173.2) (2024-12-17) + + +### Bug Fixes + +* **cli:** allow credential plugins to return `null` for `expiration` ([#32554](https://github.com/aws/aws-cdk/issues/32554)) ([e59b1db](https://github.com/aws/aws-cdk/commit/e59b1db4d8da5fc11d0e3beeb136593440100325)) +* **cli:** doesn't support plugins that return initially empty credentials ([#32552](https://github.com/aws/aws-cdk/issues/32552)) ([7ee9b90](https://github.com/aws/aws-cdk/commit/7ee9b909695aca317a11aad16ca983dcc6d6f85a)) + ## [2.173.1](https://github.com/aws/aws-cdk/compare/v2.173.0...v2.173.1) (2024-12-14) diff --git a/version.v2.json b/version.v2.json index 31d52cf7acfd4..e4c6ace5a1c12 100644 --- a/version.v2.json +++ b/version.v2.json @@ -1,4 +1,4 @@ { - "version": "2.173.1", - "alphaVersion": "2.173.1-alpha.0" + "version": "2.173.2", + "alphaVersion": "2.173.2-alpha.0" } \ No newline at end of file