Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(iam): Assume role actions #16725

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 43 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/principals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ export interface IPrincipal extends IGrantable {
*/
readonly assumeRoleAction: string;

/**
* When this Principal is used in an AssumeRole policy, the actions to use.
*/
readonly assumeRoleActions?: string[];

/**
* Return the policy fragment that identifies this principal in a Policy.
*/
Expand Down Expand Up @@ -103,6 +108,11 @@ export abstract class PrincipalBase implements IPrincipal {
*/
public readonly assumeRoleAction: string = 'sts:AssumeRole';

/**
* When this Principal is used in an AssumeRole policy, the actions to use.
*/
public readonly assumeRoleActions?: string[] = [];

public addToPolicy(statement: PolicyStatement): boolean {
return this.addToPrincipalPolicy(statement).statementAdded;
}
Expand Down Expand Up @@ -141,6 +151,25 @@ export abstract class PrincipalBase implements IPrincipal {
public withConditions(conditions: Conditions): IPrincipal {
return new PrincipalWithConditions(this, conditions);
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thoughts here are that it would be very difficult to change the public API of all the Principals.

However, that might be a better long term solution.

Additionally another approach would be to not attempt to support extension at all, and just create a new class e.g. PrincipalWithAssumeRoleActions.

Please give some advise 👍

* Given we have a principal with trust relationship, we can extend the assume
* role actions with additional actions
*
* @param additionalAssumeRoleAction String assume role action
* @returns void
*/
public addAssumeRoleAction(additionalAssumeRoleAction: string) {
// Ensure the default assumeRoleAction is included
if (!this.assumeRoleActions?.includes(this.assumeRoleAction)) {
this.assumeRoleActions?.push(this.assumeRoleAction);
}

// Insert the additional assume role action
if (!this.assumeRoleActions?.includes(additionalAssumeRoleAction)) {
this.assumeRoleActions?.push(additionalAssumeRoleAction);
}
}
}

/**
Expand Down Expand Up @@ -627,14 +656,26 @@ export class CompositePrincipal extends PrincipalBase {
if (p.assumeRoleAction !== this.assumeRoleAction) {
throw new Error(
'Cannot add multiple principals with different "assumeRoleAction". ' +
`Expecting "${this.assumeRoleAction}", got "${p.assumeRoleAction}"`);
`Expecting "${this.assumeRoleAction}", got "${p.assumeRoleAction}"`);
}

if (!(
Array.isArray(this.assumeRoleActions) &&
Array.isArray(p.assumeRoleActions) &&
this.assumeRoleActions.length === p.assumeRoleActions.length &&
this.assumeRoleActions.every((action) => p.assumeRoleActions?.includes(action))
)) {
throw new Error(
'Cannot add multiple principals with different "assumeRoleActions". ' +
`Expecting "${JSON.stringify(this.assumeRoleActions)}", got "${JSON.stringify(p.assumeRoleActions)}"`,
);
}

const fragment = p.policyFragment;
if (fragment.conditions && Object.keys(fragment.conditions).length > 0) {
throw new Error(
'Components of a CompositePrincipal must not have conditions. ' +
`Tried to add the following fragment: ${JSON.stringify(fragment)}`);
`Tried to add the following fragment: ${JSON.stringify(fragment)}`);
simonireilly marked this conversation as resolved.
Show resolved Hide resolved
}

this.principals.push(p);
Expand Down
9 changes: 7 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,12 @@ export interface IRole extends IIdentity {
function createAssumeRolePolicy(principal: IPrincipal, externalIds: string[]) {
const statement = new AwsStarStatement();
statement.addPrincipals(principal);
statement.addActions(principal.assumeRoleAction);

if (Array.isArray(principal.assumeRoleActions) && principal.assumeRoleActions.length > 1) {
Copy link
Contributor Author

@simonireilly simonireilly Oct 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given we have assumeRoleActions, which includes more than just the base assumeRoleAction then we would favour the full set of actions from the array, instead of the single action.

statement.addActions(...principal.assumeRoleActions);
} else {
statement.addActions(principal.assumeRoleAction);
}

if (externalIds.length) {
statement.addCondition('StringEquals', { 'sts:ExternalId': externalIds.length === 1 ? externalIds[0] : externalIds });
Expand Down Expand Up @@ -540,4 +545,4 @@ export interface WithoutPolicyUpdatesOptions {
* @default false
*/
readonly addGrantsToResources?: boolean;
}
}
40 changes: 39 additions & 1 deletion packages/@aws-cdk/aws-iam/test/principals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,42 @@ test('PrincipalWithConditions inherits principalAccount from AccountPrincipal ',
// THEN
expect(accountPrincipal.principalAccount).toStrictEqual('123456789012');
expect(principalWithConditions.principalAccount).toStrictEqual('123456789012');
});
});

test('Principal assume Role actions can be extended with additional actions', () => {
const stack = new Stack();
const federatedPrincipal = new iam.FederatedPrincipal(
'cognito-identity.amazonaws.com',
{ StringEquals: { hairColor: 'blond' } },
);

federatedPrincipal.addAssumeRoleAction('sts:TagSession');

new iam.Role(stack, 'Role', {
assumedBy: federatedPrincipal,
});

// THEN
expect(stack).toHaveResource('AWS::IAM::Role', {
AssumeRolePolicyDocument: {
Statement: [
{
Action: [
'sts:AssumeRole',
'sts:TagSession',
],
Condition: {
StringEquals: {
hairColor: 'blond',
},
},
Effect: 'Allow',
Principal: {
Federated: 'cognito-identity.amazonaws.com',
},
},
],
Version: '2012-10-17',
},
});
});