From fb0822d121917579e2f20dc03b6691755ae514c8 Mon Sep 17 00:00:00 2001
From: Adam Ruka <adamruka@amazon.com>
Date: Tue, 30 Jul 2019 13:35:18 -0700
Subject: [PATCH] fix(iam): correctly limit the default PolicyName to 128
 characters

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
---
 packages/@aws-cdk/aws-iam/lib/policy.ts       | 13 ++++++--
 packages/@aws-cdk/aws-iam/test/test.policy.ts | 31 ++++++++++++-------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/packages/@aws-cdk/aws-iam/lib/policy.ts b/packages/@aws-cdk/aws-iam/lib/policy.ts
index 0617aacf6a5a1..4f00a6c5c2365 100644
--- a/packages/@aws-cdk/aws-iam/lib/policy.ts
+++ b/packages/@aws-cdk/aws-iam/lib/policy.ts
@@ -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';
@@ -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', {
diff --git a/packages/@aws-cdk/aws-iam/test/test.policy.ts b/packages/@aws-cdk/aws-iam/test/test.policy.ts
index 7fe0fb17affb6..b6eaf3985eaa1 100644
--- a/packages/@aws-cdk/aws-iam/test/test.policy.ts
+++ b/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) {
@@ -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();
 
@@ -275,5 +284,5 @@ export = {
       }
       return r;
     }
-  }
+  },
 };