Skip to content

Commit

Permalink
fix(secretsmanager): fromSecretPartialArn() has incorrect grant polic…
Browse files Browse the repository at this point in the history
…ies (#12665)

The grants for secrets imported by partial ARN were missing the '-??????' suffix
to account for the SecretsManager-supplied suffix. A refactor unifies this logic
better into one place.

fixes #12411


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
njlynch authored Jan 25, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 003b6d4 commit 560915e
Showing 2 changed files with 75 additions and 8 deletions.
15 changes: 7 additions & 8 deletions packages/@aws-cdk/aws-secretsmanager/lib/secret.ts
Original file line number Diff line number Diff line change
@@ -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
@@ -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({
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
@@ -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';
@@ -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

0 comments on commit 560915e

Please sign in to comment.