Skip to content

Commit 672130e

Browse files
author
vikasrohit
authored
Merge pull request #343 from topcoder-platform/cf18
Cf18
2 parents 1082b48 + 8851cd6 commit 672130e

File tree

14 files changed

+459
-81
lines changed

14 files changed

+459
-81
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/milestoneTemplates/clone.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ module.exports = [
3030
(req, res, next) => {
3131
let result;
3232

33-
return models.sequelize.transaction(tx =>
33+
return models.sequelize.transaction(() =>
3434
// Find the product template
3535
models.MilestoneTemplate.findAll({
3636
where: {
@@ -48,7 +48,7 @@ module.exports = [
4848
milestone.createdBy = req.authUser.userId; // eslint-disable-line no-param-reassign
4949
milestone.updatedBy = req.authUser.userId; // eslint-disable-line no-param-reassign
5050
});
51-
return models.MilestoneTemplate.bulkCreate(newMilestoneTemplates, { transaction: tx });
51+
return models.MilestoneTemplate.bulkCreate(newMilestoneTemplates);
5252
})
5353
.then(() => { // eslint-disable-line arrow-body-style
5454
return models.MilestoneTemplate.findAll({

‎src/routes/milestoneTemplates/create.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ module.exports = [
5151
});
5252
let result;
5353

54-
return models.sequelize.transaction(tx =>
54+
return models.sequelize.transaction(() =>
5555
// Create the milestone template
56-
models.MilestoneTemplate.create(entity, { transaction: tx })
56+
models.MilestoneTemplate.create(entity)
5757
.then((createdEntity) => {
5858
// Omit deletedAt and deletedBy
5959
result = _.omit(createdEntity.toJSON(), 'deletedAt', 'deletedBy');
@@ -67,7 +67,6 @@ module.exports = [
6767
id: { $ne: result.id },
6868
order: { $gte: result.order },
6969
},
70-
transaction: tx,
7170
});
7271
}),
7372
)

‎src/routes/milestones/create.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ module.exports = [
7373
return next(apiErr);
7474
}
7575

76-
return models.sequelize.transaction(tx =>
76+
return models.sequelize.transaction(() =>
7777
// Save to DB
78-
models.Milestone.create(entity, { transaction: tx })
78+
models.Milestone.create(entity)
7979
.then((createdEntity) => {
8080
// Omit deletedAt, deletedBy
8181
result = _.omit(createdEntity.toJSON(), 'deletedAt', 'deletedBy');
@@ -88,7 +88,6 @@ module.exports = [
8888
id: { $ne: result.id },
8989
order: { $gte: result.order },
9090
},
91-
transaction: tx,
9291
});
9392
}),
9493
)

‎src/routes/milestones/delete.js‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,10 @@ module.exports = [
2929
id: req.params.milestoneId,
3030
};
3131

32-
return models.sequelize.transaction(tx =>
32+
return models.sequelize.transaction(() =>
3333
// Find the milestone
3434
models.Milestone.findOne({
3535
where,
36-
transaction: tx,
3736
})
3837
.then((milestone) => {
3938
// Not found
@@ -44,8 +43,8 @@ module.exports = [
4443
}
4544

4645
// Update the deletedBy, and soft delete
47-
return milestone.update({ deletedBy: req.authUser.userId }, { transaction: tx })
48-
.then(() => milestone.destroy({ transaction: tx }));
46+
return milestone.update({ deletedBy: req.authUser.userId })
47+
.then(() => milestone.destroy());
4948
}),
5049
)
5150
.then((deleted) => {

‎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();

0 commit comments

Comments
 (0)