fix(iam): correctly limit the default PolicyName to 128 characters (#3487)

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 #3402

Author: skinny85

packages/@aws-cdk/aws-iam/lib/policy.ts

@@ -96,7 +96,7 @@ 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: () => generatePolicyName(scope, resource.logicalId) })
     });
 
     const resource = new CfnPolicy(this, 'Resource', {

packages/@aws-cdk/aws-iam/lib/util.ts

@@ -1,4 +1,4 @@
-import { Lazy } from '@aws-cdk/core';
+import { DefaultTokenResolver, IConstruct, Lazy, StringConcat, Tokenization } from '@aws-cdk/core';
 import { IPolicy } from './policy';
 
 const MAX_POLICY_NAME_LEN = 128;
@@ -16,8 +16,25 @@ export function undefinedIfEmpty(f: () => string[]): string[] {
  * 128 characters, so we take the last 128 characters (in order to make sure the hash
  * is there).
  */
-export function generatePolicyName(logicalId: string) {
-  return logicalId.substring(Math.max(logicalId.length - MAX_POLICY_NAME_LEN, 0), logicalId.length);
+export function generatePolicyName(scope: IConstruct, logicalId: string): string {
+  // as logicalId is itself a Token, resolve it first
+  const resolvedLogicalId = Tokenization.resolve(logicalId, {
+    scope,
+    resolver: new DefaultTokenResolver(new StringConcat()),
+  });
+  return lastNCharacters(resolvedLogicalId, MAX_POLICY_NAME_LEN);
+}
+
+/**
+ * Returns a string composed of the last n characters of str.
+ * If str is shorter than n, returns str.
+ *
+ * @param str the string to return the last n characters of
+ * @param n how many characters to return
+ */
+function lastNCharacters(str: string, n: number) {
+  const startIndex = Math.max(str.length - n, 0);
+  return str.substring(startIndex, str.length);
 }
 
 /**
@@ -61,4 +78,4 @@ export function mergePrincipal(target: { [key: string]: string[] }, source: { [k
   }
 
   return target;
-}
+}

packages/@aws-cdk/aws-iam/test/test.policy.ts

@@ -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) {
@@ -254,17 +255,29 @@ export = {
     test.done();
   },
 
+  "generated policy name is the same as the logical id if it's shorter than 128 characters"(test: Test) {
+    const stack = new Stack();
+
+    createPolicyWithLogicalId(stack, 'Foo');
+
+    expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
+      "PolicyName": "Foo",
+    }));
+
+    test.done();
+  },
+
   '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 logicalIdOver128 = 'PREFIX' + logicalId128;
 
-    const logicalId128 = '[' + dup(128 - 2) + ']';
-    test.equal(generatePolicyName(logicalId128), logicalId128);
+    createPolicyWithLogicalId(stack, logicalIdOver128);
 
-    const withPrefix = 'PREFIX' + logicalId128;
-    test.equal(generatePolicyName(withPrefix), logicalId128, 'ensure prefix is omitted');
+    expect(stack).to(haveResourceLike('AWS::IAM::Policy', {
+      "PolicyName": logicalId128,
+    }));
 
     test.done();
 
@@ -275,5 +288,18 @@ export = {
       }
       return r;
     }
-  }
+  },
 };
+
+function createPolicyWithLogicalId(stack: Stack, logicalId: string): void {
+  const policy = new Policy(stack, logicalId);
+  const cfnPolicy = policy.node.defaultChild as CfnPolicy;
+  cfnPolicy.overrideLogicalId(logicalId); // 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() }));
+}