Skip to content

Fix copilotAndAbove permission to check that users is a member of the project #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from Jul 9, 2019
Merged

Fix copilotAndAbove permission to check that users is a member of the project #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d33094d
Fix copilotAndAbove permission to check that users is a member of the…
mfikria Jul 6, 2019
ff7bb77
update allowed roles
mfikria Jul 6, 2019
1720640
update unit test
mfikria Jul 6, 2019
6dc2075
add case for unit test "Users with Project manager roles CANNOT do ac…
mfikria Jul 7, 2019
0c6ca3b
update loginc for validating project member roles
mfikria Jul 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions src/permissions/copilotAndAbove.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,45 @@
import _ from 'lodash';
import util from '../util';
import { MANAGER_ROLES, USER_ROLE } from '../constants';
import {
PROJECT_MEMBER_ROLE,
ADMIN_ROLES,
} from '../constants';
import models from '../models';


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

if (!hasAccess) {
return reject(new Error('You do not have permissions to perform this action'));
if (isAdmin) {
return resolve(true);
}

return resolve(true);
return models.ProjectMember.getActiveProjectMembers(projectId)
.then((members) => {
req.context = req.context || {};
req.context.currentProjectMembers = members;
const validMemberProjectRoles = [
PROJECT_MEMBER_ROLE.MANAGER,
PROJECT_MEMBER_ROLE.COPILOT,
];
// check if the copilot or manager has access to this project
const isMember = _.some(
members,
m => m.userId === req.authUser.userId && validMemberProjectRoles.includes(m.role),
);

if (!isMember) {
// the copilot or manager is not a registered project member
return reject(new Error('You do not have permissions to perform this action'));
}
return resolve(true);
});
});
66 changes: 64 additions & 2 deletions src/routes/phaseProducts/create.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ describe('Phase Products', () => {
request(server)
.post(`/v4/projects/99999/phases/${phaseId}/products`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.send({ param: body })
.expect('Content-Type', /json/)
Expand All @@ -188,7 +188,7 @@ describe('Phase Products', () => {
request(server)
.post(`/v4/projects/${projectId}/phases/99999/products`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.send({ param: body })
.expect('Content-Type', /json/)
Expand Down Expand Up @@ -220,6 +220,68 @@ describe('Phase Products', () => {
});
});

it('should return 201 if requested by admin', (done) => {
request(server)
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
.set({
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.send({ param: body })
.expect('Content-Type', /json/)
.expect(201)
.end(done);
});

it('should return 201 if requested by manager which is a member', (done) => {
models.ProjectMember.create({
id: 3,
userId: testUtil.userIds.manager,
projectId,
role: 'manager',
isPrimary: false,
createdBy: 1,
updatedBy: 1,
}).then(() => {
request(server)
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.send({ param: body })
.expect('Content-Type', /json/)
.expect(201)
.end(done);
});
});

it('should return 403 if requested by manager which is not a member', (done) => {
request(server)
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.send({ param: body })
.expect('Content-Type', /json/)
.expect(403)
.end(done);
});

it('should return 403 if requested by non-member copilot', (done) => {
models.ProjectMember.destroy({
where: { userId: testUtil.userIds.copilot, projectId },
}).then(() => {
request(server)
.post(`/v4/projects/${projectId}/phases/${phaseId}/products`)
.set({
Authorization: `Bearer ${testUtil.jwts.copilot}`,
})
.send({ param: body })
.expect('Content-Type', /json/)
.expect(403)
.end(done);
});
});

describe('Bus api', () => {
let createEventSpy;
const sandbox = sinon.sandbox.create();
Expand Down
60 changes: 57 additions & 3 deletions src/routes/phaseProducts/delete.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ describe('Phase Products', () => {
request(server)
.delete(`/v4/projects/999/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.expect('Content-Type', /json/)
.expect(404, done);
Expand All @@ -166,7 +166,7 @@ describe('Phase Products', () => {
request(server)
.delete(`/v4/projects/${projectId}/phases/99999/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.expect('Content-Type', /json/)
.expect(404, done);
Expand All @@ -176,7 +176,7 @@ describe('Phase Products', () => {
request(server)
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/99999`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.expect('Content-Type', /json/)
.expect(404, done);
Expand All @@ -192,6 +192,60 @@ describe('Phase Products', () => {
.end(err => expectAfterDelete(projectId, phaseId, productId, err, done));
});

it('should return 204 if requested by admin', (done) => {
request(server)
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.expect(204)
.end(done);
});

it('should return 204 if requested by manager which is a member', (done) => {
models.ProjectMember.create({
id: 3,
userId: testUtil.userIds.manager,
projectId,
role: 'manager',
isPrimary: false,
createdBy: 1,
updatedBy: 1,
}).then(() => {
request(server)
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.expect(204)
.end(done);
});
});

it('should return 403 if requested by manager which is not a member', (done) => {
request(server)
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.expect(403)
.end(done);
});

it('should return 403 if requested by non-member copilot', (done) => {
models.ProjectMember.destroy({
where: { userId: testUtil.userIds.copilot, projectId },
}).then(() => {
request(server)
.delete(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.copilot}`,
})
.expect(403)
.end(done);
});
});

describe('Bus api', () => {
let createEventSpy;
const sandbox = sinon.sandbox.create();
Expand Down
72 changes: 67 additions & 5 deletions src/routes/phaseProducts/update.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('Phase Products', () => {
lastName: 'lName',
email: 'some@abc.com',
};
before((done) => {
beforeEach((done) => {
// mocks
testUtil.clearDb()
.then(() => {
Expand Down Expand Up @@ -144,7 +144,7 @@ describe('Phase Products', () => {
request(server)
.patch(`/v4/projects/999/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.send({ param: updateBody })
.expect('Content-Type', /json/)
Expand All @@ -155,7 +155,7 @@ describe('Phase Products', () => {
request(server)
.patch(`/v4/projects/${projectId}/phases/99999/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.copilot}`,
})
.send({ param: updateBody })
.expect('Content-Type', /json/)
Expand All @@ -166,7 +166,7 @@ describe('Phase Products', () => {
request(server)
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/99999`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.copilot}`,
})
.send({ param: updateBody })
.expect('Content-Type', /json/)
Expand All @@ -177,7 +177,7 @@ describe('Phase Products', () => {
request(server)
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/99999`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
Authorization: `Bearer ${testUtil.jwts.copilot}`,
})
.send({
param: {
Expand Down Expand Up @@ -214,6 +214,68 @@ describe('Phase Products', () => {
});
});

it('should return 200 if requested by admin', (done) => {
request(server)
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.connectAdmin}`,
})
.send({ param: updateBody })
.expect('Content-Type', /json/)
.expect(200)
.end(done);
});

it('should return 200 if requested by manager which is a member', (done) => {
models.ProjectMember.create({
id: 3,
userId: testUtil.userIds.manager,
projectId,
role: 'manager',
isPrimary: false,
createdBy: 1,
updatedBy: 1,
}).then(() => {
request(server)
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.send({ param: updateBody })
.expect('Content-Type', /json/)
.expect(200)
.end(done);
});
});

it('should return 403 if requested by manager which is not a member', (done) => {
request(server)
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.send({ param: updateBody })
.expect('Content-Type', /json/)
.expect(403)
.end(done);
});

it('should return 403 if requested by non-member copilot', (done) => {
models.ProjectMember.destroy({
where: { userId: testUtil.userIds.copilot, projectId },
}).then(() => {
request(server)
.patch(`/v4/projects/${projectId}/phases/${phaseId}/products/${productId}`)
.set({
Authorization: `Bearer ${testUtil.jwts.copilot}`,
})
.send({ param: updateBody })
.expect('Content-Type', /json/)
.expect(403)
.end(done);
});
});

describe('Bus api', () => {
let createEventSpy;
const sandbox = sinon.sandbox.create();
Expand Down
Loading