CB2-10665: Remove JWT from Log (#98)

* feat(cb2-10665): do not log a valid JWT token * feat(cb2-10665): revert use of test jwt json in logger unit tests * feat(cb2-10665): add unit tests to check email is correct in logs * feat(cb2-10665): refactor code to ensure logs are always populated * feat(cb2-10665): add roles to log event * feat(cb2-10665): bump axios version * feat(cb2-10665): remove duplicate information from the log error --------- Co-authored-by: Michael <[email protected]>
dvsa · Mar 27, 2024 · b54672e · b54672e
1 parent 0b88189
commit b54672e
Show file tree

Hide file tree

Showing 8 changed files with 128 additions and 85 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -31,7 +31,7 @@
  "dependencies": {
  "aws-sdk": "2.1358.0",
  "aws-xray-sdk": "3.5.0",
- "axios": "1.3.5",
+ "axios": "1.6.8",
  "js-yaml": "4.1.0",
  "jsonwebtoken": "9.0.0",
  "qs": "6.11.1"

diff --git a/src/common/Logger.ts b/src/common/Logger.ts
@@ -2,7 +2,7 @@ import { ILogEvent } from "../models/ILogEvent";
 import { JWT_MESSAGE } from "../models/enums";
 import { ILogError } from "../models/ILogError";
 
-export const writeLogMessage = (log: ILogEvent, jwt: string, error?: any) => {
+export const writeLogMessage = (log: ILogEvent, error?: any) => {
  if (!error) {
  log.statusCode = 200;
  console.log(log);
@@ -32,7 +32,6 @@ export const writeLogMessage = (log: ILogEvent, jwt: string, error?: any) => {
  }
  }
  log.error = logError;
- log.error.token = jwt;
  console.error(log);
  }
  return log;

diff --git a/src/functions/authorizer.ts b/src/functions/authorizer.ts
@@ -21,7 +21,7 @@ export const authorizer = async (event: APIGatewayTokenAuthorizerEvent, context:
  const logEvent: ILogEvent = {};
 
  if (!process.env.AZURE_TENANT_ID || !process.env.AZURE_CLIENT_ID) {
- writeLogMessage(logEvent, event.authorizationToken, JWT_MESSAGE.INVALID_ID_SETUP);
+ writeLogMessage(logEvent, JWT_MESSAGE.INVALID_ID_SETUP);
  return unauthorisedPolicy();
  }
 
@@ -36,10 +36,10 @@ export const authorizer = async (event: APIGatewayTokenAuthorizerEvent, context:
  }
 
  reportNoValidRoles(jwt, event, context, logEvent);
- writeLogMessage(logEvent, event.authorizationToken, JWT_MESSAGE.INVALID_ROLES);
+ writeLogMessage(logEvent, JWT_MESSAGE.INVALID_ROLES);
  return unauthorisedPolicy();
  } catch (error: any) {
- writeLogMessage(logEvent, event.authorizationToken, error);
+ writeLogMessage(logEvent, error);
  return unauthorisedPolicy();
  }
 };

diff --git a/src/models/ILogError.ts b/src/models/ILogError.ts
@@ -1,5 +1,5 @@
+import Role from "../services/roles";
 export interface ILogError {
  name?: string;
  message?: string;
- token?: string;
 }
diff --git a/src/services/tokens.ts b/src/services/tokens.ts
@@ -19,8 +19,6 @@ export const getValidJwt = async (authorizationToken: string, logEvent: ILogEven
  throw new Error(JWT_MESSAGE.DECODE_FAILED);
  }
 
- await checkSignature(authorizationToken, decoded, tenantId, clientId);
-
  let username;
  const payload = decoded.payload as CVSJWTPayload;
 
@@ -35,7 +33,11 @@ export const getValidJwt = async (authorizationToken: string, logEvent: ILogEven
  }
 
  logEvent.email = username;
+ logEvent.roles = (decoded.payload as JwtPayload).roles;
  logEvent.tokenExpiry = new Date((payload.exp as number) * 1000).toISOString();
+
+ await checkSignature(authorizationToken, decoded, tenantId, clientId);
+
  return decoded;
 };
 

diff --git a/tests/unit/common/Logger.unitTest.ts b/tests/unit/common/Logger.unitTest.ts
@@ -3,15 +3,16 @@ import { ILogEvent } from "../../../src/models/ILogEvent";
 import errorLogEvent from "../../resources/errorLogEvent.json";
 import { writeLogMessage } from "../../../src/common/Logger";
 import successLogEvent from "../../resources/successLogEvent.json";
+import Role from "../../../src/services/roles";
 
 describe("test writeLogMessage method", () => {
  const logError: ILogError = {};
  const logErrorEvent: ILogEvent = errorLogEvent;
- logErrorEvent.roles = [{ name: "name1", access: "read" }];
+ logErrorEvent.roles = [{name: "test", access: "read"}] as Role[];
 
  context("when only the log event is passed in", () => {
  it("should return no errors", () => {
- const returnValue: ILogEvent = writeLogMessage(successLogEvent, "mock-jwt", null);
+ const returnValue: ILogEvent = writeLogMessage(successLogEvent, null);
 
  expect(returnValue.statusCode).toBe(200);
  });
@@ -23,46 +24,52 @@ describe("test writeLogMessage method", () => {
  console.log = jest.fn();
 
  logError.name = "TokenExpiredError";
- const returnValue: ILogEvent = writeLogMessage(logErrorEvent,"mock-jwt", error);
+ const returnValue: ILogEvent = writeLogMessage(logErrorEvent, error);
 
  expect(returnValue.error?.name).toBe("TokenExpiredError");
  expect(returnValue.error?.message).toBe("[JWT-ERROR-07] Error at undefined");
- expect(returnValue.error?.token).toBe("mock-jwt");
+ expect(returnValue.email).toBe(logErrorEvent.email);
+ expect(returnValue.roles).toBe(logErrorEvent.roles);
+ });
  });
 
  it("should log NotBeforeError", () => {
  const error: ILogError = { name: "NotBeforeError" };
  console.log = jest.fn();
 
  logError.name = "NotBeforeError";
- const returnValue: ILogEvent = writeLogMessage(logErrorEvent,"mock-jwt", error);
+ const returnValue: ILogEvent = writeLogMessage(logErrorEvent, error);
 
  expect(returnValue.error?.name).toBe("NotBeforeError");
  expect(returnValue.error?.message).toBe("[JWT-ERROR-08] undefined until undefined");
- expect(returnValue.error?.token).toBe("mock-jwt");
+ expect(returnValue.email).toBe(logErrorEvent.email);
+ expect(returnValue.roles).toBe(logErrorEvent.roles);
+
  });
 
  it("should log JsonWebTokenError", () => {
  const error: ILogError = { name: "JsonWebTokenError", message: "test" };
  console.log = jest.fn();
 
  logError.name = "JsonWebTokenError";
- const returnValue: ILogEvent = writeLogMessage(logErrorEvent,"mock-jwt", error);
+ const returnValue: ILogEvent = writeLogMessage(logErrorEvent, error);
 
  expect(returnValue.error?.name).toBe("JsonWebTokenError");
  expect(returnValue.error?.message).toBe("[JWT-ERROR-09] test");
- expect(returnValue.error?.token).toBe("mock-jwt");
+ expect(returnValue.email).toBe(logErrorEvent.email);
+ expect(returnValue.roles).toBe(logErrorEvent.roles);
+
  });
 
  it("should log the default error", () => {
  const error: ILogError = { name: "Error", message: "Error" };
  console.log = jest.fn();
 
- const returnValue: ILogEvent = writeLogMessage(logErrorEvent,"mock-jwt", error);
+ const returnValue: ILogEvent = writeLogMessage(logErrorEvent, error);
 
  expect(returnValue.error?.name).toBe("Error");
  expect(returnValue.error?.message).toBe("Error");
- expect(returnValue.error?.token).toBe("mock-jwt");
+ expect(returnValue.email).toBe(logErrorEvent.email);
+ expect(returnValue.roles).toBe(logErrorEvent.roles);
  });
- });
 });