Skip to content

Commit

Permalink
Introduce IPolicyStatement
Browse files Browse the repository at this point in the history
PolicyStatement is no longer IResolvable (doesn't need to be)

Remove AwsPrincipal, which was always an alias for ArnPrincipal
which is much clearer.

Introduce `PolicyStatement.fromAttributes({ ... })` as an
alternative to the fluent interface chaining of old `PolicyStatement`.
  • Loading branch information
rix0rrr committed Jun 11, 2019
1 parent 9e21e6e commit 4093679
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 62 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ If multiple principals are added to the policy statement, they will be merged to
const statement = new PolicyStatement();
statement.addServicePrincipal('cloudwatch.amazonaws.com');
statement.addServicePrincipal('ec2.amazonaws.com');
statement.addAwsPrincipal('arn:aws:boom:boom');
statement.addArnPrincipal('arn:aws:boom:boom');
```

Will result in:
Expand Down
17 changes: 13 additions & 4 deletions packages/@aws-cdk/aws-iam/lib/policy-document.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import cdk = require('@aws-cdk/cdk');
import { IPostProcessor } from '@aws-cdk/cdk';
import { PolicyStatement } from './policy-statement';
import { IPolicyStatement } from './policy-statement';

/**
* Properties for a new PolicyDocument
Expand All @@ -18,7 +18,7 @@ export interface PolicyDocumentProps {
* A PolicyDocument is a collection of statements
*/
export class PolicyDocument implements cdk.IResolvable {
private readonly statements = new Array<PolicyStatement>();
private readonly statements = new Array<IPolicyStatement>();
private readonly autoAssignSids: boolean;

constructor(props: PolicyDocumentProps = {}) {
Expand Down Expand Up @@ -47,7 +47,7 @@ export class PolicyDocument implements cdk.IResolvable {
*
* @param statement the statement to add.
*/
public addStatements(...statement: PolicyStatement[]) {
public addStatements(...statement: IPolicyStatement[]) {
this.statements.push(...statement);
}

Expand All @@ -60,13 +60,22 @@ export class PolicyDocument implements cdk.IResolvable {
});
}

/**
* JSON-ify the document
*
* Used when JSON.stringify() is called
*/
public toJSON() {
return this.render();
}

private render(): any {
if (this.isEmpty) {
return undefined;
}

const doc = {
Statement: this.statements,
Statement: this.statements.map(s => s.toStatementJson()),
Version: '2012-10-17'
};

Expand Down
81 changes: 66 additions & 15 deletions packages/@aws-cdk/aws-iam/lib/policy-statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,34 @@ import { AccountPrincipal, AccountRootPrincipal, Anyone, ArnPrincipal, Canonical
FederatedPrincipal, IPrincipal, ServicePrincipal, ServicePrincipalOpts } from './principals';
import { mergePrincipal } from './util';

/**
* An abstract PolicyStatement
*/
export interface IPolicyStatement {
/**
* Render the policy statement to JSON
*/
toStatementJson(): any;
}

/**
* Represents a statement in an IAM policy document.
*/
export class PolicyStatement implements cdk.IResolvable {
export class PolicyStatement implements IPolicyStatement {
public static fromAttributes(attrs: PolicyStatementAttributes): IPolicyStatement {
const st = new PolicyStatement();
st.addActions(...attrs.actions || []);
(attrs.principals || []).forEach(p => st.addPrincipal(p));
st.addResources(...attrs.resourceArns || []);
if (attrs.conditions !== undefined) {
st.addConditions(attrs.conditions);
}
return st;
}

/**
* Statement ID for this statement
*/
public sid?: string;

private action = new Array<any>();
Expand Down Expand Up @@ -51,16 +75,12 @@ export class PolicyStatement implements cdk.IResolvable {
return this;
}

public addAwsPrincipal(arn: string): this {
return this.addPrincipal(new ArnPrincipal(arn));
}

public addAwsAccountPrincipal(accountId: string): this {
return this.addPrincipal(new AccountPrincipal(accountId));
}

public addArnPrincipal(arn: string): this {
return this.addAwsPrincipal(arn);
return this.addPrincipal(new ArnPrincipal(arn));
}

/**
Expand Down Expand Up @@ -180,14 +200,7 @@ export class PolicyStatement implements cdk.IResolvable {
return this.addCondition('StringEquals', { 'sts:ExternalId': accountId });
}

//
// Serialization
//
public resolve(_context: cdk.IResolveContext): any {
return this.toJson();
}

public toJson(): any {
public toStatementJson(): any {
return {
Action: _norm(this.action),
Condition: _norm(this.condition),
Expand Down Expand Up @@ -251,12 +264,50 @@ export class PolicyStatement implements cdk.IResolvable {
});
}

/**
* JSON-ify the statement
*
* Used when JSON.stringify() is called
*/
public toJSON() {
return this.toJson();
return this.toStatementJson();
}
}

export enum PolicyStatementEffect {
Allow = 'Allow',
Deny = 'Deny',
}

/**
* Interface for creating a policy statement
*/
export interface PolicyStatementAttributes {
/**
* List of actions to add to the statement
*
* @default - no actions
*/
actions?: string[];

/**
* List of principals to add to the statement
*
* @default - no principals
*/
principals?: IPrincipal[];

/**
* Resource ARNs to add to the statement
*
* @default - no principals
*/
resourceArns?: string[];

/**
* Conditions to add to the statement
*
* @default - no condition
*/
conditions?: {[key: string]: any};
}
83 changes: 44 additions & 39 deletions packages/@aws-cdk/aws-iam/test/test.policy-document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export = {
p.addAwsAccountPrincipal(`my${Token.asString({ account: 'account' })}name`);
p.limitToAccount('12221121221');

test.deepEqual(stack.resolve(p), { Action:
test.deepEqual(stack.resolve(p.toStatementJson()), { Action:
[ 'sqs:SendMessage',
'dynamodb:CreateTable',
'dynamodb:DeleteTable' ],
Expand Down Expand Up @@ -63,7 +63,7 @@ export = {
'Permission allows specifying multiple actions upon construction'(test: Test) {
const stack = new Stack();
const perm = new PolicyStatement().addResource('MyResource').addActions('Action1', 'Action2', 'Action3');
test.deepEqual(stack.resolve(perm), {
test.deepEqual(stack.resolve(perm.toStatementJson()), {
Effect: 'Allow',
Action: [ 'Action1', 'Action2', 'Action3' ],
Resource: 'MyResource' });
Expand All @@ -82,7 +82,7 @@ export = {
const p = new PolicyStatement();
const canoncialUser = "averysuperduperlongstringfor";
p.addPrincipal(new CanonicalUserPrincipal(canoncialUser));
test.deepEqual(stack.resolve(p), {
test.deepEqual(stack.resolve(p.toStatementJson()), {
Effect: "Allow",
Principal: {
CanonicalUser: canoncialUser
Expand All @@ -96,7 +96,7 @@ export = {

const p = new PolicyStatement();
p.addAccountRootPrincipal();
test.deepEqual(stack.resolve(p), {
test.deepEqual(stack.resolve(p.toStatementJson()), {
Effect: "Allow",
Principal: {
AWS: {
Expand All @@ -120,7 +120,7 @@ export = {
const stack = new Stack();
const p = new PolicyStatement();
p.addFederatedPrincipal("com.amazon.cognito", { StringEquals: { key: 'value' }});
test.deepEqual(stack.resolve(p), {
test.deepEqual(stack.resolve(p.toStatementJson()), {
Effect: "Allow",
Principal: {
Federated: "com.amazon.cognito"
Expand All @@ -138,7 +138,7 @@ export = {
const p = new PolicyStatement();
p.addAwsAccountPrincipal('1234');
p.addAwsAccountPrincipal('5678');
test.deepEqual(stack.resolve(p), {
test.deepEqual(stack.resolve(p.toStatementJson()), {
Effect: 'Allow',
Principal: {
AWS: [
Expand Down Expand Up @@ -181,7 +181,7 @@ export = {

'true if there is a principal'(test: Test) {
const p = new PolicyStatement();
p.addAwsPrincipal('bla');
p.addArnPrincipal('bla');
test.equal(p.hasPrincipal, true);
test.done();
}
Expand Down Expand Up @@ -244,34 +244,14 @@ export = {
}
},

'addAwsPrincipal/addArnPrincipal are the aliases'(test: Test) {
const stack = new Stack();
const p = new PolicyDocument();

p.addStatements(new PolicyStatement().addAwsPrincipal('111222-A'));
p.addStatements(new PolicyStatement().addArnPrincipal('111222-B'));
p.addStatements(new PolicyStatement().addPrincipal(new ArnPrincipal('111222-C')));

test.deepEqual(stack.resolve(p), {
Statement: [ {
Effect: 'Allow', Principal: { AWS: '111222-A' } },
{ Effect: 'Allow', Principal: { AWS: '111222-B' } },
{ Effect: 'Allow', Principal: { AWS: '111222-C' } }
],
Version: '2012-10-17'
});

test.done();
},

'addResources() will not break a list-encoded Token'(test: Test) {
const stack = new Stack();

const statement = new PolicyStatement()
.addActions(...Lazy.listValue({ produce: () => ['a', 'b', 'c'] }))
.addResources(...Lazy.listValue({ produce: () => ['x', 'y', 'z'] }));

test.deepEqual(stack.resolve(statement), {
test.deepEqual(stack.resolve(statement.toStatementJson()), {
Effect: 'Allow',
Action: ['a', 'b', 'c'],
Resource: ['x', 'y', 'z'],
Expand Down Expand Up @@ -308,7 +288,7 @@ export = {
};
const s = new PolicyStatement().addAccountRootPrincipal()
.addPrincipal(arrayPrincipal);
test.deepEqual(stack.resolve(s), {
test.deepEqual(stack.resolve(s.toStatementJson()), {
Effect: 'Allow',
Principal: {
AWS: [
Expand All @@ -324,12 +304,12 @@ export = {
'policy statements with multiple principal types can be created using multiple addPrincipal calls'(test: Test) {
const stack = new Stack();
const s = new PolicyStatement()
.addAwsPrincipal('349494949494')
.addArnPrincipal('349494949494')
.addServicePrincipal('test.service')
.addResource('resource')
.addAction('action');

test.deepEqual(stack.resolve(s), {
test.deepEqual(stack.resolve(s.toStatementJson()), {
Action: 'action',
Effect: 'Allow',
Principal: { AWS: '349494949494', Service: 'test.service' },
Expand All @@ -346,7 +326,7 @@ export = {
.addAction('test:Action')
.addServicePrincipal('codedeploy.amazonaws.com');

test.deepEqual(stack.resolve(s), {
test.deepEqual(stack.resolve(s.toStatementJson()), {
Effect: 'Allow',
Action: 'test:Action',
Principal: { Service: 'codedeploy.cn-north-1.amazonaws.com.cn' }
Expand All @@ -361,7 +341,7 @@ export = {
.addAction('test:Action')
.addServicePrincipal('codedeploy.amazonaws.com', { region: 'cn-north-1' });

test.deepEqual(stack.resolve(s), {
test.deepEqual(stack.resolve(s.toStatementJson()), {
Effect: 'Allow',
Action: 'test:Action',
Principal: { Service: 'codedeploy.cn-north-1.amazonaws.com.cn' }
Expand All @@ -376,7 +356,7 @@ export = {
.addAction('test:Action')
.addServicePrincipal('test.service-principal.dev');

test.deepEqual(stack.resolve(s), {
test.deepEqual(stack.resolve(s.toStatementJson()), {
Effect: 'Allow',
Action: 'test:Action',
Principal: { Service: 'test.service-principal.dev' }
Expand All @@ -392,7 +372,7 @@ export = {
const stack = new Stack();
const p = new CompositePrincipal(new ArnPrincipal('i:am:an:arn'));
const statement = new PolicyStatement().addPrincipal(p);
test.deepEqual(stack.resolve(statement), { Effect: 'Allow', Principal: { AWS: 'i:am:an:arn' } });
test.deepEqual(stack.resolve(statement.toStatementJson()), { Effect: 'Allow', Principal: { AWS: 'i:am:an:arn' } });
test.done();
},

Expand Down Expand Up @@ -420,10 +400,10 @@ export = {
const statement = new PolicyStatement().addPrincipal(p);

// add via policy statement
statement.addAwsPrincipal('aws-principal-3');
statement.addArnPrincipal('aws-principal-3');
statement.addCondition('cond2', { boom: 123 });

test.deepEqual(stack.resolve(statement), {
test.deepEqual(stack.resolve(statement.toStatementJson()), {
Condition: {
cond2: { boom: 123 }
},
Expand Down Expand Up @@ -524,5 +504,30 @@ export = {
],
});
test.done();
}
};
},

'fromAttributes is equivalent'(test: Test) {
const stack = new Stack();

const doc1 = new PolicyDocument();
doc1.addStatements(new PolicyStatement()
.addActions('action1', 'action2')
.addAllResources()
.addArnPrincipal('arn')
.addCondition('key', { equals: 'value' }));

const doc2 = new PolicyDocument();
doc2.addStatements(PolicyStatement.fromAttributes({
actions: ['action1', 'action2'],
resourceArns: ['*'],
principals: [new ArnPrincipal('arn')],
conditions: {
key: { equals: 'value' }
}
}));

test.deepEqual(stack.resolve(doc1), stack.resolve(doc2));

test.done();
},
};
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 @@ -11,7 +11,7 @@ const key = new Key(stack, 'MyKey', { retain: false });
key.addToResourcePolicy(new PolicyStatement()
.addAllResources()
.addAction('kms:encrypt')
.addAwsPrincipal(stack.accountId));
.addArnPrincipal(stack.accountId));

key.addAlias('alias/bar');

Expand Down
Loading

0 comments on commit 4093679

Please sign in to comment.