From 425efbc3625d4512b83225dcb1d1c155d13b4b9e Mon Sep 17 00:00:00 2001 From: Otavio Macedo <288203+otaviomacedo@users.noreply.github.com> Date: Thu, 14 Nov 2024 15:44:12 +0000 Subject: [PATCH 1/3] fix(cli): failure to get credentials when session token is not set (#32134) In Node.js, if you assign `undefined` to an environment variable, that variable ends up having the string `"undefined"`. If we are using IAM user credentials, `AWS_SESSION_TOKEN` should not be set, but because we were not handling this edge case, it was getting assigned an invalid value: ``` Welcome to Node.js v22.9.0. Type ".help" for more information. > process.env.AWS_SESSION_TOKEN || process.env.AMAZON_SESSION_TOKEN undefined > process.env.AWS_SESSION_TOKEN = process.env.AWS_SESSION_TOKEN || process.env.AMAZON_SESSION_TOKEN undefined > process.env.AWS_SESSION_TOKEN 'undefined' ``` Closes https://github.com/aws/aws-cdk/issues/32120. ### Checklist - [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *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/awscli-compatible.ts | 7 ++- .../api/aws-auth/awscli-compatible.test.ts | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts 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 58bb2138ef9ba..782a7a2222b11 100644 --- a/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts +++ b/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts @@ -182,11 +182,16 @@ function caBundlePathFromEnvironment(): string | undefined { function shouldPrioritizeEnv() { const id = process.env.AWS_ACCESS_KEY_ID || process.env.AMAZON_ACCESS_KEY_ID; const key = process.env.AWS_SECRET_ACCESS_KEY || process.env.AMAZON_SECRET_ACCESS_KEY; - process.env.AWS_SESSION_TOKEN = process.env.AWS_SESSION_TOKEN || process.env.AMAZON_SESSION_TOKEN; if (!!id && !!key) { process.env.AWS_ACCESS_KEY_ID = id; process.env.AWS_SECRET_ACCESS_KEY = key; + + const sessionToken = process.env.AWS_SESSION_TOKEN ?? process.env.AMAZON_SESSION_TOKEN; + if (sessionToken) { + process.env.AWS_SESSION_TOKEN = sessionToken; + } + return true; } diff --git a/packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts b/packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts new file mode 100644 index 0000000000000..1c9ef12e45624 --- /dev/null +++ b/packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts @@ -0,0 +1,45 @@ +import { AwsCliCompatible } from '../../../lib/api/aws-auth/awscli-compatible'; + +describe('Session token', () => { + beforeEach(() => { + process.env.AWS_ACCESS_KEY_ID = 'foo'; + process.env.AWS_SECRET_ACCESS_KEY = 'bar'; + }); + + test('does not mess up with session token env variables if they are undefined', async () => { + // Making sure these variables are not defined + delete process.env.AWS_SESSION_TOKEN; + delete process.env.AMAZON_SESSION_TOKEN; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toBeUndefined(); + }); + + test('preserves AWS_SESSION_TOKEN if it is defined', async () => { + process.env.AWS_SESSION_TOKEN = 'aaa'; + delete process.env.AMAZON_SESSION_TOKEN; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toEqual('aaa'); + }); + + test('assigns AWS_SESSION_TOKEN if it is not defined but AMAZON_SESSION_TOKEN is', async () => { + delete process.env.AWS_SESSION_TOKEN; + process.env.AMAZON_SESSION_TOKEN = 'aaa'; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toEqual('aaa'); + }); + + test('preserves AWS_SESSION_TOKEN if both are defined', async () => { + process.env.AWS_SESSION_TOKEN = 'aaa'; + process.env.AMAZON_SESSION_TOKEN = 'bbb'; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toEqual('aaa'); + }); +}); \ No newline at end of file From 9859f33a11fb385419c611945bd9bb171b458dad Mon Sep 17 00:00:00 2001 From: Eli Polonsky Date: Thu, 14 Nov 2024 22:51:21 +0200 Subject: [PATCH 2/3] fix(cli): region specified in `~/.aws/credentials` is ignored (#32133) We just didn't consider the shared credentials file as returned by the SDK when loading configuration. Closes #32130. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *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/awscli-compatible.ts | 20 +- .../api/aws-auth/awscli-compatible.test.ts | 286 ++++++++++++++++++ 2 files changed, 305 insertions(+), 1 deletion(-) 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 782a7a2222b11..be6462243031f 100644 --- a/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts +++ b/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts @@ -149,7 +149,25 @@ export class AwsCliCompatible { */ async function getRegionFromIni(profile: string): Promise { const sharedFiles = await loadSharedConfigFiles({ ignoreCache: true }); - return sharedFiles?.configFile?.[profile]?.region || sharedFiles?.configFile?.default?.region; + + // Priority: + // + // credentials come before config because aws-cli v1 behaves like that. + // + // 1. profile-region-in-credentials + // 2. profile-region-in-config + // 3. default-region-in-credentials + // 4. default-region-in-config + + return getRegionFromIniFile(profile, sharedFiles.credentialsFile) + ?? getRegionFromIniFile(profile, sharedFiles.configFile) + ?? getRegionFromIniFile('default', sharedFiles.credentialsFile) + ?? getRegionFromIniFile('default', sharedFiles.configFile); + +} + +function getRegionFromIniFile(profile: string, data?: any) { + return data?.[profile]?.region; } function tryGetCACert(bundlePath?: string) { diff --git a/packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts b/packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts index 1c9ef12e45624..c8d7ccb886012 100644 --- a/packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts +++ b/packages/aws-cdk/test/api/aws-auth/awscli-compatible.test.ts @@ -1,5 +1,291 @@ +import * as os from 'os'; +import * as path from 'path'; +import * as fs from 'fs-extra'; import { AwsCliCompatible } from '../../../lib/api/aws-auth/awscli-compatible'; +describe('AwsCliCompatible.region', () => { + + beforeEach(() => { + + // make sure we don't mistakenly point to an unrelated file + process.env.AWS_CONFIG_FILE = '/dev/null'; + process.env.AWS_SHARED_CREDENTIALS_FILE = '/dev/null'; + + // these take precedence over the ini files so we need to disable them for + // the test to invoke the right function + delete process.env.AWS_REGION; + delete process.env.AMAZON_REGION; + delete process.env.AWS_DEFAULT_REGION; + delete process.env.AMAZON_DEFAULT_REGION; + + }); + + test('default region can be specified in config', async () => { + + const config = ` + [default] + region=region-in-config + `; + + await expect(region({ configFile: config })).resolves.toBe('region-in-config'); + }); + + test('default region can be specified in credentials', async () => { + + const creds = ` + [default] + region=region-in-credentials + `; + + await expect(region({ credentialsFile: creds })).resolves.toBe('region-in-credentials'); + + }); + + test('profile region can be specified in config', async () => { + + const config = ` + [profile user1] + region=region-in-config + `; + + await expect(region({ configFile: config, profile: 'user1' })).resolves.toBe('region-in-config'); + + }); + + test('profile region can be specified in credentials', async () => { + + const creds = ` + [user1] + region=region-in-credentials + `; + + await expect(region({ credentialsFile: creds, profile: 'user1' })).resolves.toBe('region-in-credentials'); + + }); + + test('with profile | profile-region-in-credentials is priority 1', async () => { + + const config = ` + [default] + region=default-region-in-config + + [profile user] + region=profile-region-in-config + + `; + + const creds = ` + [default] + region=default-region-in-credentials + + [user] + region=profile-region-in-credentials + `; + + await expect(region({ credentialsFile: creds, configFile: config, profile: 'user' })).resolves.toBe('profile-region-in-credentials'); + }); + + test('with profile | profile-region-in-config is priority 2', async () => { + + const config = ` + [default] + region=default-region-in-config + + [profile user] + region=profile-region-in-config + + `; + + const creds = ` + [default] + region=default-region-in-credentials + + [user] + `; + + await expect(region({ credentialsFile: creds, configFile: config, profile: 'user' })).resolves.toBe('profile-region-in-config'); + }); + + test('with profile | default-region-in-credentials is priority 3', async () => { + + const config = ` + [default] + region=default-region-in-config + + [profile user] + + `; + + const creds = ` + [default] + region=default-region-in-credentials + + [user] + `; + + await expect(region({ credentialsFile: creds, configFile: config, profile: 'user' })).resolves.toBe('default-region-in-credentials'); + }); + + test('with profile | default-region-in-config is priority 4', async () => { + + const config = ` + [default] + region=default-region-in-config + + [profile user] + + `; + + const creds = ` + [default] + + [user] + `; + + await expect(region({ credentialsFile: creds, configFile: config, profile: 'user' })).resolves.toBe('default-region-in-config'); + }); + + test('with profile | us-east-1 is priority 5', async () => { + + const config = ` + [default] + + [profile user] + + `; + + const creds = ` + [default] + + [user] + `; + + await expect(region({ credentialsFile: creds, configFile: config, profile: 'user' })).resolves.toBe('us-east-1'); + }); + + test('without profile | default-region-in-credentials is priority 1', async () => { + + const config = ` + [default] + region=default-region-in-config + + `; + + const creds = ` + [default] + region=default-region-in-credentials + + `; + + await expect(region({ credentialsFile: creds, configFile: config })).resolves.toBe('default-region-in-credentials'); + }); + + test('without profile | default-region-in-config is priority 2', async () => { + + const config = ` + [default] + region=default-region-in-config + + `; + + const creds = ` + [default] + + `; + + await expect(region({ credentialsFile: creds, configFile: config })).resolves.toBe('default-region-in-config'); + }); + + test('without profile | us-east-1 is priority 3', async () => { + + const config = ` + [default] + + `; + + const creds = ` + [default] + + `; + + await expect(region({ credentialsFile: creds, configFile: config })).resolves.toBe('us-east-1'); + }); + +}); + +async function region(opts: { + readonly configFile?: string; + readonly credentialsFile?: string; + readonly profile?: string; +}) { + + const workdir = fs.mkdtempSync(path.join(os.tmpdir(), 'awscli-compatible.test')); + + try { + + if (opts.configFile) { + const configPath = path.join(workdir, 'config'); + fs.writeFileSync(configPath, opts.configFile); + process.env.AWS_CONFIG_FILE = configPath; + } + + if (opts.credentialsFile) { + const credentialsPath = path.join(workdir, 'credentials'); + fs.writeFileSync(credentialsPath, opts.credentialsFile); + process.env.AWS_SHARED_CREDENTIALS_FILE = credentialsPath; + } + + return await AwsCliCompatible.region(opts.profile); + + } finally { + fs.removeSync(workdir); + } +} + +describe('Session token', () => { + beforeEach(() => { + process.env.AWS_ACCESS_KEY_ID = 'foo'; + process.env.AWS_SECRET_ACCESS_KEY = 'bar'; + }); + + test('does not mess up with session token env variables if they are undefined', async () => { + // Making sure these variables are not defined + delete process.env.AWS_SESSION_TOKEN; + delete process.env.AMAZON_SESSION_TOKEN; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toBeUndefined(); + }); + + test('preserves AWS_SESSION_TOKEN if it is defined', async () => { + process.env.AWS_SESSION_TOKEN = 'aaa'; + delete process.env.AMAZON_SESSION_TOKEN; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toEqual('aaa'); + }); + + test('assigns AWS_SESSION_TOKEN if it is not defined but AMAZON_SESSION_TOKEN is', async () => { + delete process.env.AWS_SESSION_TOKEN; + process.env.AMAZON_SESSION_TOKEN = 'aaa'; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toEqual('aaa'); + }); + + test('preserves AWS_SESSION_TOKEN if both are defined', async () => { + process.env.AWS_SESSION_TOKEN = 'aaa'; + process.env.AMAZON_SESSION_TOKEN = 'bbb'; + + await AwsCliCompatible.credentialChainBuilder(); + + expect(process.env.AWS_SESSION_TOKEN).toEqual('aaa'); + }); +}); + describe('Session token', () => { beforeEach(() => { process.env.AWS_ACCESS_KEY_ID = 'foo'; From 36fd4499745d53b23c27f01685f9d970310932a0 Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Thu, 14 Nov 2024 21:07:36 +0000 Subject: [PATCH 3/3] chore(release): 2.167.1 --- 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 6c406d901c4b0..9f623242c9034 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.167.1-alpha.0](https://github.com/aws/aws-cdk/compare/v2.167.0-alpha.0...v2.167.1-alpha.0) (2024-11-14) + ## [2.167.0-alpha.0](https://github.com/aws/aws-cdk/compare/v2.166.0-alpha.0...v2.167.0-alpha.0) (2024-11-13) diff --git a/CHANGELOG.v2.md b/CHANGELOG.v2.md index ce703ab35f34c..aa4d8a4b4d6fd 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.167.1](https://github.com/aws/aws-cdk/compare/v2.167.0...v2.167.1) (2024-11-14) + + +### Bug Fixes + +* **cli:** failure to get credentials when session token is not set ([#32134](https://github.com/aws/aws-cdk/issues/32134)) ([425efbc](https://github.com/aws/aws-cdk/commit/425efbc3625d4512b83225dcb1d1c155d13b4b9e)), closes [#32120](https://github.com/aws/aws-cdk/issues/32120) +* **cli:** region specified in `~/.aws/credentials` is ignored ([#32133](https://github.com/aws/aws-cdk/issues/32133)) ([9859f33](https://github.com/aws/aws-cdk/commit/9859f33a11fb385419c611945bd9bb171b458dad)), closes [#32130](https://github.com/aws/aws-cdk/issues/32130) + ## [2.167.0](https://github.com/aws/aws-cdk/compare/v2.166.0...v2.167.0) (2024-11-13) diff --git a/version.v2.json b/version.v2.json index d829f14cdd44b..df4d1fbe25797 100644 --- a/version.v2.json +++ b/version.v2.json @@ -1,4 +1,4 @@ { - "version": "2.167.0", - "alphaVersion": "2.167.0-alpha.0" + "version": "2.167.1", + "alphaVersion": "2.167.1-alpha.0" } \ No newline at end of file