diff --git a/src/routes/projects/get.js b/src/routes/projects/get.js index 148601b2..caa04531 100644 --- a/src/routes/projects/get.js +++ b/src/routes/projects/get.js @@ -3,6 +3,7 @@ import config from 'config'; import { middleware as tcMiddleware } from 'tc-core-library-js'; import models from '../../models'; import util from '../../util'; +import { PERMISSION } from '../../permissions/constants'; const ES_PROJECT_INDEX = config.get('elasticsearchConfig.indexName'); const ES_PROJECT_TYPE = config.get('elasticsearchConfig.docType'); @@ -103,7 +104,8 @@ const retrieveProjectFromES = (projectId, req) => { fields = fields ? fields.split(',') : []; fields = util.parseFields(fields, { projects: PROJECT_ATTRIBUTES, - project_members: util.addUserDetailsFieldsIfAllowed(PROJECT_MEMBER_ATTRIBUTES_ES, req), + project_members: util.hasPermissionByReq(PERMISSION.READ_PROJECT_MEMBER, req) + ? util.addUserDetailsFieldsIfAllowed(PROJECT_MEMBER_ATTRIBUTES_ES, req) : null, project_member_invites: PROJECT_MEMBER_INVITE_ATTRIBUTES, project_phases: PROJECT_PHASE_ATTRIBUTES, project_phases_products: PROJECT_PHASE_PRODUCTS_ATTRIBUTES, @@ -143,7 +145,9 @@ const retrieveProjectFromDB = (projectId, req) => { return Promise.reject(apiErr); } // check context for project members - project.members = _.map(req.context.currentProjectMembers, m => _.pick(m, fields.project_members)); + if (util.hasPermissionByReq(PERMISSION.READ_PROJECT_MEMBER, req)) { + project.members = _.map(req.context.currentProjectMembers, m => _.pick(m, fields.project_members)); + } // check if attachments field was requested if (!req.query.fields || _.indexOf(req.query.fields, 'attachments') > -1) { return util.getProjectAttachments(req, project.id); diff --git a/src/routes/projects/get.spec.js b/src/routes/projects/get.spec.js index a12e46f6..3faa8a0e 100644 --- a/src/routes/projects/get.spec.js +++ b/src/routes/projects/get.spec.js @@ -268,7 +268,7 @@ describe('GET Project', () => { should.not.exist(resJson.billingAccountId); should.exist(resJson.name); resJson.status.should.be.eql('draft'); - resJson.members.should.have.lengthOf(2); + should.not.exist(resJson.members); done(); } }); diff --git a/src/routes/projects/list.js b/src/routes/projects/list.js index a9ea8445..3998e688 100755 --- a/src/routes/projects/list.js +++ b/src/routes/projects/list.js @@ -492,6 +492,11 @@ const retrieveProjectsFromDB = (req, criteria, sort, ffields) => { // make sure project.id is part of fields if (_.indexOf(fields.projects, 'id') < 0) fields.projects.push('id'); + // add userId to project_members field so it can be used to check READ_PROJECT_MEMBER permission below. + const addMembersUserId = fields.project_members.length > 0 && _.indexOf(fields.project_members, 'userId') < 0; + if (addMembersUserId) { + fields.project_members.push('userId'); + } const retrieveAttachments = !req.query.fields || req.query.fields.indexOf('attachments') > -1; const retrieveMembers = !req.query.fields || !!fields.project_members.length; @@ -533,7 +538,19 @@ const retrieveProjectsFromDB = (req, criteria, sort, ffields) => { const p = fp; // if values length is 1 it could be either attachments or members if (retrieveMembers) { - p.members = _.filter(allMembers, m => m.projectId === p.id); + const pMembers = _.filter(allMembers, m => m.projectId === p.id); + // check if have permission to read project members + if (util.hasPermission(PERMISSION.READ_PROJECT_MEMBER, req.authUser, pMembers)) { + if (addMembersUserId) { + // remove the userId from the returned members array if it was added before + // as it is only needed for checking permission. + _.forEach(pMembers, (m) => { + const fm = m; + delete fm.userId; + }); + } + p.members = pMembers; + } } if (retrieveAttachments) { p.attachments = _.filter(allAttachments, a => a.projectId === p.id); @@ -562,12 +579,36 @@ const retrieveProjects = (req, criteria, sort, ffields) => { if (_.indexOf(fields.projects, 'id') < 0) { fields.projects.push('id'); } + // add userId to project_members field so it can be used to check READ_PROJECT_MEMBER permission below. + const addMembersUserId = fields.project_members.length > 0 && _.indexOf(fields.project_members, 'userId') < 0; + if (addMembersUserId) { + fields.project_members.push('userId'); + } const searchCriteria = parseElasticSearchCriteria(criteria, fields, order) || {}; return new Promise((accept, reject) => { const es = util.getElasticSearchClient(); es.search(searchCriteria).then((docs) => { const rows = _.map(docs.hits.hits, single => single._source); // eslint-disable-line no-underscore-dangle + if (rows) { + _.forEach(rows, (p) => { + const fp = p; + if (fp.members) { + // check if have permission to read project members + if (!util.hasPermission(PERMISSION.READ_PROJECT_MEMBER, req.authUser, fp.members)) { + delete fp.members; + } + if (fp.members && addMembersUserId) { + // remove the userId from the returned members array if it was added before + // as it is only needed for checking permission. + _.forEach(fp.members, (m) => { + const fm = m; + delete fm.userId; + }); + } + } + }); + } accept({ rows, count: docs.hits.total, pageSize: criteria.limit, page: criteria.page }); }).catch(reject); }); diff --git a/src/routes/projects/list.spec.js b/src/routes/projects/list.spec.js index e454a181..f57ecdb3 100644 --- a/src/routes/projects/list.spec.js +++ b/src/routes/projects/list.spec.js @@ -405,6 +405,29 @@ describe('LIST Project', () => { }); }); + it('should not include the project members using M2M token without "read:project-members" scope', (done) => { + request(server) + .get('/v5/projects') + .set({ + Authorization: `Bearer ${testUtil.m2m['read:projects']}`, + }) + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + if (err) { + done(err); + } else { + const resJson = res.body; + should.exist(resJson); + resJson.should.have.lengthOf(3); + resJson.forEach((project) => { + should.not.exist(project.members); + }); + done(); + } + }); + }); + it('should return the project when project that is in reviewed state in which the copilot is its member or has been invited', (done) => { request(server) .get('/v5/projects') @@ -1163,7 +1186,7 @@ describe('LIST Project', () => { request(server) .get('/v5/projects/') .set({ - Authorization: `Bearer ${testUtil.jwts.member2}`, + Authorization: `Bearer ${testUtil.jwts.member}`, }) .expect('Content-Type', /json/) .expect(200) @@ -1185,7 +1208,7 @@ describe('LIST Project', () => { request(server) .get('/v5/projects/?fields=members.email,members.id') .set({ - Authorization: `Bearer ${testUtil.jwts.member2}`, + Authorization: `Bearer ${testUtil.jwts.member}`, }) .expect('Content-Type', /json/) .expect(200)