Skip to content

Commit

Permalink
feat(iam): immutable IAM roles
Browse files Browse the repository at this point in the history
This adds an IRole implementation that ignores all mutating operations.
To accommodate this new behavior, add a new method to IIdentity:
createAndAttachPolicy, which is meant to replace attachInlinePolicy,
which can leave Policy resources unattached, which is illegal in CloudFormation.

Fixes aws#2985
Fixes aws#4465
  • Loading branch information
skinny85 committed Oct 21, 2019
1 parent ddb05f9 commit f40edeb
Show file tree
Hide file tree
Showing 11 changed files with 245 additions and 20 deletions.
17 changes: 8 additions & 9 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,7 @@ export class Project extends ProjectBase {
},
}));

const policy = new iam.Policy(this, 'PolicyDocument', {
policyName: 'CodeBuildEC2Policy',
const policy = this.role.addPolicy('CodeBuildVpcPolicy', {
statements: [
new iam.PolicyStatement({
resources: ['*'],
Expand All @@ -962,13 +961,13 @@ export class Project extends ProjectBase {
}),
],
});
this.role.attachInlinePolicy(policy);

// add an explicit dependency between the EC2 Policy and this Project -
// otherwise, creating the Project fails,
// as it requires these permissions to be already attached to the Project's Role
const cfnPolicy = policy.node.findChild('Resource') as CfnResource;
project.addDependsOn(cfnPolicy);
if (policy) {
// add an explicit dependency between the VPC Policy and this Project -
// otherwise, creating the Project fails,
// as it requires these permissions to be already attached to the Project's Role
const cfnPolicy = policy.node.defaultChild as CfnResource;
project.addDependsOn(cfnPolicy);
}
}

private validateCodePipelineSettings(artifacts: IArtifacts) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@
]
}
},
"MyProjectPolicyDocument646EE0F2": {
"MyProjectRoleCodeBuildVpcPolicyE806ED54": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
Expand All @@ -364,7 +364,7 @@
],
"Version": "2012-10-17"
},
"PolicyName": "CodeBuildEC2Policy",
"PolicyName": "MyProjectRoleCodeBuildVpcPolicyE806ED54",
"Roles": [
{
"Ref": "MyProjectRole9BBE5233"
Expand Down Expand Up @@ -414,7 +414,7 @@
}
},
"DependsOn": [
"MyProjectPolicyDocument646EE0F2"
"MyProjectRoleCodeBuildVpcPolicyE806ED54"
]
}
}
Expand Down
24 changes: 23 additions & 1 deletion packages/@aws-cdk/aws-codebuild/test/test.project.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
import { countResources, expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
import ec2 = require('@aws-cdk/aws-ec2');
import iam = require('@aws-cdk/aws-iam');
import s3 = require('@aws-cdk/aws-s3');
Expand Down Expand Up @@ -274,4 +274,26 @@ export = {

test.done();
},

'can use an imported Role with mutable = false for a Project within a VPC'(test: Test) {
const stack = new cdk.Stack();

const importedRole = iam.Role.fromRoleArn(stack, 'Role',
'arn:aws:iam::1234567890:role/service-role/codebuild-bruiser-service-role', {
mutable: false,
});
const vpc = new ec2.Vpc(stack, 'Vpc');

new codebuild.Project(stack, 'Project', {
source: codebuild.Source.gitHubEnterprise({
httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo',
}),
role: importedRole,
vpc,
});

expect(stack).to(countResources('AWS::IAM::Policy', 0));

test.done();
},
};
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Construct, Lazy, Resource, Stack } from '@aws-cdk/core';
import { CfnGroup } from './iam.generated';
import { IIdentity } from './identity-base';
import { IManagedPolicy } from './managed-policy';
import { Policy } from './policy';
import { Policy, PolicyProps } from './policy';
import { PolicyStatement } from './policy-statement';
import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals';
import { IUser } from './user';
Expand Down Expand Up @@ -73,6 +73,12 @@ abstract class GroupBase extends Resource implements IGroup {
return new ArnPrincipal(this.groupArn).policyFragment;
}

public addPolicy(id: string, props?: PolicyProps): Policy | undefined {
const policy = new Policy(this, id, props);
this.attachInlinePolicy(policy);
return policy;
}

