From 6044b3f19f042c2f45feada9be49c484952a4d9a Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Mon, 27 Apr 2026 15:00:16 +0100 Subject: [PATCH 1/9] feat: create-session-login-lambda Co-authored-by: Copilot --- .gitignore | 1 + .../src/lib/login/nhs-login-service.test.ts | 5 + lambdas/src/lib/login/nhs-login-service.ts | 4 + lambdas/src/session-login-lambda/cookies.ts | 20 + .../src/session-login-lambda/index.test.ts | 211 +++++++++++ lambdas/src/session-login-lambda/index.ts | 132 +++++++ lambdas/src/session-login-lambda/init.test.ts | 229 +++++++++++ lambdas/src/session-login-lambda/init.ts | 176 +++++++++ lambdas/src/session-login-lambda/schemas.ts | 7 + .../session-login-service.test.ts | 275 ++++++++++++++ .../session-login-service.ts | 202 ++++++++++ local-environment/infra/main.tf | 45 +++ package.json | 1 + postman/README.md | 51 +++ ...ew_wiremock.local.postman_environment.json | 75 ++++ ...n_preview_wiremock.postman_collection.json | 356 ++++++++++++++++++ scripts/config/gitleaks.toml | 2 +- 17 files changed, 1791 insertions(+), 1 deletion(-) create mode 100644 lambdas/src/session-login-lambda/cookies.ts create mode 100644 lambdas/src/session-login-lambda/index.test.ts create mode 100644 lambdas/src/session-login-lambda/index.ts create mode 100644 lambdas/src/session-login-lambda/init.test.ts create mode 100644 lambdas/src/session-login-lambda/init.ts create mode 100644 lambdas/src/session-login-lambda/schemas.ts create mode 100644 lambdas/src/session-login-lambda/session-login-service.test.ts create mode 100644 lambdas/src/session-login-lambda/session-login-service.ts create mode 100644 postman/README.md create mode 100644 postman/session_login_preview_wiremock.local.postman_environment.json create mode 100644 postman/session_login_preview_wiremock.postman_collection.json diff --git a/.gitignore b/.gitignore index c629dcbad..b435c6478 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,7 @@ output.json # Node node_modules dist +.pnpm-store # Next.js .next diff --git a/lambdas/src/lib/login/nhs-login-service.test.ts b/lambdas/src/lib/login/nhs-login-service.test.ts index aa1011bd9..8768c50b8 100644 --- a/lambdas/src/lib/login/nhs-login-service.test.ts +++ b/lambdas/src/lib/login/nhs-login-service.test.ts @@ -49,6 +49,7 @@ function verificationSuccess(sub: string): NhsTokenVerificationResult { payload: { sub, iss: "https://issuer.example", + aud: "hometest", iat: 1234567890, exp: 1234571490, } as JwtPayload, @@ -108,6 +109,8 @@ describe("NhsLoginService.executeCallback", () => { nhsAccessToken: "nhs-access-token", nhsRefreshToken: "nhs-refresh-token", idTokenSubject: "user-123", + idTokenIssuer: "https://issuer.example", + idTokenAudience: "hometest", }, }); }); @@ -276,6 +279,8 @@ describe("NhsLoginService.executeCallback", () => { nhsAccessToken: "nhs-access-token", nhsRefreshToken: undefined, idTokenSubject: "user-123", + idTokenIssuer: "https://issuer.example", + idTokenAudience: "hometest", }, }); }); diff --git a/lambdas/src/lib/login/nhs-login-service.ts b/lambdas/src/lib/login/nhs-login-service.ts index d044ef7e7..99f28cfff 100644 --- a/lambdas/src/lib/login/nhs-login-service.ts +++ b/lambdas/src/lib/login/nhs-login-service.ts @@ -8,6 +8,8 @@ export interface INhsLoginResult { nhsAccessToken: string; nhsRefreshToken?: string; idTokenSubject: string; + idTokenIssuer?: string; + idTokenAudience?: string | string[]; } export type NhsLoginErrorCode = @@ -107,6 +109,8 @@ export class NhsLoginService implements INhsLoginService { nhsAccessToken: tokenResponse.access_token, nhsRefreshToken: this.normalizeRefreshToken(tokenResponse.refresh_token), idTokenSubject, + idTokenIssuer: idTokenResult.payload.iss, + idTokenAudience: idTokenResult.payload.aud, }, }; } diff --git a/lambdas/src/session-login-lambda/cookies.ts b/lambdas/src/session-login-lambda/cookies.ts new file mode 100644 index 000000000..6640f92d0 --- /dev/null +++ b/lambdas/src/session-login-lambda/cookies.ts @@ -0,0 +1,20 @@ +export const PREVIEW_SESSION_ACCESS_COOKIE_NAME = "preview_auth"; +export const PREVIEW_SESSION_REFRESH_COOKIE_NAME = "preview_auth_refresh"; +export const PREVIEW_SESSION_ACCESS_COOKIE_PATH = "/session-preview"; +export const PREVIEW_SESSION_REFRESH_COOKIE_PATH = "/session-preview/refresh"; + +function secureAttribute(secure: boolean): string { + return secure ? " Secure;" : ""; +} + +export function buildPreviewAccessCookie(token: string, sameSite: string, secure: boolean): string { + return `${PREVIEW_SESSION_ACCESS_COOKIE_NAME}=${token}; HttpOnly; Path=${PREVIEW_SESSION_ACCESS_COOKIE_PATH}; SameSite=${sameSite};${secureAttribute(secure)}`; +} + +export function buildPreviewRefreshCookie( + token: string, + sameSite: string, + secure: boolean, +): string { + return `${PREVIEW_SESSION_REFRESH_COOKIE_NAME}=${token}; HttpOnly; Path=${PREVIEW_SESSION_REFRESH_COOKIE_PATH}; SameSite=${sameSite};${secureAttribute(secure)}`; +} diff --git a/lambdas/src/session-login-lambda/index.test.ts b/lambdas/src/session-login-lambda/index.test.ts new file mode 100644 index 000000000..5ffeb6e86 --- /dev/null +++ b/lambdas/src/session-login-lambda/index.test.ts @@ -0,0 +1,211 @@ +import type { APIGatewayProxyEvent } from "aws-lambda"; + +const mockInit = jest.fn(); + +jest.mock("./init", () => ({ + init: () => mockInit(), +})); + +describe("session-login-lambda", () => { + beforeEach(() => { + jest.resetModules(); + process.env.ALLOW_ORIGIN = "http://localhost:3000"; + mockInit.mockReset(); + }); + + it("returns 400 when body is missing", async () => { + mockInit.mockResolvedValue({ + sessionLoginService: { + executeCallback: jest.fn(), + }, + authCookieSameSite: "Lax", + authCookieSecure: false, + }); + + const { lambdaHandler } = await import("./index"); + + const response = await lambdaHandler({ body: null, headers: {} } as APIGatewayProxyEvent); + + expect(response.statusCode).toBe(400); + expect(response.body).toContain("Body is required"); + }); + + it("returns preview cookies on success without using legacy cookie names or paths", async () => { + mockInit.mockResolvedValue({ + sessionLoginService: { + executeCallback: jest.fn().mockResolvedValue({ + success: true, + result: { + userInfo: { + issuer: "https://id-token.example", + audience: "client-id-123", + subject: "user-123", + familyName: "MILLAR", + givenName: "Mona", + identityProofingLevel: "P9", + email: "test.user@example.com", + emailVerified: true, + phoneNumberVerified: true, + birthDate: "1990-01-01", + nhsNumber: "9999999999", + gpOdsCode: "A12345", + }, + userInfoResponse: { + iss: "https://userinfo.example", + aud: "userinfo-audience", + sub: "user-123", + family_name: "MILLAR", + given_name: "Mona", + identity_proofing_level: "P9", + email: "test.user@example.com", + email_verified: "true", + phone_number_verified: "true", + birthdate: "1990-01-01", + nhs_number: "9999999999", + gp_registration_details: { gp_ods_code: "A12345" }, + }, + sessionId: "550e8400-e29b-41d4-a716-446655440000", + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + sessionCreatedAt: "2026-04-27T10:15:30.000Z", + signedAccessToken: "signed-preview-access", + signedRefreshToken: "signed-preview-refresh", + }, + }), + }, + authCookieSameSite: "Lax", + authCookieSecure: false, + }); + + const { lambdaHandler } = await import("./index"); + + const response = await lambdaHandler({ + body: JSON.stringify({ code: "abc" }), + headers: { + "x-correlation-id": "550e8400-e29b-41d4-a716-446655440000", + }, + } as unknown as APIGatewayProxyEvent); + + expect(response.statusCode).toBe(200); + expect(response.headers?.["X-Correlation-ID"]).toBe("550e8400-e29b-41d4-a716-446655440000"); + expect(response.multiValueHeaders?.["Set-Cookie"]).toEqual([ + "preview_auth=signed-preview-access; HttpOnly; Path=/session-preview; SameSite=Lax;", + "preview_auth_refresh=signed-preview-refresh; HttpOnly; Path=/session-preview/refresh; SameSite=Lax;", + ]); + }); + + it("includes Secure in preview cookies when configured", async () => { + mockInit.mockResolvedValue({ + sessionLoginService: { + executeCallback: jest.fn().mockResolvedValue({ + success: true, + result: { + userInfo: { + issuer: "https://id-token.example", + audience: "client-id-123", + subject: "user-123", + familyName: "MILLAR", + givenName: "Mona", + identityProofingLevel: "P9", + email: "test.user@example.com", + emailVerified: true, + phoneNumberVerified: true, + birthDate: "1990-01-01", + nhsNumber: "9999999999", + gpOdsCode: "A12345", + }, + userInfoResponse: { + iss: "https://userinfo.example", + aud: "userinfo-audience", + sub: "user-123", + family_name: "MILLAR", + given_name: "Mona", + identity_proofing_level: "P9", + email: "test.user@example.com", + email_verified: "true", + phone_number_verified: "true", + birthdate: "1990-01-01", + nhs_number: "9999999999", + gp_registration_details: { gp_ods_code: "A12345" }, + }, + sessionId: "550e8400-e29b-41d4-a716-446655440000", + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + sessionCreatedAt: "2026-04-27T10:15:30.000Z", + signedAccessToken: "signed-preview-access", + signedRefreshToken: "signed-preview-refresh", + }, + }), + }, + authCookieSameSite: "None", + authCookieSecure: true, + }); + + const { lambdaHandler } = await import("./index"); + + const response = await lambdaHandler({ + body: JSON.stringify({ code: "abc" }), + headers: { + "x-correlation-id": "550e8400-e29b-41d4-a716-446655440000", + }, + } as unknown as APIGatewayProxyEvent); + + expect(response.multiValueHeaders?.["Set-Cookie"]).toEqual([ + "preview_auth=signed-preview-access; HttpOnly; Path=/session-preview; SameSite=None; Secure;", + "preview_auth_refresh=signed-preview-refresh; HttpOnly; Path=/session-preview/refresh; SameSite=None; Secure;", + ]); + }); + + it("returns 401 for preview authentication failures", async () => { + mockInit.mockResolvedValue({ + sessionLoginService: { + executeCallback: jest.fn().mockResolvedValue({ + success: false, + error: { + code: "ID_TOKEN_VERIFICATION_FAILED", + message: "Unable to verify NHS identity token", + }, + }), + }, + authCookieSameSite: "Lax", + authCookieSecure: false, + }); + + const { lambdaHandler } = await import("./index"); + + const response = await lambdaHandler({ + body: JSON.stringify({ code: "abc" }), + headers: { + "x-correlation-id": "550e8400-e29b-41d4-a716-446655440000", + }, + } as unknown as APIGatewayProxyEvent); + + expect(response.statusCode).toBe(401); + expect(response.body).toBe(JSON.stringify({ message: "Unable to verify NHS identity token" })); + }); + + it("returns 502 for upstream NHS exchange failures", async () => { + mockInit.mockResolvedValue({ + sessionLoginService: { + executeCallback: jest.fn().mockResolvedValue({ + success: false, + error: { + code: "TOKEN_EXCHANGE_FAILED", + message: "Unable to exchange NHS authorization code", + }, + }), + }, + authCookieSameSite: "Lax", + authCookieSecure: false, + }); + + const { lambdaHandler } = await import("./index"); + + const response = await lambdaHandler({ + body: JSON.stringify({ code: "abc" }), + headers: { + "x-correlation-id": "550e8400-e29b-41d4-a716-446655440000", + }, + } as unknown as APIGatewayProxyEvent); + + expect(response.statusCode).toBe(502); + }); +}); diff --git a/lambdas/src/session-login-lambda/index.ts b/lambdas/src/session-login-lambda/index.ts new file mode 100644 index 000000000..378cb7f64 --- /dev/null +++ b/lambdas/src/session-login-lambda/index.ts @@ -0,0 +1,132 @@ +import { randomUUID } from "node:crypto"; + +import middy from "@middy/core"; +import cors from "@middy/http-cors"; +import httpErrorHandler from "@middy/http-error-handler"; +import httpSecurityHeaders from "@middy/http-security-headers"; +import { type APIGatewayProxyEvent, type APIGatewayProxyResult } from "aws-lambda"; + +import { securityHeaders } from "../lib/http/security-headers"; +import { defaultCorsOptions } from "../lib/security/cors-configuration"; +import { createJsonResponse, getCorrelationIdFromEventHeaders } from "../lib/utils/utils"; +import { generateReadableError } from "../lib/utils/validation-utils"; +import { buildPreviewAccessCookie, buildPreviewRefreshCookie } from "./cookies"; +import { init } from "./init"; +import { type SessionLoginBody, SessionLoginBodySchema } from "./schemas"; +import { type SessionLoginErrorCode } from "./session-login-service"; + +const name = "session-login-lambda"; + +function resolveCorrelationId(event: APIGatewayProxyEvent): string { + try { + return getCorrelationIdFromEventHeaders(event); + } catch (error) { + const correlationId = randomUUID(); + const reason = error instanceof Error ? error.message : "Unknown correlation ID error"; + console.info(name, "Generated fallback correlation ID", { correlationId, reason }); + return correlationId; + } +} + +function parseBody(body: string | null): SessionLoginBody { + if (body === null) { + throw new Error("Body is required"); + } + + let parsedBody: unknown; + + try { + parsedBody = JSON.parse(body); + } catch (error) { + throw new Error("Invalid JSON in request body", { cause: error }); + } + + const validationResult = SessionLoginBodySchema.safeParse(parsedBody); + + if (!validationResult.success) { + throw new Error(`Validation failed: ${generateReadableError(validationResult.error)}`); + } + + return validationResult.data; +} + +function statusCodeForError(code: SessionLoginErrorCode): number { + switch (code) { + case "TOKEN_EXCHANGE_FAILED": + case "USER_INFO_FAILED": + return 502; + case "ID_TOKEN_VERIFICATION_FAILED": + case "ACCESS_TOKEN_VERIFICATION_FAILED": + case "ID_TOKEN_SUB_MISSING": + case "SUBJECT_MISMATCH": + case "ID_TOKEN_ISS_MISSING": + case "ID_TOKEN_AUD_INVALID": + return 401; + case "SESSION_DATA_INVALID": + case "SESSION_PERSIST_FAILED": + default: + return 500; + } +} + +export const lambdaHandler = async ( + event: APIGatewayProxyEvent, +): Promise => { + const correlationId = resolveCorrelationId(event); + let body: SessionLoginBody; + + try { + body = parseBody(event.body); + } catch (error) { + return createJsonResponse( + 400, + { + message: error instanceof Error ? error.message : "Invalid request body", + }, + { "X-Correlation-ID": correlationId }, + ); + } + + const { sessionLoginService, authCookieSameSite, authCookieSecure } = await init(); + const result = await sessionLoginService.executeCallback(body.code); + + if (!result.success) { + console.error(name, "Preview session login failed", { correlationId, code: result.error.code }); + return createJsonResponse( + statusCodeForError(result.error.code), + { message: result.error.message }, + { "X-Correlation-ID": correlationId }, + ); + } + + const response = createJsonResponse( + 200, + { ...result.result.userInfoResponse }, + { + "X-Correlation-ID": correlationId, + }, + ); + + return { + ...response, + multiValueHeaders: { + "Set-Cookie": [ + buildPreviewAccessCookie( + result.result.signedAccessToken, + authCookieSameSite, + authCookieSecure, + ), + buildPreviewRefreshCookie( + result.result.signedRefreshToken, + authCookieSameSite, + authCookieSecure, + ), + ], + }, + }; +}; + +export const handler = middy(lambdaHandler) + .use(httpSecurityHeaders(securityHeaders)) + .use(cors(defaultCorsOptions)) + .use(httpErrorHandler()); diff --git a/lambdas/src/session-login-lambda/init.test.ts b/lambdas/src/session-login-lambda/init.test.ts new file mode 100644 index 000000000..888474d5a --- /dev/null +++ b/lambdas/src/session-login-lambda/init.test.ts @@ -0,0 +1,229 @@ +import type { buildEnvironment } from "./init"; + +export type SessionLoginBuildEnvironmentFn = typeof buildEnvironment; + +const mockSessionLoginRetrieveMandatoryEnvVariable = jest.fn(); +const mockSessionLoginRetrieveOptionalEnvVariable = jest.fn(); +const mockSessionLoginRetrieveOptionalEnvVariableWithDefault = jest.fn(); +const mockSessionLoginGetSecretValue = jest.fn(); +const mockSessionLoginPostgresConfigFromEnv = jest.fn(); + +const mockSessionLoginClientInstance = { + exchangeAuthCodeForTokens: jest.fn(), + getUserInfo: jest.fn(), + fetchPublicKeyById: jest.fn(), +}; +const mockSessionLoginServiceInstance = { executeCallback: jest.fn() }; + +jest.mock("../lib/utils/utils", () => ({ + retrieveMandatoryEnvVariable: (...args: unknown[]) => + mockSessionLoginRetrieveMandatoryEnvVariable(...args), + retrieveOptionalEnvVariable: (...args: unknown[]) => + mockSessionLoginRetrieveOptionalEnvVariable(...args), + retrieveOptionalEnvVariableWithDefault: (...args: unknown[]) => + mockSessionLoginRetrieveOptionalEnvVariableWithDefault(...args), +})); + +jest.mock("../lib/secrets/secrets-manager-client", () => ({ + AwsSecretsClient: jest.fn().mockImplementation(() => ({ + getSecretValue: mockSessionLoginGetSecretValue, + })), +})); + +jest.mock("../lib/db/db-config", () => ({ + postgresConfigFromEnv: (...args: unknown[]) => mockSessionLoginPostgresConfigFromEnv(...args), +})); + +jest.mock("../lib/db/db-client", () => ({ + PostgresDbClient: jest.fn().mockImplementation(() => ({})), +})); + +jest.mock("../lib/db/session-db-client", () => ({ + SessionDbClient: jest.fn().mockImplementation(() => ({ createSession: jest.fn() })), +})); + +jest.mock("../lib/http/http-client", () => ({ + FetchHttpClient: jest.fn().mockImplementation(() => ({})), +})); + +jest.mock("../lib/login/nhs-login-jwt-helper", () => ({ + NhsLoginJwtHelper: jest.fn().mockImplementation(() => ({})), +})); + +jest.mock("../lib/login/nhs-login-client", () => ({ + NhsLoginClient: jest.fn().mockImplementation(() => mockSessionLoginClientInstance), +})); + +jest.mock("../lib/login/nhs-token-verifier", () => ({ + NhsTokenVerifier: jest.fn().mockImplementation(() => ({ verifyIdToken: jest.fn() })), +})); + +jest.mock("../lib/login/nhs-login-service", () => ({ + NhsLoginService: jest.fn().mockImplementation(() => ({ executeCallback: jest.fn() })), +})); + +jest.mock("../lib/auth/session-token-service", () => ({ + SessionTokenService: jest.fn().mockImplementation(() => ({ + signAccessToken: jest.fn(), + signRefreshToken: jest.fn(), + })), +})); + +jest.mock("./session-login-service", () => ({ + SessionLoginService: jest.fn().mockImplementation(() => mockSessionLoginServiceInstance), +})); + +jest.mock("jwks-rsa", () => ({ + JwksClient: jest.fn().mockImplementation(() => ({})), +})); + +describe("session-login-lambda init", () => { + const baseEnvValues: Record = { + AWS_REGION: "eu-west-2", + NHS_LOGIN_BASE_ENDPOINT_URL: "https://nhs-login.example", + NHS_LOGIN_CLIENT_ID: "hometest-preview", + NHS_LOGIN_REDIRECT_URL: "https://preview.example/session-preview/login", + NHS_LOGIN_PRIVATE_KEY_SECRET_NAME: "nhs-login-private-key", + AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME: "preview-cookie-private-key", + AUTH_SESSION_MAX_DURATION_MINUTES: "60", + AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES: "15", + AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES: "120", + AUTH_COOKIE_SAME_SITE: "Lax", + }; + + const mockSessionLoginPostgresConfig = { + user: "test-user", + host: "test-host", + port: 5432, + database: "test-db", + password: jest.fn().mockResolvedValue("test-password"), + }; + + function setEnvVariableMocks(overrides: Record = {}): void { + const values = { ...baseEnvValues, ...overrides }; + + mockSessionLoginRetrieveMandatoryEnvVariable.mockImplementation((key: string) => { + const value = values[key]; + if (!value) { + throw new Error(`Missing environment variable: ${key}`); + } + return value; + }); + mockSessionLoginRetrieveOptionalEnvVariable.mockImplementation((key: string) => values[key]); + mockSessionLoginRetrieveOptionalEnvVariableWithDefault.mockImplementation( + (key: string, defaultValue: string) => values[key] ?? defaultValue, + ); + } + + beforeEach(() => { + jest.resetModules(); + jest.clearAllMocks(); + + setEnvVariableMocks(); + mockSessionLoginGetSecretValue + .mockResolvedValueOnce("test-nhs-login-private-key") + .mockResolvedValueOnce(JSON.stringify({ key: "test-preview-cookie-private-key" })); + mockSessionLoginPostgresConfigFromEnv.mockReturnValue(mockSessionLoginPostgresConfig); + }); + + it("initializes and wires isolated preview dependencies with expected configuration", async () => { + const { buildEnvironment: init } = await import("./init"); + const result = await init(); + + const { AwsSecretsClient } = await import("../lib/secrets/secrets-manager-client"); + const { NhsLoginClient } = await import("../lib/login/nhs-login-client"); + const { NhsTokenVerifier } = await import("../lib/login/nhs-token-verifier"); + const { SessionTokenService } = await import("../lib/auth/session-token-service"); + const { SessionLoginService } = await import("./session-login-service"); + + expect(AwsSecretsClient).toHaveBeenCalledWith("eu-west-2"); + expect(mockSessionLoginGetSecretValue).toHaveBeenNthCalledWith(1, "nhs-login-private-key"); + expect(mockSessionLoginGetSecretValue).toHaveBeenNthCalledWith(2, "preview-cookie-private-key"); + expect(mockSessionLoginPostgresConfigFromEnv).toHaveBeenCalledWith(expect.any(Object)); + expect(NhsLoginClient).toHaveBeenCalledTimes(1); + expect(NhsTokenVerifier).toHaveBeenCalledWith({ + keyProvider: mockSessionLoginClientInstance, + issuer: "https://nhs-login.example", + }); + expect(SessionTokenService).toHaveBeenCalledWith({ + privateKey: "test-preview-cookie-private-key", + accessTokenExpiryDurationMinutes: 15, + refreshTokenExpiryDurationMinutes: 120, + }); + expect(SessionLoginService).toHaveBeenCalledWith({ + nhsLoginService: expect.any(Object), + sessionDbClient: expect.any(Object), + sessionTokenService: expect.any(Object), + sessionMaxDurationMinutes: 60, + }); + expect(result).toEqual({ + sessionLoginService: mockSessionLoginServiceInstance, + authCookieSameSite: "Lax", + authCookieSecure: true, + }); + }); + + it("uses the configured JWKS URI when present", async () => { + setEnvVariableMocks({ NHS_LOGIN_JWKS_URI: "https://nhs-login.example/custom-jwks" }); + + const { buildEnvironment: init } = await import("./init"); + await init(); + + const { JwksClient } = await import("jwks-rsa"); + expect(JwksClient).toHaveBeenCalledWith({ + cache: true, + rateLimit: true, + jwksUri: "https://nhs-login.example/custom-jwks", + }); + }); + + it("throws when a required environment variable is missing", async () => { + setEnvVariableMocks({ AWS_REGION: "" }); + + const { buildEnvironment: init } = await import("./init"); + await expect(init()).rejects.toThrow("Missing environment variable: AWS_REGION"); + }); + + it("throws when a duration is not a positive integer", async () => { + setEnvVariableMocks({ AUTH_SESSION_MAX_DURATION_MINUTES: "0" }); + + const { buildEnvironment: init } = await import("./init"); + await expect(init()).rejects.toThrow( + "AUTH_SESSION_MAX_DURATION_MINUTES must be a positive integer", + ); + }); + + describe("singleton protection", () => { + it("constructs dependencies only once no matter how many times init() is called", async () => { + const { init: singletonInit } = await import("./init"); + + const promise1 = singletonInit(); + const promise2 = singletonInit(); + + const { AwsSecretsClient } = await import("../lib/secrets/secrets-manager-client"); + expect(AwsSecretsClient).toHaveBeenCalledTimes(1); + expect(promise1).toBe(promise2); + }); + }); + + describe("rejection retry", () => { + it("clears the cached environment on rejection so subsequent calls can retry", async () => { + mockSessionLoginGetSecretValue.mockReset(); + mockSessionLoginGetSecretValue + .mockRejectedValueOnce(new Error("Secrets Manager unavailable")) + .mockResolvedValueOnce("test-nhs-login-private-key") + .mockResolvedValueOnce(JSON.stringify({ key: "test-preview-cookie-private-key" })); + + const { init: singletonInit } = await import("./init"); + + await expect(singletonInit()).rejects.toThrow("Secrets Manager unavailable"); + + const result = await singletonInit(); + expect(result).toEqual({ + sessionLoginService: mockSessionLoginServiceInstance, + authCookieSameSite: "Lax", + authCookieSecure: true, + }); + }); + }); +}); diff --git a/lambdas/src/session-login-lambda/init.ts b/lambdas/src/session-login-lambda/init.ts new file mode 100644 index 000000000..24bb6ffa6 --- /dev/null +++ b/lambdas/src/session-login-lambda/init.ts @@ -0,0 +1,176 @@ +import { JwksClient } from "jwks-rsa"; + +import { SessionTokenService } from "../lib/auth/session-token-service"; +import { PostgresDbClient } from "../lib/db/db-client"; +import { postgresConfigFromEnv } from "../lib/db/db-config"; +import { SessionDbClient } from "../lib/db/session-db-client"; +import { FetchHttpClient } from "../lib/http/http-client"; +import { NhsLoginClient } from "../lib/login/nhs-login-client"; +import { NhsLoginJwtHelper } from "../lib/login/nhs-login-jwt-helper"; +import { NhsLoginService } from "../lib/login/nhs-login-service"; +import { NhsTokenVerifier } from "../lib/login/nhs-token-verifier"; +import { type INhsLoginConfig } from "../lib/models/nhs-login/nhs-login-config"; +import { AwsSecretsClient } from "../lib/secrets/secrets-manager-client"; +import { + retrieveMandatoryEnvVariable, + retrieveOptionalEnvVariable, + retrieveOptionalEnvVariableWithDefault, +} from "../lib/utils/utils"; +import { type ISessionLoginService } from "./session-login-service"; +import { SessionLoginService } from "./session-login-service"; + +interface SessionLoginEnvVariables { + awsRegion: string; + nhsLoginBaseEndpointUrl: string; + nhsLoginJwksUri: string | undefined; + nhsLoginClientId: string; + nhsLoginRedirectUrl: string; + nhsLoginPrivateKeySecretName: string; + authCookiePrivateKeysSecretName: string; + authSessionMaxDurationMinutes: number; + authAccessTokenExpiryDurationMinutes: number; + authRefreshTokenExpiryDurationMinutes: number; + authCookieSameSite: string; + authCookieSecure: boolean; +} + +export interface SessionLoginLambdaDependencies { + sessionLoginService: ISessionLoginService; + authCookieSameSite: string; + authCookieSecure: boolean; +} + +function parsePositiveInteger(name: string, value: string): number { + const parsedValue = Number.parseInt(value, 10); + + if (!Number.isInteger(parsedValue) || parsedValue <= 0) { + throw new Error(`${name} must be a positive integer`); + } + + return parsedValue; +} + +function parseAuthCookiePrivateKey(secretValue: string): string { + try { + const parsedSecret = JSON.parse(secretValue) as Record; + const privateKey = parsedSecret.key; + + if (typeof privateKey === "string" && privateKey.trim().length > 0) { + return privateKey; + } + + throw new Error( + "AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME must be either a non-JSON private key string or a JSON object containing a non-empty 'key' entry.", + ); + } catch (error) { + if (!(error instanceof SyntaxError)) { + throw error; + } + } + + if (secretValue.trim().length === 0) { + throw new Error("AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME secret value must not be empty."); + } + + return secretValue; +} + +function loadEnv(): SessionLoginEnvVariables { + return { + awsRegion: retrieveMandatoryEnvVariable("AWS_REGION"), + nhsLoginBaseEndpointUrl: retrieveMandatoryEnvVariable("NHS_LOGIN_BASE_ENDPOINT_URL"), + nhsLoginJwksUri: retrieveOptionalEnvVariable("NHS_LOGIN_JWKS_URI"), + nhsLoginClientId: retrieveMandatoryEnvVariable("NHS_LOGIN_CLIENT_ID"), + nhsLoginRedirectUrl: retrieveMandatoryEnvVariable("NHS_LOGIN_REDIRECT_URL"), + nhsLoginPrivateKeySecretName: retrieveMandatoryEnvVariable("NHS_LOGIN_PRIVATE_KEY_SECRET_NAME"), + authCookiePrivateKeysSecretName: retrieveMandatoryEnvVariable( + "AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME", + ), + authSessionMaxDurationMinutes: parsePositiveInteger( + "AUTH_SESSION_MAX_DURATION_MINUTES", + retrieveMandatoryEnvVariable("AUTH_SESSION_MAX_DURATION_MINUTES"), + ), + authAccessTokenExpiryDurationMinutes: parsePositiveInteger( + "AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES", + retrieveMandatoryEnvVariable("AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES"), + ), + authRefreshTokenExpiryDurationMinutes: parsePositiveInteger( + "AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES", + retrieveMandatoryEnvVariable("AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES"), + ), + authCookieSameSite: retrieveMandatoryEnvVariable("AUTH_COOKIE_SAME_SITE"), + authCookieSecure: + retrieveOptionalEnvVariableWithDefault("AUTH_COOKIE_SECURE", "true").toLowerCase() === "true", + }; +} + +export async function buildEnvironment(): Promise { + const envVars = loadEnv(); + const secretsClient = new AwsSecretsClient(envVars.awsRegion); + const nhsLoginPrivateKey = await secretsClient.getSecretValue( + envVars.nhsLoginPrivateKeySecretName, + ); + const authCookiePrivateKeySecret = await secretsClient.getSecretValue( + envVars.authCookiePrivateKeysSecretName, + ); + + const nhsLoginConfig: INhsLoginConfig = { + clientId: envVars.nhsLoginClientId, + expiresIn: 60, + redirectUri: envVars.nhsLoginRedirectUrl, + baseUri: envVars.nhsLoginBaseEndpointUrl, + privateKey: nhsLoginPrivateKey, + jwksUri: envVars.nhsLoginJwksUri || undefined, + }; + + const nhsLoginJwtHelper = new NhsLoginJwtHelper(nhsLoginConfig); + const httpClient = new FetchHttpClient(); + const jwksClient = new JwksClient({ + cache: true, + rateLimit: true, + jwksUri: nhsLoginConfig.jwksUri ?? `${nhsLoginConfig.baseUri}/.well-known/jwks.json`, + }); + const nhsLoginClient = new NhsLoginClient( + nhsLoginConfig, + nhsLoginJwtHelper, + httpClient, + jwksClient, + ); + const nhsTokenVerifier = new NhsTokenVerifier({ + keyProvider: nhsLoginClient, + issuer: nhsLoginConfig.baseUri, + }); + const nhsLoginService = new NhsLoginService({ + nhsTokenVerifier, + nhsLoginClient, + }); + const dbClient = new PostgresDbClient(postgresConfigFromEnv(secretsClient)); + const sessionDbClient = new SessionDbClient(dbClient); + const sessionTokenService = new SessionTokenService({ + privateKey: parseAuthCookiePrivateKey(authCookiePrivateKeySecret), + accessTokenExpiryDurationMinutes: envVars.authAccessTokenExpiryDurationMinutes, + refreshTokenExpiryDurationMinutes: envVars.authRefreshTokenExpiryDurationMinutes, + }); + const sessionLoginService = new SessionLoginService({ + nhsLoginService, + sessionDbClient, + sessionTokenService, + sessionMaxDurationMinutes: envVars.authSessionMaxDurationMinutes, + }); + + return { + sessionLoginService, + authCookieSameSite: envVars.authCookieSameSite, + authCookieSecure: envVars.authCookieSecure, + }; +} + +let _env: Promise | undefined; + +export function init(): Promise { + _env ??= buildEnvironment().catch((error) => { + _env = undefined; + throw error; + }); + return _env; +} diff --git a/lambdas/src/session-login-lambda/schemas.ts b/lambdas/src/session-login-lambda/schemas.ts new file mode 100644 index 000000000..31eac973c --- /dev/null +++ b/lambdas/src/session-login-lambda/schemas.ts @@ -0,0 +1,7 @@ +import { z } from "zod"; + +export const SessionLoginBodySchema = z.object({ + code: z.string().min(1, "code is required"), +}); + +export type SessionLoginBody = z.infer; diff --git a/lambdas/src/session-login-lambda/session-login-service.test.ts b/lambdas/src/session-login-lambda/session-login-service.test.ts new file mode 100644 index 000000000..10bdb1d9d --- /dev/null +++ b/lambdas/src/session-login-lambda/session-login-service.test.ts @@ -0,0 +1,275 @@ +import { type ISessionTokenService } from "../lib/auth/session-token-service"; +import { type SessionDbClient } from "../lib/db/session-db-client"; +import { + type INhsLoginService, + type NhsLoginExecutionResult, +} from "../lib/login/nhs-login-service"; +import { SessionLoginService } from "./session-login-service"; + +function createNhsLoginSuccess( + overrides: Partial["result"]> = {}, +): Extract { + return { + success: true, + result: { + userInfo: { + iss: "https://userinfo.example", + aud: "userinfo-audience", + sub: "user-123", + family_name: "MILLAR", + given_name: "Mona", + identity_proofing_level: "P9", + email: "test.user@example.com", + email_verified: "true", + phone_number_verified: "false", + birthdate: "1990-01-01", + nhs_number: "9999999999", + gp_registration_details: { + gp_ods_code: "A12345", + }, + }, + nhsAccessToken: "nhs-access-token", + nhsRefreshToken: "nhs-refresh-token", + idTokenSubject: "user-123", + idTokenIssuer: "https://id-token.example", + idTokenAudience: "client-id-123", + ...overrides, + }, + }; +} + +describe("SessionLoginService.executeCallback", () => { + let nhsLoginServiceMock: jest.Mocked; + let sessionDbClientMock: jest.Mocked>; + let sessionTokenServiceMock: jest.Mocked; + let uuidGeneratorMock: jest.Mock; + const fixedNow = new Date("2026-04-27T10:15:30.000Z"); + + beforeEach(() => { + nhsLoginServiceMock = { + executeCallback: jest.fn(), + }; + + sessionDbClientMock = { + createSession: jest.fn(), + }; + + sessionTokenServiceMock = { + signAccessToken: jest.fn().mockReturnValue("signed-preview-access"), + signRefreshToken: jest.fn().mockReturnValue("signed-preview-refresh"), + }; + + uuidGeneratorMock = jest + .fn() + .mockReturnValueOnce("550e8400-e29b-41d4-a716-446655440000") + .mockReturnValueOnce("650e8400-e29b-41d4-a716-446655440000"); + }); + + it("creates a preview session and signs session tokens with isolated claims", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue(createNhsLoginSuccess()); + sessionDbClientMock.createSession.mockResolvedValue({ + sessionId: "550e8400-e29b-41d4-a716-446655440000", + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + nhsAccessToken: "nhs-access-token", + userInfo: { + issuer: "https://id-token.example", + audience: "client-id-123", + subject: "user-123", + familyName: "MILLAR", + givenName: "Mona", + identityProofingLevel: "P9", + email: "test.user@example.com", + emailVerified: true, + phoneNumberVerified: false, + birthDate: "1990-01-01", + nhsNumber: "9999999999", + gpOdsCode: "A12345", + }, + sessionCreatedAt: fixedNow.toISOString(), + lastRefreshAt: fixedNow.toISOString(), + maxExpiresAt: "2026-04-27T11:15:30.000Z", + }); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + uuidGenerator: uuidGeneratorMock, + clock: () => fixedNow, + }); + + const result = await service.executeCallback("auth-code"); + + expect(sessionDbClientMock.createSession).toHaveBeenCalledWith({ + sessionId: "550e8400-e29b-41d4-a716-446655440000", + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + nhsAccessToken: "nhs-access-token", + userInfo: { + issuer: "https://id-token.example", + audience: "client-id-123", + subject: "user-123", + familyName: "MILLAR", + givenName: "Mona", + identityProofingLevel: "P9", + email: "test.user@example.com", + emailVerified: true, + phoneNumberVerified: false, + birthDate: "1990-01-01", + nhsNumber: "9999999999", + gpOdsCode: "A12345", + }, + sessionCreatedAt: fixedNow.toISOString(), + lastRefreshAt: fixedNow.toISOString(), + maxExpiresAt: "2026-04-27T11:15:30.000Z", + }); + expect(sessionTokenServiceMock.signAccessToken).toHaveBeenCalledWith({ + sessionId: "550e8400-e29b-41d4-a716-446655440000", + sessionCreatedAt: fixedNow.toISOString(), + }); + expect(sessionTokenServiceMock.signRefreshToken).toHaveBeenCalledWith({ + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + }); + expect(result).toEqual({ + success: true, + result: { + userInfo: { + issuer: "https://id-token.example", + audience: "client-id-123", + subject: "user-123", + familyName: "MILLAR", + givenName: "Mona", + identityProofingLevel: "P9", + email: "test.user@example.com", + emailVerified: true, + phoneNumberVerified: false, + birthDate: "1990-01-01", + nhsNumber: "9999999999", + gpOdsCode: "A12345", + }, + userInfoResponse: createNhsLoginSuccess().result.userInfo, + sessionId: "550e8400-e29b-41d4-a716-446655440000", + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + sessionCreatedAt: fixedNow.toISOString(), + signedAccessToken: "signed-preview-access", + signedRefreshToken: "signed-preview-refresh", + }, + }); + }); + + it("passes through NHS login failures", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue({ + success: false, + error: { + code: "TOKEN_EXCHANGE_FAILED", + message: "Unable to exchange NHS authorization code", + }, + }); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + }); + + await expect(service.executeCallback("bad-code")).resolves.toEqual({ + success: false, + error: { + code: "TOKEN_EXCHANGE_FAILED", + message: "Unable to exchange NHS authorization code", + }, + }); + expect(sessionDbClientMock.createSession).not.toHaveBeenCalled(); + }); + + it("fails when the verified ID token issuer is missing", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue( + createNhsLoginSuccess({ idTokenIssuer: " " }), + ); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + }); + + await expect(service.executeCallback("auth-code")).resolves.toEqual({ + success: false, + error: { + code: "ID_TOKEN_ISS_MISSING", + message: "NHS identity token is missing a required issuer claim", + }, + }); + }); + + it("fails when the verified ID token audience is not a single string", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue( + createNhsLoginSuccess({ idTokenAudience: ["client-id-123"] }), + ); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + }); + + await expect(service.executeCallback("auth-code")).resolves.toEqual({ + success: false, + error: { + code: "ID_TOKEN_AUD_INVALID", + message: "NHS identity token has an invalid audience claim", + }, + }); + }); + + it("fails when the NHS number does not satisfy the session schema constraint", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue( + createNhsLoginSuccess({ + userInfo: { + ...createNhsLoginSuccess().result.userInfo, + nhs_number: "ABC", + }, + }), + ); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + }); + + await expect(service.executeCallback("auth-code")).resolves.toEqual({ + success: false, + error: { + code: "SESSION_DATA_INVALID", + message: "NHS user information is missing required session fields", + }, + }); + }); + + it("returns SESSION_PERSIST_FAILED when Aurora persistence fails", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue(createNhsLoginSuccess()); + sessionDbClientMock.createSession.mockRejectedValue(new Error("db down")); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + uuidGenerator: uuidGeneratorMock, + clock: () => fixedNow, + }); + + await expect(service.executeCallback("auth-code")).resolves.toEqual({ + success: false, + error: { + code: "SESSION_PERSIST_FAILED", + message: "Unable to create a preview session after successful NHS Login authentication", + }, + }); + }); +}); diff --git a/lambdas/src/session-login-lambda/session-login-service.ts b/lambdas/src/session-login-lambda/session-login-service.ts new file mode 100644 index 000000000..6892493fb --- /dev/null +++ b/lambdas/src/session-login-lambda/session-login-service.ts @@ -0,0 +1,202 @@ +import { randomUUID } from "node:crypto"; + +import { type ISessionTokenService } from "../lib/auth/session-token-service"; +import { type CreateSessionInput, type SessionDbClient } from "../lib/db/session-db-client"; +import { type INhsLoginService, type NhsLoginErrorCode } from "../lib/login/nhs-login-service"; +import { type INhsUserInfoResponseModel } from "../lib/models/nhs-login/nhs-login-user-info-response-model"; +import { mapNhsUserInfoToSessionUserInfo } from "../lib/models/session/session"; + +export interface SessionLoginSuccessResult { + userInfo: CreateSessionInput["userInfo"]; + userInfoResponse: INhsUserInfoResponseModel; + sessionId: string; + refreshTokenId: string; + sessionCreatedAt: string; + signedAccessToken: string; + signedRefreshToken: string; +} + +export type SessionLoginErrorCode = + | NhsLoginErrorCode + | "ID_TOKEN_ISS_MISSING" + | "ID_TOKEN_AUD_INVALID" + | "SESSION_DATA_INVALID" + | "SESSION_PERSIST_FAILED"; + +export interface SessionLoginError { + code: SessionLoginErrorCode; + message: string; +} + +export interface SessionLoginSuccess { + success: true; + result: SessionLoginSuccessResult; +} + +export interface SessionLoginFailure { + success: false; + error: SessionLoginError; +} + +export type SessionLoginExecutionResult = SessionLoginSuccess | SessionLoginFailure; + +export interface ISessionLoginService { + executeCallback: (code: string) => Promise; +} + +export type SessionLoginSessionDbClient = Pick; + +export interface SessionLoginServiceParams { + nhsLoginService: INhsLoginService; + sessionDbClient: SessionLoginSessionDbClient; + sessionTokenService: ISessionTokenService; + sessionMaxDurationMinutes: number; + uuidGenerator?: () => string; + clock?: () => Date; +} + +export class SessionLoginService implements ISessionLoginService { + private readonly nhsLoginService: INhsLoginService; + private readonly sessionDbClient: SessionLoginSessionDbClient; + private readonly sessionTokenService: ISessionTokenService; + private readonly sessionMaxDurationMinutes: number; + private readonly uuidGenerator: () => string; + private readonly clock: () => Date; + + constructor(params: SessionLoginServiceParams) { + this.nhsLoginService = params.nhsLoginService; + this.sessionDbClient = params.sessionDbClient; + this.sessionTokenService = params.sessionTokenService; + this.sessionMaxDurationMinutes = params.sessionMaxDurationMinutes; + this.uuidGenerator = params.uuidGenerator ?? randomUUID; // dependency injection allows for easier testing, but default to crypto.randomUUID if not provided + this.clock = params.clock ?? (() => new Date()); // dependency injection allows for easier testing, but default to current time if not provided + } + + public async executeCallback(code: string): Promise { + const nhsLoginResult = await this.nhsLoginService.executeCallback(code); + + if (!nhsLoginResult.success) { + return nhsLoginResult; + } + + const idTokenIssuer = nhsLoginResult.result.idTokenIssuer?.trim(); + + if (!idTokenIssuer) { + return this.failure( + "ID_TOKEN_ISS_MISSING", + "NHS identity token is missing a required issuer claim", + ); + } + + const idTokenAudience = this.normalizeAudience(nhsLoginResult.result.idTokenAudience); + + if (!idTokenAudience) { + return this.failure( + "ID_TOKEN_AUD_INVALID", + "NHS identity token has an invalid audience claim", + ); + } + + const sessionId = this.uuidGenerator(); + const refreshTokenId = this.uuidGenerator(); + const sessionCreatedAt = this.clock().toISOString(); + const lastRefreshAt = sessionCreatedAt; + const maxExpiresAt = new Date( + Date.parse(sessionCreatedAt) + this.sessionMaxDurationMinutes * 60_000, + ).toISOString(); + + if (!(Date.parse(maxExpiresAt) > Date.parse(sessionCreatedAt))) { + return this.failure( + "SESSION_DATA_INVALID", + "Session expiry configuration produced an invalid maximum expiry time", + ); + } + + const userInfo = mapNhsUserInfoToSessionUserInfo(nhsLoginResult.result.userInfo); + userInfo.issuer = idTokenIssuer; + userInfo.audience = idTokenAudience; + + if (!this.isValidSessionUserInfo(userInfo)) { + return this.failure( + "SESSION_DATA_INVALID", + "NHS user information is missing required session fields", + ); + } + + try { + await this.sessionDbClient.createSession({ + sessionId, + refreshTokenId, + nhsAccessToken: nhsLoginResult.result.nhsAccessToken, + userInfo, + sessionCreatedAt, + lastRefreshAt, + maxExpiresAt, + }); + } catch { + return this.failure( + "SESSION_PERSIST_FAILED", + "Unable to create a preview session after successful NHS Login authentication", + ); + } + + const signedAccessToken = this.sessionTokenService.signAccessToken({ + sessionId, + sessionCreatedAt, + }); + const signedRefreshToken = this.sessionTokenService.signRefreshToken({ + refreshTokenId, + }); + + return { + success: true, + result: { + userInfo, + userInfoResponse: nhsLoginResult.result.userInfo, + sessionId, + refreshTokenId, + sessionCreatedAt, + signedAccessToken, + signedRefreshToken, + }, + }; + } + + private normalizeAudience(audience: string | string[] | undefined): string | undefined { + if (typeof audience !== "string") { + return undefined; + } + + const trimmedAudience = audience.trim(); + return trimmedAudience.length > 0 ? trimmedAudience : undefined; + } + + private isValidSessionUserInfo(userInfo: CreateSessionInput["userInfo"]): boolean { + return ( + this.hasValue(userInfo.issuer) && + this.hasValue(userInfo.audience) && + this.hasValue(userInfo.subject) && + this.hasValue(userInfo.familyName) && + this.hasValue(userInfo.givenName) && + this.hasValue(userInfo.identityProofingLevel) && + this.hasValue(userInfo.email) && + this.hasValue(userInfo.birthDate) && + this.hasValue(userInfo.gpOdsCode) && + /^[0-9]{10}$/.test(userInfo.nhsNumber) + ); + } + + private hasValue(value: string): boolean { + return value.trim().length > 0; + } + + private failure(code: SessionLoginErrorCode, message: string): SessionLoginFailure { + return { + success: false, + error: { + code, + message, + }, + }; + } +} diff --git a/local-environment/infra/main.tf b/local-environment/infra/main.tf index 65e07e99b..c12c349d2 100644 --- a/local-environment/infra/main.tf +++ b/local-environment/infra/main.tf @@ -260,6 +260,51 @@ module "login_lambda" { } } +module "session_login_lambda" { + source = "./modules/lambda" + + project_name = var.project_name + function_name = "session-login" + zip_path = "${path.module}/../../lambdas/dist/session-login-lambda.zip" + lambda_role_arn = aws_iam_role.lambda_role.arn + environment = var.environment + api_gateway_id = aws_api_gateway_rest_api.api.id + api_gateway_root_resource_id = aws_api_gateway_rest_api.api.root_resource_id + api_gateway_execution_arn = aws_api_gateway_rest_api.api.execution_arn + api_path = "session-preview/login" + lambda_role_policy_attachment = aws_iam_role_policy_attachment.lambda_basic + http_method = "POST" + timeout = 30 + + enable_cors = true + cors_allow_origin = "http://localhost:3000" + cors_allow_methods = ["POST", "OPTIONS"] + cors_allow_headers = ["Content-Type", "Authorization", "X-Correlation-ID"] + cors_allow_credentials = true + + environment_variables = { + NODE_OPTIONS = "--enable-source-maps" + ALLOW_ORIGIN = "http://localhost:3000" + NHS_LOGIN_BASE_ENDPOINT_URL = local.resolved_nhs_login_base_url + NHS_LOGIN_CLIENT_ID = var.local_nhs_login_client_id + NHS_LOGIN_REDIRECT_URL = var.local_nhs_login_redirect_url + NHS_LOGIN_PRIVATE_KEY_SECRET_NAME = var.local_nhs_login_private_key_secret_name + AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME = var.local_auth_cookie_private_keys_secret_name + AUTH_SESSION_MAX_DURATION_MINUTES = var.local_auth_session_max_duration_minutes + AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES = var.local_auth_access_token_expiry_duration_minutes + AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES = var.local_auth_refresh_token_expiry_duration_minutes + AUTH_COOKIE_SAME_SITE = var.local_auth_cookie_same_site + AUTH_COOKIE_SECURE = var.local_auth_cookie_secure + DB_USERNAME = "app_user" + DB_ADDRESS = "postgres-db" + DB_PORT = "5432" + DB_NAME = "local_hometest_db" + DB_SCHEMA = "hometest" + DB_SECRET_NAME = "postgres-db-password" + DB_SSL = "false" + } +} + module "session_lambda" { source = "./modules/lambda" diff --git a/package.json b/package.json index 7882566f1..07b5bd716 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,7 @@ "postinstall": "pnpm -C ui install --frozen-lockfile && pnpm -C lambdas install --frozen-lockfile && pnpm -C tests install --frozen-lockfile", "test": "pnpm -C ui run test && pnpm -C lambdas run test", "test:playwright": "UI_BASE_URL=$(terraform -chdir=local-environment/infra output -raw ui_url) API_BASE_URL=$(terraform -chdir=local-environment/infra output -raw api_base_url) pnpm -C tests run test:chrome", + "test:postman:session-login-preview": "API_GATEWAY_ID=$(terraform -chdir=local-environment/infra output -raw api_gateway_id) && pnpm dlx newman run postman/session_login_preview_wiremock.postman_collection.json -e postman/session_login_preview_wiremock.local.postman_environment.json --env-var api_gateway_id=$API_GATEWAY_ID --env-var execute_api_base_localstack=http://localhost:4566/_aws/execute-api/$API_GATEWAY_ID/local --env-var session_login_endpoint_localstack=http://localhost:4566/_aws/execute-api/$API_GATEWAY_ID/local/session-preview/login --reporters cli", "format": "(git diff --name-only --diff-filter=ACMR -z; git diff --name-only --cached --diff-filter=ACMR -z) | sort -zu | xargs -0 sh -c '[ \"$#\" -gt 0 ] && pnpm run format:pre-commit -- \"$@\"' sh", "format:pre-commit": "prettier --write --ignore-unknown", "lint": "pnpm -C ui run lint && pnpm -C lambdas run lint && pnpm -C tests run lint", diff --git a/postman/README.md b/postman/README.md new file mode 100644 index 000000000..f11f2e80b --- /dev/null +++ b/postman/README.md @@ -0,0 +1,51 @@ +# Postman Collections + +This directory contains manual API collections for local and upstream testing. + +## Preview Session Login With WireMock + +Use these files together: + +- `session_login_preview_wiremock.postman_collection.json` +- `session_login_preview_wiremock.local.postman_environment.json` + +This collection exercises the preview-only `session-login-lambda` on the local LocalStack API while NHS Login upstreams are mocked by WireMock on `http://localhost:8080`. + +### Manual Postman Usage + +1. Start the local environment so LocalStack, WireMock, and the lambdas are running. +2. Import the collection and the local environment file. +3. Refresh the `api_gateway_id` in the imported environment if you have recreated the local Terraform stack. +4. Run `Capture Mock Authorisation Code` if you want Postman to fetch a fresh mock code from WireMock. +5. Run `200: success with mock auth code` to hit the preview route. + +### Keeping `api_gateway_id` Up To Date + +The environment is set up so only `api_gateway_id` needs manual refresh. +`execute_api_base_localstack` and `session_login_endpoint_localstack` are derived from it. + +To get the current value from the active local stack, run: + +```bash +terraform -chdir=local-environment/infra output -raw api_gateway_id +``` + +If you are at the repository root and prefer the repo script toolchain, this is equivalent: + +```bash +pnpm exec terraform -chdir=local-environment/infra output -raw api_gateway_id +``` + +Update the imported Postman environment variable `api_gateway_id` with that output whenever you run `pnpm run local:terraform:apply`, `pnpm run local:start`, or any other workflow that recreates the API Gateway. + +If you do not recreate LocalStack Terraform resources, the value usually stays stable and does not need changing between manual runs. + +### CLI Alternative + +For a non-manual run, use: + +```bash +pnpm run test:postman:session-login-preview +``` + +That script resolves the current `api_gateway_id` from Terraform at runtime and passes the correct endpoint values to Newman, so you do not need to edit the environment file first. diff --git a/postman/session_login_preview_wiremock.local.postman_environment.json b/postman/session_login_preview_wiremock.local.postman_environment.json new file mode 100644 index 000000000..817212b15 --- /dev/null +++ b/postman/session_login_preview_wiremock.local.postman_environment.json @@ -0,0 +1,75 @@ +{ + "id": "b6ff8bb4-a31a-4d76-b3f9-783c6da99557", + "name": "Session Login Preview Local (WireMock)", + "values": [ + { + "key": "api_gateway_id", + "value": "4lshw3ry0l", + "type": "default", + "enabled": true + }, + { + "key": "wiremock_nhs_login_base_url", + "value": "http://localhost:8080", + "type": "default", + "enabled": true + }, + { + "key": "execute_api_base_localstack", + "value": "http://localhost:4566/_aws/execute-api/{{api_gateway_id}}/local", + "type": "default", + "enabled": true + }, + { + "key": "session_login_endpoint_localstack", + "value": "{{execute_api_base_localstack}}/session-preview/login", + "type": "default", + "enabled": true + }, + { + "key": "nhs_login_client_id", + "value": "hometest", + "type": "default", + "enabled": true + }, + { + "key": "nhs_login_scope", + "value": "openid profile email phone nhs_number", + "type": "default", + "enabled": true + }, + { + "key": "nhs_login_redirect_url", + "value": "http://localhost:3000/callback", + "type": "default", + "enabled": true + }, + { + "key": "oauth_state", + "value": "preview-session-login-state", + "type": "default", + "enabled": true + }, + { + "key": "oauth_state_returned", + "value": "", + "type": "default", + "enabled": true + }, + { + "key": "wiremock_auth_code", + "value": "wiremock-auth-code", + "type": "default", + "enabled": true + }, + { + "key": "correlation_id", + "value": "550e8400-e29b-41d4-a716-446655440000", + "type": "default", + "enabled": true + } + ], + "_postman_variable_scope": "environment", + "_postman_exported_at": "2026-04-27T12:00:00.000Z", + "_postman_exported_using": "GitHub Copilot" +} diff --git a/postman/session_login_preview_wiremock.postman_collection.json b/postman/session_login_preview_wiremock.postman_collection.json new file mode 100644 index 000000000..f83956d3f --- /dev/null +++ b/postman/session_login_preview_wiremock.postman_collection.json @@ -0,0 +1,356 @@ +{ + "info": { + "_postman_id": "7f0a54d3-0373-4b4b-8d79-3b1cd83ba5ab", + "name": "Session Login Preview (WireMock)", + "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json", + "description": "Exercises the preview-only session-login-lambda against the local LocalStack API while NHS Login upstreams are stubbed by WireMock. Populate api_gateway_id from local-environment/infra outputs or terraform state, then run the authorize request first to refresh the mock auth code if needed." + }, + "item": [ + { + "name": "WireMock NHS Login", + "item": [ + { + "name": "Capture Mock Authorisation Code", + "request": { + "auth": { + "type": "noauth" + }, + "method": "GET", + "header": [], + "url": { + "raw": "{{wiremock_nhs_login_base_url}}/authorize?client_id={{nhs_login_client_id}}&response_type=code&scope={{nhs_login_scope}}&redirect_uri={{nhs_login_redirect_url}}&state={{oauth_state}}", + "host": [ + "{{wiremock_nhs_login_base_url}}" + ], + "path": [ + "authorize" + ], + "query": [ + { + "key": "client_id", + "value": "{{nhs_login_client_id}}" + }, + { + "key": "response_type", + "value": "code" + }, + { + "key": "scope", + "value": "{{nhs_login_scope}}" + }, + { + "key": "redirect_uri", + "value": "{{nhs_login_redirect_url}}" + }, + { + "key": "state", + "value": "{{oauth_state}}" + } + ] + }, + "description": "Calls the WireMock /authorize stub directly and captures the redirected code into the wiremock_auth_code collection variable." + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test(\"WireMock authorize returns a redirect\", function () {", + " pm.response.to.have.status(302);", + "});", + "", + "const redirectLocation = pm.response.headers.get(\"Location\");", + "pm.expect(redirectLocation, \"Location header\").to.be.a(\"string\");", + "", + "const codeMatch = redirectLocation.match(/[?&]code=([^&]+)/);", + "pm.expect(codeMatch, \"authorization code in redirect\").to.not.equal(null);", + "if (codeMatch) {", + " pm.collectionVariables.set(\"wiremock_auth_code\", decodeURIComponent(codeMatch[1]));", + "}", + "", + "const stateMatch = redirectLocation.match(/[?&]state=([^&]+)/);", + "if (stateMatch) {", + " pm.collectionVariables.set(\"oauth_state_returned\", decodeURIComponent(stateMatch[1]));", + "}", + "" + ] + } + } + ], + "protocolProfileBehavior": { + "followRedirects": false + } + } + ] + }, + { + "name": "Preview Session Login Lambda", + "item": [ + { + "name": "200: success with mock auth code", + "request": { + "auth": { + "type": "noauth" + }, + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json", + "type": "text" + }, + { + "key": "X-Correlation-ID", + "value": "{{correlation_id}}", + "type": "text" + } + ], + "body": { + "mode": "raw", + "raw": "{\n \"code\": \"{{wiremock_auth_code}}\"\n}", + "options": { + "raw": { + "language": "json" + } + } + }, + "url": { + "raw": "{{session_login_endpoint_localstack}}", + "host": [ + "{{session_login_endpoint_localstack}}" + ] + }, + "description": "Posts the captured mock NHS Login code to the preview-only session-login-lambda. By default wiremock_auth_code is pre-seeded with wiremock-auth-code, so this request also works without the authorize step." + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test(\"Preview session login returns 200\", function () {", + " pm.response.to.have.status(200);", + "});", + "", + "pm.test(\"Correlation ID is echoed\", function () {", + " pm.expect(pm.response.headers.get(\"X-Correlation-ID\")).to.eql(pm.collectionVariables.get(\"correlation_id\"));", + "});", + "", + "const body = pm.response.json();", + "pm.test(\"User info payload is returned\", function () {", + " pm.expect(body.sub).to.eql(\"test-sub-123\");", + " pm.expect(body.nhs_number).to.eql(\"9912003071\");", + " pm.expect(body.gp_registration_details.gp_ods_code).to.eql(\"Y12345\");", + "});", + "", + "const setCookieHeaders = pm.response.headers", + " .all()", + " .filter((header) => header.key.toLowerCase() === \"set-cookie\")", + " .map((header) => header.value);", + "", + "pm.test(\"Preview cookies are set on isolated names and paths\", function () {", + " pm.expect(setCookieHeaders.some((value) => value.includes(\"preview_auth=\") && value.includes(\"Path=/session-preview\"))).to.eql(true);", + " pm.expect(setCookieHeaders.some((value) => value.includes(\"preview_auth_refresh=\") && value.includes(\"Path=/session-preview/refresh\"))).to.eql(true);", + "});", + "" + ] + } + } + ] + }, + { + "name": "400: missing code", + "request": { + "auth": { + "type": "noauth" + }, + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json", + "type": "text" + }, + { + "key": "X-Correlation-ID", + "value": "{{correlation_id}}", + "type": "text" + } + ], + "body": { + "mode": "raw", + "raw": "{}", + "options": { + "raw": { + "language": "json" + } + } + }, + "url": { + "raw": "{{session_login_endpoint_localstack}}", + "host": [ + "{{session_login_endpoint_localstack}}" + ] + } + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test(\"Missing code returns 400\", function () {", + " pm.response.to.have.status(400);", + "});", + "", + "const body = pm.response.json();", + "pm.test(\"Validation message is returned\", function () {", + " pm.expect(body.message).to.include(\"Validation failed\");", + "});", + "" + ] + } + } + ] + }, + { + "name": "400: invalid JSON", + "request": { + "auth": { + "type": "noauth" + }, + "method": "POST", + "header": [ + { + "key": "Content-Type", + "value": "application/json", + "type": "text" + }, + { + "key": "X-Correlation-ID", + "value": "{{correlation_id}}", + "type": "text" + } + ], + "body": { + "mode": "raw", + "raw": "{ \"code\": }", + "options": { + "raw": { + "language": "json" + } + } + }, + "url": { + "raw": "{{session_login_endpoint_localstack}}", + "host": [ + "{{session_login_endpoint_localstack}}" + ] + } + }, + "response": [], + "event": [ + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "pm.test(\"Invalid JSON returns 400\", function () {", + " pm.response.to.have.status(400);", + "});", + "", + "const body = pm.response.json();", + "pm.test(\"JSON parsing message is returned\", function () {", + " pm.expect(body.message).to.eql(\"Invalid JSON in request body\");", + "});", + "" + ] + } + } + ] + } + ] + } + ], + "event": [ + { + "listen": "prerequest", + "script": { + "type": "text/javascript", + "exec": [ + "" + ] + } + }, + { + "listen": "test", + "script": { + "type": "text/javascript", + "exec": [ + "" + ] + } + } + ], + "variable": [ + { + "key": "api_gateway_id", + "value": "", + "type": "string" + }, + { + "key": "wiremock_nhs_login_base_url", + "value": "http://localhost:8080", + "type": "string" + }, + { + "key": "execute_api_base_localstack", + "value": "http://localhost:4566/_aws/execute-api/{{api_gateway_id}}/local", + "type": "string" + }, + { + "key": "session_login_endpoint_localstack", + "value": "{{execute_api_base_localstack}}/session-preview/login", + "type": "string" + }, + { + "key": "nhs_login_client_id", + "value": "hometest", + "type": "string" + }, + { + "key": "nhs_login_scope", + "value": "openid profile email phone nhs_number", + "type": "string" + }, + { + "key": "nhs_login_redirect_url", + "value": "http://localhost:3000/callback", + "type": "string" + }, + { + "key": "oauth_state", + "value": "preview-session-login-state", + "type": "string" + }, + { + "key": "oauth_state_returned", + "value": "", + "type": "string" + }, + { + "key": "wiremock_auth_code", + "value": "wiremock-auth-code", + "type": "string" + }, + { + "key": "correlation_id", + "value": "550e8400-e29b-41d4-a716-446655440000", + "type": "string" + } + ] +} diff --git a/scripts/config/gitleaks.toml b/scripts/config/gitleaks.toml index 3bdaf7ab0..a0d711fc1 100644 --- a/scripts/config/gitleaks.toml +++ b/scripts/config/gitleaks.toml @@ -27,7 +27,7 @@ paths = [ '''tests/.session-cache/*''', '''tests/WorkerUserSession*''', '''.session-cache/*''', - '''ui/.next/*''', '''ui/build/*''', + '''ui/.next/*''', '''ui/build/*''', '''ui/.pnpm-store/*''', '''cdk.out/*''', '''.idea/*''' ] From c0dbd8efc74f3187d9aff92c01cd4f1883ae6c1d Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 10:51:19 +0100 Subject: [PATCH 2/9] fix: correct-lambda-response-and-db-session-creation-fix Co-authored-by: Copilot --- lambdas/src/lib/db/db-retry.ts | 5 +++++ lambdas/src/lib/db/session-db-client.test.ts | 2 +- lambdas/src/lib/db/session-db-client.ts | 7 +++++-- .../src/session-login-lambda/index.test.ts | 21 +++++++++++++++++++ lambdas/src/session-login-lambda/index.ts | 7 ++++++- .../session-login-service.test.ts | 1 - .../session-login-service.ts | 15 ++++++------- ...n_preview_wiremock.postman_collection.json | 11 ++++++---- 8 files changed, 51 insertions(+), 18 deletions(-) diff --git a/lambdas/src/lib/db/db-retry.ts b/lambdas/src/lib/db/db-retry.ts index 0a42d0ab5..4dcc8856a 100644 --- a/lambdas/src/lib/db/db-retry.ts +++ b/lambdas/src/lib/db/db-retry.ts @@ -42,6 +42,11 @@ const TRANSIENT_NETWORK_ERROR_CODES = new Set([ // Source: https://www.postgresql.org/docs/current/errcodes-appendix.html const TRANSIENT_POSTGRES_ERROR_CODES = new Set(["57P01", "57P02", "57P03"]); +// Note: unique-violation conflicts (for example PostgreSQL 23505) are intentionally not treated +// as retryable here. The generic retry layer cannot distinguish a replay of the same successful +// write after a transient connection failure from a genuine duplicate-key conflict. Session create +// recovery for that case is deferred to a dedicated follow-up change. + const TRANSIENT_ERROR_PATTERNS = [ /cannot connect now/i, /connection terminated unexpectedly/i, diff --git a/lambdas/src/lib/db/session-db-client.test.ts b/lambdas/src/lib/db/session-db-client.test.ts index 26cecc46d..1316b9c08 100644 --- a/lambdas/src/lib/db/session-db-client.test.ts +++ b/lambdas/src/lib/db/session-db-client.test.ts @@ -135,7 +135,7 @@ describe("SessionDbClient", () => { const [query] = mockDbClient.query.mock.calls[0]; expect(query).toContain("$13::date"); - expect(query).toContain("ON CONFLICT (session_id) DO UPDATE"); + expect(query).not.toContain("ON CONFLICT (session_id) DO UPDATE"); }); it("should retry transient insert failures", async () => { diff --git a/lambdas/src/lib/db/session-db-client.ts b/lambdas/src/lib/db/session-db-client.ts index 48108ceaf..2d21c7849 100644 --- a/lambdas/src/lib/db/session-db-client.ts +++ b/lambdas/src/lib/db/session-db-client.ts @@ -118,8 +118,11 @@ export class SessionDbClient { $17::timestamptz, $18::timestamptz ) - ON CONFLICT (session_id) DO UPDATE - SET session_id = EXCLUDED.session_id + -- Intentionally use a plain INSERT here. + -- We do not use ON CONFLICT ... DO UPDATE for session creation because a true + -- session_id or refresh_token_id collision must never overwrite another user's session. + -- A future ticket will add safe recovery for ambiguous duplicate-key failures that can + -- happen after a transient connection error during session creation. RETURNING ${SESSION_COLUMNS}; `; diff --git a/lambdas/src/session-login-lambda/index.test.ts b/lambdas/src/session-login-lambda/index.test.ts index 5ffeb6e86..2ea8317dc 100644 --- a/lambdas/src/session-login-lambda/index.test.ts +++ b/lambdas/src/session-login-lambda/index.test.ts @@ -87,6 +87,27 @@ describe("session-login-lambda", () => { expect(response.statusCode).toBe(200); expect(response.headers?.["X-Correlation-ID"]).toBe("550e8400-e29b-41d4-a716-446655440000"); + expect(response.body).toBe( + JSON.stringify({ + userInfo: { + issuer: "https://id-token.example", + audience: "client-id-123", + subject: "user-123", + familyName: "MILLAR", + givenName: "Mona", + identityProofingLevel: "P9", + email: "test.user@example.com", + emailVerified: true, + phoneNumberVerified: true, + birthDate: "1990-01-01", + nhsNumber: "9999999999", + gpOdsCode: "A12345", + }, + sessionId: "550e8400-e29b-41d4-a716-446655440000", + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + sessionCreatedAt: "2026-04-27T10:15:30.000Z", + }), + ); expect(response.multiValueHeaders?.["Set-Cookie"]).toEqual([ "preview_auth=signed-preview-access; HttpOnly; Path=/session-preview; SameSite=Lax;", "preview_auth_refresh=signed-preview-refresh; HttpOnly; Path=/session-preview/refresh; SameSite=Lax;", diff --git a/lambdas/src/session-login-lambda/index.ts b/lambdas/src/session-login-lambda/index.ts index 378cb7f64..fc2778c0c 100644 --- a/lambdas/src/session-login-lambda/index.ts +++ b/lambdas/src/session-login-lambda/index.ts @@ -101,7 +101,12 @@ export const lambdaHandler = async ( const response = createJsonResponse( 200, - { ...result.result.userInfoResponse }, + { + userInfo: result.result.userInfo, + sessionId: result.result.sessionId, + refreshTokenId: result.result.refreshTokenId, + sessionCreatedAt: result.result.sessionCreatedAt, + }, { "X-Correlation-ID": correlationId, }, diff --git a/lambdas/src/session-login-lambda/session-login-service.test.ts b/lambdas/src/session-login-lambda/session-login-service.test.ts index 10bdb1d9d..328dac4be 100644 --- a/lambdas/src/session-login-lambda/session-login-service.test.ts +++ b/lambdas/src/session-login-lambda/session-login-service.test.ts @@ -147,7 +147,6 @@ describe("SessionLoginService.executeCallback", () => { nhsNumber: "9999999999", gpOdsCode: "A12345", }, - userInfoResponse: createNhsLoginSuccess().result.userInfo, sessionId: "550e8400-e29b-41d4-a716-446655440000", refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", sessionCreatedAt: fixedNow.toISOString(), diff --git a/lambdas/src/session-login-lambda/session-login-service.ts b/lambdas/src/session-login-lambda/session-login-service.ts index 6892493fb..e12ec8aef 100644 --- a/lambdas/src/session-login-lambda/session-login-service.ts +++ b/lambdas/src/session-login-lambda/session-login-service.ts @@ -1,14 +1,12 @@ import { randomUUID } from "node:crypto"; import { type ISessionTokenService } from "../lib/auth/session-token-service"; -import { type CreateSessionInput, type SessionDbClient } from "../lib/db/session-db-client"; +import { type SessionDbClient } from "../lib/db/session-db-client"; import { type INhsLoginService, type NhsLoginErrorCode } from "../lib/login/nhs-login-service"; -import { type INhsUserInfoResponseModel } from "../lib/models/nhs-login/nhs-login-user-info-response-model"; -import { mapNhsUserInfoToSessionUserInfo } from "../lib/models/session/session"; +import { ISessionUserInfo, mapNhsUserInfoToSessionUserInfo } from "../lib/models/session/session"; export interface SessionLoginSuccessResult { - userInfo: CreateSessionInput["userInfo"]; - userInfoResponse: INhsUserInfoResponseModel; + userInfo: ISessionUserInfo; sessionId: string; refreshTokenId: string; sessionCreatedAt: string; @@ -88,7 +86,7 @@ export class SessionLoginService implements ISessionLoginService { ); } - const idTokenAudience = this.normalizeAudience(nhsLoginResult.result.idTokenAudience); + const idTokenAudience = this.normaliseAudience(nhsLoginResult.result.idTokenAudience); if (!idTokenAudience) { return this.failure( @@ -152,7 +150,6 @@ export class SessionLoginService implements ISessionLoginService { success: true, result: { userInfo, - userInfoResponse: nhsLoginResult.result.userInfo, sessionId, refreshTokenId, sessionCreatedAt, @@ -162,7 +159,7 @@ export class SessionLoginService implements ISessionLoginService { }; } - private normalizeAudience(audience: string | string[] | undefined): string | undefined { + private normaliseAudience(audience: string | string[] | undefined): string | undefined { if (typeof audience !== "string") { return undefined; } @@ -171,7 +168,7 @@ export class SessionLoginService implements ISessionLoginService { return trimmedAudience.length > 0 ? trimmedAudience : undefined; } - private isValidSessionUserInfo(userInfo: CreateSessionInput["userInfo"]): boolean { + private isValidSessionUserInfo(userInfo: ISessionUserInfo): boolean { return ( this.hasValue(userInfo.issuer) && this.hasValue(userInfo.audience) && diff --git a/postman/session_login_preview_wiremock.postman_collection.json b/postman/session_login_preview_wiremock.postman_collection.json index f83956d3f..24c2f452d 100644 --- a/postman/session_login_preview_wiremock.postman_collection.json +++ b/postman/session_login_preview_wiremock.postman_collection.json @@ -140,10 +140,13 @@ "});", "", "const body = pm.response.json();", - "pm.test(\"User info payload is returned\", function () {", - " pm.expect(body.sub).to.eql(\"test-sub-123\");", - " pm.expect(body.nhs_number).to.eql(\"9912003071\");", - " pm.expect(body.gp_registration_details.gp_ods_code).to.eql(\"Y12345\");", + "pm.test(\"Session payload is returned\", function () {", + " pm.expect(body.sessionId).to.be.a(\"string\").and.not.empty;", + " pm.expect(body.refreshTokenId).to.be.a(\"string\").and.not.empty;", + " pm.expect(body.sessionCreatedAt).to.be.a(\"string\").and.not.empty;", + " pm.expect(body.userInfo.subject).to.eql(\"test-sub-123\");", + " pm.expect(body.userInfo.nhsNumber).to.eql(\"9912003071\");", + " pm.expect(body.userInfo.gpOdsCode).to.eql(\"Y12345\");", "});", "", "const setCookieHeaders = pm.response.headers", From dfc490c116c1199ca4c971c43d82d827e80aa6cb Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 13:58:14 +0100 Subject: [PATCH 3/9] fix: add-phone-number-to-user-info-and-session Co-authored-by: Copilot --- .../000011_add_phone_number_to_session_table.sql | 11 +++++++++++ database/schema_diagram.puml | 1 + lambdas/src/lib/db/session-db-client.test.ts | 7 ++++++- lambdas/src/lib/db/session-db-client.ts | 14 ++++++++++---- lambdas/src/lib/login/nhs-login-client.test.ts | 7 ++++++- lambdas/src/lib/login/nhs-login-service.test.ts | 7 ++++++- lambdas/src/lib/login/test-user-mapping.test.ts | 7 ++++++- .../nhs-login-user-info-response-model.ts | 1 + lambdas/src/lib/models/session/session.test.ts | 10 +++++++++- lambdas/src/lib/models/session/session.ts | 3 +++ lambdas/src/login-lambda/login-service.test.ts | 11 ++++++++--- lambdas/src/session-login-lambda/index.test.ts | 5 +++++ .../session-login-service.test.ts | 4 ++++ .../session-login-lambda/session-login-service.ts | 1 + 14 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 database/migrations/000011_add_phone_number_to_session_table.sql diff --git a/database/migrations/000011_add_phone_number_to_session_table.sql b/database/migrations/000011_add_phone_number_to_session_table.sql new file mode 100644 index 000000000..6d64d2565 --- /dev/null +++ b/database/migrations/000011_add_phone_number_to_session_table.sql @@ -0,0 +1,11 @@ +-- +goose Up +-- Preview sessions are short-lived; invalidate existing rows so the new mandatory field can be added safely. +DELETE FROM session; + +ALTER TABLE session +ADD COLUMN IF NOT EXISTS phone_number varchar(20) NOT NULL; + + +-- +goose Down +ALTER TABLE session +DROP COLUMN IF EXISTS phone_number; diff --git a/database/schema_diagram.puml b/database/schema_diagram.puml index 124b02cec..92bcd573f 100644 --- a/database/schema_diagram.puml +++ b/database/schema_diagram.puml @@ -97,6 +97,7 @@ entity "session" as session { identity_proofing_level : varchar(10) email : varchar(320) email_verified : boolean + phone_number: varchar(20) phone_number_verified : boolean birth_date : date nhs_number : varchar(10) diff --git a/lambdas/src/lib/db/session-db-client.test.ts b/lambdas/src/lib/db/session-db-client.test.ts index 1316b9c08..d4545948a 100644 --- a/lambdas/src/lib/db/session-db-client.test.ts +++ b/lambdas/src/lib/db/session-db-client.test.ts @@ -37,6 +37,7 @@ function buildCreateSessionInput(overrides: Partial = {}): C identityProofingLevel: "P9", email: "jane@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: false, birthDate: "1990-01-01", nhsNumber: "1234567890", @@ -64,6 +65,7 @@ function buildSessionRow(overrides: Record = {}): Record { session.userInfo.identityProofingLevel, session.userInfo.email, session.userInfo.emailVerified, + session.userInfo.phoneNumber, session.userInfo.phoneNumberVerified, session.userInfo.birthDate, session.userInfo.nhsNumber, @@ -134,7 +137,7 @@ describe("SessionDbClient", () => { ); const [query] = mockDbClient.query.mock.calls[0]; - expect(query).toContain("$13::date"); + expect(query).toContain("$14::date"); expect(query).not.toContain("ON CONFLICT (session_id) DO UPDATE"); }); @@ -224,6 +227,7 @@ describe("SessionDbClient", () => { identity_proofing_level: "P5", email: "jo@example.com", email_verified: false, + phone_number: "+447700900124", phone_number_verified: true, birth_date: "1985-05-05", nhs_number: "9999999999", @@ -244,6 +248,7 @@ describe("SessionDbClient", () => { identityProofingLevel: "P5", email: "jo@example.com", emailVerified: false, + phoneNumber: "+447700900124", phoneNumberVerified: true, birthDate: "1985-05-05", nhsNumber: "9999999999", diff --git a/lambdas/src/lib/db/session-db-client.ts b/lambdas/src/lib/db/session-db-client.ts index 2d21c7849..651399835 100644 --- a/lambdas/src/lib/db/session-db-client.ts +++ b/lambdas/src/lib/db/session-db-client.ts @@ -23,6 +23,7 @@ interface SessionRow { identity_proofing_level: string; email: string; email_verified: boolean; + phone_number: string; phone_number_verified: boolean; birth_date: string; nhs_number: string; @@ -61,6 +62,7 @@ const SESSION_COLUMNS = ` identity_proofing_level, email, email_verified, + phone_number, phone_number_verified, birth_date, nhs_number, @@ -90,6 +92,7 @@ export class SessionDbClient { identity_proofing_level, email, email_verified, + phone_number, phone_number_verified, birth_date, nhs_number, @@ -111,12 +114,13 @@ export class SessionDbClient { $10, $11, $12, - $13::date, - $14, + $13, + $14::date, $15, - $16::timestamptz, + $16, $17::timestamptz, - $18::timestamptz + $18::timestamptz, + $19::timestamptz ) -- Intentionally use a plain INSERT here. -- We do not use ON CONFLICT ... DO UPDATE for session creation because a true @@ -138,6 +142,7 @@ export class SessionDbClient { session.userInfo.identityProofingLevel, session.userInfo.email, session.userInfo.emailVerified, + session.userInfo.phoneNumber, session.userInfo.phoneNumberVerified, session.userInfo.birthDate, session.userInfo.nhsNumber, @@ -293,6 +298,7 @@ export class SessionDbClient { identityProofingLevel: row.identity_proofing_level, email: row.email, emailVerified: row.email_verified, + phoneNumber: row.phone_number, phoneNumberVerified: row.phone_number_verified, birthDate: row.birth_date, nhsNumber: row.nhs_number, diff --git a/lambdas/src/lib/login/nhs-login-client.test.ts b/lambdas/src/lib/login/nhs-login-client.test.ts index 7606b90b4..e158d880d 100644 --- a/lambdas/src/lib/login/nhs-login-client.test.ts +++ b/lambdas/src/lib/login/nhs-login-client.test.ts @@ -11,7 +11,7 @@ import * as testUserMapping from "./test-user-mapping"; function createUserInfo( overrides: Partial = {}, ): INhsUserInfoResponseModel { - return { + const defaults: INhsUserInfoResponseModel = { iss: "https://issuer.example", aud: "hometest", sub: "user-123", @@ -20,12 +20,17 @@ function createUserInfo( identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "true", birthdate: "1990-01-01", nhs_number: "9999999999", gp_registration_details: { gp_ods_code: "A12345", }, + }; + + return { + ...defaults, ...overrides, }; } diff --git a/lambdas/src/lib/login/nhs-login-service.test.ts b/lambdas/src/lib/login/nhs-login-service.test.ts index 8768c50b8..bedf0c838 100644 --- a/lambdas/src/lib/login/nhs-login-service.test.ts +++ b/lambdas/src/lib/login/nhs-login-service.test.ts @@ -12,7 +12,7 @@ import { function createUserInfo( overrides: Partial = {}, ): INhsUserInfoResponseModel { - return { + const defaults: INhsUserInfoResponseModel = { iss: "https://issuer.example", aud: "hometest", sub: "user-123", @@ -21,10 +21,15 @@ function createUserInfo( identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "true", birthdate: "1990-01-01", nhs_number: "9999999999", gp_registration_details: { gp_ods_code: "A12345" }, + }; + + return { + ...defaults, ...overrides, }; } diff --git a/lambdas/src/lib/login/test-user-mapping.test.ts b/lambdas/src/lib/login/test-user-mapping.test.ts index b1e4148b0..e6bf9ba87 100644 --- a/lambdas/src/lib/login/test-user-mapping.test.ts +++ b/lambdas/src/lib/login/test-user-mapping.test.ts @@ -4,7 +4,7 @@ import { enrichUserInfoWithTestFirstName } from "./test-user-mapping"; function createUserInfo( overrides: Partial = {}, ): INhsUserInfoResponseModel { - return { + const defaults: INhsUserInfoResponseModel = { iss: "https://issuer.example", aud: "hometest", sub: "user-123", @@ -13,12 +13,17 @@ function createUserInfo( identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "true", birthdate: "1990-01-01", nhs_number: "9999999999", gp_registration_details: { gp_ods_code: "A12345", }, + }; + + return { + ...defaults, ...overrides, }; } diff --git a/lambdas/src/lib/models/nhs-login/nhs-login-user-info-response-model.ts b/lambdas/src/lib/models/nhs-login/nhs-login-user-info-response-model.ts index fd4b42288..ef48a5abd 100644 --- a/lambdas/src/lib/models/nhs-login/nhs-login-user-info-response-model.ts +++ b/lambdas/src/lib/models/nhs-login/nhs-login-user-info-response-model.ts @@ -7,6 +7,7 @@ export interface INhsUserInfoResponseModel { identity_proofing_level: string; email: string; email_verified: string; + phone_number: string; phone_number_verified: string; birthdate: string; nhs_number: string; diff --git a/lambdas/src/lib/models/session/session.test.ts b/lambdas/src/lib/models/session/session.test.ts index 912468280..f3af35496 100644 --- a/lambdas/src/lib/models/session/session.test.ts +++ b/lambdas/src/lib/models/session/session.test.ts @@ -8,7 +8,7 @@ import { function createNhsUserInfo( overrides: Partial = {}, ): INhsUserInfoResponseModel { - return { + const defaults: INhsUserInfoResponseModel = { iss: "https://issuer.example", aud: "hometest", sub: "user-123", @@ -17,12 +17,17 @@ function createNhsUserInfo( identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "false", birthdate: "1990-01-01", nhs_number: "9999999999", gp_registration_details: { gp_ods_code: "A12345", }, + }; + + return { + ...defaults, ...overrides, }; } @@ -37,6 +42,7 @@ function createSessionUserInfo(overrides: Partial = {}): ISess identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: false, birthDate: "1990-01-01", nhsNumber: "9999999999", @@ -60,6 +66,7 @@ describe("mapNhsUserInfoToSessionUserInfo", () => { identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: false, birthDate: "1990-01-01", nhsNumber: "9999999999", @@ -91,6 +98,7 @@ describe("mapSessionUserInfoToNhsUserInfo", () => { identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "false", birthdate: "1990-01-01", nhs_number: "9999999999", diff --git a/lambdas/src/lib/models/session/session.ts b/lambdas/src/lib/models/session/session.ts index c32db122e..0dbe4e563 100644 --- a/lambdas/src/lib/models/session/session.ts +++ b/lambdas/src/lib/models/session/session.ts @@ -9,6 +9,7 @@ export interface ISessionUserInfo { identityProofingLevel: string; email: string; emailVerified: boolean; + phoneNumber: string; phoneNumberVerified: boolean; birthDate: string; nhsNumber: string; @@ -43,6 +44,7 @@ export function mapNhsUserInfoToSessionUserInfo( identityProofingLevel: userInfo.identity_proofing_level, email: userInfo.email, emailVerified: parseVerificationFlag(userInfo.email_verified), + phoneNumber: userInfo.phone_number, phoneNumberVerified: parseVerificationFlag(userInfo.phone_number_verified), birthDate: userInfo.birthdate, nhsNumber: userInfo.nhs_number, @@ -62,6 +64,7 @@ export function mapSessionUserInfoToNhsUserInfo( identity_proofing_level: userInfo.identityProofingLevel, email: userInfo.email, email_verified: toVerificationFlag(userInfo.emailVerified), + phone_number: userInfo.phoneNumber, phone_number_verified: toVerificationFlag(userInfo.phoneNumberVerified), birthdate: userInfo.birthDate, nhs_number: userInfo.nhsNumber, diff --git a/lambdas/src/login-lambda/login-service.test.ts b/lambdas/src/login-lambda/login-service.test.ts index f12224009..17b4550ab 100644 --- a/lambdas/src/login-lambda/login-service.test.ts +++ b/lambdas/src/login-lambda/login-service.test.ts @@ -1,12 +1,12 @@ -import { LoginService } from "./login-service"; -import { type ITokenService } from "../lib/login/token-service"; import { type INhsLoginClient } from "../lib/login/nhs-login-client"; +import { type ITokenService } from "../lib/login/token-service"; import { type INhsUserInfoResponseModel } from "../lib/models/nhs-login/nhs-login-user-info-response-model"; +import { LoginService } from "./login-service"; function createUserInfo( overrides: Partial = {}, ): INhsUserInfoResponseModel { - return { + const defaults: INhsUserInfoResponseModel = { iss: "https://issuer.example", aud: "hometest", sub: "user-123", @@ -15,12 +15,17 @@ function createUserInfo( identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "true", birthdate: "1990-01-01", nhs_number: "9999999999", gp_registration_details: { gp_ods_code: "A12345", }, + }; + + return { + ...defaults, ...overrides, }; } diff --git a/lambdas/src/session-login-lambda/index.test.ts b/lambdas/src/session-login-lambda/index.test.ts index 2ea8317dc..7ad0aa511 100644 --- a/lambdas/src/session-login-lambda/index.test.ts +++ b/lambdas/src/session-login-lambda/index.test.ts @@ -45,6 +45,7 @@ describe("session-login-lambda", () => { identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: true, birthDate: "1990-01-01", nhsNumber: "9999999999", @@ -59,6 +60,7 @@ describe("session-login-lambda", () => { identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "true", birthdate: "1990-01-01", nhs_number: "9999999999", @@ -98,6 +100,7 @@ describe("session-login-lambda", () => { identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: true, birthDate: "1990-01-01", nhsNumber: "9999999999", @@ -129,6 +132,7 @@ describe("session-login-lambda", () => { identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: true, birthDate: "1990-01-01", nhsNumber: "9999999999", @@ -143,6 +147,7 @@ describe("session-login-lambda", () => { identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "true", birthdate: "1990-01-01", nhs_number: "9999999999", diff --git a/lambdas/src/session-login-lambda/session-login-service.test.ts b/lambdas/src/session-login-lambda/session-login-service.test.ts index 328dac4be..9481fd62b 100644 --- a/lambdas/src/session-login-lambda/session-login-service.test.ts +++ b/lambdas/src/session-login-lambda/session-login-service.test.ts @@ -21,6 +21,7 @@ function createNhsLoginSuccess( identity_proofing_level: "P9", email: "test.user@example.com", email_verified: "true", + phone_number: "+447700900123", phone_number_verified: "false", birthdate: "1990-01-01", nhs_number: "9999999999", @@ -80,6 +81,7 @@ describe("SessionLoginService.executeCallback", () => { identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: false, birthDate: "1990-01-01", nhsNumber: "9999999999", @@ -114,6 +116,7 @@ describe("SessionLoginService.executeCallback", () => { identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: false, birthDate: "1990-01-01", nhsNumber: "9999999999", @@ -142,6 +145,7 @@ describe("SessionLoginService.executeCallback", () => { identityProofingLevel: "P9", email: "test.user@example.com", emailVerified: true, + phoneNumber: "+447700900123", phoneNumberVerified: false, birthDate: "1990-01-01", nhsNumber: "9999999999", diff --git a/lambdas/src/session-login-lambda/session-login-service.ts b/lambdas/src/session-login-lambda/session-login-service.ts index e12ec8aef..73433bef8 100644 --- a/lambdas/src/session-login-lambda/session-login-service.ts +++ b/lambdas/src/session-login-lambda/session-login-service.ts @@ -177,6 +177,7 @@ export class SessionLoginService implements ISessionLoginService { this.hasValue(userInfo.givenName) && this.hasValue(userInfo.identityProofingLevel) && this.hasValue(userInfo.email) && + this.hasValue(userInfo.phoneNumber) && this.hasValue(userInfo.birthDate) && this.hasValue(userInfo.gpOdsCode) && /^[0-9]{10}$/.test(userInfo.nhsNumber) From 81eca9691287289ee66d72d946a95ced399cc45c Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 15:13:18 +0100 Subject: [PATCH 4/9] chore: address-copilot-review-comments Co-authored-by: Copilot --- database/schema_diagram.puml | 2 +- lambdas/src/lib/db/session-db-client.test.ts | 1 + .../src/session-login-lambda/cookies.test.ts | 39 ++++++++++++ lambdas/src/session-login-lambda/cookies.ts | 30 +++++++++- .../src/session-login-lambda/index.test.ts | 30 ++++++++++ lambdas/src/session-login-lambda/index.ts | 10 +++- lambdas/src/session-login-lambda/init.test.ts | 33 ++++++++++- lambdas/src/session-login-lambda/init.ts | 19 ++++-- .../session-login-service.test.ts | 59 ++++++++++++++++++- .../session-login-service.ts | 24 ++++++-- 10 files changed, 228 insertions(+), 19 deletions(-) create mode 100644 lambdas/src/session-login-lambda/cookies.test.ts diff --git a/database/schema_diagram.puml b/database/schema_diagram.puml index 92bcd573f..2200eb69f 100644 --- a/database/schema_diagram.puml +++ b/database/schema_diagram.puml @@ -97,7 +97,7 @@ entity "session" as session { identity_proofing_level : varchar(10) email : varchar(320) email_verified : boolean - phone_number: varchar(20) + phone_number : varchar(20) phone_number_verified : boolean birth_date : date nhs_number : varchar(10) diff --git a/lambdas/src/lib/db/session-db-client.test.ts b/lambdas/src/lib/db/session-db-client.test.ts index d4545948a..b696f6942 100644 --- a/lambdas/src/lib/db/session-db-client.test.ts +++ b/lambdas/src/lib/db/session-db-client.test.ts @@ -137,6 +137,7 @@ describe("SessionDbClient", () => { ); const [query] = mockDbClient.query.mock.calls[0]; + expect(query).toContain("$13,"); expect(query).toContain("$14::date"); expect(query).not.toContain("ON CONFLICT (session_id) DO UPDATE"); }); diff --git a/lambdas/src/session-login-lambda/cookies.test.ts b/lambdas/src/session-login-lambda/cookies.test.ts new file mode 100644 index 000000000..f0c504c44 --- /dev/null +++ b/lambdas/src/session-login-lambda/cookies.test.ts @@ -0,0 +1,39 @@ +import { + buildPreviewAccessCookie, + buildPreviewRefreshCookie, + parsePreviewCookieSameSite, +} from "./cookies"; + +describe("parsePreviewCookieSameSite", () => { + it("accepts supported SameSite values", () => { + expect(parsePreviewCookieSameSite("Strict", true)).toBe("Strict"); + expect(parsePreviewCookieSameSite("Lax", false)).toBe("Lax"); + expect(parsePreviewCookieSameSite("None", true)).toBe("None"); + }); + + it("rejects unsupported SameSite values", () => { + expect(() => parsePreviewCookieSameSite("Invalid", true)).toThrow( + "AUTH_COOKIE_SAME_SITE must be one of Strict, Lax, or None", + ); + }); + + it("rejects SameSite=None when secure is false", () => { + expect(() => parsePreviewCookieSameSite("None", false)).toThrow( + "AUTH_COOKIE_SECURE must be true when AUTH_COOKIE_SAME_SITE is None", + ); + }); +}); + +describe("preview cookie builders", () => { + it("builds the preview access cookie", () => { + expect(buildPreviewAccessCookie("token", "Lax", false)).toBe( + "preview_auth=token; HttpOnly; Path=/session-preview; SameSite=Lax;", + ); + }); + + it("builds the preview refresh cookie", () => { + expect(buildPreviewRefreshCookie("token", "None", true)).toBe( + "preview_auth_refresh=token; HttpOnly; Path=/session-preview/refresh; SameSite=None; Secure;", + ); + }); +}); diff --git a/lambdas/src/session-login-lambda/cookies.ts b/lambdas/src/session-login-lambda/cookies.ts index 6640f92d0..024ae6a2e 100644 --- a/lambdas/src/session-login-lambda/cookies.ts +++ b/lambdas/src/session-login-lambda/cookies.ts @@ -3,17 +3,43 @@ export const PREVIEW_SESSION_REFRESH_COOKIE_NAME = "preview_auth_refresh"; export const PREVIEW_SESSION_ACCESS_COOKIE_PATH = "/session-preview"; export const PREVIEW_SESSION_REFRESH_COOKIE_PATH = "/session-preview/refresh"; +export type PreviewCookieSameSite = "Strict" | "Lax" | "None"; + +export function parsePreviewCookieSameSite( + sameSite: string, + secure: boolean, +): PreviewCookieSameSite { + const trimmedSameSite = sameSite.trim(); + + switch (trimmedSameSite) { + case "Strict": + case "Lax": + return trimmedSameSite; + case "None": + if (!secure) { + throw new Error("AUTH_COOKIE_SECURE must be true when AUTH_COOKIE_SAME_SITE is None"); + } + return trimmedSameSite; + default: + throw new Error("AUTH_COOKIE_SAME_SITE must be one of Strict, Lax, or None"); + } +} + function secureAttribute(secure: boolean): string { return secure ? " Secure;" : ""; } -export function buildPreviewAccessCookie(token: string, sameSite: string, secure: boolean): string { +export function buildPreviewAccessCookie( + token: string, + sameSite: PreviewCookieSameSite, + secure: boolean, +): string { return `${PREVIEW_SESSION_ACCESS_COOKIE_NAME}=${token}; HttpOnly; Path=${PREVIEW_SESSION_ACCESS_COOKIE_PATH}; SameSite=${sameSite};${secureAttribute(secure)}`; } export function buildPreviewRefreshCookie( token: string, - sameSite: string, + sameSite: PreviewCookieSameSite, secure: boolean, ): string { return `${PREVIEW_SESSION_REFRESH_COOKIE_NAME}=${token}; HttpOnly; Path=${PREVIEW_SESSION_REFRESH_COOKIE_PATH}; SameSite=${sameSite};${secureAttribute(secure)}`; diff --git a/lambdas/src/session-login-lambda/index.test.ts b/lambdas/src/session-login-lambda/index.test.ts index 7ad0aa511..6ee8ec26e 100644 --- a/lambdas/src/session-login-lambda/index.test.ts +++ b/lambdas/src/session-login-lambda/index.test.ts @@ -7,10 +7,20 @@ jest.mock("./init", () => ({ })); describe("session-login-lambda", () => { + let consoleInfoSpy: jest.SpyInstance; + let consoleErrorSpy: jest.SpyInstance; + beforeEach(() => { jest.resetModules(); process.env.ALLOW_ORIGIN = "http://localhost:3000"; mockInit.mockReset(); + consoleInfoSpy = jest.spyOn(console, "info").mockImplementation(() => undefined); + consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => undefined); + }); + + afterEach(() => { + consoleInfoSpy.mockRestore(); + consoleErrorSpy.mockRestore(); }); it("returns 400 when body is missing", async () => { @@ -206,6 +216,16 @@ describe("session-login-lambda", () => { expect(response.statusCode).toBe(401); expect(response.body).toBe(JSON.stringify({ message: "Unable to verify NHS identity token" })); + expect(consoleInfoSpy).toHaveBeenCalledWith( + "session-login-lambda", + "Preview session login failed", + { + correlationId: "550e8400-e29b-41d4-a716-446655440000", + code: "ID_TOKEN_VERIFICATION_FAILED", + statusCode: 401, + }, + ); + expect(consoleErrorSpy).not.toHaveBeenCalled(); }); it("returns 502 for upstream NHS exchange failures", async () => { @@ -233,5 +253,15 @@ describe("session-login-lambda", () => { } as unknown as APIGatewayProxyEvent); expect(response.statusCode).toBe(502); + expect(consoleInfoSpy).toHaveBeenCalledWith( + "session-login-lambda", + "Preview session login failed", + { + correlationId: "550e8400-e29b-41d4-a716-446655440000", + code: "TOKEN_EXCHANGE_FAILED", + statusCode: 502, + }, + ); + expect(consoleErrorSpy).not.toHaveBeenCalled(); }); }); diff --git a/lambdas/src/session-login-lambda/index.ts b/lambdas/src/session-login-lambda/index.ts index fc2778c0c..ed8ad4e1d 100644 --- a/lambdas/src/session-login-lambda/index.ts +++ b/lambdas/src/session-login-lambda/index.ts @@ -91,9 +91,15 @@ export const lambdaHandler = async ( const result = await sessionLoginService.executeCallback(body.code); if (!result.success) { - console.error(name, "Preview session login failed", { correlationId, code: result.error.code }); + const statusCode = statusCodeForError(result.error.code); + console.info(name, "Preview session login failed", { + correlationId, + code: result.error.code, + statusCode, + }); + return createJsonResponse( - statusCodeForError(result.error.code), + statusCode, { message: result.error.message }, { "X-Correlation-ID": correlationId }, ); diff --git a/lambdas/src/session-login-lambda/init.test.ts b/lambdas/src/session-login-lambda/init.test.ts index 888474d5a..40420f9a0 100644 --- a/lambdas/src/session-login-lambda/init.test.ts +++ b/lambdas/src/session-login-lambda/init.test.ts @@ -55,7 +55,7 @@ jest.mock("../lib/login/nhs-login-client", () => ({ })); jest.mock("../lib/login/nhs-token-verifier", () => ({ - NhsTokenVerifier: jest.fn().mockImplementation(() => ({ verifyIdToken: jest.fn() })), + NhsTokenVerifier: jest.fn().mockImplementation(() => ({ verifyToken: jest.fn() })), })); jest.mock("../lib/login/nhs-login-service", () => ({ @@ -155,6 +155,7 @@ describe("session-login-lambda init", () => { sessionDbClient: expect.any(Object), sessionTokenService: expect.any(Object), sessionMaxDurationMinutes: 60, + nhsLoginClientId: "hometest-preview", }); expect(result).toEqual({ sessionLoginService: mockSessionLoginServiceInstance, @@ -193,6 +194,36 @@ describe("session-login-lambda init", () => { ); }); + it("throws when AUTH_COOKIE_SAME_SITE is not an allowed value", async () => { + setEnvVariableMocks({ AUTH_COOKIE_SAME_SITE: "Invalid" }); + + const { buildEnvironment: init } = await import("./init"); + await expect(init()).rejects.toThrow( + "AUTH_COOKIE_SAME_SITE must be one of Strict, Lax, or None", + ); + }); + + it("throws when SameSite=None is configured without Secure", async () => { + setEnvVariableMocks({ AUTH_COOKIE_SAME_SITE: "None", AUTH_COOKIE_SECURE: "false" }); + + const { buildEnvironment: init } = await import("./init"); + await expect(init()).rejects.toThrow( + "AUTH_COOKIE_SECURE must be true when AUTH_COOKIE_SAME_SITE is None", + ); + }); + + it("throws when the preview cookie private key secret value has an invalid JSON shape", async () => { + mockSessionLoginGetSecretValue.mockReset(); + mockSessionLoginGetSecretValue + .mockResolvedValueOnce("test-nhs-login-private-key") + .mockResolvedValueOnce(JSON.stringify({ key: " " })); + + const { buildEnvironment: init } = await import("./init"); + await expect(init()).rejects.toThrow( + "AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME secret value must be either a non-JSON private key string or a JSON object containing a non-empty 'key' entry.", + ); + }); + describe("singleton protection", () => { it("constructs dependencies only once no matter how many times init() is called", async () => { const { init: singletonInit } = await import("./init"); diff --git a/lambdas/src/session-login-lambda/init.ts b/lambdas/src/session-login-lambda/init.ts index 24bb6ffa6..e7696662b 100644 --- a/lambdas/src/session-login-lambda/init.ts +++ b/lambdas/src/session-login-lambda/init.ts @@ -16,6 +16,7 @@ import { retrieveOptionalEnvVariable, retrieveOptionalEnvVariableWithDefault, } from "../lib/utils/utils"; +import { type PreviewCookieSameSite, parsePreviewCookieSameSite } from "./cookies"; import { type ISessionLoginService } from "./session-login-service"; import { SessionLoginService } from "./session-login-service"; @@ -30,13 +31,13 @@ interface SessionLoginEnvVariables { authSessionMaxDurationMinutes: number; authAccessTokenExpiryDurationMinutes: number; authRefreshTokenExpiryDurationMinutes: number; - authCookieSameSite: string; + authCookieSameSite: PreviewCookieSameSite; authCookieSecure: boolean; } export interface SessionLoginLambdaDependencies { sessionLoginService: ISessionLoginService; - authCookieSameSite: string; + authCookieSameSite: PreviewCookieSameSite; authCookieSecure: boolean; } @@ -60,7 +61,7 @@ function parseAuthCookiePrivateKey(secretValue: string): string { } throw new Error( - "AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME must be either a non-JSON private key string or a JSON object containing a non-empty 'key' entry.", + "AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME secret value must be either a non-JSON private key string or a JSON object containing a non-empty 'key' entry.", ); } catch (error) { if (!(error instanceof SyntaxError)) { @@ -76,6 +77,9 @@ function parseAuthCookiePrivateKey(secretValue: string): string { } function loadEnv(): SessionLoginEnvVariables { + const authCookieSecure = + retrieveOptionalEnvVariableWithDefault("AUTH_COOKIE_SECURE", "true").toLowerCase() === "true"; + return { awsRegion: retrieveMandatoryEnvVariable("AWS_REGION"), nhsLoginBaseEndpointUrl: retrieveMandatoryEnvVariable("NHS_LOGIN_BASE_ENDPOINT_URL"), @@ -98,9 +102,11 @@ function loadEnv(): SessionLoginEnvVariables { "AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES", retrieveMandatoryEnvVariable("AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES"), ), - authCookieSameSite: retrieveMandatoryEnvVariable("AUTH_COOKIE_SAME_SITE"), - authCookieSecure: - retrieveOptionalEnvVariableWithDefault("AUTH_COOKIE_SECURE", "true").toLowerCase() === "true", + authCookieSameSite: parsePreviewCookieSameSite( + retrieveMandatoryEnvVariable("AUTH_COOKIE_SAME_SITE"), + authCookieSecure, + ), + authCookieSecure, }; } @@ -156,6 +162,7 @@ export async function buildEnvironment(): Promise { sessionDbClient: sessionDbClientMock, sessionTokenService: sessionTokenServiceMock, sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", uuidGenerator: uuidGeneratorMock, clock: () => fixedNow, }); @@ -174,6 +175,7 @@ describe("SessionLoginService.executeCallback", () => { sessionDbClient: sessionDbClientMock, sessionTokenService: sessionTokenServiceMock, sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", }); await expect(service.executeCallback("bad-code")).resolves.toEqual({ @@ -196,6 +198,7 @@ describe("SessionLoginService.executeCallback", () => { sessionDbClient: sessionDbClientMock, sessionTokenService: sessionTokenServiceMock, sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", }); await expect(service.executeCallback("auth-code")).resolves.toEqual({ @@ -207,9 +210,58 @@ describe("SessionLoginService.executeCallback", () => { }); }); - it("fails when the verified ID token audience is not a single string", async () => { + it("accepts an array-form audience when it includes the configured client ID", async () => { nhsLoginServiceMock.executeCallback.mockResolvedValue( - createNhsLoginSuccess({ idTokenAudience: ["client-id-123"] }), + createNhsLoginSuccess({ idTokenAudience: ["other-audience", "client-id-123"] }), + ); + + sessionDbClientMock.createSession.mockResolvedValue({ + sessionId: "550e8400-e29b-41d4-a716-446655440000", + refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", + nhsAccessToken: "nhs-access-token", + userInfo: { + issuer: "https://id-token.example", + audience: "client-id-123", + subject: "user-123", + familyName: "MILLAR", + givenName: "Mona", + identityProofingLevel: "P9", + email: "test.user@example.com", + emailVerified: true, + phoneNumber: "+447700900123", + phoneNumberVerified: false, + birthDate: "1990-01-01", + nhsNumber: "9999999999", + gpOdsCode: "A12345", + }, + sessionCreatedAt: fixedNow.toISOString(), + lastRefreshAt: fixedNow.toISOString(), + maxExpiresAt: "2026-04-27T11:15:30.000Z", + }); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", + uuidGenerator: uuidGeneratorMock, + clock: () => fixedNow, + }); + + await expect(service.executeCallback("auth-code")).resolves.toMatchObject({ + success: true, + result: { + userInfo: { + audience: "client-id-123", + }, + }, + }); + }); + + it("fails when the verified ID token audience does not include the configured client ID", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue( + createNhsLoginSuccess({ idTokenAudience: ["other-audience"] }), ); const service = new SessionLoginService({ @@ -217,6 +269,7 @@ describe("SessionLoginService.executeCallback", () => { sessionDbClient: sessionDbClientMock, sessionTokenService: sessionTokenServiceMock, sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", }); await expect(service.executeCallback("auth-code")).resolves.toEqual({ @@ -243,6 +296,7 @@ describe("SessionLoginService.executeCallback", () => { sessionDbClient: sessionDbClientMock, sessionTokenService: sessionTokenServiceMock, sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", }); await expect(service.executeCallback("auth-code")).resolves.toEqual({ @@ -263,6 +317,7 @@ describe("SessionLoginService.executeCallback", () => { sessionDbClient: sessionDbClientMock, sessionTokenService: sessionTokenServiceMock, sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", uuidGenerator: uuidGeneratorMock, clock: () => fixedNow, }); diff --git a/lambdas/src/session-login-lambda/session-login-service.ts b/lambdas/src/session-login-lambda/session-login-service.ts index 73433bef8..745d0b132 100644 --- a/lambdas/src/session-login-lambda/session-login-service.ts +++ b/lambdas/src/session-login-lambda/session-login-service.ts @@ -49,6 +49,7 @@ export interface SessionLoginServiceParams { sessionDbClient: SessionLoginSessionDbClient; sessionTokenService: ISessionTokenService; sessionMaxDurationMinutes: number; + nhsLoginClientId: string; uuidGenerator?: () => string; clock?: () => Date; } @@ -58,6 +59,7 @@ export class SessionLoginService implements ISessionLoginService { private readonly sessionDbClient: SessionLoginSessionDbClient; private readonly sessionTokenService: ISessionTokenService; private readonly sessionMaxDurationMinutes: number; + private readonly nhsLoginClientId: string; private readonly uuidGenerator: () => string; private readonly clock: () => Date; @@ -66,6 +68,7 @@ export class SessionLoginService implements ISessionLoginService { this.sessionDbClient = params.sessionDbClient; this.sessionTokenService = params.sessionTokenService; this.sessionMaxDurationMinutes = params.sessionMaxDurationMinutes; + this.nhsLoginClientId = params.nhsLoginClientId; this.uuidGenerator = params.uuidGenerator ?? randomUUID; // dependency injection allows for easier testing, but default to crypto.randomUUID if not provided this.clock = params.clock ?? (() => new Date()); // dependency injection allows for easier testing, but default to current time if not provided } @@ -86,7 +89,7 @@ export class SessionLoginService implements ISessionLoginService { ); } - const idTokenAudience = this.normaliseAudience(nhsLoginResult.result.idTokenAudience); + const idTokenAudience = this.resolveValidatedAudience(nhsLoginResult.result.idTokenAudience); if (!idTokenAudience) { return this.failure( @@ -159,13 +162,24 @@ export class SessionLoginService implements ISessionLoginService { }; } - private normaliseAudience(audience: string | string[] | undefined): string | undefined { - if (typeof audience !== "string") { + private resolveValidatedAudience(audience: string | string[] | undefined): string | undefined { + const trimmedExpectedAudience = this.nhsLoginClientId.trim(); + + if (trimmedExpectedAudience.length === 0) { return undefined; } - const trimmedAudience = audience.trim(); - return trimmedAudience.length > 0 ? trimmedAudience : undefined; + if (typeof audience === "string") { + return audience.trim() === trimmedExpectedAudience ? trimmedExpectedAudience : undefined; + } + + if (Array.isArray(audience)) { + return audience.some((entry) => entry.trim() === trimmedExpectedAudience) + ? trimmedExpectedAudience + : undefined; + } + + return undefined; } private isValidSessionUserInfo(userInfo: ISessionUserInfo): boolean { From 441f32c0d7420e4f60d084c961320d5a1dda8fbd Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 15:23:22 +0100 Subject: [PATCH 5/9] chore: tidy-unit-tests Co-authored-by: Copilot --- .../src/session-login-lambda/index.test.ts | 30 ------------------- lambdas/src/session-login-lambda/init.test.ts | 29 +++++++++++------- 2 files changed, 19 insertions(+), 40 deletions(-) diff --git a/lambdas/src/session-login-lambda/index.test.ts b/lambdas/src/session-login-lambda/index.test.ts index 6ee8ec26e..dfe884ad4 100644 --- a/lambdas/src/session-login-lambda/index.test.ts +++ b/lambdas/src/session-login-lambda/index.test.ts @@ -61,21 +61,6 @@ describe("session-login-lambda", () => { nhsNumber: "9999999999", gpOdsCode: "A12345", }, - userInfoResponse: { - iss: "https://userinfo.example", - aud: "userinfo-audience", - sub: "user-123", - family_name: "MILLAR", - given_name: "Mona", - identity_proofing_level: "P9", - email: "test.user@example.com", - email_verified: "true", - phone_number: "+447700900123", - phone_number_verified: "true", - birthdate: "1990-01-01", - nhs_number: "9999999999", - gp_registration_details: { gp_ods_code: "A12345" }, - }, sessionId: "550e8400-e29b-41d4-a716-446655440000", refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", sessionCreatedAt: "2026-04-27T10:15:30.000Z", @@ -148,21 +133,6 @@ describe("session-login-lambda", () => { nhsNumber: "9999999999", gpOdsCode: "A12345", }, - userInfoResponse: { - iss: "https://userinfo.example", - aud: "userinfo-audience", - sub: "user-123", - family_name: "MILLAR", - given_name: "Mona", - identity_proofing_level: "P9", - email: "test.user@example.com", - email_verified: "true", - phone_number: "+447700900123", - phone_number_verified: "true", - birthdate: "1990-01-01", - nhs_number: "9999999999", - gp_registration_details: { gp_ods_code: "A12345" }, - }, sessionId: "550e8400-e29b-41d4-a716-446655440000", refreshTokenId: "650e8400-e29b-41d4-a716-446655440000", sessionCreatedAt: "2026-04-27T10:15:30.000Z", diff --git a/lambdas/src/session-login-lambda/init.test.ts b/lambdas/src/session-login-lambda/init.test.ts index 40420f9a0..8c968491c 100644 --- a/lambdas/src/session-login-lambda/init.test.ts +++ b/lambdas/src/session-login-lambda/init.test.ts @@ -13,6 +13,13 @@ const mockSessionLoginClientInstance = { getUserInfo: jest.fn(), fetchPublicKeyById: jest.fn(), }; +const mockSessionLoginTokenVerifierInstance = { verifyToken: jest.fn() }; +const mockNhsLoginServiceInstance = { executeCallback: jest.fn() }; +const mockSessionDbClientInstance = { createSession: jest.fn() }; +const mockSessionTokenServiceInstance = { + signAccessToken: jest.fn(), + signRefreshToken: jest.fn(), +}; const mockSessionLoginServiceInstance = { executeCallback: jest.fn() }; jest.mock("../lib/utils/utils", () => ({ @@ -39,7 +46,7 @@ jest.mock("../lib/db/db-client", () => ({ })); jest.mock("../lib/db/session-db-client", () => ({ - SessionDbClient: jest.fn().mockImplementation(() => ({ createSession: jest.fn() })), + SessionDbClient: jest.fn().mockImplementation(() => mockSessionDbClientInstance), })); jest.mock("../lib/http/http-client", () => ({ @@ -55,18 +62,15 @@ jest.mock("../lib/login/nhs-login-client", () => ({ })); jest.mock("../lib/login/nhs-token-verifier", () => ({ - NhsTokenVerifier: jest.fn().mockImplementation(() => ({ verifyToken: jest.fn() })), + NhsTokenVerifier: jest.fn().mockImplementation(() => mockSessionLoginTokenVerifierInstance), })); jest.mock("../lib/login/nhs-login-service", () => ({ - NhsLoginService: jest.fn().mockImplementation(() => ({ executeCallback: jest.fn() })), + NhsLoginService: jest.fn().mockImplementation(() => mockNhsLoginServiceInstance), })); jest.mock("../lib/auth/session-token-service", () => ({ - SessionTokenService: jest.fn().mockImplementation(() => ({ - signAccessToken: jest.fn(), - signRefreshToken: jest.fn(), - })), + SessionTokenService: jest.fn().mockImplementation(() => mockSessionTokenServiceInstance), })); jest.mock("./session-login-service", () => ({ @@ -133,6 +137,7 @@ describe("session-login-lambda init", () => { const { AwsSecretsClient } = await import("../lib/secrets/secrets-manager-client"); const { NhsLoginClient } = await import("../lib/login/nhs-login-client"); const { NhsTokenVerifier } = await import("../lib/login/nhs-token-verifier"); + const { NhsLoginService } = await import("../lib/login/nhs-login-service"); const { SessionTokenService } = await import("../lib/auth/session-token-service"); const { SessionLoginService } = await import("./session-login-service"); @@ -145,15 +150,19 @@ describe("session-login-lambda init", () => { keyProvider: mockSessionLoginClientInstance, issuer: "https://nhs-login.example", }); + expect(NhsLoginService).toHaveBeenCalledWith({ + nhsTokenVerifier: mockSessionLoginTokenVerifierInstance, + nhsLoginClient: mockSessionLoginClientInstance, + }); expect(SessionTokenService).toHaveBeenCalledWith({ privateKey: "test-preview-cookie-private-key", accessTokenExpiryDurationMinutes: 15, refreshTokenExpiryDurationMinutes: 120, }); expect(SessionLoginService).toHaveBeenCalledWith({ - nhsLoginService: expect.any(Object), - sessionDbClient: expect.any(Object), - sessionTokenService: expect.any(Object), + nhsLoginService: mockNhsLoginServiceInstance, + sessionDbClient: mockSessionDbClientInstance, + sessionTokenService: mockSessionTokenServiceInstance, sessionMaxDurationMinutes: 60, nhsLoginClientId: "hometest-preview", }); From e7373e6ef90156279e6ac1f7b63762fbd7732f3f Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 15:34:03 +0100 Subject: [PATCH 6/9] chore: make-session-table-alteration-safe Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../migrations/000018_add_phone_number_to_session.sql | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lambdas/goose-migrator-lambda/migrations/000018_add_phone_number_to_session.sql b/lambdas/goose-migrator-lambda/migrations/000018_add_phone_number_to_session.sql index f8dd63b76..80dfa7db6 100644 --- a/lambdas/goose-migrator-lambda/migrations/000018_add_phone_number_to_session.sql +++ b/lambdas/goose-migrator-lambda/migrations/000018_add_phone_number_to_session.sql @@ -1,8 +1,13 @@ -- +goose Up ALTER TABLE session -ADD COLUMN IF NOT EXISTS phone_number varchar(20) NOT NULL; +ADD COLUMN IF NOT EXISTS phone_number varchar(20); +UPDATE session +SET phone_number = '' +WHERE phone_number IS NULL; +ALTER TABLE session +ALTER COLUMN phone_number SET NOT NULL; -- +goose Down ALTER TABLE session DROP COLUMN IF EXISTS phone_number; From 593589fc185ac80e8feb75e95ed9c94d61536c85 Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 15:34:44 +0100 Subject: [PATCH 7/9] chore: clean-postman-environment Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- ...ession_login_preview_wiremock.local.postman_environment.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postman/session_login_preview_wiremock.local.postman_environment.json b/postman/session_login_preview_wiremock.local.postman_environment.json index 817212b15..91f0f12b7 100644 --- a/postman/session_login_preview_wiremock.local.postman_environment.json +++ b/postman/session_login_preview_wiremock.local.postman_environment.json @@ -4,7 +4,7 @@ "values": [ { "key": "api_gateway_id", - "value": "4lshw3ry0l", + "value": "", "type": "default", "enabled": true }, From a48d76b67eec8b8c2161937079ccaba88ef91307 Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 16:00:30 +0100 Subject: [PATCH 8/9] chore: tighten-user-info-validation Co-authored-by: Copilot --- .../session-login-service.test.ts | 54 +++++++++++++++++++ .../session-login-service.ts | 32 ++++++++--- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/lambdas/src/session-login-lambda/session-login-service.test.ts b/lambdas/src/session-login-lambda/session-login-service.test.ts index 110fe79db..eccd04299 100644 --- a/lambdas/src/session-login-lambda/session-login-service.test.ts +++ b/lambdas/src/session-login-lambda/session-login-service.test.ts @@ -308,6 +308,60 @@ describe("SessionLoginService.executeCallback", () => { }); }); + it("fails gracefully when a scalar user info claim is missing", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue( + createNhsLoginSuccess({ + userInfo: { + ...createNhsLoginSuccess().result.userInfo, + phone_number: undefined as unknown as string, + }, + }), + ); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", + }); + + await expect(service.executeCallback("auth-code")).resolves.toEqual({ + success: false, + error: { + code: "SESSION_DATA_INVALID", + message: "NHS user information is missing required session fields", + }, + }); + }); + + it("fails gracefully when a nested user info claim is missing", async () => { + nhsLoginServiceMock.executeCallback.mockResolvedValue( + createNhsLoginSuccess({ + userInfo: { + ...createNhsLoginSuccess().result.userInfo, + gp_registration_details: undefined as unknown as { gp_ods_code: string }, + }, + }), + ); + + const service = new SessionLoginService({ + nhsLoginService: nhsLoginServiceMock, + sessionDbClient: sessionDbClientMock, + sessionTokenService: sessionTokenServiceMock, + sessionMaxDurationMinutes: 60, + nhsLoginClientId: "client-id-123", + }); + + await expect(service.executeCallback("auth-code")).resolves.toEqual({ + success: false, + error: { + code: "SESSION_DATA_INVALID", + message: "NHS user information is missing required session fields", + }, + }); + }); + it("returns SESSION_PERSIST_FAILED when Aurora persistence fails", async () => { nhsLoginServiceMock.executeCallback.mockResolvedValue(createNhsLoginSuccess()); sessionDbClientMock.createSession.mockRejectedValue(new Error("db down")); diff --git a/lambdas/src/session-login-lambda/session-login-service.ts b/lambdas/src/session-login-lambda/session-login-service.ts index 745d0b132..43a5d7c81 100644 --- a/lambdas/src/session-login-lambda/session-login-service.ts +++ b/lambdas/src/session-login-lambda/session-login-service.ts @@ -3,6 +3,7 @@ import { randomUUID } from "node:crypto"; import { type ISessionTokenService } from "../lib/auth/session-token-service"; import { type SessionDbClient } from "../lib/db/session-db-client"; import { type INhsLoginService, type NhsLoginErrorCode } from "../lib/login/nhs-login-service"; +import { type INhsUserInfoResponseModel } from "../lib/models/nhs-login/nhs-login-user-info-response-model"; import { ISessionUserInfo, mapNhsUserInfoToSessionUserInfo } from "../lib/models/session/session"; export interface SessionLoginSuccessResult { @@ -113,11 +114,13 @@ export class SessionLoginService implements ISessionLoginService { ); } - const userInfo = mapNhsUserInfoToSessionUserInfo(nhsLoginResult.result.userInfo); - userInfo.issuer = idTokenIssuer; - userInfo.audience = idTokenAudience; + const userInfo = this.buildValidatedSessionUserInfo( + nhsLoginResult.result.userInfo, + idTokenIssuer, + idTokenAudience, + ); - if (!this.isValidSessionUserInfo(userInfo)) { + if (!userInfo) { return this.failure( "SESSION_DATA_INVALID", "NHS user information is missing required session fields", @@ -182,6 +185,22 @@ export class SessionLoginService implements ISessionLoginService { return undefined; } + private buildValidatedSessionUserInfo( + nhsUserInfo: INhsUserInfoResponseModel, + issuer: string, + audience: string, + ): ISessionUserInfo | undefined { + try { + const userInfo = mapNhsUserInfoToSessionUserInfo(nhsUserInfo); + userInfo.issuer = issuer; + userInfo.audience = audience; + + return this.isValidSessionUserInfo(userInfo) ? userInfo : undefined; + } catch { + return undefined; + } + } + private isValidSessionUserInfo(userInfo: ISessionUserInfo): boolean { return ( this.hasValue(userInfo.issuer) && @@ -194,12 +213,13 @@ export class SessionLoginService implements ISessionLoginService { this.hasValue(userInfo.phoneNumber) && this.hasValue(userInfo.birthDate) && this.hasValue(userInfo.gpOdsCode) && + this.hasValue(userInfo.nhsNumber) && /^[0-9]{10}$/.test(userInfo.nhsNumber) ); } - private hasValue(value: string): boolean { - return value.trim().length > 0; + private hasValue(value: unknown): value is string { + return typeof value === "string" && value.trim().length > 0; } private failure(code: SessionLoginErrorCode, message: string): SessionLoginFailure { From 1baebfa97db1d8ea296b9123f0fd8a4cd5e312cd Mon Sep 17 00:00:00 2001 From: Bill Wirz Date: Tue, 28 Apr 2026 16:21:06 +0100 Subject: [PATCH 9/9] chore: tighten-number-parsing-of-env-vars Co-authored-by: Copilot --- lambdas/src/session-login-lambda/init.test.ts | 18 ++++++++++++++++++ lambdas/src/session-login-lambda/init.ts | 8 +++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lambdas/src/session-login-lambda/init.test.ts b/lambdas/src/session-login-lambda/init.test.ts index 8c968491c..e3a238d55 100644 --- a/lambdas/src/session-login-lambda/init.test.ts +++ b/lambdas/src/session-login-lambda/init.test.ts @@ -203,6 +203,24 @@ describe("session-login-lambda init", () => { ); }); + it("throws when a duration contains trailing characters", async () => { + setEnvVariableMocks({ AUTH_SESSION_MAX_DURATION_MINUTES: "60abc" }); + + const { buildEnvironment: init } = await import("./init"); + await expect(init()).rejects.toThrow( + "AUTH_SESSION_MAX_DURATION_MINUTES must be a positive integer", + ); + }); + + it("throws when a duration is not a whole number", async () => { + setEnvVariableMocks({ AUTH_SESSION_MAX_DURATION_MINUTES: "60.5" }); + + const { buildEnvironment: init } = await import("./init"); + await expect(init()).rejects.toThrow( + "AUTH_SESSION_MAX_DURATION_MINUTES must be a positive integer", + ); + }); + it("throws when AUTH_COOKIE_SAME_SITE is not an allowed value", async () => { setEnvVariableMocks({ AUTH_COOKIE_SAME_SITE: "Invalid" }); diff --git a/lambdas/src/session-login-lambda/init.ts b/lambdas/src/session-login-lambda/init.ts index e7696662b..5e8dcaaa2 100644 --- a/lambdas/src/session-login-lambda/init.ts +++ b/lambdas/src/session-login-lambda/init.ts @@ -42,7 +42,13 @@ export interface SessionLoginLambdaDependencies { } function parsePositiveInteger(name: string, value: string): number { - const parsedValue = Number.parseInt(value, 10); + const trimmedValue = value.trim(); + + if (!/^[1-9]\d*$/.test(trimmedValue)) { + throw new Error(`${name} must be a positive integer`); + } + + const parsedValue = Number(trimmedValue); if (!Number.isInteger(parsedValue) || parsedValue <= 0) { throw new Error(`${name} must be a positive integer`);