SIMSBIOHUB 878: Team Middleware for Authorization (#339)

* update middleware

* explicit pk names

* feat: adds service for team authorization

* test: TeamAuthorizationService

* feat: creates models for team authorization

* feat: adds repository for team authorization queries

* test: TeamAuthorizationRepository

* refact: calls TeamAuthorizationService to determine if user is authorized

* test: AuthorizationService

* refact: repository methods use queryBuilder pattern so queries read like sql

* refact: sql statement to find team policy by submission_feature_id

* test: findTeamPolicyBySubmissionFeature repository method

* refact: calls repository method to find a team policy by submission_feature_id to determine if the user is authorized

* test: TeamAuthorizationService

* misc fix

* feat: adds team discriminator to data_request routes

* refact: removes the not yet implemented 'ticket' discriminator type

* refact: adds system_user discriminator to confirm user is in the system

* feat: updates '/data-requests' route handler to support team_ids filter

* feat: adds method to TeamRepository to get teams by system_user_id

* feat: adds method to TeamService to get team_ids by system_user_id

* feat: adds team_ids filter to DataRequestRepository

* refact: findDataRequests to use system_user_id to get data_requests for the user's teams

* test: DataRequestRepository team_ids filter

* test: DataRequestService

* fix: joins team_member and team tables to get teams for system_user_id

* fix: misc

* refact: removes policy from TeamAuthorizationEntity and makes submissionId required

* refact: removes team_ids param

* misc

* refact: adds service method to get data requests by system user id

* misc

* refact: adds repository method to get data requests associated to the system user's team

* refact: for non-admin data-request route calls findDataRequestsBySystemUserId with system_user_id

* refact: parseQueryParams to be less verbose

* refact: removes findTeamMembershipByPolicy

* refact: removes getTeamIdsBySystemUserId

* refact: returns record_end_date and throws an error if expired

* cleanup

* misc

* refact: returns false if the record has expired instead of throwing an error

* clean up

* refact: reduces duplication by mutating the base query to apply filters

* fix: failing tests

---------

Co-authored-by: Macgregor Aubertin-Young <macgregor.aubertin-young@gov.bc.ca>

Author: J-pilon

api/src/models/team-authorization.ts

@@ -0,0 +1,15 @@
+import { z } from 'zod';
+
+export const TeamPolicyRecord = z.object({
+  team_policy_id: z.string().uuid(),
+  record_end_date: z.string().nullable()
+});
+
+export type TeamPolicyRecord = z.infer<typeof TeamPolicyRecord>;
+
+export const DataRequestRecord = z.object({
+  data_request_id: z.string().uuid(),
+  record_end_date: z.string().nullable()
+});
+
+export type DataRequestRecord = z.infer<typeof DataRequestRecord>;

api/src/paths/data-request/index.test.ts

@@ -3,10 +3,13 @@ import { describe } from 'mocha';
 import sinon from 'sinon';
 import sinonChai from 'sinon-chai';
 import { createDataRequest, findDataRequests } from '.';
+import { SYSTEM_ROLE } from '../../constants/roles';
 import * as db from '../../database/db';
 import { ApiError } from '../../errors/api-error';
 import { DataRequestWithStatus } from '../../models/data-request';
+import { SystemUserExtended } from '../../models/user';
 import { DataRequestService } from '../../services/data-request-service';
+import { UserService } from '../../services/user-service';
 import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db';
 
 chai.use(sinonChai);
@@ -29,6 +32,38 @@ describe('data-request', () => {
     }
   };
 
+  const mockAdminUser: SystemUserExtended = {
+    system_user_id: 1,
+    user_identifier: 'admin@example.com',
+    user_guid: 'admin-guid',
+    user_identity_source_id: 1,
+    identity_source: 'IDIR',
+    record_effective_date: '2025-01-01',
+    record_end_date: null,
+    create_date: '2025-01-01',
+    create_user: 1,
+    update_date: null,
+    update_user: null,
+    revision_count: 0,
+    display_name: 'Admin User',
+    email: 'admin@example.com',
+    given_name: 'Admin',
+    family_name: 'User',
+    agency: null,
+    notes: null,
+    role_ids: [1],
+    role_names: [SYSTEM_ROLE.SYSTEM_ADMIN]
+  };
+
+  const mockNonAdminUser: SystemUserExtended = {
+    ...mockAdminUser,
+    system_user_id: 2,
+    user_identifier: 'member@example.com',
+    user_guid: 'member-guid',
+    role_ids: [2],
+    role_names: [SYSTEM_ROLE.MEMBER]
+  };
+
   describe('findDataRequests', () => {
     it('throws error if DB connection fails to open', async () => {
       const mockDBConnection = getMockDBConnection({
@@ -52,37 +87,51 @@ describe('data-request', () => {
       }
     });
 
-    it('calls DataRequestService.findDataRequests with no filters and returns 200', async () => {
+    it('calls DataRequestService.findDataRequestsBySystemUserId for non-admin with no filters and returns 200', async () => {
       const mockDBConnection = getMockDBConnection({
+        systemUserId: () => mockNonAdminUser.system_user_id,
         commit: sinon.stub(),
         rollback: sinon.stub(),
         release: sinon.stub()
       });
       sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
+      sinon.stub(UserService.prototype, 'getUserById').resolves(mockNonAdminUser);
 
-      const stub = sinon.stub(DataRequestService.prototype, 'findDataRequests').resolves([mockDataRequestWithStatus]);
+      const stub = sinon
+        .stub(DataRequestService.prototype, 'findDataRequestsBySystemUserId')
+        .resolves([mockDataRequestWithStatus]);
 
       const requestHandler = findDataRequests();
       const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
 
       await requestHandler(mockReq, mockRes, mockNext);
 
-      expect(stub).to.have.been.calledOnceWith({});
+      expect(stub).to.have.been.calledOnceWith(mockNonAdminUser.system_user_id, {
+        date_from: undefined,
+        date_to: undefined,
+        requested_by: undefined,
+        team_id: undefined,
+        status: undefined
+      });
       expect(mockDBConnection.commit).to.have.been.calledOnce;
       expect(mockDBConnection.release).to.have.been.calledOnce;
       expect(mockRes.statusValue).to.equal(200);
       expect(mockRes.jsonValue).to.eql([mockDataRequestWithStatus]);
     });
 
-    it('parses query params and passes filters to service', async () => {
+    it('parses query params and passes filters to service for non-admin', async () => {
       const mockDBConnection = getMockDBConnection({
+        systemUserId: () => mockNonAdminUser.system_user_id,
         commit: sinon.stub(),
         rollback: sinon.stub(),
         release: sinon.stub()
       });
       sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
+      sinon.stub(UserService.prototype, 'getUserById').resolves(mockNonAdminUser);
 
-      const stub = sinon.stub(DataRequestService.prototype, 'findDataRequests').resolves([mockDataRequestWithStatus]);
+      const stub = sinon
+        .stub(DataRequestService.prototype, 'findDataRequestsBySystemUserId')
+        .resolves([mockDataRequestWithStatus]);
 
       const requestHandler = findDataRequests();
       const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();
@@ -97,7 +146,7 @@ describe('data-request', () => {
 
       await requestHandler(mockReq, mockRes, mockNext);
 
-      expect(stub).to.have.been.calledOnceWith({
+      expect(stub).to.have.been.calledOnceWith(mockNonAdminUser.system_user_id, {
         date_from: '2025-01-01',
         date_to: '2025-01-31',
         requested_by: 1,
@@ -109,13 +158,14 @@ describe('data-request', () => {
 
     it('rolls back and rethrows if service throws', async () => {
       const mockDBConnection = getMockDBConnection({
+        systemUserId: () => mockAdminUser.system_user_id,
         commit: sinon.stub(),
         rollback: sinon.stub(),
         release: sinon.stub()
       });
       sinon.stub(db, 'getDBConnection').returns(mockDBConnection);
-
-      sinon.stub(DataRequestService.prototype, 'findDataRequests').rejects(new Error('Service error'));
+      sinon.stub(UserService.prototype, 'getUserById').resolves(mockAdminUser);
+      sinon.stub(DataRequestService.prototype, 'findDataRequestsBySystemUserId').rejects(new Error('Service error'));
 
       const requestHandler = findDataRequests();
       const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

api/src/paths/data-request/index.ts

@@ -1,6 +1,5 @@
 import { Request, RequestHandler } from 'express';
 import { Operation } from 'express-openapi';
-import { SYSTEM_ROLE } from '../../constants/roles';
 import { getDBConnection } from '../../database/db';
 import { DataRequestFilters } from '../../models/data-request';
 import {
@@ -20,8 +19,7 @@ export const GET: Operation = [
     return {
       and: [
         {
-          validSystemRoles: [SYSTEM_ROLE.SYSTEM_ADMIN],
-          discriminator: 'SystemRole'
+          discriminator: 'SystemUser'
         }
       ]
     };
@@ -72,7 +70,7 @@ GET.apiDoc = {
       in: 'query',
       name: 'team_id',
       required: false,
-      schema: { type: 'string', format: 'uuid', description: 'Filter by team ID' }
+      schema: { type: 'string', format: 'uuid', description: 'Filter by a single team ID' }
     },
     {
       in: 'query',
@@ -139,9 +137,10 @@ export function findDataRequests(): RequestHandler {
       await connection.open();
 
       const filters = parseQueryParams(req);
+      const systemUserId = connection.systemUserId();
 
       const dataRequestService = new DataRequestService(connection);
-      const dataRequests = await dataRequestService.findDataRequests(filters);
+      const dataRequests = await dataRequestService.findDataRequestsBySystemUserId(systemUserId, filters);
 
       await connection.commit();
 
@@ -190,16 +189,16 @@ export function createDataRequest(): RequestHandler {
 
 /**
  * Parses query params from the request into a filters object for data request list.
- * Returns empty object when no filter params are present.
+ * Query params are strings or undefined per the OpenAPI spec.
  */
 function parseQueryParams(req: Request<unknown, unknown, unknown, DataRequestFilters>): DataRequestFilters {
-  const { date_from, date_to, requested_by, team_id, status } = req.query;
+  const q = req.query;
   const filters: DataRequestFilters = {
-    ...(date_from && { date_from: String(date_from) }),
-    ...(date_to && { date_to: String(date_to) }),
-    ...(requested_by !== undefined && { requested_by: Number(requested_by) }),
-    ...(team_id && { team_id: String(team_id) }),
-    ...(status && { status: String(status) as DataRequestFilters['status'] })
+    date_from: q.date_from ?? undefined,
+    date_to: q.date_to ?? undefined,
+    requested_by: q.requested_by ? Number(q.requested_by) : undefined,
+    team_id: q.team_id ?? undefined,
+    status: q.status ?? undefined
   };
   return filters;
 }

api/src/paths/data-request/{dataRequestId}/index.ts

@@ -11,38 +11,53 @@ import { getLogger } from '../../../utils/logger';
 const defaultLog = getLogger('paths/data-request/{dataRequestId}');
 
 export const GET: Operation = [
-  authorizeRequestHandler(() => {
+  authorizeRequestHandler((req) => {
     return {
-      and: [
+      or: [
         {
           validSystemRoles: [SYSTEM_ROLE.SYSTEM_ADMIN],
           discriminator: 'SystemRole'
+        },
+        {
+          discriminator: 'Team',
+          entity: 'data_request',
+          dataRequestId: req.params.dataRequestId
         }
       ]
     };
   }),
   getDataRequestById()
 ];
 export const PUT: Operation = [
-  authorizeRequestHandler(() => {
+  authorizeRequestHandler((req) => {
     return {
-      and: [
+      or: [
         {
           validSystemRoles: [SYSTEM_ROLE.SYSTEM_ADMIN],
           discriminator: 'SystemRole'
+        },
+        {
+          discriminator: 'Team',
+          entity: 'data_request',
+          dataRequestId: req.params.dataRequestId
         }
       ]
     };
   }),
   updateDataRequest()
 ];
 export const DELETE: Operation = [
-  authorizeRequestHandler(() => {
+  authorizeRequestHandler((req) => {
     return {
-      and: [
+      or: [
         {
           validSystemRoles: [SYSTEM_ROLE.SYSTEM_ADMIN],
           discriminator: 'SystemRole'
+        },
+        {
+          discriminator: 'Team',
+          entity: 'data_request',
+          dataRequestId: req.params.dataRequestId
         }
       ]
     };

api/src/paths/submission/{submissionId}/features/{submissionFeatureId}/index.ts

@@ -14,9 +14,10 @@ export const GET: Operation = [
     return {
       and: [
         {
-          submissionId: Number(req.params.submissionId),
+          discriminator: 'Team',
+          entity: 'submission_feature',
           submissionFeatureId: Number(req.params.submissionFeatureId),
-          discriminator: 'AccessPolicy'
+          submissionId: Number(req.params.submissionId)
         }
       ]
     };

api/src/repositories/authorization/team-authorization-repository.test.ts

@@ -0,0 +1,58 @@
+import { expect } from 'chai';
+import { describe } from 'mocha';
+import { QueryResult } from 'pg';
+import sinon from 'sinon';
+import { getMockDBConnection } from '../../__mocks__/db';
+import { TeamAuthorizationRepository } from './team-authorization-repository';
+
+describe('TeamAuthorizationRepository', () => {
+  afterEach(() => {
+    sinon.restore();
+  });
+
+  describe('findTeamMembershipByDataRequest', () => {
+    it('returns a record when the user has team access to the data request', async () => {
+      const mockRow = { data_request_id: 'dr-1', record_end_date: null };
+      const mockResponse = { rowCount: 1, rows: [mockRow] } as unknown as Promise<QueryResult<any>>;
+      const mockConnection = getMockDBConnection({ knex: async () => mockResponse });
+
+      const repository = new TeamAuthorizationRepository(mockConnection);
+      const result = await repository.findTeamMembershipByDataRequest(1, 'dr-1');
+
+      expect(result).to.eql(mockRow);
+    });
+
+    it('returns null when the user does not have team access to the data request', async () => {
+      const mockResponse = { rowCount: 0, rows: [] } as unknown as Promise<QueryResult<any>>;
+      const mockConnection = getMockDBConnection({ knex: async () => mockResponse });
+
+      const repository = new TeamAuthorizationRepository(mockConnection);
+      const result = await repository.findTeamMembershipByDataRequest(1, 'dr-1');
+
+      expect(result).to.be.null;
+    });
+  });
+
+  describe('findTeamPolicyBySubmissionFeature', () => {
+    it('returns a record when the user has team policy access to the submission feature', async () => {
+      const mockRow = { team_policy_id: 'tp-1', record_end_date: null };
+      const mockResponse = { rowCount: 1, rows: [mockRow] } as unknown as QueryResult<any>;
+      const mockConnection = getMockDBConnection({ sql: async () => mockResponse });
+
+      const repository = new TeamAuthorizationRepository(mockConnection);
+      const result = await repository.findTeamPolicyBySubmissionFeature(1, 100);
+
+      expect(result).to.eql(mockRow);
+    });
+
+    it('returns null when the user does not have team policy access to the submission feature', async () => {
+      const mockResponse = { rowCount: 0, rows: [] } as unknown as QueryResult<any>;
+      const mockConnection = getMockDBConnection({ sql: async () => mockResponse });
+
+      const repository = new TeamAuthorizationRepository(mockConnection);
+      const result = await repository.findTeamPolicyBySubmissionFeature(1, 100);
+
+      expect(result).to.be.null;
+    });
+  });
+});