/**
* Attaches a policy to this group.
* @param policy The policy to attach.
Expand Down
24 changes: 21 additions & 3 deletions packages/@aws-cdk/aws-iam/lib/identity-base.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,34 @@
import { IResource } from '@aws-cdk/core';
import { IManagedPolicy } from './managed-policy';
import { Policy } from "./policy";
import { Policy, PolicyProps } from "./policy";
import { IPrincipal } from "./principals";

/**
* A construct that represents an IAM principal, such as a user, group or role.
*/
export interface IIdentity extends IPrincipal, IResource {
/**
* Attaches an inline policy to this principal.
* This is the same as calling `policy.addToXxx(principal)`.
* Creates and attaches a Policy to this principal, if it is mutable.
* If this principal is immutable, does nothing, and returns undefined.
*
* @param id the logical identifier to use for the new policy resource.
* It will be created in the scope of this principal (if it's mutable)
* @param props the construction properties of the new policy
* @returns the newly created Policy resource if this principal is mutable,
* or undefined if this principal is immutable
*/
addPolicy(id: string, props?: PolicyProps): Policy | undefined;

/**
* Attaches an inline policy to this principal, if this principal is mutable.
* For a mutable principal, it is the same as calling `policy.addToXxx(principal)`.
* For an immutable principal, this method does nothing.
*
* @param policy The policy resource to attach to this principal [disable-awslint:ref-via-interface]
*
* @deprecated for immutable principals, this will leave the policy passed as the first parameter
* possibly not attached to any principal, which is invalid in CloudFormation.
* Use the {@link addPolicy} method instead
*/
attachInlinePolicy(policy: Policy): void;

Expand Down
52 changes: 52 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/immutable-role.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { Grant } from './grant';
import { IManagedPolicy } from './managed-policy';
import { Policy, PolicyProps } from './policy';
import { PolicyStatement } from './policy-statement';
import { IPrincipal } from './principals';
import { IRole } from './role';

/**
* An immutable wrapper around an IRole that 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) {
}

public addPolicy(_id: string, _props?: PolicyProps): Policy | undefined {
return undefined;
}

public attachInlinePolicy(_policy: Policy): void {
// do nothing
}

public addManagedPolicy(_policy: IManagedPolicy): void {
// do nothing
}

public addToPolicy(_statement: PolicyStatement): boolean {
return false;
}

public grant(grantee: IPrincipal, ...actions: string[]): Grant {
return this.role.grant(grantee, ...actions);
}

public grantPassRole(grantee: IPrincipal): Grant {
return this.role.grantPassRole(grantee);
}
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-iam/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export * from './policy-document';
export * from './policy-statement';
export * from './managed-policy';
export * from './role';
export * from './immutable-role';
export * from './policy';
export * from './user';
export * from './group';
Expand Down
8 changes: 7 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/lazy-role.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import cdk = require('@aws-cdk/core');
import { Grant } from './grant';
import { IManagedPolicy } from './managed-policy';
import { Policy } from './policy';
import { Policy, PolicyProps } from './policy';
import { PolicyStatement } from './policy-statement';
import { IPrincipal, PrincipalPolicyFragment } from './principals';
import { IRole, Role, RoleProps } from './role';
Expand Down Expand Up @@ -49,6 +49,12 @@ export class LazyRole extends cdk.Resource implements IRole {
}
}

public addPolicy(id: string, props?: PolicyProps): Policy | undefined {
const policy = new Policy(this, id, props);
this.attachInlinePolicy(policy);
return policy;
}

/**
* Attaches a policy to this role.
* @param policy The policy to attach
Expand Down
20 changes: 19 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Grant } from './grant';
import { CfnRole } from './iam.generated';
import { IIdentity } from './identity-base';
import { IManagedPolicy } from './managed-policy';
import { Policy } from './policy';
import { Policy, PolicyProps } from './policy';
import { PolicyDocument } from './policy-document';
import { PolicyStatement } from './policy-statement';
import { ArnPrincipal, IPrincipal, PrincipalPolicyFragment } from './principals';
Expand Down Expand Up @@ -168,6 +168,8 @@ export class Role extends Resource implements IRole {

public abstract attachInlinePolicy(policy: Policy): void;

public abstract addPolicy(id: string, props?: PolicyProps): Policy | undefined;

public addManagedPolicy(_policy: IManagedPolicy): void {
// FIXME: Add warning that we're ignoring this
}
Expand Down Expand Up @@ -207,6 +209,12 @@ export class Role extends Resource implements IRole {
return true;
}

public addPolicy(identifier: string, props?: PolicyProps): Policy | undefined {
const policy = new Policy(this, identifier, props);
this.attachInlinePolicy(policy);
return policy;
}

public attachInlinePolicy(policy: Policy): void {
const policyAccount = Stack.of(policy).account;

Expand All @@ -222,6 +230,10 @@ export class Role extends Resource implements IRole {
return false;
}

public addPolicy(_id: string, _props?: PolicyProps): Policy | undefined {
return undefined;
}

public attachInlinePolicy(_policy: Policy): void {
// do nothing
}
Expand Down Expand Up @@ -353,6 +365,12 @@ export class Role extends Resource implements IRole {
this.managedPolicies.push(policy);
}

public addPolicy(id: string, props?: PolicyProps): Policy | undefined {
const policy = new Policy(this, id, props);
this.attachInlinePolicy(policy);
return policy;
}

/**
* Attaches a policy to this role.
* @param policy The policy to attach
Expand Down
12 changes: 11 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { IGroup } from './group';
import { CfnUser } from './iam.generated';
import { IIdentity } from './identity-base';
import { IManagedPolicy } from './managed-policy';
import { Policy } from './policy';
import { Policy, PolicyProps } from './policy';
import { PolicyStatement } from './policy-statement';
import { ArnPrincipal, PrincipalPolicyFragment } from './principals';
import { IPrincipal } from './principals';
Expand Down Expand Up @@ -147,6 +147,10 @@ export class User extends Resource implements IIdentity, IUser {
throw new Error('Cannot add imported User to Group');
}

public addPolicy(_id: string, _props?: PolicyProps): Policy | undefined {
return undefined;
}

public attachInlinePolicy(_policy: Policy): void {
throw new Error('Cannot add inline policy to imported User');
}
Expand Down Expand Up @@ -234,6 +238,12 @@ export class User extends Resource implements IIdentity, IUser {
this.managedPolicies.push(policy);
}

public addPolicy(id: string, props?: PolicyProps): Policy | undefined {
const policy = new Policy(this, id, props);
this.attachInlinePolicy(policy);
return policy;
}

/**
* Attaches a policy to this user.
*/
Expand Down
Loading

0 comments on commit f40edeb

Please sign in to comment.