Skip to content

Commit 8df56f6

Browse files
maxceemmaxceem
authored
Merge pull request #336 from tc-mfikria/cf18-332
Fix copilotAndAbove permission to check that users is a member of the project #332
2 parents d9c94ef + 0c6ca3b commit 8df56f6

File tree

7 files changed

+442
-60
lines changed

7 files changed

+442
-60
lines changed

‎src/permissions/copilotAndAbove.js

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,45 @@
1+
import _ from 'lodash';
12
import util from '../util';
2-
import { MANAGER_ROLES, USER_ROLE } from '../constants';
3+
import {
4+
PROJECT_MEMBER_ROLE,
5+
ADMIN_ROLES,
6+
} from '../constants';
7+
import models from '../models';
38

49

510
/**
6-
* Permission to alloow copilot and above roles to perform certain operations
11+
* Permission to allow copilot and above roles to perform certain operations
12+
* - User with Topcoder admins roles should be able to perform the operations.
13+
* - Project members with copilot and manager Project roles should be also able to perform the operations.
714
* @param {Object} req the express request instance
815
* @return {Promise} returns a promise
916
*/
1017
module.exports = req => new Promise((resolve, reject) => {
11-
const hasAccess = util.hasRoles(req, [...MANAGER_ROLES, USER_ROLE.COPILOT]);
18+
const projectId = _.parseInt(req.params.projectId);
19+
const isAdmin = util.hasRoles(req, ADMIN_ROLES);
1220

13-
if (!hasAccess) {
14-
return reject(new Error('You do not have permissions to perform this action'));
21+
if (isAdmin) {
22+
return resolve(true);
1523
}
1624

17-
return resolve(true);
25+
return models.ProjectMember.getActiveProjectMembers(projectId)
26+
.then((members) => {
27+
req.context = req.context || {};
28+
req.context.currentProjectMembers = members;
29+
const validMemberProjectRoles = [
30+
PROJECT_MEMBER_ROLE.MANAGER,
31+
PROJECT_MEMBER_ROLE.COPILOT,
32+
];
33+
// check if the copilot or manager has access to this project
34+
const isMember = _.some(
35+
members,
36+
m => m.userId === req.authUser.userId && validMemberProjectRoles.includes(m.role),
37+
);
38+
39+
if (!isMember) {
40+
// the copilot or manager is not a registered project member
41+
return reject(new Error('You do not have permissions to perform this action'));
42+
}
43+
return resolve(true);
44+
});
1845
});

‎src/routes/phaseProducts/create.spec.js

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ describe('Phase Products', () => {
177177
request(server)
178178
.post(`/v4/projects/99999/phases/${phaseId}/products`)
179179
.set({
180-
Authorization: `Bearer ${testUtil.jwts.manager}`,
180+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
181181
})
182182
.send({ param: body })
183183
.expect('Content-Type', /json/)
@@ -188,7 +188,7 @@ describe('Phase Products', () => {
188188
request(server)
189189
.post(`/v4/projects/${projectId}/phases/99999/products`)
190190
.set({
191-
Authorization: `Bearer ${testUtil.jwts.manager}`,
191+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
192192
})
193193
.send({ param: body })
194194
.expect('Content-Type', /json/)
@@ -220,6 +220,68 @@ describe('Phase Products', () => {
220220
});
221221
});
222222

223+
it('should return 201 if requested by admin', (done) => {
224+
request(server)
225+
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
226+
.set({
227+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
228+
})
229+
.send({ param: body })
230+
.expect('Content-Type', /json/)
231+
.expect(201)
232+
.end(done);
233+
});
234+
235+
it('should return 201 if requested by manager which is a member', (done) => {
236+
models.ProjectMember.create({
237+
id: 3,
238+
userId: testUtil.userIds.manager,
239+
projectId,
240+
role: 'manager',
241+
isPrimary: false,
242+
createdBy: 1,
243+
updatedBy: 1,
244+
}).then(() => {
245+
request(server)
246+
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
247+
.set({
248+
Authorization: `Bearer ${testUtil.jwts.manager}`,
249+
})
250+
.send({ param: body })
251+
.expect('Content-Type', /json/)
252+
.expect(201)
253+
.end(done);
254+
});
255+
});
256+
257+
it('should return 403 if requested by manager which is not a member', (done) => {
258+
request(server)
259+
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
260+
.set({
261+
Authorization: `Bearer ${testUtil.jwts.manager}`,
262+
})
263+
.send({ param: body })
264+
.expect('Content-Type', /json/)
265+
.expect(403)
266+
.end(done);
267+
});
268+
269+
it('should return 403 if requested by non-member copilot', (done) => {
270+
models.ProjectMember.destroy({
271+
where: { userId: testUtil.userIds.copilot, projectId },
272+
}).then(() => {
273+
request(server)
274+
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
275+
.set({
276+
Authorization: `Bearer ${testUtil.jwts.copilot}`,
277+
})
278+
.send({ param: body })
279+
.expect('Content-Type', /json/)
280+
.expect(403)
281+
.end(done);
282+
});
283+
});
284+
223285
describe('Bus api', () => {
224286
let createEventSpy;
225287
const sandbox = sinon.sandbox.create();

‎src/routes/phaseProducts/delete.spec.js

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ describe('Phase Products', () => {
156156
request(server)
157157
.delete(`/v4/projects/999/phases/${phaseId}/products/${productId}`)
158158
.set({
159-
Authorization: `Bearer ${testUtil.jwts.manager}`,
159+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
160160
})
161161
.expect('Content-Type', /json/)
162162
.expect(404, done);
@@ -166,7 +166,7 @@ describe('Phase Products', () => {
166166
request(server)
167167
.delete(`/v4/projects/${projectId}/phases/99999/products/${productId}`)
168168
.set({
169-
Authorization: `Bearer ${testUtil.jwts.manager}`,
169+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
170170
})
171171
.expect('Content-Type', /json/)
172172
.expect(404, done);
@@ -176,7 +176,7 @@ describe('Phase Products', () => {
176176
request(server)
177177
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/99999`)
178178
.set({
179-
Authorization: `Bearer ${testUtil.jwts.manager}`,
179+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
180180
})
181181
.expect('Content-Type', /json/)
182182
.expect(404, done);
@@ -192,6 +192,60 @@ describe('Phase Products', () => {
192192
.end(err => expectAfterDelete(projectId, phaseId, productId, err, done));
193193
});
194194

195+
it('should return 204 if requested by admin', (done) => {
196+
request(server)
197+
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
198+
.set({
199+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
200+
})
201+
.expect(204)
202+
.end(done);
203+
});
204+
205+
it('should return 204 if requested by manager which is a member', (done) => {
206+
models.ProjectMember.create({
207+
id: 3,
208+
userId: testUtil.userIds.manager,
209+
projectId,
210+
role: 'manager',
211+
isPrimary: false,
212+
createdBy: 1,
213+
updatedBy: 1,
214+
}).then(() => {
215+
request(server)
216+
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
217+
.set({
218+
Authorization: `Bearer ${testUtil.jwts.manager}`,
219+
})
220+
.expect(204)
221+
.end(done);
222+
});
223+
});
224+
225+
it('should return 403 if requested by manager which is not a member', (done) => {
226+
request(server)
227+
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
228+
.set({
229+
Authorization: `Bearer ${testUtil.jwts.manager}`,
230+
})
231+
.expect(403)
232+
.end(done);
233+
});
234+
235+
it('should return 403 if requested by non-member copilot', (done) => {
236+
models.ProjectMember.destroy({
237+
where: { userId: testUtil.userIds.copilot, projectId },
238+
}).then(() => {
239+
request(server)
240+
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
241+
.set({
242+
Authorization: `Bearer ${testUtil.jwts.copilot}`,
243+
})
244+
.expect(403)
245+
.end(done);
246+
});
247+
});
248+
195249
describe('Bus api', () => {
196250
let createEventSpy;
197251
const sandbox = sinon.sandbox.create();

‎src/routes/phaseProducts/update.spec.js

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('Phase Products', () => {
5151
lastName: 'lName',
5252
email: 'some@abc.com',
5353
};
54-
before((done) => {
54+
beforeEach((done) => {
5555
// mocks
5656
testUtil.clearDb()
5757
.then(() => {
@@ -144,7 +144,7 @@ describe('Phase Products', () => {
144144
request(server)
145145
.patch(`/v4/projects/999/phases/${phaseId}/products/${productId}`)
146146
.set({
147-
Authorization: `Bearer ${testUtil.jwts.manager}`,
147+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
148148
})
149149
.send({ param: updateBody })
150150
.expect('Content-Type', /json/)
@@ -155,7 +155,7 @@ describe('Phase Products', () => {
155155
request(server)
156156
.patch(`/v4/projects/${projectId}/phases/99999/products/${productId}`)
157157
.set({
158-
Authorization: `Bearer ${testUtil.jwts.manager}`,
158+
Authorization: `Bearer ${testUtil.jwts.copilot}`,
159159
})
160160
.send({ param: updateBody })
161161
.expect('Content-Type', /json/)
@@ -166,7 +166,7 @@ describe('Phase Products', () => {
166166
request(server)
167167
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/99999`)
168168
.set({
169-
Authorization: `Bearer ${testUtil.jwts.manager}`,
169+
Authorization: `Bearer ${testUtil.jwts.copilot}`,
170170
})
171171
.send({ param: updateBody })
172172
.expect('Content-Type', /json/)
@@ -177,7 +177,7 @@ describe('Phase Products', () => {
177177
request(server)
178178
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/99999`)
179179
.set({
180-
Authorization: `Bearer ${testUtil.jwts.manager}`,
180+
Authorization: `Bearer ${testUtil.jwts.copilot}`,
181181
})
182182
.send({
183183
param: {
@@ -214,6 +214,68 @@ describe('Phase Products', () => {
214214
});
215215
});
216216

217+
it('should return 200 if requested by admin', (done) => {
218+
request(server)
219+
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
220+
.set({
221+
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
222+
})
223+
.send({ param: updateBody })
224+
.expect('Content-Type', /json/)
225+
.expect(200)
226+
.end(done);
227+
});
228+
229+
it('should return 200 if requested by manager which is a member', (done) => {
230+
models.ProjectMember.create({
231+
id: 3,
232+
userId: testUtil.userIds.manager,
233+
projectId,
234+
role: 'manager',
235+
isPrimary: false,
236+
createdBy: 1,
237+
updatedBy: 1,
238+
}).then(() => {
239+
request(server)
240+
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
241+
.set({
242+
Authorization: `Bearer ${testUtil.jwts.manager}`,
243+
})
244+
.send({ param: updateBody })
245+
.expect('Content-Type', /json/)
246+
.expect(200)
247+
.end(done);
248+
});
249+
});
250+
251+
it('should return 403 if requested by manager which is not a member', (done) => {
252+
request(server)
253+
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
254+
.set({
255+
Authorization: `Bearer ${testUtil.jwts.manager}`,
256+
})
257+
.send({ param: updateBody })
258+
.expect('Content-Type', /json/)
259+
.expect(403)
260+
.end(done);
261+
});
262+
263+
it('should return 403 if requested by non-member copilot', (done) => {
264+
models.ProjectMember.destroy({
265+
where: { userId: testUtil.userIds.copilot, projectId },
266+
}).then(() => {
267+
request(server)
268+
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
269+
.set({
270+
Authorization: `Bearer ${testUtil.jwts.copilot}`,
271+
})
272+
.send({ param: updateBody })
273+
.expect('Content-Type', /json/)
274+
.expect(403)
275+
.end(done);
276+
});
277+
});
278+
217279
describe('Bus api', () => {
218280
let createEventSpy;
219281
const sandbox = sinon.sandbox.create();

0 commit comments

Comments
 (0)