fix(iam): only attach policies to imported roles if the accounts match (

#3716)

We allow attaching policies to the imported role if the account passed in the ARN
matches the account of the stack the policy belongs to,
or if either one of those accounts was not specified (i.e., is env-agnostic),
or if the ARN used for importing was dynamic.

Fixes #2985
Fixes #3025

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

@@ -1,4 +1,4 @@
-import { Construct, Duration, Lazy, Resource, Stack } from '@aws-cdk/core';
+import { Construct, Duration, Lazy, Resource, Stack, Token } from '@aws-cdk/core';
 import { Grant } from './grant';
 import { CfnRole } from './iam.generated';
 import { IIdentity } from './identity-base';
@@ -123,29 +123,78 @@ export interface RoleProps {
   readonly maxSessionDuration?: Duration;
 }
 
+/**
+ * Options allowing customizing the behavior of {@link Role.fromRoleArn}.
+ */
+export interface FromRoleArnOptions {
+  /**
+   * Whether the imported role can be modified by attaching policy resources to it.
+   *
+   * @default true
+   *
+   * @experimental
+   */
+  readonly mutable?: boolean;
+}
+
 /**
  * IAM Role
  *
  * Defines an IAM role. The role is created with an assume policy document associated with
  * the specified AWS service principal defined in `serviceAssumeRole`.
  */
 export class Role extends Resource implements IRole {
-
   /**
-   * Imports an external role by ARN
+   * Imports an external role by ARN.
+   *
    * @param scope construct scope
    * @param id construct id
    * @param roleArn the ARN of the role to import
+   * @param options allow customizing the behavior of the returned role
    */
-  public static fromRoleArn(scope: Construct, id: string, roleArn: string): IRole {
+  public static fromRoleArn(scope: Construct, id: string, roleArn: string, options: FromRoleArnOptions = {}): IRole {
+    const scopeStack = Stack.of(scope);
+    const parsedArn = scopeStack.parseArn(roleArn);
+    const roleName = parsedArn.resourceName!;
 
-    class Import extends Resource implements IRole {
+    abstract class Import extends Resource implements IRole {
       public readonly grantPrincipal: IPrincipal = this;
       public readonly assumeRoleAction: string = 'sts:AssumeRole';
       public readonly policyFragment = new ArnPrincipal(roleArn).policyFragment;
       public readonly roleArn = roleArn;
-      public readonly roleName = Stack.of(scope).parseArn(roleArn).resourceName!;
+      public readonly roleName = roleName;
+
+      public abstract addToPolicy(statement: PolicyStatement): boolean;
+
+      public abstract attachInlinePolicy(policy: Policy): void;
 
+      public addManagedPolicy(_policy: IManagedPolicy): void {
+        // FIXME: Add warning that we're ignoring this
+      }
+
+      /**
+       * Grant permissions to the given principal to pass this role.
+       */
+      public grantPassRole(identity: IPrincipal): Grant {
+        return this.grant(identity, 'iam:PassRole');
+      }
+
+      /**
+       * Grant the actions defined in actions to the identity Principal on this resource.
+       */
+      public grant(grantee: IPrincipal, ...actions: string[]): Grant {
+        return Grant.addToPrincipal({
+          grantee,
+          actions,
+          resourceArns: [this.roleArn],
+          scope: this,
+        });
+      }
+    }
+
+    const roleAccount = parsedArn.account;
+
+    class MutableImport extends Import {
       private readonly attachedPolicies = new AttachedPolicies();
       private defaultPolicy?: Policy;
 
@@ -159,36 +208,36 @@ export class Role extends Resource implements IRole {
       }
 
       public attachInlinePolicy(policy: Policy): void {
-        this.attachedPolicies.attach(policy);
-        policy.attachToRole(this);
-      }
+        const policyAccount = Stack.of(policy).account;
 
-      public addManagedPolicy(_policy: IManagedPolicy): void {
-        // FIXME: Add warning that we're ignoring this
+        if (accountsAreEqualOrOneIsUnresolved(policyAccount, roleAccount)) {
+          this.attachedPolicies.attach(policy);
+          policy.attachToRole(this);
+        }
       }
+    }
 
-      /**
-       * Grant the actions defined in actions to the identity Principal on this resource.
-       */
-      public grant(grantee: IPrincipal, ...actions: string[]): Grant {
-        return Grant.addToPrincipal({
-          grantee,
-          actions,
-          resourceArns: [this.roleArn],
-          scope: this
-        });
+    class ImmutableImport extends Import {
+      public addToPolicy(_statement: PolicyStatement): boolean {
+        return false;
       }
 
-      /**
-       * Grant permissions to the given principal to pass this role.
-       */
-      public grantPassRole(identity: IPrincipal): Grant {
-        return this.grant(identity, 'iam:PassRole');
+      public attachInlinePolicy(_policy: Policy): void {
+        // do nothing
       }
     }
 
-    return new Import(scope, id);
+    const scopeAccount = scopeStack.account;
+
+    return options.mutable !== false && accountsAreEqualOrOneIsUnresolved(scopeAccount, roleAccount)
+      ? new MutableImport(scope, id)
+      : new ImmutableImport(scope, id);
 
+    function accountsAreEqualOrOneIsUnresolved(account1: string | undefined,
+                                               account2: string | undefined): boolean {
+      return Token.isUnresolved(account1) || Token.isUnresolved(account2) ||
+        account1 === account2;
+    }
   }
 
   public readonly grantPrincipal: IPrincipal = this;

packages/@aws-cdk/aws-iam/package.json

@@ -96,6 +96,7 @@
   },
   "awslint": {
     "exclude": [
+      "from-signature:@aws-cdk/aws-iam.Role.fromRoleArn",
       "construct-interface-extends-iconstruct:@aws-cdk/aws-iam.IManagedPolicy",
       "resource-interface-extends-resource:@aws-cdk/aws-iam.IManagedPolicy"
     ]