From d7b06d05a2fb730fcf991318147149fd850c987b Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Tue, 9 Feb 2021 16:16:31 -0800 Subject: [PATCH] docs(s3): explicitly call out the Bucket.grantWrite() change in the docs (#12737) Mention the `@aws-cdk/aws-s3:grantWriteWithoutAcl` feature flag in the JSDocs of `grantWrite()` / `grantReadWrite()` in `IBucket`, and also in the Changelog for version `1.85.0`, when this was introduced. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- CHANGELOG.md | 6 ++++ packages/@aws-cdk/aws-s3/lib/bucket.ts | 35 +++++++++++------------- packages/@aws-cdk/cx-api/lib/features.ts | 2 +- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bf7a31e44214..e62bd18aecd9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -121,6 +121,12 @@ used by CDK Pipelines) must upgrade their bootstrap stacks. Run `cdk bootstrap`. ## [1.85.0](https://github.com/aws/aws-cdk/compare/v1.84.0...v1.85.0) (2021-01-14) * **s3-deployment**: This version includes an important update, please upgrade to prevent deployment failure. This is in prepartion of Lambda deprecation of the request module in boto, more details are available in [AWS blog](https://aws.amazon.com/blogs/compute/upcoming-changes-to-the-python-sdk-in-aws-lambda/). Note, users of versions < `1.81.0` will not be impacted by this deprecation, but are still encouraged to upgrade to the latest version. +* **s3**: The `grantWrite()` and `grantReadWrite()` methods no longer add the `s3:PutObject*` permissions that included `s3:PutObjectAcl`, + which could be used to grant read/write object access to IAM principals in other accounts. + This change is gated behind the `@aws-cdk/aws-s3:grantWriteWithoutAcl` feature flag, + so make sure to set it to `true` in the `context` key of your `cdk.json` file when upgrading. + If you still need the principal to have `s3:PutObjectAcl` permissions after upgrading, + use the new `grantPutAcl()` method. ### Features diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index 525b22f6afd1d..859f774565a93 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -147,6 +147,14 @@ export interface IBucket extends IResource { * If encryption is used, permission to use the key to encrypt the contents * of written files will also be granted to the same principal. * + * Before CDK version 1.85.0, this method granted the `s3:PutObject*` permission that included `s3:PutObjectAcl`, + * which could be used to grant read/write object access to IAM principals in other accounts. + * If you want to get rid of that behavior, update your CDK version to 1.85.0 or later, + * and make sure the `@aws-cdk/aws-s3:grantWriteWithoutAcl` feature flag is set to `true` + * in the `context` key of your cdk.json file. + * If you've already updated, but still need the principal to have permissions to modify the ACLs, + * use the {@link grantPutAcl} method. + * * @param identity The principal * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*') */ @@ -190,6 +198,14 @@ export interface IBucket extends IResource { * If an encryption key is used, permission to use the key for * encrypt/decrypt will also be granted. * + * Before CDK version 1.85.0, this method granted the `s3:PutObject*` permission that included `s3:PutObjectAcl`, + * which could be used to grant read/write object access to IAM principals in other accounts. + * If you want to get rid of that behavior, update your CDK version to 1.85.0 or later, + * and make sure the `@aws-cdk/aws-s3:grantWriteWithoutAcl` feature flag is set to `true` + * in the `context` key of your cdk.json file. + * If you've already updated, but still need the principal to have permissions to modify the ACLs, + * use the {@link grantPutAcl} method. + * * @param identity The principal * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*') */ @@ -587,15 +603,6 @@ abstract class BucketBase extends Resource implements IBucket { this.arnForObjects(objectsKeyPattern)); } - /** - * Grant write permissions to this bucket to an IAM principal. - * - * If encryption is used, permission to use the key to encrypt the contents - * of written files will also be granted to the same principal. - * - * @param identity The principal - * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*') - */ public grantWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*') { return this.grant(identity, this.writeActions, perms.KEY_WRITE_ACTIONS, this.bucketArn, @@ -632,16 +639,6 @@ abstract class BucketBase extends Resource implements IBucket { this.arnForObjects(objectsKeyPattern)); } - /** - * Grants read/write permissions for this bucket and it's contents to an IAM - * principal (Role/Group/User). - * - * If an encryption key is used, permission to use the key for - * encrypt/decrypt will also be granted. - * - * @param identity The principal - * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*') - */ public grantReadWrite(identity: iam.IGrantable, objectsKeyPattern: any = '*') { const bucketActions = perms.BUCKET_READ_ACTIONS.concat(this.writeActions); // we need unique permissions because some permissions are common between read and write key actions diff --git a/packages/@aws-cdk/cx-api/lib/features.ts b/packages/@aws-cdk/cx-api/lib/features.ts index a9773d86ead60..57c9a5738ee96 100644 --- a/packages/@aws-cdk/cx-api/lib/features.ts +++ b/packages/@aws-cdk/cx-api/lib/features.ts @@ -83,7 +83,7 @@ export const KMS_DEFAULT_KEY_POLICIES = '@aws-cdk/aws-kms:defaultKeyPolicies'; /** * Change the old 's3:PutObject*' permission to 's3:PutObject' on Bucket, * as the former includes 's3:PutObjectAcl', - * which allows changing the visibility of an object written to the Bucket. + * which could be used to grant read/write object access to IAM principals in other accounts. * Use a feature flag to make sure existing customers who might be relying * on the overly-broad permissions are not broken. */