Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIMS-1773: Auditor Scoping #2593

Merged
merged 23 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion express-api/src/controllers/parcels/parcelsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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);
Expand Down
16 changes: 14 additions & 2 deletions express-api/src/controllers/projects/projectsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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)) {
Expand All @@ -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.');
}

Expand Down Expand Up @@ -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');
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions express-api/src/controllers/users/usersController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
50 changes: 44 additions & 6 deletions express-api/src/services/properties/propertiesServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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[],
Expand All @@ -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[],
) => {
Expand All @@ -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[],
) => {
Expand All @@ -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[],
) => {
Expand All @@ -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[],
) => {
Expand Down Expand Up @@ -453,7 +454,7 @@ const makeBuildingUpsertObject = async (
};
};

type Lookups = {
export type Lookups = {
classifications: PropertyClassification[];
constructionTypes: BuildingConstructionType[];
predominateUses: BuildingPredominateUse[];
Expand Down Expand Up @@ -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;
45 changes: 3 additions & 42 deletions express-api/src/services/properties/propertyWorker.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand Down
7 changes: 5 additions & 2 deletions express-api/src/utilities/authorizationChecks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
2 changes: 1 addition & 1 deletion express-api/tests/testUtils/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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);
Expand Down
Loading
Loading