Skip to content

Members & invites permission fixes #573

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 13 commits into from
May 11, 2020
Merged

Members & invites permission fixes #573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2cd7ea6
Merge branch 'feature/protect-billing-account' into hotfix/post-relea…
May 6, 2020
8b51873
Only return members with project object in these GET /projects and GE…
gets0ul May 6, 2020
613beeb
- use separate scope for invites
gets0ul May 6, 2020
cb1517e
filter invites by email in case-insensitive way
gets0ul May 8, 2020
1c3cd6a
- Fix checking project members permission logic in GET /projects endp…
gets0ul May 8, 2020
fc1a929
Fix read project members permission logic from DB
gets0ul May 9, 2020
b49c0a3
fix typo and lint error
gets0ul May 11, 2020
6103784
Merge pull request #569 from gets0ul/issue_560
maxceem May 11, 2020
4d29820
Merge branch 'feature/members-invites-permission-fixes' into issue_561
maxceem May 11, 2020
3a7f1ad
Merge pull request #570 from gets0ul/issue_561
maxceem May 11, 2020
f09e4df
fix: lint
May 11, 2020
da6dee6
fix: "generalPermission" middleware
May 11, 2020
2f82d14
Merge branch 'develop' into feature/members-invites-permission-fixes
May 11, 2020
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
48 changes: 24 additions & 24 deletions docs/permissions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,8 +636,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">read:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">read:project-invites</span>
</div>
</div>
</div>
Expand Down Expand Up @@ -670,8 +670,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">read:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">read:project-invites</span>
</div>
</div>
</div>
Expand Down Expand Up @@ -704,8 +704,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand Down Expand Up @@ -734,8 +734,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand All @@ -759,8 +759,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand All @@ -782,8 +782,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand All @@ -806,8 +806,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand All @@ -831,8 +831,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand All @@ -854,8 +854,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand All @@ -879,8 +879,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand Down Expand Up @@ -909,8 +909,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand All @@ -934,8 +934,8 @@ <h2 class="anchor-container">

<div>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-members</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">all:project-invites</span>
<span class="badge badge-dark" title="Allowed Topcoder Role">write:project-invites</span>
</div>
</div>
</div>
Expand Down
5 changes: 5 additions & 0 deletions src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,11 @@ export const M2M_SCOPES = {
READ: 'read:project-members',
WRITE: 'write:project-members',
},
PROJECT_INVITES: {
ALL: 'all:project-invites',
READ: 'read:project-invites',
WRITE: 'write:project-invites',
},
};

export const TIMELINE_REFERENCES = {
Expand Down
36 changes: 24 additions & 12 deletions src/permissions/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ const SCOPES_PROJECT_MEMBERS_WRITE = [
M2M_SCOPES.PROJECT_MEMBERS.WRITE,
];

const SCOPES_PROJECT_INVITES_READ = [
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
M2M_SCOPES.PROJECT_INVITES.ALL,
M2M_SCOPES.PROJECT_INVITES.READ,
];

const SCOPES_PROJECT_INVITES_WRITE = [
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
M2M_SCOPES.PROJECT_INVITES.ALL,
M2M_SCOPES.PROJECT_INVITES.WRITE,
];

/**
* The full list of possible permission rules in Project Service
*/
Expand Down Expand Up @@ -332,7 +344,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
description: 'Who can view own invite.',
},
topcoderRoles: ALL,
scopes: SCOPES_PROJECT_MEMBERS_READ,
scopes: SCOPES_PROJECT_INVITES_READ,
},

READ_PROJECT_INVITE_NOT_OWN: {
Expand All @@ -343,7 +355,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
},
topcoderRoles: TOPCODER_ROLES_MANAGERS_AND_ADMINS,
projectRoles: ALL,
scopes: SCOPES_PROJECT_MEMBERS_READ,
scopes: SCOPES_PROJECT_INVITES_READ,
},

CREATE_PROJECT_INVITE_CUSTOMER: {
Expand All @@ -354,7 +366,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
},
topcoderRoles: TOPCODER_ROLES_MANAGERS_AND_ADMINS,
projectRoles: ALL,
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

CREATE_PROJECT_INVITE_NON_CUSTOMER: {
Expand All @@ -365,7 +377,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
},
topcoderRoles: TOPCODER_ROLES_ADMINS,
projectRoles: PROJECT_ROLES_MANAGEMENT,
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

CREATE_PROJECT_INVITE_COPILOT_DIRECTLY: {
Expand All @@ -378,7 +390,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
...TOPCODER_ROLES_ADMINS,
USER_ROLE.COPILOT_MANAGER,
],
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

UPDATE_PROJECT_INVITE_OWN: {
Expand All @@ -388,7 +400,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
description: 'Who can update own invite.',
},
topcoderRoles: ALL,
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

UPDATE_PROJECT_INVITE_NOT_OWN: {
Expand All @@ -398,7 +410,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
description: 'Who can update invites for other members.',
},
topcoderRoles: TOPCODER_ROLES_ADMINS,
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

