Skip to content

Commit 266ff0e

Browse files
maxceemmaxceem
authored
Merge pull request #564 from topcoder-platform/feature/protect-billing-account
Protect billing account
2 parents 95e1638 + 2de283b commit 266ff0e

File tree

7 files changed

+121
-22
lines changed

7 files changed

+121
-22
lines changed

‎docs/permissions.html‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,10 +273,10 @@ <h2 class="anchor-container">
273273
<div class="row border-top">
274274
<div class="col py-2">
275275
<div class="permission-title anchor-container">
276-
<a href="#UPDATE_PROJECT_DIRECT_PROJECT_ID" name="UPDATE_PROJECT_DIRECT_PROJECT_ID" class="anchor"></a>Update Project property &quot;directProjectId&quot;
276+
<a href="#MANAGE_PROJECT_DIRECT_PROJECT_ID" name="MANAGE_PROJECT_DIRECT_PROJECT_ID" class="anchor"></a>Manage Project property &quot;directProjectId&quot;
277277
</div>
278-
<div class="permission-variable"><small><code>UPDATE_PROJECT_DIRECT_PROJECT_ID</code></small></div>
279-
<div class="text-black-50 small-text"></div>
278+
<div class="permission-variable"><small><code>MANAGE_PROJECT_DIRECT_PROJECT_ID</code></small></div>
279+
<div class="text-black-50 small-text">Who can set or update the &quot;directProjectId&quot; property.</div>
280280
</div>
281281
<div class="col-9 py-2">
282282
<div>
@@ -294,6 +294,29 @@ <h2 class="anchor-container">
294294
</div>
295295
</div>
296296
</div>
297+
<div class="row border-top">
298+
<div class="col py-2">
299+
<div class="permission-title anchor-container">
300+
<a href="#MANAGE_PROJECT_BILLING_ACCOUNT_ID" name="MANAGE_PROJECT_BILLING_ACCOUNT_ID" class="anchor"></a>Manage Project property &quot;billingAccountId&quot;
301+
</div>
302+
<div class="permission-variable"><small><code>MANAGE_PROJECT_BILLING_ACCOUNT_ID</code></small></div>
303+
<div class="text-black-50 small-text">Who can set or update the &quot;billingAccountId&quot; property.</div>
304+
</div>
305+
<div class="col-9 py-2">
306+
<div>
307+
</div>
308+
309+
<div>
310+
<span class="badge badge-success" title="Allowed Topcoder Role">Connect Manager</span>
311+
<span class="badge badge-success" title="Allowed Topcoder Role">administrator</span>
312+
</div>
313+
314+
<div>
315+
<span class="badge badge-dark" title="Allowed Topcoder Role">all:connect_project</span>
316+
<span class="badge badge-dark" title="Allowed Topcoder Role">write:projects-billing-accounts</span>
317+
</div>
318+
</div>
319+
</div>
297320
<div class="row border-top">
298321
<div class="col py-2">
299322
<div class="permission-title anchor-container">

‎src/constants.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,11 +268,13 @@ export const REGEX = {
268268
};
269269

