Skip to content

Commit

Permalink
fix(cli): OS usernames cannot have Unicode characters (#10451)
Browse files Browse the repository at this point in the history
When assuming a role for uploading assets in the new-style synthesized
stacks, the OS username was used to build the session name out of.

OS usernames have a character set that is wider than the allowed
characters in `RoleSessionName` though, so we needed to sanitize
them.

Fixes #10401.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Sep 22, 2020
1 parent 799da48 commit 635f0ed
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 2 deletions.
11 changes: 10 additions & 1 deletion packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class SdkProvider {
params: {
RoleArn: roleArn,
...externalId ? { ExternalId: externalId } : {},
RoleSessionName: `aws-cdk-${os.userInfo().username}`,
RoleSessionName: `aws-cdk-${safeUsername()}`,
},
stsConfig: {
region,
Expand Down Expand Up @@ -362,4 +362,13 @@ function readIfPossible(filename: string): string | undefined {
debug(e);
return undefined;
}
}

/**
* Return the username with characters invalid for a RoleSessionName removed
*
* @see https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#API_AssumeRole_RequestParameters
*/
function safeUsername() {
return os.userInfo().username.replace(/[^\w+=,.@-]/g, '@');
}
10 changes: 10 additions & 0 deletions packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ export class SDK implements ISDK {
return { accountId, partition };
}));
}

/**
* Return the current credentials
*
* Don't use -- only used to write tests around assuming roles.
*/
public async currentCredentials(): Promise<AWS.Credentials> {
await this.credentials.getPromise();
return this.credentials;
}
}

/**
Expand Down
39 changes: 39 additions & 0 deletions packages/aws-cdk/test/api/sdk-provider.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as os from 'os';
import * as cxapi from '@aws-cdk/cx-api';
import * as AWS from 'aws-sdk';
import * as SDKMock from 'aws-sdk-mock';
Expand Down Expand Up @@ -271,6 +272,44 @@ describe('with default config files', () => {
await expect(sdk.s3().listBuckets().promise()).rejects.toThrow('did you bootstrap');
await expect(sdk.s3().listBuckets().promise()).rejects.toThrow('Nope!');
});

test('assuming a role sanitizes the username into the session name', async () => {
// GIVEN
SDKMock.restore();

await withMocked(os, 'userInfo', async (userInfo) => {
userInfo.mockReturnValue({ username: 'skål', uid: 1, gid: 1, homedir: '/here', shell: '/bin/sh' });

await withMocked((new AWS.STS()).constructor.prototype, 'assumeRole', async (assumeRole) => {
let assumeRoleRequest;

assumeRole.mockImplementation(function (
this: any,
request: AWS.STS.Types.AssumeRoleRequest,
cb?: (err: Error | null, x: AWS.STS.Types.AssumeRoleResponse) => void) {

// Part of the request is stored on "this"
assumeRoleRequest = { ...this.config.params, ...request };

const response = {
Credentials: { AccessKeyId: `${uid}aid`, Expiration: new Date(), SecretAccessKey: 's', SessionToken: '' },
};
if (cb) { cb(null, response); }
return { promise: () => Promise.resolve(response) };
});

// WHEN
const provider = new SdkProvider(new AWS.CredentialProviderChain([() => new AWS.Credentials({ accessKeyId: 'a', secretAccessKey: 's' })]), 'eu-somewhere');
const sdk = await provider.withAssumedRole('bla.role.arn', undefined, undefined);

await sdk.currentCredentials();

expect(assumeRoleRequest).toEqual(expect.objectContaining({
RoleSessionName: 'aws-cdk-sk@l',
}));
});
});
});
});

describe('Plugins', () => {
Expand Down
11 changes: 10 additions & 1 deletion packages/cdk-assets/bin/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class DefaultAwsClient implements IAws {
params: {
RoleArn: roleArn,
ExternalId: externalId,
RoleSessionName: `cdk-assets-${os.userInfo().username}`,
RoleSessionName: `cdk-assets-${safeUsername()}`,
},
stsConfig: {
region,
Expand All @@ -149,3 +149,12 @@ class DefaultAwsClient implements IAws {
});
}
}

/**
* Return the username with characters invalid for a RoleSessionName removed
*
* @see https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#API_AssumeRole_RequestParameters
*/
function safeUsername() {
return os.userInfo().username.replace(/[^\w+=,.@-]/g, '@');
}

0 comments on commit 635f0ed

Please sign in to comment.