Skip to content

Commit 34af2aa

Browse files
committed
refactor: allow "email" field instead of disallow
we had the logic which removed "email" field from the allowed fields for non-admins this logic has been rewritten to add "email" field to the allowed fields for admins to avoid accidental "email" leaking
1 parent a1c98b3 commit 34af2aa

File tree

3 files changed

+25
-29
lines changed

3 files changed

+25
-29
lines changed

src/routes/projects/get.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,12 @@ const ES_PROJECT_TYPE = config.get('elasticsearchConfig.docType');
2222
// var permissions = require('tc-core-library-js').middleware.permissions
2323
const permissions = tcMiddleware.permissions;
2424
const PROJECT_ATTRIBUTES = _.without(_.keys(models.Project.rawAttributes), 'utm', 'deletedAt');
25-
const PROJECT_MEMBER_ATTRIBUTES = _.concat(_.without(_.keys(models.ProjectMember.rawAttributes), 'deletedAt'),
26-
['firstName', 'lastName', 'handle', 'email']);
25+
const PROJECT_MEMBER_ATTRIBUTES = _.concat(_.without(_.keys(models.ProjectMember.rawAttributes), 'deletedAt'));
26+
// project members has some additional fields stored in ES index, which we don't have in DB
27+
const PROJECT_MEMBER_ATTRIBUTES_ES = _.concat(
28+
PROJECT_MEMBER_ATTRIBUTES,
29+
['firstName', 'lastName', 'handle'], // 'email' can be added when allowed by `addEmailFieldIfAllowed`
30+
);
2731
const PROJECT_MEMBER_INVITE_ATTRIBUTES = _.without(_.keys(models.ProjectMemberInvite.rawAttributes), 'deletedAt');
2832
const PROJECT_ATTACHMENT_ATTRIBUTES = _.without(_.keys(models.ProjectAttachment.rawAttributes), 'deletedAt');
2933
const PROJECT_PHASE_ATTRIBUTES = _.without(
@@ -102,16 +106,13 @@ const retrieveProjectFromES = (projectId, req) => {
102106
fields = fields ? fields.split(',') : [];
103107
fields = util.parseFields(fields, {
104108
projects: PROJECT_ATTRIBUTES,
105-
project_members: PROJECT_MEMBER_ATTRIBUTES,
109+
project_members: util.addEmailFieldIfAllowed(PROJECT_MEMBER_ATTRIBUTES_ES, req),
106110
project_member_invites: PROJECT_MEMBER_INVITE_ATTRIBUTES,
107111
project_phases: PROJECT_PHASE_ATTRIBUTES,
108112
project_phases_products: PROJECT_PHASE_PRODUCTS_ATTRIBUTES,
109113
attachments: PROJECT_ATTACHMENT_ATTRIBUTES,
110114
});
111115

112-
// if user is not admin, ignore email field for project_members
113-
fields = util.ignoreEmailField(req, fields);
114-
115116
const searchCriteria = parseElasticSearchCriteria(projectId, fields) || {};
116117
return new Promise((accept, reject) => {
117118
const es = util.getElasticSearchClient();

src/routes/projects/list.js

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ const PROJECT_ATTRIBUTES = _.without(_.keys(models.Project.rawAttributes),
2626
'utm',
2727
'deletedAt',
2828
);
29-
const PROJECT_MEMBER_ATTRIBUTES = _.concat(_.without(
30-
_.keys(models.ProjectMember.rawAttributes),
31-
'deletedAt',
32-
), ['firstName', 'lastName', 'handle', 'email']);
29+
const PROJECT_MEMBER_ATTRIBUTES = _.without(_.keys(models.ProjectMember.rawAttributes));
30+
// project members has some additional fields stored in ES index, which we don't have in DB
31+
const PROJECT_MEMBER_ATTRIBUTES_ES = _.concat(
32+
PROJECT_MEMBER_ATTRIBUTES,
33+
['firstName', 'lastName', 'handle'], // 'email' can be added when allowed by `addEmailFieldIfAllowed`
34+
);
3335
const PROJECT_MEMBER_INVITE_ATTRIBUTES = _.without(
3436
_.keys(models.ProjectMemberInvite.rawAttributes),
3537
'deletedAt',
@@ -551,17 +553,13 @@ const retrieveProjects = (req, criteria, sort, ffields) => {
551553
// parse the fields string to determine what fields are to be returned
552554
fields = util.parseFields(fields, {
553555
projects: PROJECT_ATTRIBUTES,
554-
project_members: PROJECT_MEMBER_ATTRIBUTES,
556+
project_members: util.addEmailFieldIfAllowed(PROJECT_MEMBER_ATTRIBUTES_ES, req),
555557
project_member_invites: PROJECT_MEMBER_INVITE_ATTRIBUTES,
556558
project_phases: PROJECT_PHASE_ATTRIBUTES,
557559
project_phases_products: PROJECT_PHASE_PRODUCTS_ATTRIBUTES,
558560
attachments: PROJECT_ATTACHMENT_ATTRIBUTES,
559561
});
560562

561-
562-
// if user is not admin, ignore email field for project_members
563-
fields = util.ignoreEmailField(req, fields);
564-
565563
// make sure project.id is part of fields
566564
if (_.indexOf(fields.projects, 'id') < 0) {
567565
fields.projects.push('id');

src/util.js

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -262,27 +262,24 @@ _.assignIn(util, {
262262
}
263263
return fields;
264264
},
265+
265266
/**
266-
* Remove email field for PROJECT_MEMBER_ATTRIBUTES, if user is not admin
267-
* @param {object} req request object
268-
* @param {object} fields fields object
269-
* @return {object} the parsed array
267+
* Add `email` field to the list of field, if it's allowed to a user who made the request
268+
*
269+
* @param {Array} fields fields list
270+
* @param {Object} req request object
271+
*
272+
* @return {Array} fields list with 'email' if allowed
270273
*/
271-
ignoreEmailField: (req, fields) => {
272-
if (!fields.project_members) {
273-
return fields;
274-
}
275-
276-
// Only Topcoder Admins can get all the fields
274+
addEmailFieldIfAllowed: (fields, req) => {
275+
// Only Topcoder Admins can get email
277276
if (util.hasPermission({ topcoderRoles: [USER_ROLE.TOPCODER_ADMIN] }, req.authUser)) {
278-
return fields;
277+
return _.concat(fields, ['email']);
279278
}
280279

281-
// for non topcoder admins remove emails from the field list
282-
_.assign(fields, { project_members: _.filter(fields.project_members, f => f !== 'email') });
283-
284280
return fields;
285281
},
282+
286283
/**
287284
* Parse the query filters
288285
* @param {String} fqueryFilter the query filter string

0 commit comments

Comments
 (0)