PIMS-1773: Auditor Scoping (#2593)

Co-authored-by: Graham Stewart <[email protected]>
Co-authored-by: dbarkowsky <[email protected]>

express-api/src/controllers/buildings/buildingsController.ts

@@ -5,6 +5,7 @@ import userServices from '@/services/users/usersServices';
 import { SSOUser } from '@bcgov/citz-imb-sso-express';
 import { Building } from '@/typeorm/Entities/Building';
 import { checkUserAgencyPermission, isAdmin, isAuditor } from '@/utilities/authorizationChecks';
+import { Roles } from '@/constants/roles';
 
 /**
  * @description Gets all buildings satisfying the filter parameters.
@@ -52,12 +53,14 @@ export const getBuilding = async (req: Request, res: Response) => {
     return res.status(400).send('Building Id is invalid.');
   }
 
+  // admin and auditors are permitted to see any building
+  const permittedRoles = [Roles.ADMIN, Roles.AUDITOR];
   const kcUser = req.user as unknown as SSOUser;
   const building = await buildingService.getBuildingById(buildingId);
 
   if (!building) {
     return res.status(404).send('Building matching this ID was not found.');
-  } else if (!(await checkUserAgencyPermission(kcUser, [building.AgencyId]))) {
+  } else if (!(await checkUserAgencyPermission(kcUser, [building.AgencyId], permittedRoles))) {
     return res.status(403).send('You are not authorized to view this building.');
   }
   return res.status(200).send(building);

express-api/src/controllers/parcels/parcelsController.ts

@@ -4,6 +4,7 @@ import { ParcelFilter, ParcelFilterSchema } from '@/services/parcels/parcelSchem
 import { SSOUser } from '@bcgov/citz-imb-sso-express';
 import userServices from '@/services/users/usersServices';
 import { Parcel } from '@/typeorm/Entities/Parcel';
+import { Roles } from '@/constants/roles';
 import { checkUserAgencyPermission, isAdmin, isAuditor } from '@/utilities/authorizationChecks';
 
 /**
@@ -25,11 +26,13 @@ export const getParcel = async (req: Request, res: Response) => {
     return res.status(400).send('Parcel ID was invalid.');
   }
 
+  // admin and auditors are permitted to see any parcel
+  const permittedRoles = [Roles.ADMIN, Roles.AUDITOR];
   const kcUser = req.user as unknown as SSOUser;
   const parcel = await parcelServices.getParcelById(parcelId);
   if (!parcel) {
     return res.status(404).send('Parcel matching this internal ID not found.');
-  } else if (!(await checkUserAgencyPermission(kcUser, [parcel.AgencyId]))) {
+  } else if (!(await checkUserAgencyPermission(kcUser, [parcel.AgencyId], permittedRoles))) {
     return res.status(403).send('You are not authorized to view this parcel.');
   }
   return res.status(200).send(parcel);

express-api/src/controllers/projects/projectsController.ts

@@ -6,6 +6,7 @@ import userServices from '@/services/users/usersServices';
 import { isAdmin, isAuditor, checkUserAgencyPermission } from '@/utilities/authorizationChecks';
 import { DeepPartial } from 'typeorm';
 import { Project } from '@/typeorm/Entities/Project';
+import { Roles } from '@/constants/roles';
 import notificationServices from '@/services/notifications/notificationServices';
 
 /**
@@ -22,6 +23,8 @@ export const getDisposalProject = async (req: Request, res: Response) => {
    *   "bearerAuth" : []
    * }]
    */
+  // admins are permitted to view any project
+  const permittedRoles = [Roles.ADMIN];
   const user = req.user as SSOUser;
   const projectId = Number(req.params.projectId);
   if (isNaN(projectId)) {
@@ -32,7 +35,7 @@ export const getDisposalProject = async (req: Request, res: Response) => {
     return res.status(404).send('Project matching this internal ID not found.');
   }
 
-  if (!(await checkUserAgencyPermission(user, [project.AgencyId]))) {
+  if (!(await checkUserAgencyPermission(user, [project.AgencyId], permittedRoles))) {
     return res.status(403).send('You are not authorized to view this project.');
   }
 
@@ -98,6 +101,11 @@ export const deleteDisposalProject = async (req: Request, res: Response) => {
    *   "bearerAuth" : []
    * }]
    */
+  // Only admins can delete projects
+  if (!isAdmin(req.user)) {
+    return res.status(403).send('Projects can only be deleted by Administrator role.');
+  }
+
   const projectId = Number(req.params.projectId);
   if (isNaN(projectId)) {
     return res.status(400).send('Invalid Project ID');
@@ -123,6 +131,10 @@ export const deleteDisposalProject = async (req: Request, res: Response) => {
  * @returns {Response}      A 200 status with the new project.
  */
 export const addDisposalProject = async (req: Request, res: Response) => {
+  // Auditors can no add projects
+  if (isAuditor(req.user)) {
+    return res.status(403).send('Projects can not be added by user with Auditor role.');
+  }
   // Extract project data from request body
   // Extract projectData and propertyIds from the request body
   const {
@@ -157,7 +169,7 @@ export const getProjects = async (req: Request, res: Response) => {
   }
   const filterResult = filter.data;
   const kcUser = req.user as unknown as SSOUser;
-  if (!(isAdmin(kcUser) || isAuditor(kcUser))) {
+  if (!isAdmin(kcUser)) {
     // get array of user's agencies
     const usersAgencies = await userServices.getAgencies(kcUser.preferred_username);
     filterResult.agencyId = usersAgencies;

express-api/src/controllers/properties/propertiesController.ts

@@ -16,6 +16,7 @@ import { AppDataSource } from '@/appDataSource';
 import { ImportResult } from '@/typeorm/Entities/ImportResult';
 import { readFile } from 'xlsx';
 import logger from '@/utilities/winstonLogger';
+import { Roles } from '@/constants/roles';
 
 /**
  * @description Search for a single keyword across multiple different fields in both parcels and buildings.
@@ -81,10 +82,16 @@ export const getPropertiesForMap = async (req: Request, res: Response) => {
 
   // Controlling for agency search visibility
   const kcUser = req.user;
+  // admin and suditors can see any property
+  const permittedRoles = [Roles.ADMIN, Roles.AUDITOR];
   // Admins and auditors see all, otherwise...
   if (!(isAdmin(kcUser) || isAuditor(kcUser))) {
     const requestedAgencies = filterResult.AgencyIds;
-    const userHasAgencies = await checkUserAgencyPermission(kcUser, requestedAgencies);
+    const userHasAgencies = await checkUserAgencyPermission(
+      kcUser,
+      requestedAgencies,
+      permittedRoles,
+    );
     // If not agencies were requested or if the user doesn't have those requested agencies
     if (!requestedAgencies || !userHasAgencies) {
       // Then only show that user's agencies instead.

express-api/src/controllers/users/usersController.ts

@@ -4,7 +4,7 @@ import { SSOUser } from '@bcgov/citz-imb-sso-express';
 import { decodeJWT } from '@/utilities/decodeJWT';
 import { UserFiltering, UserFilteringSchema } from '@/controllers/users/usersSchema';
 import { z } from 'zod';
-import { isAdmin, isAuditor } from '@/utilities/authorizationChecks';
+import { isAdmin } from '@/utilities/authorizationChecks';
 import notificationServices from '@/services/notifications/notificationServices';
 import getConfig from '@/constants/config';
 import logger from '@/utilities/winstonLogger';
@@ -23,7 +23,7 @@ const filterUsersByAgencies = async (req: Request, res: Response, ssoUser: SSOUs
   const filterResult = filter.data;
 
   let users;
-  if (isAdmin(ssoUser) || isAuditor(ssoUser)) {
+  if (isAdmin(ssoUser)) {
     users = await userServices.getUsers(filterResult as UserFiltering);
   } else {
     // Get agencies associated with the requesting user
@@ -204,7 +204,7 @@ export const getUserById = async (req: Request, res: Response) => {
     const user = await userServices.getUserById(uuid.data);
 
     if (user) {
-      if (!isAdmin(ssoUser) && !isAuditor(ssoUser)) {
+      if (!isAdmin(ssoUser)) {
         // check if user has the correct agencies
         const usersAgencies = await userServices.hasAgencies(ssoUser.preferred_username, [
           user.AgencyId,

express-api/src/services/properties/propertiesServices.ts

@@ -32,6 +32,7 @@ import userServices from '../users/usersServices';
 import { Brackets, FindManyOptions, FindOptionsWhere, ILike, In, QueryRunner } from 'typeorm';
 import { SSOUser } from '@bcgov/citz-imb-sso-express';
 import { PropertyType } from '@/constants/propertyType';
+import { parentPort } from 'worker_threads';
 
 /**
  * Perform a fuzzy search for properties based on the provided keyword.
@@ -165,7 +166,7 @@ const numberOrNull = (value: any) => {
  * @returns The agency if the user has permission, otherwise throws an error.
  * @throws Error if the agency code is not supported or if the user does not have permission to add properties for the agency.
  */
-const getAgencyOrThrowIfMismatched = (
+export const getAgencyOrThrowIfMismatched = (
   row: Record<string, any>,
   lookups: Lookups,
   roles: string[],
@@ -189,7 +190,7 @@ const getAgencyOrThrowIfMismatched = (
  * @param {PropertyClassification[]} classifications - The list of property classifications to search from.
  * @returns {number} The classification ID.
  */
-const getClassificationOrThrow = (
+export const getClassificationOrThrow = (
   row: Record<string, any>,
   classifications: PropertyClassification[],
 ) => {
@@ -213,7 +214,7 @@ const getClassificationOrThrow = (
  * @param adminAreas - The array of AdministrativeArea objects to search for a match.
  * @returns The ID of the administrative area if found, otherwise throws an error.
  */
-const getAdministrativeAreaOrThrow = (
+export const getAdministrativeAreaOrThrow = (
   row: Record<string, any>,
   adminAreas: AdministrativeArea[],
 ) => {
@@ -237,7 +238,7 @@ const getAdministrativeAreaOrThrow = (
  * @param predominateUses - The list of available building predominate uses.
  * @returns The ID of the predominate use if found, otherwise throws an error.
  */
-const getBuildingPredominateUseOrThrow = (
+export const getBuildingPredominateUseOrThrow = (
   row: Record<string, any>,
   predominateUses: BuildingPredominateUse[],
 ) => {
@@ -263,7 +264,7 @@ const getBuildingPredominateUseOrThrow = (
  * @returns The ID of the matched building construction type.
  * @throws Error if the construction type cannot be determined from the provided data.
  */
-const getBuildingConstructionTypeOrThrow = (
+export const getBuildingConstructionTypeOrThrow = (
   row: Record<string, any>,
   constructionTypes: BuildingConstructionType[],
 ) => {
@@ -453,7 +454,7 @@ const makeBuildingUpsertObject = async (
   };
 };
 
-type Lookups = {
+export type Lookups = {
   classifications: PropertyClassification[];
   constructionTypes: BuildingConstructionType[];
   predominateUses: BuildingPredominateUse[];
@@ -752,13 +753,50 @@ const getPropertiesForExport = async (filter: PropertyUnionFilter) => {
   return properties;
 };
 
+/**
+ * Asynchronously processes a file for property import, initializing a new database connection for the worker thread.
+ * Reads the file content, imports properties as JSON, and saves the results to the database.
+ * Handles exceptions and ensures database connection cleanup after processing.
+ * @param filePath The path to the file to be processed.
+ * @param resultRowId The ID of the result row in the database.
+ * @param user The user initiating the import.
+ * @param roles The roles assigned to the user.
+ * @returns A list of bulk upload row results after processing the file.
+ */
+const processFile = async (filePath: string, resultRowId: number, user: User, roles: string[]) => {
+  await AppDataSource.initialize(); //Since this function is going to be called from a new process, requires a new database connection.
+  let results: BulkUploadRowResult[] = [];
+  try {
+    parentPort.postMessage('Database connection for worker thread has been initialized');
+    const file = xlsx.readFile(filePath); //It's better to do the read here rather than the parent process because any arguments passed to this function are copied rather than referenced.
+    const sheetName = file.SheetNames[0];
+    const worksheet = file.Sheets[sheetName];
+
+    results = await propertyServices.importPropertiesAsJSON(worksheet, user, roles, resultRowId);
+    return results; // Note that this return still works with finally as long as return is not called from finally block.
+  } catch (e) {
+    parentPort.postMessage('Aborting file upload: ' + e.message);
+    parentPort.postMessage('Aborting stack: ' + e.stack);
+  } finally {
+    await AppDataSource.getRepository(ImportResult).save({
+      Id: resultRowId,
+      CompletionPercentage: 1.0,
+      Results: results,
+      UpdatedById: user.Id,
+      UpdatedOn: new Date(),
+    });
+    await AppDataSource.destroy(); //Not sure whether this is necessary but seems like the safe thing to do.
+  }
+};
+
 const propertyServices = {
   propertiesFuzzySearch,
   getPropertiesForMap,
   importPropertiesAsJSON,
   getPropertiesUnion,
   getImportResults,
   getPropertiesForExport,
+  processFile,
 };
 
 export default propertyServices;

express-api/src/services/properties/propertyWorker.ts

@@ -1,47 +1,8 @@
 import { parentPort, workerData } from 'worker_threads';
-import propertyServices, { BulkUploadRowResult } from './propertiesServices';
-import xlsx from 'xlsx';
-import { AppDataSource } from '@/appDataSource';
-import { ImportResult } from '@/typeorm/Entities/ImportResult';
-import { User } from '@/typeorm/Entities/User';
+import propertyServices from './propertiesServices';
 
-/**
- * Asynchronously processes a file for property import, initializing a new database connection for the worker thread.
- * Reads the file content, imports properties as JSON, and saves the results to the database.
- * Handles exceptions and ensures database connection cleanup after processing.
- * @param filePath The path to the file to be processed.
- * @param resultRowId The ID of the result row in the database.
- * @param user The user initiating the import.
- * @param roles The roles assigned to the user.
- * @returns A list of bulk upload row results after processing the file.
- */
-const processFile = async (filePath: string, resultRowId: number, user: User, roles: string[]) => {
-  await AppDataSource.initialize(); //Since this function is going to be called from a new process, requires a new database connection.
-  let results: BulkUploadRowResult[] = [];
-  try {
-    parentPort.postMessage('Database connection for worker thread has been initialized');
-    const file = xlsx.readFile(filePath); //It's better to do the read here rather than the parent process because any arguments passed to this function are copied rather than referenced.
-    const sheetName = file.SheetNames[0];
-    const worksheet = file.Sheets[sheetName];
-
-    results = await propertyServices.importPropertiesAsJSON(worksheet, user, roles, resultRowId);
-
-    return results; // Note that this return still works with finally as long as return is not called from finally block.
-  } catch (e) {
-    parentPort.postMessage('Aborting file upload: ' + e.message);
-  } finally {
-    await AppDataSource.getRepository(ImportResult).save({
-      Id: resultRowId,
-      CompletionPercentage: 1.0,
-      Results: results,
-      UpdatedById: user.Id,
-      UpdatedOn: new Date(),
-    });
-    await AppDataSource.destroy(); //Not sure whether this is necessary but seems like the safe thing to do.
-  }
-};
-
-processFile(workerData.filePath, workerData.resultRowId, workerData.user, workerData.roles)
+propertyServices
+  .processFile(workerData.filePath, workerData.resultRowId, workerData.user, workerData.roles)
   .then((results) => {
     parentPort.postMessage('File processing succeeded.');
     parentPort.postMessage(

express-api/src/utilities/authorizationChecks.ts

@@ -67,16 +67,19 @@ export const isUserActive = async (kcUser: SSOUser): Promise<boolean> => {
 export const checkUserAgencyPermission = async (
   kcUser: SSOUser,
   agencyIds: number[],
+  permittedRoles: Roles[],
 ): Promise<boolean> => {
   // Check if undefined, has length of 0, or if the only element is undefined
   if (!agencyIds || agencyIds.length === 0 || !agencyIds.at(0)) {
     return false;
   }
-  if (!isAdmin(kcUser) && !isAuditor(kcUser)) {
+  const userRolePermission = kcUser?.hasRoles(permittedRoles, { requireAllRoles: false });
+  // if the user is not an admin, nor has a permitted role scope results
+  if (!isAdmin(kcUser) && !userRolePermission) {
     // check if current user belongs to any of the specified agencies
     const userAgencies = await userServices.hasAgencies(kcUser.preferred_username, agencyIds);
     return userAgencies;
   }
-  // Admins and auditors have permission by default
+  // Admins have permission by default
   return true;
 };

express-api/tests/testUtils/factories.ts

@@ -428,7 +428,7 @@ export const produceAdminArea = (props?: Partial<AdministrativeArea>): Administr
 };
 
 export const produceClassification = (
-  props: Partial<PropertyClassification>,
+  props?: Partial<PropertyClassification>,
 ): PropertyClassification => {
   const classification: PropertyClassification = {
     Id: faker.number.int(),

express-api/tests/unit/controllers/buildings/buildingsController.test.ts

@@ -50,6 +50,7 @@ describe('UNIT - Buildings', () => {
       };
       mockRequest.params.buildingId = '1';
       _hasAgencies.mockImplementationOnce(() => true);
+      mockRequest.setUser({ hasRoles: () => true });
       _getBuildingById.mockImplementationOnce(() => buildingWithAgencyId1);
       await controllers.getBuilding(mockRequest, mockResponse);
       expect(mockResponse.statusValue).toBe(200);
@@ -69,12 +70,9 @@ describe('UNIT - Buildings', () => {
     });
 
     it('should return 403 when user does not have correct agencies', async () => {
-      const buildingWithAgencyId1 = {
-        AgencyId: 1,
-      };
       mockRequest.params.buildingId = '1';
+      mockRequest.setUser({ client_roles: [Roles.GENERAL_USER], hasRoles: () => false });
       _hasAgencies.mockImplementationOnce(() => false);
-      _getBuildingById.mockImplementationOnce(() => buildingWithAgencyId1);
       await controllers.getBuilding(mockRequest, mockResponse);
       expect(mockResponse.statusValue).toBe(403);
     });

express-api/tests/unit/controllers/parcels/parcelsController.test.ts

@@ -45,7 +45,8 @@ describe('UNIT - Parcels', () => {
   describe('GET /properties/parcels/:parcelId', () => {
     it('should return 200 with a correct response body', async () => {
       mockRequest.params.parcelId = '1';
-      _hasAgencies.mockImplementationOnce(() => true);
+      // _hasAgencies.mockImplementationOnce(() => true);
+      mockRequest.setUser({ client_roles: [Roles.ADMIN] });
       await controllers.getParcel(mockRequest, mockResponse);
       expect(mockResponse.statusValue).toBe(200);
     });
@@ -72,7 +73,7 @@ describe('UNIT - Parcels', () => {
     });
     it('should return with status 403 when user doenst have permission to view parcel', async () => {
       mockRequest.params.parcelId = '1';
-      mockRequest.setUser({ client_roles: [Roles.GENERAL_USER] });
+      mockRequest.setUser({ client_roles: [Roles.GENERAL_USER], hasRoles: () => false });
       _hasAgencies.mockImplementationOnce(() => false);
       await controllers.getParcel(mockRequest, mockResponse);
       expect(mockResponse.statusValue).toBe(403);