From e04ceb9d6475301c6b34f27ce52915b2ec12b423 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 28 Oct 2024 12:30:36 +0000 Subject: [PATCH 01/42] build: use improved blaise-login components --- package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 6d88349..de51df1 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "axios": "^1.7.4", "blaise-api-node-client": "git+https://github.com/ONSdigital/blaise-api-node-client", "blaise-design-system-react-components": "git+https://github.com/ONSdigital/blaise-design-system-react-components#0.14.0", - "blaise-login-react": "git+https://github.com/ONSdigital/blaise-login-react#1.1.0", + "blaise-login-react": "git+https://github.com/ONSdigital/blaise-login-react#BLAIS5-4287-b", "dotenv": "^10.0.0", "ejs": "^3.1.10", "express": "^4.19.2", diff --git a/yarn.lock b/yarn.lock index 8dcbdee..6408b7e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6296,9 +6296,9 @@ bl@^4.0.3, bl@^4.1.0: path-parse "^1.0.7" typescript "~4.2.2" -"blaise-login-react@git+https://github.com/ONSdigital/blaise-login-react#1.1.0": +"blaise-login-react@git+https://github.com/ONSdigital/blaise-login-react#BLAIS5-4287-b": version "0.0.0" - resolved "git+https://github.com/ONSdigital/blaise-login-react#39f4182a62b1faaf01039fb8c94adc296ac78766" + resolved "git+https://github.com/ONSdigital/blaise-login-react#7aa5c3c8ea165218b7c98e35bed1cdc4fb409712" bluebird@^3.5.5: version "3.7.2" From af4e2d0c12b9fa2133b4c0907910c16ea6e8824e Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 28 Oct 2024 12:31:05 +0000 Subject: [PATCH 02/42] wip: add logging --- server/BlaiseAPI/index.ts | 17 ++++++++++++----- server/server.ts | 9 +++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/server/BlaiseAPI/index.ts b/server/BlaiseAPI/index.ts index 207294a..2890182 100644 --- a/server/BlaiseAPI/index.ts +++ b/server/BlaiseAPI/index.ts @@ -2,6 +2,9 @@ import express, { Request, Response, Router } from "express"; import { CustomConfig } from "../interfaces/server"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import BlaiseApiClient from "blaise-api-node-client"; +import createLogger from "../pino"; + +const pinoLogger = createLogger(); export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaiseApiClient: BlaiseApiClient): Router { const router = express.Router(); @@ -37,13 +40,13 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise await blaiseApiClient.changeUserRole(user, role); await blaiseApiClient.changeUserServerParks(user, newServerParks, newDefaultServerPark); const successMessage = `Successfully updated user role and permissions to ${role} for ${user}`; - console.log(successMessage + ` at ${(new Date()).toLocaleTimeString("en-UK")} ${(new Date()).toLocaleDateString("en-UK")}`); + pinoLogger.logger.info(successMessage + ` at ${(new Date()).toLocaleTimeString("en-UK")} ${(new Date()).toLocaleDateString("en-UK")}`); return res.status(200).json({ message: successMessage + " today at " + (new Date()).toLocaleTimeString("en-UK") }); } catch (error) { const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}: ${error}`; - console.error(errorMessage); + pinoLogger.logger.error(errorMessage); return res.status(500).json({ message: errorMessage }); @@ -58,13 +61,14 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise try { const user = await blaiseApiClient.getUser(req.params.user); const successMessage = `Successfully fetched user details for ${req.params.user}`; + pinoLogger.logger.info(successMessage); return res.status(200).json({ message: successMessage, data: user }); } catch (error) { const errorMessage = `Error whilst trying to retrieve user ${req.params.user}: ${error}`; - console.error(errorMessage); + pinoLogger.logger.error(errorMessage); return res.status(500).json({ message: errorMessage, error: error @@ -84,9 +88,10 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise } blaiseApiClient.changePassword(req.params.user, password).then(() => { + pinoLogger.logger.info(`Successfully changed password for ${req.params.user}`); return res.status(204).json(null); }).catch((error: unknown) => { - console.error(error); + pinoLogger.logger.error(error); return res.status(500).json(error); }); }); @@ -107,7 +112,7 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise router.post("/api/users", auth.Middleware, async function (req: Request, res: Response) { const data = req.body; if(!data.role){ - return res.status(400).json(); + return res.status(400).json({ message: "No role provided for user creation" }); } const roleServerParksOverride = config.RoleToServerParksMap[data.role]; if (roleServerParksOverride != null) { @@ -118,6 +123,8 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise data.serverParks = defaultServerPark; data.defaultServerPark = defaultServerPark[0]; } + const currentUser = req.headers.user; + pinoLogger.logger.info(`${currentUser} has created user: ${data.username}`); return res.status(200).json(await blaiseApiClient.createUser(data)); }); diff --git a/server/server.ts b/server/server.ts index 2fca2ba..8116d5d 100644 --- a/server/server.ts +++ b/server/server.ts @@ -15,9 +15,9 @@ import fs from "fs"; export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi, auth: Auth): Express { - const pinoLogger = pino(); + const pinoLogger = createLogger(); profiler.start({ logLevel: 4 }).catch((err: unknown) => { - pinoLogger.error(`Failed to start profiler: ${err}`); + pinoLogger.logger.error(`Failed to start profiler: ${err}`); }); const upload = multer(); @@ -28,9 +28,6 @@ export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi axios.defaults.timeout = 10000; - const logger = createLogger(); - server.use(logger); - // where ever the react built package is const buildFolder = "../build"; @@ -38,7 +35,7 @@ export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi // Health Check endpoint server.get("/bam-ui/:version/health", async function (req: Request, res: Response) { - pinoLogger.info("Heath Check endpoint called"); + pinoLogger.logger.info("Heath Check endpoint called"); res.status(200).json({ healthy: true }); }); From 44327f0542de5c5717d86852daa1a5c43b2b5ff9 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 28 Oct 2024 12:59:54 +0000 Subject: [PATCH 03/42] wip: add logging --- server/server.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/server.ts b/server/server.ts index 8116d5d..a431588 100644 --- a/server/server.ts +++ b/server/server.ts @@ -28,6 +28,10 @@ export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi axios.defaults.timeout = 10000; + const logger = createLogger(); + server.use(logger); + logger.logger.info("Server started"); + // where ever the react built package is const buildFolder = "../build"; From 1824256b1bfbb38679d8c97252f6cd95d16e162a Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:33:13 +0000 Subject: [PATCH 04/42] build: use latest blaise-login-react; use cloud logging (same as DQS ver) --- package.json | 3 ++- yarn.lock | 25 +++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index de51df1..093130e 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "formik": "2.4.2" }, "dependencies": { + "@google-cloud/logging": "^9.6.0", "@google-cloud/profiler": "^4.1.1", "@testing-library/dom": "^8.3.0", "@testing-library/jest-dom": "^5.16.4", @@ -34,7 +35,7 @@ "axios": "^1.7.4", "blaise-api-node-client": "git+https://github.com/ONSdigital/blaise-api-node-client", "blaise-design-system-react-components": "git+https://github.com/ONSdigital/blaise-design-system-react-components#0.14.0", - "blaise-login-react": "git+https://github.com/ONSdigital/blaise-login-react#BLAIS5-4287-b", + "blaise-login-react": "git+https://github.com/ONSdigital/blaise-login-react#1.1.1", "dotenv": "^10.0.0", "ejs": "^3.1.10", "express": "^4.19.2", diff --git a/yarn.lock b/yarn.lock index 6408b7e..e111c4f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3171,6 +3171,27 @@ stream-events "^1.0.5" uuid "^8.0.0" +"@google-cloud/logging@^9.6.0": + version "9.9.0" + resolved "https://registry.yarnpkg.com/@google-cloud/logging/-/logging-9.9.0.tgz#d13fd6ae359e02123809b2bd640a0a8f71793434" + integrity sha512-rJQ0i9COI1WbtWuGyXL8edF4OA3XHcvgAq5I2DZmjKFvmCLCcspFIWZtztxgd0UgmrrshQVZ0R76oBHjTGggkg== + dependencies: + "@google-cloud/common" "^3.4.1" + "@google-cloud/paginator" "^3.0.0" + "@google-cloud/projectify" "^2.0.0" + "@google-cloud/promisify" "^2.0.0" + arrify "^2.0.1" + dot-prop "^6.0.0" + eventid "^2.0.0" + extend "^3.0.2" + gcp-metadata "^4.0.0" + google-auth-library "^7.0.0" + google-gax "^2.24.1" + on-finished "^2.3.0" + pumpify "^2.0.1" + stream-events "^1.0.5" + uuid "^8.0.0" + "@google-cloud/paginator@^3.0.0": version "3.0.7" resolved "https://registry.yarnpkg.com/@google-cloud/paginator/-/paginator-3.0.7.tgz#fb6f8e24ec841f99defaebf62c75c2e744dd419b" @@ -6296,9 +6317,9 @@ bl@^4.0.3, bl@^4.1.0: path-parse "^1.0.7" typescript "~4.2.2" -"blaise-login-react@git+https://github.com/ONSdigital/blaise-login-react#BLAIS5-4287-b": +"blaise-login-react@git+https://github.com/ONSdigital/blaise-login-react#1.1.1": version "0.0.0" - resolved "git+https://github.com/ONSdigital/blaise-login-react#7aa5c3c8ea165218b7c98e35bed1cdc4fb409712" + resolved "git+https://github.com/ONSdigital/blaise-login-react#f7fc532e1938ccb45831816311c85336510383ae" bluebird@^3.5.5: version "3.7.2" From 5f7d1249a3dab10ac1d408b14717d48ea592f734 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:35:02 +0000 Subject: [PATCH 05/42] feat: add and configure gcp cloud logging --- server/AuditLogger/index.ts | 56 +++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 server/AuditLogger/index.ts diff --git a/server/AuditLogger/index.ts b/server/AuditLogger/index.ts new file mode 100644 index 0000000..4c6d1d9 --- /dev/null +++ b/server/AuditLogger/index.ts @@ -0,0 +1,56 @@ +import { Logging } from "@google-cloud/logging"; +import { IncomingMessage } from "http"; + +export interface AuditLog { + id: string; + timestamp: string; + message: string; + severity: string; +} + +export default class AuditLogger { + projectId: string; + logger: Logging; + logName: string; + + constructor(projectId: string) { + this.projectId = projectId; + this.logger = new Logging({ projectId: this.projectId }); + this.logName = `projects/${this.projectId}/logs/stdout`; + } + + info(logger: IncomingMessage["log"], message: string): void { + logger.info(`AUDIT_LOG: ${message}`); + } + + error(logger: IncomingMessage["log"], message: string): void { + logger.error(`AUDIT_LOG: ${message}`); + } + + async getLogs(): Promise { + const auditLogs: AuditLog[] = []; + const log = this.logger.log(this.logName); + const [entries] = await log.getEntries({ filter: "jsonPayload.message=~\"^AUDIT_LOG: \"", maxResults: 50 }); + for (const entry of entries) { + let id = ""; + let timestamp = ""; + let severity = "INFO"; + if (entry.metadata.insertId != null) { + id = entry.metadata.insertId; + } + if (entry.metadata.timestamp != null) { + timestamp = entry.metadata.timestamp.toString(); + } + if (entry.metadata.severity != null) { + severity = entry.metadata.severity.toString(); + } + auditLogs.push({ + id: id, + timestamp: timestamp, + message: entry.data.message.replace(/^AUDIT_LOG: /, ""), + severity: severity + }); + } + return auditLogs; + } +} From 3e16abfa4a9d87914dc1be6155b7e9fc3b69aeed Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:35:42 +0000 Subject: [PATCH 06/42] fix: trim trailing whitespaces for name and pwd --- src/pages/users/UserUpload/NewUser.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pages/users/UserUpload/NewUser.tsx b/src/pages/users/UserUpload/NewUser.tsx index 1b56027..90fbd30 100644 --- a/src/pages/users/UserUpload/NewUser.tsx +++ b/src/pages/users/UserUpload/NewUser.tsx @@ -25,9 +25,10 @@ function NewUserComponent(): ReactElement { setUsername(formData.username); if (formData.username && formData.password) { const newUser: NewUser = { - name: formData.username, - password: formData.password, + name: formData.username.trim(), + password: formData.password.trim(), role: role, + // TODO: Are these needed? These should be set in the server depending on the role defaultServerPark: "gusty", serverParks: ["gusty"] }; From 600ed11f6cd04c74e3fcbbd93b9cf0f842ca6685 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:36:26 +0000 Subject: [PATCH 07/42] wip: use pino http logger and cloud logging (test with BlaiseAPI) --- server/BlaiseAPI/index.ts | 20 ++++++++++++-------- server/server.ts | 19 +++++++++---------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/server/BlaiseAPI/index.ts b/server/BlaiseAPI/index.ts index 2890182..f8eca48 100644 --- a/server/BlaiseAPI/index.ts +++ b/server/BlaiseAPI/index.ts @@ -3,16 +3,18 @@ import { CustomConfig } from "../interfaces/server"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import BlaiseApiClient from "blaise-api-node-client"; import createLogger from "../pino"; +import AuditLogger from "../AuditLogger"; const pinoLogger = createLogger(); -export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaiseApiClient: BlaiseApiClient): Router { +export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaiseApiClient: BlaiseApiClient, auditLogger: AuditLogger): Router { const router = express.Router(); router.get("/api/roles", auth.Middleware, async function (req: Request, res: Response) { res.status(200).json(await blaiseApiClient.getUserRoles()); }); router.get("/api/users", auth.Middleware, async function (req: Request, res: Response) { + auditLogger.info(req.log, `USER: ${req.headers.user} has successfully fetched all users`); res.status(200).json(await blaiseApiClient.getUsers()); }); @@ -40,13 +42,13 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise await blaiseApiClient.changeUserRole(user, role); await blaiseApiClient.changeUserServerParks(user, newServerParks, newDefaultServerPark); const successMessage = `Successfully updated user role and permissions to ${role} for ${user}`; - pinoLogger.logger.info(successMessage + ` at ${(new Date()).toLocaleTimeString("en-UK")} ${(new Date()).toLocaleDateString("en-UK")}`); + auditLogger.info(req.log, successMessage + ` at ${(new Date()).toLocaleTimeString("en-UK")} ${(new Date()).toLocaleDateString("en-UK")}`); return res.status(200).json({ message: successMessage + " today at " + (new Date()).toLocaleTimeString("en-UK") }); } catch (error) { const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}: ${error}`; - pinoLogger.logger.error(errorMessage); + auditLogger.error(req.log, errorMessage + ` at ${(new Date()).toLocaleTimeString("en-UK")} ${(new Date()).toLocaleDateString("en-UK")}`); return res.status(500).json({ message: errorMessage }); @@ -61,14 +63,14 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise try { const user = await blaiseApiClient.getUser(req.params.user); const successMessage = `Successfully fetched user details for ${req.params.user}`; - pinoLogger.logger.info(successMessage); + auditLogger.info(req.log, successMessage); return res.status(200).json({ message: successMessage, data: user }); } catch (error) { const errorMessage = `Error whilst trying to retrieve user ${req.params.user}: ${error}`; - pinoLogger.logger.error(errorMessage); + auditLogger.error(req.log, errorMessage); return res.status(500).json({ message: errorMessage, error: error @@ -88,10 +90,10 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise } blaiseApiClient.changePassword(req.params.user, password).then(() => { - pinoLogger.logger.info(`Successfully changed password for ${req.params.user}`); + auditLogger.info(req.log, `Successfully changed password for ${req.params.user}`); return res.status(204).json(null); }).catch((error: unknown) => { - pinoLogger.logger.error(error); + auditLogger.error(req.log, `Error whilst trying to change password for ${req.params.user}: ${error}`); return res.status(500).json(error); }); }); @@ -104,8 +106,10 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise } if (!user) { + auditLogger.error(req.log, "No user provided for deletion"); return res.status(400).json(); } + auditLogger.info(req.log, `Successfully deleted user: ${user}`); return res.status(204).json(await blaiseApiClient.deleteUser(user)); }); @@ -124,7 +128,7 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise data.defaultServerPark = defaultServerPark[0]; } const currentUser = req.headers.user; - pinoLogger.logger.info(`${currentUser} has created user: ${data.username}`); + auditLogger.info(req.log, `${currentUser} has created user: ${data.username}`); return res.status(200).json(await blaiseApiClient.createUser(data)); }); diff --git a/server/server.ts b/server/server.ts index a431588..d68a1c7 100644 --- a/server/server.ts +++ b/server/server.ts @@ -7,11 +7,11 @@ import BlaiseAPIRouter from "./BlaiseAPI"; import multer from "multer"; import * as profiler from "@google-cloud/profiler"; import { newLoginHandler, Auth } from "blaise-login-react/blaise-login-react-server"; -import pino from "pino"; import { CustomConfig } from "./interfaces/server"; import BlaiseApi from "blaise-api-node-client"; import { Express } from "express"; import fs from "fs"; +import AuditLogger from "./AuditLogger"; export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi, auth: Auth): Express { @@ -21,32 +21,29 @@ export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi }); const upload = multer(); - const server = express(); + const logger = createLogger(); server.use(upload.any()); - - axios.defaults.timeout = 10000; - - const logger = createLogger(); server.use(logger); - logger.logger.info("Server started"); + axios.defaults.timeout = 10000; // where ever the react built package is const buildFolder = "../build"; - + const auditLogger = new AuditLogger(config.ProjectId); const loginHandler = newLoginHandler(auth, blaiseApi); // Health Check endpoint server.get("/bam-ui/:version/health", async function (req: Request, res: Response) { - pinoLogger.logger.info("Heath Check endpoint called"); + auditLogger.info(req.log, "Heath Check endpoint called"); + req.log.info("Heath Check endpoint called"); res.status(200).json({ healthy: true }); }); server.use("/", loginHandler); // All Endpoints calling the Blaise API - server.use("/", BlaiseAPIRouter(config, auth, blaiseApi)); + server.use("/", BlaiseAPIRouter(config, auth, blaiseApi, auditLogger)); // treat the index.html as a template and substitute the values at runtime server.set("views", path.join(__dirname, "/views")); @@ -67,8 +64,10 @@ export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi server.use(function (err, _req, res, _next) { if (err && err.stack) { + auditLogger.error(res, err.stack); console.error(err.stack); } else { + auditLogger.error(res, "An undefined error occurred"); console.error("An undefined error occurred"); } res.render("../views/500.html", {}); From 8cf5e3e4cd37bfeb85a4811adaf8b28d2151b75c Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:16:22 +0000 Subject: [PATCH 08/42] wip: log current user or requester --- server/BlaiseAPI/index.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/server/BlaiseAPI/index.ts b/server/BlaiseAPI/index.ts index f8eca48..a6ff8d5 100644 --- a/server/BlaiseAPI/index.ts +++ b/server/BlaiseAPI/index.ts @@ -2,11 +2,8 @@ import express, { Request, Response, Router } from "express"; import { CustomConfig } from "../interfaces/server"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import BlaiseApiClient from "blaise-api-node-client"; -import createLogger from "../pino"; import AuditLogger from "../AuditLogger"; -const pinoLogger = createLogger(); - export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaiseApiClient: BlaiseApiClient, auditLogger: AuditLogger): Router { const router = express.Router(); router.get("/api/roles", auth.Middleware, async function (req: Request, res: Response) { @@ -14,7 +11,8 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise }); router.get("/api/users", auth.Middleware, async function (req: Request, res: Response) { - auditLogger.info(req.log, `USER: ${req.headers.user} has successfully fetched all users`); + const currentUser = auth.GetUser(auth.GetToken(req)); + auditLogger.info(req.log, `USER: ${currentUser} has successfully fetched all users`); res.status(200).json(await blaiseApiClient.getUsers()); }); From 7c9c1ecbda01cf732949617f776ca4ac6c4d1091 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:37:04 +0000 Subject: [PATCH 09/42] wip: log current user or requester --- server/BlaiseAPI/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/BlaiseAPI/index.ts b/server/BlaiseAPI/index.ts index a6ff8d5..41cf3cf 100644 --- a/server/BlaiseAPI/index.ts +++ b/server/BlaiseAPI/index.ts @@ -11,8 +11,6 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise }); router.get("/api/users", auth.Middleware, async function (req: Request, res: Response) { - const currentUser = auth.GetUser(auth.GetToken(req)); - auditLogger.info(req.log, `USER: ${currentUser} has successfully fetched all users`); res.status(200).json(await blaiseApiClient.getUsers()); }); @@ -77,6 +75,8 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise }); router.get("/api/change-password/:user", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); + auditLogger.info(req.log, `CURRENT USER: ${currentUser}`); let { password } = req.headers; if (Array.isArray(password)) { From 3759235fbefe773aa030835734514e657c960451 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:58:10 +0000 Subject: [PATCH 10/42] wip: log current user or requester --- server/BlaiseAPI/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/server/BlaiseAPI/index.ts b/server/BlaiseAPI/index.ts index 41cf3cf..e169405 100644 --- a/server/BlaiseAPI/index.ts +++ b/server/BlaiseAPI/index.ts @@ -77,6 +77,7 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise router.get("/api/change-password/:user", auth.Middleware, async function (req: Request, res: Response) { const currentUser = auth.GetUser(auth.GetToken(req)); auditLogger.info(req.log, `CURRENT USER: ${currentUser}`); + req.log.info(`CURRENT USER: ${currentUser}`); let { password } = req.headers; if (Array.isArray(password)) { From e8723a606e05deb3122f9ffd8775dbdf295cc32c Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 21:12:41 +0000 Subject: [PATCH 11/42] feat: log current user initiating the action --- server/BlaiseAPI/index.ts | 27 ++++++++++++++++----------- server/server.ts | 1 - 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/server/BlaiseAPI/index.ts b/server/BlaiseAPI/index.ts index e169405..a3a1c29 100644 --- a/server/BlaiseAPI/index.ts +++ b/server/BlaiseAPI/index.ts @@ -6,6 +6,7 @@ import AuditLogger from "../AuditLogger"; export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaiseApiClient: BlaiseApiClient, auditLogger: AuditLogger): Router { const router = express.Router(); + router.get("/api/roles", auth.Middleware, async function (req: Request, res: Response) { res.status(200).json(await blaiseApiClient.getUserRoles()); }); @@ -15,6 +16,7 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise }); router.patch("/api/users/:user/rolesAndPermissions", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); const { role } = req.body; const user = req.params.user; let newServerParks = [""]; @@ -37,14 +39,15 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise try { await blaiseApiClient.changeUserRole(user, role); await blaiseApiClient.changeUserServerParks(user, newServerParks, newDefaultServerPark); - const successMessage = `Successfully updated user role and permissions to ${role} for ${user}`; - auditLogger.info(req.log, successMessage + ` at ${(new Date()).toLocaleTimeString("en-UK")} ${(new Date()).toLocaleDateString("en-UK")}`); + const successMessage = `${currentUser.name || "Unknown"} has successfully updated user role and permissions to ${role} for ${user}`; + auditLogger.info(req.log, successMessage); return res.status(200).json({ - message: successMessage + " today at " + (new Date()).toLocaleTimeString("en-UK") + message: successMessage }); } catch (error) { const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}: ${error}`; - auditLogger.error(req.log, errorMessage + ` at ${(new Date()).toLocaleTimeString("en-UK")} ${(new Date()).toLocaleDateString("en-UK")}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has failed to update user role and permissions to ${role} for ${user}`); + auditLogger.error(req.log, errorMessage); return res.status(500).json({ message: errorMessage }); @@ -76,8 +79,6 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise router.get("/api/change-password/:user", auth.Middleware, async function (req: Request, res: Response) { const currentUser = auth.GetUser(auth.GetToken(req)); - auditLogger.info(req.log, `CURRENT USER: ${currentUser}`); - req.log.info(`CURRENT USER: ${currentUser}`); let { password } = req.headers; if (Array.isArray(password)) { @@ -89,15 +90,17 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise } blaiseApiClient.changePassword(req.params.user, password).then(() => { - auditLogger.info(req.log, `Successfully changed password for ${req.params.user}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully changed the password for ${req.params.user}`); return res.status(204).json(null); }).catch((error: unknown) => { + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has failed to change the password for ${req.params.user}`); auditLogger.error(req.log, `Error whilst trying to change password for ${req.params.user}: ${error}`); return res.status(500).json(error); }); }); router.delete("/api/users", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); let { user } = req.headers; if (Array.isArray(user)) { @@ -108,15 +111,18 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise auditLogger.error(req.log, "No user provided for deletion"); return res.status(400).json(); } - auditLogger.info(req.log, `Successfully deleted user: ${user}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user: ${user}`); return res.status(204).json(await blaiseApiClient.deleteUser(user)); }); router.post("/api/users", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); const data = req.body; - if(!data.role){ + + if(!data.role) { return res.status(400).json({ message: "No role provided for user creation" }); } + const roleServerParksOverride = config.RoleToServerParksMap[data.role]; if (roleServerParksOverride != null) { data.serverParks = roleServerParksOverride; @@ -126,8 +132,7 @@ export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaise data.serverParks = defaultServerPark; data.defaultServerPark = defaultServerPark[0]; } - const currentUser = req.headers.user; - auditLogger.info(req.log, `${currentUser} has created user: ${data.username}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user: ${data.username}`); return res.status(200).json(await blaiseApiClient.createUser(data)); }); diff --git a/server/server.ts b/server/server.ts index d68a1c7..a0c6f9a 100644 --- a/server/server.ts +++ b/server/server.ts @@ -36,7 +36,6 @@ export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi // Health Check endpoint server.get("/bam-ui/:version/health", async function (req: Request, res: Response) { auditLogger.info(req.log, "Heath Check endpoint called"); - req.log.info("Heath Check endpoint called"); res.status(200).json({ healthy: true }); }); From 0909143ce334894beb23ed741f9ed5913c46887f Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 29 Oct 2024 21:14:04 +0000 Subject: [PATCH 12/42] test: involve auth process in tests by using mock JWT values --- package.json | 1 + server/tests/index.test.ts | 59 +++++++++++++++++++++++++++----------- yarn.lock | 16 +++++++++++ 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index 093130e..72a51fc 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "eslint-plugin-prettier": "^5.1.3", "eslint-plugin-react": "^7.28.0", "eslint-plugin-react-hooks": "^4.3.0", + "jsonwebtoken": "^9.0.2", "pino-pretty": "^4.7.1", "supertest": "^6.1.6" }, diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index 351f70d..d21aa7f 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -8,21 +8,32 @@ import { loadConfigFromEnv } from "../Config"; import BlaiseApiClient, { NewUser, User, UserRole } from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import { IMock, Mock, It, Times } from "typemoq"; +import jwt from "jsonwebtoken"; // Temporary fix for Jest open handle issue (gcp profiler TCPWRAP error) jest.mock("@google-cloud/profiler", () => ({ start: jest.fn().mockReturnValue(Promise.resolve()) })); +const mockSecret = "super-secret-key"; +process.env = Object.assign(process.env, { + SESSION_SECRET: mockSecret, + PROJECT_ID: "mockProjectId", + BLAISE_API_URL: "http://blaise-api", + SERVER_PARK: "mockServerPark", + SESSION_TIMEOUT: "12h" +}); const config = loadConfigFromEnv(); -const blaiseApiMock: IMock = Mock.ofType(BlaiseApiClient); -Auth.prototype.ValidateToken = jest.fn().mockReturnValue(true); const auth = new Auth(config); +const mockUser: User = { name: "testUser", role: "DST", defaultServerPark: "park1", serverParks: ["park1", "park2"] }; +const mockAuthToken = jwt.sign({ "user": mockUser }, mockSecret); + +const blaiseApiMock: IMock = Mock.ofType(BlaiseApiClient); + const server = GetNodeServer(config, blaiseApiMock.object, auth); const sut = supertest(server); describe("Test Heath Endpoint", () => { - it("should return a 200 status and json message", async () => { const response = await sut.get("/bam-ui/version/health"); @@ -35,9 +46,9 @@ describe("app engine start", () => { it("should return a 200 status and json message", async () => { process.env = Object.assign({ - PROJECT_ID: "mock", - BLAISE_API_URL: "http://mock", - SERVER_PARK: "mock", + PROJECT_ID: "mockProjectId", + BLAISE_API_URL: "http://blaise-api", + SERVER_PARK: "mockServerPark", SESSION_TIMEOUT: "12h" }); @@ -49,8 +60,6 @@ describe("app engine start", () => { import role_to_serverparks_map from "../role-to-serverparks-map.json"; import { size } from "lodash"; -import { Exception } from "typemoq/Error/Exception"; -import { Errback } from "express"; describe("Test /api/users POST endpoint", () => { it("should call Blaise API createUser endpoint with correct serverParks for each role EXISTING in server/role-to-serverparks-map.json AND return http status OK_200", async () => { let currentRoleNo = 0; @@ -71,6 +80,7 @@ describe("Test /api/users POST endpoint", () => { blaiseApiMock.setup((api) => api.createUser(It.isAny())).returns(async () => newUser); const response = await sut.post("/api/users") + .set("Authorization", `${mockAuthToken}`) .field("role", roleName); expect(response.statusCode).toEqual(200); @@ -97,7 +107,8 @@ describe("Test /api/users POST endpoint", () => { blaiseApiMock.setup((api) => api.createUser(It.isAny())).returns(async () => newUser); const response = await sut.post("/api/users") - .field("role", roleName); + .field("role", roleName) + .set("Authorization", `${mockAuthToken}`); expect(response.statusCode).toEqual(200); blaiseApiMock.verify(a => a.createUser(It.is( @@ -111,10 +122,12 @@ describe("Test /api/users POST endpoint", () => { it("should return http status BAD_REQUEST_400 if role is empty OR hasn't been specified in the request", async () => { let response = await sut.post("/api/users") - .field("role", ""); + .field("role", "") + .set("Authorization", `${mockAuthToken}`); expect(response.statusCode).toEqual(400); - response = await sut.post("/api/users"); + response = await sut.post("/api/users") + .set("Authorization", `${mockAuthToken}`); expect(response.statusCode).toEqual(400); }); }); @@ -132,6 +145,7 @@ describe("Test /api/users DELETE endpoint", () => { blaiseApiMock.setup((api) => api.deleteUser(It.isAny())).returns(_ => Promise.resolve(null)); const username = "user-123"; const response = await sut.delete("/api/users") + .set("Authorization", `${mockAuthToken}`) .set("user", username); expect(response.statusCode).toEqual(204); @@ -140,10 +154,12 @@ describe("Test /api/users DELETE endpoint", () => { it("should call Blaise API deleteUser endpoint for INVALID or MISSING user header field AND return http status BAD_REQUEST_400", async () => { let response = await sut.delete("/api/users") + .set("Authorization", `${mockAuthToken}`) .set("user", ""); expect(response.statusCode).toEqual(400); - response = await sut.delete("/api/users"); + response = await sut.delete("/api/users") + .set("Authorization", `${mockAuthToken}`); expect(response.statusCode).toEqual(400); }); }); @@ -164,7 +180,8 @@ describe("Test /api/users GET endpoint", () => { const userArray : NewUser [] = [newUser1, newUser2, newUser3]; blaiseApiMock.setup((api) => api.getUsers()).returns(_ => Promise.resolve(userArray)); - const response = await sut.get("/api/users"); + const response = await sut.get("/api/users") + .set("Authorization", `${mockAuthToken}`); expect(response.statusCode).toEqual(200); blaiseApiMock.verify(a => a.getUsers(), Times.once()); @@ -185,13 +202,15 @@ describe("Test /api/roles GET endpoint", () => { const userRoleArray : UserRole [] = [userRole1, userRole2, userRole3]; blaiseApiMock.setup((api) => api.getUserRoles()).returns(_ => Promise.resolve(userRoleArray)); - const response = await sut.get("/api/roles"); + const response = await sut.get("/api/roles") + .set("Authorization", `${mockAuthToken}`); expect(response.statusCode).toEqual(200); blaiseApiMock.verify(a => a.getUserRoles(), Times.once()); }); }); +// TODO: Changing a resource should be a PATCH request not a GET request describe("Test /api/change-password/:user GET endpoint", () => { beforeEach(() => { blaiseApiMock.reset(); @@ -207,6 +226,7 @@ describe("Test /api/change-password/:user GET endpoint", () => { blaiseApiMock.setup((api) => api.changePassword(It.isAnyString(), It.isAnyString())).returns(_ => Promise.resolve(null)); const response = await sut.get("/api/change-password/"+username) + .set("Authorization", `${mockAuthToken}`) .set("password", password); expect(response.statusCode).toEqual(204); @@ -217,7 +237,8 @@ describe("Test /api/change-password/:user GET endpoint", () => { const username = "user1"; const password = ""; - const response = await sut.get("/api/change-password/"+username) + const response = await sut.get("/api/change-password/"+ username) + .set("Authorization", `${mockAuthToken}`) .set("password", password); expect(response.statusCode).toEqual(400); @@ -231,7 +252,8 @@ describe("Test /api/change-password/:user GET endpoint", () => { blaiseApiMock.setup((a) => a.changePassword(It.isAnyString(), It.isAnyString())) .returns(_ => Promise.reject(errorMessage)); - const response = await sut.get("/api/change-password/"+username) + const response = await sut.get("/api/change-password/"+ username) + .set("Authorization", `${mockAuthToken}`) .set("password", password); expect(response.statusCode).toEqual(500); @@ -260,10 +282,11 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { .returns(async () => null); const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) + .set("Authorization", `${mockAuthToken}`) .send({ role }); expect(response.statusCode).toEqual(200); - expect(response.body.message).toContain(`Successfully updated user role and permissions to ${role} for ${user}`); + expect(response.body.message).toContain(`${mockUser.name} has successfully updated user role and permissions to ${role} for ${user}`); blaiseApiMock.verify(api => api.changeUserRole(It.isValue(user), It.isValue(role)), Times.once()); blaiseApiMock.verify(api => api.changeUserServerParks(It.isValue(user), It.isValue(serverParks), It.isValue(defaultServerPark)), Times.once()); }); @@ -273,6 +296,7 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { const role = ""; const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) + .set("Authorization", `${mockAuthToken}`) .send({ role }); expect(response.statusCode).toEqual(400); @@ -287,6 +311,7 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { .returns(async () => { throw new Error(errorMessage); }); const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) + .set("Authorization", `${mockAuthToken}`) .send({ role }); expect(response.statusCode).toEqual(500); diff --git a/yarn.lock b/yarn.lock index e111c4f..9e80077 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11788,6 +11788,22 @@ jsonwebtoken@^8.5.1: ms "^2.1.1" semver "^5.6.0" +jsonwebtoken@^9.0.2: + version "9.0.2" + resolved "https://registry.yarnpkg.com/jsonwebtoken/-/jsonwebtoken-9.0.2.tgz#65ff91f4abef1784697d40952bb1998c504caaf3" + integrity sha512-PRp66vJ865SSqOlgqS8hujT5U4AOgMfhrwYIuIhfKaoSCZcirrmASQr8CX7cUg+RMih+hgznrjp99o+W4pJLHQ== + dependencies: + jws "^3.2.2" + lodash.includes "^4.3.0" + lodash.isboolean "^3.0.3" + lodash.isinteger "^4.0.4" + lodash.isnumber "^3.0.3" + lodash.isplainobject "^4.0.6" + lodash.isstring "^4.0.1" + lodash.once "^4.0.0" + ms "^2.1.1" + semver "^7.5.4" + "jsx-ast-utils@^2.4.1 || ^3.0.0": version "3.3.3" resolved "https://registry.yarnpkg.com/jsx-ast-utils/-/jsx-ast-utils-3.3.3.tgz#76b3e6e6cece5c69d49a5792c3d01bd1a0cdc7ea" From 5ff600fb1c747cb65810edde54b63374aec5ca2d Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Wed, 30 Oct 2024 18:33:35 +0000 Subject: [PATCH 13/42] feat/refactor: add logging to create, delete and update/edit actions --- server/index.ts | 11 +-- server/interfaces/logger.ts | 6 ++ server/logger/cloudLogging.ts | 50 ++++++++++++ server/logger/pinoLogger.ts | 43 +++++++++++ server/routes/auditLogs.ts | 34 +++++++++ server/routes/blaiseApi.ts | 140 ++++++++++++++++++++++++++++++++++ server/server.ts | 43 ++++++----- 7 files changed, 301 insertions(+), 26 deletions(-) create mode 100644 server/interfaces/logger.ts create mode 100644 server/logger/cloudLogging.ts create mode 100644 server/logger/pinoLogger.ts create mode 100644 server/routes/auditLogs.ts create mode 100644 server/routes/blaiseApi.ts diff --git a/server/index.ts b/server/index.ts index eaef685..56ba7f5 100644 --- a/server/index.ts +++ b/server/index.ts @@ -1,12 +1,12 @@ import GetNodeServer from "./server"; -import pino from "pino"; -import { loadConfigFromEnv } from "./Config"; +import { loadConfigFromEnv } from "./config"; import BlaiseApiClient from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import dotenv from "dotenv"; +import createLogger from "./logger/pinoLogger"; +import AuditLogger from "./logger/cloudLogging"; const port: string = process.env.PORT || "5002"; -const logger = pino(); if (process.env.NODE_ENV !== "production") { dotenv.config(); @@ -14,7 +14,8 @@ if (process.env.NODE_ENV !== "production") { const config = loadConfigFromEnv(); const blaiseApiClient = new BlaiseApiClient(config.BlaiseApiUrl); const auth = new Auth(config); -const server = GetNodeServer(config, blaiseApiClient, auth); +const pinoLogger = createLogger(); +const server = GetNodeServer(config, blaiseApiClient, auth, pinoLogger); server.listen(port); -logger.info("App is listening on port " + port); +pinoLogger.logger.info("BAM nodejs server is listening on port " + port); diff --git a/server/interfaces/logger.ts b/server/interfaces/logger.ts new file mode 100644 index 0000000..9b46185 --- /dev/null +++ b/server/interfaces/logger.ts @@ -0,0 +1,6 @@ +export interface AuditLog { + id: string; + timestamp: string; + message: string; + severity: string; +} \ No newline at end of file diff --git a/server/logger/cloudLogging.ts b/server/logger/cloudLogging.ts new file mode 100644 index 0000000..389f35d --- /dev/null +++ b/server/logger/cloudLogging.ts @@ -0,0 +1,50 @@ +import { Logging } from "@google-cloud/logging"; +import { IncomingMessage } from "http"; +import { AuditLog } from "../interfaces/logger"; + +export default class AuditLogger { + projectId: string; + logger: Logging; + logName: string; + + constructor(projectId: string) { + this.projectId = projectId; + this.logger = new Logging({ projectId: this.projectId }); + this.logName = `projects/${this.projectId}/logs/stdout`; + } + + info(logger: IncomingMessage["log"], message: string): void { + logger.info(`AUDIT_LOG: ${message}`); + } + + error(logger: IncomingMessage["log"], message: string): void { + logger.error(`AUDIT_LOG: ${message}`); + } + + async getLogs(): Promise { + const auditLogs: AuditLog[] = []; + const log = this.logger.log(this.logName); + const [entries] = await log.getEntries({ filter: "jsonPayload.message=~\"^AUDIT_LOG: \"", maxResults: 50 }); + for (const entry of entries) { + let id = ""; + let timestamp = ""; + let severity = "INFO"; + if (entry.metadata.insertId != null) { + id = entry.metadata.insertId; + } + if (entry.metadata.timestamp != null) { + timestamp = entry.metadata.timestamp.toString(); + } + if (entry.metadata.severity != null) { + severity = entry.metadata.severity.toString(); + } + auditLogs.push({ + id: id, + timestamp: timestamp, + message: entry.data.message.replace(/^AUDIT_LOG: /, ""), + severity: severity + }); + } + return auditLogs; + } +} diff --git a/server/logger/pinoLogger.ts b/server/logger/pinoLogger.ts new file mode 100644 index 0000000..4594ef2 --- /dev/null +++ b/server/logger/pinoLogger.ts @@ -0,0 +1,43 @@ +import logger from "pino-http"; + +// https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#logseverity +const PinoLevelToSeverityLookup = { + trace: "DEBUG", + debug: "DEBUG", + info: "INFO", + warn: "WARNING", + error: "ERROR", + fatal: "CRITICAL" +}; + +const defaultPinoConf = { + messageKey: "message", + formatters: { + level(label: unknown, number: unknown) { + return { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + severity: PinoLevelToSeverityLookup[label] || PinoLevelToSeverityLookup["info"], + level: number + }; + }, + log(info: never) { + return { info }; + } + }, + serializers: { + req: (req) => ({ + method: req.method, + url: req.url, + user: req.raw.user + }) + } +}; + +export default function createLogger(options = { autoLogging: false }): logger.HttpLogger { + let pinoConfig = {}; + if (process.env.NODE_ENV === "production") { + pinoConfig = defaultPinoConf; + } + return logger(Object.assign({}, options, pinoConfig)); +} diff --git a/server/routes/auditLogs.ts b/server/routes/auditLogs.ts new file mode 100644 index 0000000..d91c770 --- /dev/null +++ b/server/routes/auditLogs.ts @@ -0,0 +1,34 @@ +import express, { Request, Response, Router } from "express"; +import AuditLogger from "../logger/cloudLogging"; +import { Auth } from "blaise-login-react/blaise-login-react-server"; + +export default function auditLogs(auditlogger: AuditLogger, auth: Auth): Router { + const router = express.Router(); + + const auditHandler = new AuditHandler(auditlogger, auth); + return router.get("/api/audit", auth.Middleware, auditHandler.GetAuditLogs); +} + +export class AuditHandler { + auditLogger: AuditLogger; + auth: Auth; + + constructor(auditLogger: AuditLogger, auth: Auth) { + this.auditLogger = auditLogger; + this.auth = auth; + + this.GetAuditLogs = this.GetAuditLogs.bind(this); + } + + async GetAuditLogs(req: Request, res: Response): Promise { + const currentUser = this.auth.GetUser(this.auth.GetToken(req)); + try { + const logs = await this.auditLogger.getLogs(); + this.auditLogger.info(req.log, `${currentUser.name} retrieved audit logs`); + return res.status(200).json(logs); + } catch (error: unknown) { + this.auditLogger.error(req.log, `${currentUser.name} failed to get audit logs`); + return res.status(500).json(error); + } + } +} diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts new file mode 100644 index 0000000..61c5e48 --- /dev/null +++ b/server/routes/blaiseApi.ts @@ -0,0 +1,140 @@ +import express, { Request, Response, Router } from "express"; +import { CustomConfig } from "../interfaces/server"; +import { Auth } from "blaise-login-react/blaise-login-react-server"; +import BlaiseApiClient from "blaise-api-node-client"; +import AuditLogger from "../logger/cloudLogging"; + +export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiClient: BlaiseApiClient, auditLogger: AuditLogger): Router { + const router = express.Router(); + + router.get("/api/roles", auth.Middleware, async function (req: Request, res: Response) { + res.status(200).json(await blaiseApiClient.getUserRoles()); + }); + + router.get("/api/users", auth.Middleware, async function (req: Request, res: Response) { + res.status(200).json(await blaiseApiClient.getUsers()); + }); + + router.patch("/api/users/:user/rolesAndPermissions", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); + const { role } = req.body; + const user = req.params.user; + let newServerParks = [""]; + let newDefaultServerPark = ""; + + if (!req.params.user || !req.body.role) { + return res.status(400).json("No user or role provided"); + } + + const roleServerParksOverride = config.RoleToServerParksMap[role]; + if (roleServerParksOverride != null) { + newServerParks = roleServerParksOverride; + newDefaultServerPark = roleServerParksOverride[0]; + } else { + const defaultServerPark = config.RoleToServerParksMap["DEFAULT"]; + newServerParks = defaultServerPark; + newDefaultServerPark = defaultServerPark[0]; + } + + try { + await blaiseApiClient.changeUserRole(user, role); + await blaiseApiClient.changeUserServerParks(user, newServerParks, newDefaultServerPark); + const successMessage = `${currentUser.name || "Unknown"} has successfully updated user role and permissions to ${role} for ${user}`; + auditLogger.info(req.log, successMessage); + return res.status(200).json({ + message: successMessage + }); + } catch (error) { + const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}: ${error}`; + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has failed to update user role and permissions to ${role} for ${user}`); + auditLogger.error(req.log, errorMessage); + return res.status(500).json({ + message: errorMessage + }); + } + }); + + router.get("/api/users/:user", auth.Middleware, async function (req: Request, res: Response) { + if (!req.params.user) { + return res.status(400).json("No user provided"); + } + + try { + const user = await blaiseApiClient.getUser(req.params.user); + const successMessage = `Successfully fetched user details for ${req.params.user}`; + auditLogger.info(req.log, successMessage); + return res.status(200).json({ + message: successMessage, + data: user + }); + } catch (error) { + const errorMessage = `Error whilst trying to retrieve user ${req.params.user}: ${error}`; + auditLogger.error(req.log, errorMessage); + return res.status(500).json({ + message: errorMessage, + error: error + }); + } + }); + + router.get("/api/change-password/:user", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); + let { password } = req.headers; + + if (Array.isArray(password)) { + password = password.join(""); + } + + if (!req.params.user || !password) { + return res.status(400).json(); + } + + blaiseApiClient.changePassword(req.params.user, password).then(() => { + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully changed the password for ${req.params.user}`); + return res.status(204).json(null); + }).catch((error: unknown) => { + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has failed to change the password for ${req.params.user}`); + auditLogger.error(req.log, `Error whilst trying to change password for ${req.params.user}: ${error}`); + return res.status(500).json(error); + }); + }); + + router.delete("/api/users", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); + let { user } = req.headers; + + if (Array.isArray(user)) { + user = user.join(""); + } + + if (!user) { + auditLogger.error(req.log, "No user provided for deletion"); + return res.status(400).json(); + } + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user: ${user}`); + return res.status(204).json(await blaiseApiClient.deleteUser(user)); + }); + + router.post("/api/users", auth.Middleware, async function (req: Request, res: Response) { + const currentUser = auth.GetUser(auth.GetToken(req)); + const data = req.body; + + if(!data.role) { + return res.status(400).json({ message: "No role provided for user creation" }); + } + + const roleServerParksOverride = config.RoleToServerParksMap[data.role]; + if (roleServerParksOverride != null) { + data.serverParks = roleServerParksOverride; + data.defaultServerPark = roleServerParksOverride[0]; + } else { + const defaultServerPark = config.RoleToServerParksMap["DEFAULT"]; + data.serverParks = defaultServerPark; + data.defaultServerPark = defaultServerPark[0]; + } + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user: ${data.username}`); + return res.status(200).json(await blaiseApiClient.createUser(data)); + }); + + return router; +} diff --git a/server/server.ts b/server/server.ts index a0c6f9a..615dedf 100644 --- a/server/server.ts +++ b/server/server.ts @@ -2,8 +2,7 @@ import express, { Request, Response } from "express"; import axios from "axios"; import path from "path"; import ejs from "ejs"; -import createLogger from "./pino"; -import BlaiseAPIRouter from "./BlaiseAPI"; +import createLogger from "./logger/pinoLogger"; import multer from "multer"; import * as profiler from "@google-cloud/profiler"; import { newLoginHandler, Auth } from "blaise-login-react/blaise-login-react-server"; @@ -11,38 +10,40 @@ import { CustomConfig } from "./interfaces/server"; import BlaiseApi from "blaise-api-node-client"; import { Express } from "express"; import fs from "fs"; -import AuditLogger from "./AuditLogger"; +import AuditLogger from "./logger/cloudLogging"; +import auditLogs from "./routes/auditLogs"; +import blaiseApi from "./routes/blaiseApi"; +import { Logger } from "pino"; +import { HttpLogger } from "pino-http"; -export default function GetNodeServer(config: CustomConfig, blaiseApi: BlaiseApi, auth: Auth): Express +export default function GetNodeServer(config: CustomConfig, blaiseApiClient: BlaiseApi, auth: Auth, logger: HttpLogger): Express { - const pinoLogger = createLogger(); - profiler.start({ logLevel: 4 }).catch((err: unknown) => { - pinoLogger.logger.error(`Failed to start profiler: ${err}`); - }); - + const auditLogger = new AuditLogger(config.ProjectId); + const pinoLogger = logger; const upload = multer(); const server = express(); - const logger = createLogger(); + // where ever the react built package is + const buildFolder = "../build"; server.use(upload.any()); - server.use(logger); + server.use(pinoLogger); axios.defaults.timeout = 10000; - // where ever the react built package is - const buildFolder = "../build"; - const auditLogger = new AuditLogger(config.ProjectId); - const loginHandler = newLoginHandler(auth, blaiseApi); + const loginRouter = newLoginHandler(auth, blaiseApiClient); + const auditRouter = auditLogs(auditLogger, auth); + const blaiseApiRouter = blaiseApi(config, auth, blaiseApiClient, auditLogger); - // Health Check endpoint + profiler.start({ logLevel: 4 }).catch((err: unknown) => { + pinoLogger.logger.info(`Failed to start GCP profiler: ${err}`); + }); + // GCP health check server.get("/bam-ui/:version/health", async function (req: Request, res: Response) { auditLogger.info(req.log, "Heath Check endpoint called"); res.status(200).json({ healthy: true }); }); - - server.use("/", loginHandler); - - // All Endpoints calling the Blaise API - server.use("/", BlaiseAPIRouter(config, auth, blaiseApi, auditLogger)); + server.use("/", loginRouter); + server.use("/", blaiseApiRouter); + server.use("/", auditRouter); // treat the index.html as a template and substitute the values at runtime server.set("views", path.join(__dirname, "/views")); From c7a4e0036e726687d895c469bbe4f57588f09e18 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Wed, 30 Oct 2024 18:34:01 +0000 Subject: [PATCH 14/42] refactor: restructure server folder --- server/AuditLogger/index.ts | 56 --------------- server/BlaiseAPI/index.ts | 140 ------------------------------------ server/interfaces/server.ts | 1 - server/pino/index.ts | 43 ----------- 4 files changed, 240 deletions(-) delete mode 100644 server/AuditLogger/index.ts delete mode 100644 server/BlaiseAPI/index.ts delete mode 100644 server/pino/index.ts diff --git a/server/AuditLogger/index.ts b/server/AuditLogger/index.ts deleted file mode 100644 index 4c6d1d9..0000000 --- a/server/AuditLogger/index.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { Logging } from "@google-cloud/logging"; -import { IncomingMessage } from "http"; - -export interface AuditLog { - id: string; - timestamp: string; - message: string; - severity: string; -} - -export default class AuditLogger { - projectId: string; - logger: Logging; - logName: string; - - constructor(projectId: string) { - this.projectId = projectId; - this.logger = new Logging({ projectId: this.projectId }); - this.logName = `projects/${this.projectId}/logs/stdout`; - } - - info(logger: IncomingMessage["log"], message: string): void { - logger.info(`AUDIT_LOG: ${message}`); - } - - error(logger: IncomingMessage["log"], message: string): void { - logger.error(`AUDIT_LOG: ${message}`); - } - - async getLogs(): Promise { - const auditLogs: AuditLog[] = []; - const log = this.logger.log(this.logName); - const [entries] = await log.getEntries({ filter: "jsonPayload.message=~\"^AUDIT_LOG: \"", maxResults: 50 }); - for (const entry of entries) { - let id = ""; - let timestamp = ""; - let severity = "INFO"; - if (entry.metadata.insertId != null) { - id = entry.metadata.insertId; - } - if (entry.metadata.timestamp != null) { - timestamp = entry.metadata.timestamp.toString(); - } - if (entry.metadata.severity != null) { - severity = entry.metadata.severity.toString(); - } - auditLogs.push({ - id: id, - timestamp: timestamp, - message: entry.data.message.replace(/^AUDIT_LOG: /, ""), - severity: severity - }); - } - return auditLogs; - } -} diff --git a/server/BlaiseAPI/index.ts b/server/BlaiseAPI/index.ts deleted file mode 100644 index a3a1c29..0000000 --- a/server/BlaiseAPI/index.ts +++ /dev/null @@ -1,140 +0,0 @@ -import express, { Request, Response, Router } from "express"; -import { CustomConfig } from "../interfaces/server"; -import { Auth } from "blaise-login-react/blaise-login-react-server"; -import BlaiseApiClient from "blaise-api-node-client"; -import AuditLogger from "../AuditLogger"; - -export default function BlaiseAPIRouter(config: CustomConfig, auth: Auth, blaiseApiClient: BlaiseApiClient, auditLogger: AuditLogger): Router { - const router = express.Router(); - - router.get("/api/roles", auth.Middleware, async function (req: Request, res: Response) { - res.status(200).json(await blaiseApiClient.getUserRoles()); - }); - - router.get("/api/users", auth.Middleware, async function (req: Request, res: Response) { - res.status(200).json(await blaiseApiClient.getUsers()); - }); - - router.patch("/api/users/:user/rolesAndPermissions", auth.Middleware, async function (req: Request, res: Response) { - const currentUser = auth.GetUser(auth.GetToken(req)); - const { role } = req.body; - const user = req.params.user; - let newServerParks = [""]; - let newDefaultServerPark = ""; - - if (!req.params.user || !req.body.role) { - return res.status(400).json("No user or role provided"); - } - - const roleServerParksOverride = config.RoleToServerParksMap[role]; - if (roleServerParksOverride != null) { - newServerParks = roleServerParksOverride; - newDefaultServerPark = roleServerParksOverride[0]; - } else { - const defaultServerPark = config.RoleToServerParksMap["DEFAULT"]; - newServerParks = defaultServerPark; - newDefaultServerPark = defaultServerPark[0]; - } - - try { - await blaiseApiClient.changeUserRole(user, role); - await blaiseApiClient.changeUserServerParks(user, newServerParks, newDefaultServerPark); - const successMessage = `${currentUser.name || "Unknown"} has successfully updated user role and permissions to ${role} for ${user}`; - auditLogger.info(req.log, successMessage); - return res.status(200).json({ - message: successMessage - }); - } catch (error) { - const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}: ${error}`; - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has failed to update user role and permissions to ${role} for ${user}`); - auditLogger.error(req.log, errorMessage); - return res.status(500).json({ - message: errorMessage - }); - } - }); - - router.get("/api/users/:user", auth.Middleware, async function (req: Request, res: Response) { - if (!req.params.user) { - return res.status(400).json("No user provided"); - } - - try { - const user = await blaiseApiClient.getUser(req.params.user); - const successMessage = `Successfully fetched user details for ${req.params.user}`; - auditLogger.info(req.log, successMessage); - return res.status(200).json({ - message: successMessage, - data: user - }); - } catch (error) { - const errorMessage = `Error whilst trying to retrieve user ${req.params.user}: ${error}`; - auditLogger.error(req.log, errorMessage); - return res.status(500).json({ - message: errorMessage, - error: error - }); - } - }); - - router.get("/api/change-password/:user", auth.Middleware, async function (req: Request, res: Response) { - const currentUser = auth.GetUser(auth.GetToken(req)); - let { password } = req.headers; - - if (Array.isArray(password)) { - password = password.join(""); - } - - if (!req.params.user || !password) { - return res.status(400).json(); - } - - blaiseApiClient.changePassword(req.params.user, password).then(() => { - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully changed the password for ${req.params.user}`); - return res.status(204).json(null); - }).catch((error: unknown) => { - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has failed to change the password for ${req.params.user}`); - auditLogger.error(req.log, `Error whilst trying to change password for ${req.params.user}: ${error}`); - return res.status(500).json(error); - }); - }); - - router.delete("/api/users", auth.Middleware, async function (req: Request, res: Response) { - const currentUser = auth.GetUser(auth.GetToken(req)); - let { user } = req.headers; - - if (Array.isArray(user)) { - user = user.join(""); - } - - if (!user) { - auditLogger.error(req.log, "No user provided for deletion"); - return res.status(400).json(); - } - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user: ${user}`); - return res.status(204).json(await blaiseApiClient.deleteUser(user)); - }); - - router.post("/api/users", auth.Middleware, async function (req: Request, res: Response) { - const currentUser = auth.GetUser(auth.GetToken(req)); - const data = req.body; - - if(!data.role) { - return res.status(400).json({ message: "No role provided for user creation" }); - } - - const roleServerParksOverride = config.RoleToServerParksMap[data.role]; - if (roleServerParksOverride != null) { - data.serverParks = roleServerParksOverride; - data.defaultServerPark = roleServerParksOverride[0]; - } else { - const defaultServerPark = config.RoleToServerParksMap["DEFAULT"]; - data.serverParks = defaultServerPark; - data.defaultServerPark = defaultServerPark[0]; - } - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user: ${data.username}`); - return res.status(200).json(await blaiseApiClient.createUser(data)); - }); - - return router; -} diff --git a/server/interfaces/server.ts b/server/interfaces/server.ts index 9a82c4e..061cbb8 100644 --- a/server/interfaces/server.ts +++ b/server/interfaces/server.ts @@ -1,5 +1,4 @@ import { AuthConfig } from "blaise-login-react/blaise-login-react-server"; -import { string } from "prop-types"; export interface CustomConfig extends AuthConfig { BlaiseApiUrl: string diff --git a/server/pino/index.ts b/server/pino/index.ts deleted file mode 100644 index 4594ef2..0000000 --- a/server/pino/index.ts +++ /dev/null @@ -1,43 +0,0 @@ -import logger from "pino-http"; - -// https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#logseverity -const PinoLevelToSeverityLookup = { - trace: "DEBUG", - debug: "DEBUG", - info: "INFO", - warn: "WARNING", - error: "ERROR", - fatal: "CRITICAL" -}; - -const defaultPinoConf = { - messageKey: "message", - formatters: { - level(label: unknown, number: unknown) { - return { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - severity: PinoLevelToSeverityLookup[label] || PinoLevelToSeverityLookup["info"], - level: number - }; - }, - log(info: never) { - return { info }; - } - }, - serializers: { - req: (req) => ({ - method: req.method, - url: req.url, - user: req.raw.user - }) - } -}; - -export default function createLogger(options = { autoLogging: false }): logger.HttpLogger { - let pinoConfig = {}; - if (process.env.NODE_ENV === "production") { - pinoConfig = defaultPinoConf; - } - return logger(Object.assign({}, options, pinoConfig)); -} From e937804181557062762b2a19a7b135086a318f48 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Wed, 30 Oct 2024 18:34:19 +0000 Subject: [PATCH 15/42] test: include log asserts in unit tests --- server/tests/index.test.ts | 93 +++++++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 11 deletions(-) diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index d21aa7f..c3af4ba 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -4,11 +4,17 @@ import supertest from "supertest"; import GetNodeServer from "../server"; -import { loadConfigFromEnv } from "../Config"; +import AuditLogger from "../logger/cloudLogging"; +import { loadConfigFromEnv } from "../config"; import BlaiseApiClient, { NewUser, User, UserRole } from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import { IMock, Mock, It, Times } from "typemoq"; +import role_to_serverparks_map from "../role-to-serverparks-map.json"; +import { size } from "lodash"; import jwt from "jsonwebtoken"; +import createLogger from "../logger/pinoLogger"; +import pino from "pino"; +import { HttpLogger } from "pino-http"; // Temporary fix for Jest open handle issue (gcp profiler TCPWRAP error) jest.mock("@google-cloud/profiler", () => ({ @@ -28,21 +34,35 @@ const auth = new Auth(config); const mockUser: User = { name: "testUser", role: "DST", defaultServerPark: "park1", serverParks: ["park1", "park2"] }; const mockAuthToken = jwt.sign({ "user": mockUser }, mockSecret); +const logger: pino.Logger = pino(); +logger.child = jest.fn(() => logger); +const logInfo = jest.spyOn(logger, "info"); +const httpLogger: HttpLogger = createLogger({ logger: logger }); + const blaiseApiMock: IMock = Mock.ofType(BlaiseApiClient); -const server = GetNodeServer(config, blaiseApiMock.object, auth); +const server = GetNodeServer(config, blaiseApiMock.object, auth, httpLogger); const sut = supertest(server); -describe("Test Heath Endpoint", () => { +describe("GCP health check", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it("should return a 200 status and json message", async () => { const response = await sut.get("/bam-ui/version/health"); + const log = logInfo.mock.calls[0][0]; + expect(log).toEqual("AUDIT_LOG: Heath Check endpoint called"); expect(response.statusCode).toEqual(200); expect(response.body).toStrictEqual({ healthy: true }); }); }); -describe("app engine start", () => { +describe("GCP app engine start", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); it("should return a 200 status and json message", async () => { process.env = Object.assign({ @@ -58,9 +78,12 @@ describe("app engine start", () => { }); }); -import role_to_serverparks_map from "../role-to-serverparks-map.json"; -import { size } from "lodash"; -describe("Test /api/users POST endpoint", () => { +// Blaise API endpoints +describe("POST /api/users endpoint", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it("should call Blaise API createUser endpoint with correct serverParks for each role EXISTING in server/role-to-serverparks-map.json AND return http status OK_200", async () => { let currentRoleNo = 0; const totalRoleCount = size(role_to_serverparks_map); @@ -132,9 +155,10 @@ describe("Test /api/users POST endpoint", () => { }); }); -describe("Test /api/users DELETE endpoint", () => { +describe("DELETE /api/users endpoint", () => { beforeEach(() => { blaiseApiMock.reset(); + jest.clearAllMocks(); }); afterAll(() => { @@ -164,7 +188,11 @@ describe("Test /api/users DELETE endpoint", () => { }); }); -describe("Test /api/users GET endpoint", () => { +describe("GET /api/users endpoint", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it("should call Blaise API getUsers endpoint AND return http status OK_200", async () => { const newUser1 : NewUser = { name: "name1", @@ -188,7 +216,11 @@ describe("Test /api/users GET endpoint", () => { }); }); -describe("Test /api/roles GET endpoint", () => { +describe("GET /api/roles endpoint", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it("should call Blaise API getUserRoles endpoint AND return http status OK_200", async () => { const userRole1 : UserRole = { name: "name1", @@ -211,9 +243,10 @@ describe("Test /api/roles GET endpoint", () => { }); // TODO: Changing a resource should be a PATCH request not a GET request -describe("Test /api/change-password/:user GET endpoint", () => { +describe("GET /api/change-password/:user endpoint", () => { beforeEach(() => { blaiseApiMock.reset(); + jest.clearAllMocks(); }); afterAll(() => { @@ -265,6 +298,7 @@ describe("Test /api/change-password/:user GET endpoint", () => { describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { beforeEach(() => { blaiseApiMock.reset(); + jest.clearAllMocks(); }); afterAll(() => { @@ -285,6 +319,8 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { .set("Authorization", `${mockAuthToken}`) .send({ role }); + const log = logInfo.mock.calls[0][0]; + expect(log).toEqual("AUDIT_LOG: testUser has successfully updated user role and permissions to IPS Manager for testUser"); expect(response.statusCode).toEqual(200); expect(response.body.message).toContain(`${mockUser.name} has successfully updated user role and permissions to ${role} for ${user}`); blaiseApiMock.verify(api => api.changeUserRole(It.isValue(user), It.isValue(role)), Times.once()); @@ -318,3 +354,38 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { expect(response.body.message).toContain(errorMessage); }); }); + +// AuditLogs endpoints +// TODO: Complete test case +// describe("GET /api/auditLogs endpoint", () => { +// it.todo("should call AuditLogger getAuditLogs endpoint", async () => { +// const logs = [ +// { +// id: "test-id", +// timestamp: "test-timestamp", +// message: "test message", +// severity: "INFO" +// }, +// { +// id: "test-id-2", +// timestamp: "test-timestamp-2", +// message: "test message 2", +// severity: "ERROR" +// }, +// { +// id: "test-id-3", +// timestamp: "test-timestamp-3", +// message: "test message 3", +// severity: "CRITICAL" +// } +// ]; +// auditLoggerMockGetLogs.mockResolvedValue(logs); + +// const response = await sut.get("/api/auditLogs") +// .set("Authorization", `${mockAuthToken}`); + +// expect(response.statusCode).toEqual(200); +// expect(response.body).toStrictEqual(logs); +// auditLoggerMockInfo.mockClear(); +// }); +// }); \ No newline at end of file From df3c82efbdacf8dc7e43c8dff799ca0d62b4d29c Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Thu, 31 Oct 2024 08:53:25 +0000 Subject: [PATCH 16/42] chore: remove unused import --- server/tests/index.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index c3af4ba..2608e88 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -4,7 +4,6 @@ import supertest from "supertest"; import GetNodeServer from "../server"; -import AuditLogger from "../logger/cloudLogging"; import { loadConfigFromEnv } from "../config"; import BlaiseApiClient, { NewUser, User, UserRole } from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; From a4ab55ab5646b2643caaa460c61c114ce50b646d Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Thu, 31 Oct 2024 08:55:33 +0000 Subject: [PATCH 17/42] chore: remove unused import --- server/index.ts | 1 - server/server.ts | 2 -- 2 files changed, 3 deletions(-) diff --git a/server/index.ts b/server/index.ts index 56ba7f5..45d4a08 100644 --- a/server/index.ts +++ b/server/index.ts @@ -4,7 +4,6 @@ import BlaiseApiClient from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import dotenv from "dotenv"; import createLogger from "./logger/pinoLogger"; -import AuditLogger from "./logger/cloudLogging"; const port: string = process.env.PORT || "5002"; diff --git a/server/server.ts b/server/server.ts index 615dedf..4be59c3 100644 --- a/server/server.ts +++ b/server/server.ts @@ -2,7 +2,6 @@ import express, { Request, Response } from "express"; import axios from "axios"; import path from "path"; import ejs from "ejs"; -import createLogger from "./logger/pinoLogger"; import multer from "multer"; import * as profiler from "@google-cloud/profiler"; import { newLoginHandler, Auth } from "blaise-login-react/blaise-login-react-server"; @@ -13,7 +12,6 @@ import fs from "fs"; import AuditLogger from "./logger/cloudLogging"; import auditLogs from "./routes/auditLogs"; import blaiseApi from "./routes/blaiseApi"; -import { Logger } from "pino"; import { HttpLogger } from "pino-http"; export default function GetNodeServer(config: CustomConfig, blaiseApiClient: BlaiseApi, auth: Auth, logger: HttpLogger): Express From b376eb85538d6683766680f5858fac2fb6643b2b Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Thu, 31 Oct 2024 09:02:16 +0000 Subject: [PATCH 18/42] wip: fix unit test? --- server/tests/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index 2608e88..afbbb83 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -4,7 +4,6 @@ import supertest from "supertest"; import GetNodeServer from "../server"; -import { loadConfigFromEnv } from "../config"; import BlaiseApiClient, { NewUser, User, UserRole } from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import { IMock, Mock, It, Times } from "typemoq"; @@ -14,6 +13,7 @@ import jwt from "jsonwebtoken"; import createLogger from "../logger/pinoLogger"; import pino from "pino"; import { HttpLogger } from "pino-http"; +import { loadConfigFromEnv } from "../config"; // Temporary fix for Jest open handle issue (gcp profiler TCPWRAP error) jest.mock("@google-cloud/profiler", () => ({ From 7f12c989fc504eb23bb82e8a701d1f8026af105d Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Thu, 31 Oct 2024 09:04:59 +0000 Subject: [PATCH 19/42] wip: fix unit test? --- server/tests/config.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 587b851..2963c05 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -2,7 +2,7 @@ * @jest-environment node */ -import { loadConfigFromEnv } from "../Config"; +import { loadConfigFromEnv } from "../config"; describe("Config setup", () => { afterEach(() => { From 6ecd43cc654b6e3b3a4c9ed8ff7049ae4b7ffb1b Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Thu, 31 Oct 2024 09:10:20 +0000 Subject: [PATCH 20/42] wip: fix unit test? --- server/index.ts | 2 +- server/tests/config.test.ts | 2 +- server/tests/index.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/index.ts b/server/index.ts index 45d4a08..e591782 100644 --- a/server/index.ts +++ b/server/index.ts @@ -1,5 +1,5 @@ import GetNodeServer from "./server"; -import { loadConfigFromEnv } from "./config"; +import { loadConfigFromEnv } from "./Config"; import BlaiseApiClient from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; import dotenv from "dotenv"; diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 2963c05..587b851 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -2,7 +2,7 @@ * @jest-environment node */ -import { loadConfigFromEnv } from "../config"; +import { loadConfigFromEnv } from "../Config"; describe("Config setup", () => { afterEach(() => { diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index afbbb83..4f15951 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -13,7 +13,7 @@ import jwt from "jsonwebtoken"; import createLogger from "../logger/pinoLogger"; import pino from "pino"; import { HttpLogger } from "pino-http"; -import { loadConfigFromEnv } from "../config"; +import { loadConfigFromEnv } from "../Config"; // Temporary fix for Jest open handle issue (gcp profiler TCPWRAP error) jest.mock("@google-cloud/profiler", () => ({ From 525b9ccee787174519d6faf3dac73e96b8d06d7c Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:09:36 +0000 Subject: [PATCH 21/42] wip: fix deploy? --- package.json | 2 +- server/logger/cloudLogging.ts | 1 + server/tests/index.test.ts | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 72a51fc..76add15 100644 --- a/package.json +++ b/package.json @@ -42,6 +42,7 @@ "formik": "2.4.2", "history": "^4.9.0", "jest-cucumber": "^3.0.0", + "jsonwebtoken": "^9.0.2", "lodash": "^4.17.21", "multer": "^1.4.2", "number-to-words": "^1.2.4", @@ -84,7 +85,6 @@ "eslint-plugin-prettier": "^5.1.3", "eslint-plugin-react": "^7.28.0", "eslint-plugin-react-hooks": "^4.3.0", - "jsonwebtoken": "^9.0.2", "pino-pretty": "^4.7.1", "supertest": "^6.1.6" }, diff --git a/server/logger/cloudLogging.ts b/server/logger/cloudLogging.ts index 389f35d..32a8af4 100644 --- a/server/logger/cloudLogging.ts +++ b/server/logger/cloudLogging.ts @@ -1,3 +1,4 @@ +// eslint-disable-next-line import/no-extraneous-dependencies import { Logging } from "@google-cloud/logging"; import { IncomingMessage } from "http"; import { AuditLog } from "../interfaces/logger"; diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index 4f15951..7a536d7 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -9,6 +9,7 @@ import { Auth } from "blaise-login-react/blaise-login-react-server"; import { IMock, Mock, It, Times } from "typemoq"; import role_to_serverparks_map from "../role-to-serverparks-map.json"; import { size } from "lodash"; +// eslint-disable-next-line import/no-extraneous-dependencies import jwt from "jsonwebtoken"; import createLogger from "../logger/pinoLogger"; import pino from "pino"; From 0206e7c00be5de5407d0f4321532e6cb354054d5 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:34:49 +0000 Subject: [PATCH 22/42] wip: checking logs for roles and permissions --- server/routes/blaiseApi.ts | 4 ++-- server/tests/index.test.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index 61c5e48..d26625b 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -39,14 +39,14 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli try { await blaiseApiClient.changeUserRole(user, role); await blaiseApiClient.changeUserServerParks(user, newServerParks, newDefaultServerPark); - const successMessage = `${currentUser.name || "Unknown"} has successfully updated user role and permissions to ${role} for ${user}`; + const successMessage = `${currentUser.name || "Unknown user"} has successfully updated user role and permissions to ${role} for ${user}`; auditLogger.info(req.log, successMessage); return res.status(200).json({ message: successMessage }); } catch (error) { const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}: ${error}`; - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has failed to update user role and permissions to ${role} for ${user}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown user"} has failed to update user role and permissions to ${role} for ${user}`); auditLogger.error(req.log, errorMessage); return res.status(500).json({ message: errorMessage diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index 7a536d7..4ce7bbe 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -12,7 +12,7 @@ import { size } from "lodash"; // eslint-disable-next-line import/no-extraneous-dependencies import jwt from "jsonwebtoken"; import createLogger from "../logger/pinoLogger"; -import pino from "pino"; +import pino, { Bindings, ChildLoggerOptions } from "pino"; import { HttpLogger } from "pino-http"; import { loadConfigFromEnv } from "../Config"; @@ -34,6 +34,7 @@ const auth = new Auth(config); const mockUser: User = { name: "testUser", role: "DST", defaultServerPark: "park1", serverParks: ["park1", "park2"] }; const mockAuthToken = jwt.sign({ "user": mockUser }, mockSecret); +// TODO: Investigate const logger: pino.Logger = pino(); logger.child = jest.fn(() => logger); const logInfo = jest.spyOn(logger, "info"); From 98180f51a996ac488c8ad2f21fbe22482ead6d87 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 09:51:06 +0000 Subject: [PATCH 23/42] chore: remove audit logs endpoint --- server/routes/auditLogs.ts | 34 ------------------ server/routes/blaiseApi.ts | 71 +++++++++++++++++++++----------------- server/server.ts | 3 -- 3 files changed, 40 insertions(+), 68 deletions(-) delete mode 100644 server/routes/auditLogs.ts diff --git a/server/routes/auditLogs.ts b/server/routes/auditLogs.ts deleted file mode 100644 index d91c770..0000000 --- a/server/routes/auditLogs.ts +++ /dev/null @@ -1,34 +0,0 @@ -import express, { Request, Response, Router } from "express"; -import AuditLogger from "../logger/cloudLogging"; -import { Auth } from "blaise-login-react/blaise-login-react-server"; - -export default function auditLogs(auditlogger: AuditLogger, auth: Auth): Router { - const router = express.Router(); - - const auditHandler = new AuditHandler(auditlogger, auth); - return router.get("/api/audit", auth.Middleware, auditHandler.GetAuditLogs); -} - -export class AuditHandler { - auditLogger: AuditLogger; - auth: Auth; - - constructor(auditLogger: AuditLogger, auth: Auth) { - this.auditLogger = auditLogger; - this.auth = auth; - - this.GetAuditLogs = this.GetAuditLogs.bind(this); - } - - async GetAuditLogs(req: Request, res: Response): Promise { - const currentUser = this.auth.GetUser(this.auth.GetToken(req)); - try { - const logs = await this.auditLogger.getLogs(); - this.auditLogger.info(req.log, `${currentUser.name} retrieved audit logs`); - return res.status(200).json(logs); - } catch (error: unknown) { - this.auditLogger.error(req.log, `${currentUser.name} failed to get audit logs`); - return res.status(500).json(error); - } - } -} diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index d26625b..11df9d8 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -62,14 +62,12 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli try { const user = await blaiseApiClient.getUser(req.params.user); const successMessage = `Successfully fetched user details for ${req.params.user}`; - auditLogger.info(req.log, successMessage); return res.status(200).json({ message: successMessage, data: user }); } catch (error) { const errorMessage = `Error whilst trying to retrieve user ${req.params.user}: ${error}`; - auditLogger.error(req.log, errorMessage); return res.status(500).json({ message: errorMessage, error: error @@ -100,40 +98,51 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli }); router.delete("/api/users", auth.Middleware, async function (req: Request, res: Response) { - const currentUser = auth.GetUser(auth.GetToken(req)); - let { user } = req.headers; - - if (Array.isArray(user)) { - user = user.join(""); - } - - if (!user) { - auditLogger.error(req.log, "No user provided for deletion"); - return res.status(400).json(); + try { + const currentUser = auth.GetUser(auth.GetToken(req)); + let { user } = req.headers; + + if (Array.isArray(user)) { + user = user.join(""); + } + + if (!user) { + auditLogger.error(req.log, "No user provided for deletion"); + return res.status(400).json(); + } + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user: ${user}`); + return res.status(204).json(await blaiseApiClient.deleteUser(user)); + } catch (error) { + auditLogger.error(req.log, `Error whilst trying to delete user: ${error}`); + return res.status(500).json(error); } - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user: ${user}`); - return res.status(204).json(await blaiseApiClient.deleteUser(user)); }); router.post("/api/users", auth.Middleware, async function (req: Request, res: Response) { - const currentUser = auth.GetUser(auth.GetToken(req)); - const data = req.body; - - if(!data.role) { - return res.status(400).json({ message: "No role provided for user creation" }); - } - - const roleServerParksOverride = config.RoleToServerParksMap[data.role]; - if (roleServerParksOverride != null) { - data.serverParks = roleServerParksOverride; - data.defaultServerPark = roleServerParksOverride[0]; - } else { - const defaultServerPark = config.RoleToServerParksMap["DEFAULT"]; - data.serverParks = defaultServerPark; - data.defaultServerPark = defaultServerPark[0]; + try { + const currentUser = auth.GetUser(auth.GetToken(req)); + const data = req.body; + + if(!data.role) { + return res.status(400).json({ message: "No role provided for user creation" }); + } + + const roleServerParksOverride = config.RoleToServerParksMap[data.role]; + if (roleServerParksOverride != null) { + data.serverParks = roleServerParksOverride; + data.defaultServerPark = roleServerParksOverride[0]; + } else { + const defaultServerPark = config.RoleToServerParksMap["DEFAULT"]; + data.serverParks = defaultServerPark; + data.defaultServerPark = defaultServerPark[0]; + } + console.log(data); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user: ${data.name}`); + return res.status(200).json(await blaiseApiClient.createUser(data)); + } catch (error) { + auditLogger.error(req.log, `Error whilst trying to create user: ${error}`); + return res.status(500).json(error); } - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user: ${data.username}`); - return res.status(200).json(await blaiseApiClient.createUser(data)); }); return router; diff --git a/server/server.ts b/server/server.ts index 4be59c3..a82ef9d 100644 --- a/server/server.ts +++ b/server/server.ts @@ -10,7 +10,6 @@ import BlaiseApi from "blaise-api-node-client"; import { Express } from "express"; import fs from "fs"; import AuditLogger from "./logger/cloudLogging"; -import auditLogs from "./routes/auditLogs"; import blaiseApi from "./routes/blaiseApi"; import { HttpLogger } from "pino-http"; @@ -28,7 +27,6 @@ export default function GetNodeServer(config: CustomConfig, blaiseApiClient: Bla axios.defaults.timeout = 10000; const loginRouter = newLoginHandler(auth, blaiseApiClient); - const auditRouter = auditLogs(auditLogger, auth); const blaiseApiRouter = blaiseApi(config, auth, blaiseApiClient, auditLogger); profiler.start({ logLevel: 4 }).catch((err: unknown) => { @@ -41,7 +39,6 @@ export default function GetNodeServer(config: CustomConfig, blaiseApiClient: Bla }); server.use("/", loginRouter); server.use("/", blaiseApiRouter); - server.use("/", auditRouter); // treat the index.html as a template and substitute the values at runtime server.set("views", path.join(__dirname, "/views")); From a9d64b05cc3f770bba053abab09869dee1b9c9d9 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 09:51:55 +0000 Subject: [PATCH 24/42] test: relocate blaise api tests --- server/tests/index.test.ts | 330 +---------------------- server/tests/routes/blaiseApi.test.ts | 362 ++++++++++++++++++++++++++ 2 files changed, 368 insertions(+), 324 deletions(-) create mode 100644 server/tests/routes/blaiseApi.test.ts diff --git a/server/tests/index.test.ts b/server/tests/index.test.ts index 4ce7bbe..169dc61 100644 --- a/server/tests/index.test.ts +++ b/server/tests/index.test.ts @@ -4,17 +4,14 @@ import supertest from "supertest"; import GetNodeServer from "../server"; -import BlaiseApiClient, { NewUser, User, UserRole } from "blaise-api-node-client"; +import BlaiseApiClient from "blaise-api-node-client"; import { Auth } from "blaise-login-react/blaise-login-react-server"; -import { IMock, Mock, It, Times } from "typemoq"; -import role_to_serverparks_map from "../role-to-serverparks-map.json"; -import { size } from "lodash"; -// eslint-disable-next-line import/no-extraneous-dependencies -import jwt from "jsonwebtoken"; -import createLogger from "../logger/pinoLogger"; -import pino, { Bindings, ChildLoggerOptions } from "pino"; +import { IMock, Mock } from "typemoq"; +import pino from "pino"; import { HttpLogger } from "pino-http"; import { loadConfigFromEnv } from "../Config"; +// eslint-disable-next-line import/no-extraneous-dependencies +import createLogger from "../logger/pinoLogger"; // Temporary fix for Jest open handle issue (gcp profiler TCPWRAP error) jest.mock("@google-cloud/profiler", () => ({ @@ -31,10 +28,7 @@ process.env = Object.assign(process.env, { }); const config = loadConfigFromEnv(); const auth = new Auth(config); -const mockUser: User = { name: "testUser", role: "DST", defaultServerPark: "park1", serverParks: ["park1", "park2"] }; -const mockAuthToken = jwt.sign({ "user": mockUser }, mockSecret); -// TODO: Investigate const logger: pino.Logger = pino(); logger.child = jest.fn(() => logger); const logInfo = jest.spyOn(logger, "info"); @@ -77,316 +71,4 @@ describe("GCP app engine start", () => { expect(response.statusCode).toEqual(200); }); -}); - -// Blaise API endpoints -describe("POST /api/users endpoint", () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it("should call Blaise API createUser endpoint with correct serverParks for each role EXISTING in server/role-to-serverparks-map.json AND return http status OK_200", async () => { - let currentRoleNo = 0; - const totalRoleCount = size(role_to_serverparks_map); - for(const roleName in role_to_serverparks_map) - { - blaiseApiMock.reset(); - console.log("Running for role %i of %i: %s", ++currentRoleNo, totalRoleCount, roleName); - - const spmap = role_to_serverparks_map[roleName]; - const newUser : NewUser = { - name: "name1", - password: "password1", - role: roleName, - serverParks: spmap, - defaultServerPark: spmap[0] - }; - blaiseApiMock.setup((api) => api.createUser(It.isAny())).returns(async () => newUser); - - const response = await sut.post("/api/users") - .set("Authorization", `${mockAuthToken}`) - .field("role", roleName); - - expect(response.statusCode).toEqual(200); - blaiseApiMock.verify(a => a.createUser(It.is( - x=> x.defaultServerPark == newUser.defaultServerPark - && x.role == newUser.role - && Array.isArray(x.serverParks) && x.serverParks.every(item => typeof item === "string") - && x.serverParks.every((val, idx) => val === newUser.serverParks[idx]) - )), Times.exactly(1)); - expect(response.body).toStrictEqual(newUser); - } - }); - - it("should call Blaise API createUser endpoint with DEFAULT serverParks for a role MISSING in server/role-to-serverparks-map.json AND return http status OK_200)", async () => { - const roleName = "this role is missing in server/role-to-serverparks-map.json file"; - const spmap = role_to_serverparks_map.DEFAULT; - const newUser : NewUser = { - name: "name1", - password: "password1", - role: roleName, - serverParks: spmap, - defaultServerPark: spmap[0] - }; - blaiseApiMock.setup((api) => api.createUser(It.isAny())).returns(async () => newUser); - - const response = await sut.post("/api/users") - .field("role", roleName) - .set("Authorization", `${mockAuthToken}`); - - expect(response.statusCode).toEqual(200); - blaiseApiMock.verify(a => a.createUser(It.is( - x=> x.defaultServerPark == newUser.defaultServerPark - && x.role == newUser.role - && Array.isArray(x.serverParks) && x.serverParks.every(item => typeof item === "string") - && x.serverParks.every((val, idx) => val === newUser.serverParks[idx]) - )), Times.exactly(1)); - expect(response.body).toStrictEqual(newUser); - }); - - it("should return http status BAD_REQUEST_400 if role is empty OR hasn't been specified in the request", async () => { - let response = await sut.post("/api/users") - .field("role", "") - .set("Authorization", `${mockAuthToken}`); - expect(response.statusCode).toEqual(400); - - response = await sut.post("/api/users") - .set("Authorization", `${mockAuthToken}`); - expect(response.statusCode).toEqual(400); - }); -}); - -describe("DELETE /api/users endpoint", () => { - beforeEach(() => { - blaiseApiMock.reset(); - jest.clearAllMocks(); - }); - - afterAll(() => { - blaiseApiMock.reset(); - }); - - it("should call Blaise API deleteUser endpoint for VALID user header field AND return http status NO_CONTENT_204", async () => { - blaiseApiMock.setup((api) => api.deleteUser(It.isAny())).returns(_ => Promise.resolve(null)); - const username = "user-123"; - const response = await sut.delete("/api/users") - .set("Authorization", `${mockAuthToken}`) - .set("user", username); - - expect(response.statusCode).toEqual(204); - blaiseApiMock.verify(a => a.deleteUser(It.isValue(username)), Times.once()); - }); - - it("should call Blaise API deleteUser endpoint for INVALID or MISSING user header field AND return http status BAD_REQUEST_400", async () => { - let response = await sut.delete("/api/users") - .set("Authorization", `${mockAuthToken}`) - .set("user", ""); - expect(response.statusCode).toEqual(400); - - response = await sut.delete("/api/users") - .set("Authorization", `${mockAuthToken}`); - expect(response.statusCode).toEqual(400); - }); -}); - -describe("GET /api/users endpoint", () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it("should call Blaise API getUsers endpoint AND return http status OK_200", async () => { - const newUser1 : NewUser = { - name: "name1", - password: "password1", - role: "role1", - serverParks: ["sp1", "sp2"], - defaultServerPark: "sp1" - }; - const newUser2 = newUser1; - newUser2.name = "name2"; - const newUser3 = newUser2; - newUser3.name = "name3"; - const userArray : NewUser [] = [newUser1, newUser2, newUser3]; - blaiseApiMock.setup((api) => api.getUsers()).returns(_ => Promise.resolve(userArray)); - - const response = await sut.get("/api/users") - .set("Authorization", `${mockAuthToken}`); - - expect(response.statusCode).toEqual(200); - blaiseApiMock.verify(a => a.getUsers(), Times.once()); - }); -}); - -describe("GET /api/roles endpoint", () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it("should call Blaise API getUserRoles endpoint AND return http status OK_200", async () => { - const userRole1 : UserRole = { - name: "name1", - description: "desc1", - permissions: ["perm1", "perm2"] - }; - const userRole2 = userRole1; - userRole2.name = "name2"; - const userRole3 = userRole2; - userRole3.name = "name3"; - const userRoleArray : UserRole [] = [userRole1, userRole2, userRole3]; - blaiseApiMock.setup((api) => api.getUserRoles()).returns(_ => Promise.resolve(userRoleArray)); - - const response = await sut.get("/api/roles") - .set("Authorization", `${mockAuthToken}`); - - expect(response.statusCode).toEqual(200); - blaiseApiMock.verify(a => a.getUserRoles(), Times.once()); - }); -}); - -// TODO: Changing a resource should be a PATCH request not a GET request -describe("GET /api/change-password/:user endpoint", () => { - beforeEach(() => { - blaiseApiMock.reset(); - jest.clearAllMocks(); - }); - - afterAll(() => { - blaiseApiMock.reset(); - }); - - it("should call Blaise API changePassword endpoint for VALID request AND return http status NO_CONTENT_204", async () => { - const username = "user1"; - const password = "password-1234"; - blaiseApiMock.setup((api) => api.changePassword(It.isAnyString(), It.isAnyString())).returns(_ => Promise.resolve(null)); - - const response = await sut.get("/api/change-password/"+username) - .set("Authorization", `${mockAuthToken}`) - .set("password", password); - - expect(response.statusCode).toEqual(204); - blaiseApiMock.verify(a => a.changePassword(It.isValue(username), It.isValue(password)), Times.once()); - }); - - it("should NOT call Blaise API changePassword endpoint for INVALID request AND return http status BAD_REQUEST_400", async () => { - const username = "user1"; - const password = ""; - - const response = await sut.get("/api/change-password/"+ username) - .set("Authorization", `${mockAuthToken}`) - .set("password", password); - - expect(response.statusCode).toEqual(400); - blaiseApiMock.verify(a => a.changePassword(It.isAnyString(), It.isAnyString()), Times.never()); - }); - - it("should return error message if external Blaise API changePassword endpoint rejects request with error message AND should return http status INTERNAL_SERVER_ERROR_500", async () => { - const username = "user1"; - const password = "password-1234"; - const errorMessage = "Error occured when calling changePassword on Blaise API Rest Service"; - blaiseApiMock.setup((a) => a.changePassword(It.isAnyString(), It.isAnyString())) - .returns(_ => Promise.reject(errorMessage)); - - const response = await sut.get("/api/change-password/"+ username) - .set("Authorization", `${mockAuthToken}`) - .set("password", password); - - expect(response.statusCode).toEqual(500); - blaiseApiMock.verify(a => a.changePassword(It.isAnyString(), It.isAnyString()), Times.once()); - expect(response.body).toStrictEqual(errorMessage); - }); -}); - -describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { - beforeEach(() => { - blaiseApiMock.reset(); - jest.clearAllMocks(); - }); - - afterAll(() => { - blaiseApiMock.reset(); - }); - - it("should update user role and permissions successfully and return http status 200", async () => { - const user = "testUser"; - const role = "IPS Manager"; - const serverParks = ["gusty", "cma"]; - const defaultServerPark = "gusty"; - blaiseApiMock.setup(api => api.changeUserRole(It.isValue(user), It.isValue(role))) - .returns(async () => null); - blaiseApiMock.setup(api => api.changeUserServerParks(It.isValue(user), It.isValue(serverParks), It.isValue(defaultServerPark))) - .returns(async () => null); - - const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) - .set("Authorization", `${mockAuthToken}`) - .send({ role }); - - const log = logInfo.mock.calls[0][0]; - expect(log).toEqual("AUDIT_LOG: testUser has successfully updated user role and permissions to IPS Manager for testUser"); - expect(response.statusCode).toEqual(200); - expect(response.body.message).toContain(`${mockUser.name} has successfully updated user role and permissions to ${role} for ${user}`); - blaiseApiMock.verify(api => api.changeUserRole(It.isValue(user), It.isValue(role)), Times.once()); - blaiseApiMock.verify(api => api.changeUserServerParks(It.isValue(user), It.isValue(serverParks), It.isValue(defaultServerPark)), Times.once()); - }); - - it("should return http status BAD_REQUEST_400 if role or user is not provided", async () => { - const user = "testUser"; - const role = ""; - - const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) - .set("Authorization", `${mockAuthToken}`) - .send({ role }); - - expect(response.statusCode).toEqual(400); - expect(response.body).toEqual("No user or role provided"); - }); - - it("should return http status INTERNAL_SERVER_ERROR_500 if Blaise API client throws an error", async () => { - const user = "testUser"; - const role = "admin"; - const errorMessage = "Blaise API client error"; - blaiseApiMock.setup(api => api.changeUserRole(It.isAny(), It.isAny())) - .returns(async () => { throw new Error(errorMessage); }); - - const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) - .set("Authorization", `${mockAuthToken}`) - .send({ role }); - - expect(response.statusCode).toEqual(500); - expect(response.body.message).toContain(errorMessage); - }); -}); - -// AuditLogs endpoints -// TODO: Complete test case -// describe("GET /api/auditLogs endpoint", () => { -// it.todo("should call AuditLogger getAuditLogs endpoint", async () => { -// const logs = [ -// { -// id: "test-id", -// timestamp: "test-timestamp", -// message: "test message", -// severity: "INFO" -// }, -// { -// id: "test-id-2", -// timestamp: "test-timestamp-2", -// message: "test message 2", -// severity: "ERROR" -// }, -// { -// id: "test-id-3", -// timestamp: "test-timestamp-3", -// message: "test message 3", -// severity: "CRITICAL" -// } -// ]; -// auditLoggerMockGetLogs.mockResolvedValue(logs); - -// const response = await sut.get("/api/auditLogs") -// .set("Authorization", `${mockAuthToken}`); - -// expect(response.statusCode).toEqual(200); -// expect(response.body).toStrictEqual(logs); -// auditLoggerMockInfo.mockClear(); -// }); -// }); \ No newline at end of file +}); \ No newline at end of file diff --git a/server/tests/routes/blaiseApi.test.ts b/server/tests/routes/blaiseApi.test.ts new file mode 100644 index 0000000..14709a5 --- /dev/null +++ b/server/tests/routes/blaiseApi.test.ts @@ -0,0 +1,362 @@ +/** + * @jest-environment node + */ + +import supertest from "supertest"; +import GetNodeServer from "../../server"; +import BlaiseApiClient, { NewUser, User, UserRole } from "blaise-api-node-client"; +import { Auth } from "blaise-login-react/blaise-login-react-server"; +import { IMock, Mock, It, Times } from "typemoq"; +import role_to_serverparks_map from "../../role-to-serverparks-map.json"; +import { size } from "lodash"; +import createLogger from "../../logger/pinoLogger"; +import pino from "pino"; +import { HttpLogger } from "pino-http"; +import { loadConfigFromEnv } from "../../Config"; +// eslint-disable-next-line import/no-extraneous-dependencies +import jwt from "jsonwebtoken"; + +// Temporary fix for Jest open handle issue (gcp profiler TCPWRAP error) +jest.mock("@google-cloud/profiler", () => ({ + start: jest.fn().mockReturnValue(Promise.resolve()) +})); + +const mockSecret = "super-secret-key"; +process.env = Object.assign(process.env, { + SESSION_SECRET: mockSecret, + PROJECT_ID: "mockProjectId", + BLAISE_API_URL: "http://blaise-api", + SERVER_PARK: "mockServerPark", + SESSION_TIMEOUT: "12h" +}); +const config = loadConfigFromEnv(); +const auth = new Auth(config); +const mockUser: User = { name: "testUser", role: "DST", defaultServerPark: "park1", serverParks: ["park1", "park2"] }; +const mockAuthToken = jwt.sign({ "user": mockUser }, mockSecret); + +const logger: pino.Logger = pino(); +logger.child = jest.fn(() => logger); +const logInfo = jest.spyOn(logger, "info"); +const logError = jest.spyOn(logger, "error"); +const httpLogger: HttpLogger = createLogger({ logger: logger }); + +const blaiseApiMock: IMock = Mock.ofType(BlaiseApiClient); + +const server = GetNodeServer(config, blaiseApiMock.object, auth, httpLogger); +const sut = supertest(server); + +// Blaise API endpoints +describe("POST /api/users endpoint", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should call Blaise API createUser endpoint with correct serverParks for each role EXISTING in server/role-to-serverparks-map.json AND return http status OK_200", async () => { + let currentRoleNo = 0; + const totalRoleCount = size(role_to_serverparks_map); + for(const roleName in role_to_serverparks_map) + { + blaiseApiMock.reset(); + console.log("Running for role %i of %i: %s", ++currentRoleNo, totalRoleCount, roleName); + + const spmap = role_to_serverparks_map[roleName]; + const newUser : NewUser = { + name: "name1", + password: "password1", + role: roleName, + serverParks: spmap, + defaultServerPark: spmap[0] + }; + blaiseApiMock.setup((api) => api.createUser(It.isAny())).returns(async () => newUser); + + const response = await sut.post("/api/users") + .set("Authorization", `${mockAuthToken}`) + .field("name", newUser.name) + .field("role", roleName); + + const log = logInfo.mock.calls[0][0]; + expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully created user: ${newUser.name}`); + expect(response.statusCode).toEqual(200); + blaiseApiMock.verify(a => a.createUser(It.is( + x=> x.defaultServerPark == newUser.defaultServerPark + && x.role == newUser.role + && Array.isArray(x.serverParks) && x.serverParks.every(item => typeof item === "string") + && x.serverParks.every((val, idx) => val === newUser.serverParks[idx]) + )), Times.exactly(1)); + expect(response.body).toStrictEqual(newUser); + } + }); + + it("should call Blaise API createUser endpoint with DEFAULT serverParks for a role MISSING in server/role-to-serverparks-map.json AND return http status OK_200)", async () => { + const roleName = "this role is missing in server/role-to-serverparks-map.json file"; + const spmap = role_to_serverparks_map.DEFAULT; + const newUser : NewUser = { + name: "name1", + password: "password1", + role: roleName, + serverParks: spmap, + defaultServerPark: spmap[0] + }; + blaiseApiMock.setup((api) => api.createUser(It.isAny())).returns(async () => newUser); + + const response = await sut.post("/api/users") + .field("name", newUser.name) + .field("role", roleName) + .set("Authorization", `${mockAuthToken}`); + + const log = logInfo.mock.calls[0][0]; + expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully created user: ${newUser.name}`); + expect(response.statusCode).toEqual(200); + blaiseApiMock.verify(a => a.createUser(It.is( + x=> x.defaultServerPark == newUser.defaultServerPark + && x.role == newUser.role + && Array.isArray(x.serverParks) && x.serverParks.every(item => typeof item === "string") + && x.serverParks.every((val, idx) => val === newUser.serverParks[idx]) + )), Times.exactly(1)); + expect(response.body).toStrictEqual(newUser); + }); + + it("should return http status BAD_REQUEST_400 if role is empty OR hasn't been specified in the request", async () => { + let response = await sut.post("/api/users") + .field("role", "") + .set("Authorization", `${mockAuthToken}`); + + expect(response.statusCode).toEqual(400); + expect(response.body.message).toEqual("No role provided for user creation"); + + response = await sut.post("/api/users") + .set("Authorization", `${mockAuthToken}`); + expect(response.statusCode).toEqual(400); + expect(response.body.message).toEqual("No role provided for user creation"); + }); +}); + +describe("DELETE /api/users endpoint", () => { + beforeEach(() => { + blaiseApiMock.reset(); + jest.clearAllMocks(); + }); + + afterAll(() => { + blaiseApiMock.reset(); + }); + + it("should call Blaise API deleteUser endpoint for VALID user header field AND return http status NO_CONTENT_204", async () => { + blaiseApiMock.setup((api) => api.deleteUser(It.isAny())).returns(_ => Promise.resolve(null)); + const username = "user-123"; + const response = await sut.delete("/api/users") + .set("Authorization", `${mockAuthToken}`) + .set("user", username); + + const log = logInfo.mock.calls[0][0]; + expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully deleted user: ${username}`); + expect(response.statusCode).toEqual(204); + blaiseApiMock.verify(a => a.deleteUser(It.isValue(username)), Times.once()); + }); + + it("should return error message if external Blaise API deleteUser endpoint rejects request with error message AND should return http status INTERNAL_SERVER_ERROR_500", async () => { + const username = "user-123"; + const errorMessage = "Error occured when calling deleteUser on Blaise API Rest Service"; + blaiseApiMock.setup((a) => a.deleteUser(It.isAnyString())) + .returns(_ => Promise.reject(errorMessage)); + + const response = await sut.delete("/api/users") + .set("Authorization", `${mockAuthToken}`) + .set("user", username); + + const log = logError.mock.calls[0][0]; + expect(log).toEqual(`AUDIT_LOG: Error whilst trying to delete user: ${errorMessage}`); + expect(response.statusCode).toEqual(500); + blaiseApiMock.verify(a => a.deleteUser(It.isValue(username)), Times.once()); + expect(response.body).toStrictEqual(errorMessage); + }); + + it("should call Blaise API deleteUser endpoint for INVALID or MISSING user header field AND return http status BAD_REQUEST_400", async () => { + let response = await sut.delete("/api/users") + .set("Authorization", `${mockAuthToken}`) + .set("user", ""); + + const log = logError.mock.calls[0][0]; + expect(log).toEqual("AUDIT_LOG: No user provided for deletion"); + expect(response.statusCode).toEqual(400); + + response = await sut.delete("/api/users") + .set("Authorization", `${mockAuthToken}`); + expect(response.statusCode).toEqual(400); + }); +}); + +describe("GET /api/users endpoint", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should call Blaise API getUsers endpoint AND return http status OK_200", async () => { + const newUser1 : NewUser = { + name: "name1", + password: "password1", + role: "role1", + serverParks: ["sp1", "sp2"], + defaultServerPark: "sp1" + }; + const newUser2 = newUser1; + newUser2.name = "name2"; + const newUser3 = newUser2; + newUser3.name = "name3"; + const userArray : NewUser [] = [newUser1, newUser2, newUser3]; + blaiseApiMock.setup((api) => api.getUsers()).returns(_ => Promise.resolve(userArray)); + + const response = await sut.get("/api/users") + .set("Authorization", `${mockAuthToken}`); + + expect(response.statusCode).toEqual(200); + blaiseApiMock.verify(a => a.getUsers(), Times.once()); + }); +}); + +describe("GET /api/roles endpoint", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should call Blaise API getUserRoles endpoint AND return http status OK_200", async () => { + const userRole1 : UserRole = { + name: "name1", + description: "desc1", + permissions: ["perm1", "perm2"] + }; + const userRole2 = userRole1; + userRole2.name = "name2"; + const userRole3 = userRole2; + userRole3.name = "name3"; + const userRoleArray : UserRole [] = [userRole1, userRole2, userRole3]; + blaiseApiMock.setup((api) => api.getUserRoles()).returns(_ => Promise.resolve(userRoleArray)); + + const response = await sut.get("/api/roles") + .set("Authorization", `${mockAuthToken}`); + + expect(response.statusCode).toEqual(200); + blaiseApiMock.verify(a => a.getUserRoles(), Times.once()); + }); +}); + +// TODO: Changing a resource should be a PATCH request not a GET request +describe("GET /api/change-password/:user endpoint", () => { + beforeEach(() => { + blaiseApiMock.reset(); + jest.clearAllMocks(); + }); + + afterAll(() => { + blaiseApiMock.reset(); + }); + + it("should call Blaise API changePassword endpoint for VALID request AND return http status NO_CONTENT_204", async () => { + const username = "user1"; + const password = "password-1234"; + blaiseApiMock.setup((api) => api.changePassword(It.isAnyString(), It.isAnyString())).returns(_ => Promise.resolve(null)); + + const response = await sut.get("/api/change-password/"+username) + .set("Authorization", `${mockAuthToken}`) + .set("password", password); + + const log = logInfo.mock.calls[0][0]; + expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully changed the password for ${username}`); + expect(response.statusCode).toEqual(204); + blaiseApiMock.verify(a => a.changePassword(It.isValue(username), It.isValue(password)), Times.once()); + }); + + it("should NOT call Blaise API changePassword endpoint for INVALID request AND return http status BAD_REQUEST_400", async () => { + const username = "user1"; + const password = ""; + + const response = await sut.get("/api/change-password/"+ username) + .set("Authorization", `${mockAuthToken}`) + .set("password", password); + + expect(response.statusCode).toEqual(400); + blaiseApiMock.verify(a => a.changePassword(It.isAnyString(), It.isAnyString()), Times.never()); + }); + + it("should return error message if external Blaise API changePassword endpoint rejects request with error message AND should return http status INTERNAL_SERVER_ERROR_500", async () => { + const username = "user1"; + const password = "password-1234"; + const errorMessage = "Error occured when calling changePassword on Blaise API Rest Service"; + blaiseApiMock.setup((a) => a.changePassword(It.isAnyString(), It.isAnyString())) + .returns(_ => Promise.reject(errorMessage)); + + const response = await sut.get("/api/change-password/"+ username) + .set("Authorization", `${mockAuthToken}`) + .set("password", password); + + const log = logError.mock.calls[0][0]; + expect(log).toEqual(`AUDIT_LOG: Error whilst trying to change password for ${username}: ${errorMessage}`); + expect(response.statusCode).toEqual(500); + blaiseApiMock.verify(a => a.changePassword(It.isAnyString(), It.isAnyString()), Times.once()); + expect(response.body).toStrictEqual(errorMessage); + }); +}); + +describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { + beforeEach(() => { + blaiseApiMock.reset(); + jest.clearAllMocks(); + }); + + afterAll(() => { + blaiseApiMock.reset(); + }); + + it("should update user role and permissions successfully and return http status 200", async () => { + const user = "testUser"; + const role = "IPS Manager"; + const serverParks = ["gusty", "cma"]; + const defaultServerPark = "gusty"; + blaiseApiMock.setup(api => api.changeUserRole(It.isValue(user), It.isValue(role))) + .returns(async () => null); + blaiseApiMock.setup(api => api.changeUserServerParks(It.isValue(user), It.isValue(serverParks), It.isValue(defaultServerPark))) + .returns(async () => null); + + const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) + .set("Authorization", `${mockAuthToken}`) + .send({ role }); + + const log = logInfo.mock.calls[0][0]; + expect(log).toEqual("AUDIT_LOG: testUser has successfully updated user role and permissions to IPS Manager for testUser"); + expect(response.statusCode).toEqual(200); + expect(response.body.message).toContain(`${mockUser.name} has successfully updated user role and permissions to ${role} for ${user}`); + blaiseApiMock.verify(api => api.changeUserRole(It.isValue(user), It.isValue(role)), Times.once()); + blaiseApiMock.verify(api => api.changeUserServerParks(It.isValue(user), It.isValue(serverParks), It.isValue(defaultServerPark)), Times.once()); + }); + + it("should return http status BAD_REQUEST_400 if role or user is not provided", async () => { + const user = "testUser"; + const role = ""; + + const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) + .set("Authorization", `${mockAuthToken}`) + .send({ role }); + + expect(response.statusCode).toEqual(400); + expect(response.body).toEqual("No user or role provided"); + }); + + it("should return http status INTERNAL_SERVER_ERROR_500 if Blaise API client throws an error", async () => { + const user = "testUser"; + const role = "admin"; + const errorMessage = "Blaise API client error"; + blaiseApiMock.setup(api => api.changeUserRole(It.isAny(), It.isAny())) + .returns(async () => { throw new Error(errorMessage); }); + + const response = await sut.patch(`/api/users/${user}/rolesAndPermissions`) + .set("Authorization", `${mockAuthToken}`) + .send({ role }); + + const infoLog = logInfo.mock.calls[0][0]; + const errorLog = logError.mock.calls[0][0]; + expect(infoLog).toEqual(`AUDIT_LOG: ${mockUser.name} has failed to update user role and permissions to ${role} for ${user}`); + expect(errorLog).toEqual(`AUDIT_LOG: Error whilst trying to update user role and permissions to admin for ${mockUser.name}: Error: ${errorMessage}`); + expect(response.statusCode).toEqual(500); + expect(response.body.message).toContain(errorMessage); + }); +}); \ No newline at end of file From f2791cc8029a18ad28ec22559b1b12b8aafe9cea Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 09:53:18 +0000 Subject: [PATCH 25/42] refactor: trim whitespace in text user inputs; allow custom IDs for ONSPasswordInputs; remove unnecessary user fields --- package.json | 2 +- .../users/UserProfileEdits/ChangePassword.tsx | 21 ++++++++++++------- src/pages/users/UserUpload/NewUser.tsx | 8 ++----- yarn.lock | 21 ++++++++++++++++--- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index 76add15..5278df3 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "@testing-library/user-event": "^13.5.0", "axios": "^1.7.4", "blaise-api-node-client": "git+https://github.com/ONSdigital/blaise-api-node-client", - "blaise-design-system-react-components": "git+https://github.com/ONSdigital/blaise-design-system-react-components#0.14.0", + "blaise-design-system-react-components": "git+https://github.com/ONSdigital/blaise-design-system-react-components#BLAIS5-4254", "blaise-login-react": "git+https://github.com/ONSdigital/blaise-login-react#1.1.1", "dotenv": "^10.0.0", "ejs": "^3.1.10", diff --git a/src/pages/users/UserProfileEdits/ChangePassword.tsx b/src/pages/users/UserProfileEdits/ChangePassword.tsx index d56ba4e..f38daf2 100644 --- a/src/pages/users/UserProfileEdits/ChangePassword.tsx +++ b/src/pages/users/UserProfileEdits/ChangePassword.tsx @@ -18,22 +18,27 @@ export default function ChangePassword(): ReactElement { const [redirect, setRedirect] = useState(false); const changePassword = () => { - if (password === "") { + const sanitisedPassword = password.trim(); + const sanitisedConfirmPassword = confirmPassword.trim(); + + if (sanitisedPassword === "" || sanitisedConfirmPassword === "") { setMessage("Passwords cannot be blank"); return; } - if (password !== confirmPassword) { + if (sanitisedPassword !== sanitisedConfirmPassword) { setMessage("Passwords do not match"); return; } setButtonLoading(true); const authManager = new AuthManager(); - fetch("/api/change-password/" + viewedUsername, { - "headers": Object.assign({}, { - "password": password - }, authManager.authHeader()) - }) + + fetch("/api/change-password/" + viewedUsername, + { + "headers": Object.assign({}, { + "password": sanitisedPassword + }, authManager.authHeader()) + }) .then((r: Response) => { if (r.status === 204) { setButtonLoading(false); @@ -82,10 +87,12 @@ export default function ChangePassword(): ReactElement {
changePassword()}> setPassword(e.target.value)} /> setConfirmPassword(e.target.value)} /> Date: Mon, 4 Nov 2024 09:53:37 +0000 Subject: [PATCH 26/42] test: trim whitespace in text user inputs; allow custom IDs for ONSPasswordInputs --- .../UserProfileEdits/ChangePassword.test.tsx | 71 +++++++++++++++---- .../ChangePassword.test.tsx.snap | 8 +-- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/src/pages/users/UserProfileEdits/ChangePassword.test.tsx b/src/pages/users/UserProfileEdits/ChangePassword.test.tsx index 02c04f6..08ac627 100644 --- a/src/pages/users/UserProfileEdits/ChangePassword.test.tsx +++ b/src/pages/users/UserProfileEdits/ChangePassword.test.tsx @@ -16,7 +16,7 @@ jest.mock("react-router-dom", () => ({ jest.mock("blaise-login-react/blaise-login-react-client", () => ({ AuthManager: jest.fn().mockImplementation(() => ({ authHeader: () => ({ - Authorization: "Bearer " + process.env.MOCK_AUTH_TOKEN + Authorization: process.env.MOCK_AUTH_TOKEN }) })) })); @@ -42,7 +42,10 @@ beforeEach(() => { (useParams as jest.Mock).mockReturnValue({ user: mockUserDetails.name }); }); -afterEach(() => cleanup()); +afterEach(() => { + jest.clearAllMocks(); + cleanup(); +}); describe("ChangePassword Component", () => { it("matches the snapshot", async () => { @@ -55,6 +58,26 @@ describe("ChangePassword Component", () => { expect(asFragment()).toMatchSnapshot(); }); + it("displays error message when passwords are empty", async () => { + const { findByText, getByLabelText, getByText } = render( + + + + ); + + const newPasswordInput = getByLabelText("New password"); + const confirmPasswordInput = getByLabelText("Confirm password"); + const saveButton = getByText("Save"); + + act(() => { + userEvent.type(newPasswordInput, ""); + userEvent.type(confirmPasswordInput, ""); + userEvent.click(saveButton); + }); + + expect(await findByText(/Passwords cannot be blank/i)).toBeVisible(); + }); + it("displays error message when passwords do not match", async () => { const { findByText, getByLabelText, getByText } = render( @@ -68,13 +91,39 @@ describe("ChangePassword Component", () => { act(() => { userEvent.type(newPasswordInput, "password123"); - userEvent.type(confirmPasswordInput, "password321"); + userEvent.type(confirmPasswordInput, "password321333"); userEvent.click(saveButton); }); expect(await findByText(/Passwords do not match/i)).toBeVisible(); }); + it("calls fetch with correct parameters upon form submission with matching passwords that remove any trailing whitespaces", async () => { + const { getByLabelText, getByText, findByText } = render( + + + + ); + + const newPasswordInput = getByLabelText("New password"); + const confirmPasswordInput = getByLabelText("Confirm password"); + const saveButton = getByText("Save"); + + // Wait for state update + act(() => { + userEvent.type(newPasswordInput, "password123 "); + userEvent.type(confirmPasswordInput, "password123 "); + userEvent.click(saveButton); + }); + + expect(fetch).toHaveBeenCalledWith("/api/change-password/testUser", { + headers: { + Authorization: process.env.MOCK_AUTH_TOKEN, + password: "password123" + } + }); + }); + it("calls fetch with correct parameters upon form submission with matching passwords", async () => { const { getByLabelText, getByText, findByText } = render( @@ -87,19 +136,17 @@ describe("ChangePassword Component", () => { const saveButton = getByText("Save"); // Wait for state update - act(async () => { + act(() => { userEvent.type(newPasswordInput, "password123"); userEvent.type(confirmPasswordInput, "password123"); userEvent.click(saveButton); }); - // Improvement: Figure out why the fetch function is not being called - // expect(fetch).toHaveBeenCalledTimes(1); - // expect(fetch).toHaveBeenCalledWith("/api/change-password/testUser", { - // "headers": { - // "Authorization": "Bearer " + process.env.MOCK_AUTH_TOKEN, - // "password": "password123" - // } - // }); + expect(fetch).toHaveBeenCalledWith("/api/change-password/testUser", { + headers: { + Authorization: process.env.MOCK_AUTH_TOKEN, + password: "password123" + } + }); }); }); \ No newline at end of file diff --git a/src/pages/users/UserProfileEdits/__snapshots__/ChangePassword.test.tsx.snap b/src/pages/users/UserProfileEdits/__snapshots__/ChangePassword.test.tsx.snap index 8b72aae..97b9927 100644 --- a/src/pages/users/UserProfileEdits/__snapshots__/ChangePassword.test.tsx.snap +++ b/src/pages/users/UserProfileEdits/__snapshots__/ChangePassword.test.tsx.snap @@ -103,7 +103,7 @@ exports[`ChangePassword Component matches the snapshot 1`] = ` > @@ -128,7 +128,7 @@ exports[`ChangePassword Component matches the snapshot 1`] = ` @@ -138,7 +138,7 @@ exports[`ChangePassword Component matches the snapshot 1`] = ` > @@ -163,7 +163,7 @@ exports[`ChangePassword Component matches the snapshot 1`] = ` From b72e261cbb4b3649ff6b69d91b8dd705eeb9890f Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:34:11 +0000 Subject: [PATCH 27/42] refactor: assign correct serverparks based on role, before sending to server --- src/ClientConfig.ts | 2 ++ src/pages/users/UserUpload/NewUser.tsx | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ClientConfig.ts b/src/ClientConfig.ts index 9157e5f..f5403f0 100644 --- a/src/ClientConfig.ts +++ b/src/ClientConfig.ts @@ -1,4 +1,5 @@ export interface ClientConfig { + DefaultServerPark: string; RoleToServerParksMap: { [key: string]: string[] } ; } import role_to_serverparks_map_json from "./role-to-serverparks-map.json"; @@ -6,6 +7,7 @@ import role_to_serverparks_map_json from "./role-to-serverparks-map.json"; export function loadConfigFromEnv(): ClientConfig { const roleToServerParksMap: { [key: string]: string[] } = role_to_serverparks_map_json; return { + DefaultServerPark: "gusty", RoleToServerParksMap: roleToServerParksMap }; } \ No newline at end of file diff --git a/src/pages/users/UserUpload/NewUser.tsx b/src/pages/users/UserUpload/NewUser.tsx index 89e89b0..f35c354 100644 --- a/src/pages/users/UserUpload/NewUser.tsx +++ b/src/pages/users/UserUpload/NewUser.tsx @@ -18,7 +18,7 @@ function NewUserComponent(): ReactElement { const [message, setMessage] = useState(""); const [redirect, setRedirect] = useState(false); const [roleList, setRoleList] = useState([]); - const cconfig = loadConfigFromEnv(); + const config = loadConfigFromEnv(); async function createNewUser(formData: UserForm) { setUsername(formData.username); @@ -26,7 +26,9 @@ function NewUserComponent(): ReactElement { const newUser: NewUser = { name: formData.username.trim(), password: formData.password.trim(), - role: role + role: role, + defaultServerPark: config.DefaultServerPark, + serverParks: config.RoleToServerParksMap[role] }; setButtonLoading(true); const created = await addNewUser(newUser); From 1b308a934675ae43f9c536a934b2eb453f0cd138 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:52:54 +0000 Subject: [PATCH 28/42] refactor: assign correct serverparks based on role, before sending to server --- src/ClientConfig.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ClientConfig.ts b/src/ClientConfig.ts index f5403f0..792561d 100644 --- a/src/ClientConfig.ts +++ b/src/ClientConfig.ts @@ -7,7 +7,7 @@ import role_to_serverparks_map_json from "./role-to-serverparks-map.json"; export function loadConfigFromEnv(): ClientConfig { const roleToServerParksMap: { [key: string]: string[] } = role_to_serverparks_map_json; return { - DefaultServerPark: "gusty", + DefaultServerPark: roleToServerParksMap["DEFAULT"][0], RoleToServerParksMap: roleToServerParksMap }; } \ No newline at end of file From b4b50ef371853924f6a7f97cd2d2a66dba9b7e62 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:12:23 +0000 Subject: [PATCH 29/42] chore: remove console log --- server/routes/blaiseApi.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index 11df9d8..a314fd7 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -136,7 +136,6 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli data.serverParks = defaultServerPark; data.defaultServerPark = defaultServerPark[0]; } - console.log(data); auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user: ${data.name}`); return res.status(200).json(await blaiseApiClient.createUser(data)); } catch (error) { From f55a3ac9cf2a33953534baa97667346b962a81b8 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 12:50:39 +0000 Subject: [PATCH 30/42] ref/test: improve logs and add error test case --- server/routes/blaiseApi.ts | 6 ++--- server/tests/routes/blaiseApi.test.ts | 33 +++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index a314fd7..41c0e8c 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -45,7 +45,7 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli message: successMessage }); } catch (error) { - const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}: ${error}`; + const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}, with error message: ${error}`; auditLogger.info(req.log, `${currentUser.name || "Unknown user"} has failed to update user role and permissions to ${role} for ${user}`); auditLogger.error(req.log, errorMessage); return res.status(500).json({ @@ -136,10 +136,10 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli data.serverParks = defaultServerPark; data.defaultServerPark = defaultServerPark[0]; } - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user: ${data.name}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully created user, ${data.name}, with an assigned role of ${data.role}`); return res.status(200).json(await blaiseApiClient.createUser(data)); } catch (error) { - auditLogger.error(req.log, `Error whilst trying to create user: ${error}`); + auditLogger.error(req.log, `Error whilst trying to create new user, ${req.body.name}, with error message: ${error}`); return res.status(500).json(error); } }); diff --git a/server/tests/routes/blaiseApi.test.ts b/server/tests/routes/blaiseApi.test.ts index 14709a5..9f1b3b6 100644 --- a/server/tests/routes/blaiseApi.test.ts +++ b/server/tests/routes/blaiseApi.test.ts @@ -54,8 +54,9 @@ describe("POST /api/users endpoint", () => { it("should call Blaise API createUser endpoint with correct serverParks for each role EXISTING in server/role-to-serverparks-map.json AND return http status OK_200", async () => { let currentRoleNo = 0; const totalRoleCount = size(role_to_serverparks_map); - for(const roleName in role_to_serverparks_map) + for (const roleName in role_to_serverparks_map) { + logInfo.mockReset(); blaiseApiMock.reset(); console.log("Running for role %i of %i: %s", ++currentRoleNo, totalRoleCount, roleName); @@ -74,8 +75,7 @@ describe("POST /api/users endpoint", () => { .field("name", newUser.name) .field("role", roleName); - const log = logInfo.mock.calls[0][0]; - expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully created user: ${newUser.name}`); + expect(logInfo.mock.calls[0][0]).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully created user, ${newUser.name}, with an assigned role of ${roleName}`); expect(response.statusCode).toEqual(200); blaiseApiMock.verify(a => a.createUser(It.is( x=> x.defaultServerPark == newUser.defaultServerPark @@ -105,7 +105,7 @@ describe("POST /api/users endpoint", () => { .set("Authorization", `${mockAuthToken}`); const log = logInfo.mock.calls[0][0]; - expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully created user: ${newUser.name}`); + expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully created user, ${newUser.name}, with an assigned role of ${roleName}`); expect(response.statusCode).toEqual(200); blaiseApiMock.verify(a => a.createUser(It.is( x=> x.defaultServerPark == newUser.defaultServerPark @@ -129,6 +129,29 @@ describe("POST /api/users endpoint", () => { expect(response.statusCode).toEqual(400); expect(response.body.message).toEqual("No role provided for user creation"); }); + + it("should return http status INTERNAL_SERVER_ERROR_500 if Blaise API client throws an error", async () => { + const roleName = "IPS Manager"; + const spmap = role_to_serverparks_map.DEFAULT; + const newUser : NewUser = { + name: "name1", + password: "password1", + role: roleName, + serverParks: spmap, + defaultServerPark: spmap[0] + }; + const errorMessage = "Blaise API client error"; + blaiseApiMock.setup((api) => api.createUser(It.isAny())).throws(new Error(errorMessage)); + + const response = await sut.post("/api/users") + .set("Authorization", `${mockAuthToken}`) + .field("name", newUser.name) + .field("role", roleName); + + const log = logError.mock.calls[0][0]; + expect(log).toEqual(`AUDIT_LOG: Error whilst trying to create new user, ${newUser.name}, with error message: Error: ${errorMessage}`); + expect(response.statusCode).toEqual(500); + }); }); describe("DELETE /api/users endpoint", () => { @@ -355,7 +378,7 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { const infoLog = logInfo.mock.calls[0][0]; const errorLog = logError.mock.calls[0][0]; expect(infoLog).toEqual(`AUDIT_LOG: ${mockUser.name} has failed to update user role and permissions to ${role} for ${user}`); - expect(errorLog).toEqual(`AUDIT_LOG: Error whilst trying to update user role and permissions to admin for ${mockUser.name}: Error: ${errorMessage}`); + expect(errorLog).toEqual(`AUDIT_LOG: Error whilst trying to update user role and permissions to admin for ${mockUser.name}, with error message: Error: ${errorMessage}`); expect(response.statusCode).toEqual(500); expect(response.body.message).toContain(errorMessage); }); From 6a15aaaff34c32ec4a1bbdae4475ea5b413f3125 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 13:29:34 +0000 Subject: [PATCH 31/42] ref: improve logs --- server/routes/blaiseApi.ts | 4 ++-- server/tests/routes/blaiseApi.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index 41c0e8c..2b38c37 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -110,10 +110,10 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli auditLogger.error(req.log, "No user provided for deletion"); return res.status(400).json(); } - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user: ${user}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user, ${user}`); return res.status(204).json(await blaiseApiClient.deleteUser(user)); } catch (error) { - auditLogger.error(req.log, `Error whilst trying to delete user: ${error}`); + auditLogger.error(req.log, `Error whilst trying to delete user, ${req.headers.user}, with error message: ${error}`); return res.status(500).json(error); } }); diff --git a/server/tests/routes/blaiseApi.test.ts b/server/tests/routes/blaiseApi.test.ts index 9f1b3b6..6d3a03c 100644 --- a/server/tests/routes/blaiseApi.test.ts +++ b/server/tests/routes/blaiseApi.test.ts @@ -172,7 +172,7 @@ describe("DELETE /api/users endpoint", () => { .set("user", username); const log = logInfo.mock.calls[0][0]; - expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully deleted user: ${username}`); + expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully deleted user, ${username}`); expect(response.statusCode).toEqual(204); blaiseApiMock.verify(a => a.deleteUser(It.isValue(username)), Times.once()); }); @@ -188,7 +188,7 @@ describe("DELETE /api/users endpoint", () => { .set("user", username); const log = logError.mock.calls[0][0]; - expect(log).toEqual(`AUDIT_LOG: Error whilst trying to delete user: ${errorMessage}`); + expect(log).toEqual(`AUDIT_LOG: Error whilst trying to delete user, ${username}, with error message: ${errorMessage}`); expect(response.statusCode).toEqual(500); blaiseApiMock.verify(a => a.deleteUser(It.isValue(username)), Times.once()); expect(response.body).toStrictEqual(errorMessage); From 65e70da92b06b64217cb2ffba26f6297839d232e Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 14:03:21 +0000 Subject: [PATCH 32/42] ref: improve logs --- server/routes/blaiseApi.ts | 2 +- server/tests/routes/blaiseApi.test.ts | 2 +- src/pages/users/UserProfileEdits/ChangePassword.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index 2b38c37..7e17f72 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -110,7 +110,7 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli auditLogger.error(req.log, "No user provided for deletion"); return res.status(400).json(); } - auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user, ${user}`); + auditLogger.info(req.log, `${currentUser.name || "Unknown"} has successfully deleted user called ${user}`); return res.status(204).json(await blaiseApiClient.deleteUser(user)); } catch (error) { auditLogger.error(req.log, `Error whilst trying to delete user, ${req.headers.user}, with error message: ${error}`); diff --git a/server/tests/routes/blaiseApi.test.ts b/server/tests/routes/blaiseApi.test.ts index 6d3a03c..df0cc4a 100644 --- a/server/tests/routes/blaiseApi.test.ts +++ b/server/tests/routes/blaiseApi.test.ts @@ -172,7 +172,7 @@ describe("DELETE /api/users endpoint", () => { .set("user", username); const log = logInfo.mock.calls[0][0]; - expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully deleted user, ${username}`); + expect(log).toEqual(`AUDIT_LOG: ${mockUser.name} has successfully deleted user called ${username}`); expect(response.statusCode).toEqual(204); blaiseApiMock.verify(a => a.deleteUser(It.isValue(username)), Times.once()); }); diff --git a/src/pages/users/UserProfileEdits/ChangePassword.tsx b/src/pages/users/UserProfileEdits/ChangePassword.tsx index f38daf2..e83409f 100644 --- a/src/pages/users/UserProfileEdits/ChangePassword.tsx +++ b/src/pages/users/UserProfileEdits/ChangePassword.tsx @@ -72,7 +72,7 @@ export default function ChangePassword(): ReactElement { currentUser, updatedPanel: { visible: true, - message: "Password for user " + viewedUsername + " changed at " + (new Date()).toLocaleTimeString("en-UK") + " " + (new Date()).toLocaleDateString("en-UK"), + message: "Password for user " + viewedUsername, status: "success" } }} From 96d595080edb5ea45ef21439790a9ab27669580f Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 14:30:16 +0000 Subject: [PATCH 33/42] ref: improve logs --- src/pages/users/UserProfileEdits/ChangePassword.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/users/UserProfileEdits/ChangePassword.tsx b/src/pages/users/UserProfileEdits/ChangePassword.tsx index e83409f..fed6fe0 100644 --- a/src/pages/users/UserProfileEdits/ChangePassword.tsx +++ b/src/pages/users/UserProfileEdits/ChangePassword.tsx @@ -72,7 +72,7 @@ export default function ChangePassword(): ReactElement { currentUser, updatedPanel: { visible: true, - message: "Password for user " + viewedUsername, + message: "Password changed for user called " + viewedUsername, status: "success" } }} From f6d46481992f85418cbe17e9f2d801e8bb48663f Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 14:30:50 +0000 Subject: [PATCH 34/42] ref: improve logs --- src/pages/users/UserProfileEdits/ChangePassword.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/users/UserProfileEdits/ChangePassword.tsx b/src/pages/users/UserProfileEdits/ChangePassword.tsx index fed6fe0..1a4a4b7 100644 --- a/src/pages/users/UserProfileEdits/ChangePassword.tsx +++ b/src/pages/users/UserProfileEdits/ChangePassword.tsx @@ -72,7 +72,7 @@ export default function ChangePassword(): ReactElement { currentUser, updatedPanel: { visible: true, - message: "Password changed for user called " + viewedUsername, + message: "Password successfully changed for user called " + viewedUsername, status: "success" } }} From fd20b678463c206a5d62a20b3e6acfcc035b3bf8 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 16:01:55 +0000 Subject: [PATCH 35/42] ref: improve logs --- server/routes/blaiseApi.ts | 4 ++-- server/tests/routes/blaiseApi.test.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index 7e17f72..3b55657 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -42,14 +42,14 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli const successMessage = `${currentUser.name || "Unknown user"} has successfully updated user role and permissions to ${role} for ${user}`; auditLogger.info(req.log, successMessage); return res.status(200).json({ - message: successMessage + message: "Successfully updated user role and permissions to " + role + " for " + user }); } catch (error) { const errorMessage = `Error whilst trying to update user role and permissions to ${role} for ${req.params.user}, with error message: ${error}`; auditLogger.info(req.log, `${currentUser.name || "Unknown user"} has failed to update user role and permissions to ${role} for ${user}`); auditLogger.error(req.log, errorMessage); return res.status(500).json({ - message: errorMessage + message: "Failed to update user role and permissions to " + role + " for " + user }); } }); diff --git a/server/tests/routes/blaiseApi.test.ts b/server/tests/routes/blaiseApi.test.ts index df0cc4a..97b16af 100644 --- a/server/tests/routes/blaiseApi.test.ts +++ b/server/tests/routes/blaiseApi.test.ts @@ -347,7 +347,7 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { const log = logInfo.mock.calls[0][0]; expect(log).toEqual("AUDIT_LOG: testUser has successfully updated user role and permissions to IPS Manager for testUser"); expect(response.statusCode).toEqual(200); - expect(response.body.message).toContain(`${mockUser.name} has successfully updated user role and permissions to ${role} for ${user}`); + expect(response.body.message).toContain(`Successfully updated user role and permissions to ${role} for ${user}`); blaiseApiMock.verify(api => api.changeUserRole(It.isValue(user), It.isValue(role)), Times.once()); blaiseApiMock.verify(api => api.changeUserServerParks(It.isValue(user), It.isValue(serverParks), It.isValue(defaultServerPark)), Times.once()); }); @@ -380,6 +380,6 @@ describe("PATCH /api/users/:user/rolesAndPermissions endpoint", () => { expect(infoLog).toEqual(`AUDIT_LOG: ${mockUser.name} has failed to update user role and permissions to ${role} for ${user}`); expect(errorLog).toEqual(`AUDIT_LOG: Error whilst trying to update user role and permissions to admin for ${mockUser.name}, with error message: Error: ${errorMessage}`); expect(response.statusCode).toEqual(500); - expect(response.body.message).toContain(errorMessage); + expect(response.body.message).toContain("Failed to update user role and permissions to admin for testUser"); }); }); \ No newline at end of file From 5a57a47a9dc8f3641365590d4334d5d339ce2fad Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Mon, 4 Nov 2024 16:41:47 +0000 Subject: [PATCH 36/42] ref: improve logs --- src/pages/users/UserProfileEdits/DeleteUser.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/users/UserProfileEdits/DeleteUser.tsx b/src/pages/users/UserProfileEdits/DeleteUser.tsx index 012412d..fcaee74 100644 --- a/src/pages/users/UserProfileEdits/DeleteUser.tsx +++ b/src/pages/users/UserProfileEdits/DeleteUser.tsx @@ -30,7 +30,7 @@ function DeleteUser(): ReactElement { return; } - setReturnPanel({ visible: true, message: "User " + viewedUsername + " deleted", status: "success" }); + setReturnPanel({ visible: true, message: "Successfully deleted user called " + viewedUsername, status: "success" }); setRedirect(true); } From 0168d135e2bd73bd6f1eda7dcdc4b6d8be2364fd Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 5 Nov 2024 09:37:03 +0000 Subject: [PATCH 37/42] fix: sanitize message to prevent log injection --- server/logger/cloudLogging.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/logger/cloudLogging.ts b/server/logger/cloudLogging.ts index 32a8af4..791d41d 100644 --- a/server/logger/cloudLogging.ts +++ b/server/logger/cloudLogging.ts @@ -15,11 +15,15 @@ export default class AuditLogger { } info(logger: IncomingMessage["log"], message: string): void { - logger.info(`AUDIT_LOG: ${message}`); + const logFormat = "AUDIT_LOG: message"; + const log = logFormat.replace("message", message); + logger.info(log); } error(logger: IncomingMessage["log"], message: string): void { - logger.error(`AUDIT_LOG: ${message}`); + const logFormat = "AUDIT_LOG: message"; + const log = logFormat.replace("message", message); + logger.error(log); } async getLogs(): Promise { From 736d33edfbfb0740c325dcb029f5df2cce963af4 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 5 Nov 2024 09:40:09 +0000 Subject: [PATCH 38/42] chore: remove unused method --- src/pages/users/UserProfileEdits/ChangePassword.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/users/UserProfileEdits/ChangePassword.test.tsx b/src/pages/users/UserProfileEdits/ChangePassword.test.tsx index 08ac627..9efe36b 100644 --- a/src/pages/users/UserProfileEdits/ChangePassword.test.tsx +++ b/src/pages/users/UserProfileEdits/ChangePassword.test.tsx @@ -99,7 +99,7 @@ describe("ChangePassword Component", () => { }); it("calls fetch with correct parameters upon form submission with matching passwords that remove any trailing whitespaces", async () => { - const { getByLabelText, getByText, findByText } = render( + const { getByLabelText, getByText } = render( From 5580c25fd046798c9442ccd1bfce3766298aad4e Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 5 Nov 2024 12:54:25 +0000 Subject: [PATCH 39/42] chore: remove comments --- server/routes/blaiseApi.ts | 2 +- server/tests/routes/blaiseApi.test.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index 3b55657..fbcef94 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -84,7 +84,7 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli } if (!req.params.user || !password) { - return res.status(400).json(); + return res.status(400).json("No user or password provided"); } blaiseApiClient.changePassword(req.params.user, password).then(() => { diff --git a/server/tests/routes/blaiseApi.test.ts b/server/tests/routes/blaiseApi.test.ts index 97b16af..96430ab 100644 --- a/server/tests/routes/blaiseApi.test.ts +++ b/server/tests/routes/blaiseApi.test.ts @@ -263,7 +263,6 @@ describe("GET /api/roles endpoint", () => { }); }); -// TODO: Changing a resource should be a PATCH request not a GET request describe("GET /api/change-password/:user endpoint", () => { beforeEach(() => { blaiseApiMock.reset(); From 5252d263b2b856d76552683f07d473f811503d19 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 5 Nov 2024 13:46:23 +0000 Subject: [PATCH 40/42] build: update design system release reference --- package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 5278df3..b7fe793 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "@testing-library/user-event": "^13.5.0", "axios": "^1.7.4", "blaise-api-node-client": "git+https://github.com/ONSdigital/blaise-api-node-client", - "blaise-design-system-react-components": "git+https://github.com/ONSdigital/blaise-design-system-react-components#BLAIS5-4254", + "blaise-design-system-react-components": "git+https://github.com/ONSdigital/blaise-design-system-react-components#0.14.1", "blaise-login-react": "git+https://github.com/ONSdigital/blaise-login-react#1.1.1", "dotenv": "^10.0.0", "ejs": "^3.1.10", diff --git a/yarn.lock b/yarn.lock index 0a7c207..45186ef 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6308,9 +6308,9 @@ bl@^4.0.3, bl@^4.1.0: regenerator-runtime "^0.14.1" typescript "~4.2.2" -"blaise-design-system-react-components@git+https://github.com/ONSdigital/blaise-design-system-react-components#BLAIS5-4254": +"blaise-design-system-react-components@git+https://github.com/ONSdigital/blaise-design-system-react-components#0.14.1": version "1.0.0" - resolved "git+https://github.com/ONSdigital/blaise-design-system-react-components#b16f4497287a661daac81cc8a0346fc363ae4e33" + resolved "git+https://github.com/ONSdigital/blaise-design-system-react-components#488cbea69e734b101c654eacaf999afdae709d97" dependencies: "@testing-library/dom" "^10.4.0" formik "^2.2.9" From fb143be8f086e0d50b2a60901efff66bc7932ee2 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 5 Nov 2024 14:50:44 +0000 Subject: [PATCH 41/42] feat/test: sanitize log messages whilst preserving readability --- server/logger/cloudLogging.ts | 12 +++++++---- server/tests/logger/cloudLogging.test.ts | 26 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 server/tests/logger/cloudLogging.test.ts diff --git a/server/logger/cloudLogging.ts b/server/logger/cloudLogging.ts index 791d41d..d4ef996 100644 --- a/server/logger/cloudLogging.ts +++ b/server/logger/cloudLogging.ts @@ -3,6 +3,12 @@ import { Logging } from "@google-cloud/logging"; import { IncomingMessage } from "http"; import { AuditLog } from "../interfaces/logger"; +export function formatLogMessage(text: string): string { + const message = text.replace(/[^\x20-\x7E\r\n]+/g, ""); + const logFormat = "AUDIT_LOG: message"; + return logFormat.replace("message", message); +} + export default class AuditLogger { projectId: string; logger: Logging; @@ -15,14 +21,12 @@ export default class AuditLogger { } info(logger: IncomingMessage["log"], message: string): void { - const logFormat = "AUDIT_LOG: message"; - const log = logFormat.replace("message", message); + const log = formatLogMessage(message); logger.info(log); } error(logger: IncomingMessage["log"], message: string): void { - const logFormat = "AUDIT_LOG: message"; - const log = logFormat.replace("message", message); + const log = formatLogMessage(message); logger.error(log); } diff --git a/server/tests/logger/cloudLogging.test.ts b/server/tests/logger/cloudLogging.test.ts new file mode 100644 index 0000000..dc7f776 --- /dev/null +++ b/server/tests/logger/cloudLogging.test.ts @@ -0,0 +1,26 @@ +import { formatLogMessage } from "../../logger/cloudLogging"; + +describe("formatLogMessage utility function ensures complex log messages are sanitized but still readable", () => { + it("should preserve newlines and carriage returns", () => { + const inputMessage = "Error: Something went wrong\nDetails: Invalid input\r\nPlease try again."; + const expectedOutput = "AUDIT_LOG: Error: Something went wrong\nDetails: Invalid input\r\nPlease try again."; + + const formattedMessage = formatLogMessage(inputMessage); + + console.log("Expected log format:", JSON.stringify(expectedOutput)); + console.log("Received log format:", JSON.stringify(formattedMessage)); + + expect(formattedMessage).toBe(expectedOutput); + }); + + it("should remove non-printable characters", () => { + const message = "Error: Something went wrong\x01\x02\n at FunctionName (file.js:10:15)\x03\x04\n at AnotherFunction (file.js:20:25)\r\n at YetAnotherFunction (file.js:30:35)"; + const expectedOutput = "AUDIT_LOG: Error: Something went wrong\n at FunctionName (file.js:10:15)\n at AnotherFunction (file.js:20:25)\r\n at YetAnotherFunction (file.js:30:35)"; + const formattedMessage = formatLogMessage(message); + + console.log("Expected log format:", JSON.stringify(expectedOutput)); + console.log("Received log format:", JSON.stringify(formattedMessage)); + + expect(formattedMessage).toBe(expectedOutput); + }); +}); \ No newline at end of file From 41721d595d8f977261147b6818e84796583bfa15 Mon Sep 17 00:00:00 2001 From: kristian4res <57638182+kristian4res@users.noreply.github.com> Date: Tue, 5 Nov 2024 14:58:54 +0000 Subject: [PATCH 42/42] ref: improve code --- server/routes/blaiseApi.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/routes/blaiseApi.ts b/server/routes/blaiseApi.ts index fbcef94..b7695e6 100644 --- a/server/routes/blaiseApi.ts +++ b/server/routes/blaiseApi.ts @@ -19,10 +19,10 @@ export default function blaiseApi(config: CustomConfig, auth: Auth, blaiseApiCli const currentUser = auth.GetUser(auth.GetToken(req)); const { role } = req.body; const user = req.params.user; - let newServerParks = [""]; - let newDefaultServerPark = ""; + let newServerParks: string[]; + let newDefaultServerPark: string; - if (!req.params.user || !req.body.role) { + if (!user || !role) { return res.status(400).json("No user or role provided"); }