diff --git a/package.json b/package.json
index e6a0618f..ea0e6adb 100644
--- a/package.json
+++ b/package.json
@@ -30,7 +30,7 @@
"local:init": "npm run reset:all && npm run data:import",
"babel-node-script": "node --require dotenv/config --require babel-core/register",
"generate:doc:permissions": "npm run babel-node-script -- scripts/permissions-doc",
- "generate:doc:permissions:dev": "nodemon --watch scripts/permissions-doc --watch src --ext js,jsx,hbs --exec --exec \"npm run babel-node-script scripts/permissions-doc\""
+ "generate:doc:permissions:dev": "nodemon --watch scripts/permissions-doc --watch src --ext js,jsx,hbs --exec \"npm run babel-node-script -- scripts/permissions-doc\""
},
"repository": {
"type": "git",
diff --git a/src/permissions/constants.js b/src/permissions/constants.js
index 029b1959..1e4b7172 100644
--- a/src/permissions/constants.js
+++ b/src/permissions/constants.js
@@ -53,44 +53,73 @@ import {
M2M_SCOPES,
} from '../constants';
+/**
+ * All Project Roles
+ */
const PROJECT_ROLES_ALL = _.values(PROJECT_MEMBER_ROLE);
+
+/**
+ * "Management Level" Project Roles
+ */
const PROJECT_ROLES_MANAGEMENT = _.difference(PROJECT_ROLES_ALL, [
PROJECT_MEMBER_ROLE.COPILOT,
PROJECT_MEMBER_ROLE.CUSTOMER,
PROJECT_MEMBER_ROLE.OBSERVER,
]);
+/**
+ * This is a special constant to indicate that all project members or any logged-in user
+ * has permission.
+ */
const ALL = true;
+/**
+ * M2M scopes to "read" projects
+ */
const SCOPES_PROJECTS_READ = [
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
M2M_SCOPES.PROJECTS.ALL,
M2M_SCOPES.PROJECTS.READ,
];
+/**
+ * M2M scopes to "write" projects
+ */
const SCOPES_PROJECTS_WRITE = [
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
M2M_SCOPES.PROJECTS.ALL,
M2M_SCOPES.PROJECTS.WRITE,
];
+/**
+ * M2M scopes to "write" billingAccountId property
+ */
const SCOPES_PROJECTS_WRITE_BILLING_ACCOUNTS = [
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
M2M_SCOPES.PROJECTS.WRITE_BILLING_ACCOUNTS,
];
+/**
+ * M2M scopes to "read" projects members
+ */
const SCOPES_PROJECT_MEMBERS_READ = [
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
M2M_SCOPES.PROJECT_MEMBERS.ALL,
M2M_SCOPES.PROJECT_MEMBERS.READ,
];
+/**
+ * M2M scopes to "write" projects members
+ */
const SCOPES_PROJECT_MEMBERS_WRITE = [
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
M2M_SCOPES.PROJECT_MEMBERS.ALL,
M2M_SCOPES.PROJECT_MEMBERS.WRITE,
];
+/**
+ * The full list of possible permission rules in Project Service
+ */
export const PERMISSION = { // eslint-disable-line import/prefer-default-export
/*
* Project
@@ -430,7 +459,85 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
},
- /**
+ /*
+ * Project Attachments
+ */
+ CREATE_PROJECT_ATTACHMENT: {
+ meta: {
+ title: 'Create Project Attachment',
+ group: 'Project Attachment',
+ },
+ topcoderRoles: TOPCODER_ROLES_MANAGERS_AND_ADMINS,
+ projectRoles: ALL,
+ scopes: SCOPES_PROJECTS_WRITE,
+ },
+
+ READ_PROJECT_ATTACHMENT_OWN_OR_ALLOWED: {
+ meta: {
+ title: 'Read Project Attachment (own or allowed)',
+ group: 'Project Attachment',
+ description: 'Who can view own attachment or an attachment of another user when they are in the "allowed" list.',
+ },
+ topcoderRoles: TOPCODER_ROLES_MANAGERS_AND_ADMINS,
+ projectRoles: ALL,
+ scopes: SCOPES_PROJECTS_READ,
+ },
+
+ READ_PROJECT_ATTACHMENT_NOT_OWN_AND_NOT_ALLOWED: {
+ meta: {
+ title: 'Read Project Attachment (not own and not allowed)',
+ group: 'Project Attachment',
+ description: 'Who can view attachment of another user when they are not in "allowed" users list.',
+ },
+ topcoderRoles: TOPCODER_ROLES_ADMINS,
+ scopes: SCOPES_PROJECTS_READ,
+ },
+
+ UPDATE_PROJECT_ATTACHMENT_OWN: {
+ meta: {
+ title: 'Update Project Attachment (own)',
+ group: 'Project Attachment',
+ description: 'Who can edit attachment they created.',
+ },
+ topcoderRoles: TOPCODER_ROLES_MANAGERS_AND_ADMINS,
+ projectRoles: ALL,
+ scopes: SCOPES_PROJECTS_WRITE,
+ },
+
+ UPDATE_PROJECT_ATTACHMENT_NOT_OWN: {
+ meta: {
+ title: 'Update Project Attachment (not own)',
+ group: 'Project Attachment',
+ description: 'Who can edit attachment created by another user.',
+ },
+ topcoderRoles: TOPCODER_ROLES_ADMINS,
+ scopes: SCOPES_PROJECTS_WRITE,
+ },
+
+ DELETE_PROJECT_ATTACHMENT_OWN: {
+ meta: {
+ title: 'Delete Project Attachment (own)',
+ group: 'Project Attachment',
+ description: 'Who can delete attachment they created.',
+ },
+ topcoderRoles: TOPCODER_ROLES_MANAGERS_AND_ADMINS,
+ projectRoles: ALL,
+ scopes: SCOPES_PROJECTS_WRITE,
+ },
+
+ DELETE_PROJECT_ATTACHMENT_NOT_OWN: {
+ meta: {
+ title: 'Delete Project Attachment (not own)',
+ group: 'Project Attachment',
+ description: 'Who can delete attachment created by another user.',
+ },
+ topcoderRoles: TOPCODER_ROLES_ADMINS,
+ scopes: SCOPES_PROJECTS_WRITE,
+ },
+
+ /*
+ * DEPRECATED - THIS PERMISSION RULE HAS TO BE REMOVED
+ *
* Permissions defined by logic: **WHO** can do actions with such a permission.
*/
ROLES_COPILOT_AND_ABOVE: {
@@ -448,6 +555,10 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
},
};
+/**
+ * Matrix which define Project Roles and corresponding Topcoder Roles of users
+ * who may join with such Project Roles.
+ */
export const PROJECT_TO_TOPCODER_ROLES_MATRIX = {
[PROJECT_MEMBER_ROLE.CUSTOMER]: _.values(USER_ROLE),
[PROJECT_MEMBER_ROLE.MANAGER]: [
diff --git a/src/permissions/index.js b/src/permissions/index.js
index 52fcfe8a..96349fd2 100644
--- a/src/permissions/index.js
+++ b/src/permissions/index.js
@@ -4,8 +4,6 @@ const Authorizer = require('tc-core-library-js').Authorizer;
const projectView = require('./project.view');
const projectEdit = require('./project.edit');
const projectAdmin = require('./admin.ops');
-const projectAttachmentUpdate = require('./project.updateAttachment');
-const projectAttachmentDownload = require('./project.downloadAttachment');
const connectManagerOrAdmin = require('./connectManagerOrAdmin.ops');
const copilotAndAbove = require('./copilotAndAbove');
const workManagementPermissions = require('./workManagementForTemplate');
@@ -54,11 +52,19 @@ module.exports = () => {
PERMISSION.DELETE_PROJECT_INVITE_NOT_OWN_NON_CUSTOMER,
]));
- Authorizer.setPolicy('project.addAttachment', projectEdit);
- Authorizer.setPolicy('project.updateAttachment', projectAttachmentUpdate);
- Authorizer.setPolicy('project.removeAttachment', projectAttachmentUpdate);
- Authorizer.setPolicy('project.downloadAttachment', projectAttachmentDownload);
- Authorizer.setPolicy('project.listAttachment', projectView);
+ Authorizer.setPolicy('projectAttachment.create', generalPermission(PERMISSION.CREATE_PROJECT_ATTACHMENT));
+ Authorizer.setPolicy('projectAttachment.view', generalPermission([
+ PERMISSION.READ_PROJECT_ATTACHMENT_OWN_OR_ALLOWED,
+ PERMISSION.READ_PROJECT_ATTACHMENT_NOT_OWN_AND_NOT_ALLOWED,
+ ]));
+ Authorizer.setPolicy('projectAttachment.edit', generalPermission([
+ PERMISSION.UPDATE_PROJECT_ATTACHMENT_OWN,
+ PERMISSION.UPDATE_PROJECT_ATTACHMENT_NOT_OWN,
+ ]));
+ Authorizer.setPolicy('projectAttachment.delete', generalPermission([
+ PERMISSION.DELETE_PROJECT_ATTACHMENT_OWN,
+ PERMISSION.DELETE_PROJECT_ATTACHMENT_NOT_OWN,
+ ]));
Authorizer.setPolicy('project.admin', projectAdmin);
diff --git a/src/permissions/project.downloadAttachment.js b/src/permissions/project.downloadAttachment.js
deleted file mode 100644
index 743bf299..00000000
--- a/src/permissions/project.downloadAttachment.js
+++ /dev/null
@@ -1,37 +0,0 @@
-import _ from 'lodash';
-import util from '../util';
-import models from '../models';
-
-/**
- * Connect admin and Topcoder admins are allowed to download any project attachments
- * Rest can update attachments that they created or given access
- * @param {Object} freq the express request instance
- * @return {Promise} Returns a promise
- */
-module.exports = freq => new Promise((resolve, reject) => {
- const projectId = _.parseInt(freq.params.projectId);
- const attachmentId = _.parseInt(freq.params.id);
- const userId = freq.authUser.userId;
-
- if (util.hasAdminRole(freq)) {
- return resolve(true);
- }
- return models.ProjectAttachment.getAttachmentById(projectId, attachmentId)
- .then((attachment) => {
- const req = freq;
- req.context = req.context || {};
- req.context.existingAttachment = attachment;
-
- // deligate not found to the actual handler
- if (!attachment) {
- return resolve(true);
- }
-
- if (attachment.createdBy === userId || attachment.allowedUsers === null ||
- attachment.allowedUsers.indexOf(userId) >= 0) {
- return resolve(true);
- }
-
- return reject(new Error('You\'re not allowed to download'));
- });
-});
diff --git a/src/permissions/project.updateAttachment.js b/src/permissions/project.updateAttachment.js
deleted file mode 100644
index 2ca57d3f..00000000
--- a/src/permissions/project.updateAttachment.js
+++ /dev/null
@@ -1,36 +0,0 @@
-import _ from 'lodash';
-import util from '../util';
-import models from '../models';
-
-/**
- * Connect admin and Topcoder admins are allowed to update any project attachments
- * Rest can update attachments that they created
- * @param {Object} freq the express request instance
- * @return {Promise} Returns a promise
- */
-module.exports = freq => new Promise((resolve, reject) => {
- const projectId = _.parseInt(freq.params.projectId);
- const attachmentId = _.parseInt(freq.params.id);
-
- if (util.hasAdminRole(freq)) {
- return resolve(true);
- }
-
- return models.ProjectAttachment.getAttachmentById(projectId, attachmentId)
- .then((attachment) => {
- const req = freq;
- req.context = req.context || {};
- req.context.existingAttachment = attachment;
-
- // deligate not found to the actual handler
- if (!attachment) {
- return resolve(true);
- }
-
- if (attachment.createdBy === req.authUser.userId) {
- return resolve(true);
- }
-
- return reject(new Error('Only admins and the user that uploaded the docs can modify'));
- });
-});
diff --git a/src/routes/attachments/create.js b/src/routes/attachments/create.js
index d8ee9c36..81c1a80a 100644
--- a/src/routes/attachments/create.js
+++ b/src/routes/attachments/create.js
@@ -33,7 +33,7 @@ const addAttachmentValidations = {
module.exports = [
// handles request validations
validate(addAttachmentValidations),
- permissions('project.addAttachment'),
+ permissions('projectAttachment.create'),
/*
* Add project attachment
* In development mode we have to mock the ec2 file transfer and file service calls
diff --git a/src/routes/attachments/create.spec.js b/src/routes/attachments/create.spec.js
index ad9df03f..1efc9ff9 100644
--- a/src/routes/attachments/create.spec.js
+++ b/src/routes/attachments/create.spec.js
@@ -205,6 +205,34 @@ describe('Project Attachments', () => {
});
});
+ it('should create project successfully using M2M token with "write:projects" scope', (done) => {
+ request(server)
+ .post(`/v5/projects/${project1.id}/attachments/`)
+ .set({
+ Authorization: `Bearer ${testUtil.m2m['write:projects']}`,
+ })
+ .send(fileAttachmentBody)
+ .expect('Content-Type', /json/)
+ .expect(201)
+ .end((err, res) => {
+ if (err) {
+ done(err);
+ } else {
+ const resJson = res.body;
+ should.exist(resJson);
+ postSpy.should.have.been.calledOnce;
+ getSpy.should.have.been.calledOnce;
+ stub.restore();
+ resJson.title.should.equal(fileAttachmentBody.title);
+ resJson.tags.should.eql(fileAttachmentBody.tags);
+ resJson.type.should.eql(fileAttachmentBody.type);
+ resJson.downloadUrl.should.exist;
+ resJson.projectId.should.equal(project1.id);
+ done();
+ }
+ });
+ });
+
describe('Bus api', () => {
let createEventSpy;
diff --git a/src/routes/attachments/delete.js b/src/routes/attachments/delete.js
index 723e32e9..6685feff 100644
--- a/src/routes/attachments/delete.js
+++ b/src/routes/attachments/delete.js
@@ -19,7 +19,7 @@ import { EVENT, RESOURCES, ATTACHMENT_TYPES } from '../../constants';
const permissions = tcMiddleware.permissions;
module.exports = [
- permissions('project.removeAttachment'),
+ permissions('projectAttachment.delete'),
(req, res, next) => {
const projectId = _.parseInt(req.params.projectId);
const attachmentId = _.parseInt(req.params.id);
diff --git a/src/routes/attachments/delete.spec.js b/src/routes/attachments/delete.spec.js
index b308b1a5..96aecd55 100644
--- a/src/routes/attachments/delete.spec.js
+++ b/src/routes/attachments/delete.spec.js
@@ -268,6 +268,61 @@ describe('Project Attachments delete', () => {
});
});
+ it('should remove attachment file using M2M token with "write:projects" scope', (done) => {
+ const mockHttpClient = _.merge(testUtil.mockHttpClient, {
+ delete: () => Promise.resolve({
+ status: 200,
+ data: {
+ id: 'requesterId',
+ version: 'v3',
+ result: {
+ success: true,
+ status: 200,
+ content: true,
+ },
+ },
+ }),
+ });
+ const deleteSpy = sinon.spy(mockHttpClient, 'delete');
+ sandbox.stub(util, 'getHttpClient', () => mockHttpClient);
+ request(server)
+ .delete(`/v5/projects/${project1.id}/attachments/${attachments[1].id}`)
+ .set({
+ Authorization: `Bearer ${testUtil.m2m['write:projects']}`,
+ })
+ .expect(204)
+ .end((err) => {
+ if (err) {
+ done(err);
+ } else {
+ setTimeout(() =>
+ models.ProjectAttachment.findOne({
+ where: {
+ projectId: project1.id,
+ id: attachments[1].id,
+ },
+ paranoid: false,
+ })
+ .then((res) => {
+ if (!res) {
+ throw new Error('Should found the entity');
+ } else {
+ deleteSpy.called.should.be.false;
+ chai.assert.isNotNull(res.deletedAt);
+ chai.assert.isNotNull(res.deletedBy);
+
+ request(server)
+ .get(`/v5/projects/${project1.id}/attachments/${attachments[1].id}`)
+ .set({
+ Authorization: `Bearer ${testUtil.jwts.admin}`,
+ })
+ .expect(404, done);
+ }
+ }), 500);
+ }
+ });
+ });
+
describe('Bus api', () => {
let createEventSpy;
diff --git a/src/routes/attachments/get.js b/src/routes/attachments/get.js
index 20f14805..374d96f6 100644
--- a/src/routes/attachments/get.js
+++ b/src/routes/attachments/get.js
@@ -4,6 +4,7 @@ import { middleware as tcMiddleware } from 'tc-core-library-js';
import models from '../../models';
import util from '../../util';
import { ATTACHMENT_TYPES } from '../../constants';
+import permissionUtils from '../../utils/permissions';
/**
* API to get a project attachment.
@@ -36,7 +37,7 @@ const getPreSignedUrl = async (req, attachment) => {
};
module.exports = [
- permissions('project.downloadAttachment'),
+ permissions('projectAttachment.view'),
(req, res, next) => {
const projectId = _.parseInt(req.params.projectId);
const attachmentId = _.parseInt(req.params.id);
@@ -72,14 +73,6 @@ module.exports = [
projectId,
},
})
- .then((attachment) => {
- if (!attachment) {
- const err = new Error('Record not found');
- err.status = 404;
- return Promise.reject(err);
- }
- return getPreSignedUrl(req, attachment);
- })
.catch((error) => {
req.log.error('Error fetching attachment', error);
const rerr = error;
@@ -88,8 +81,23 @@ module.exports = [
});
}
req.log.debug('attachment found in ES');
- const attachment = data[0].inner_hits.attachments.hits.hits[0]._source; // eslint-disable-line no-underscore-dangle
+ return data[0].inner_hits.attachments.hits.hits[0]._source; // eslint-disable-line no-underscore-dangle
+ })
+ // check permissions
+ .then((attachment) => {
+ // if don't have permissions we would return 404 below as users shouldn't even know if attachment exists
+ if (!permissionUtils.hasReadAccessToAttachment(attachment, req)) {
+ return null;
+ }
+ return attachment;
+ })
+ .then((attachment) => {
+ if (!attachment) {
+ const err = new Error('Record not found');
+ err.status = 404;
+ return Promise.reject(err);
+ }
return getPreSignedUrl(req, attachment);
})
.then((result) => {
diff --git a/src/routes/attachments/get.spec.js b/src/routes/attachments/get.spec.js
index efa831ed..c030adfe 100644
--- a/src/routes/attachments/get.spec.js
+++ b/src/routes/attachments/get.spec.js
@@ -44,7 +44,14 @@ describe('Get Project attachments Tests', () => {
isPrimary: true,
createdBy: 1,
updatedBy: 1,
- }).then(() => models.ProjectAttachment.create({
+ }).then(() => models.ProjectMember.create({
+ userId: 40051335,
+ projectId: project1.id,
+ role: 'customer',
+ isPrimary: false,
+ createdBy: 1,
+ updatedBy: 1,
+ })).then(() => models.ProjectAttachment.create({
projectId: project1.id,
title: 'test.txt',
description: 'blah',
@@ -78,24 +85,24 @@ describe('Get Project attachments Tests', () => {
sandbox.restore();
});
- it('should return 403 if USER does not have permissions', (done) => {
+ it.only('should return 404 if USER does not have permissions', (done) => {
request(server)
.get(`/v5/projects/${project1.id}/attachments/${attachment.id}`)
.set({
Authorization: `Bearer ${testUtil.jwts.member2}`,
})
.send()
- .expect(403, done);
+ .expect(404, done);
});
- it('should return 403 if MANAGER does not have permissions', (done) => {
+ it('should return 404 if MANAGER does not have permissions', (done) => {
request(server)
.get(`/v5/projects/${project1.id}/attachments/${attachment.id}`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.send()
- .expect(403, done);
+ .expect(404, done);
});
it('should return 404 if attachment was not found', (done) => {
@@ -137,5 +144,15 @@ describe('Get Project attachments Tests', () => {
.send()
.expect(200, done);
});
+
+ it('should return 200 when using M2M token with scope "read:projects"', (done) => {
+ request(server)
+ .get(`/v5/projects/${project1.id}/attachments/${attachment.id}`)
+ .set({
+ Authorization: `Bearer ${testUtil.m2m['read:projects']}`,
+ })
+ .send()
+ .expect(200, done);
+ });
});
});
diff --git a/src/routes/attachments/list.js b/src/routes/attachments/list.js
index 19b5fd75..b8f25b88 100644
--- a/src/routes/attachments/list.js
+++ b/src/routes/attachments/list.js
@@ -4,6 +4,7 @@ import Joi from 'joi';
import { middleware as tcMiddleware } from 'tc-core-library-js';
import models from '../../models';
import util from '../../util';
+import permissionUtils from '../../utils/permissions';
/**
* API to get project attachments.
@@ -20,7 +21,7 @@ const schema = {
module.exports = [
validate(schema),
- permissions('project.listAttachment'),
+ permissions('projectAttachment.view'),
(req, res, next) => {
const projectId = _.parseInt(req.params.projectId);
@@ -54,12 +55,16 @@ module.exports = [
attributes: { exclude: ['deletedAt', 'deletedBy'] },
raw: true,
})
- .then(attachments => res.json(attachments))
.catch(next);
}
req.log.debug('attachments found in ES');
- return res.json(data[0].inner_hits.attachments.hits.hits.map(hit => hit._source)); // eslint-disable-line no-underscore-dangle
+ return data[0].inner_hits.attachments.hits.hits.map(hit => hit._source); // eslint-disable-line no-underscore-dangle
})
+ // filter out attachments which user cannot see
+ .then(attachments => attachments.filter(attachment =>
+ permissionUtils.hasReadAccessToAttachment(attachment, req),
+ ))
+ .then(attachments => res.json(attachments))
.catch(next);
},
];
diff --git a/src/routes/attachments/list.spec.js b/src/routes/attachments/list.spec.js
index 433243ea..cf54c467 100644
--- a/src/routes/attachments/list.spec.js
+++ b/src/routes/attachments/list.spec.js
@@ -32,13 +32,20 @@ describe('Project Attachments download', () => {
project1 = p;
// create members
return models.ProjectMember.create({
- userId: 40051332,
+ userId: testUtil.userIds.copilot,
projectId: project1.id,
role: 'copilot',
isPrimary: true,
createdBy: 1,
updatedBy: 1,
- }).then(() => models.ProjectAttachment.bulkCreate([{
+ }).then(() => models.ProjectMember.create({
+ userId: testUtil.userIds.member,
+ projectId: project1.id,
+ role: 'customer',
+ isPrimary: false,
+ createdBy: 1,
+ updatedBy: 1,
+ })).then(() => models.ProjectAttachment.bulkCreate([{
projectId: project1.id,
title: 'test.txt',
description: 'blah',
@@ -50,6 +57,7 @@ describe('Project Attachments download', () => {
tags: ['tag1', 'tag2'],
createdBy: testUtil.userIds.copilot,
updatedBy: 1,
+ allowedUsers: [testUtil.userIds.member],
}, {
projectId: project1.id,
title: 'link test 1',
@@ -61,6 +69,7 @@ describe('Project Attachments download', () => {
tags: ['tag3'],
createdBy: testUtil.userIds.copilot,
updatedBy: 1,
+ allowedUsers: [testUtil.userIds.member2],
}]).then(() => done()));
});
});
@@ -77,37 +86,83 @@ describe('Project Attachments download', () => {
.expect(403, done);
});
- it('should return 403 for member', (done) => {
+ it('should return 403 for a regular user who is not a member of the project', (done) => {
request(server)
.get(`/v5/projects/${project1.id}/attachments`)
.set({
- Authorization: `Bearer ${testUtil.jwts.member}`,
+ Authorization: `Bearer ${testUtil.jwts.member2}`,
})
.send()
.expect(403, done);
});
- it('should return 200 for manager', (done) => {
+ it('should not return attachments to a manager if they are not owner and not allowed', (done) => {
request(server)
.get(`/v5/projects/${project1.id}/attachments`)
.set({
Authorization: `Bearer ${testUtil.jwts.manager}`,
})
.send()
- .expect(200, done);
+ .expect(200)
+ .end((err, res) => {
+ if (err) {
+ done(err);
+ } else {
+ const resJson = res.body;
+
+ resJson.should.have.length(0);
+
+ done();
+ }
+ });
});
- it('should return 200 for copilot', (done) => {
+ it('should return attachments to its owner', (done) => {
request(server)
.get(`/v5/projects/${project1.id}/attachments`)
.set({
Authorization: `Bearer ${testUtil.jwts.copilot}`,
})
.send()
- .expect(200, done);
+ .expect(200)
+ .end((err, res) => {
+ if (err) {
+ done(err);
+ } else {
+ const resJson = res.body;
+
+ resJson.should.have.length(2);
+ resJson[0].createdBy.should.be.eql(testUtil.userIds.copilot);
+ resJson[1].createdBy.should.be.eql(testUtil.userIds.copilot);
+
+ done();
+ }
+ });
});
- it('should return 200 for admin', (done) => {
+ it('should return attachments to the user who is allowed', (done) => {
+ request(server)
+ .get(`/v5/projects/${project1.id}/attachments`)
+ .set({
+ Authorization: `Bearer ${testUtil.jwts.member}`,
+ })
+ .send()
+ .expect(200)
+ .end((err, res) => {
+ if (err) {
+ done(err);
+ } else {
+ const resJson = res.body;
+
+ resJson.should.have.length(1);
+ resJson[0].allowedUsers.indexOf(testUtil.userIds.member).should.not.equal(-1);
+
+ done();
+ }
+ });
+ });
+
+ it('should return all attachments to admin', (done) => {
request(server)
.get(`/v5/projects/${project1.id}/attachments`)
.set({
@@ -116,32 +171,78 @@ describe('Project Attachments download', () => {
.send()
.expect(200)
.end((err, res) => {
- const resJson = res.body;
- resJson.should.have.length(2);
- resJson[0].description.should.be.eql('blah');
- resJson[0].path.should.be.eql('https://media.topcoder.com/projects/1/test.txt');
- resJson[0].projectId.should.be.eql(project1.id);
- resJson[0].type.should.be.eql(ATTACHMENT_TYPES.FILE);
- resJson[0].createdBy.should.be.eql(testUtil.userIds.copilot);
- resJson[0].updatedBy.should.be.eql(1);
- should.exist(resJson[0].createdAt);
- should.exist(resJson[0].updatedAt);
- should.not.exist(resJson[0].deletedBy);
- should.not.exist(resJson[0].deletedAt);
-
- resJson[1].projectId.should.be.eql(project1.id);
- resJson[1].description.should.be.eql('link test description');
- resJson[1].path.should.be.eql('https://media.topcoder.com/projects/1/test2.txt');
- resJson[1].type.should.be.eql(ATTACHMENT_TYPES.LINK);
- resJson[1].tags.should.be.eql(['tag3']);
- resJson[1].createdBy.should.be.eql(testUtil.userIds.copilot);
- resJson[1].updatedBy.should.be.eql(1);
- should.exist(resJson[0].createdAt);
- should.exist(resJson[0].updatedAt);
- should.not.exist(resJson[0].deletedBy);
- should.not.exist(resJson[0].deletedAt);
-
- done();
+ if (err) {
+ done(err);
+ } else {
+ const resJson = res.body;
+ resJson.should.have.length(2);
+ resJson[0].description.should.be.eql('blah');
+ resJson[0].path.should.be.eql('https://media.topcoder.com/projects/1/test.txt');
+ resJson[0].projectId.should.be.eql(project1.id);
+ resJson[0].type.should.be.eql(ATTACHMENT_TYPES.FILE);
+ resJson[0].createdBy.should.be.eql(testUtil.userIds.copilot);
+ resJson[0].updatedBy.should.be.eql(1);
+ should.exist(resJson[0].createdAt);
+ should.exist(resJson[0].updatedAt);
+ should.not.exist(resJson[0].deletedBy);
+ should.not.exist(resJson[0].deletedAt);
+
+ resJson[1].projectId.should.be.eql(project1.id);
+ resJson[1].description.should.be.eql('link test description');
+ resJson[1].path.should.be.eql('https://media.topcoder.com/projects/1/test2.txt');
+ resJson[1].type.should.be.eql(ATTACHMENT_TYPES.LINK);
+ resJson[1].tags.should.be.eql(['tag3']);
+ resJson[1].createdBy.should.be.eql(testUtil.userIds.copilot);
+ resJson[1].updatedBy.should.be.eql(1);
+ should.exist(resJson[0].createdAt);
+ should.exist(resJson[0].updatedAt);
+ should.not.exist(resJson[0].deletedBy);
+ should.not.exist(resJson[0].deletedAt);
+
+ done();
+ }
+ });
+ });
+
+ it('should return all attachments using M2M token with "read:projects" scope', (done) => {
+ request(server)
+ .get(`/v5/projects/${project1.id}/attachments`)
+ .set({
+ Authorization: `Bearer ${testUtil.m2m['read:projects']}`,
+ })
+ .send()
+ .expect(200)
+ .end((err, res) => {
+ if (err) {
+ done(err);
+ } else {
+ const resJson = res.body;
+ resJson.should.have.length(2);
+ resJson[0].description.should.be.eql('blah');
+ resJson[0].path.should.be.eql('https://media.topcoder.com/projects/1/test.txt');
+ resJson[0].projectId.should.be.eql(project1.id);
+ resJson[0].type.should.be.eql(ATTACHMENT_TYPES.FILE);
+ resJson[0].createdBy.should.be.eql(testUtil.userIds.copilot);
+ resJson[0].updatedBy.should.be.eql(1);
+ should.exist(resJson[0].createdAt);
+ should.exist(resJson[0].updatedAt);
+ should.not.exist(resJson[0].deletedBy);
+ should.not.exist(resJson[0].deletedAt);
+
+ resJson[1].projectId.should.be.eql(project1.id);
+ resJson[1].description.should.be.eql('link test description');
+ resJson[1].path.should.be.eql('https://media.topcoder.com/projects/1/test2.txt');
+ resJson[1].type.should.be.eql(ATTACHMENT_TYPES.LINK);
+ resJson[1].tags.should.be.eql(['tag3']);
+ resJson[1].createdBy.should.be.eql(testUtil.userIds.copilot);
+ resJson[1].updatedBy.should.be.eql(1);
+ should.exist(resJson[0].createdAt);
+ should.exist(resJson[0].updatedAt);
+ should.not.exist(resJson[0].deletedBy);
+ should.not.exist(resJson[0].deletedAt);
+
+ done();
+ }
});
});
});
diff --git a/src/routes/attachments/update.js b/src/routes/attachments/update.js
index a8beb68d..176ef5b1 100644
--- a/src/routes/attachments/update.js
+++ b/src/routes/attachments/update.js
@@ -8,6 +8,7 @@ import {
import models from '../../models';
import util from '../../util';
import { EVENT, RESOURCES } from '../../constants';
+import { PERMISSION } from '../../permissions/constants';
/**
* API to update a project member.
@@ -27,7 +28,7 @@ const updateProjectAttachmentValidation = {
module.exports = [
// handles request validations
validate(updateProjectAttachmentValidation),
- permissions('project.updateAttachment'),
+ permissions('projectAttachment.edit'),
/*
* Update a attachment if the user has access
*/
@@ -48,12 +49,21 @@ module.exports = [
const err = new Error('project attachment not found for project id ' +
`${projectId} and member id ${attachmentId}`);
err.status = 404;
- reject(err);
- } else {
- previousValue = _.cloneDeep(existing.get({ plain: true }));
- _.extend(existing, updatedProps);
- existing.save().then(accept).catch(reject);
+ return reject(err);
}
+ previousValue = _.cloneDeep(existing.get({ plain: true }));
+
+ if (
+ previousValue.createdBy !== req.authUser.userId &&
+ !util.hasPermissionByReq(PERMISSION.UPDATE_PROJECT_ATTACHMENT_NOT_OWN, req)
+ ) {
+ const err = new Error('You don\'t have permission to update attachment created by another user.');
+ err.status = 403;
+ return reject(err);
+ }
+
+ _.extend(existing, updatedProps);
+ return existing.save().then(accept).catch(reject);
})).then((updated) => {
req.log.debug('updated project attachment', JSON.stringify(updated, null, 2));
res.json(updated);
diff --git a/src/routes/attachments/update.spec.js b/src/routes/attachments/update.spec.js
index 9331747a..04b0254c 100644
--- a/src/routes/attachments/update.spec.js
+++ b/src/routes/attachments/update.spec.js
@@ -40,38 +40,46 @@ describe('Project Attachments update', () => {
isPrimary: true,
createdBy: 1,
updatedBy: 1,
- }).then(() => models.ProjectAttachment.create({
+ }).then(() => models.ProjectMember.create({
+ userId: 40051331,
projectId: project1.id,
- title: 'test.txt',
- description: 'blah',
- contentType: 'application/unknown',
- size: 12312,
- category: null,
- path: 'https://media.topcoder.com/projects/1/test.txt',
- type: ATTACHMENT_TYPES.FILE,
- tags: ['tag1', 'tag2', 'tag3'],
- createdBy: testUtil.userIds.copilot,
+ role: 'customer',
+ isPrimary: false,
+ createdBy: 1,
updatedBy: 1,
- allowedUsers: [],
- }).then((a1) => {
- attachment = a1;
- models.ProjectAttachment.create(
- {
- projectId: project1.id,
- title: 'Test Link 1',
- description: 'Test link 1 description',
- size: 123456,
- category: null,
- path: 'https://connect.topcoder-dev.com/projects/8600/assets',
- type: ATTACHMENT_TYPES.LINK,
- tags: ['tag3', 'tag4'],
- createdBy: testUtil.userIds.copilot,
- updatedBy: 1,
- }).then((_link) => {
- link = _link;
- done();
- });
- }));
+ }))
+ .then(() => models.ProjectAttachment.create({
+ projectId: project1.id,
+ title: 'test.txt',
+ description: 'blah',
+ contentType: 'application/unknown',
+ size: 12312,
+ category: null,
+ path: 'https://media.topcoder.com/projects/1/test.txt',
+ type: ATTACHMENT_TYPES.FILE,
+ tags: ['tag1', 'tag2', 'tag3'],
+ createdBy: testUtil.userIds.copilot,
+ updatedBy: 1,
+ allowedUsers: [],
+ }).then((a1) => {
+ attachment = a1;
+ models.ProjectAttachment.create(
+ {
+ projectId: project1.id,
+ title: 'Test Link 1',
+ description: 'Test link 1 description',
+ size: 123456,
+ category: null,
+ path: 'https://connect.topcoder-dev.com/projects/8600/assets',
+ type: ATTACHMENT_TYPES.LINK,
+ tags: ['tag3', 'tag4'],
+ createdBy: testUtil.userIds.copilot,
+ updatedBy: 1,
+ }).then((_link) => {
+ link = _link;
+ done();
+ });
+ }));
});
});
});
@@ -186,6 +194,27 @@ describe('Project Attachments update', () => {
});
});
+ it('should update the project using M2M token with "write:projects" scope', (done) => {
+ request(server)
+ .patch(`/v5/projects/${project1.id}/attachments/${attachment.id}`)
+ .set({
+ Authorization: `Bearer ${testUtil.m2m['write:projects']}`,
+ })
+ .send({ title: 'updated title m2m', description: 'updated description m2m' })
+ .expect(200)
+ .end((err, res) => {
+ if (err) {
+ done(err);
+ } else {
+ const resJson = res.body;
+ should.exist(resJson);
+ resJson.title.should.equal('updated title m2m');
+ resJson.description.should.equal('updated description m2m');
+ done();
+ }
+ });
+ });
+
describe('Bus api', () => {
let createEventSpy;
diff --git a/src/routes/projectMemberInvites/get.js b/src/routes/projectMemberInvites/get.js
index 23ea0524..0bae06d2 100644
--- a/src/routes/projectMemberInvites/get.js
+++ b/src/routes/projectMemberInvites/get.js
@@ -93,7 +93,7 @@ module.exports = [
// check there is an existing invite for the user with status PENDING
// handle 404
let errMsg;
- if (req.context.inviteType === 'all') {
+ if (util.hasPermissionByReq(PERMISSION.READ_PROJECT_INVITE_NOT_OWN, req)) {
errMsg = `invite not found for project id ${projectId}, inviteId ${inviteId}`;
} else {
errMsg = `invite not found for project id ${projectId}, inviteId ${inviteId}, ` +
diff --git a/src/utils/permissions.js b/src/utils/permissions.js
new file mode 100644
index 00000000..253318ba
--- /dev/null
+++ b/src/utils/permissions.js
@@ -0,0 +1,45 @@
+/**
+ * Helper methods related to checking permissions
+ */
+import _ from 'lodash';
+import util from '../util';
+import { PERMISSION } from '../permissions/constants';
+
+const permissionUtils = {
+ /**
+ * Check if request from the user has permission to READ attachment
+ *
+ * @param {Object} attachment attachment
+ * @param {express.Request} req request
+ *
+ * @returns {Boolean} true if has permission
+ */
+ hasReadAccessToAttachment: (attachment, req) => {
+ if (!attachment) {
+ return false;
+ }
+
+ const isOwnAttachment = attachment.createdBy === req.authUser.userId;
+ const isAllowedAttachment = attachment.allowedUsers === null ||
+ _.includes(attachment.allowedUsers, req.authUser.userId);
+
+ if (
+ util.hasPermissionByReq(PERMISSION.READ_PROJECT_ATTACHMENT_OWN_OR_ALLOWED, req) && (
+ isOwnAttachment || isAllowedAttachment
+ )
+ ) {
+ return true;
+ }
+
+ if (
+ util.hasPermissionByReq(PERMISSION.READ_PROJECT_ATTACHMENT_NOT_OWN_AND_NOT_ALLOWED, req) &&
+ !isOwnAttachment && !isAllowedAttachment
+ ) {
+ return true;
+ }
+
+ return false;
+ },
+};
+
+export default permissionUtils;