fix: rbac now checks permission for both projects (#838)

- When updating a toggle
   - If the project is updated, the user performing the operation
     will need UPDATE_FEATURE permission for both old and new project

fixes: #837

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);
+});

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.
+            // Temporary workaround to figure out projectId for feature toggle updates.
-            if ([UPDATE_FEATURE, DELETE_FEATURE].includes(permission)) {
+            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';
             }