From 51cd486cfd007d2c69c1dc710d95dfb7b08ffda2 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 31 Oct 2018 15:10:27 +0200 Subject: [PATCH 1/2] feat(aws-sqs): improvements to IAM grants API Moved `grantXxx` methods from `Queue` to `QueueRef`, so they can now be performed on imported queues. Added commonly needed permissions to `grantConsumeMessages` and `grantSendMessages` such as `sqs:GetQueueAttributes`, `sqs:GetQueueUrl` and the various `sqs:xxxBatch` actions. Added support for adding arbitrary actions to each of the grant methods. Exposed `queue.grant(...actions)` as a general purpose grant method which allows users to customize the set of actions for this specific resource/principal pair. BREAKING CHANGE: `queue.grantReceiveMessages` has been removed. It is unlikely that this would be sufficient to interact with a queue. Alternatively you can use `queue.grantConsumeMessages` or `queue.grant('sqs:ReceiveMessage')` if there's a need to only grant this action. --- packages/@aws-cdk/aws-sqs/lib/perms.ts | 12 -- packages/@aws-cdk/aws-sqs/lib/queue-ref.ts | 87 ++++++++++ packages/@aws-cdk/aws-sqs/lib/queue.ts | 60 ------- packages/@aws-cdk/aws-sqs/test/test.sqs.ts | 179 +++++++++++++-------- 4 files changed, 201 insertions(+), 137 deletions(-) delete mode 100644 packages/@aws-cdk/aws-sqs/lib/perms.ts diff --git a/packages/@aws-cdk/aws-sqs/lib/perms.ts b/packages/@aws-cdk/aws-sqs/lib/perms.ts deleted file mode 100644 index a9062475e14cc..0000000000000 --- a/packages/@aws-cdk/aws-sqs/lib/perms.ts +++ /dev/null @@ -1,12 +0,0 @@ -export const QUEUE_GET_ACTIONS = [ - "sqs:ReceiveMessage", -]; - -export const QUEUE_CONSUME_ACTIONS = [ - "sqs:ChangeMessageVisibility", - "sqs:DeleteMessage", -]; - -export const QUEUE_PUT_ACTIONS = [ - "sqs:SendMessage", -]; \ No newline at end of file diff --git a/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts b/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts index 55d591881e936..67a7c1389bc1e 100644 --- a/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts +++ b/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts @@ -110,6 +110,93 @@ export abstract class QueueRef extends cdk.Construct implements s3n.IBucketNotif dependencies: [ this.policy! ] }; } + + /** + * Grant permissions to consume messages from a queue + * + * This will grant the following permissions: + * + * - sqs:ChangeMessageVisibility + * - sqs:ChangeMessageVisibilityBatch + * - sqs:DeleteMessage + * - sqs:ReceiveMessage + * - sqs:DeleteMessageBatch + * - sqs:GetQueueAttributes + * - sqs:GetQueueUrl + * + * @param identity Principal to grant consume rights to + * @param queueActions additional queue actions to allow + */ + public grantConsumeMessages(identity?: iam.IPrincipal, ...queueActions: string[]) { + this.grant(identity, + 'sqs:ReceiveMessage', + 'sqs:ChangeMessageVisibility', + 'sqs:ChangeMessageVisibilityBatch', + 'sqs:GetQueueUrl', + 'sqs:DeleteMessage', + 'sqs:DeleteMessageBatch', + 'sqs:GetQueueAttributes', + ...queueActions); + } + + /** + * Grant access to send messages to a queue to the given identity. + * + * This will grant the following permissions: + * + * - sqs:SendMessage + * - sqs:SendMessageBatch + * - sqs:GetQueueAttributes + * - sqs:GetQueueUrl + * + * @param identity Principal to grant send rights to + * @param queueActions additional queue actions to allow + */ + public grantSendMessages(identity?: iam.IPrincipal, ...queueActions: string[]) { + this.grant(identity, + 'sqs:SendMessage', + 'sqs:SendMessageBatch', + 'sqs:GetQueueAttributes', + 'sqs:GetQueueUrl', + ...queueActions); + } + + /** + * Grant an IAM principal permissions to purge all messages from the queue. + * + * This will grant the following permissions: + * + * - sqs:PurgeQueue + * - sqs:GetQueueAttributes + * - sqs:GetQueueUrl + * + * @param identity Principal to grant send rights to + * @param queueActions additional queue actions to allow + */ + public grantPurge(identity?: iam.IPrincipal, ...queueActions: string[]) { + this.grant(identity, + 'sqs:PurgeQueue', + 'sqs:GetQueueAttributes', + 'sqs:GetQueueUrl', + ...queueActions); + } + + /** + * Grant the actions defined in queueActions to the identity Principal given + * on this SQS queue resource. + * + * @param identity Principal to grant right to + * @param queueActions The actions to grant + */ + public grant(identity?: iam.IPrincipal, ...queueActions: string[]) { + if (!identity) { + return; + } + + identity.addToPolicy(new iam.PolicyStatement() + .addResource(this.queueArn) + .addActions(...queueActions)); + } } /** diff --git a/packages/@aws-cdk/aws-sqs/lib/queue.ts b/packages/@aws-cdk/aws-sqs/lib/queue.ts index 9cea41ee1fc0b..ea6133ecbf754 100644 --- a/packages/@aws-cdk/aws-sqs/lib/queue.ts +++ b/packages/@aws-cdk/aws-sqs/lib/queue.ts @@ -1,7 +1,5 @@ -import iam = require('@aws-cdk/aws-iam'); import kms = require('@aws-cdk/aws-kms'); import cdk = require('@aws-cdk/cdk'); -import perms = require('./perms'); import { QueueRef } from './queue-ref'; import { cloudformation } from './sqs.generated'; import { validateProps } from './validate-props'; @@ -277,64 +275,6 @@ export class Queue extends QueueRef { } } - /** - * Grant permissions to consume messages from a queue - * - * This will grant the following permissions: - * - * - sqs:ChangeMessageVisibility - * - sqs:DeleteMessage - * - sqs:ReceiveMessage - * - * @param identity Principal to grant consume rights to - */ - public grantConsumeMessages(identity?: iam.IPrincipal) { - this.grant(identity, perms.QUEUE_GET_ACTIONS.concat(perms.QUEUE_CONSUME_ACTIONS)); - } - - /** - * Grant access to receive messages from a queue to - * the given identity. - * - * This will grant sqs:ReceiveMessage - * - * @param identity Principal to grant receive rights to - */ - public grantReceiveMessages(identity?: iam.IPrincipal) { - this.grant(identity, perms.QUEUE_GET_ACTIONS); - } - - /** - * Grant access to send messages to a queue to the - * given identity. - * - * This will grant sqs:SendMessage - * - * @param identity Principal to grant send rights to - */ - public grantSendMessages(identity?: iam.IPrincipal) { - this.grant(identity, perms.QUEUE_PUT_ACTIONS); - } - - /** - * Grant the actions defined in queueActions - * to the identity Principal given. - * - * @param identity Principal to grant right to - * @param queueActions The actions to grant - */ - private grant(identity: iam.IPrincipal | undefined, - queueActions: string[]) { - - if (!identity) { - return; - } - - identity.addToPolicy(new iam.PolicyStatement() - .addResource(this.queueArn) - .addActions(...queueActions)); - } - /** * Look at the props, see if the FIFO props agree, and return the correct subset of props */ diff --git a/packages/@aws-cdk/aws-sqs/test/test.sqs.ts b/packages/@aws-cdk/aws-sqs/test/test.sqs.ts index 21f9529d14333..856ac9103762e 100644 --- a/packages/@aws-cdk/aws-sqs/test/test.sqs.ts +++ b/packages/@aws-cdk/aws-sqs/test/test.sqs.ts @@ -1,10 +1,11 @@ import { expect, haveResource } from '@aws-cdk/assert'; -import { ArnPrincipal, PolicyStatement, Role, ServicePrincipal } from '@aws-cdk/aws-iam'; +import iam = require('@aws-cdk/aws-iam'); import kms = require('@aws-cdk/aws-kms'); import s3 = require('@aws-cdk/aws-s3'); import { resolve, Stack } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; import sqs = require('../lib'); +import { Queue } from '../lib'; // tslint:disable:object-literal-key-quotes @@ -56,7 +57,7 @@ export = { 'addToPolicy will automatically create a policy for this queue'(test: Test) { const stack = new Stack(); const queue = new sqs.Queue(stack, 'MyQueue'); - queue.addToResourcePolicy(new PolicyStatement().addAllResources().addActions('sqs:*').addPrincipal(new ArnPrincipal('arn'))); + queue.addToResourcePolicy(new iam.PolicyStatement().addAllResources().addActions('sqs:*').addPrincipal(new iam.ArnPrincipal('arn'))); expect(stack).toMatch({ "Resources": { "MyQueueE6CA6235": { @@ -113,92 +114,114 @@ export = { test.done(); }, - 'iam': { - 'grants permission to consume messages'(test: Test) { - const stack = new Stack(); - const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('lambda.amazonaws.com') }); - const queue = new sqs.Queue(stack, 'Queue'); - queue.grantConsumeMessages(role); + 'grants': { + 'grantConsumeMessages'(test: Test) { + testGrant((q, p) => q.grantConsumeMessages(p), + 'sqs:ReceiveMessage', + 'sqs:ChangeMessageVisibility', + 'sqs:ChangeMessageVisibilityBatch', + 'sqs:GetQueueUrl', + 'sqs:DeleteMessage', + 'sqs:DeleteMessageBatch', + 'sqs:GetQueueAttributes', + ); + test.done(); + }, - expect(stack).to(haveResource('AWS::IAM::Policy', { - "PolicyDocument": { - "Statement": [ - { - "Action": [ - "sqs:ReceiveMessage", - "sqs:ChangeMessageVisibility", - "sqs:DeleteMessage" - ], - "Effect": "Allow", - "Resource": { - "Fn::GetAtt": - [ - "Queue4A7E3555", - "Arn" - ] - } - } - ] - } - })); + 'grantConsumeMessages with additional actions'(test: Test) { + testGrant((q, p) => q.grantConsumeMessages(p, 'sqs:additional', 'sqs:action'), + 'sqs:ReceiveMessage', + 'sqs:ChangeMessageVisibility', + 'sqs:ChangeMessageVisibilityBatch', + 'sqs:GetQueueUrl', + 'sqs:DeleteMessage', + 'sqs:DeleteMessageBatch', + 'sqs:GetQueueAttributes', + 'sqs:additional', + 'sqs:action' + ); + test.done(); + }, + 'grantSendMessages'(test: Test) { + testGrant((q, p) => q.grantSendMessages(p), + 'sqs:SendMessage', + 'sqs:SendMessageBatch', + 'sqs:GetQueueAttributes', + 'sqs:GetQueueUrl', + ); test.done(); }, - 'grants permission to receive messages'(test: Test) { - const stack = new Stack(); - const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('lambda.amazonaws.com') }); - const queue = new sqs.Queue(stack, 'Queue'); - queue.grantReceiveMessages(role); + 'grantSendMessages with additional actions'(test: Test) { + testGrant((q, p) => q.grantSendMessages(p, 'sqs:foo'), + 'sqs:SendMessage', + 'sqs:SendMessageBatch', + 'sqs:GetQueueAttributes', + 'sqs:GetQueueUrl', + 'sqs:foo' + ); + test.done(); + }, - expect(stack).to(haveResource('AWS::IAM::Policy', { - "PolicyDocument": { - "Statement": [ - { - "Action": "sqs:ReceiveMessage", - "Effect": "Allow", - "Resource": { - "Fn::GetAtt": - [ - "Queue4A7E3555", - "Arn" - ] - } - } - ] - } - })); + 'grantPurge'(test: Test) { + testGrant((q, p) => q.grantPurge(p), + 'sqs:PurgeQueue', + 'sqs:GetQueueAttributes', + 'sqs:GetQueueUrl', + ); + test.done(); + }, + 'grantPurge with additional actions'(test: Test) { + testGrant((q, p) => q.grantPurge(p, 'hello', 'world'), + 'sqs:PurgeQueue', + 'sqs:GetQueueAttributes', + 'sqs:GetQueueUrl', + 'hello', + 'world' + ); test.done(); }, - 'grants permission to send messages'(test: Test) { + 'grant() is general purpose'(test: Test) { + testGrant((q, p) => q.grant(p, 'hello', 'world'), + 'hello', + 'world' + ); + test.done(); + }, + + 'grants also work on imported queues'(test: Test) { const stack = new Stack(); - const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('lambda.amazonaws.com') }); - const queue = new sqs.Queue(stack, 'Queue'); - queue.grantSendMessages(role); + const queue = Queue.import(stack, 'Import', { + queueArn: 'imported-queue-arn', + queueUrl: 'https://queue-url' + }); + + const user = new iam.User(stack, 'User'); + + queue.grantPurge(user); expect(stack).to(haveResource('AWS::IAM::Policy', { "PolicyDocument": { "Statement": [ { - "Action": "sqs:SendMessage", + "Action": [ + "sqs:PurgeQueue", + "sqs:GetQueueAttributes", + "sqs:GetQueueUrl" + ], "Effect": "Allow", - "Resource": { - "Fn::GetAtt": - [ - "Queue4A7E3555", - "Arn" - ] - } + "Resource": "imported-queue-arn" } - ] + ], + "Version": "2012-10-17" } })); test.done(); } - }, 'queue encryption': { @@ -500,3 +523,29 @@ export = { } }; + +function testGrant(action: (q: Queue, principal: iam.IPrincipal) => void, ...expectedActions: string[]) { + const stack = new Stack(); + const queue = new Queue(stack, 'MyQueue'); + const principal = new iam.User(stack, 'User'); + + action(queue, principal); + + expect(stack).to(haveResource('AWS::IAM::Policy', { + "PolicyDocument": { + "Statement": [ + { + "Action": expectedActions, + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "MyQueueE6CA6235", + "Arn" + ] + } + } + ], + "Version": "2012-10-17" + } + })); +} \ No newline at end of file From ebf57a7a9a8bbac76e1fbabf42ea6f5e6be35a02 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Thu, 1 Nov 2018 10:44:56 +0200 Subject: [PATCH 2/2] Remove additional actions for grant methods --- packages/@aws-cdk/aws-sqs/lib/queue-ref.ts | 17 ++++------ packages/@aws-cdk/aws-sqs/test/test.sqs.ts | 37 ---------------------- 2 files changed, 6 insertions(+), 48 deletions(-) diff --git a/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts b/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts index 67a7c1389bc1e..bd5c7c3acdae7 100644 --- a/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts +++ b/packages/@aws-cdk/aws-sqs/lib/queue-ref.ts @@ -125,9 +125,8 @@ export abstract class QueueRef extends cdk.Construct implements s3n.IBucketNotif * - sqs:GetQueueUrl * * @param identity Principal to grant consume rights to - * @param queueActions additional queue actions to allow */ - public grantConsumeMessages(identity?: iam.IPrincipal, ...queueActions: string[]) { + public grantConsumeMessages(identity?: iam.IPrincipal) { this.grant(identity, 'sqs:ReceiveMessage', 'sqs:ChangeMessageVisibility', @@ -135,8 +134,7 @@ export abstract class QueueRef extends cdk.Construct implements s3n.IBucketNotif 'sqs:GetQueueUrl', 'sqs:DeleteMessage', 'sqs:DeleteMessageBatch', - 'sqs:GetQueueAttributes', - ...queueActions); + 'sqs:GetQueueAttributes'); } /** @@ -150,15 +148,13 @@ export abstract class QueueRef extends cdk.Construct implements s3n.IBucketNotif * - sqs:GetQueueUrl * * @param identity Principal to grant send rights to - * @param queueActions additional queue actions to allow */ - public grantSendMessages(identity?: iam.IPrincipal, ...queueActions: string[]) { + public grantSendMessages(identity?: iam.IPrincipal) { this.grant(identity, 'sqs:SendMessage', 'sqs:SendMessageBatch', 'sqs:GetQueueAttributes', - 'sqs:GetQueueUrl', - ...queueActions); + 'sqs:GetQueueUrl'); } /** @@ -173,12 +169,11 @@ export abstract class QueueRef extends cdk.Construct implements s3n.IBucketNotif * @param identity Principal to grant send rights to * @param queueActions additional queue actions to allow */ - public grantPurge(identity?: iam.IPrincipal, ...queueActions: string[]) { + public grantPurge(identity?: iam.IPrincipal) { this.grant(identity, 'sqs:PurgeQueue', 'sqs:GetQueueAttributes', - 'sqs:GetQueueUrl', - ...queueActions); + 'sqs:GetQueueUrl'); } /** diff --git a/packages/@aws-cdk/aws-sqs/test/test.sqs.ts b/packages/@aws-cdk/aws-sqs/test/test.sqs.ts index 856ac9103762e..d6481db432925 100644 --- a/packages/@aws-cdk/aws-sqs/test/test.sqs.ts +++ b/packages/@aws-cdk/aws-sqs/test/test.sqs.ts @@ -128,21 +128,6 @@ export = { test.done(); }, - 'grantConsumeMessages with additional actions'(test: Test) { - testGrant((q, p) => q.grantConsumeMessages(p, 'sqs:additional', 'sqs:action'), - 'sqs:ReceiveMessage', - 'sqs:ChangeMessageVisibility', - 'sqs:ChangeMessageVisibilityBatch', - 'sqs:GetQueueUrl', - 'sqs:DeleteMessage', - 'sqs:DeleteMessageBatch', - 'sqs:GetQueueAttributes', - 'sqs:additional', - 'sqs:action' - ); - test.done(); - }, - 'grantSendMessages'(test: Test) { testGrant((q, p) => q.grantSendMessages(p), 'sqs:SendMessage', @@ -153,17 +138,6 @@ export = { test.done(); }, - 'grantSendMessages with additional actions'(test: Test) { - testGrant((q, p) => q.grantSendMessages(p, 'sqs:foo'), - 'sqs:SendMessage', - 'sqs:SendMessageBatch', - 'sqs:GetQueueAttributes', - 'sqs:GetQueueUrl', - 'sqs:foo' - ); - test.done(); - }, - 'grantPurge'(test: Test) { testGrant((q, p) => q.grantPurge(p), 'sqs:PurgeQueue', @@ -173,17 +147,6 @@ export = { test.done(); }, - 'grantPurge with additional actions'(test: Test) { - testGrant((q, p) => q.grantPurge(p, 'hello', 'world'), - 'sqs:PurgeQueue', - 'sqs:GetQueueAttributes', - 'sqs:GetQueueUrl', - 'hello', - 'world' - ); - test.done(); - }, - 'grant() is general purpose'(test: Test) { testGrant((q, p) => q.grant(p, 'hello', 'world'), 'hello',