Merge pull request serverless-heaven#121 from HyperBrain/hotfix-serve…

…rless-1-27 Fixed detection of Principal for Serverless 1.27
1stdibs · May 8, 2018 · aa26c92 · aa26c92
2 parents fb51d7b + 43af014
commit aa26c92
Show file tree

Hide file tree

Showing 6 changed files with 185 additions and 4,049 deletions.
diff --git a/lib/stackops/apiGateway.js b/lib/stackops/apiGateway.js
@@ -204,7 +204,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
 		const apiLambdaPermissions =
 				_.assign({},
 					_.pickBy(_.pickBy(stageStack.Resources, [ 'Type', 'AWS::Lambda::Permission' ]),
-						['Properties.Principal', 'apigateway.amazonaws.com']));
+						permission => utils.hasPermissionPrincipal(permission, 'apigateway')));
 
 		const apiMethods = _.assign({}, _.pickBy(stageStack.Resources, [ 'Type', 'AWS::ApiGateway::Method' ]));
 		const authorizers = _.assign({}, _.pickBy(stageStack.Resources, [ 'Type', 'AWS::ApiGateway::Authorizer' ]));
@@ -292,7 +292,9 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
 					'Fn::Join': [
 						'',
 						[
-							'arn:aws:execute-api:',
+							'arn:',
+							{ Ref: 'AWS::Partition' },
+							':execute-api:',
 							{ Ref: 'AWS::Region' },
 							':',
 							{ Ref: 'AWS::AccountId' },

diff --git a/lib/stackops/cwEvents.js b/lib/stackops/cwEvents.js
@@ -16,7 +16,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
 	const cwEventLambdaPermissions =
 			_.assign({},
 				_.pickBy(_.pickBy(stageStack.Resources, [ 'Type', 'AWS::Lambda::Permission' ]),
-					['Properties.Principal', 'events.amazonaws.com']));
+					permission => utils.hasPermissionPrincipal(permission, 'events')));
 
 	_.forOwn(cwEvents, (cwEvent, name) => {
 		// Reference alias as FunctionName

diff --git a/lib/stackops/snsEvents.js b/lib/stackops/snsEvents.js
@@ -67,7 +67,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
 	const snsLambdaPermissions =
 			_.assign({},
 				_.pickBy(_.pickBy(stageStack.Resources, [ 'Type', 'AWS::Lambda::Permission' ]),
-					[ 'Properties.Principal', 'sns.amazonaws.com' ]));
+					permission => utils.hasPermissionPrincipal(permission, 'sns')));
 
 	// Adjust permission to reference the function aliases
 	_.forOwn(snsLambdaPermissions, (permission, name) => {

diff --git a/lib/utils.js b/lib/utils.js
@@ -87,6 +87,23 @@ class Utils {
 		}, alias);
 	}
 
+	/**
+	 * Checks if a CF resource permission targets the given service as Principal.
+	 * @param {string} service 
+	 */
+	static hasPermissionPrincipal(permission, service) {
+		const principal = _.get(permission, 'Properties.Principal');
+		if (_.isString(principal)) {
+			return _.startsWith(principal, service);
+		} else if (_.isPlainObject(principal)) {
+			const join = principal['Fn::Join'];
+			if (join) {
+				return _.some(join[1], joinPart => _.isString(joinPart) && _.startsWith(joinPart, service));
+			}
+		}
+		return false;
+	}
+
 }
 
 module.exports = Utils;
diff --git a/test/utils.test.js b/test/utils.test.js
@@ -169,16 +169,16 @@ describe('Utils', function() {
 			expect(Utils.findAllReferences(testRoot))
 				.to.deep.equal([
 					{
-						"path": "other",
-						"ref": "Ref#4"
+						'path': 'other',
+						'ref': 'Ref#4'
 					},
 					{
-						"path": "items[1].otherTestItem.prop2",
-						"ref": "Ref#3"
+						'path': 'items[1].otherTestItem.prop2',
+						'ref': 'Ref#3'
 					},
 					{
-						"path": "items[1].otherTestItem.prop1.arrayTest[0]",
-						"ref": "Ref#2"
+						'path': 'items[1].otherTestItem.prop1.arrayTest[0]',
+						'ref': 'Ref#2'
 					}
 				]);
 		});
