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(events-targets): policy restricts access to the same account as the Queue, not the Rule #22766

Merged
merged 2 commits into from
Nov 4, 2022
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
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-events-targets/lib/sqs.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as events from '@aws-cdk/aws-events';
import * as iam from '@aws-cdk/aws-iam';
import * as sqs from '@aws-cdk/aws-sqs';
import { Aws, FeatureFlags } from '@aws-cdk/core';
import { FeatureFlags } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { addToDeadLetterQueueResourcePolicy, TargetBaseProps, bindBaseTargetConfig } from './util';

Expand Down Expand Up @@ -62,9 +62,9 @@ export class SqsQueue implements events.IRuleTarget {
ArnEquals: { 'aws:SourceArn': rule.ruleArn },
};
} else if (restrictToSameAccount) {
// Aadd only the account id as a condition, to avoid circular dependency. See issue #11158.
// Add only the account id as a condition, to avoid circular dependency. See issue #11158.
conditions = {
StringEquals: { 'aws:SourceAccount': Aws.ACCOUNT_ID },
StringEquals: { 'aws:SourceAccount': rule.env.account },
};
}

Expand Down
52 changes: 25 additions & 27 deletions packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Template } from '@aws-cdk/assertions';
import { Match, Template } from '@aws-cdk/assertions';
import * as events from '@aws-cdk/aws-events';
import * as kms from '@aws-cdk/aws-kms';
import * as sqs from '@aws-cdk/aws-sqs';
import { Duration, Stack } from '@aws-cdk/core';
import { App, Duration, Stack } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import * as targets from '../../lib';

Expand Down Expand Up @@ -144,24 +144,38 @@ test('multiple uses of a queue as a target results in multi policy statement bec
});

test('Encrypted queues result in a policy statement with aws:sourceAccount condition when the feature flag is on', () => {
const app = new App();
// GIVEN
const stack = new Stack();
stack.node.setContext(cxapi.EVENTS_TARGET_QUEUE_SAME_ACCOUNT, true);
const queue = new sqs.Queue(stack, 'MyQueue', {
encryptionMasterKey: kms.Key.fromKeyArn(stack, 'key', 'arn:aws:kms:us-west-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab'),
const ruleStack = new Stack(app, 'ruleStack', {
env: {
account: '111111111111',
region: 'us-east-1',
},
});
ruleStack.node.setContext(cxapi.EVENTS_TARGET_QUEUE_SAME_ACCOUNT, true);

const rule = new events.Rule(stack, 'MyRule', {
const rule = new events.Rule(ruleStack, 'MyRule', {
schedule: events.Schedule.rate(Duration.hours(1)),
});

const queueStack = new Stack(app, 'queueStack', {
env: {
account: '222222222222',
region: 'us-east-1',
},
});
const queue = new sqs.Queue(queueStack, 'MyQueue', {
encryptionMasterKey: kms.Key.fromKeyArn(queueStack, 'key', 'arn:aws:kms:us-west-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab'),
});


// WHEN
rule.addTarget(new targets.SqsQueue(queue));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::SQS::QueuePolicy', {
Template.fromStack(queueStack).hasResourceProperties('AWS::SQS::QueuePolicy', {
PolicyDocument: {
Statement: [
Statement: Match.arrayWith([
{
Action: [
'sqs:SendMessage',
Expand All @@ -170,7 +184,7 @@ test('Encrypted queues result in a policy statement with aws:sourceAccount condi
],
Condition: {
StringEquals: {
'aws:SourceAccount': { Ref: 'AWS::AccountId' },
'aws:SourceAccount': '111111111111',
},
},
Effect: 'Allow',
Expand All @@ -182,27 +196,11 @@ test('Encrypted queues result in a policy statement with aws:sourceAccount condi
],
},
},
],
]),
Version: '2012-10-17',
},
Queues: [{ Ref: 'MyQueueE6CA6235' }],
});

Template.fromStack(stack).hasResourceProperties('AWS::Events::Rule', {
ScheduleExpression: 'rate(1 hour)',
State: 'ENABLED',
Targets: [
{
Arn: {
'Fn::GetAtt': [
'MyQueueE6CA6235',
'Arn',
],
},
Id: 'Target0',
},
],
});
});

test('Encrypted queues result in a permissive policy statement when the feature flag is off', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ export const ENABLE_PARTITION_LITERALS = '@aws-cdk/core:enablePartitionLiterals'

/**
* This flag applies to SQS Queues that are used as the target of event Rules. When enabled, only principals
* from the same account as the Queue can send messages. If a queue is unencrypted, this restriction will
* from the same account as the Rule can send messages. If a queue is unencrypted, this restriction will
* always apply, regardless of the value of this flag.
*/
export const EVENTS_TARGET_QUEUE_SAME_ACCOUNT = '@aws-cdk/aws-events:eventsTargetQueueSameAccount';
Expand Down