Skip to content

Commit

Permalink
fix(iam): User.fromUserArn does not work for ARNs that include a pa…
Browse files Browse the repository at this point in the history
…th (#16269)

If a IAM user has a path, the ARN contains the path, e.g. `arn:aws:iam::account-id:user/path/MyUserName`.
Method `User.fromUserArn` parses this ARN to `userName`: `path/MyUserName`. The path is not removed correctly. The correct username would be `MyUserName`.

This PR changes the parsing of property `userName` to remove the path correctly. The logic is implemented according to [iam.Role](https://github.com/aws/aws-cdk/blob/d5ca419448e84f0cbb25dbd90d48fb4c407ede5c/packages/%40aws-cdk/aws-iam/lib/role.ts#L191-L194) where a similar conversion is necessary to support service roles.

Fixes #16256.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
jumic authored Oct 4, 2021
1 parent 0c8ecb8 commit 5c69c94
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 2 deletions.
11 changes: 10 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ export class User extends Resource implements IIdentity, IUser {
/**
* Import an existing user given a user ARN.
*
* If the ARN comes from a Token, the User cannot have a path; if so, any attempt
* to reference its username will fail.
*
* @param scope construct scope
* @param id construct id
* @param userArn the ARN of an existing user to import
Expand All @@ -167,6 +170,9 @@ export class User extends Resource implements IIdentity, IUser {
/**
* Import an existing user given user attributes.
*
* If the ARN comes from a Token, the User cannot have a path; if so, any attempt
* to reference its username will fail.
*
* @param scope construct scope
* @param id construct id
* @param attrs the attributes of the user to import
Expand All @@ -175,7 +181,10 @@ export class User extends Resource implements IIdentity, IUser {
class Import extends Resource implements IUser {
public readonly grantPrincipal: IPrincipal = this;
public readonly principalAccount = Aws.ACCOUNT_ID;
public readonly userName: string = Arn.extractResourceName(attrs.userArn, 'user');
// Resource name with path can have multiple elements separated by slash.
// Therefore, use element after last slash as userName. Happens to work for Tokens since
// they don't have a '/' in them.
public readonly userName: string = Arn.extractResourceName(attrs.userArn, 'user').split('/').pop()!;
public readonly userArn: string = attrs.userArn;
public readonly assumeRoleAction: string = 'sts:AssumeRole';
public readonly policyFragment: PrincipalPolicyFragment = new ArnPrincipal(attrs.userArn).policyFragment;
Expand Down
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-iam/test/integ.user.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,21 @@
"NameForUserImportedByArn": {
"Value": "rossrhodes"
},
"NameForUserImportedByArnPath": {
"Value": "johndoe"
},
"NameForUserImportedByArnPathMultiple": {
"Value": "johndoe"
},
"NameForUserImportedByAttributes": {
"Value": "johndoe"
},
"NameForUserImportedByAttributesPath": {
"Value": "johndoe"
},
"NameForUserImportedByAttributesPathMultiple": {
"Value": "johndoe"
},
"NameForUserImportedByName": {
"Value": "janedoe"
}
Expand Down
12 changes: 12 additions & 0 deletions packages/@aws-cdk/aws-iam/test/integ.user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,25 @@ new User(stack, 'MyUser', {
});

const userImportedByArn = User.fromUserArn(stack, 'ImportedUserByArn', 'arn:aws:iam::123456789012:user/rossrhodes');
const userImportedByArnWithPath = User.fromUserArn(stack, 'ImportedUserByArnPath', 'arn:aws:iam::123456789012:user/path/johndoe');
const userImportedByArnPathMultiple = User.fromUserArn(stack, 'ImportedUserByArnPathMultiple', 'arn:aws:iam::123456789012:user/p/a/t/h/johndoe');
const userImportedByAttributes = User.fromUserAttributes(stack, 'ImportedUserByAttributes', {
userArn: 'arn:aws:iam::123456789012:user/johndoe',
});
const userImportedByAttributesPath = User.fromUserAttributes(stack, 'ImportedUserByAttributesPath', {
userArn: 'arn:aws:iam::123456789012:user/path/johndoe',
});
const userImportedByAttributesPathMultiple = User.fromUserAttributes(stack, 'ImportedUserByAttributesPathMultiple', {
userArn: 'arn:aws:iam::123456789012:user/p/a/t/h/johndoe',
});
const userImportedByName = User.fromUserName(stack, 'ImportedUserByName', 'janedoe');

new CfnOutput(stack, 'NameForUserImportedByArn', { value: userImportedByArn.userName });
new CfnOutput(stack, 'NameForUserImportedByArnPath', { value: userImportedByArnWithPath.userName });
new CfnOutput(stack, 'NameForUserImportedByArnPathMultiple', { value: userImportedByArnPathMultiple.userName });
new CfnOutput(stack, 'NameForUserImportedByAttributes', { value: userImportedByAttributes.userName });
new CfnOutput(stack, 'NameForUserImportedByAttributesPath', { value: userImportedByAttributesPath.userName });
new CfnOutput(stack, 'NameForUserImportedByAttributesPathMultiple', { value: userImportedByAttributesPathMultiple.userName });
new CfnOutput(stack, 'NameForUserImportedByName', { value: userImportedByName.userName });

app.synth();
82 changes: 81 additions & 1 deletion packages/@aws-cdk/aws-iam/test/user.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import '@aws-cdk/assert-internal/jest';
import { App, SecretValue, Stack } from '@aws-cdk/core';
import { App, SecretValue, Stack, Token } from '@aws-cdk/core';
import { Group, ManagedPolicy, Policy, PolicyStatement, User } from '../lib';

describe('IAM user', () => {
Expand Down Expand Up @@ -106,6 +106,58 @@ describe('IAM user', () => {
expect(stack.resolve(user.userName)).toStrictEqual(userName);
});

test('user imported by tokenized user ARN has a name', () => {
// GIVEN
const stack = new Stack();

// WHEN
const user = User.fromUserArn(stack, 'import', Token.asString({ Ref: 'ARN' }));

// THEN
expect(stack.resolve(user.userName)).toStrictEqual({
'Fn::Select': [1, { 'Fn::Split': [':user/', { Ref: 'ARN' }] }],
});
});

test('user imported by user ARN with path', () => {
// GIVEN
const stack = new Stack();
const userName = 'MyUserName';

// WHEN
const user = User.fromUserArn(stack, 'import', `arn:aws:iam::account-id:user/path/${userName}`);

// THEN
expect(stack.resolve(user.userName)).toStrictEqual(userName);
});

test('user imported by user ARN with path (multiple elements)', () => {
// GIVEN
const stack = new Stack();
const userName = 'MyUserName';

// WHEN
const user = User.fromUserArn(stack, 'import', `arn:aws:iam::account-id:user/p/a/t/h/${userName}`);

// THEN
expect(stack.resolve(user.userName)).toStrictEqual(userName);
});

test('user imported by tokenized user attributes has a name', () => {
// GIVEN
const stack = new Stack();

// WHEN
const user = User.fromUserAttributes(stack, 'import', {
userArn: Token.asString({ Ref: 'ARN' }),
});

// THEN
expect(stack.resolve(user.userName)).toStrictEqual({
'Fn::Select': [1, { 'Fn::Split': [':user/', { Ref: 'ARN' }] }],
});
});

test('user imported by user attributes has a name', () => {
// GIVEN
const stack = new Stack();
Expand All @@ -120,6 +172,34 @@ describe('IAM user', () => {
expect(stack.resolve(user.userName)).toStrictEqual(userName);
});

test('user imported by user attributes with path has a name', () => {
// GIVEN
const stack = new Stack();
const userName = 'MyUserName';

// WHEN
const user = User.fromUserAttributes(stack, 'import', {
userArn: `arn:aws:iam::account-id:user/path/${userName}`,
});

// THEN
expect(stack.resolve(user.userName)).toStrictEqual(userName);
});

test('user imported by user attributes with path (multiple elements) has a name', () => {
// GIVEN
const stack = new Stack();
const userName = 'MyUserName';

// WHEN
const user = User.fromUserAttributes(stack, 'import', {
userArn: `arn:aws:iam::account-id:user/p/a/t/h/${userName}`,
});

// THEN
expect(stack.resolve(user.userName)).toStrictEqual(userName);
});

test('add to policy of imported user', () => {
// GIVEN
const stack = new Stack();
Expand Down

0 comments on commit 5c69c94

Please sign in to comment.