Skip to content

Commit f99d315

Browse files
kenleytomlindaniel-cottone
kenleytomlin
authored and
daniel-cottone
committed
Add support for complex policies with wildcards (dherault#373)
1 parent a7ed5e8 commit f99d315

File tree

5 files changed

+296
-3
lines changed

5 files changed

+296
-3
lines changed

‎src/authCanExecuteResource.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
const authMatchPolicyResource = require('./authMatchPolicyResource');
2+
3+
module.exports = (policy, resource) => {
4+
const Statement = policy.Statement;
5+
6+
return Statement.some(statement => {
7+
if (Array.isArray(statement.Resource)) {
8+
return statement.Effect === 'Allow'
9+
&& statement.Resource.some(policyResource => (
10+
authMatchPolicyResource(policyResource, resource)
11+
));
12+
}
13+
14+
return statement.Effect === 'Allow'
15+
&& authMatchPolicyResource(statement.Resource, resource);
16+
});
17+
};

‎src/authMatchPolicyResource.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
module.exports = (policyResource, resource) => {
2+
if (policyResource === resource) {
3+
return true;
4+
}
5+
else if (policyResource.includes('*')) {
6+
//Policy contains a wildcard resource
7+
const splitPolicyResource = policyResource.split(':');
8+
const splitResource = resource.split(':');
9+
//These variables contain api id, stage, method and the path
10+
//for the requested resource and the resource defined in the policy
11+
const splitPolicyResourceApi = splitPolicyResource[5].split('/');
12+
const splitResourceApi = splitResource[5].split('/');
13+
14+
return splitPolicyResourceApi.every((resourceFragment, index) => {
15+
if (splitResourceApi.length >= index + 1) {
16+
return (splitResourceApi[index] === resourceFragment || resourceFragment === '*');
17+
}
18+
//The last position in the policy resource is a '*' it matches all
19+
//following resource fragments
20+
21+
return splitPolicyResourceApi[splitPolicyResourceApi.length - 1] === '*';
22+
});
23+
}
24+
25+
return false;
26+
};

‎src/createAuthScheme.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const functionHelper = require('./functionHelper');
77
const debugLog = require('./debugLog');
88
const utils = require('./utils');
99
const _ = require('lodash');
10+
const authCanExecuteResource = require('./authCanExecuteResource');
1011

1112
module.exports = function createAuthScheme(authFun, authorizerOptions, funName, endpointPath, options, serverlessLog, servicePath, serverless) {
1213
const authFunName = authorizerOptions.name;
@@ -96,14 +97,14 @@ module.exports = function createAuthScheme(authFun, authorizerOptions, funName,
9697
return reply(Boom.forbidden('No principalId set on the Response'));
9798
}
9899

99-
serverlessLog(`Authorization function returned a successful response: (λ: ${authFunName})`, policy);
100-
101-
if (policy.policyDocument.Statement[0].Effect === 'Deny') {
100+
if (!authCanExecuteResource(policy.policyDocument, event.methodArn)) {
102101
serverlessLog(`Authorization response didn't authorize user to access resource: (λ: ${authFunName})`, err);
103102

104103
return reply(Boom.forbidden('User is not authorized to access this resource'));
105104
}
106105

106+
serverlessLog(`Authorization function returned a successful response: (λ: ${authFunName})`, policy);
107+
107108
// Set the credentials for the rest of the pipeline
108109
return reply.continue({ credentials: { user: policy.principalId, context: policy.context } });
109110
};

‎test/unit/authCanExecuteResourceTest.js‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/* global describe context it */
2+
3+
'use strict';
4+
5+
const chai = require('chai');
6+
const dirtyChai = require('dirty-chai');
7+
const authCanExecuteResource = require('../../src/authCanExecuteResource');
8+
9+
const expect = chai.expect;
10+
chai.use(dirtyChai);
11+
12+
describe('authCanExecuteResource', () => {
13+
context('when the policy has one Statement in an array', () => {
14+
const setup = (Effect, Resource) => ({
15+
Statement: [
16+
{
17+
Effect,
18+
Resource,
19+
},
20+
],
21+
});
22+
const resource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs';
23+
24+
context('when the Resource is in an Allow statement', () => {
25+
context('and the Resource is an array', () => {
26+
it('returns true', () => {
27+
const policy = setup(
28+
'Allow',
29+
[resource]
30+
);
31+
32+
const canExecute = authCanExecuteResource(policy, resource);
33+
expect(canExecute).to.eq(true);
34+
});
35+
});
36+
37+
it('returns true', () => {
38+
const policy = setup(
39+
'Allow',
40+
resource
41+
);
42+
43+
const canExecute = authCanExecuteResource(policy, resource);
44+
expect(canExecute).to.eq(true);
45+
});
46+
});
47+
48+
context('when the Resource is in a Deny statement', () => {
49+
context('and Resource is an array', () => {
50+
it('returns true', () => {
51+
const policy = setup(
52+
'Deny',
53+
[resource]
54+
);
55+
56+
const canExecute = authCanExecuteResource(policy, resource);
57+
expect(canExecute).to.eq(false);
58+
});
59+
});
60+
it('returns false', () => {
61+
const policy = setup(
62+
'Deny',
63+
resource
64+
);
65+
66+
const canExecute = authCanExecuteResource(policy, resource);
67+
expect(canExecute).to.eq(false);
68+
});
69+
});
70+
});
71+
72+
context('when the policy has multiple Statements', () => {
73+
const setup = statements => (
74+
{
75+
Statement: statements.map((statement) => (
76+
{
77+
Effect: statement.Effect,
78+
Resource: statement.Resource,
79+
}
80+
)),
81+
}
82+
);
83+
const resourceOne = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs';
84+
const resourceTwo = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dogs';
85+
86+
context('when the Resource is in an Allow statement', () => {
87+
context('and the Resource is an array', () => {
88+
it('returns true', () => {
89+
const policy = setup(
90+
[{
91+
Effect: 'Allow',
92+
Resource: [resourceOne],
93+
},
94+
{
95+
Effect: 'Deny',
96+
Resource: [resourceTwo],
97+
}]
98+
);
99+
100+
const canExecute = authCanExecuteResource(policy, resourceOne);
101+
expect(canExecute).to.eq(true);
102+
});
103+
});
104+
105+
it('returns true', () => {
106+
const policy = setup(
107+
[{
108+
Effect: 'Allow',
109+
Resource: resourceOne,
110+
}],
111+
[{
112+
Effect: 'Deny',
113+
Resource: resourceTwo,
114+
}]
115+
);
116+
const canExecute = authCanExecuteResource(policy, resourceOne);
117+
expect(canExecute).to.eq(true);
118+
});
119+
});
120+
121+
context('when the resource is in a Deny statement', () => {
122+
context('and the Resource is an array', () => {
123+
it('returns true', () => {
124+
const policy = setup(
125+
[{
126+
Effect: 'Allow',
127+
Resource: [resourceOne],
128+
},
129+
{
130+
Effect: 'Deny',
131+
Resource: [resourceTwo],
132+
}]
133+
);
134+
135+
const canExecute = authCanExecuteResource(policy, resourceTwo);
136+
expect(canExecute).to.eq(false);
137+
});
138+
});
139+
it('returns false', () => {
140+
const policy = setup(
141+
[{
142+
Effect: 'Allow',
143+
Resource: resourceOne,
144+
}],
145+
[{
146+
Effect: 'Deny',
147+
Resource: resourceTwo,
148+
}]
149+
);
150+
151+
const canExecute = authCanExecuteResource(policy, resourceTwo);
152+
expect(canExecute).to.eq(false);
153+
});
154+
});
155+
});
156+
});

‎test/unit/authMatchPolicyResourceTest.js‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/* global describe context it */
2+
3+
'use strict';
4+
5+
const chai = require('chai');
6+
const dirtyChai = require('dirty-chai');
7+
const authMatchPolicyResource = require('../../src/authMatchPolicyResource');
8+
9+
const expect = chai.expect;
10+
chai.use(dirtyChai);
11+
12+
describe('authMatchPolicyResource', () => {
13+
context('when resource has no wildcards', () => {
14+
const resource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs';
15+
context('and the resource matches', () => {
16+
it('returns true', () => {
17+
expect(
18+
authMatchPolicyResource(resource, resource)
19+
).to.eq(true);
20+
});
21+
});
22+
context('when the resource has one wildcard to match everything', () => {
23+
const wildcardResource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/*';
24+
it('returns true', () => {
25+
expect(
26+
authMatchPolicyResource(wildcardResource, resource)
27+
).to.eq(true);
28+
});
29+
});
30+
context('when the resource has wildcards', () => {
31+
const wildcardResource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/*';
32+
context('and it matches', () => {
33+
it('returns true', () => {
34+
expect(
35+
authMatchPolicyResource(wildcardResource, resource)
36+
).to.eq(true);
37+
});
38+
});
39+
context('and it does not match', () => {
40+
it('returns false', () => {
41+
const resource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/PUT/dinosaurs';
42+
43+
expect(
44+
authMatchPolicyResource(wildcardResource, resource)
45+
).to.eq(false);
46+
});
47+
});
48+
context('when the resource has multiple wildcards', () => {
49+
const wildcardResource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/*/*/stats';
50+
51+
context('and the wildcard is between two fragments', () => {
52+
const wildcardResource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/*/dinosaurs/*';
53+
context('and it matches', () => {
54+
it('returns true', () => {
55+
const resource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs/stats';
56+
57+
expect(
58+
authMatchPolicyResource(wildcardResource, resource)
59+
).to.eq(true);
60+
});
61+
});
62+
context('and it does not match', () => {
63+
it('returns false', () => {
64+
const resource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/cats/stats';
65+
66+
expect(
67+
authMatchPolicyResource(wildcardResource, resource)
68+
).to.eq(false);
69+
});
70+
});
71+
});
72+
context('and it matches', () => {
73+
it('returns true', () => {
74+
const resource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/GET/dinosaurs/stats';
75+
76+
expect(
77+
authMatchPolicyResource(wildcardResource, resource)
78+
).to.eq(true);
79+
});
80+
});
81+
context('and it does not match', () => {
82+
it('returns false', () => {
83+
const resource = 'arn:aws:execute-api:eu-west-1:random-account-id:random-api-id/development/PUT/dinosaurs/xx';
84+
85+
expect(
86+
authMatchPolicyResource(wildcardResource, resource)
87+
).to.eq(false);
88+
});
89+
});
90+
});
91+
});
92+
});
93+
});

0 commit comments

Comments
 (0)