Skip to content

Commit

Permalink
The proxy configuration was not being properly passed to the
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
rix0rrr committed Apr 10, 2020
1 parent d0dfe79 commit 376fa40
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 14 deletions.
10 changes: 7 additions & 3 deletions packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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()) {
Expand Down
23 changes: 13 additions & 10 deletions packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,23 @@ 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,
/**
* Default region
*/
public readonly defaultRegion: string,
httpOptions: SdkHttpOptions = {}) {
this.httpOptions = defaultHttpOptions(httpOptions);
private readonly sdkOptions: ConfigurationOptions = {}) {
}

/**
Expand All @@ -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);
}

/**
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 = {};

Expand Down
48 changes: 48 additions & 0 deletions packages/aws-cdk/test/api/sdk-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
`),
});

Expand Down Expand Up @@ -138,6 +149,43 @@ 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', () => {
// eslint-disable-next-line @typescript-eslint/no-require-imports
class FakeAgent extends require('https').Agent {
public addRequest(_: any, __: any) {
// 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;
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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/test/util/mock-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 376fa40

Please sign in to comment.