Skip to content

Commit

Permalink
feat(aws-iam): refactor grants, add OrganizationPrincipal (#1623)
Browse files Browse the repository at this point in the history
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)`.
  • Loading branch information
rix0rrr authored Apr 3, 2019
1 parent ccbb6dd commit 1bb8ca9
Show file tree
Hide file tree
Showing 65 changed files with 3,349 additions and 544 deletions.
8 changes: 4 additions & 4 deletions design/aws-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
// ...
}

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 2 additions & 11 deletions packages/@aws-cdk/assets-docker/test/test.image-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": "*"
}
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}

/**
Expand Down Expand Up @@ -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}*`);
}
}

Expand All @@ -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[];
}

/**
Expand All @@ -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[];
}

/**
Expand Down
14 changes: 7 additions & 7 deletions packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codebuild/lib/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 9 additions & 2 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
Expand All @@ -381,6 +384,7 @@ class ImportedProject extends ProjectBase {
resource: 'project',
resourceName: props.projectName,
});
this.grantPrincipal = new iam.ImportedResourcePrincipal({ resource: this });

this.projectName = props.projectName;
}
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codebuild/lib/source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 8 additions & 8 deletions packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Expand Down Expand Up @@ -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;
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codepipeline/lib/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Loading

0 comments on commit 1bb8ca9

Please sign in to comment.