Add CS plugin permissions

packages/circus-api/src/api/admin/groups/index.test.ts

@@ -50,7 +50,14 @@ const basicGroupData = {
   writeProjects: [],
   addSeriesProjects: [],
   viewPersonalInfoProjects: [],
-  moderateProjects: []
+  moderateProjects: [],
+  readPlugin: [],
+  executePlugin: [],
+  manageJobs: [],
+  inputPersonalFeedback: [],
+  inputConsensualFeedback: [],
+  manageFeedback: [],
+  viewPersonalInfo: []
 };
 
 it('should add a new group', async () => {
@@ -87,6 +94,65 @@ it('should return error for invalid group update', async () => {
   expect(res3.data.error).toMatch(/Group ID cannot be specified/);
 });
 
+const permissionsDependencies = {
+  writeProjects: ['readProjects'],
+  viewPersonalInfoProjects: ['readProjects'],
+  moderateProjects: ['readProjects'],
+  addSeriesProjects: ['readProjects'],
+  executePlugin: ['readPlugin'],
+  manageJobs: ['readPlugin'],
+  inputPersonalFeedback: ['readPlugin'],
+  inputConsensualFeedback: ['readPlugin'],
+  manageFeedback: ['readPlugin'],
+  viewPersonalInfo: ['readPlugin']
+};
+
+const updateGroupData = async (
+  data: typeof basicGroupData,
+  expectedStatus: number
+) => {
+  const res = await axios.request({
+    method: 'patch',
+    url: 'api/admin/groups/1',
+    data
+  });
+  expect(res.status).toBe(expectedStatus);
+};
+
+describe('group permission updates', () => {
+  describe('success cases', () => {
+    it.each(Object.entries(permissionsDependencies))(
+      '%s: should update successfully when dependencies are met',
+      async (permission, dependencies) => {
+        const data = {
+          ...basicGroupData,
+          [permission]: ['sampleId'],
+          ...dependencies.reduce(
+            (acc, dep) => ({ ...acc, [dep]: ['sampleId'] }),
+            {}
+          )
+        };
+
+        await updateGroupData(data, 204);
+      }
+    );
+  });
+
+  describe('failure cases', () => {
+    it.each(Object.entries(permissionsDependencies))(
+      '%s: should fail to update when dependencies are not met',
+      async permission => {
+        const data = {
+          ...basicGroupData,
+          [permission]: ['sampleId']
+        };
+
+        await updateGroupData(data, 400);
+      }
+    );
+  });
+});
+
 it('should return global-privileges', async () => {
   const res = await axios('api/admin/global-privileges');
   expect(res.data.length).toBeGreaterThan(1);

packages/circus-api/src/api/admin/groups/index.ts

@@ -1,7 +1,7 @@
 import status from 'http-status';
-import performSearch from '../../performSearch';
 import { globalPrivileges } from '../../../privilegeUtils';
 import { RouteMiddleware } from '../../../typings/middlewares';
+import performSearch from '../../performSearch';
 
 export const handleSearch: RouteMiddleware = ({ models }) => {
   return async (ctx, next) => {
@@ -20,8 +20,49 @@ export const handleGet: RouteMiddleware = ({ models }) => {
   };
 };
 
+const permissionsDependencies = {
+  writeProjects: ['readProjects'],
+  viewPersonalInfoProjects: ['readProjects'],
+  moderateProjects: ['readProjects'],
+  addSeriesProjects: ['readProjects'],
+  executePlugin: ['readPlugin'],
+  manageJobs: ['readPlugin'],
+  inputPersonalFeedback: ['readPlugin'],
+  inputConsensualFeedback: ['readPlugin'],
+  manageFeedback: ['readPlugin'],
+  viewPersonalInfo: ['readPlugin']
+};
+
+const checkPermissionsConsistency = (permissions: any) => {
+  for (const [permission, dependencies] of Object.entries(
+    permissionsDependencies
+  )) {
+    const ids = permissions[permission] || [];
+
+    for (const dependency of dependencies) {
+      const dependentIds = permissions[dependency] || [];
+
+      const inconsistentIds = ids.filter(
+        (id: any) => !dependentIds.includes(id)
+      );
+      if (inconsistentIds.length > 0) {
+        throw new Error(
+          `All items with ${permission} must have ${dependency}. Inconsistent IDs: ${inconsistentIds.join(
+            ', '
+          )}`
+        );
+      }
+    }
+  }
+};
+
 export const handlePatch: RouteMiddleware = ({ models }) => {
   return async (ctx, next) => {
+    try {
+      checkPermissionsConsistency(ctx.request.body);
+    } catch (err: any) {
+      ctx.throw(status.BAD_REQUEST, err.message);
+    }
     const groupId = parseInt(ctx.params.groupId);
     await models.group.modifyOne(groupId, ctx.request.body);
     ctx.body = null;

packages/circus-api/src/api/plugin-jobs/index.test.ts

@@ -17,10 +17,12 @@ beforeAll(async () => {
 
 afterAll(async () => await apiTest.tearDown());
 
-// (bob belongs to vega.org domain)
+// (alice belongs to sirius.org domain, and has readPlugin and manageFeedback permissions)
+// (bob belongs to vega.org domain, and has readPlugin, executePlugin and inputPersonalFeedback permissions)
 // (guest belongs to no domain)
-// (dave belongs to sirius.org and vega.org domain)
-// (frank belongs to sirius.org and vega.org domain)
+// (dave belongs to sirius.org and vega.org domain, and
+//  has readPlugin, executePlugin, manageJobs, inputConsensualFeedback, inputPersonalFeedback, manageFeedback, and viewPersonalInfo permissions)
+// (frank belongs to sirius.org and vega.org domain, and has readPlugin permission)
 
 // 111.222.333.444.444: (  1) [sirius.org]
 // 111.222.333.444.555: (150) [sirius.org]
@@ -61,7 +63,7 @@ describe('plugin-job search', () => {
   test('Filter by power user domain', async () => {
     const res = await bob.get('api/plugin-jobs');
     expect(res.status).toBe(200);
-    expect(res.data.items).toHaveLength(3);
+    expect(res.data.items).toHaveLength(2);
   });
 
   test('Filter by guest domain', async () => {
@@ -78,6 +80,17 @@ describe('plugin-job search', () => {
     expect(res.status).toBe(400);
   });
 
+  test('should not be able to filter by patientInfo if user has no viewPersonalInfo', async () => {
+    // Alice has global privilege `personalInfoView` but no viewPersonalInfo for plugins.
+    const res = await alice.get('api/plugin-jobs', {
+      params: {
+        filter: JSON.stringify({ 'patientInfo.patientName': 'Sakura' })
+      }
+    });
+    expect(res.status).toBe(200);
+    expect(res.data.items).toHaveLength(0);
+  });
+
   test('should be searchable if patient information is not used', async () => {
     const res = await frank.get('api/plugin-jobs', {
       params: {
@@ -156,7 +169,7 @@ describe('search by mylist', () => {
 });
 
 describe('plugin-job registration', () => {
-  test('should register a new plug-in job', async () => {
+  test('should register a new plug-in job if a user has executePlugin', async () => {
     const res = await dave.request({
       method: 'post',
       url: 'api/plugin-jobs',
@@ -174,6 +187,24 @@ describe('plugin-job registration', () => {
     expect(res.status).toBe(201);
   });
 
+  test('should not register a new plug-in job if a user has no executePlugin', async () => {
+    const res = await frank.request({
+      method: 'post',
+      url: 'api/plugin-jobs',
+      data: {
+        pluginId:
+          'd135e1fbb368e35f940ae8e6deb171e90273958dc3938de5a8237b73bb42d9c2',
+        series: [
+          {
+            seriesUid: '111.222.333.444.555',
+            partialVolumeDescriptor: { start: 25, end: 85, delta: 3 }
+          }
+        ]
+      }
+    });
+    expect(res.status).toBe(400);
+  });
+
   test('should reject if series image out of range', async () => {
     // Series image out of range
     const res = await dave.request({
@@ -314,6 +345,15 @@ describe('job invalidation', () => {
     });
     expect(res.status).toBe(status.UNPROCESSABLE_ENTITY);
   });
+
+  test('invalidation fails if a user has no manageJobs', async () => {
+    const res = await frank.request({
+      method: 'patch',
+      url: 'api/plugin-jobs/01f5dt3qn9877g072t9y7h7pjp',
+      data: { status: 'invalidated' }
+    });
+    expect(res.status).toBe(status.UNAUTHORIZED);
+  });
 });
 
 test('should return a finished plug-in job', async () => {
@@ -365,6 +405,13 @@ describe('delete feedback', () => {
     expect(await getCount()).toBe(2);
   });
 
+  test('should not delete one feedback if a user has no manageFeedback', async () => {
+    const res = await frank.delete(
+      `api/plugin-jobs/${jobId}/feedback/01grtppnyxbdherkv2krd2n72a`
+    );
+    expect(res.status).toBe(status.UNAUTHORIZED);
+  });
+
   test('return 404 for non-existing feedback', async () => {
     const res = await alice.delete(
       `api/plugin-jobs/${jobId}/feedback/thisfeedbackdoesnotexist`

packages/circus-api/src/api/plugin-jobs/index.ts

@@ -75,6 +75,18 @@ export const handlePatch: RouteMiddleware = ({ transactionManager, cs }) => {
     const jobId = ctx.params.jobId;
     const newStatus = ctx.request.body.status as 'cancelled' | 'invalidated';
     await transactionManager.withTransaction(async models => {
+      const jobDoc = await models.pluginJob.findByIdOrFail(jobId);
+      const pluginId = jobDoc.pluginId;
+
+      if (
+        newStatus === 'invalidated' &&
+        !ctx.userPrivileges.accessiblePlugins.some(
+          p => p.roles.includes('manageJobs') && p.pluginId === pluginId
+        )
+      ) {
+        ctx.throw(401, `You do not have permission to invalidate this job.`);
+      }
+
       const job = await models.pluginJob.findByIdOrFail(jobId);
       if (
         (newStatus === 'cancelled' && job.status !== 'in_queue') ||
@@ -135,7 +147,23 @@ export const handleSearch: RouteMiddleware = ({ models }) => {
     const domainFilter = {
       domain: { $in: ctx.userPrivileges.domains }
     };
-    const filter = { $and: [customFilter!, domainFilter] };
+
+    const accessiblePluginIds = ctx.userPrivileges.accessiblePlugins
+      .filter(p => p.roles.includes('readPlugin'))
+      .map(p => p.pluginId);
+    const accessiblePluginFilter = {
+      pluginId: { $in: accessiblePluginIds }
+    };
+    const filter = {
+      $and: [customFilter!, domainFilter, accessiblePluginFilter]
+    };
+
+    const canViewPersonalInfoPluginIds = ctx.userPrivileges.accessiblePlugins
+      .filter(
+        p =>
+          p.roles.includes('readPlugin') && p.roles.includes('viewPersonalInfo')
+      )
+      .map(p => p.pluginId);
 
     const myListId = ctx.params.myListId;
     const user = ctx.user;
@@ -168,10 +196,13 @@ export const handleSearch: RouteMiddleware = ({ models }) => {
       { $match: { volId: 0 } }, // primary series only
       {
         $addFields: {
-          // Conditionally appends "patientInfo" field
-          ...(canViewPersonalInfo
-            ? { patientInfo: '$seriesInfo.patientInfo' }
-            : {}),
+          patientInfo: {
+            $cond: [
+              { $in: ['$pluginId', canViewPersonalInfoPluginIds] },
+              '$seriesInfo.patientInfo',
+              null
+            ]
+          },
           seriesUid: '$series.seriesUid', // primary series UID only
           studyUid: '$seriesInfo.studyUid',
           seriesDate: '$seriesInfo.seriesDate',
@@ -291,12 +322,7 @@ export const handlePostFeedback: RouteMiddleware = ({ models }) => {
   return async (ctx, next) => {
     const jobId = ctx.params.jobId;
     const job = await models.pluginJob.findByIdOrFail(jobId);
-
-    const mode = ctx.params.mode;
-    if (mode !== 'personal' && mode !== 'consensual') {
-      ctx.throw(status.BAD_REQUEST, 'Invalid feedback mode string.');
-    }
-    const isConsensual = mode === 'consensual';
+    const isConsensual = ctx.request.url.split('/')[4] === 'consensual';
 
     const feedbacks = job.feedbacks as any[]; // TODO: Fix typing
     if (feedbacks.find(f => f.isConsensual)) {

packages/circus-api/src/api/plugin-jobs/index.yaml

@@ -32,6 +32,7 @@ routes:
   - verb: patch
     path: /plugin-jobs/:jobId
     requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: 'executePlugin'
     requestSchema:
       type: object
       properties:
@@ -57,33 +58,46 @@ routes:
   - verb: get
     path: /plugin-jobs/:jobId
     requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: readPlugin
     description: >
       Returns the detail of the specified plug-in job.
     responseSchema: pluginJob|dbEntry
   # Attachment
   - verb: get
     path: /plugin-jobs/:jobId/attachment
     requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: readPlugin
     description: >
       Return the list of attachments from the plug-in.
     handler: handleGetAttachmentList
   - verb: get
     path: /plugin-jobs/:jobId/attachment/:path+
     requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: readPlugin
     description: >
       Returns the specified file output from the plug-in.
     handler: handleGetAttachment
   # Feedback
   - verb: post
-    path: /plugin-jobs/:jobId/feedback/:mode
+    path: /plugin-jobs/:jobId/feedback/consensual
     requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: inputConsensualFeedback
     description: >
-      Creates a new feedback entry for the specified plug-in job.
+      Creates a new consensual feedback entry for the specified plug-in job.
+    requestSchema: pluginJobFeedback|only data,actionLog
+    handler: handlePostFeedback
+  - verb: post
+    path: /plugin-jobs/:jobId/feedback/personal
+    requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: inputPersonalFeedback
+    description: >
+      Creates a new personal feedback entry for the specified plug-in job.
     requestSchema: pluginJobFeedback|only data,actionLog
     handler: handlePostFeedback
   - verb: get
     path: /plugin-jobs/:jobId/feedback
     requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: readPlugin
     description: >
       Lists feedback entries for the specified plug-in job.
     responseSchema:
@@ -93,8 +107,8 @@ routes:
     handler: handleGetFeedback
   - verb: delete
     handler: handleDeleteFeedback
-    requiredGlobalPrivilege: manageServer
     path: /plugin-jobs/:jobId/feedback/:feedbackId
     requiredSeriesDomainCheck: true
+    requiredPluginPrivilege: manageFeedback
     description: >
       Removes feedback items.