From 18d83aa9110976afb24437b0d9e48cff7efe1baa Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 10 Apr 2020 11:36:00 +0200
Subject: [PATCH 1/2] fix(cli): profile AssumeRole credentials don't work via
proxy
The proxy configuration was not being properly passed to the
`AWS.SharedIniFileCredentials` object, so if users configure AssumeRole
credentials in their `~/.aws/config` file, it would try to connect to
STS directly instead of going through the proxy.
Properly parse and pass the HTTP options to the right AWS file.
(Note that this object only takes `HTTPOptions` and not
`ConfigurationOptions`, so there's no way to override the user agent
on this initial STS call).
Fixes #7265.
---
.../lib/api/aws-auth/awscli-compatible.ts | 10 +++--
.../aws-cdk/lib/api/aws-auth/sdk-provider.ts | 24 +++++-----
.../aws-cdk/test/api/sdk-provider.test.ts | 45 +++++++++++++++++++
packages/aws-cdk/test/util/mock-sdk.ts | 2 +-
4 files changed, 67 insertions(+), 14 deletions(-)
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 268b93ddf3056..dc1c5df8c8ce8 100644
--- a/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts
+++ b/packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts
@@ -30,7 +30,11 @@ export class AwsCliCompatible {
* 3. Respects $AWS_SHARED_CREDENTIALS_FILE.
* 4. Respects $AWS_DEFAULT_PROFILE in addition to $AWS_PROFILE.
*/
- public static async credentialChain(profile: string | undefined, ec2creds: boolean | undefined, containerCreds: boolean | undefined) {
+ public static async credentialChain(
+ profile: string | undefined,
+ ec2creds: boolean | undefined,
+ containerCreds: boolean | undefined,
+ httpOptions: AWS.HTTPOptions | undefined) {
await forceSdkToReadConfigIfPresent();
profile = profile || process.env.AWS_PROFILE || process.env.AWS_DEFAULT_PROFILE || 'default';
@@ -41,11 +45,11 @@ export class AwsCliCompatible {
];
if (await fs.pathExists(credentialsFileName())) {
- sources.push(() => new AWS.SharedIniFileCredentials({ profile, filename: credentialsFileName() }));
+ sources.push(() => new AWS.SharedIniFileCredentials({ profile, filename: credentialsFileName(), httpOptions }));
}
if (await fs.pathExists(configFileName())) {
- sources.push(() => new AWS.SharedIniFileCredentials({ profile, filename: credentialsFileName() }));
+ sources.push(() => new AWS.SharedIniFileCredentials({ profile, filename: credentialsFileName(), httpOptions }));
}
if (containerCreds ?? hasEcsCredentials()) {
diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts b/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
index 52523d1e52a50..cbac7f10530f5 100644
--- a/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
+++ b/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
@@ -88,14 +88,15 @@ export class SdkProvider {
* class `AwsCliCompatible` for the details.
*/
public static async withAwsCliCompatibleDefaults(options: SdkProviderOptions = {}) {
- const chain = await AwsCliCompatible.credentialChain(options.profile, options.ec2creds, options.containerCreds);
+ const sdkOptions = parseHttpOptions(options.httpOptions ?? {});
+
+ const chain = await AwsCliCompatible.credentialChain(options.profile, options.ec2creds, options.containerCreds, sdkOptions.httpOptions);
const region = await AwsCliCompatible.region(options.profile);
- return new SdkProvider(chain, region, options.httpOptions);
+ return new SdkProvider(chain, region, sdkOptions);
}
private readonly plugins = new CredentialPlugins();
- private readonly httpOptions: ConfigurationOptions;
public constructor(
private readonly defaultChain: AWS.CredentialProviderChain,
@@ -103,8 +104,7 @@ export class SdkProvider {
* Default region
*/
public readonly defaultRegion: string,
- httpOptions: SdkHttpOptions = {}) {
- this.httpOptions = defaultHttpOptions(httpOptions);
+ private readonly sdkOptions: ConfigurationOptions = {}) {
}
/**
@@ -116,7 +116,7 @@ export class SdkProvider {
public async forEnvironment(accountId: string | undefined, region: string | undefined, mode: Mode): Promise<ISDK> {
const env = await this.resolveEnvironment(accountId, region);
const creds = await this.obtainCredentials(env.account, mode);
- return new SDK(creds, env.region, this.httpOptions);
+ return new SDK(creds, env.region, this.sdkOptions);
}
/**
@@ -139,12 +139,12 @@ export class SdkProvider {
},
stsConfig: {
region,
- ...this.httpOptions,
+ ...this.sdkOptions,
},
masterCredentials: await this.defaultCredentials(),
});
- return new SDK(creds, region, this.httpOptions);
+ return new SDK(creds, region, this.sdkOptions);
}
/**
@@ -199,7 +199,7 @@ export class SdkProvider {
throw new Error('Unable to resolve AWS credentials (setup with "aws configure")');
}
- return new SDK(creds, this.defaultRegion, this.httpOptions).currentAccount();
+ return new SDK(creds, this.defaultRegion, this.sdkOptions).currentAccount();
} catch (e) {
debug('Unable to determine the default AWS account:', e);
return undefined;
@@ -269,8 +269,11 @@ export interface Account {
* Get HTTP options for the SDK
*
* Read from user input or environment variables.
+ *
+ * Returns a complete `ConfigurationOptions` object because that's where
+ * `customUserAgent` lives, but `httpOptions` is the most important attribute.
*/
-function defaultHttpOptions(options: SdkHttpOptions) {
+function parseHttpOptions(options: SdkHttpOptions) {
const config: ConfigurationOptions = {};
config.httpOptions = {};
@@ -298,6 +301,7 @@ function defaultHttpOptions(options: SdkHttpOptions) {
debug('Using proxy server: %s', proxyAddress);
// eslint-disable-next-line @typescript-eslint/no-require-imports
const ProxyAgent: any = require('proxy-agent');
+ // tslint:disable-next-line:no-console
config.httpOptions.agent = new ProxyAgent(proxyAddress);
}
if (caBundlePath) {
diff --git a/packages/aws-cdk/test/api/sdk-provider.test.ts b/packages/aws-cdk/test/api/sdk-provider.test.ts
index 1ae9e099d209d..35fd5c5764faa 100644
--- a/packages/aws-cdk/test/api/sdk-provider.test.ts
+++ b/packages/aws-cdk/test/api/sdk-provider.test.ts
@@ -34,6 +34,10 @@ beforeEach(() => {
[foo]
aws_access_key_id=${uid}fooccess
aws_secret_access_key=secret
+
+ [assumer]
+ aws_access_key_id=${uid}assumer
+ aws_secret_access_key=secret
`),
'/home/me/.bxt/config': dedent(`
[default]
@@ -46,6 +50,13 @@ beforeEach(() => {
aws_access_key_id=${uid}booccess
aws_secret_access_key=boocret
# No region here
+
+ [profile assumable]
+ role_arn=arn:aws:iam::12356789012:role/Assumable
+ source_profile=assumer
+
+ [profile assumer]
+ region=us-east-2
`),
});
@@ -138,6 +149,40 @@ describe('CLI compatible credentials loading', () => {
await expect(provider.forEnvironment(`${uid}some_account_#`, 'def', Mode.ForReading)).rejects.toThrow('Need to perform AWS calls');
});
+
+ test('even when using a profile to assume another profile, STS calls goes through the proxy', async () => {
+ // Messy mocking
+ let called = false;
+ jest.mock('proxy-agent', () => {
+ class FakeAgent extends require('https').Agent {
+ public addRequest(_: any, __: any) {
+ // This error takes 6 seconds to be completely handled. It might be retries in the SDK
+ // somewhere, or something about the Node event loop. I've spent an hour trying to figure
+ // it out and I can't, and I gave up. We'll just have to live with this.
+ const error = new Error('ABORTED BY TEST');
+ (error as any).code = 'RequestAbortedError';
+ (error as any).retryable = false;
+ called = true;
+ throw error;
+ }
+ }
+ return FakeAgent;
+ });
+
+ // WHEN
+ const provider = await SdkProvider.withAwsCliCompatibleDefaults({ ...defaultCredOptions,
+ ec2creds: false,
+ profile: 'assumable',
+ httpOptions: {
+ proxyAddress: 'http://DOESNTMATTER/',
+ }
+ });
+
+ await provider.defaultAccount();
+
+ // THEN -- the fake proxy agent got called, we don't care about the result
+ expect(called).toEqual(true);
+ });
});
describe('Plugins', () => {
diff --git a/packages/aws-cdk/test/util/mock-sdk.ts b/packages/aws-cdk/test/util/mock-sdk.ts
index efc10b2d0ec66..1c1cbcf29506d 100644
--- a/packages/aws-cdk/test/util/mock-sdk.ts
+++ b/packages/aws-cdk/test/util/mock-sdk.ts
@@ -12,7 +12,7 @@ export class MockSdkProvider extends SdkProvider {
private readonly sdk: ISDK;
constructor() {
- super(new AWS.CredentialProviderChain([]), 'bermuda-triangle-1337', { userAgent: 'aws-cdk/jest' });
+ super(new AWS.CredentialProviderChain([]), 'bermuda-triangle-1337', { customUserAgent: 'aws-cdk/jest' });
// SDK contains a real SDK, since some test use 'AWS-mock' to replace the underlying
// AWS calls which a real SDK would do, and some tests use the 'stub' functionality below.
From 72dd7e91a8f1ef59bda489a3d02220b71d628aeb Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Fri, 10 Apr 2020 13:29:30 +0200
Subject: [PATCH 2/2] Review comments
---
packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts | 1 -
packages/aws-cdk/test/api/sdk-provider.test.ts | 9 ++++++---
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts b/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
index cbac7f10530f5..d0b977225824a 100644
--- a/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
+++ b/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
@@ -301,7 +301,6 @@ function parseHttpOptions(options: SdkHttpOptions) {
debug('Using proxy server: %s', proxyAddress);
// eslint-disable-next-line @typescript-eslint/no-require-imports
const ProxyAgent: any = require('proxy-agent');
- // tslint:disable-next-line:no-console
config.httpOptions.agent = new ProxyAgent(proxyAddress);
}
if (caBundlePath) {
diff --git a/packages/aws-cdk/test/api/sdk-provider.test.ts b/packages/aws-cdk/test/api/sdk-provider.test.ts
index 35fd5c5764faa..e6ea2f0048837 100644
--- a/packages/aws-cdk/test/api/sdk-provider.test.ts
+++ b/packages/aws-cdk/test/api/sdk-provider.test.ts
@@ -154,11 +154,14 @@ describe('CLI compatible credentials loading', () => {
// Messy mocking
let called = false;
jest.mock('proxy-agent', () => {
+ // eslint-disable-next-line @typescript-eslint/no-require-imports
class FakeAgent extends require('https').Agent {
public addRequest(_: any, __: any) {
- // This error takes 6 seconds to be completely handled. It might be retries in the SDK
- // somewhere, or something about the Node event loop. I've spent an hour trying to figure
- // it out and I can't, and I gave up. We'll just have to live with this.
+ // FIXME: this error takes 6 seconds to be completely handled. It
+ // might be retries in the SDK somewhere, or something about the Node
+ // event loop. I've spent an hour trying to figure it out and I can't,
+ // and I gave up. We'll just have to live with this until someone gets
+ // inspired.
const error = new Error('ABORTED BY TEST');
(error as any).code = 'RequestAbortedError';
(error as any).retryable = false;