From 9b82ef59b6cc651d77e74d3c0cc5c8168bad3ea1 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Sun, 20 Jan 2019 14:36:35 +0000 Subject: [PATCH 1/5] - recursive get all the Task states and issue policy statements - consolidate permissions by action and resource to minimise the no. of statements --- lib/deploy/stepFunctions/compileIamRole.js | 221 ++++++++++++++++-- ...m-role-statemachine-execution-template.txt | 10 +- 2 files changed, 209 insertions(+), 22 deletions(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index 9e9f32c5..bb995cb3 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -3,43 +3,238 @@ const _ = require('lodash'); const BbPromise = require('bluebird'); const path = require('path'); +function getTaskStates(states) { + return _.flatMap(states, state => { + switch (state.Type) { + case 'Task': + return [state]; + case 'Parallel': + const parallelStates = _.flatMap(state.Branches, branch => _.values(branch.States)); + return getTaskStates(parallelStates); + default: + return []; + } + }); +} + +function sqsQueueUrlToArn(serverless, queueUrl) { + const regex = /https:\/\/sqs.(.*).amazonaws.com\/(.*)\/(.*)/g; + const match = regex.exec(queueUrl); + if (match) { + const region = match[1]; + const accountId = match[2]; + const queueName = match[3]; + return `arn:aws:sqs:${region}:${accountId}:${queueName}`; + } + serverless.cli.consoleLog(`Unable to parse SQS queue url [${queueUrl}], using '*' instead`); + return '*'; +} + +function getSqsPermissions(serverless, state) { + if (_.has(state, 'Parameters.QueueUrl') || + _.has(state, ['Parameters', 'QueueUrl.$'])) { + // if queue URL is provided by input, then need pervasive permissions (i.e. '*') + const queueArn = state.Parameters['QueueUrl.$'] + ? '*' + : sqsQueueUrlToArn(serverless, state.Parameters.QueueUrl); + return [{ action: 'sqs:SendMessage', resource: queueArn }]; + } + serverless.cli.consoleLog('SQS task missing Parameters.QueueUrl or Parameters.QueueUrl.$'); + return []; +} + +function getSnsPermissions(serverless, state) { + if (_.has(state, 'Parameters.TopicArn') || + _.has(state, ['Parameters', 'TopicArn.$'])) { + // if topic ARN is provided by input, then need pervasive permissions + const topicArn = state.Parameters['TopicArn.$'] ? '*' : state.Parameters.TopicArn; + return [{ action: 'sns:Publish', resource: topicArn }]; + } + serverless.cli.consoleLog('SNS task missing Parameters.TopicArn or Parameters.TopicArn.$'); + return []; +} + +function getDynamoDBArn(tableName) { + return { + 'Fn::Join': [ + ':', + [ + 'arn:aws:dynamodb', + { Ref: 'AWS::Region' }, + { Ref: 'AWS::AccountId' }, + `table/${tableName}`, + ], + ], + }; +} + +function getBatchPermissions() { + return [{ + action: 'batch:SubmitJob,batch:DescribeJobs,batch:TerminateJob', + resource: '*', + }, { + action: 'events:PutTargets,events:PutRule,events:DescribeRule', + resource: { + 'Fn::Join': [ + ':', + [ + 'arn:aws:events', + { Ref: 'AWS::Region' }, + { Ref: 'AWS::AccountId' }, + 'rules/StepFunctionsGetEventsForBatchJobsRule', + ], + ], + }, + }]; +} + +function getEcsPermissions() { + return [{ + action: 'ecs:RunTask,ecs:StopTask,ecs:DescribeTasks', + resource: '*', + }, { + action: 'events:PutTargets,events:PutRule,events:DescribeRule', + resource: { + 'Fn::Join': [ + ':', + [ + 'arn:aws:events', + { Ref: 'AWS::Region' }, + { Ref: 'AWS::AccountId' }, + 'rules/StepFunctionsGetEventsForECSTaskRule', + ], + ], + }, + }]; +} + +function getDynamoDBPermissions(action, state) { + const tableArn = state.Parameters['TableName.$'] + ? '*' + : getDynamoDBArn(state.Parameters.TableName); + + return [{ + action, + resource: tableArn, + }]; +} + +// if there are multiple permissions with the same action, then collapsed them into one +// permission instead, and collect the resources into an array +function consolidatePermissionsByAction(permissions) { + return _.chain(permissions) + .groupBy(perm => perm.action) + .mapValues(perms => { + // find the unique resources + let resources = _.uniqWith(_.flatMap(perms, p => p.resource), _.isEqual); + if (resources.includes('*')) { + resources = '*'; + } + + return { + action: perms[0].action, + resource: resources, + }; + }) + .values() + .value(); // unchain +} + +function consolidatePermissionsByResource(permissions) { + return _.chain(permissions) + .groupBy(p => JSON.stringify(p.resource)) + .mapValues(perms => { + // find unique actions + const actions = _.uniq(_.flatMap(perms, p => p.action.split(','))); + + return { + action: actions.join(','), + resource: perms[0].resource, + }; + }) + .values() + .value(); // unchain +} + +function getIamPermissions(serverless, taskStates) { + return _.flatMap(taskStates, state => { + switch (state.Resource) { + case 'arn:aws:states:::sqs:sendMessage': + return getSqsPermissions(serverless, state); + + case 'arn:aws:states:::sns:publish': + return getSnsPermissions(serverless, state); + + case 'arn:aws:states:::dynamodb:updateItem': + return getDynamoDBPermissions('dynamodb:UpdateItem', state); + case 'arn:aws:states:::dynamodb:putItem': + return getDynamoDBPermissions('dynamodb:PutItem', state); + case 'arn:aws:states:::dynamodb:getItem': + return getDynamoDBPermissions('dynamodb:GetItem', state); + case 'arn:aws:states:::dynamodb:deleteItem': + return getDynamoDBPermissions('dynamodb:DeleteItem', state); + + case 'arn:aws:states:::batch:submitJob.sync': + case 'arn:aws:states:::batch:submitJob': + return getBatchPermissions(); + + case 'arn:aws:states:::ecs:runTask.sync': + case 'arn:aws:states:::ecs:runTask': + return getEcsPermissions(); + + default: + if (state.Resource.startsWith('arn:aws:lambda')) { + return [{ + action: 'lambda:InvokeFunction', + resource: state.Resource, + }]; + } + serverless.cli.consoleLog('Cannot generate IAM policy statement for Task state', state); + return []; + } + }); +} + module.exports = { compileIamRole() { const customRolesProvided = []; - let functionArns = []; + let iamPermissions = []; this.getAllStateMachines().forEach((stateMachineName) => { const stateMachineObj = this.getStateMachine(stateMachineName); customRolesProvided.push('role' in stateMachineObj); - const stateMachineJson = JSON.stringify(stateMachineObj); - const regex = new RegExp(/"Resource":"([\w\-:*#{}.$]*)"/gi); - let match = regex.exec(stateMachineJson); - while (match !== null) { - functionArns.push(match[1]); - match = regex.exec(stateMachineJson); - } + const taskStates = getTaskStates(stateMachineObj.definition.States); + iamPermissions = iamPermissions.concat(getIamPermissions(this.serverless, taskStates)); }); if (_.isEqual(_.uniq(customRolesProvided), [true])) { return BbPromise.resolve(); } - functionArns = _.uniq(functionArns); - let iamRoleStateMachineExecutionTemplate = this.serverless.utils.readFileSync( + const iamRoleStateMachineExecutionTemplate = this.serverless.utils.readFileSync( path.join(__dirname, '..', '..', 'iam-role-statemachine-execution-template.txt') ); - iamRoleStateMachineExecutionTemplate = + iamPermissions = consolidatePermissionsByAction(iamPermissions); + iamPermissions = consolidatePermissionsByResource(iamPermissions); + + const iamStatements = iamPermissions.map(p => ({ + Effect: 'Allow', + Action: p.action.split(','), + Resource: p.resource, + })); + + const iamRoleJson = iamRoleStateMachineExecutionTemplate .replace('[region]', this.options.region) .replace('[PolicyName]', this.getStateMachinePolicyName()) - .replace('[functions]', JSON.stringify(functionArns)); + .replace('[Statements]', JSON.stringify(iamStatements)); const iamRoleStateMachineLogicalId = this.getiamRoleStateMachineLogicalId(); const newIamRoleStateMachineExecutionObject = { - [iamRoleStateMachineLogicalId]: JSON.parse(iamRoleStateMachineExecutionTemplate), + [iamRoleStateMachineLogicalId]: JSON.parse(iamRoleJson), }; _.merge(this.serverless.service.provider.compiledCloudFormationTemplate.Resources, diff --git a/lib/iam-role-statemachine-execution-template.txt b/lib/iam-role-statemachine-execution-template.txt index 49b01c02..38596d6d 100644 --- a/lib/iam-role-statemachine-execution-template.txt +++ b/lib/iam-role-statemachine-execution-template.txt @@ -18,15 +18,7 @@ "PolicyName": "[PolicyName]", "PolicyDocument": { "Version": "2012-10-17", - "Statement": [ - { - "Effect": "Allow", - "Action": [ - "lambda:InvokeFunction" - ], - "Resource": [functions] - } - ] + "Statement": [Statements] } } ] From 19ffa3505b385da826bf844243fb3320dac187e3 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Mon, 21 Jan 2019 10:58:31 +0000 Subject: [PATCH 2/5] - refactored the IAM role test - added tests for SNS, SQS, DynamoDB and handling for nested parallel states --- .../stepFunctions/compileIamRole.test.js | 335 +++++++++++++++--- 1 file changed, 279 insertions(+), 56 deletions(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.test.js b/lib/deploy/stepFunctions/compileIamRole.test.js index 437258b4..d5edd9f2 100644 --- a/lib/deploy/stepFunctions/compileIamRole.test.js +++ b/lib/deploy/stepFunctions/compileIamRole.test.js @@ -1,5 +1,6 @@ 'use strict'; +const _ = require('lodash'); const expect = require('chai').expect; const Serverless = require('serverless/lib/Serverless'); const AwsProvider = require('serverless/lib/plugins/aws/provider/awsProvider'); @@ -91,82 +92,304 @@ describe('#compileIamRole', () => { const worldLambda = 'arn:aws:lambda:*:*:function:world'; const fooLambda = 'arn:aws:lambda:us-west-2::function:foo_'; const barLambda = 'arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:bar'; - const bazLambda = 'arn:aws:lambda:#{AWS::Region}:#{AWS::AccountId}:function:${baz}'; - serverless.service.stepFunctions = { - stateMachines: { - myStateMachine1: { - name: 'stateMachineBeta1', - definition: { - StartAt: 'Hello', - States: { - Hello: { - Type: 'Task', - Resource: helloLambda, - End: true, - }, - }, + + const genStateMachine = (name, lambda1, lambda2) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: lambda1, + Next: 'B', + }, + B: { + Type: 'Task', + Resource: lambda2, + End: true, }, }, - myStateMachine2: { - name: 'stateMachineBeta2', - definition: { - StartAt: 'World', - States: { - World: { - Type: 'Task', - Resource: worldLambda, - End: true, - }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1', helloLambda, worldLambda), + myStateMachine2: genStateMachine('stateMachineBeta2', fooLambda, barLambda), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement[0].Resource) + .to.be.deep.equal([helloLambda, worldLambda, fooLambda, barLambda]); + }); + + it('should give sns:Publish permission for only SNS topics referenced by state machine', () => { + const helloTopic = 'arn:aws:sns:#{AWS::Region}:#{AWS::AccountId}:hello'; + const worldTopic = 'arn:aws:sns:us-east-1:#{AWS::AccountId}:world'; + + const genStateMachine = (name, snsTopic) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sns:publish', + Parameters: { + Message: '42', + TopicArn: snsTopic, }, + End: true, }, }, - myStateMachine3: { - name: 'stateMachineBeta3', - definition: { - StartAt: 'Foo', - States: { - Hello: { - Type: 'Task', - Resource: fooLambda, - End: true, - }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1', helloTopic), + myStateMachine2: genStateMachine('stateMachineBeta2', worldTopic), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement[0].Resource) + .to.be.deep.equal([helloTopic, worldTopic]); + }); + + it('should give sqs:SendMessage permission for only SQS referenced by state machine', () => { + const helloQueue = 'https://sqs.#{AWS::Region}.amazonaws.com/#{AWS::AccountId}/hello'; + const helloQueueArn = 'arn:aws:sqs:#{AWS::Region}:#{AWS::AccountId}:hello'; + const worldQueue = 'https://sqs.us-east-1.amazonaws.com/#{AWS::AccountId}/world'; + const worldQueueArn = 'arn:aws:sqs:us-east-1:#{AWS::AccountId}:world'; + + const genStateMachine = (name, queueUrl) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sqs:sendMessage', + Parameters: { + QueueUrl: queueUrl, + Message: '42', }, + End: true, }, }, - myStateMachine4: { - name: 'stateMachineBeta4', - definition: { - StartAt: 'Bar', - States: { - Hello: { - Type: 'Task', - Resource: barLambda, - End: true, - }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1', helloQueue), + myStateMachine2: genStateMachine('stateMachineBeta2', worldQueue), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement[0].Resource) + .to.be.deep.equal([helloQueueArn, worldQueueArn]); + }); + + it('should give dynamodb permission for only tables referenced by state machine', () => { + const helloTable = 'hello'; + const helloTableArn = { + 'Fn::Join': [ + ':', ['arn:aws:dynamodb', { Ref: 'AWS::Region' }, { Ref: 'AWS::AccountId' }, 'table/hello'], + ], + }; + const worldTable = 'world'; + const worldTableArn = { + 'Fn::Join': [ + ':', ['arn:aws:dynamodb', { Ref: 'AWS::Region' }, { Ref: 'AWS::AccountId' }, 'table/world'], + ], + }; + + const genStateMachine = (name, tableName) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:updateItem', + Parameters: { + TableName: tableName, }, + Next: 'B', }, - }, - myStateMachine5: { - name: 'stateMachineBeta5', - definition: { - StartAt: 'Baz', - States: { - Hello: { - Type: 'Task', - Resource: bazLambda, - End: true, - }, + B: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:putItem', + Parameters: { + TableName: tableName, }, + End: true, }, }, }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1', helloTable), + myStateMachine2: genStateMachine('stateMachineBeta2', worldTable), + }, }; serverlessStepFunctions.compileIamRole(); const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement[0].Action) + .to.be.deep.equal(['dynamodb:UpdateItem', 'dynamodb:PutItem']); expect(policy.PolicyDocument.Statement[0].Resource) - .to.be.deep.equal([helloLambda, worldLambda, fooLambda, barLambda, bazLambda]); + .to.be.deep.equal([helloTableArn, worldTableArn]); + }); + + it('should handle nested parallel states', () => { + const getStateMachine = (name, lambdaArn, snsTopicArn, sqsQueueUrl, dynamodbTable) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sns:publish', + Parameters: { + Message: '42', + TopicArn: snsTopicArn, + }, + Next: 'Parallel', + }, + Parallel: { + Type: 'Parallel', + Branches: [{ + StartAt: 'B', + States: { + B: { + Type: 'Task', + Resource: lambdaArn, + End: true, + }, + }, + }, { + StartAt: 'C', + States: { + C: { + Type: 'Task', + Resource: 'arn:aws:states:::sqs:sendMessage', + Parameters: { + QueueUrl: sqsQueueUrl, + }, + }, + }, + }, { + StartAt: 'NestedParallel', + States: { + NestedParallel: { + Type: 'Parallel', + Branches: [{ + StartAt: 'D', + States: { + D: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:updateItem', + Parameters: { + TableName: dynamodbTable, + }, + }, + }, + }, { + StartAt: 'E', + States: { + E: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:putItem', + Parameters: { + TableName: dynamodbTable, + }, + }, + }, + }], + }, + }, + }], + }, + }, + }, + }); + + const [lambda1, lambda2] = [ + 'arn:aws:lambda:us-west-2:1234567890:function:foo', + 'arn:aws:lambda:us-west-1:#{AWS::AccountId}:function:bar', + ]; + + const [sns1, sns2] = [ + 'arn:aws:sns:us-east-1:1234567890:foo', + 'arn:aws:sns:us-east-2:#{AWS::AccountId}:bar', + ]; + + const [sqs1, sqs2] = [ + 'https://sqs.us-east-1.amazonaws.com/1234567890/foo', + 'https://sqs.us-east-2.amazonaws.com/#{AWS::AccountId}/bar', + ]; + const [sqsArn1, sqsArn2] = [ + 'arn:aws:sqs:us-east-1:1234567890:foo', + 'arn:aws:sqs:us-east-2:#{AWS::AccountId}:bar', + ]; + + const [dynamodb1, dynamodb2] = ['foo', 'bar']; + const [dynamodbArn1, dynamodbArn2] = [{ + 'Fn::Join': [ + ':', ['arn:aws:dynamodb', { Ref: 'AWS::Region' }, { Ref: 'AWS::AccountId' }, 'table/foo'], + ], + }, { + 'Fn::Join': [ + ':', ['arn:aws:dynamodb', { Ref: 'AWS::Region' }, { Ref: 'AWS::AccountId' }, 'table/bar'], + ], + }]; + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: getStateMachine('sm1', lambda1, sns1, sqs1, dynamodb1), + myStateMachine2: getStateMachine('sm2', lambda2, sns2, sqs2, dynamodb2), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0].PolicyDocument.Statement; + + const lambdaPermissions = statements.filter(s => + _.isEqual(s.Action, ['lambda:InvokeFunction'])); + expect(lambdaPermissions).to.have.lengthOf(1); + expect(lambdaPermissions[0].Resource).to.deep.eq([lambda1, lambda2]); + + const snsPermissions = statements.filter(s => _.isEqual(s.Action, ['sns:Publish'])); + expect(snsPermissions).to.have.lengthOf(1); + expect(snsPermissions[0].Resource).to.deep.eq([sns1, sns2]); + + const sqsPermissions = statements.filter(s => _.isEqual(s.Action, ['sqs:SendMessage'])); + expect(sqsPermissions).to.have.lengthOf(1); + expect(sqsPermissions[0].Resource).to.deep.eq([sqsArn1, sqsArn2]); + + const dynamodbPermissions = statements.filter(s => + _.isEqual(s.Action, ['dynamodb:UpdateItem', 'dynamodb:PutItem'])); + expect(dynamodbPermissions).to.have.lengthOf(1); + expect(dynamodbPermissions[0].Resource).to.deep.eq([dynamodbArn1, dynamodbArn2]); }); }); From 2de6f0cfff5aae0a73a9c9858ed5b290a277d4da Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Mon, 21 Jan 2019 11:03:45 +0000 Subject: [PATCH 3/5] - fixed linting issue --- lib/deploy/stepFunctions/compileIamRole.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index bb995cb3..c8d6ab51 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -6,13 +6,16 @@ const path = require('path'); function getTaskStates(states) { return _.flatMap(states, state => { switch (state.Type) { - case 'Task': + case 'Task': { return [state]; - case 'Parallel': + } + case 'Parallel': { const parallelStates = _.flatMap(state.Branches, branch => _.values(branch.States)); return getTaskStates(parallelStates); - default: + } + default: { return []; + } } }); } From 0b83026c6b21c5f4a0314444eceb7162006f4e71 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Mon, 21 Jan 2019 14:27:07 +0000 Subject: [PATCH 4/5] - flashed out more tests to reach 100% coverage - don't give me permissions if queueUrl is invalid, makes more sense --- lib/deploy/stepFunctions/compileIamRole.js | 4 +- .../stepFunctions/compileIamRole.test.js | 439 +++++++++++++++++- 2 files changed, 439 insertions(+), 4 deletions(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index c8d6ab51..b330a67e 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -29,8 +29,8 @@ function sqsQueueUrlToArn(serverless, queueUrl) { const queueName = match[3]; return `arn:aws:sqs:${region}:${accountId}:${queueName}`; } - serverless.cli.consoleLog(`Unable to parse SQS queue url [${queueUrl}], using '*' instead`); - return '*'; + serverless.cli.consoleLog(`Unable to parse SQS queue url [${queueUrl}]`); + return []; } function getSqsPermissions(serverless, state) { diff --git a/lib/deploy/stepFunctions/compileIamRole.test.js b/lib/deploy/stepFunctions/compileIamRole.test.js index d5edd9f2..4446cbbc 100644 --- a/lib/deploy/stepFunctions/compileIamRole.test.js +++ b/lib/deploy/stepFunctions/compileIamRole.test.js @@ -16,6 +16,7 @@ describe('#compileIamRole', () => { serverless.service.service = 'step-functions'; serverless.service.provider.compiledCloudFormationTemplate = { Resources: {} }; serverless.setProvider('aws', new AwsProvider(serverless)); + serverless.cli = { consoleLog: console.log }; const options = { stage: 'dev', region: 'ap-northeast-1', @@ -164,6 +165,87 @@ describe('#compileIamRole', () => { .to.be.deep.equal([helloTopic, worldTopic]); }); + it('should give sns:Publish permission to * whenever TopicArn.$ is seen', () => { + const helloTopic = 'arn:aws:sns:#{AWS::Region}:#{AWS::AccountId}:hello'; + const worldTopic = 'arn:aws:sns:us-east-1:#{AWS::AccountId}:world'; + + const genStateMachine = (name, snsTopic) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sns:publish', + Parameters: { + Message: '42', + TopicArn: snsTopic, + }, + Next: 'B', + }, + B: { + Type: 'Task', + Resource: 'arn:aws:states:::sns:publish', + Parameters: { + Message: '42', + 'TopicArn.$': '$.snsTopic', + }, + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1', helloTopic), + myStateMachine2: genStateMachine('stateMachineBeta2', worldTopic), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + + // even though some tasks target specific topic ARNs, but because some other states + // use TopicArn.$ we need to give broad permissions to be able to publish to any + // topic that the input specifies + expect(policy.PolicyDocument.Statement[0].Resource).to.equal('*'); + }); + + it('should not give sns:Publish permission if TopicArn and TopicArn.$ are missing', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sns:publish', + Parameters: { + MessageBody: '42', + }, + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement).to.have.lengthOf(0); + }); + it('should give sqs:SendMessage permission for only SQS referenced by state machine', () => { const helloQueue = 'https://sqs.#{AWS::Region}.amazonaws.com/#{AWS::AccountId}/hello'; const helloQueueArn = 'arn:aws:sqs:#{AWS::Region}:#{AWS::AccountId}:hello'; @@ -203,6 +285,121 @@ describe('#compileIamRole', () => { .to.be.deep.equal([helloQueueArn, worldQueueArn]); }); + it('should give sqs:SendMessage permission to * whenever QueueUrl.$ is seen', () => { + const helloQueue = 'https://sqs.#{AWS::Region}.amazonaws.com/#{AWS::AccountId}/hello'; + const worldQueue = 'https://sqs.us-east-1.amazonaws.com/#{AWS::AccountId}/world'; + + const genStateMachine = (name, queueUrl) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sqs:sendMessage', + Parameters: { + QueueUrl: queueUrl, + Message: '42', + }, + Next: 'B', + }, + B: { + Type: 'Task', + Resource: 'arn:aws:states:::sqs:sendMessage', + Parameters: { + 'QueueUrl.$': '$.queueUrl', + Message: '42', + }, + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1', helloQueue), + myStateMachine2: genStateMachine('stateMachineBeta2', worldQueue), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + + // even if some tasks are targetting specific queues, because QueueUrl.$ is seen + // we need to give broad permissions allow the queue URL to be specified by input + expect(policy.PolicyDocument.Statement[0].Resource).to.equal('*'); + }); + + it('should not give sqs:SendMessage permission if QueueUrl and QueueUrl.$ are missing', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sqs:sendMessage', + Parameters: { + Message: '42', + }, + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement).to.have.lengthOf(0); + }); + + it('should not give sqs:SendMessage permission if QueueUrl is invalid', () => { + const invalidQueueUrl = 'https://sqs.us-east-1.amazonaws.com/hello'; + + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::sqs:sendMessage', + Parameters: { + QueueUrl: invalidQueueUrl, + Message: '42', + }, + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement[0].Resource).to.have.lengthOf(0); + }); + it('should give dynamodb permission for only tables referenced by state machine', () => { const helloTable = 'hello'; const helloTableArn = { @@ -236,6 +433,22 @@ describe('#compileIamRole', () => { Parameters: { TableName: tableName, }, + Next: 'C', + }, + C: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:getItem', + Parameters: { + TableName: tableName, + }, + Next: 'D', + }, + D: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:deleteItem', + Parameters: { + TableName: tableName, + }, End: true, }, }, @@ -254,11 +467,175 @@ describe('#compileIamRole', () => { .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution .Properties.Policies[0]; expect(policy.PolicyDocument.Statement[0].Action) - .to.be.deep.equal(['dynamodb:UpdateItem', 'dynamodb:PutItem']); + .to.be.deep.equal([ + 'dynamodb:UpdateItem', + 'dynamodb:PutItem', + 'dynamodb:GetItem', + 'dynamodb:DeleteItem', + ]); expect(policy.PolicyDocument.Statement[0].Resource) .to.be.deep.equal([helloTableArn, worldTableArn]); }); + it('should give dynamodb permission to * whenever TableName.$ is seen', () => { + const helloTable = 'hello'; + const worldTable = 'world'; + + const genStateMachine = (name, tableName) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:updateItem', + Parameters: { + TableName: tableName, + }, + Next: 'B', + }, + B: { + Type: 'Task', + Resource: 'arn:aws:states:::dynamodb:updateItem', + Parameters: { + 'TableName.$': '$.tableName', + }, + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1', helloTable), + myStateMachine2: genStateMachine('stateMachineBeta2', worldTable), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const policy = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0]; + expect(policy.PolicyDocument.Statement[0].Action) + .to.be.deep.equal(['dynamodb:UpdateItem']); + + // even though some tasks target specific tables, because TableName.$ is used we + // have to give broad permissions to allow execution to talk to whatever table + // the input specifies + expect(policy.PolicyDocument.Statement[0].Resource).to.equal('*'); + }); + + it('should give batch permissions (too permissive, but mirrors console behaviour)', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::batch:submitJob', + Next: 'B', + }, + B: { + Type: 'Task', + Resource: 'arn:aws:states:::batch:submitJob.sync', + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0].PolicyDocument.Statement; + + const batchPermissions = statements.filter(s => + _.isEqual(s.Action, ['batch:SubmitJob', 'batch:DescribeJobs', 'batch:TerminateJob']) + ); + expect(batchPermissions).to.have.lengthOf(1); + expect(batchPermissions[0].Resource).to.equal('*'); + + const eventPermissions = statements.filter(s => + _.isEqual(s.Action, ['events:PutTargets', 'events:PutRule', 'events:DescribeRule']) + ); + expect(eventPermissions).to.has.lengthOf(1); + expect(eventPermissions[0].Resource).to.deep.eq([{ + 'Fn::Join': [ + ':', + [ + 'arn:aws:events', + { Ref: 'AWS::Region' }, + { Ref: 'AWS::AccountId' }, + 'rules/StepFunctionsGetEventsForBatchJobsRule', + ], + ], + }]); + }); + + it('should give ECS permissions (too permissive, but mirrors console behaviour)', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::ecs:runTask', + Next: 'B', + }, + B: { + Type: 'Task', + Resource: 'arn:aws:states:::ecs:runTask.sync', + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0].PolicyDocument.Statement; + + const ecsPermissions = statements.filter(s => + _.isEqual(s.Action, ['ecs:RunTask', 'ecs:StopTask', 'ecs:DescribeTasks']) + ); + expect(ecsPermissions).to.have.lengthOf(1); + expect(ecsPermissions[0].Resource).to.equal('*'); + + const eventPermissions = statements.filter(s => + _.isEqual(s.Action, ['events:PutTargets', 'events:PutRule', 'events:DescribeRule']) + ); + expect(eventPermissions).to.has.lengthOf(1); + expect(eventPermissions[0].Resource).to.deep.eq([{ + 'Fn::Join': [ + ':', + [ + 'arn:aws:events', + { Ref: 'AWS::Region' }, + { Ref: 'AWS::AccountId' }, + 'rules/StepFunctionsGetEventsForECSTaskRule', + ], + ], + }]); + }); + it('should handle nested parallel states', () => { const getStateMachine = (name, lambdaArn, snsTopicArn, sqsQueueUrl, dynamodbTable) => ({ name, @@ -272,7 +649,7 @@ describe('#compileIamRole', () => { Message: '42', TopicArn: snsTopicArn, }, - Next: 'Parallel', + Next: 'Pass', }, Parallel: { Type: 'Parallel', @@ -392,4 +769,62 @@ describe('#compileIamRole', () => { expect(dynamodbPermissions).to.have.lengthOf(1); expect(dynamodbPermissions[0].Resource).to.deep.eq([dynamodbArn1, dynamodbArn2]); }); + + it('should not generate any permissions for Pass states', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Pass', + Result: 42, + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0].PolicyDocument.Statement; + expect(statements).to.have.lengthOf(0); + }); + + it('should not generate any permissions for Task states not yet supported', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Task', + Resource: 'arn:aws:states:::foo:bar', + End: true, + }, + }, + }, + }); + + serverless.service.stepFunctions = { + stateMachines: { + myStateMachine1: genStateMachine('stateMachineBeta1'), + myStateMachine2: genStateMachine('stateMachineBeta2'), + }, + }; + + serverlessStepFunctions.compileIamRole(); + const statements = serverlessStepFunctions.serverless.service + .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution + .Properties.Policies[0].PolicyDocument.Statement; + expect(statements).to.have.lengthOf(0); + }); }); From 647961fa35394751741723e960ad2a113e6e7d70 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Mon, 21 Jan 2019 16:46:18 +0000 Subject: [PATCH 5/5] - removed distructoring syntax which breaks older nodejs builds --- .../stepFunctions/compileIamRole.test.js | 42 +++++++++---------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.test.js b/lib/deploy/stepFunctions/compileIamRole.test.js index 4446cbbc..b95712e0 100644 --- a/lib/deploy/stepFunctions/compileIamRole.test.js +++ b/lib/deploy/stepFunctions/compileIamRole.test.js @@ -709,35 +709,31 @@ describe('#compileIamRole', () => { }, }); - const [lambda1, lambda2] = [ - 'arn:aws:lambda:us-west-2:1234567890:function:foo', - 'arn:aws:lambda:us-west-1:#{AWS::AccountId}:function:bar', - ]; - - const [sns1, sns2] = [ - 'arn:aws:sns:us-east-1:1234567890:foo', - 'arn:aws:sns:us-east-2:#{AWS::AccountId}:bar', - ]; - - const [sqs1, sqs2] = [ - 'https://sqs.us-east-1.amazonaws.com/1234567890/foo', - 'https://sqs.us-east-2.amazonaws.com/#{AWS::AccountId}/bar', - ]; - const [sqsArn1, sqsArn2] = [ - 'arn:aws:sqs:us-east-1:1234567890:foo', - 'arn:aws:sqs:us-east-2:#{AWS::AccountId}:bar', - ]; - - const [dynamodb1, dynamodb2] = ['foo', 'bar']; - const [dynamodbArn1, dynamodbArn2] = [{ + const lambda1 = 'arn:aws:lambda:us-west-2:1234567890:function:foo'; + const lambda2 = 'arn:aws:lambda:us-west-1:#{AWS::AccountId}:function:bar'; + + const sns1 = 'arn:aws:sns:us-east-1:1234567890:foo'; + const sns2 = 'arn:aws:sns:us-east-2:#{AWS::AccountId}:bar'; + + const sqs1 = 'https://sqs.us-east-1.amazonaws.com/1234567890/foo'; + const sqs2 = 'https://sqs.us-east-2.amazonaws.com/#{AWS::AccountId}/bar'; + + const sqsArn1 = 'arn:aws:sqs:us-east-1:1234567890:foo'; + const sqsArn2 = 'arn:aws:sqs:us-east-2:#{AWS::AccountId}:bar'; + + const dynamodb1 = 'foo'; + const dynamodb2 = 'bar'; + + const dynamodbArn1 = { 'Fn::Join': [ ':', ['arn:aws:dynamodb', { Ref: 'AWS::Region' }, { Ref: 'AWS::AccountId' }, 'table/foo'], ], - }, { + }; + const dynamodbArn2 = { 'Fn::Join': [ ':', ['arn:aws:dynamodb', { Ref: 'AWS::Region' }, { Ref: 'AWS::AccountId' }, 'table/bar'], ], - }]; + }; serverless.service.stepFunctions = { stateMachines: {