Merge pull request #165 from theburningmonk/feature/fix_empty_perms

fix generated IAM role when there are no Task states

Author: horike37

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

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);
   });
 });