Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(secretsmanager): fromSecretPartialArn() has incorrect grant policies #12665

Merged
merged 2 commits into from
Jan 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions packages/@aws-cdk/aws-secretsmanager/lib/secret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,14 @@ abstract class SecretBase extends Resource implements ISecret {
}

/**
* Provides an identifier for this secret for use in IAM policies. Typically, this is just the secret ARN.
* However, secrets imported by name require a different format.
* Provides an identifier for this secret for use in IAM policies.
* If there is a full ARN, this is just the ARN;
* if we have a partial ARN -- due to either importing by secret name or partial ARN --
* then we need to add a suffix to capture the full ARN's format.
*/
protected get arnForPolicies() { return this.secretArn; }
protected get arnForPolicies() {
return this.secretFullArn ? this.secretFullArn : `${this.secretArn}-??????`;
}

/**
* Attach a target to this secret
Expand Down Expand Up @@ -351,11 +355,6 @@ export class Secret extends SecretBase {
public readonly secretArn = this.partialArn;
protected readonly autoCreatePolicy = false;
public get secretFullArn() { return undefined; }
// Overrides the secretArn for grant* methods, where the secretArn must be in ARN format.
// Also adds a wildcard to the resource name to support the SecretsManager-provided suffix.
protected get arnForPolicies(): string {
return this.partialArn + '-??????';
}
// Creates a "partial" ARN from the secret name. The "full" ARN would include the SecretsManager-provided suffix.
private get partialArn(): string {
return Stack.of(this).formatArn({
Expand Down
68 changes: 68 additions & 0 deletions packages/@aws-cdk/aws-secretsmanager/test/secret.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,40 @@ test('fromSecretCompleteArn', () => {
expect(stack.resolve(secret.secretValueFromJson('password'))).toEqual(`{{resolve:secretsmanager:${secretArn}:SecretString:password::}}`);
});

test('fromSecretCompleteArn - grants', () => {
// GIVEN
const secretArn = 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:MySecret-f3gDy9';
const secret = secretsmanager.Secret.fromSecretCompleteArn(stack, 'Secret', secretArn);
const role = new iam.Role(stack, 'Role', { assumedBy: new iam.AccountRootPrincipal() });

// WHEN
secret.grantRead(role);
secret.grantWrite(role);

// THEN
expect(stack).toHaveResource('AWS::IAM::Policy', {
PolicyDocument: {
Version: '2012-10-17',
Statement: [{
Action: [
'secretsmanager:GetSecretValue',
'secretsmanager:DescribeSecret',
],
Effect: 'Allow',
Resource: secretArn,
},
{
Action: [
'secretsmanager:PutSecretValue',
'secretsmanager:UpdateSecret',
],
Effect: 'Allow',
Resource: secretArn,
}],
},
});
});

test('fromSecretPartialArn', () => {
// GIVEN
const secretArn = 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:MySecret';
Expand All @@ -669,6 +703,40 @@ test('fromSecretPartialArn', () => {
expect(stack.resolve(secret.secretValueFromJson('password'))).toEqual(`{{resolve:secretsmanager:${secretArn}:SecretString:password::}}`);
});

test('fromSecretPartialArn - grants', () => {
// GIVEN
const secretArn = 'arn:aws:secretsmanager:eu-west-1:111111111111:secret:MySecret';
const secret = secretsmanager.Secret.fromSecretPartialArn(stack, 'Secret', secretArn);
const role = new iam.Role(stack, 'Role', { assumedBy: new iam.AccountRootPrincipal() });

// WHEN
secret.grantRead(role);
secret.grantWrite(role);

// THEN
expect(stack).toHaveResource('AWS::IAM::Policy', {
PolicyDocument: {
Version: '2012-10-17',
Statement: [{
Action: [
'secretsmanager:GetSecretValue',
'secretsmanager:DescribeSecret',
],
Effect: 'Allow',
Resource: `${secretArn}-??????`,
},
{
Action: [
'secretsmanager:PutSecretValue',
'secretsmanager:UpdateSecret',
],
Effect: 'Allow',
Resource: `${secretArn}-??????`,
}],
},
});
});

describe('fromSecretAttributes', () => {
test('import by attributes', () => {
// GIVEN
Expand Down