Skip to content

Commit 4d29820

Browse files
authored
Merge branch 'feature/members-invites-permission-fixes' into issue_561
2 parents b49c0a3 + 6103784 commit 4d29820

File tree

4 files changed

+72
-7
lines changed

4 files changed

+72
-7
lines changed

src/routes/projects/get.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ const retrieveProjectFromES = (projectId, req) => {
104104
fields = fields ? fields.split(',') : [];
105105
fields = util.parseFields(fields, {
106106
projects: PROJECT_ATTRIBUTES,
107-
project_members: util.addUserDetailsFieldsIfAllowed(PROJECT_MEMBER_ATTRIBUTES_ES, req),
107+
project_members: util.hasPermissionByReq(PERMISSION.READ_PROJECT_MEMBER, req)
108+
? util.addUserDetailsFieldsIfAllowed(PROJECT_MEMBER_ATTRIBUTES_ES, req) : null,
108109
project_member_invites: PROJECT_MEMBER_INVITE_ATTRIBUTES,
109110
project_phases: PROJECT_PHASE_ATTRIBUTES,
110111
project_phases_products: PROJECT_PHASE_PRODUCTS_ATTRIBUTES,
@@ -163,7 +164,9 @@ const retrieveProjectFromDB = (projectId, req) => {
163164
return Promise.reject(apiErr);
164165
}
165166
// check context for project members
166-
project.members = _.map(req.context.currentProjectMembers, m => _.pick(m, fields.project_members));
167+
if (util.hasPermissionByReq(PERMISSION.READ_PROJECT_MEMBER, req)) {
168+
project.members = _.map(req.context.currentProjectMembers, m => _.pick(m, fields.project_members));
169+
}
167170
// check if attachments field was requested
168171
if (!req.query.fields || _.indexOf(req.query.fields, 'attachments') > -1) {
169172
return util.getProjectAttachments(req, project.id);

src/routes/projects/get.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ describe('GET Project', () => {
268268
should.not.exist(resJson.billingAccountId);
269269
should.exist(resJson.name);
270270
resJson.status.should.be.eql('draft');
271-
resJson.members.should.have.lengthOf(2);
271+
should.not.exist(resJson.members);
272272
done();
273273
}
274274
});

src/routes/projects/list.js

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,11 @@ const retrieveProjectsFromDB = (req, criteria, sort, ffields) => {
492492

493493
// make sure project.id is part of fields
494494
if (_.indexOf(fields.projects, 'id') < 0) fields.projects.push('id');
495+
// add userId to project_members field so it can be used to check READ_PROJECT_MEMBER permission below.
496+
const addMembersUserId = fields.project_members.length > 0 && _.indexOf(fields.project_members, 'userId') < 0;
497+
if (addMembersUserId) {
498+
fields.project_members.push('userId');
499+
}
495500
const retrieveAttachments = !req.query.fields || req.query.fields.indexOf('attachments') > -1;
496501
const retrieveMembers = !req.query.fields || !!fields.project_members.length;
497502

@@ -533,7 +538,19 @@ const retrieveProjectsFromDB = (req, criteria, sort, ffields) => {
533538
const p = fp;
534539
// if values length is 1 it could be either attachments or members
535540
if (retrieveMembers) {
536-
p.members = _.filter(allMembers, m => m.projectId === p.id);
541+
const pMembers = _.filter(allMembers, m => m.projectId === p.id);
542+
// check if have permission to read project members
543+
if (util.hasPermission(PERMISSION.READ_PROJECT_MEMBER, req.authUser, pMembers)) {
544+
if (addMembersUserId) {
545+
// remove the userId from the returned members array if it was added before
546+
// as it is only needed for checking permission.
547+
_.forEach(pMembers, (m) => {
548+
const fm = m;
549+
delete fm.userId;
550+
});
551+
}
552+
p.members = pMembers;
553+
}
537554
}
538555
if (retrieveAttachments) {
539556
p.attachments = _.filter(allAttachments, a => a.projectId === p.id);
@@ -562,6 +579,11 @@ const retrieveProjects = (req, criteria, sort, ffields) => {
562579
if (_.indexOf(fields.projects, 'id') < 0) {
563580
fields.projects.push('id');
564581
}
582+
// add userId to project_members field so it can be used to check READ_PROJECT_MEMBER permission below.
583+
const addMembersUserId = fields.project_members.length > 0 && _.indexOf(fields.project_members, 'userId') < 0;
584+
if (addMembersUserId) {
585+
fields.project_members.push('userId');
586+
}
565587

566588
const searchCriteria = parseElasticSearchCriteria(criteria, fields, order) || {};
567589
return new Promise((accept, reject) => {
@@ -588,6 +610,23 @@ const retrieveProjects = (req, criteria, sort, ffields) => {
588610
});
589611
}
590612
}
613+
_.forEach(rows, (p) => {
614+
const fp = p;
615+
if (fp.members) {
616+
// check if have permission to read project members
617+
if (!util.hasPermission(PERMISSION.READ_PROJECT_MEMBER, req.authUser, fp.members)) {
618+
delete fp.members;
619+
}
620+
if (fp.members && addMembersUserId) {
621+
// remove the userId from the returned members array if it was added before
622+
// as it is only needed for checking permission.
623+
_.forEach(fp.members, (m) => {
624+
const fm = m;
625+
delete fm.userId;
626+
});
627+
}
628+
}
629+
});
591630
}
592631
accept({ rows, count: docs.hits.total, pageSize: criteria.limit, page: criteria.page });
593632
}).catch(reject);

src/routes/projects/list.spec.js

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ describe('LIST Project', () => {
404404
}
405405
});
406406
});
407-
407+
408408
it('should return the project with empty invites using M2M token without "read:project-invites" scope', (done) => {
409409
request(server)
410410
.get('/v5/projects')
@@ -428,6 +428,29 @@ describe('LIST Project', () => {
428428
});
429429
});
430430

431+
it('should not include the project members using M2M token without "read:project-members" scope', (done) => {
432+
request(server)
433+
.get('/v5/projects')
434+
.set({
435+
Authorization: `Bearer ${testUtil.m2m['read:projects']}`,
436+
})
437+
.expect('Content-Type', /json/)
438+
.expect(200)
439+
.end((err, res) => {
440+
if (err) {
441+
done(err);
442+
} else {
443+
const resJson = res.body;
444+
should.exist(resJson);
445+
resJson.should.have.lengthOf(3);
446+
resJson.forEach((project) => {
447+
should.not.exist(project.members);
448+
});
449+
done();
450+
}
451+
});
452+
});
453+
431454
it('should return the project when project that is in reviewed state in which the copilot is its member or has been invited', (done) => {
432455
request(server)
433456
.get('/v5/projects')
@@ -1186,7 +1209,7 @@ describe('LIST Project', () => {
11861209
request(server)
11871210
.get('/v5/projects/')
11881211
.set({
1189-
Authorization: `Bearer ${testUtil.jwts.member2}`,
1212+
Authorization: `Bearer ${testUtil.jwts.member}`,
11901213
})
11911214
.expect('Content-Type', /json/)
11921215
.expect(200)
@@ -1208,7 +1231,7 @@ describe('LIST Project', () => {
12081231
request(server)
12091232
.get('/v5/projects/?fields=members.email,members.id')
12101233
.set({
1211-
Authorization: `Bearer ${testUtil.jwts.member2}`,
1234+
Authorization: `Bearer ${testUtil.jwts.member}`,
12121235
})
12131236
.expect('Content-Type', /json/)
12141237
.expect(200)

0 commit comments

Comments
 (0)