Code cleanup

Author: chris-elliott-nhsd

frontend/src/__tests__/utils/csrf-utils.test.ts

@@ -10,6 +10,7 @@ import { cookies } from 'next/headers';
 import { sign } from 'jsonwebtoken';
 import type { ReadonlyRequestCookies } from 'next/dist/server/web/spec-extension/adapters/request-cookies';
 import { BinaryLike, BinaryToTextEncoding } from 'node:crypto';
+import { getAccessTokenServer } from '@utils/amplify-utils';
 
 class MockHmac {
   constructor() {}
@@ -35,6 +36,7 @@ jest.mock('node:crypto', () => ({
   createHmac: () => new MockHmac(),
   randomBytes: () => 'salt',
 }));
+jest.mock('@utils/amplify-utils');
 
 const OLD_ENV = { ...process.env };
 
@@ -80,11 +82,7 @@ describe('getCsrfFormValue', () => {
 
 describe('getSessionId', () => {
   test('errors when access token not found', async () => {
-    jest.mocked(cookies).mockReturnValue(
-      mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [],
-      })
-    );
+    jest.mocked(getAccessTokenServer).mockResolvedValue(undefined);
 
     await expect(() => getSessionId()).rejects.toThrow(
       'Could not get access token'
@@ -99,33 +97,15 @@ describe('getSessionId', () => {
       'key'
     );
 
-    jest.mocked(cookies).mockReturnValue(
-      mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [
-          {
-            name: 'Cognito.123.accessToken',
-            value: mockEmptyJwt,
-          },
-        ],
-      })
-    );
+    jest.mocked(getAccessTokenServer).mockResolvedValue(mockEmptyJwt);
 
     await expect(() => getSessionId()).rejects.toThrow(
       'Could not get session ID'
     );
   });
 
   test('returns session id', async () => {
-    jest.mocked(cookies).mockReturnValue(
-      mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [
-          {
-            name: 'Cognito.123.accessToken',
-            value: mockJwt,
-          },
-        ],
-      })
-    );
+    jest.mocked(getAccessTokenServer).mockResolvedValue(mockJwt);
 
     const sessionId = await getSessionId();
 
@@ -164,16 +144,7 @@ describe('verifyCsrfTokenFull', () => {
   test('missing CSRF cookie', async () => {
     const formData = mockDeep<FormData>();
 
-    jest.mocked(cookies).mockReturnValue(
-      mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [
-          {
-            name: 'Cognito.123.accessToken',
-            value: mockJwt,
-          },
-        ],
-      })
-    );
+    jest.mocked(getAccessTokenServer).mockResolvedValue(mockJwt);
 
     await expect(() => verifyCsrfTokenFull(formData)).rejects.toThrow(
       'missing CSRF cookie'
@@ -185,14 +156,9 @@ describe('verifyCsrfTokenFull', () => {
       get: () => null,
     });
 
+    jest.mocked(getAccessTokenServer).mockResolvedValue(mockJwt);
     jest.mocked(cookies).mockReturnValue(
       mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [
-          {
-            name: 'Cognito.123.accessToken',
-            value: mockJwt,
-          },
-        ],
         get: (_: string) => ({
           name: 'csrf_token',
           value: 'hmac.salt',
@@ -210,14 +176,9 @@ describe('verifyCsrfTokenFull', () => {
       get: () => 'hmac2.salt',
     });
 
+    jest.mocked(getAccessTokenServer).mockResolvedValue(mockJwt);
     jest.mocked(cookies).mockReturnValue(
       mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [
-          {
-            name: 'Cognito.123.accessToken',
-            value: mockJwt,
-          },
-        ],
         get: (_: string) => ({
           name: 'csrf_token',
           value: 'hmac.salt',
@@ -235,14 +196,9 @@ describe('verifyCsrfTokenFull', () => {
       get: () => 'hmac2.salt',
     });
 
+    jest.mocked(getAccessTokenServer).mockResolvedValue(mockJwt);
     jest.mocked(cookies).mockReturnValue(
       mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [
-          {
-            name: 'Cognito.123.accessToken',
-            value: mockJwt,
-          },
-        ],
         get: (_: string) => ({
           name: 'csrf_token',
           value: 'hmac2.salt',
@@ -260,14 +216,9 @@ describe('verifyCsrfTokenFull', () => {
       get: () => 'hmac.salt',
     });
 
+    jest.mocked(getAccessTokenServer).mockResolvedValue(mockJwt);
     jest.mocked(cookies).mockReturnValue(
       mockDeep<ReadonlyRequestCookies>({
-        getAll: () => [
-          {
-            name: 'Cognito.123.accessToken',
-            value: mockJwt,
-          },
-        ],
         get: (_: string) => ({
           name: 'csrf_token',
           value: 'hmac.salt',

frontend/src/utils/csrf-utils.ts

@@ -4,14 +4,13 @@ import { jwtDecode } from 'jwt-decode';
 import { createHmac, randomBytes } from 'node:crypto';
 import { cookies } from 'next/headers';
 import { getEnvironmentVariable } from './get-environment-variable';
+import { getAccessTokenServer } from './amplify-utils';
 
 export const getCsrfFormValue = async () =>
   cookies().get('csrf_token')?.value ?? 'no_token';
 
 export const getSessionId = async () => {
-  const accessToken = cookies()
-    .getAll()
-    .find(({ name }) => name.endsWith('accessToken'))?.value;
+  const accessToken = await getAccessTokenServer();
 
   if (!accessToken) {
     throw new Error('Could not get access token');

infrastructure/terraform/components/app/ssm_parameter_csrf_secret.tf

@@ -2,12 +2,6 @@ resource "aws_ssm_parameter" "csrf_secret" {
   name        = "/${local.csi}/csrf_secret"
   description = "The Basic Auth password used for the amplify app. This parameter is sourced from Github Environment variables"
 
-  type  = "String"
-  value = var.CSRF_SECRET != "unset" ? var.CSRF_SECRET : random_bytes.csrf_secret[0].hex
-}
-
-resource "random_bytes" "csrf_secret" {
-  count = var.CSRF_SECRET == "unset" ? 1 : 0
-
-  length = 16
+  type  = "SecureString"
+  value = var.CSRF_SECRET
 }

infrastructure/terraform/components/app/variables.tf

@@ -110,7 +110,6 @@ variable "CSRF_SECRET" {
   # Github only does uppercase env vars
   type        = string
   description = "Secure cryptographic key to be used for generating CSRF tokens - This is entended to be read from CI variables and not commited to any codebase"
-  default     = "unset"
 }
 
 variable "branch_name" {