From d36f2a8a486277b69217c2aad5ee331beb0b9a92 Mon Sep 17 00:00:00 2001 From: Glib Shpychka <23005347+gshpychka@users.noreply.github.com> Date: Mon, 10 Jul 2023 19:33:27 +0300 Subject: [PATCH 1/4] add unit test --- .../aws-secretsmanager/test/secret.test.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/test/secret.test.ts b/packages/aws-cdk-lib/aws-secretsmanager/test/secret.test.ts index 18849d76aff72..628649d7ee4be 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/test/secret.test.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/test/secret.test.ts @@ -1357,3 +1357,36 @@ describe('secretObjectValue', () => { }); }); }); + +test('cross-environment grant with direct object reference', () => { + // GIVEN + const producerStack = new cdk.Stack(app, 'ProducerStack', { env: { region: 'foo', account: '1111111111' } }); + const consumerStack = new cdk.Stack(app, 'ConsumerStack', { env: { region: 'bar', account: '1111111111' } }); + const secret = new secretsmanager.Secret(producerStack, 'Secret', { secretName: 'MySecret' }); + const role = new iam.Role(consumerStack, 'Role', { assumedBy: new iam.AccountRootPrincipal() }); + + // WHEN + secret.grantRead(role); + + // THEN + Template.fromStack(consumerStack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: { + Version: '2012-10-17', + Statement: [{ + Action: [ + 'secretsmanager:GetSecretValue', + 'secretsmanager:DescribeSecret', + ], + Effect: 'Allow', + Resource: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':secretsmanager:foo:1111111111:secret:MySecret-??????', + ]], + }, + }], + }, + }); + +}); From dd61f42c1487bddfb219fe4ea4862174d6827152 Mon Sep 17 00:00:00 2001 From: Glib Shpychka <23005347+gshpychka@users.noreply.github.com> Date: Mon, 10 Jul 2023 19:34:15 +0300 Subject: [PATCH 2/4] add suffix to ARN if cross-env --- .../aws-cdk-lib/aws-secretsmanager/lib/secret.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts index 9a5ab700556ec..98fc9a6f13eb3 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts @@ -4,7 +4,7 @@ import { RotationSchedule, RotationScheduleOptions } from './rotation-schedule'; import * as secretsmanager from './secretsmanager.generated'; import * as iam from '../../aws-iam'; import * as kms from '../../aws-kms'; -import { ArnFormat, FeatureFlags, Fn, IResource, Lazy, RemovalPolicy, Resource, ResourceProps, SecretValue, Stack, Token, TokenComparison } from '../../core'; +import { ArnFormat, FeatureFlags, Fn, IResolveContext, IResource, Lazy, RemovalPolicy, Resource, ResourceProps, SecretValue, Stack, Token, TokenComparison } from '../../core'; import * as cxapi from '../../cx-api'; const SECRET_SYMBOL = Symbol.for('@aws-cdk/secretsmanager.Secret'); @@ -450,7 +450,18 @@ abstract class SecretBase extends Resource implements ISecret { * then we need to add a suffix to capture the full ARN's format. */ protected get arnForPolicies() { - return this.secretFullArn ? this.secretFullArn : `${this.secretArn}-??????`; + return Lazy.uncachedString({ + produce: (context: IResolveContext) => { + const consumingStack = Stack.of(context.scope); + if (this.stack.account !== consumingStack.account || + (this.stack.region !== consumingStack.region && + !consumingStack._crossRegionReferences) || !this.secretFullArn) { + return `${this.secretArn}-??????`; + } else { + return this.secretArn; + } + }, + }); } /** From 4f23619ac773e002c48a6bcf51bfb5d890bd2afc Mon Sep 17 00:00:00 2001 From: Glib Shpychka <23005347+gshpychka@users.noreply.github.com> Date: Tue, 11 Jul 2023 13:43:32 +0300 Subject: [PATCH 3/4] return secretFullArn explicitly --- packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts index 98fc9a6f13eb3..2046518e53bfa 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts @@ -458,7 +458,7 @@ abstract class SecretBase extends Resource implements ISecret { !consumingStack._crossRegionReferences) || !this.secretFullArn) { return `${this.secretArn}-??????`; } else { - return this.secretArn; + return this.secretFullArn; } }, }); From 918748a673d05a052035e4b4f587b14bf831ff42 Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Fri, 21 Jul 2023 17:30:52 +0100 Subject: [PATCH 4/4] use the same lazy everywhere --- .../aws-iam/test/merge-statements.test.ts | 36 ++++++++++++++++++- .../aws-secretsmanager/lib/secret.ts | 26 +++++++------- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/packages/aws-cdk-lib/aws-iam/test/merge-statements.test.ts b/packages/aws-cdk-lib/aws-iam/test/merge-statements.test.ts index 12796f768b741..45984b95ac5b3 100644 --- a/packages/aws-cdk-lib/aws-iam/test/merge-statements.test.ts +++ b/packages/aws-cdk-lib/aws-iam/test/merge-statements.test.ts @@ -1,4 +1,4 @@ -import { App, Stack } from '../../core'; +import { App, Lazy, Stack } from '../../core'; import * as iam from '../lib'; import { PolicyStatement } from '../lib'; @@ -490,6 +490,40 @@ test('lazily generated statements are merged correctly', () => { ]); }); +test('merge statements if resource is a lazy', () => { + const stack = new Stack(); + const user1 = new iam.User(stack, 'User1'); + const user2 = new iam.User(stack, 'User2'); + const resourceToken = Lazy.string({ + produce: () => 'a', + }); + + assertMerged([ + new iam.PolicyStatement({ + resources: [resourceToken], + actions: ['service:Action'], + principals: [user1], + }), + new iam.PolicyStatement({ + resources: [resourceToken], + actions: ['service:Action'], + principals: [user2], + }), + ], [ + { + Effect: 'Allow', + Resource: 'a', + Action: 'service:Action', + Principal: { + AWS: [ + { 'Fn::GetAtt': ['User1E278A736', 'Arn'] }, + { 'Fn::GetAtt': ['User21F1486D1', 'Arn'] }, + ], + }, + }, + ]); +}); + function assertNoMerge(statements: iam.PolicyStatement[]) { const app = new App(); const stack = new Stack(app, 'Stack'); diff --git a/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts b/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts index 2046518e53bfa..6f214e3448cc7 100644 --- a/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts +++ b/packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts @@ -340,9 +340,22 @@ abstract class SecretBase extends Resource implements ISecret { protected abstract readonly autoCreatePolicy: boolean; private policy?: ResourcePolicy; + private _arnForPolicies: string; constructor(scope: Construct, id: string, props: ResourceProps = {}) { super(scope, id, props); + this._arnForPolicies = Lazy.uncachedString({ + produce: (context: IResolveContext) => { + const consumingStack = Stack.of(context.scope); + if (this.stack.account !== consumingStack.account || + (this.stack.region !== consumingStack.region && + !consumingStack._crossRegionReferences) || !this.secretFullArn) { + return `${this.secretArn}-??????`; + } else { + return this.secretFullArn; + } + }, + }); this.node.addValidation({ validate: () => this.policy?.document.validateForResourcePolicy() ?? [] }); } @@ -450,18 +463,7 @@ abstract class SecretBase extends Resource implements ISecret { * then we need to add a suffix to capture the full ARN's format. */ protected get arnForPolicies() { - return Lazy.uncachedString({ - produce: (context: IResolveContext) => { - const consumingStack = Stack.of(context.scope); - if (this.stack.account !== consumingStack.account || - (this.stack.region !== consumingStack.region && - !consumingStack._crossRegionReferences) || !this.secretFullArn) { - return `${this.secretArn}-??????`; - } else { - return this.secretFullArn; - } - }, - }); + return this._arnForPolicies; } /**