Skip to content

Commit 18d9114

Browse files
HyperBrainHyperBrain
authored
Merge pull request #121 from HyperBrain/hotfix-serverless-1-27
Fixed detection of Principal for Serverless 1.27
2 parents ac7d9f9 + da18823 commit 18d9114

File tree

6 files changed

+185
-4049
lines changed

6 files changed

+185
-4049
lines changed

‎lib/stackops/apiGateway.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
204204
const apiLambdaPermissions =
205205
_.assign({},
206206
_.pickBy(_.pickBy(stageStack.Resources, [ 'Type', 'AWS::Lambda::Permission' ]),
207-
['Properties.Principal', 'apigateway.amazonaws.com']));
207+
permission => utils.hasPermissionPrincipal(permission, 'apigateway')));
208208

209209
const apiMethods = _.assign({}, _.pickBy(stageStack.Resources, [ 'Type', 'AWS::ApiGateway::Method' ]));
210210
const authorizers = _.assign({}, _.pickBy(stageStack.Resources, [ 'Type', 'AWS::ApiGateway::Authorizer' ]));
@@ -292,7 +292,9 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
292292
'Fn::Join': [
293293
'',
294294
[
295-
'arn:aws:execute-api:',
295+
'arn:',
296+
{ Ref: 'AWS::Partition' },
297+
':execute-api:',
296298
{ Ref: 'AWS::Region' },
297299
':',
298300
{ Ref: 'AWS::AccountId' },

‎lib/stackops/cwEvents.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
1616
const cwEventLambdaPermissions =
1717
_.assign({},
1818
_.pickBy(_.pickBy(stageStack.Resources, [ 'Type', 'AWS::Lambda::Permission' ]),
19-
['Properties.Principal', 'events.amazonaws.com']));
19+
permission => utils.hasPermissionPrincipal(permission, 'events')));
2020

2121
_.forOwn(cwEvents, (cwEvent, name) => {
2222
// Reference alias as FunctionName

‎lib/stackops/snsEvents.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
6767
const snsLambdaPermissions =
6868
_.assign({},
6969
_.pickBy(_.pickBy(stageStack.Resources, [ 'Type', 'AWS::Lambda::Permission' ]),
70-
[ 'Properties.Principal', 'sns.amazonaws.com' ]));
70+
permission => utils.hasPermissionPrincipal(permission, 'sns')));
7171

7272
// Adjust permission to reference the function aliases
7373
_.forOwn(snsLambdaPermissions, (permission, name) => {

‎lib/utils.js

+17
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,23 @@ class Utils {
8787
}, alias);
8888
}
8989

90+
/**
91+
* Checks if a CF resource permission targets the given service as Principal.
92+
* @param {string} service
93+
*/
94+
static hasPermissionPrincipal(permission, service) {
95+
const principal = _.get(permission, 'Properties.Principal');
96+
if (_.isString(principal)) {
97+
return _.startsWith(principal, service);
98+
} else if (_.isPlainObject(principal)) {
99+
const join = principal['Fn::Join'];
100+
if (join) {
101+
return _.some(join[1], joinPart => _.isString(joinPart) && _.startsWith(joinPart, service));
102+
}
103+
}
104+
return false;
105+
}
106+
90107
}
91108

92109
module.exports = Utils;

‎test/utils.test.js

