Skip to content

Commit

Permalink
fix(secretsmanager): SecretRotation for secret imported by name has i…
Browse files Browse the repository at this point in the history
…ncorrect permissions (aws#18567)

The SecretRotation class currently always grants permissions to
`secret.secretArn`; the correct value actually should either by the
`secretFullArn` or `secretPartialArn` plus a suffix. This logic is currently
covered by `SecretBase.arnForPolicies`. I opted to copy the logic rather than
expose the member on both `SecretBase` and `ISecret`, but if more of these cases
rise up, that may be the right solution.

fixes aws#18424

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
njlynch authored and LukvonStrom committed Jan 26, 2022
1 parent 4045819 commit 29faa56
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class RotationSchedule extends Resource {
'secretsmanager:PutSecretValue',
'secretsmanager:UpdateSecretVersionStage',
],
resources: [props.secret.secretArn],
resources: [props.secret.secretFullArn ? props.secret.secretFullArn : `${props.secret.secretArn}-??????`],
}),
);
props.rotationLambda.addToRolePolicy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,57 @@ test('assign permissions for rotation schedule with a rotation Lambda', () => {
});
});

test('grants correct permissions for secret imported by name', () => {
// GIVEN
const secret = secretsmanager.Secret.fromSecretNameV2(stack, 'Secret', 'mySecretName');
const rotationLambda = new lambda.Function(stack, 'Lambda', {
runtime: lambda.Runtime.NODEJS_10_X,
code: lambda.Code.fromInline('export.handler = event => event;'),
handler: 'index.handler',
});

// WHEN
new secretsmanager.RotationSchedule(stack, 'RotationSchedule', {
secret,
rotationLambda,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: Match.arrayWith([
{
Action: [
'secretsmanager:DescribeSecret',
'secretsmanager:GetSecretValue',
'secretsmanager:PutSecretValue',
'secretsmanager:UpdateSecretVersionStage',
],
Effect: 'Allow',
Resource: {
'Fn::Join': ['', [
'arn:',
{ Ref: 'AWS::Partition' },
':secretsmanager:',
{ Ref: 'AWS::Region' },
':',
{ Ref: 'AWS::AccountId' },
':secret:mySecretName-??????',
]],
},
},
]),
Version: '2012-10-17',
},
PolicyName: 'LambdaServiceRoleDefaultPolicyDAE46E21',
Roles: [
{
Ref: 'LambdaServiceRoleA8ED4D3B',
},
],
});
});

test('assign kms permissions for rotation schedule with a rotation Lambda', () => {
// GIVEN
const encryptionKey = new kms.Key(stack, 'Key');
Expand Down

0 comments on commit 29faa56

Please sign in to comment.