Skip to content

Commit

Permalink
fix(s3): Make IBucket.arnForObject accept only (exactly) one key pattern
Browse files Browse the repository at this point in the history
The variadic signature allowed the method to be invoked with zero
arguments, resulting in an unusable ARN. Instead of accepting an array
of path elements that will be concatenated, accept only one pattern and
use it as-is.

BREAKING CHANGE: The `IBucket.arnForObject` method no longer
concatenates path fragments on your behalf. Pass the `/`-concatenated
key pattern instead.
  • Loading branch information
RomainMuller committed May 15, 2019
1 parent 7020846 commit 910462b
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 10 deletions.
11 changes: 3 additions & 8 deletions packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,8 @@ export interface IBucket extends IResource {
/**
* Returns an ARN that represents all objects within the bucket that match
* the key pattern specified. To represent all keys, specify ``"*"``.
*
* If you specify multiple components for keyPattern, they will be concatenated::
*
* arnForObjects('home/', team, '/', user, '/*')
*
*/
arnForObjects(...keyPattern: string[]): string;
arnForObjects(keyPattern: string): string;

/**
* Grant read permissions for this bucket and it's contents to an IAM
Expand Down Expand Up @@ -369,8 +364,8 @@ abstract class BucketBase extends Resource implements IBucket {
* arnForObjects('home/', team, '/', user, '/*')
*
*/
public arnForObjects(...keyPattern: string[]): string {
return `${this.bucketArn}/${keyPattern.join('')}`;
public arnForObjects(keyPattern: string): string {
return `${this.bucketArn}/${keyPattern}`;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-s3/test/test.bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ export = {
const user = new iam.User(stack, 'MyUser');
const team = new iam.Group(stack, 'MyTeam');

const resource = bucket.arnForObjects('home/', team.groupName, '/', user.userName, '/*');
const resource = bucket.arnForObjects(`home/${team.groupName}/${user.userName}/*`);
const p = new iam.PolicyStatement().addResource(resource).addAction('s3:GetObject');

test.deepEqual(bucket.node.resolve(p), {
Expand Down Expand Up @@ -599,7 +599,7 @@ export = {
// you can even use the bucket name, which will be extracted from the arn provided.
const user = new iam.User(stack, 'MyUser');
user.addToPolicy(new iam.PolicyStatement()
.addResource(bucket.arnForObjects('my/folder/', bucket.bucketName))
.addResource(bucket.arnForObjects(`my/folder/${bucket.bucketName}`))
.addAction('s3:*'));

expect(stack).toMatch({
Expand Down

0 comments on commit 910462b

Please sign in to comment.