feat(aws-iam): refactor grants, add OrganizationPrincipal (#1623)

Add support for non-identity Principals in grants (for example, principals that represent accounts or organization IDs). For resources that support them, the required IAM statements will be added to the resource policy. For resources that don't support them (because they don't have resource policies) an error will be thrown. Add a new `OrganizationPrincipal` principal which represents all identities in the given AWS Organization. Grant methods no longer accept an optional principal. Instead, they accept an `IGrantable`, which encodes constructs that have a principal to grant to. This principal must be always present, but may be a principal that can't do any work other than emitting warnings for imported resources. For construct authors, all grant methods must now return an `iam.Grant` object, and all should be implemented by calling one of the static factory methods on `iam.Grant` and returning its result. Fixes #236. BREAKING CHANGE: `grant(function.role)` and `grant(project.role)` are now `grant(function)` and `grant(role)`.
aws · Apr 3, 2019 · 1bb8ca9 · 1bb8ca9
1 parent ccbb6dd
commit 1bb8ca9
Show file tree

Hide file tree

Showing 65 changed files with 3,349 additions and 544 deletions.
diff --git a/design/aws-guidelines.md b/design/aws-guidelines.md
@@ -326,9 +326,9 @@ export interface IFoo extends cdk.IConstruct, ISomething {
   readonly connections: ec2.Connections;
 
   // permission grants (adds statements to the principal's policy)
-  grant(principal?: iam.IPrincipal, ...actions: string[]): void;
-  grantFoo(principal?: iam.IPrincipal): void;
-  grantBar(principal?: iam.IPrincipal): void;
+  grant(grantee?: iam.IGrantable, ...actions: string[]): void;
+  grantFoo(grantee?: iam.IGrantable): void;
+  grantBar(grantee?: iam.IGrantable): void;
 
   // resource policy (if applicable)
   addToResourcePolicy(statement: iam.PolicyStatement): void;
@@ -364,7 +364,7 @@ export abstract class FooBase extends cdk.Construct implements IFoo {
   public abstract export(): FooAttributes;
 
   // grants can usually be shared
-  public grantYyy(principal?: iam.IPrincipal) {
+  public grantYyy(grantee?: iam.IGrantable) {
     // ...
   }
 

diff --git a/package.json b/package.json
@@ -4,7 +4,8 @@
     "include": "dependencies/node-version"
   },
   "scripts": {
-    "pkglint": "tools/pkglint/bin/pkglint -f ."
+    "pkglint": "tools/pkglint/bin/pkglint -f .",
+    "build-all": "tsc -b"
   },
   "devDependencies": {
     "@types/node": "8.10.40",

diff --git a/packages/@aws-cdk/assets-docker/test/test.image-asset.ts b/packages/@aws-cdk/assets-docker/test/test.image-asset.ts
@@ -61,22 +61,13 @@ export = {
                   ":",
                   { "Ref": "AWS::AccountId" },
                   ":repository/",
-                  {
-                      "Fn::GetAtt": [
-                        "ImageAdoptRepositoryE1E84E35",
-                        "RepositoryName"
-                      ]
-                  }
+                  { "Fn::GetAtt": [ "ImageAdoptRepositoryE1E84E35", "RepositoryName" ] }
                 ]
               ]
             }
           },
           {
-            "Action": [
-              "ecr:GetAuthorizationToken",
-              "logs:CreateLogStream",
-              "logs:PutLogEvents"
-            ],
+            "Action": "ecr:GetAuthorizationToken",
             "Effect": "Allow",
             "Resource": "*"
           }

diff --git a/packages/@aws-cdk/assets/lib/asset.ts b/packages/@aws-cdk/assets/lib/asset.ts
@@ -36,7 +36,7 @@ export interface GenericAssetProps {
    * A list of principals that should be able to read this asset from S3.
    * You can use `asset.grantRead(principal)` to grant read permissions later.
    */
-  readonly readers?: iam.IPrincipal[];
+  readonly readers?: iam.IGrantable[];
 }
 
 /**
@@ -171,12 +171,12 @@ export class Asset extends cdk.Construct {
   /**
    * Grants read permissions to the principal on the asset's S3 object.
    */
-  public grantRead(principal?: iam.IPrincipal) {
+  public grantRead(grantee: iam.IGrantable) {
     // We give permissions on all files with the same prefix. Presumably
     // different versions of the same file will have the same prefix
     // and we don't want to accidentally revoke permission on old versions
     // when deploying a new version.
-    this.bucket.grantRead(principal, `${this.s3Prefix}*`);
+    this.bucket.grantRead(grantee, `${this.s3Prefix}*`);
   }
 }
 
