Skip to content

Commit

Permalink
fix(iam): correctly limit the default PolicyName to 128 characters
Browse files Browse the repository at this point in the history
Our logic for trimming the length of the default IAM policy name was not working,
as it wasn't updated when logicalId became a Token rather than a literate string,
and so it was never actually triggered
(it just checked that the display name of the Token was less than 128 characters,
which it always is).
The fix is to resolve the logical ID Token before applying the trimming logic.

Fixes aws#3402
  • Loading branch information
skinny85 committed Jul 30, 2019
1 parent 4cb9755 commit fb0822d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 13 deletions.
13 changes: 11 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, IResource, Lazy, Resource } from '@aws-cdk/core';
import { Construct, DefaultTokenResolver, IResource, Lazy, Resource, StringConcat, Tokenization } from '@aws-cdk/core';
import { IGroup } from './group';
import { CfnPolicy } from './iam.generated';
import { PolicyDocument } from './policy-document';
Expand Down Expand Up @@ -96,7 +96,16 @@ export class Policy extends Resource implements IPolicy {
// generatePolicyName will take the last 128 characters of the logical id since
// policy names are limited to 128. the last 8 chars are a stack-unique hash, so
// that shouod be sufficient to ensure uniqueness within a principal.
Lazy.stringValue({ produce: () => generatePolicyName(resource.logicalId) })
Lazy.stringValue({
produce: () => {
// as logicalId is itself a Token,
// resolve it first before passing to generatePolicyName()
return generatePolicyName(Tokenization.resolve(resource.logicalId, {
scope,
resolver: new DefaultTokenResolver(new StringConcat()),
}));
},
}),
});

const resource = new CfnPolicy(this, 'Resource', {
Expand Down
31 changes: 20 additions & 11 deletions packages/@aws-cdk/aws-iam/test/test.policy.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { expect } from '@aws-cdk/assert';
import { expect, haveResourceLike } from '@aws-cdk/assert';
import { App, Stack } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { Group, Policy, PolicyStatement, Role, ServicePrincipal, User } from '../lib';
import { generatePolicyName } from '../lib/util';
import { AnyPrincipal, CfnPolicy, Group, Policy, PolicyStatement, Role, ServicePrincipal, User } from '../lib';

// tslint:disable:object-literal-key-quotes

export = {
'fails when policy is empty'(test: Test) {
Expand Down Expand Up @@ -255,16 +256,24 @@ export = {
},

'generated policy name only uses the last 128 characters of the logical id'(test: Test) {
test.equal(generatePolicyName('Foo'), 'Foo');
const stack = new Stack();

const logicalId50 = '[' + dup(50 - 2) + ']';
test.equal(generatePolicyName(logicalId50), logicalId50);
const logicalId128 = 'a' + dup(128 - 2) + 'a';
const withPrefix = 'PREFIX' + logicalId128;

const logicalId128 = '[' + dup(128 - 2) + ']';
test.equal(generatePolicyName(logicalId128), logicalId128);
const policy = new Policy(stack, withPrefix);
const cfnPolicy = policy.node.defaultChild as CfnPolicy;
cfnPolicy.overrideLogicalId(withPrefix); // force a particular logical ID
// add statements & principal to satisfy validation
policy.addStatements(new PolicyStatement({
actions: ['*'],
resources: ['*'],
}));
policy.attachToRole(new Role(stack, 'Role', { assumedBy: new AnyPrincipal() }));

const withPrefix = 'PREFIX' + logicalId128;
test.equal(generatePolicyName(withPrefix), logicalId128, 'ensure prefix is omitted');
expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
"PolicyName": logicalId128,
}));

test.done();

Expand All @@ -275,5 +284,5 @@ export = {
}
return r;
}
}
},
};

0 comments on commit fb0822d

Please sign in to comment.