From ea4ca3ea251e54921c39ee79f321cae2701837ad Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 15 Jan 2020 10:44:05 +0100 Subject: [PATCH] feat(iam): `Role.withoutPolicyUpdates()` (#5569) Add a new method to `Role` called `withoutPolicyUpdates()`, which returns a wrapper object that will drop all policy updates. This can be used in situations where you want to skip the default IAM permission adding behavior of CDK. Needs only be used with roles that are created in the same CDK application; for imported roles, supplying `mutable=false` when calling `Role.fromRoleArn()` is sufficient. Fixes #2985. Fixes #4465. Closes #4501. --- .../aws-codebuild/test/test.project.ts | 30 +++++++ packages/@aws-cdk/aws-iam/README.md | 71 ++++++++++++++- .../aws-iam/lib/private/immutable-role.ts | 58 +++++++++++++ packages/@aws-cdk/aws-iam/lib/role.ts | 73 ++++++++-------- .../aws-iam/test/immutable-role.test.ts | 87 +++++++++++++++++++ 5 files changed, 280 insertions(+), 39 deletions(-) create mode 100644 packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts create mode 100644 packages/@aws-cdk/aws-iam/test/immutable-role.test.ts diff --git a/packages/@aws-cdk/aws-codebuild/test/test.project.ts b/packages/@aws-cdk/aws-codebuild/test/test.project.ts index 1eccd319898c8..17402b5ae1395 100644 --- a/packages/@aws-cdk/aws-codebuild/test/test.project.ts +++ b/packages/@aws-cdk/aws-codebuild/test/test.project.ts @@ -301,4 +301,34 @@ export = { test.done(); }, + + 'can use an ImmutableRole for a Project within a VPC'(test: Test) { + const stack = new cdk.Stack(); + + const role = new iam.Role(stack, 'Role', { + assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com') + }); + + const vpc = new ec2.Vpc(stack, 'Vpc'); + + new codebuild.Project(stack, 'Project', { + source: codebuild.Source.gitHubEnterprise({ + httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo', + }), + role: role.withoutPolicyUpdates(), + vpc, + }); + + expect(stack).to(countResources('AWS::IAM::Policy', 0)); + + // Check that the CodeBuild project does not have a DependsOn + expect(stack).to(haveResource('AWS::CodeBuild::Project', (res: any) => { + if (res.DependsOn && res.DependsOn.length > 0) { + throw new Error(`CodeBuild project should have no DependsOn, but got: ${JSON.stringify(res, undefined, 2)}`); + } + return true; + }, ResourcePart.CompleteDefinition)); + + test.done(); + }, }; \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/README.md b/packages/@aws-cdk/aws-iam/README.md index aefeaef3c028d..1e783fb628870 100644 --- a/packages/@aws-cdk/aws-iam/README.md +++ b/packages/@aws-cdk/aws-iam/README.md @@ -47,9 +47,78 @@ The `grant*` methods accept an `IGrantable` object. This interface is implemente You can find which `grant*` methods exist for a resource in the [AWS CDK API Reference](https://docs.aws.amazon.com/cdk/api/latest/docs/aws-construct-library.html). +### Roles + +Many AWS resources require *Roles* to operate. These Roles define the AWS API +calls an instance or other AWS service is allowed to make. + +Creating Roles and populating them with the right permissions *Statements* is +a necessary but tedious part of setting up AWS infrastructure. In order to +help you focus on your business logic, CDK will take care of creating +roles and populating them with least-privilege permissions automatically. + +All constructs that require Roles will create one for you if don't specify +one at construction time. Permissions will be added to that role +automatically if you associate the construct with other constructs from the +AWS Construct Library (for example, if you tell an *AWS CodePipeline* to trigger +an *AWS Lambda Function*, the Pipeline's Role will automatically get +`lambda:InvokeFunction` permissions on that particular Lambda Function), +or if you explicitly grant permissions using `grant` functions (see the +previous section). + +#### Opting out of automatic permissions management + +You may prefer to manage a Role's permissions yourself instead of having the +CDK automatically manage them for you. This may happen in one of the +following cases: + +* You don't like the permissions that CDK automatically generates and + want to substitute your own set. +* The least-permissions policy that the CDK generates is becoming too + big for IAM to store, and you need to add some wildcards to keep the + policy size down. + +To prevent constructs from updating your Role's policy, pass the object +returned by `myRole.withoutPolicyUpdates()` instead of `myRole` itself. + +For example, to have an AWS CodePipeline *not* automatically add the required +permissions to trigger the expected targets, do the following: + +```ts +const role = new iam.Role(this, 'Role', { + assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com'), +}); + +new codepipeline.Pipeline(this, 'Pipeline', { + // Give the Pipeline an immutable view of the Role + role: role.withoutPolicyUpdates(), +}); + +// You now have to manage the Role policies yourself +role.addToPolicy(new iam.PolicyStatement({ + action: [/* whatever actions you want */], + resource: [/* whatever resources you intend to touch */], +}); +``` + +#### Using existing roles + +If there are Roles in your account that have already been created which you +would like to use in your CDK application, you can use `Role.fromRoleArn` to +import them, as follows: + +```ts +const role = iam.Role.fromRoleArn(this, 'Role', 'arn:aws:iam::123456789012:role/MyExistingRole', { + // Set 'mutable' to 'false' to use the role as-is and prevent adding new + // policies to it. The default is 'true', which means the role may be + // modified as part of the deployment. + mutable: false, +}); +``` + ### Configuring an ExternalId -If you need to create roles that will be assumed by 3rd parties, it is generally a good idea to [require an `ExternalId` +If you need to create Roles that will be assumed by 4rd parties, it is generally a good idea to [require an `ExternalId` to assume them](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html). Configuring an `ExternalId` works like this: diff --git a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts new file mode 100644 index 0000000000000..d894cc8dbc722 --- /dev/null +++ b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts @@ -0,0 +1,58 @@ +import { DependableTrait } from '@aws-cdk/core'; +import { Grant } from '../grant'; +import { IManagedPolicy } from '../managed-policy'; +import { Policy } from '../policy'; +import { PolicyStatement } from '../policy-statement'; +import { IPrincipal } from '../principals'; +import { IRole } from '../role'; + +/** + * An immutable wrapper around an IRole + * + * This wrapper ignores all mutating operations, like attaching policies or + * adding policy statements. + * + * Useful in cases where you want to turn off CDK's automatic permissions + * management, and instead have full control over all permissions. + * + * Note: if you want to ignore all mutations for an externally defined role + * which was imported into the CDK with {@link Role.fromRoleArn}, you don't have to use this class - + * simply pass the property mutable = false when calling {@link Role.fromRoleArn}. + */ +export class ImmutableRole implements IRole { + public readonly assumeRoleAction = this.role.assumeRoleAction; + public readonly policyFragment = this.role.policyFragment; + public readonly grantPrincipal = this.role.grantPrincipal; + public readonly roleArn = this.role.roleArn; + public readonly roleName = this.role.roleName; + public readonly node = this.role.node; + public readonly stack = this.role.stack; + + constructor(private readonly role: IRole) { + // implement IDependable privately + DependableTrait.implement(this, { + dependencyRoots: [ role ] + }); + } + + public attachInlinePolicy(_policy: Policy): void { + // do nothing + } + + public addManagedPolicy(_policy: IManagedPolicy): void { + // do nothing + } + + public addToPolicy(_statement: PolicyStatement): boolean { + // Not really added, but for the purposes of consumer code pretend that it was. + return true; + } + + public grant(grantee: IPrincipal, ...actions: string[]): Grant { + return this.role.grant(grantee, ...actions); + } + + public grantPassRole(grantee: IPrincipal): Grant { + return this.role.grantPassRole(grantee); + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index b410e3cb627f2..7d3398da3a197 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -7,6 +7,7 @@ import { Policy } from './policy'; import { PolicyDocument } from './policy-document'; import { PolicyStatement } from './policy-statement'; import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals'; +import { ImmutableRole } from './private/immutable-role'; import { AttachedPolicies } from './util'; export interface RoleProps { @@ -162,16 +163,32 @@ export class Role extends Resource implements IRole { ? resourceName.slice('service-role/'.length) : resourceName; - abstract class Import extends Resource implements IRole { + class Import extends Resource implements IRole { public readonly grantPrincipal: IPrincipal = this; public readonly assumeRoleAction: string = 'sts:AssumeRole'; public readonly policyFragment = new ArnPrincipal(roleArn).policyFragment; public readonly roleArn = roleArn; public readonly roleName = roleName; + private readonly attachedPolicies = new AttachedPolicies(); + private defaultPolicy?: Policy; - public abstract addToPolicy(statement: PolicyStatement): boolean; + public addToPolicy(statement: PolicyStatement): boolean { + if (!this.defaultPolicy) { + this.defaultPolicy = new Policy(this, 'Policy'); + this.attachInlinePolicy(this.defaultPolicy); + } + this.defaultPolicy.addStatements(statement); + return true; + } + + public attachInlinePolicy(policy: Policy): void { + const policyAccount = Stack.of(policy).account; - public abstract attachInlinePolicy(policy: Policy): void; + if (accountsAreEqualOrOneIsUnresolved(policyAccount, roleAccount)) { + this.attachedPolicies.attach(policy); + policy.attachToRole(this); + } + } public addManagedPolicy(_policy: IManagedPolicy): void { // FIXME: Add warning that we're ignoring this @@ -199,44 +216,11 @@ export class Role extends Resource implements IRole { const roleAccount = parsedArn.account; - class MutableImport extends Import { - private readonly attachedPolicies = new AttachedPolicies(); - private defaultPolicy?: Policy; - - public addToPolicy(statement: PolicyStatement): boolean { - if (!this.defaultPolicy) { - this.defaultPolicy = new Policy(this, 'Policy'); - this.attachInlinePolicy(this.defaultPolicy); - } - this.defaultPolicy.addStatements(statement); - return true; - } - - public attachInlinePolicy(policy: Policy): void { - const policyAccount = Stack.of(policy).account; - - if (accountsAreEqualOrOneIsUnresolved(policyAccount, roleAccount)) { - this.attachedPolicies.attach(policy); - policy.attachToRole(this); - } - } - } - - class ImmutableImport extends Import { - public addToPolicy(_statement: PolicyStatement): boolean { - return true; - } - - public attachInlinePolicy(_policy: Policy): void { - // do nothing - } - } - const scopeAccount = scopeStack.account; return options.mutable !== false && accountsAreEqualOrOneIsUnresolved(scopeAccount, roleAccount) - ? new MutableImport(scope, id) - : new ImmutableImport(scope, id); + ? new Import(scope, id) + : new ImmutableRole(new Import(scope, id)); function accountsAreEqualOrOneIsUnresolved(account1: string | undefined, account2: string | undefined): boolean { @@ -385,6 +369,19 @@ export class Role extends Resource implements IRole { public grantPassRole(identity: IPrincipal) { return this.grant(identity, 'iam:PassRole'); } + + /** + * Return a copy of this Role object whose Policies will not be updated + * + * Use the object returned by this method if you want this Role to be used by + * a construct without it automatically updating the Role's Policies. + * + * If you do, you are responsible for adding the correct statements to the + * Role's policies yourself. + */ + public withoutPolicyUpdates(): IRole { + return new ImmutableRole(this); + } } /** diff --git a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts new file mode 100644 index 0000000000000..7a39b761bbb7f --- /dev/null +++ b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts @@ -0,0 +1,87 @@ +import '@aws-cdk/assert/jest'; +import { Stack } from '@aws-cdk/core'; +import * as iam from '../lib'; + +// tslint:disable:object-literal-key-quotes + +describe('ImmutableRole', () => { + let stack: Stack; + let mutableRole: iam.Role; + let immutableRole: iam.IRole; + + beforeEach(() => { + stack = new Stack(); + mutableRole = new iam.Role(stack, 'MutableRole', { + assumedBy: new iam.AnyPrincipal(), + }); + immutableRole = mutableRole.withoutPolicyUpdates(); + }); + + test('ignores calls to attachInlinePolicy', () => { + const user = new iam.User(stack, 'User'); + const policy = new iam.Policy(stack, 'Policy', { + statements: [new iam.PolicyStatement({ + resources: ['*'], + actions: ['s3:*'], + })], + users: [user], + }); + + immutableRole.attachInlinePolicy(policy); + + expect(stack).toHaveResource('AWS::IAM::Policy', { + "PolicyDocument": { + "Statement": [ + { + "Action": "s3:*", + "Resource": "*", + "Effect": "Allow", + }, + ], + "Version": "2012-10-17", + }, + "PolicyName": "Policy23B91518", + "Users": [ + { + "Ref": "User00B015A1", + }, + ], + }); + }); + + test('ignores calls to addManagedPolicy', () => { + mutableRole.addManagedPolicy({ managedPolicyArn: 'Arn1' }); + + immutableRole.addManagedPolicy({ managedPolicyArn: 'Arn2' }); + + expect(stack).toHaveResourceLike('AWS::IAM::Role', { + "ManagedPolicyArns": [ + 'Arn1', + ], + }); + }); + + test('ignores calls to addToPolicy', () => { + mutableRole.addToPolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: ['s3:*'], + })); + + immutableRole.addToPolicy(new iam.PolicyStatement({ + resources: ['*'], + actions: ['iam:*'], + })); + + expect(stack).toHaveResourceLike('AWS::IAM::Policy', { + "PolicyDocument": { + "Statement": [ + { + "Resource": "*", + "Action": "s3:*", + "Effect": "Allow", + }, + ], + }, + }); + }); +}); \ No newline at end of file