+162-14
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,16 @@ describe('Utils', function() {
169169
expect(Utils.findAllReferences(testRoot))
170170
.to.deep.equal([
171171
{
172-
"path": "other",
173-
"ref": "Ref#4"
172+
'path': 'other',
173+
'ref': 'Ref#4'
174174
},
175175
{
176-
"path": "items[1].otherTestItem.prop2",
177-
"ref": "Ref#3"
176+
'path': 'items[1].otherTestItem.prop2',
177+
'ref': 'Ref#3'
178178
},
179179
{
180-
"path": "items[1].otherTestItem.prop1.arrayTest[0]",
181-
"ref": "Ref#2"
180+
'path': 'items[1].otherTestItem.prop1.arrayTest[0]',
181+
'ref': 'Ref#2'
182182
}
183183
]);
184184
});
@@ -225,20 +225,20 @@ describe('Utils', function() {
225225
expect(Utils.findAllReferences(testRoot))
226226
.to.deep.equal([
227227
{
228-
"path": "other",
229-
"ref": "Ref#4"
228+
'path': 'other',
229+
'ref': 'Ref#4'
230230
},
231231
{
232-
"path": "items[1].otherTestItem.prop2",
233-
"ref": "Ref#3"
232+
'path': 'items[1].otherTestItem.prop2',
233+
'ref': 'Ref#3'
234234
},
235235
{
236-
"path": "items[1].otherTestItem.prop1.arrayTest[0]",
237-
"ref": "Ref#2"
236+
'path': 'items[1].otherTestItem.prop1.arrayTest[0]',
237+
'ref': 'Ref#2'
238238
},
239239
{
240-
"path": "items[0].testItem",
241-
"ref": "Ref"
240+
'path': 'items[0].testItem',
241+
'ref': 'Ref'
242242
}
243243
]);
244244
});
@@ -281,4 +281,152 @@ describe('Utils', function() {
281281
});
282282
});
283283
});
284+
285+
describe('#hasPermissionPrincipal()', () => {
286+
it('should work with string principals', () => {
287+
const permission = {
288+
'Type': 'AWS::Lambda::Permission',
289+
'Properties': {
290+
'FunctionName': {
291+
'Fn::GetAtt': [
292+
'MyLambdaLambdaFunction',
293+
'Arn'
294+
]
295+
},
296+
'Action': 'lambda:InvokeFunction',
297+
'Principal': 'apigateway.amazonaws.com',
298+
'SourceArn': {
299+
'Fn::Join': [
300+
'',
301+
[
302+
'arn:',
303+
{
304+
'Ref': 'AWS::Partition'
305+
},
306+
':execute-api:',
307+
{
308+
'Ref': 'AWS::Region'
309+
},
310+
':',
311+
{
312+
'Ref': 'AWS::AccountId'
313+
},
314+
':',
315+
{
316+
'Ref': 'ApiGatewayRestApi'
317+
},
318+
'/*/*'
319+
]
320+
]
321+
}
322+
}
323+
};
324+
325+
expect(Utils.hasPermissionPrincipal(permission, 'apigateway')).to.be.true;
326+
});
327+
328+
it('should work with constructed principals', () => {
329+
const permission = {
330+
'Type': 'AWS::Lambda::Permission',
331+
'Properties': {
332+
'FunctionName': {
333+
'Fn::GetAtt': [
334+
'MyLambdaLambdaFunction',
335+
'Arn'
336+
]
337+
},
338+
'Action': 'lambda:InvokeFunction',
339+
'Principal': {
340+
'Fn::Join': [
341+
'',
342+
[
343+
'apigateway.',
344+
{
345+
'Ref': 'AWS::URLSuffix'
346+
}
347+
]
348+
]
349+
},
350+
'SourceArn': {
351+
'Fn::Join': [
352+
'',
353+
[
354+
'arn:',
355+
{
356+
'Ref': 'AWS::Partition'
357+
},
358+
':execute-api:',
359+
{
360+
'Ref': 'AWS::Region'
361+
},
362+
':',
363+
{
364+
'Ref': 'AWS::AccountId'
365+
},
366+
':',
367+
{
368+
'Ref': 'ApiGatewayRestApi'
369+
},
370+
'/*/*'
371+
]
372+
]
373+
}
374+
}
375+
};
376+
377+
expect(Utils.hasPermissionPrincipal(permission, 'apigateway')).to.be.true;
378+
});
379+
380+
it ('should return false if the service is not matched', () => {
381+
const permission = {
382+
'Type': 'AWS::Lambda::Permission',
383+
'Properties': {
384+
'FunctionName': {
385+
'Fn::GetAtt': [
386+
'MyLambdaLambdaFunction',
387+
'Arn'
388+
]
389+
},
390+
'Action': 'lambda:InvokeFunction',
391+
'Principal': {
392+
'Fn::Join': [
393+
'',
394+
[
395+
'apigateway.',
396+
{
397+
'Ref': 'AWS::URLSuffix'
398+
}
399+
]
400+
]
401+
},
402+
'SourceArn': {
403+
'Fn::Join': [
404+
'',
405+
[
406+
'arn:',
407+
{
408+
'Ref': 'AWS::Partition'
409+
},
410+
':execute-api:',
411+
{
412+
'Ref': 'AWS::Region'
413+
},
414+
':',
415+
{
416+
'Ref': 'AWS::AccountId'
417+
},
418+
':',
419+
{
420+
'Ref': 'ApiGatewayRestApi'
421+
},
422+
'/*/*'
423+
]
424+
]
425+
}
426+
}
427+
};
428+
429+
expect(Utils.hasPermissionPrincipal(permission, 'events')).to.be.false;
430+
});
431+
});
284432
});

0 commit comments

Comments
 (0)