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

BREAKING CHANGE: restrict allowable types in IAM libraries #629

Merged
merged 3 commits into from
Aug 27, 2018
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export = {

fleet.addToRolePolicy(new cdk.PolicyStatement()
.addAction('*')
.addResource('*'));
.addAllResources());

expect(stack).toMatch({
"Resources": {
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-iam/lib/group.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ArnPrincipal, Construct, PolicyPrincipal, PolicyStatement } from '@aws-cdk/cdk';
import { Arn, ArnPrincipal, Construct, PolicyPrincipal, PolicyStatement } from '@aws-cdk/cdk';
import { cloudformation, GroupArn, GroupName } from './iam.generated';
import { IIdentityResource, IPrincipal, Policy } from './policy';
import { User } from './user';
Expand Down Expand Up @@ -73,7 +73,7 @@ export class Group extends Construct implements IIdentityResource, IPrincipal {
* Attaches a managed policy to this group.
* @param arn The ARN of the managed policy to attach.
*/
public attachManagedPolicy(arn: any) {
public attachManagedPolicy(arn: Arn) {
this.managedPolicies.push(arn);
}

Expand Down Expand Up @@ -104,4 +104,4 @@ export class Group extends Construct implements IIdentityResource, IPrincipal {

this.defaultPolicy.addStatement(statement);
}
}
}
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/policy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, IDependable, PolicyDocument, PolicyPrincipal, PolicyStatement, Token } from '@aws-cdk/cdk';
import { Arn, Construct, IDependable, PolicyDocument, PolicyPrincipal, PolicyStatement, Token } from '@aws-cdk/cdk';
import { Group } from './group';
import { cloudformation } from './iam.generated';
import { Role } from './role';
Expand Down Expand Up @@ -31,7 +31,7 @@ export interface IPrincipal {
* Attaches a managed policy to this principal.
* @param arn The ARN of the managed policy
*/
attachManagedPolicy(arn: any): void;
attachManagedPolicy(arn: Arn): void;
}

/**
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ArnPrincipal, Construct, IDependable, PolicyDocument, PolicyPrincipal, PolicyStatement } from '@aws-cdk/cdk';
import { Arn, ArnPrincipal, Construct, IDependable, PolicyDocument, PolicyPrincipal, PolicyStatement } from '@aws-cdk/cdk';
import { cloudformation, RoleArn, RoleName } from './iam.generated';
import { IIdentityResource, IPrincipal, Policy } from './policy';
import { AttachedPolicies, undefinedIfEmpty } from './util';
Expand All @@ -15,7 +15,7 @@ export interface RoleProps {

/**
* A list of ARNs for managed policies associated with this role.
* You can add managed policies later using `addManagedPolicy(arn)`.
* You can add managed policies later using `attachManagedPolicy(arn)`.
* @default No managed policies.
*/
managedPolicyArns?: any[];
Expand Down Expand Up @@ -97,7 +97,7 @@ export class Role extends Construct implements IIdentityResource, IPrincipal, ID
public readonly dependencyElements: IDependable[];

private defaultPolicy?: Policy;
private readonly managedPolicies: string[];
private readonly managedPolicies: Arn[];
private readonly attachedPolicies = new AttachedPolicies();

constructor(parent: Construct, name: string, props: RoleProps) {
Expand Down Expand Up @@ -140,7 +140,7 @@ export class Role extends Construct implements IIdentityResource, IPrincipal, ID
* Attaches a managed policy to this role.
* @param arn The ARN of the managed policy to attach.
*/
public attachManagedPolicy(arn: any) {
public attachManagedPolicy(arn: Arn) {
this.managedPolicies.push(arn);
}

Expand Down Expand Up @@ -169,4 +169,4 @@ function validateMaxSessionDuration(duration?: number) {
if (duration < 3600 || duration > 43200) {
throw new Error(`maxSessionDuration is set to ${duration}, but must be >= 3600sec (1hr) and <= 43200sec (12hrs)`);
}
}
}
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-iam/lib/user.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ArnPrincipal, Construct, PolicyPrincipal, PolicyStatement } from '@aws-cdk/cdk';
import { Arn, ArnPrincipal, Construct, PolicyPrincipal, PolicyStatement } from '@aws-cdk/cdk';
import { Group } from './group';
import { cloudformation, UserArn, UserName } from './iam.generated';
import { IIdentityResource, IPrincipal, Policy } from './policy';
Expand Down Expand Up @@ -79,7 +79,7 @@ export class User extends Construct implements IIdentityResource, IPrincipal {
public readonly principal: PolicyPrincipal;

private readonly groups = new Array<any>();
private readonly managedPolicies = new Array<string>();
private readonly managedPolicies = new Array<Arn>();
private readonly attachedPolicies = new AttachedPolicies();
private defaultPolicy?: Policy;

Expand Down Expand Up @@ -114,7 +114,7 @@ export class User extends Construct implements IIdentityResource, IPrincipal {
* Attaches a managed policy to the user.
* @param arn The ARN of the managed policy to attach.
*/
public attachManagedPolicy(arn: any) {
public attachManagedPolicy(arn: Arn) {
this.managedPolicies.push(arn);
}

Expand Down Expand Up @@ -152,4 +152,4 @@ export class User extends Construct implements IIdentityResource, IPrincipal {

return undefined; // no console access
}
}
}
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-iam/test/integ.policy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { App, PolicyStatement, Stack } from "@aws-cdk/cdk";
import { App, Arn, PolicyStatement, Stack } from "@aws-cdk/cdk";
import { Policy } from "../lib";
import { User } from "../lib/user";

Expand All @@ -9,11 +9,11 @@ const stack = new Stack(app, 'aws-cdk-iam-policy');
const user = new User(stack, 'MyUser');

const policy = new Policy(stack, 'HelloPolicy', { policyName: 'Default' });
policy.addStatement(new PolicyStatement().addResource('*').addAction('sqs:SendMessage'));
policy.addStatement(new PolicyStatement().addResource(new Arn('*')).addAction('sqs:SendMessage'));
policy.attachToUser(user);

const policy2 = new Policy(stack, 'GoodbyePolicy');
policy2.addStatement(new PolicyStatement().addResource('*').addAction('lambda:InvokeFunction'));
policy2.addStatement(new PolicyStatement().addResource(new Arn('*')).addAction('lambda:InvokeFunction'));
policy2.attachToUser(user);

process.stdout.write(app.run());
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-iam/test/integ.role.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { App, PolicyStatement, ServicePrincipal, Stack } from "@aws-cdk/cdk";
import { App, Arn, PolicyStatement, ServicePrincipal, Stack } from "@aws-cdk/cdk";
import { Policy, Role } from "../lib";

const app = new App(process.argv);
Expand All @@ -9,10 +9,10 @@ const role = new Role(stack, 'TestRole', {
assumedBy: new ServicePrincipal('sqs.amazonaws.com')
});

role.addToPolicy(new PolicyStatement().addResource('*').addAction('sqs:SendMessage'));
role.addToPolicy(new PolicyStatement().addResource(new Arn('*')).addAction('sqs:SendMessage'));

const policy = new Policy(stack, 'HelloPolicy', { policyName: 'Default' });
policy.addStatement(new PolicyStatement().addAction('ec2:*').addResource('*'));
policy.addStatement(new PolicyStatement().addAction('ec2:*').addResource(new Arn('*')));
policy.attachToRole(role);

process.stdout.write(app.run());
18 changes: 9 additions & 9 deletions packages/@aws-cdk/aws-iam/test/test.policy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { App, PolicyStatement, ServicePrincipal, Stack } from '@aws-cdk/cdk';
import { App, Arn, PolicyStatement, ServicePrincipal, Stack } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { Role } from '../lib';
import { Group } from '../lib/group';
Expand All @@ -21,8 +21,8 @@ export = {
const stack = new Stack(app, 'MyStack');

const policy = new Policy(stack, 'MyPolicy', { policyName: 'MyPolicyName' });
policy.addStatement(new PolicyStatement().addResource('*').addAction('sqs:SendMessage'));
policy.addStatement(new PolicyStatement().addResource('arn').addAction('sns:Subscribe'));
policy.addStatement(new PolicyStatement().addResource(new Arn('*')).addAction('sqs:SendMessage'));
policy.addStatement(new PolicyStatement().addResource(new Arn('arn')).addAction('sns:Subscribe'));

const group = new Group(stack, 'MyGroup');
group.attachInlinePolicy(policy);
Expand All @@ -47,8 +47,8 @@ export = {
const stack = new Stack(app, 'MyStack');

const policy = new Policy(stack, 'MyPolicy');
policy.addStatement(new PolicyStatement().addResource('*').addAction('sqs:SendMessage'));
policy.addStatement(new PolicyStatement().addResource('arn').addAction('sns:Subscribe'));
policy.addStatement(new PolicyStatement().addResource(new Arn('*')).addAction('sqs:SendMessage'));
policy.addStatement(new PolicyStatement().addResource(new Arn('arn')).addAction('sns:Subscribe'));

const user = new User(stack, 'MyUser');
user.attachInlinePolicy(policy);
Expand Down Expand Up @@ -84,7 +84,7 @@ export = {
users: [ user1 ],
groups: [ group1 ],
roles: [ role1 ],
statements: [ new PolicyStatement().addResource('*').addAction('dynamodb:PutItem') ],
statements: [ new PolicyStatement().addResource(new Arn('*')).addAction('dynamodb:PutItem') ],
});

test.deepEqual(app.synthesizeStack(stack.name).template, { Resources:
Expand Down Expand Up @@ -118,7 +118,7 @@ export = {
const app = new App();
const stack = new Stack(app, 'MyStack');
const p = new Policy(stack, 'MyPolicy');
p.addStatement(new PolicyStatement().addAction('*').addResource('*'));
p.addStatement(new PolicyStatement().addAction('*').addResource(new Arn('*')));

const user = new User(stack, 'MyUser');
p.attachToUser(user);
Expand Down Expand Up @@ -150,7 +150,7 @@ export = {
p.attachToUser(new User(stack, 'User2'));
p.attachToGroup(new Group(stack, 'Group1'));
p.attachToRole(new Role(stack, 'Role1', { assumedBy: new ServicePrincipal('lambda.amazonaws.com') }));
p.addStatement(new PolicyStatement().addResource('*').addAction('dynamodb:GetItem'));
p.addStatement(new PolicyStatement().addResource(new Arn('*')).addAction('dynamodb:GetItem'));

test.deepEqual(app.synthesizeStack(stack.name).template, { Resources:
{ MyTestPolicy316BDB50:
Expand Down Expand Up @@ -192,7 +192,7 @@ export = {
group.attachInlinePolicy(policy);
role.attachInlinePolicy(policy);

policy.addStatement(new PolicyStatement().addResource('*').addAction('*'));
policy.addStatement(new PolicyStatement().addResource(new Arn('*')).addAction('*'));

test.deepEqual(app.synthesizeStack(stack.name).template, { Resources:
{ MyPolicy39D66CF6:
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-iam/test/test.role.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect, haveResource } from '@aws-cdk/assert';
import { FederatedPrincipal, PolicyStatement, Resource, ServicePrincipal, Stack } from '@aws-cdk/cdk';
import { Arn, FederatedPrincipal, PolicyStatement, Resource, ServicePrincipal, Stack } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { Role } from '../lib';

Expand Down Expand Up @@ -33,7 +33,7 @@ export = {

test.ok(!('MyRoleDefaultPolicyA36BE1DD' in stack.toCloudFormation().Resources), 'initially created without a policy');

role.addToPolicy(new PolicyStatement().addResource('myresource').addAction('myaction'));
role.addToPolicy(new PolicyStatement().addResource(new Arn('myresource')).addAction('myaction'));
test.ok(stack.toCloudFormation().Resources.MyRoleDefaultPolicyA36BE1DD, 'policy resource created');

expect(stack).toMatch({ Resources:
Expand Down Expand Up @@ -66,7 +66,7 @@ export = {
managedPolicyArns: [ 'managed1', 'managed2' ]
});

role.attachManagedPolicy('managed3');
role.attachManagedPolicy(new Arn('managed3'));
expect(stack).toMatch({ Resources:
{ MyRoleF48FFE04:
{ Type: 'AWS::IAM::Role',
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-kinesis/lib/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export abstract class StreamRef extends cdk.Construct implements logs.ILogSubscr
if (!this.cloudWatchLogsRole) {
// Create a role to be assumed by CWL that can write to this stream and pass itself.
this.cloudWatchLogsRole = new iam.Role(this, 'CloudWatchLogsCanPutRecords', {
assumedBy: new cdk.ServicePrincipal(new cdk.FnConcat('logs.', new cdk.AwsRegion(), '.amazonaws.com')),
assumedBy: new cdk.ServicePrincipal(new cdk.FnConcat('logs.', new cdk.AwsRegion(), '.amazonaws.com').toString()),
});
this.cloudWatchLogsRole.addToPolicy(new cdk.PolicyStatement().addAction('kinesis:PutRecord').addResource(this.streamArn));
this.cloudWatchLogsRole.addToPolicy(new cdk.PolicyStatement().addAction('iam:PassRole').addResource(this.cloudWatchLogsRole.roleArn));
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-kms/lib/key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class EncryptionKey extends EncryptionKeyRef {
];

this.addToResourcePolicy(new PolicyStatement()
.addResource('*')
.addAllResources()
.addActions(...actions)
.addAccountRootPrincipal());
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-kms/test/integ.key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const stack = new Stack(app, `aws-cdk-kms-1`);
const key = new EncryptionKey(stack, 'MyKey');

key.addToResourcePolicy(new PolicyStatement()
.addResource('*')
.addAllResources()
.addAction('kms:encrypt')
.addAwsPrincipal(new AwsAccountId()));

Expand Down
16 changes: 8 additions & 8 deletions packages/@aws-cdk/aws-kms/test/test.key.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { exactlyMatchTemplate, expect } from '@aws-cdk/assert';
import { App, PolicyDocument, PolicyStatement, Stack } from '@aws-cdk/cdk';
import { App, Arn, PolicyDocument, PolicyStatement, Stack } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { EncryptionKey, KeyArn } from '../lib';

Expand Down Expand Up @@ -69,8 +69,8 @@ export = {
const stack = new Stack(app, 'Test');

const key = new EncryptionKey(stack, 'MyKey');
const p = new PolicyStatement().addResource('*').addAction('kms:encrypt');
p.addAwsPrincipal('arn');
const p = new PolicyStatement().addAllResources().addAction('kms:encrypt');
p.addAwsPrincipal(new Arn('arn'));
key.addToResourcePolicy(p);

expect(app.synthesizeStack(stack.name)).to(exactlyMatchTemplate({
Expand Down Expand Up @@ -144,8 +144,8 @@ export = {
enableKeyRotation: true,
enabled: false
});
const p = new PolicyStatement().addResource('*').addAction('kms:encrypt');
p.addAwsPrincipal('arn');
const p = new PolicyStatement().addAllResources().addAction('kms:encrypt');
p.addAwsPrincipal(new Arn('arn'));
key.addToResourcePolicy(p);

expect(app.synthesizeStack(stack.name)).to(exactlyMatchTemplate({
Expand Down Expand Up @@ -297,7 +297,7 @@ export = {
'import/export can be used to bring in an existing key'(test: Test) {
const stack1 = new Stack();
const policy = new PolicyDocument();
policy.addStatement(new PolicyStatement().addResource('*'));
policy.addStatement(new PolicyStatement().addAllResources());
const myKey = new EncryptionKey(stack1, 'MyKey', { policy });
const exportedKeyRef = myKey.export();

Expand Down Expand Up @@ -357,7 +357,7 @@ export = {

const key = EncryptionKey.import(stack, 'Imported', { keyArn: new KeyArn('foo/bar') });

key.addToResourcePolicy(new PolicyStatement().addResource('*').addAction('*'));
key.addToResourcePolicy(new PolicyStatement().addAllResources().addAction('*'));

test.done();
},
Expand All @@ -369,7 +369,7 @@ export = {
const key = EncryptionKey.import(stack, 'Imported', { keyArn: new KeyArn('foo/bar') });

test.throws(() =>
key.addToResourcePolicy(new PolicyStatement().addResource('*').addAction('*'), /* allowNoOp */ false),
key.addToResourcePolicy(new PolicyStatement().addAllResources().addAction('*'), /* allowNoOp */ false),
'Unable to add statement to IAM resource policy for KMS key: "foo/bar"');

test.done();
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ export abstract class FunctionRef extends cdk.Construct
//
// (Wildcards in principals are unfortunately not supported.
this.addPermission('InvokedByCloudWatchLogs', {
principal: new cdk.ServicePrincipal(new cdk.FnConcat('logs.', new cdk.AwsRegion(), '.amazonaws.com')),
principal: new cdk.ServicePrincipal(new cdk.FnConcat('logs.', new cdk.AwsRegion(), '.amazonaws.com').toString()),
sourceArn: arn
});
this.logSubscriptionDestinationPolicyAddedFor.push(arn);
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda/lib/pipeline-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class PipelineInvokeAction extends codepipeline.Action {
// allow pipeline to list functions
props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement()
.addAction('lambda:ListFunctions')
.addResource('*'));
.addAllResources());

// allow pipeline to invoke this lambda functionn
props.stage.pipelineRole.addToPolicy(new cdk.PolicyStatement()
Expand All @@ -71,7 +71,7 @@ export class PipelineInvokeAction extends codepipeline.Action {
const addToPolicy = props.addPutJobResultPolicy !== undefined ? props.addPutJobResultPolicy : true;
if (addToPolicy) {
props.lambda.addToRolePolicy(new cdk.PolicyStatement()
.addResource('*') // to avoid cycles (see docs)
.addAllResources() // to avoid cycles (see docs)
.addAction('codepipeline:PutJobSuccessResult')
.addAction('codepipeline:PutJobFailureResult'));
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda/test/integ.lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const fn = new lambda.Function(stack, 'MyLambda', {
runtime: lambda.Runtime.NodeJS610,
});

fn.addToRolePolicy(new cdk.PolicyStatement().addResource('*').addAction('*'));
fn.addToRolePolicy(new cdk.PolicyStatement().addAllResources().addAction('*'));

const version = fn.addVersion('1');

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-lambda/test/test.lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export = {
code: new lambda.InlineCode('foo'),
handler: 'index.handler',
runtime: lambda.Runtime.NodeJS610,
initialPolicy: [new cdk.PolicyStatement().addAction("*").addResource("*")]
initialPolicy: [new cdk.PolicyStatement().addAction("*").addAllResources()]
});
expect(stack).toMatch({ Resources:
{ MyLambdaServiceRole4539ECB6:
Expand Down Expand Up @@ -186,7 +186,7 @@ export = {
const stack = new cdk.Stack();
const fn = newTestLambda(stack);

test.throws(() => fn.addPermission('F1', { principal: new cdk.ArnPrincipal('just:arn') }),
test.throws(() => fn.addPermission('F1', { principal: new cdk.ArnPrincipal(new cdk.Arn('just:arn')) }),
/Invalid principal type for Lambda permission statement/);

fn.addPermission('S1', { principal: new cdk.ServicePrincipal('my-service') });
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export abstract class BucketRef extends cdk.Construct {
.addActions(...keyActions));

this.encryptionKey.addToResourcePolicy(new cdk.PolicyStatement()
.addResource('*')
.addAllResources()
.addPrincipal(identity.principal)
.addActions(...keyActions));
}
Expand Down
Loading