UPDATE_PROJECT_INVITE_REQUESTED: {
Expand All @@ -411,7 +423,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
...TOPCODER_ROLES_ADMINS,
USER_ROLE.COPILOT_MANAGER,
],
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

DELETE_PROJECT_INVITE_OWN: {
Expand All @@ -421,7 +433,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
description: 'Who can delete own invite.',
},
topcoderRoles: ALL,
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

DELETE_PROJECT_INVITE_NOT_OWN_CUSTOMER: {
Expand All @@ -432,7 +444,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
},
topcoderRoles: TOPCODER_ROLES_ADMINS,
projectRoles: ALL,
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

DELETE_PROJECT_INVITE_NOT_OWN_NON_CUSTOMER: {
Expand All @@ -443,7 +455,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
},
topcoderRoles: TOPCODER_ROLES_ADMINS,
projectRoles: PROJECT_ROLES_MANAGEMENT,
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

DELETE_PROJECT_INVITE_REQUESTED: {
Expand All @@ -456,7 +468,7 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
...TOPCODER_ROLES_ADMINS,
USER_ROLE.COPILOT_MANAGER,
],
scopes: SCOPES_PROJECT_MEMBERS_WRITE,
scopes: SCOPES_PROJECT_INVITES_WRITE,
},

/*
Expand Down
3 changes: 2 additions & 1 deletion src/permissions/generalPermission.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ module.exports = permissions => async (req) => {
// if one of the `permission` requires to know Project Members, but current route doesn't belong to any project
// this means such `permission` most likely has been applied by mistake, so we throw an error
const permissionsRequireProjectMembers = _.isArray(permissions)
? _.some(permissions, permission => util.hasPermissionByReq(permission, req))
? _.some(permissions, permission => util.isPermissionRequireProjectMembers(permission))
: util.isPermissionRequireProjectMembers(permissions);

if (_.isUndefined(req.params.projectId) && permissionsRequireProjectMembers) {
Expand All @@ -62,6 +62,7 @@ module.exports = permissions => async (req) => {
// - if user has permissions to access endpoint even we don't know if he is a member or no,
// then code would proceed and endpoint would decide to throw 404 if project doesn't exist
// or perform endpoint operation if loading project members above failed because of some other reason
req.log.error(`Cannot load project members: ${err.message}.`);
}
}

Expand Down
41 changes: 37 additions & 4 deletions src/routes/projects/get.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
import permissionUtils from '../../utils/permissions';

const ES_PROJECT_INDEX = config.get('elasticsearchConfig.indexName');
Expand Down Expand Up @@ -104,7 +105,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,
Expand All @@ -116,7 +118,26 @@ const retrieveProjectFromES = (projectId, req) => {
const es = util.getElasticSearchClient();
es.search(searchCriteria).then((docs) => {
const rows = _.map(docs.hits.hits, single => single._source); // eslint-disable-line no-underscore-dangle
accept(rows[0]);
const project = rows[0];
if (project && project.invites) {
if (!util.hasPermissionByReq(PERMISSION.READ_PROJECT_INVITE_NOT_OWN, req)) {
let invites;
if (util.hasPermissionByReq(PERMISSION.READ_PROJECT_INVITE_OWN, req)) {
// only include own invites
const currentUserId = req.authUser.userId;
const currentUserEmail = req.authUser.email;
invites = _.filter(project.invites, invite => (
(invite.userId !== null && invite.userId === currentUserId) ||
(invite.email && currentUserEmail && invite.email.toLowerCase() === currentUserEmail.toLowerCase())
));
} else {
// return empty invites
invites = [];
}
_.set(project, 'invites', invites);
}
}
accept(project);
}).catch(reject);
});
};
Expand Down Expand Up @@ -144,7 +165,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);
Expand All @@ -157,7 +180,17 @@ const retrieveProjectFromDB = (projectId, req) => {
if (attachments) {
project.attachments = attachments;
}
return models.ProjectMemberInvite.getPendingAndReguestedInvitesForProject(projectId);
if (util.hasPermissionByReq(PERMISSION.READ_PROJECT_INVITE_NOT_OWN, req)) {
// include all invites
return models.ProjectMemberInvite.getPendingAndReguestedInvitesForProject(projectId);
} else if (util.hasPermissionByReq(PERMISSION.READ_PROJECT_INVITE_OWN, req)) {
// include only own invites
const currentUserId = req.authUser.userId;
const email = req.authUser.email;
return models.ProjectMemberInvite.getPendingOrRequestedProjectInvitesForUser(projectId, email, currentUserId);
}
// empty
return Promise.resolve([]);
})
.then((invites) => {
project.invites = invites;
Expand Down
22 changes: 21 additions & 1 deletion src/routes/projects/get.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,27 @@ 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();
}
});
});

it('should return the project with empty invites using M2M token without "read:project-invites" scope', (done) => {
request(server)
.get(`/v5/projects/${project1.id}`)
.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.invites.should.be.empty;
done();
}
});
Expand Down
Loading