270270
export const M2M_SCOPES = {
271+
// for backward compatibility we should allow ALL M2M operations with `CONNECT_PROJECT_ADMIN`
271272
CONNECT_PROJECT_ADMIN: 'all:connect_project',
272273
PROJECTS: {
273274
ALL: 'all:projects',
274275
READ: 'read:projects',
275276
WRITE: 'write:projects',
277+
WRITE_BILLING_ACCOUNTS: 'write:projects-billing-accounts',
276278
},
277279
PROJECT_MEMBERS: {
278280
ALL: 'all:project-members',

‎src/permissions/constants.js‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ const SCOPES_PROJECTS_WRITE = [
7474
M2M_SCOPES.PROJECTS.WRITE,
7575
];
7676

77+
const SCOPES_PROJECTS_WRITE_BILLING_ACCOUNTS = [
78+
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
79+
M2M_SCOPES.PROJECTS.WRITE_BILLING_ACCOUNTS,
80+
];
81+
7782
const SCOPES_PROJECT_MEMBERS_READ = [
7883
M2M_SCOPES.CONNECT_PROJECT_ADMIN,
7984
M2M_SCOPES.PROJECT_MEMBERS.ALL,
@@ -142,10 +147,11 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
142147
scopes: SCOPES_PROJECTS_WRITE,
143148
},
144149

145-
UPDATE_PROJECT_DIRECT_PROJECT_ID: {
150+
MANAGE_PROJECT_DIRECT_PROJECT_ID: {
146151
meta: {
147-
title: 'Update Project property "directProjectId"',
152+
title: 'Manage Project property "directProjectId"',
148153
group: 'Project',
154+
description: 'Who can set or update the "directProjectId" property.',
149155
},
150156
topcoderRoles: [
151157
USER_ROLE.MANAGER,
@@ -154,6 +160,19 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
154160
scopes: SCOPES_PROJECTS_WRITE,
155161
},
156162

163+
MANAGE_PROJECT_BILLING_ACCOUNT_ID: {
164+
meta: {
165+
title: 'Manage Project property "billingAccountId"',
166+
group: 'Project',
167+
description: 'Who can set or update the "billingAccountId" property.',
168+
},
169+
topcoderRoles: [
170+
USER_ROLE.MANAGER,
171+
USER_ROLE.TOPCODER_ADMIN,
172+
],
173+
scopes: SCOPES_PROJECTS_WRITE_BILLING_ACCOUNTS,
174+
},
175+
157176
DELETE_PROJECT: {
158177
meta: {
159178
title: 'Delete Project',

‎src/routes/projects/create.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,18 @@ module.exports = [
387387
*/
388388
(req, res, next) => {
389389
const project = req.body;
390+
if (_.has(project, 'directProjectId') &&
391+
!util.hasPermissionByReq(PERMISSION.MANAGE_PROJECT_DIRECT_PROJECT_ID, req)) {
392+
const err = new Error('You do not have permission to set \'directProjectId\' property');
393+
err.status = 400;
394+
throw err;
395+
}
396+
if (_.has(project, 'billingAccountId') &&
397+
!util.hasPermissionByReq(PERMISSION.MANAGE_PROJECT_BILLING_ACCOUNT_ID, req)) {
398+
const err = new Error('You do not have permission to set \'billingAccountId\' property');
399+
err.status = 400;
400+
throw err;
401+
}
390402
// by default connect admin and managers joins projects as manager
391403
const userRole = util.hasPermissionByReq(PERMISSION.CREATE_PROJECT_AS_MANAGER, req)
392404
? PROJECT_MEMBER_ROLE.MANAGER

‎src/routes/projects/create.spec.js‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,6 @@ describe('Project create', () => {
265265
type: 'generic',
266266
description: 'test project',
267267
details: {},
268-
billingAccountId: 1,
269268
name: 'test project1',
270269
bookmarks: [{
271270
title: 'title1',
@@ -277,7 +276,6 @@ describe('Project create', () => {
277276
type: 'generic',
278277
description: 'test project',
279278
details: {},
280-
billingAccountId: 1,
281279
name: 'test project1',
282280
attachments: [
283281
{
@@ -399,6 +397,34 @@ describe('Project create', () => {
399397
.expect(400, done);
400398
});
401399

400+
it(`should return 400 when creating project with billingAccountId
401+
without "write:projects-billing-accounts" scope in M2M token`, (done) => {
402+
const validBody = _.cloneDeep(body);
403+
validBody.billingAccountId = 1;
404+
request(server)
405+
.post('/v5/projects')
406+
.set({
407+
Authorization: `Bearer ${testUtil.m2m['write:projects']}`,
408+
})
409+
.send(validBody)
410+
.expect('Content-Type', /json/)
411+
.expect(400, done);
412+
});
413+
414+
it(`should return 400 when creating project with directProjectId
415+
without "write:projects" scope in M2M token`, (done) => {
416+
const validBody = _.cloneDeep(body);
417+
validBody.directProjectId = 1;
418+
request(server)
419+
.post('/v5/projects')
420+
.set({
421+
Authorization: `Bearer ${testUtil.m2m['write:project-members']}`,
422+
})
423+
.send(validBody)
424+
.expect('Content-Type', /json/)
425+
.expect(400, done);
426+
});
427+
402428
it('should return 201 if valid user and data', (done) => {
403429
const validBody = _.cloneDeep(body);
404430
validBody.templateId = 3;
@@ -433,7 +459,7 @@ describe('Project create', () => {
433459
} else {
434460
const resJson = res.body;
435461
should.exist(resJson);
436-
should.exist(resJson.billingAccountId);
462+
should.not.exist(resJson.billingAccountId);
437463
should.exist(resJson.name);
438464
resJson.status.should.be.eql('in_review');
439465
resJson.type.should.be.eql(body.type);
@@ -489,7 +515,7 @@ describe('Project create', () => {
489515
} else {
490516
const resJson = res.body;
491517
should.exist(resJson);
492-
should.exist(resJson.billingAccountId);
518+
should.not.exist(resJson.billingAccountId);
493519
should.exist(resJson.name);
494520
resJson.status.should.be.eql('in_review');
495521
resJson.type.should.be.eql(body.type);
@@ -544,7 +570,7 @@ describe('Project create', () => {
544570
} else {
545571
const resJson = res.body;
546572
should.exist(resJson);
547-
should.exist(resJson.billingAccountId);
573+
should.not.exist(resJson.billingAccountId);
548574
should.exist(resJson.name);
549575
resJson.status.should.be.eql('in_review');
550576
resJson.type.should.be.eql(body.type);
@@ -598,7 +624,7 @@ describe('Project create', () => {
598624
} else {
599625
const resJson = res.body;
600626
should.exist(resJson);
601-
should.exist(resJson.billingAccountId);
627+
should.not.exist(resJson.billingAccountId);
602628
should.exist(resJson.name);
603629
resJson.status.should.be.eql('in_review');
604630
resJson.type.should.be.eql(bodyWithAttachments.type);
@@ -679,7 +705,7 @@ describe('Project create', () => {
679705
} else {
680706
const resJson = res.body;
681707
should.exist(resJson);
682-
should.exist(resJson.billingAccountId);
708+
should.not.exist(resJson.billingAccountId);
683709
should.exist(resJson.name);
684710
resJson.status.should.be.eql('in_review');
685711
resJson.type.should.be.eql(body.type);
@@ -752,7 +778,7 @@ describe('Project create', () => {
752778
} else {
753779
const resJson = res.body;
754780
should.exist(resJson);
755-
should.exist(resJson.billingAccountId);
781+
should.not.exist(resJson.billingAccountId);
756782
should.exist(resJson.name);
757783
resJson.status.should.be.eql('in_review');
758784
resJson.type.should.be.eql(body.type);
@@ -885,7 +911,7 @@ describe('Project create', () => {
885911
} else {
886912
const resJson = res.body;
887913
should.exist(resJson);
888-
should.exist(resJson.billingAccountId);
914+
should.not.exist(resJson.billingAccountId);
889915
should.exist(resJson.name);
890916
resJson.status.should.be.eql('in_review');
891917
resJson.type.should.be.eql(body.type);

‎src/routes/projects/update.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,12 @@ const validateUpdates = (existingProject, updatedProps, req) => {
143143
// }
144144
}
145145
if (_.has(updatedProps, 'directProjectId') &&
146-
!util.hasPermissionByReq(PERMISSION.UPDATE_PROJECT_DIRECT_PROJECT_ID, req)) {
147-
errors.push('Don\'t have permission to update \'directProjectId\' property');
146+
!util.hasPermissionByReq(PERMISSION.MANAGE_PROJECT_DIRECT_PROJECT_ID, req)) {
147+
errors.push('You do not have permission to update \'directProjectId\' property');
148+
}
149+
if (_.has(updatedProps, 'billingAccountId') &&
150+
!util.hasPermissionByReq(PERMISSION.MANAGE_PROJECT_BILLING_ACCOUNT_ID, req)) {
151+
errors.push('You do not have permission to update \'billingAccountId\' property');
148152
}
149153
if ((existingProject.status !== PROJECT_STATUS.DRAFT) && (updatedProps.status === PROJECT_STATUS.DRAFT)) {
150154
errors.push('cannot update a project status to draft');

‎src/routes/projects/update.spec.js‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import {
1414
PROJECT_STATUS,
1515
BUS_API_EVENT,
1616
CONNECT_NOTIFICATION_EVENT,
17-
M2M_SCOPES,
1817
} from '../../constants';
1918

2019
const should = chai.should();
@@ -192,11 +191,11 @@ describe('Project', () => {
192191
});
193192
});
194193

195-
it(`should return the project using M2M token with "${M2M_SCOPES.PROJECTS.WRITE}" scope`, (done) => {
194+
it('should return the project using M2M token with "write:projects" scope', (done) => {
196195
request(server)
197196
.patch(`/v5/projects/${project1.id}`)
198197
.set({
199-
Authorization: `Bearer ${testUtil.m2m[M2M_SCOPES.PROJECTS.WRITE]}`,
198+
Authorization: `Bearer ${testUtil.m2m['write:projects']}`,
200199
})
201200
.send({
202201
name: 'updateProject name by M2M',
@@ -584,7 +583,7 @@ describe('Project', () => {
584583
request(server)
585584
.patch(`/v5/projects/${project1.id}`)
586585
.set({
587-
Authorization: `Bearer ${testUtil.jwts.copilot}`,
586+
Authorization: `Bearer ${testUtil.jwts.manager}`,
588587
})
589588
.send({
590589
billingAccountId: 123,
@@ -600,7 +599,7 @@ describe('Project', () => {
600599
should.exist(resJson);
601600
resJson.billingAccountId.should.equal(123);
602601
resJson.updatedAt.should.not.equal('2016-06-30 00:33:07+00');
603-
resJson.updatedBy.should.equal(40051332);
602+
resJson.updatedBy.should.equal(40051334);
604603
server.services.pubsub.publish.calledWith('project.updated').should.be.true;
605604
done();
606605
}
@@ -611,7 +610,7 @@ describe('Project', () => {
611610
request(server)
612611
.patch(`/v5/projects/${project1.id}`)
613612
.set({
614-
Authorization: `Bearer ${testUtil.jwts.copilot}`,
613+
Authorization: `Bearer ${testUtil.jwts.manager}`,
615614
})
616615
.send({
617616
billingAccountId: 1,
@@ -659,6 +658,20 @@ describe('Project', () => {
659658
});
660659
});
661660

661+
it('should return 400 when updating billingAccountId without "write:projects-billing-accounts" scope in M2M token',
662+
(done) => {
663+
request(server)
664+
.patch(`/v5/projects/${project1.id}`)
665+
.set({
666+
Authorization: `Bearer ${testUtil.m2m['write:projects']}`,
667+
})
668+
.send({
669+
billingAccountId: 123,
670+
})
671+
.expect('Content-Type', /json/)
672+
.expect(400, done);
673+
});
674+
662675
it.skip('should return 200 and update bookmarks', (done) => {
663676
request(server)
664677
.patch(`/v5/projects/${project1.id}`)

0 commit comments

Comments
 (0)