@@ -190,7 +190,7 @@ export interface FileAssetProps {
    * A list of principals that should be able to read this file asset from S3.
    * You can use `asset.grantRead(principal)` to grant read permissions later.
    */
-  readonly readers?: iam.IPrincipal[];
+  readonly readers?: iam.IGrantable[];
 }
 
 /**
@@ -212,7 +212,7 @@ export interface ZipDirectoryAssetProps {
    * A list of principals that should be able to read this ZIP file from S3.
    * You can use `asset.grantRead(principal)` to grant read permissions later.
    */
-  readonly readers?: iam.IPrincipal[];
+  readonly readers?: iam.IGrantable[];
 }
 
 /**

diff --git a/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts b/packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
@@ -85,14 +85,14 @@ export class Metric {
   /**
    * Grant permissions to the given identity to write metrics.
    *
-   * @param identity The IAM identity to give permissions to.
+   * @param grantee The IAM identity to give permissions to.
    */
-  public static grantPutMetricData(identity?: iam.IPrincipal) {
-    if (!identity) { return; }
-
-    identity.addToPolicy(new iam.PolicyStatement()
-      .addAllResources()
-      .addAction("cloudwatch:PutMetricData"));
+  public static grantPutMetricData(grantee: iam.IGrantable): iam.Grant {
+    return iam.Grant.addToPrincipal({
+      grantee,
+      actions: ['cloudwatch:PutMetricData'],
+      resourceArns: ['*']
+    });
   }
 
   public readonly dimensions?: DimensionHash;

diff --git a/packages/@aws-cdk/aws-codebuild/lib/artifacts.ts b/packages/@aws-cdk/aws-codebuild/lib/artifacts.ts
@@ -130,7 +130,7 @@ export class S3BucketBuildArtifacts extends BuildArtifacts {
    * @internal
    */
   public _bind(project: Project) {
-    this.props.bucket.grantReadWrite(project.role);
+    this.props.bucket.grantReadWrite(project);
   }
 
   protected toArtifactsProperty(): any {

diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts
@@ -16,7 +16,7 @@ const CODEPIPELINE_TYPE = 'CODEPIPELINE';
 const S3_BUCKET_ENV = 'SCRIPT_S3_BUCKET';
 const S3_KEY_ENV = 'SCRIPT_S3_KEY';
 
-export interface IProject extends cdk.IConstruct, events.IEventRuleTarget {
+export interface IProject extends cdk.IConstruct, events.IEventRuleTarget, iam.IGrantable {
   /** The ARN of this Project. */
   readonly projectArn: string;
 
@@ -156,13 +156,15 @@ export interface ProjectImportProps {
  * use the {@link import} method.
  */
 export abstract class ProjectBase extends cdk.Construct implements IProject {
+  public abstract readonly grantPrincipal: iam.IPrincipal;
+
   /** The ARN of this Project. */
   public abstract readonly projectArn: string;
 
   /** The human-visible name of this Project. */
   public abstract readonly projectName: string;
 
-  /** The IAM service Role of this Project. Undefined for imported Projects. */
+  /** The IAM service Role of this Project. */
   public abstract readonly role?: iam.IRole;
 
   /** A role used by CloudWatch events to trigger a build */
@@ -369,6 +371,7 @@ export abstract class ProjectBase extends cdk.Construct implements IProject {
 }
 
 class ImportedProject extends ProjectBase {
+  public readonly grantPrincipal: iam.IPrincipal;
   public readonly projectArn: string;
   public readonly projectName: string;
   public readonly role?: iam.Role = undefined;
@@ -381,6 +384,7 @@ class ImportedProject extends ProjectBase {
       resource: 'project',
       resourceName: props.projectName,
     });
+    this.grantPrincipal = new iam.ImportedResourcePrincipal({ resource: this });
 
     this.projectName = props.projectName;
   }
@@ -572,6 +576,8 @@ export class Project extends ProjectBase {
     return new ImportedProject(scope, id, props);
   }
 
+  public readonly grantPrincipal: iam.IPrincipal;
+
   /**
    * The IAM role for this project.
    */
@@ -603,6 +609,7 @@ export class Project extends ProjectBase {
     this.role = props.role || new iam.Role(this, 'Role', {
       assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com')
     });
+    this.grantPrincipal = this.role;
 
     let cache: CfnProject.ProjectCacheProperty | undefined;
     if (props.cacheBucket) {

diff --git a/packages/@aws-cdk/aws-codebuild/lib/source.ts b/packages/@aws-cdk/aws-codebuild/lib/source.ts
@@ -167,7 +167,7 @@ export class S3BucketSource extends BuildSource {
    * @internal
    */
   public _bind(project: Project) {
-    this.bucket.grantRead(project.role);
+    this.bucket.grantRead(project);
   }
 
   protected toSourceProperty(): any {

diff --git a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts
@@ -194,7 +194,7 @@ export class LambdaDeploymentGroup extends cdk.Construct implements ILambdaDeplo
       throw new Error('A pre-hook function is already defined for this deployment group');
     }
     this.preHook = preHook;
-    this.grantPutLifecycleEventHookExecutionStatus(this.preHook.role);
+    this.grantPutLifecycleEventHookExecutionStatus(this.preHook);
     this.preHook.grantInvoke(this.role);
   }
 
@@ -208,7 +208,7 @@ export class LambdaDeploymentGroup extends cdk.Construct implements ILambdaDeplo
       throw new Error('A post-hook function is already defined for this deployment group');
     }
     this.postHook = postHook;
-    this.grantPutLifecycleEventHookExecutionStatus(this.postHook.role);
+    this.grantPutLifecycleEventHookExecutionStatus(this.postHook);
     this.postHook.grantInvoke(this.role);
   }
 
@@ -217,12 +217,12 @@ export class LambdaDeploymentGroup extends cdk.Construct implements ILambdaDeplo
    * on this deployment group resource.
    * @param principal to grant permission to
    */
-  public grantPutLifecycleEventHookExecutionStatus(principal?: iam.IPrincipal): void {
-    if (principal) {
-      principal.addToPolicy(new iam.PolicyStatement()
-        .addResource(this.deploymentGroupArn)
-        .addAction('codedeploy:PutLifecycleEventHookExecutionStatus'));
-    }
+  public grantPutLifecycleEventHookExecutionStatus(grantee: iam.IGrantable): iam.Grant {
+    return iam.Grant.addToPrincipal({
+      grantee,
+      resourceArns: [this.deploymentGroupArn],
+      actions: ['codedeploy:PutLifecycleEventHookExecutionStatus'],
+    });
   }
 
   public export(): LambdaDeploymentGroupImportProps {

diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/pipeline-actions.ts
@@ -186,9 +186,9 @@ function setCodeBuildNeededPermissions(info: codepipeline.ActionBind, project: c
 
   // allow the Project access to the Pipline's artifact Bucket
   if (needsPipelineBucketWrite) {
-    info.pipeline.grantBucketReadWrite(project.role);
+    info.pipeline.grantBucketReadWrite(project);
   } else {
-    info.pipeline.grantBucketRead(project.role);
+    info.pipeline.grantBucketRead(project);
   }
 }
 

diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/test.pipeline-actions.ts
@@ -317,11 +317,11 @@ class PipelineDouble extends cdk.Construct implements codepipeline.IPipeline {
     throw new Error('asEventRuleTarget() is unsupported in PipelineDouble');
   }
 
-  public grantBucketRead(_identity?: iam.IPrincipal): void {
+  public grantBucketRead(_identity?: iam.IGrantable): iam.Grant {
     throw new Error('grantBucketRead() is unsupported in PipelineDouble');
   }
 
-  public grantBucketReadWrite(_identity?: iam.IPrincipal): void {
+  public grantBucketReadWrite(_identity?: iam.IGrantable): iam.Grant {
     throw new Error('grantBucketReadWrite() is unsupported in PipelineDouble');
   }
 }
@@ -369,9 +369,10 @@ class RoleDouble extends iam.Role {
     super(scope, id, props);
   }
 
-  public addToPolicy(statement: iam.PolicyStatement) {
+  public addToPolicy(statement: iam.PolicyStatement): boolean {
     super.addToPolicy(statement);
     this.statements.push(statement);
+    return true;
   }
 }
 

diff --git a/packages/@aws-cdk/aws-codepipeline/lib/action.ts b/packages/@aws-cdk/aws-codepipeline/lib/action.ts
@@ -83,14 +83,14 @@ export interface IPipeline extends cdk.IConstruct, events.IEventRuleTarget {
    *
    * @param identity the IAM Identity to grant the permissions to
    */
-  grantBucketRead(identity?: iam.IPrincipal): void;
+  grantBucketRead(identity: iam.IGrantable): iam.Grant;
 
   /**
    * Grants read & write permissions to the Pipeline's S3 Bucket to the given Identity.
    *
    * @param identity the IAM Identity to grant the permissions to
    */
-  grantBucketReadWrite(identity?: iam.IPrincipal): void;
+  grantBucketReadWrite(identity: iam.IGrantable): iam.Grant;
 }
 
 /**

diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
@@ -293,12 +293,12 @@ export class Pipeline extends cdk.Construct implements IPipeline {
     return this.stages.length;
   }
 
-  public grantBucketRead(identity?: iam.IPrincipal): void {
-    this.artifactBucket.grantRead(identity);
+  public grantBucketRead(identity: iam.IGrantable): iam.Grant {
+    return this.artifactBucket.grantRead(identity);
   }
 
-  public grantBucketReadWrite(identity?: iam.IPrincipal): void {
-    this.artifactBucket.grantReadWrite(identity);
+  public grantBucketReadWrite(identity: iam.IGrantable): iam.Grant {
+    return this.artifactBucket.grantReadWrite(identity);
   }
 
   /**