@@ -225,20 +225,20 @@ describe('Utils', function() {
 			expect(Utils.findAllReferences(testRoot))
 				.to.deep.equal([
 					{
-						"path": "other",
-						"ref": "Ref#4"
+						'path': 'other',
+						'ref': 'Ref#4'
 					},
 					{
-						"path": "items[1].otherTestItem.prop2",
-						"ref": "Ref#3"
+						'path': 'items[1].otherTestItem.prop2',
+						'ref': 'Ref#3'
 					},
 					{
-						"path": "items[1].otherTestItem.prop1.arrayTest[0]",
-						"ref": "Ref#2"
+						'path': 'items[1].otherTestItem.prop1.arrayTest[0]',
+						'ref': 'Ref#2'
 					},
 					{
-						"path": "items[0].testItem",
-						"ref": "Ref"
+						'path': 'items[0].testItem',
+						'ref': 'Ref'
 					}
 				]);
 		});
@@ -281,4 +281,152 @@ describe('Utils', function() {
 			});
 		});
 	});
+
+	describe('#hasPermissionPrincipal()', () => {
+		it('should work with string principals', () => {
+			const permission = {
+				'Type': 'AWS::Lambda::Permission',
+				'Properties': {
+					'FunctionName': {
+						'Fn::GetAtt': [
+							'MyLambdaLambdaFunction',
+							'Arn'
+						]
+					},
+					'Action': 'lambda:InvokeFunction',
+					'Principal': 'apigateway.amazonaws.com',
+					'SourceArn': {
+						'Fn::Join': [
+							'',
+							[
+								'arn:',
+								{
+									'Ref': 'AWS::Partition'
+								},
+								':execute-api:',
+								{
+									'Ref': 'AWS::Region'
+								},
+								':',
+								{
+									'Ref': 'AWS::AccountId'
+								},
+								':',
+								{
+									'Ref': 'ApiGatewayRestApi'
+								},
+								'/*/*'
+							]
+						]
+					}
+				}
+			};
+
+			expect(Utils.hasPermissionPrincipal(permission, 'apigateway')).to.be.true;
+		});
+
+		it('should work with constructed principals', () => {
+			const permission = {
+				'Type': 'AWS::Lambda::Permission',
+				'Properties': {
+					'FunctionName': {
+						'Fn::GetAtt': [
+							'MyLambdaLambdaFunction',
+							'Arn'
+						]
+					},
+					'Action': 'lambda:InvokeFunction',
+					'Principal': {
+						'Fn::Join': [
+							'',
+							[
+								'apigateway.',
+								{
+									'Ref': 'AWS::URLSuffix'
+								}
+							]
+						]
+					},
+					'SourceArn': {
+						'Fn::Join': [
+							'',
+							[
+								'arn:',
+								{
+									'Ref': 'AWS::Partition'
+								},
+								':execute-api:',
+								{
+									'Ref': 'AWS::Region'
+								},
+								':',
+								{
+									'Ref': 'AWS::AccountId'
+								},
+								':',
+								{
+									'Ref': 'ApiGatewayRestApi'
+								},
+								'/*/*'
+							]
+						]
+					}
+				}
+			};
+
+			expect(Utils.hasPermissionPrincipal(permission, 'apigateway')).to.be.true;
+		});
+
+		it ('should return false if the service is not matched', () => {
+			const permission = {
+				'Type': 'AWS::Lambda::Permission',
+				'Properties': {
+					'FunctionName': {
+						'Fn::GetAtt': [
+							'MyLambdaLambdaFunction',
+							'Arn'
+						]
+					},
+					'Action': 'lambda:InvokeFunction',
+					'Principal': {
+						'Fn::Join': [
+							'',
+							[
+								'apigateway.',
+								{
+									'Ref': 'AWS::URLSuffix'
+								}
+							]
+						]
+					},
+					'SourceArn': {
+						'Fn::Join': [
+							'',
+							[
+								'arn:',
+								{
+									'Ref': 'AWS::Partition'
+								},
+								':execute-api:',
+								{
+									'Ref': 'AWS::Region'
+								},
+								':',
+								{
+									'Ref': 'AWS::AccountId'
+								},
+								':',
+								{
+									'Ref': 'ApiGatewayRestApi'
+								},
+								'/*/*'
+							]
+						]
+					}
+				}
+			};
+
+			expect(Utils.hasPermissionPrincipal(permission, 'events')).to.be.false;
+		});
+	});
 });