From 29ca84cc625925a127afed127a113595b28a04eb Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Wed, 30 Jan 2019 20:18:07 +0000 Subject: [PATCH 1/2] - fixed node4/5 error --- lib/deploy/stepFunctions/compileIamRole.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index b330a67e..0ae8a1e9 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -130,7 +130,7 @@ function consolidatePermissionsByAction(permissions) { .mapValues(perms => { // find the unique resources let resources = _.uniqWith(_.flatMap(perms, p => p.resource), _.isEqual); - if (resources.includes('*')) { + if (_.includes(resources, '*')) { resources = '*'; } From 4ca4a5c0b0244b2cac9739d18a3d27d8eb30f5f2 Mon Sep 17 00:00:00 2001 From: Yan Cui Date: Wed, 30 Jan 2019 20:42:55 +0000 Subject: [PATCH 2/2] - issue a Deny all policy statement when there are no Task states in the state machine --- lib/deploy/stepFunctions/compileIamRole.js | 25 +++++++-- .../stepFunctions/compileIamRole.test.js | 52 ++++++++++++++++--- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/lib/deploy/stepFunctions/compileIamRole.js b/lib/deploy/stepFunctions/compileIamRole.js index 0ae8a1e9..aab18e9c 100644 --- a/lib/deploy/stepFunctions/compileIamRole.js +++ b/lib/deploy/stepFunctions/compileIamRole.js @@ -198,6 +198,25 @@ function getIamPermissions(serverless, taskStates) { }); } +function getIamStatements(iamPermissions) { + // when the state machine doesn't define any Task states, and therefore doesn't need ANY + // permission, then we should follow the behaviour of the AWS console and return a policy + // that denies access to EVERYTHING + if (_.isEmpty(iamPermissions)) { + return [{ + Effect: 'Deny', + Action: '*', + Resource: '*', + }]; + } + + return iamPermissions.map(p => ({ + Effect: 'Allow', + Action: p.action.split(','), + Resource: p.resource, + })); +} + module.exports = { compileIamRole() { const customRolesProvided = []; @@ -223,11 +242,7 @@ module.exports = { iamPermissions = consolidatePermissionsByAction(iamPermissions); iamPermissions = consolidatePermissionsByResource(iamPermissions); - const iamStatements = iamPermissions.map(p => ({ - Effect: 'Allow', - Action: p.action.split(','), - Resource: p.resource, - })); + const iamStatements = getIamStatements(iamPermissions); const iamRoleJson = iamRoleStateMachineExecutionTemplate diff --git a/lib/deploy/stepFunctions/compileIamRole.test.js b/lib/deploy/stepFunctions/compileIamRole.test.js index b95712e0..15f42179 100644 --- a/lib/deploy/stepFunctions/compileIamRole.test.js +++ b/lib/deploy/stepFunctions/compileIamRole.test.js @@ -24,6 +24,14 @@ describe('#compileIamRole', () => { serverlessStepFunctions = new ServerlessStepFunctions(serverless, options); }); + const expectDenyAllPolicy = (policy) => { + const statements = policy.PolicyDocument.Statement; + expect(statements).to.have.lengthOf(1); + expect(statements[0].Effect).to.equal('Deny'); + expect(statements[0].Action).to.equal('*'); + expect(statements[0].Resource).to.equal('*'); + }; + it('should do nothing when role property exists in all statmachine properties', () => { serverless.service.stepFunctions = { stateMachines: { @@ -243,7 +251,7 @@ describe('#compileIamRole', () => { const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution .Properties.Policies[0]; - expect(policy.PolicyDocument.Statement).to.have.lengthOf(0); + expectDenyAllPolicy(policy); }); it('should give sqs:SendMessage permission for only SQS referenced by state machine', () => { @@ -362,7 +370,7 @@ describe('#compileIamRole', () => { const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution .Properties.Policies[0]; - expect(policy.PolicyDocument.Statement).to.have.lengthOf(0); + expectDenyAllPolicy(policy); }); it('should not give sqs:SendMessage permission if QueueUrl is invalid', () => { @@ -789,10 +797,10 @@ describe('#compileIamRole', () => { }; serverlessStepFunctions.compileIamRole(); - const statements = serverlessStepFunctions.serverless.service + const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution - .Properties.Policies[0].PolicyDocument.Statement; - expect(statements).to.have.lengthOf(0); + .Properties.Policies[0]; + expectDenyAllPolicy(policy); }); it('should not generate any permissions for Task states not yet supported', () => { @@ -818,9 +826,37 @@ describe('#compileIamRole', () => { }; serverlessStepFunctions.compileIamRole(); - const statements = serverlessStepFunctions.serverless.service + const policy = serverlessStepFunctions.serverless.service .provider.compiledCloudFormationTemplate.Resources.IamRoleStateMachineExecution - .Properties.Policies[0].PolicyDocument.Statement; - expect(statements).to.have.lengthOf(0); + .Properties.Policies[0]; + expectDenyAllPolicy(policy); + }); + + it('should generate a Deny all statement if state machine has no tasks', () => { + const genStateMachine = (name) => ({ + name, + definition: { + StartAt: 'A', + States: { + A: { + Type: 'Pass', + 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]; + expectDenyAllPolicy(policy); }); });