diff --git a/src/lib/middleware/rbac-middleware.test.ts b/src/lib/middleware/rbac-middleware.test.ts index 74a82706b3..2ecd1d82cf 100644 --- a/src/lib/middleware/rbac-middleware.test.ts +++ b/src/lib/middleware/rbac-middleware.test.ts @@ -230,3 +230,49 @@ test('should lookup projectId from data', async t => { t.is(accessService.hasPermission.args[0][2], projectId); }); + +test('Need access to UPDATE_FEATURE on the project you change to', async t => { + const oldProjectId = 'some-project-34'; + const newProjectId = 'some-project-35'; + const featureName = 'some-feature-toggle'; + const accessService = { + hasPermission: sinon.fake.returns(true), + }; + featureToggleStore.getProjectId = sinon.fake.returns(oldProjectId); + + const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const cb = sinon.fake(); + const req: any = { + user: new User({ username: 'user', id: 1 }), + params: { featureName }, + body: { featureName, project: newProjectId }, + }; + func(req, undefined, cb); + + await req.checkRbac(perms.UPDATE_FEATURE); + t.is(accessService.hasPermission.callCount, 2); + t.is(accessService.hasPermission.args[0][2], oldProjectId); + t.is(accessService.hasPermission.args[1][2], newProjectId); +}); + +test('Does not double check permission if not changing project when updating toggle', async t => { + const oldProjectId = 'some-project-34'; + const featureName = 'some-feature-toggle'; + const accessService = { + hasPermission: sinon.fake.returns(true), + }; + featureToggleStore.getProjectId = sinon.fake.returns(oldProjectId); + + const func = rbacMiddleware(config, { featureToggleStore }, accessService); + const cb = sinon.fake(); + const req: any = { + user: new User({ username: 'user', id: 1 }), + params: { featureName }, + body: { featureName, project: oldProjectId }, + }; + func(req, undefined, cb); + + await req.checkRbac(perms.UPDATE_FEATURE); + t.is(accessService.hasPermission.callCount, 1); + t.is(accessService.hasPermission.args[0][2], oldProjectId); +}); diff --git a/src/lib/middleware/rbac-middleware.ts b/src/lib/middleware/rbac-middleware.ts index 70dc9c02d0..d91152c486 100644 --- a/src/lib/middleware/rbac-middleware.ts +++ b/src/lib/middleware/rbac-middleware.ts @@ -46,10 +46,34 @@ const rbacMiddleware = ( // For /api/admin/projects/:projectId we will find it as part of params let { projectId } = params; - // Temporary workaround to figure our projectId for feature toggle updates. - if ([UPDATE_FEATURE, DELETE_FEATURE].includes(permission)) { + // Temporary workaround to figure out projectId for feature toggle updates. + if (permission === DELETE_FEATURE) { const { featureName } = params; projectId = await featureToggleStore.getProjectId(featureName); + } else if (permission === UPDATE_FEATURE) { + // if projectId of feature is different from project in body + // need to check that we have UPDATE_FEATURE access on both old and new project + // TODO: Look at this to make it smoother once we get around to looking at project + // Changing project of a toggle should most likely be a separate endpoint + const { featureName } = params; + projectId = await featureToggleStore.getProjectId(featureName); + const newProjectId = req.body + ? req.body.project || projectId + : projectId; + if (newProjectId !== projectId) { + return ( + accessService.hasPermission( + user, + permission, + projectId, + ) && + accessService.hasPermission( + user, + permission, + newProjectId, + ) + ); + } } else if (permission === CREATE_FEATURE) { projectId = req.body.project || 'default'; }