From 8672bb5e833d8671ecc8ea9e635150417a9fce3c Mon Sep 17 00:00:00 2001 From: Carl Fagg Date: Mon, 13 Apr 2026 15:06:31 +0100 Subject: [PATCH] refactor: login lambda refactor + updated or removed comments to reflect changes + moved hardcoded values to environment variables and terraform + created seperate env variable for auth cookie secret + removed unused code and variables --- lambdas/package-lock.json | 274 +----------------- lambdas/package.json | 2 +- lambdas/src/get-results-lambda/init.test.ts | 2 +- lambdas/src/lib/http/http-client.test.ts | 33 ++- lambdas/src/lib/http/http-client.ts | 36 ++- lambdas/src/lib/http/login-http-client.ts | 261 ----------------- .../src/lib/login/nhs-login-client.test.ts | 40 +-- lambdas/src/lib/login/nhs-login-client.ts | 24 +- lambdas/src/lib/login/test-user-mapping.ts | 52 ++-- lambdas/src/login-lambda/index.test.ts | 160 ++++++++++ lambdas/src/login-lambda/index.ts | 77 +++-- lambdas/src/login-lambda/init.test.ts | 89 +++++- lambdas/src/login-lambda/init.ts | 114 +++++--- lambdas/src/login-lambda/login-lambda.test.ts | 68 ----- lambdas/src/login-lambda/login-service.ts | 5 +- lambdas/src/session-lambda/init.test.ts | 4 +- lambdas/src/session-lambda/init.ts | 4 +- local-environment/infra/main.tf | 21 +- local-environment/infra/outputs.tf | 10 + local-environment/infra/variables.tf | 72 +++++ scripts/terraform/post-apply-env-update.sh | 4 +- ui/package-lock.json | 24 +- .../lib/services/login-service.test.ts | 96 ++++++ ui/src/__tests__/routes/CallbackPage.test.tsx | 168 +++++++++++ ui/src/__tests__/routes/LoginPage.test.tsx | 86 +++++- ui/src/lib/services/login-service.ts | 31 ++ ui/src/routes/CallbackPage.tsx | 30 +- ui/src/routes/LoginPage.tsx | 32 +- ui/src/settings.ts | 4 +- 29 files changed, 1036 insertions(+), 787 deletions(-) delete mode 100644 lambdas/src/lib/http/login-http-client.ts create mode 100644 lambdas/src/login-lambda/index.test.ts delete mode 100644 lambdas/src/login-lambda/login-lambda.test.ts create mode 100644 ui/src/__tests__/lib/services/login-service.test.ts create mode 100644 ui/src/__tests__/routes/CallbackPage.test.tsx create mode 100644 ui/src/lib/services/login-service.ts diff --git a/lambdas/package-lock.json b/lambdas/package-lock.json index e6f3130f1..5f880547b 100644 --- a/lambdas/package-lock.json +++ b/lambdas/package-lock.json @@ -15,7 +15,6 @@ "@middy/http-cors": "7.2.1", "@middy/http-error-handler": "7.2.1", "@middy/http-security-headers": "7.2.1", - "axios": "1.15.0", "cookie": "1.1.1", "dayjs": "1.11.20", "esbuild": "0.28.0", @@ -23,6 +22,7 @@ "jwks-rsa": "4.0.1", "pg": "8.20.0", "titlecase": "1.1.3", + "undici": "8.1.0", "uuid": "13.0.0", "zod": "4.3.6" }, @@ -5660,23 +5660,6 @@ "dev": true, "license": "MIT" }, - "node_modules/asynckit": { - "version": "0.4.0", - "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", - "integrity": "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==", - "license": "MIT" - }, - "node_modules/axios": { - "version": "1.15.0", - "resolved": "https://registry.npmjs.org/axios/-/axios-1.15.0.tgz", - "integrity": "sha512-wWyJDlAatxk30ZJer+GeCWS209sA42X+N5jU2jy6oHTp7ufw8uzUTVFBX9+wTfAlhiJXGS0Bq7X6efruWjuK9Q==", - "license": "MIT", - "dependencies": { - "follow-redirects": "^1.15.11", - "form-data": "^4.0.5", - "proxy-from-env": "^2.1.0" - } - }, "node_modules/b4a": { "version": "1.8.0", "resolved": "https://registry.npmjs.org/b4a/-/b4a-1.8.0.tgz", @@ -6180,19 +6163,6 @@ "node": ">=0.10.0" } }, - "node_modules/call-bind-apply-helpers": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/call-bind-apply-helpers/-/call-bind-apply-helpers-1.0.2.tgz", - "integrity": "sha512-Sp1ablJ0ivDkSzjcaJdxEunN5/XvksFJ2sMBFfq6x0ryhQV/2b/KwFe21cMpmHtPOSij8K99/wSfoEuTObmuMQ==", - "license": "MIT", - "dependencies": { - "es-errors": "^1.3.0", - "function-bind": "^1.1.2" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/callsites": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/callsites/-/callsites-3.1.0.tgz", @@ -6407,18 +6377,6 @@ "dev": true, "license": "MIT" }, - "node_modules/combined-stream": { - "version": "1.0.8", - "resolved": "https://registry.npmjs.org/combined-stream/-/combined-stream-1.0.8.tgz", - "integrity": "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==", - "license": "MIT", - "dependencies": { - "delayed-stream": "~1.0.0" - }, - "engines": { - "node": ">= 0.8" - } - }, "node_modules/compress-commons": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/compress-commons/-/compress-commons-6.0.2.tgz", @@ -6603,15 +6561,6 @@ "node": ">=0.10.0" } }, - "node_modules/delayed-stream": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/delayed-stream/-/delayed-stream-1.0.0.tgz", - "integrity": "sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==", - "license": "MIT", - "engines": { - "node": ">=0.4.0" - } - }, "node_modules/detect-newline": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/detect-newline/-/detect-newline-3.1.0.tgz", @@ -6754,20 +6703,6 @@ "uuid": "dist/bin/uuid" } }, - "node_modules/dunder-proto": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/dunder-proto/-/dunder-proto-1.0.1.tgz", - "integrity": "sha512-KIN/nDJBQRcXw0MLVhZE9iQHmG68qAVIBg9CqmUYjmQIhgij9U5MFvrqkUL5FbtyyzZuOeOt0zdeRe4UY7ct+A==", - "license": "MIT", - "dependencies": { - "call-bind-apply-helpers": "^1.0.1", - "es-errors": "^1.3.0", - "gopd": "^1.2.0" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/eastasianwidth": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/eastasianwidth/-/eastasianwidth-0.2.0.tgz", @@ -6831,51 +6766,6 @@ "is-arrayish": "^0.2.1" } }, - "node_modules/es-define-property": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/es-define-property/-/es-define-property-1.0.1.tgz", - "integrity": "sha512-e3nRfgfUZ4rNGL232gUgX06QNyyez04KdjFrF+LTRoOXmrOgFKDg4BCdsjW8EnT69eqdYGmRpJwiPVYNrCaW3g==", - "license": "MIT", - "engines": { - "node": ">= 0.4" - } - }, - "node_modules/es-errors": { - "version": "1.3.0", - "resolved": "https://registry.npmjs.org/es-errors/-/es-errors-1.3.0.tgz", - "integrity": "sha512-Zf5H2Kxt2xjTvbJvP2ZWLEICxA6j+hAmMzIlypy4xcBg1vKVnx89Wy0GbS+kf5cwCVFFzdCFh2XSCFNULS6csw==", - "license": "MIT", - "engines": { - "node": ">= 0.4" - } - }, - "node_modules/es-object-atoms": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/es-object-atoms/-/es-object-atoms-1.1.1.tgz", - "integrity": "sha512-FGgH2h8zKNim9ljj7dankFPcICIK9Cp5bm+c2gQSYePhpaG5+esrLODihIorn+Pe6FGJzWhXQotPv73jTaldXA==", - "license": "MIT", - "dependencies": { - "es-errors": "^1.3.0" - }, - "engines": { - "node": ">= 0.4" - } - }, - "node_modules/es-set-tostringtag": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/es-set-tostringtag/-/es-set-tostringtag-2.1.0.tgz", - "integrity": "sha512-j6vWzfrGVfyXxge+O0x5sh6cvxAog0a/4Rdd2K36zCMV5eJ+/+tOAngRO8cODMNWbVRdVlmGZQL2YS3yR8bIUA==", - "license": "MIT", - "dependencies": { - "es-errors": "^1.3.0", - "get-intrinsic": "^1.2.6", - "has-tostringtag": "^1.0.2", - "hasown": "^2.0.2" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/esbuild": { "version": "0.28.0", "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.28.0.tgz", @@ -7337,26 +7227,6 @@ "dev": true, "license": "ISC" }, - "node_modules/follow-redirects": { - "version": "1.15.11", - "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.11.tgz", - "integrity": "sha512-deG2P0JfjrTxl50XGCDyfI97ZGVCxIpfKYmfyrQ54n5FO/0gfIES8C/Psl6kWVDolizcaaxZJnTS0QSMxvnsBQ==", - "funding": [ - { - "type": "individual", - "url": "https://github.com/sponsors/RubenVerborgh" - } - ], - "license": "MIT", - "engines": { - "node": ">=4.0" - }, - "peerDependenciesMeta": { - "debug": { - "optional": true - } - } - }, "node_modules/foreground-child": { "version": "3.3.1", "resolved": "https://registry.npmjs.org/foreground-child/-/foreground-child-3.3.1.tgz", @@ -7374,22 +7244,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/form-data": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.5.tgz", - "integrity": "sha512-8RipRLol37bNs2bhoV67fiTEvdTrbMUYcFTiy3+wuuOnUog2QBHCZWXDRijWQfAkhBj2Uf5UnVaiWwA5vdd82w==", - "license": "MIT", - "dependencies": { - "asynckit": "^0.4.0", - "combined-stream": "^1.0.8", - "es-set-tostringtag": "^2.1.0", - "hasown": "^2.0.2", - "mime-types": "^2.1.12" - }, - "engines": { - "node": ">= 6" - } - }, "node_modules/fs-constants": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz", @@ -7423,6 +7277,7 @@ "version": "1.1.2", "resolved": "https://registry.npmjs.org/function-bind/-/function-bind-1.1.2.tgz", "integrity": "sha512-7XHNxH7qX9xG5mIwxkhumTox/MIRNcOgDrxWsMt2pAr23WHp6MrRlN7FBSFpCpr+oVO0F744iUgR82nJMfG2SA==", + "dev": true, "license": "MIT", "funding": { "url": "https://github.com/sponsors/ljharb" @@ -7448,30 +7303,6 @@ "node": "6.* || 8.* || >= 10.*" } }, - "node_modules/get-intrinsic": { - "version": "1.3.0", - "resolved": "https://registry.npmjs.org/get-intrinsic/-/get-intrinsic-1.3.0.tgz", - "integrity": "sha512-9fSjSaos/fRIVIp+xSJlE6lfwhES7LNtKaCBIamHsjr2na1BiABJPo0mOjjz8GJDURarmCPGqaiVg5mfjb98CQ==", - "license": "MIT", - "dependencies": { - "call-bind-apply-helpers": "^1.0.2", - "es-define-property": "^1.0.1", - "es-errors": "^1.3.0", - "es-object-atoms": "^1.1.1", - "function-bind": "^1.1.2", - "get-proto": "^1.0.1", - "gopd": "^1.2.0", - "has-symbols": "^1.1.0", - "hasown": "^2.0.2", - "math-intrinsics": "^1.1.0" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/get-package-type": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/get-package-type/-/get-package-type-0.1.0.tgz", @@ -7495,19 +7326,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/get-proto": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/get-proto/-/get-proto-1.0.1.tgz", - "integrity": "sha512-sTSfBjoXBp89JvIKIefqw7U2CCebsc74kiY6awiGogKtoSGbgjYE/G/+l9sF3MWFPNc9IcoOC4ODfKHfxFmp0g==", - "license": "MIT", - "dependencies": { - "dunder-proto": "^1.0.1", - "es-object-atoms": "^1.0.0" - }, - "engines": { - "node": ">= 0.4" - } - }, "node_modules/get-stream": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-6.0.1.tgz", @@ -7615,18 +7433,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/gopd": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/gopd/-/gopd-1.2.0.tgz", - "integrity": "sha512-ZUKRh6/kUFoAiTAtTYPZJ3hw9wNxx+BIBOijnlG9PnrJsCcSjs1wyyD6vJpaYtgnzDrKYRSqf3OO6Rfa93xsRg==", - "license": "MIT", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/graceful-fs": { "version": "4.2.11", "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", @@ -7666,37 +7472,11 @@ "node": ">=8" } }, - "node_modules/has-symbols": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/has-symbols/-/has-symbols-1.1.0.tgz", - "integrity": "sha512-1cDNdwJ2Jaohmb3sg4OmKaMBwuC48sYni5HUw2DvsC8LjGTLK9h+eb1X6RyuOHe4hT0ULCW68iomhjUoKUqlPQ==", - "license": "MIT", - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, - "node_modules/has-tostringtag": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/has-tostringtag/-/has-tostringtag-1.0.2.tgz", - "integrity": "sha512-NqADB8VjPFLM2V0VvHUewwwsw0ZWBaIdgo+ieHtK3hasLz4qeCRjYcqfB6AQrBggRKppKF8L52/VqdVsO47Dlw==", - "license": "MIT", - "dependencies": { - "has-symbols": "^1.0.3" - }, - "engines": { - "node": ">= 0.4" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/hasown": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/hasown/-/hasown-2.0.2.tgz", "integrity": "sha512-0hJU9SCPvmMzIBdZFqNPXWa6dqh7WdH0cII9y+CyS8rG3nL48Bclra9HmKhVVUHyPWNH5Y7xDwAB7bfgSjkUMQ==", + "dev": true, "license": "MIT", "dependencies": { "function-bind": "^1.1.2" @@ -9057,15 +8837,6 @@ "tmpl": "1.0.5" } }, - "node_modules/math-intrinsics": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/math-intrinsics/-/math-intrinsics-1.1.0.tgz", - "integrity": "sha512-/IXtbwEk5HTPyEwyKX6hGkYXxM9nbj64B+ilVJnC/R6B0pH5G4V3b0pVbL7DBj4tkhBAppbQUlf6F6Xl9LHu1g==", - "license": "MIT", - "engines": { - "node": ">= 0.4" - } - }, "node_modules/merge-stream": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/merge-stream/-/merge-stream-2.0.0.tgz", @@ -9073,27 +8844,6 @@ "dev": true, "license": "MIT" }, - "node_modules/mime-db": { - "version": "1.52.0", - "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.52.0.tgz", - "integrity": "sha512-sPU4uV7dYlvtWJxwwxHD0PuihVNiE7TyAbQ5SWxDCB9mUYvOgroQOwYQQOKPJ8CIbE+1ETVlOoK1UC2nU3gYvg==", - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/mime-types": { - "version": "2.1.35", - "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.35.tgz", - "integrity": "sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw==", - "license": "MIT", - "dependencies": { - "mime-db": "1.52.0" - }, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/mimic-fn": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/mimic-fn/-/mimic-fn-2.1.0.tgz", @@ -9789,15 +9539,6 @@ "node": ">=12.0.0" } }, - "node_modules/proxy-from-env": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-2.1.0.tgz", - "integrity": "sha512-cJ+oHTW1VAEa8cJslgmUZrc+sjRKgAKl3Zyse6+PV38hZe/V6Z14TbCuXcan9F9ghlz4QrFr2c92TNF82UkYHA==", - "license": "MIT", - "engines": { - "node": ">=10" - } - }, "node_modules/pump": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/pump/-/pump-3.0.4.tgz", @@ -11395,6 +11136,15 @@ "node": ">=0.8.0" } }, + "node_modules/undici": { + "version": "8.1.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-8.1.0.tgz", + "integrity": "sha512-E9MkTS4xXLnRPYqxH2e6Hr2/49e7WFDKczKcCaFH4VaZs2iNvHMqeIkyUAD9vM8kujy9TjVrRlQ5KkdEJxB2pw==", + "license": "MIT", + "engines": { + "node": ">=22.19.0" + } + }, "node_modules/undici-types": { "version": "7.16.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.16.0.tgz", diff --git a/lambdas/package.json b/lambdas/package.json index 8cb2f4086..c1b362930 100644 --- a/lambdas/package.json +++ b/lambdas/package.json @@ -27,7 +27,6 @@ "@middy/http-cors": "7.2.1", "@middy/http-error-handler": "7.2.1", "@middy/http-security-headers": "7.2.1", - "axios": "1.15.0", "cookie": "1.1.1", "dayjs": "1.11.20", "esbuild": "0.28.0", @@ -35,6 +34,7 @@ "jwks-rsa": "4.0.1", "pg": "8.20.0", "titlecase": "1.1.3", + "undici": "8.1.0", "uuid": "13.0.0", "zod": "4.3.6" }, diff --git a/lambdas/src/get-results-lambda/init.test.ts b/lambdas/src/get-results-lambda/init.test.ts index 73f4c5d6b..614d6dcce 100644 --- a/lambdas/src/get-results-lambda/init.test.ts +++ b/lambdas/src/get-results-lambda/init.test.ts @@ -150,7 +150,7 @@ describe("init", () => { initFn: init, components: [ { - mock: FetchHttpClient as jest.Mock, + mock: FetchHttpClient as unknown as jest.Mock, times: 1, }, { diff --git a/lambdas/src/lib/http/http-client.test.ts b/lambdas/src/lib/http/http-client.test.ts index 2d5361b09..464123aa9 100644 --- a/lambdas/src/lib/http/http-client.test.ts +++ b/lambdas/src/lib/http/http-client.test.ts @@ -51,6 +51,35 @@ describe("FetchHttpClient", () => { expect(result).toEqual({ token: "abc" }); }); + it("post preserves URLSearchParams bodies", async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce({ + ok: true, + json: async () => ({ token: "abc" }), + }); + + await client.post<{ token: string }>( + "https://example.com/token", + new URLSearchParams({ + grant_type: "authorization_code", + code: "auth-code", + }), + { Accept: "application/json" }, + "application/x-www-form-urlencoded", + ); + + expect(global.fetch).toHaveBeenCalledWith( + "https://example.com/token", + expect.objectContaining({ + method: "POST", + headers: expect.objectContaining({ + Accept: "application/json", + "Content-Type": "application/x-www-form-urlencoded", + }), + body: "grant_type=authorization_code&code=auth-code", + }), + ); + }); + it("post throws HttpError with body on failure", async () => { (global.fetch as jest.Mock).mockResolvedValueOnce({ ok: false, @@ -88,9 +117,7 @@ describe("FetchHttpClient", () => { text: async () => "bad gateway", }); - await expect( - client.postRaw("https://example.com", "payload"), - ).rejects.toEqual( + await expect(client.postRaw("https://example.com", "payload")).rejects.toEqual( expect.objectContaining({ name: "HttpError", message: "HTTP POST request failed with status: 502", diff --git a/lambdas/src/lib/http/http-client.ts b/lambdas/src/lib/http/http-client.ts index 263ec5850..a239492b1 100644 --- a/lambdas/src/lib/http/http-client.ts +++ b/lambdas/src/lib/http/http-client.ts @@ -1,14 +1,16 @@ +import { Agent } from "undici"; + export interface HttpClient { get(url: string, headers?: Record): Promise; post( url: string, - body: any, + body: unknown, headers?: Record, contentType?: string, ): Promise; postRaw( url: string, - body: any, + body: unknown, headers?: Record, contentType?: string, ): Promise; @@ -26,6 +28,26 @@ export class HttpError extends Error { } export class FetchHttpClient implements HttpClient { + private readonly dispatcher?: Agent; + + private static serializeBody(body: unknown): string | null | undefined { + if (body == null || typeof body === "string") { + return body; + } + + if (body instanceof URLSearchParams) { + return body.toString(); + } + + return JSON.stringify(body); + } + + constructor(options?: { rejectUnauthorized?: boolean }) { + if (options?.rejectUnauthorized === false) { + this.dispatcher = new Agent({ connect: { rejectUnauthorized: false } }); + } + } + async get(url: string, headers?: Record): Promise { const response = await fetch(url, { method: "GET", @@ -49,7 +71,7 @@ export class FetchHttpClient implements HttpClient { async post( url: string, - body: any, + body: unknown, headers?: Record, contentType: string = "application/json", ): Promise { @@ -60,7 +82,8 @@ export class FetchHttpClient implements HttpClient { "Content-Type": contentType, ...headers, }, - body: typeof body === "string" ? body : JSON.stringify(body), + body: FetchHttpClient.serializeBody(body), + ...(this.dispatcher ? ({ dispatcher: this.dispatcher } as unknown as RequestInit) : {}), }); if (!response.ok) { @@ -77,7 +100,7 @@ export class FetchHttpClient implements HttpClient { async postRaw( url: string, - body: any, + body: unknown, headers?: Record, contentType: string = "application/json", ): Promise { @@ -88,7 +111,8 @@ export class FetchHttpClient implements HttpClient { "Content-Type": contentType, ...headers, }, - body: typeof body === "string" ? body : JSON.stringify(body), + body: FetchHttpClient.serializeBody(body), + ...(this.dispatcher ? ({ dispatcher: this.dispatcher } as unknown as RequestInit) : {}), }); if (!response.ok) { diff --git a/lambdas/src/lib/http/login-http-client.ts b/lambdas/src/lib/http/login-http-client.ts deleted file mode 100644 index 972f3e748..000000000 --- a/lambdas/src/lib/http/login-http-client.ts +++ /dev/null @@ -1,261 +0,0 @@ -import axios, { - type RawAxiosResponseHeaders, - type AddressFamily, - type LookupAddress, - type RawAxiosRequestHeaders, -} from "axios"; -import { Resolver } from "node:dns"; - -export interface HttpErrorDetails { - httpCode?: number; - responseData?: any; - cause?: any; -} - -export interface HttpClientOptions { - timeoutInSeconds?: number; - additionalDnsServers?: string[]; -} - -export type CustomLookupType = ( - hostname: string, - options: any, - callback: ( - err: Error | null, - address: LookupAddress | LookupAddress[], - family?: AddressFamily, - ) => void, -) => void; - -export class HttpClient { - private readonly options: HttpClientOptions | undefined; - private readonly customLookup: CustomLookupType | undefined; - // ALPHA: Removed commons use. To be reintroduced for logging later. - constructor(options?: HttpClientOptions) { - this.options = options; - if ( - options?.additionalDnsServers !== undefined && - options.additionalDnsServers.length > 0 - ) { - this.customLookup = this.getCustomLookup(options.additionalDnsServers); - } - } - - async doPostRequestWithStatus( - endpointUrl: string, - body: TBody, - headers: RawAxiosRequestHeaders = {}, - ): Promise<{ data: TResponse; httpCode: number }> { - try { - const timeoutInSeconds = this.options?.timeoutInSeconds ?? 30; - - const response = await axios.post(endpointUrl, body, { - headers, - timeout: timeoutInSeconds * 1000, - lookup: this.customLookup, - }); - return { httpCode: response.status, data: response.data }; - } catch (error: any) { - const errorDetails = this.createErrorDetailsObject(error); - throw this.createErrorWithDetails("Post", errorDetails); - } - } - - async doPostRequest( - endpointUrl: string, - body: TBody, - headers: RawAxiosRequestHeaders = {}, - ): Promise { - const response = await this.doPostRequestWithStatus( - endpointUrl, - body, - headers, - ); - return response.data; - } - - async postRequest( - endpointUrl: string, - body: TBody, - headers: RawAxiosRequestHeaders = {}, - ): Promise { - const defaultHeaders = this.getDefaultRequestHeaders(); - return await this.doPostRequest(endpointUrl, body, { - ...defaultHeaders, - ...headers, - }); - } - - async doPutRequestWithStatus( - endpointUrl: string, - body: TBody, - headers: RawAxiosRequestHeaders = {}, - ): Promise<{ data: TResponse; httpCode: number }> { - try { - const timeoutInSeconds = this.options?.timeoutInSeconds ?? 30; - const response = await axios.put(endpointUrl, body, { - headers, - timeout: timeoutInSeconds * 1000, - lookup: this.customLookup, - }); - - return { httpCode: response.status, data: response.data }; - } catch (error: any) { - const errorDetails = this.createErrorDetailsObject(error); - throw this.createErrorWithDetails("Put", errorDetails); - } - } - - async putRequest( - endpointUrl: string, - body: TBody, - headers: RawAxiosRequestHeaders = {}, - ): Promise { - const defaultHeaders = this.getDefaultRequestHeaders(); - const response = await this.doPutRequestWithStatus( - endpointUrl, - body, - { - ...defaultHeaders, - ...headers, - }, - ); - - return response.data; - } - - async deleteRequest( - endpointUrl: string, - headers: RawAxiosRequestHeaders = {}, - ): Promise { - try { - const defaultHeaders = this.getDefaultRequestHeaders(); - const response = await axios.delete(endpointUrl, { - headers: { ...defaultHeaders, ...headers }, - lookup: this.customLookup, - }); - - return response.data; - } catch (error: any) { - const errorDetails = this.createErrorDetailsObject(error); - throw this.createErrorWithDetails("Delete", errorDetails); - } - } - - async doGetRequestWithHeaders( - endpointUrl: string, - headers: RawAxiosRequestHeaders = {}, - ): Promise<{ data: TResponse; headers: RawAxiosResponseHeaders }> { - const response = await axios.get(endpointUrl, { - withCredentials: false, - headers, - lookup: this.customLookup, - }); - - return { headers: response.headers, data: response.data }; - } - - async getRequest( - endpointUrl: string, - headers: RawAxiosRequestHeaders = {}, - ): Promise { - try { - const defaultHeaders = this.getDefaultRequestHeaders(); - const response = await this.doGetRequestWithHeaders( - endpointUrl, - { ...defaultHeaders, ...headers }, - ); - - return response.data; - } catch (error: any) { - const errDetails = HttpClient.getHttpErrorDetails(error); - const errJson = HttpClient.isHttpError(error) ? error.toJSON() : error; - - console.log("getRequest error", { - errorDetails: { - ...errDetails, - name: errJson.name, - code: errJson.code, - status: errJson.status, - }, - }); - - throw new Error("Get API call failure", { cause: error }); - } - } - - private getDefaultRequestHeaders(): RawAxiosRequestHeaders { - return { - Accept: "application/json", - "Content-Type": "application/json", - }; - } - - private getCustomLookup(additionalDnsServers: string[]): CustomLookupType { - const resolver = new Resolver(); - resolver.setServers(additionalDnsServers.concat(resolver.getServers())); - - const customLookup: CustomLookupType = (hostname, options, callback) => { - // Resolve both IPv4 and IPv6 addresses - if (options.family === 6) { - resolver.resolve6(hostname, (err, addresses) => { - if (err) { - callback(err, []); - return; - } - callback(null, addresses[0], 6); - }); - } else { - resolver.resolve4(hostname, (err, addresses) => { - if (err) { - callback(err, []); - return; - } - callback(null, addresses[0], 4); - }); - } - }; - return customLookup; - } - - private removeGetParams(urlString: string): string { - const url = new URL(urlString); - return url.origin.concat(url.pathname); - } - - private createErrorDetailsObject(error: any): Record { - return { - errorMessage: error.message, - errorCode: error.code, - responseStatus: error.response?.status, - responseData: error.response?.data, - errorCause: error.cause, - isHttpError: true, - }; - } - - private createErrorWithDetails( - apiMethod: string, - errorDetails: Record, - ): Error { - return new Error(`${apiMethod} API call failure`, { - cause: { - details: errorDetails, - }, - }); - } - - static isHttpError(error: any): boolean { - return ( - axios.isAxiosError(error) || error.cause?.details?.isHttpError === true - ); - } - - static getHttpErrorDetails(error: any): HttpErrorDetails { - return { - httpCode: error.cause?.details?.responseStatus, - responseData: error.cause?.details?.responseData, - cause: error.cause?.details?.errorCause, - }; - } -} diff --git a/lambdas/src/lib/login/nhs-login-client.test.ts b/lambdas/src/lib/login/nhs-login-client.test.ts index 41cc81efe..7606b90b4 100644 --- a/lambdas/src/lib/login/nhs-login-client.test.ts +++ b/lambdas/src/lib/login/nhs-login-client.test.ts @@ -1,6 +1,6 @@ import { type JwksClient } from "jwks-rsa"; -import { type HttpClient } from "../http/login-http-client"; +import { type HttpClient } from "../http/http-client"; import { type INhsLoginConfig } from "../models/nhs-login/nhs-login-config"; import { type INhsTokenResponseModel } from "../models/nhs-login/nhs-login-token-response-model"; import { type INhsUserInfoResponseModel } from "../models/nhs-login/nhs-login-user-info-response-model"; @@ -35,7 +35,7 @@ describe("NhsLoginClient.getUserInfo", () => { clientId: "client-id", expiresIn: 300, redirectUri: "https://example.com/callback", - baseUri: "https://auth.example", + baseUri: "https://auth.sandpit.signin.nhs.uk", privateKey: "private-key", }; @@ -49,9 +49,10 @@ describe("NhsLoginClient.getUserInfo", () => { it("enriches user info when userinfo has missing given_name", async () => { const rawUserInfo = createUserInfo({ family_name: "MILLAR", given_name: "" }); - const httpClientMock: Pick = { - getRequest: jest.fn().mockResolvedValue(rawUserInfo), - postRequest: jest.fn(), + const httpClientMock: Pick = { + get: jest.fn().mockResolvedValue(rawUserInfo), + post: jest.fn(), + postRaw: jest.fn(), }; const enrichSpy = jest.spyOn(testUserMapping, "enrichUserInfoWithTestFirstName"); @@ -65,7 +66,7 @@ describe("NhsLoginClient.getUserInfo", () => { const result = await client.getUserInfo("access-token"); - expect(httpClientMock.getRequest).toHaveBeenCalledWith("https://auth.example/userinfo", { + expect(httpClientMock.get).toHaveBeenCalledWith("https://auth.sandpit.signin.nhs.uk/userinfo", { Authorization: "Bearer access-token", }); expect(enrichSpy).toHaveBeenCalledWith(rawUserInfo); @@ -76,9 +77,10 @@ describe("NhsLoginClient.getUserInfo", () => { it("returns user info without enrichment when given_name is present", async () => { const rawUserInfo = createUserInfo({ family_name: "MILLAR", given_name: "Eric" }); - const httpClientMock: Pick = { - getRequest: jest.fn().mockResolvedValue(rawUserInfo), - postRequest: jest.fn(), + const httpClientMock: Pick = { + get: jest.fn().mockResolvedValue(rawUserInfo), + post: jest.fn(), + postRaw: jest.fn(), }; const enrichSpy = jest.spyOn(testUserMapping, "enrichUserInfoWithTestFirstName"); @@ -92,7 +94,7 @@ describe("NhsLoginClient.getUserInfo", () => { const result = await client.getUserInfo("access-token"); - expect(httpClientMock.getRequest).toHaveBeenCalledWith("https://auth.example/userinfo", { + expect(httpClientMock.get).toHaveBeenCalledWith("https://auth.sandpit.signin.nhs.uk/userinfo", { Authorization: "Bearer access-token", }); expect(enrichSpy).toHaveBeenCalledWith(rawUserInfo); @@ -121,9 +123,10 @@ describe("NhsLoginClient.fetchPublicKeyById", () => { }), } as unknown as JwksClient; - const httpClientMock: Pick = { - getRequest: jest.fn(), - postRequest: jest.fn(), + const httpClientMock: Pick = { + get: jest.fn(), + post: jest.fn(), + postRaw: jest.fn(), }; it("fetches public key by key id", async () => { @@ -167,9 +170,10 @@ describe("NhsLoginClient.getUserTokens", () => { scope: "openid profile", }; - const httpClientMock: Pick = { - getRequest: jest.fn(), - postRequest: jest.fn().mockResolvedValue(tokenResponse), + const httpClientMock: Pick = { + get: jest.fn(), + post: jest.fn().mockResolvedValue(tokenResponse), + postRaw: jest.fn(), }; it("exchanges authorization code for user tokens", async () => { @@ -183,9 +187,9 @@ describe("NhsLoginClient.getUserTokens", () => { const result = await client.getUserTokens("auth-code"); expect(jwtHelperMock.createClientAuthJwt).toHaveBeenCalled(); - expect(httpClientMock.postRequest).toHaveBeenCalledTimes(1); + expect(httpClientMock.post).toHaveBeenCalledTimes(1); - const postRequestMock = httpClientMock.postRequest as jest.Mock; + const postRequestMock = httpClientMock.post as jest.Mock; const firstCallArgs = postRequestMock.mock.calls[0]; const [url, params, headers] = firstCallArgs; diff --git a/lambdas/src/lib/login/nhs-login-client.ts b/lambdas/src/lib/login/nhs-login-client.ts index 812bfa203..3fa553daa 100644 --- a/lambdas/src/lib/login/nhs-login-client.ts +++ b/lambdas/src/lib/login/nhs-login-client.ts @@ -1,8 +1,9 @@ -import { type HttpClient } from "../http/login-http-client"; +import { type JwksClient } from "jwks-rsa"; + +import { type HttpClient } from "../http/http-client"; import { type INhsLoginConfig } from "../models/nhs-login/nhs-login-config"; import { type INhsTokenResponseModel } from "../models/nhs-login/nhs-login-token-response-model"; import { type INhsUserInfoResponseModel } from "../models/nhs-login/nhs-login-user-info-response-model"; -import { type JwksClient } from "jwks-rsa"; import { type NhsLoginJwtHelper } from "./nhs-login-jwt-helper"; import { enrichUserInfoWithTestFirstName } from "./test-user-mapping"; @@ -12,7 +13,6 @@ export interface INhsLoginClient { fetchPublicKeyById: (kid: string) => Promise; } -// ALPHA: Removed commons use. To be reintroduced for logging later. export class NhsLoginClient implements INhsLoginClient { private readonly nhsLoginConfig: INhsLoginConfig; private readonly nhsLoginJwtHelper: NhsLoginJwtHelper; @@ -47,23 +47,25 @@ export class NhsLoginClient implements INhsLoginClient { client_assertion: signedToken, }); - const response: INhsTokenResponseModel = await this.httpClient.postRequest< - URLSearchParams, - any - >(this.nhsLoginTokenUri, formData, { - "Content-Type": "application/x-www-form-urlencoded", - }); + const response: INhsTokenResponseModel = await this.httpClient.post( + this.nhsLoginTokenUri, + formData, + { + "Content-Type": "application/x-www-form-urlencoded", + }, + ); return response; } public async getUserInfo(userAccessToken: string): Promise { - const userInfoResponse = await this.httpClient.getRequest( + const userInfoResponse = await this.httpClient.get( this.nhsLoginUserInfoUri, { Authorization: `Bearer ${userAccessToken}`, }, ); - // ALPHA: Enrich user info with test data for missing given_name (temporary workaround) + // BETA: Enrich user info with test data for missing given_name (temporary workaround for NHS Login Sandpit) + // This will require removal as part of NHS Login Integration and onboarding (Given name will be retrieved in UserInfo) const enrichedUserInfo = enrichUserInfoWithTestFirstName(userInfoResponse); return enrichedUserInfo; } diff --git a/lambdas/src/lib/login/test-user-mapping.ts b/lambdas/src/lib/login/test-user-mapping.ts index ed7423a3d..d70eaa8ef 100644 --- a/lambdas/src/lib/login/test-user-mapping.ts +++ b/lambdas/src/lib/login/test-user-mapping.ts @@ -1,4 +1,4 @@ -import { type INhsUserInfoResponseModel } from '../models/nhs-login/nhs-login-user-info-response-model'; +import { type INhsUserInfoResponseModel } from "../models/nhs-login/nhs-login-user-info-response-model"; /** * Test user first names lookup (temporary workaround for missing scope in NHS Login) @@ -6,40 +6,40 @@ import { type INhsUserInfoResponseModel } from '../models/nhs-login/nhs-login-us * TODO: Remove this when the given_name (profile_extended) scope is available from NHS Login */ export const TEST_FIRST_NAMES: Record = { - MILLAR: 'Mona', - HUGHES: 'Iain', - MEAKIN: 'Mike', - LEACH: 'Kevin', - OLLEY: 'Arnold', - LEECH: 'Mina', - CORR: 'Lauren', - BRAY: 'Cassie Leona', - GRIGG: 'Kim', - KEWN: 'Emilie', - CURRIE: 'Amanda', - BEARDSLEY: 'TYRIQ', - 'BISSOON-LAL': 'MISBAAH', - RICK: 'JULIE', - WHONE: 'JOHAN', - 'POWELL-CID': 'Lee', - EDELSTEIN: 'GAVRIEL', - TABERT: 'ADELA', - 'BARKER-CID': 'Gail', - WRIGHT: 'GARTH', - ONIONS: 'JULIET', - PRYDE: 'Toni', - 'KELSO-CID': 'Huberto', + MILLAR: "Mona", + HUGHES: "Iain", + MEAKIN: "Mike", + LEACH: "Kevin", + OLLEY: "Arnold", + LEECH: "Mina", + CORR: "Lauren", + BRAY: "Cassie Leona", + GRIGG: "Kim", + KEWN: "Emilie", + CURRIE: "Amanda", + BEARDSLEY: "TYRIQ", + "BISSOON-LAL": "MISBAAH", + RICK: "JULIE", + WHONE: "JOHAN", + "POWELL-CID": "Lee", + EDELSTEIN: "GAVRIEL", + TABERT: "ADELA", + "BARKER-CID": "Gail", + WRIGHT: "GARTH", + ONIONS: "JULIET", + PRYDE: "Toni", + "KELSO-CID": "Huberto", }; /** * Enriches user info with test first names for known test users (temporary workaround) * Fills in missing given_name based on family_name lookup - * ALPHA: TODO: Remove this when the given_name (profile_extended) scope is available from NHS Login + * Necessary for sandpit integration * @param userInfo - The user info response from NHS Login * @returns Enriched user info with given_name populated if missing */ export function enrichUserInfoWithTestFirstName( - userInfo: INhsUserInfoResponseModel + userInfo: INhsUserInfoResponseModel, ): INhsUserInfoResponseModel { if (userInfo.given_name) { return userInfo; diff --git a/lambdas/src/login-lambda/index.test.ts b/lambdas/src/login-lambda/index.test.ts new file mode 100644 index 000000000..15a5d8068 --- /dev/null +++ b/lambdas/src/login-lambda/index.test.ts @@ -0,0 +1,160 @@ +import type { APIGatewayProxyEvent } from "aws-lambda"; + +const mockInit = jest.fn(); + +jest.mock("./init", () => ({ + init: () => mockInit(), +})); + +describe("login-lambda", () => { + beforeEach(() => { + jest.resetModules(); + process.env.AUTH_COOKIE_SAME_SITE = "Lax"; + mockInit.mockReset(); + }); + + it("returns 400 when body is missing", async () => { + mockInit.mockImplementation(async () => ({ + authTokenService: { + generateAuthAccessToken: jest.fn(), + generateAuthRefreshToken: jest.fn(), + }, + loginService: { + performLogin: jest.fn(), + }, + })); + + const { lambdaHandler } = await import("./index"); + + const event = { body: null, headers: {} } as APIGatewayProxyEvent; + + const res = await lambdaHandler(event); + + expect(res.statusCode).toBe(400); + expect(res.body).toBe(JSON.stringify({ message: "Body is required" })); + }); + + it("returns 400 when body shape is invalid", async () => { + mockInit.mockImplementation(async () => ({ + authTokenService: { + generateAuthAccessToken: jest.fn(), + generateAuthRefreshToken: jest.fn(), + }, + loginService: { + performLogin: jest.fn(), + }, + })); + + const { lambdaHandler } = await import("./index"); + + const event = { body: JSON.stringify({}), headers: {} } as APIGatewayProxyEvent; + + const res = await lambdaHandler(event); + + expect(res.statusCode).toBe(400); + expect(res.body).toContain("Validation failed"); + expect(res.body).toContain("code:"); + }); + + it("returns 200, user info, and Set-Cookie headers on success", async () => { + const loginOutput = { + nhsLoginAccessToken: "access-token", + nhsLoginRefreshToken: "refresh-token", + userInfoResponse: { + sub: "user-123", + nhs_number: "9686368973", + birthdate: "1968-02-12", + family_name: "MILLAR", + email: "testuser@example.com", + phone_number: "+447887510886", + }, + }; + + mockInit.mockImplementation(async () => ({ + authTokenService: { + generateAuthAccessToken: jest.fn().mockReturnValue("signed-access"), + generateAuthRefreshToken: jest.fn().mockReturnValue("signed-refresh"), + }, + loginService: { + performLogin: jest.fn().mockImplementation(async () => loginOutput), + }, + authCookieSameSite: "Lax", + authCookieSecure: false, + })); + + const { lambdaHandler } = await import("./index"); + + const event = { + body: JSON.stringify({ code: "abc" }), + } as unknown as APIGatewayProxyEvent; + const res = await lambdaHandler(event); + + expect(res.statusCode).toBe(200); + + const cookies = res.multiValueHeaders?.["Set-Cookie"]; + expect(cookies).toHaveLength(2); + expect(cookies?.[0]).toBe("auth=signed-access; HttpOnly; Path=/; SameSite=Lax;"); + expect(cookies?.[1]).toBe( + "auth_refresh=signed-refresh; HttpOnly; Path=/refresh-token; SameSite=Lax;", + ); + }); + + it("includes Secure attribute in Set-Cookie when authCookieSecure is true", async () => { + const loginOutput = { + nhsLoginAccessToken: "access-token", + nhsLoginRefreshToken: "refresh-token", + userInfoResponse: { sub: "user-123" }, + }; + + mockInit.mockImplementation(async () => ({ + authTokenService: { + generateAuthAccessToken: jest.fn().mockReturnValue("signed-access"), + generateAuthRefreshToken: jest.fn().mockReturnValue("signed-refresh"), + }, + loginService: { + performLogin: jest.fn().mockResolvedValue(loginOutput), + }, + authCookieSameSite: "None", + authCookieSecure: true, + })); + + const { lambdaHandler } = await import("./index"); + + const event = { + body: JSON.stringify({ code: "abc" }), + } as unknown as APIGatewayProxyEvent; + const res = await lambdaHandler(event); + + expect(res.statusCode).toBe(200); + + const cookies = res.multiValueHeaders?.["Set-Cookie"]; + expect(cookies?.[0]).toBe("auth=signed-access; HttpOnly; Path=/; SameSite=None; Secure;"); + expect(cookies?.[1]).toBe( + "auth_refresh=signed-refresh; HttpOnly; Path=/refresh-token; SameSite=None; Secure;", + ); + }); + + it("returns 500 when login fails", async () => { + mockInit.mockImplementation(async () => ({ + authTokenService: { + generateAuthAccessToken: jest.fn(), + generateAuthRefreshToken: jest.fn(), + }, + loginService: { + performLogin: jest.fn().mockRejectedValue(new Error("Login failed")), + }, + authCookieSameSite: "Lax", + authCookieSecure: false, + })); + + const { lambdaHandler } = await import("./index"); + + const event = { + body: JSON.stringify({ code: "abc" }), + } as unknown as APIGatewayProxyEvent; + const res = await lambdaHandler(event); + + expect(res.statusCode).toBe(500); + expect(res.body).toBe(JSON.stringify({ message: "Internal server error" })); + }); +}); diff --git a/lambdas/src/login-lambda/index.ts b/lambdas/src/login-lambda/index.ts index 14a71429b..2cb1fdf77 100644 --- a/lambdas/src/login-lambda/index.ts +++ b/lambdas/src/login-lambda/index.ts @@ -1,34 +1,73 @@ +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 { z } from "zod"; import { securityHeaders } from "../lib/http/security-headers"; +import { createJsonResponse, getCorrelationIdFromEventHeaders } from "../lib/utils/utils"; +import { generateReadableError } from "../lib/utils/validation-utils"; import { corsOptions } from "./cors-configuration"; import { init } from "./init"; -export interface LoginBody { - code: string; // the auth code from NHS login - // ALPHA: Removed temporarily until purpose can be determined. - // source: UserSource; - // urlSource?: string; -} +export const LoginBodySchema = z.object({ + code: z.string().min(1, "code is required"), +}); + +export type LoginBody = z.infer; + +const name = "login-lambda"; + +const 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; + } +}; + +const parseLoginBody = (body: string | null): LoginBody => { + 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 = LoginBodySchema.safeParse(parsedBody); + if (!validationResult.success) { + const errorDetails = generateReadableError(validationResult.error); + throw new Error(`Validation failed: ${errorDetails}`); + } -const className = "handler"; + return validationResult.data; +}; export const lambdaHandler = async ( event: APIGatewayProxyEvent, ): Promise => { - const { authTokenService, loginService, authCookieSameSite, authCookieSecure } = await init(); - if (event.body === null) { - return { - statusCode: 400, - body: "Invalid request, missing body", - }; + const correlationId = resolveCorrelationId(event); + let body: LoginBody; + + try { + body = parseLoginBody(event.body); + } catch (error) { + return createJsonResponse(400, { + message: error instanceof Error ? error.message : "Invalid request body", + }); } - const body = JSON.parse(event.body) as LoginBody; + const { authTokenService, loginService, authCookieSameSite, authCookieSecure } = await init(); try { const loginOutput = await loginService.performLogin(body); @@ -43,10 +82,10 @@ export const lambdaHandler = async ( }); const secureAttr = authCookieSecure ? " Secure;" : ""; + const response = createJsonResponse(200, { ...loginOutput.userInfoResponse }); return { - statusCode: 200, - body: JSON.stringify(loginOutput.userInfoResponse), + ...response, multiValueHeaders: { "Set-Cookie": [ `auth=${signedAuthAccessJwt}; HttpOnly; Path=/; SameSite=${authCookieSameSite};${secureAttr}`, @@ -55,9 +94,9 @@ export const lambdaHandler = async ( }, }; } catch (e) { - const err = e as { cause?: { details?: { responseData?: unknown } } } | undefined; - console.error(`${className} - Error in login handler:`, err?.cause?.details?.responseData); - throw e; + const message = e instanceof Error ? e.message : "Unknown login error"; + console.error(name, "Error in login handler", { correlationId, message }); + return createJsonResponse(500, { message: "Internal server error" }); } }; diff --git a/lambdas/src/login-lambda/init.test.ts b/lambdas/src/login-lambda/init.test.ts index 3c192aa15..b5223201f 100644 --- a/lambdas/src/login-lambda/init.test.ts +++ b/lambdas/src/login-lambda/init.test.ts @@ -38,8 +38,8 @@ jest.mock("../lib/login/nhs-login-jwt-helper", () => ({ NhsLoginJwtHelper: jest.fn().mockImplementation(() => mockNhsLoginJwtHelperInstance), })); -jest.mock("../lib/http/login-http-client", () => ({ - HttpClient: jest.fn().mockImplementation(() => mockHttpClientInstance), +jest.mock("../lib/http/http-client", () => ({ + FetchHttpClient: jest.fn().mockImplementation(() => mockHttpClientInstance), })); jest.mock("jwks-rsa", () => ({ @@ -64,6 +64,7 @@ describe("login-lambda init", () => { NHS_LOGIN_CLIENT_ID: "client-id-123", NHS_LOGIN_REDIRECT_URL: "https://app.example/callback", NHS_LOGIN_PRIVATE_KEY_SECRET_NAME: "nhs-login-private-key-secret", + AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME: "auth-cookie-private-keys-secret", AUTH_SESSION_MAX_DURATION_MINUTES: "120", AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES: "10", AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES: "30", @@ -92,8 +93,20 @@ describe("login-lambda init", () => { jest.clearAllMocks(); setMandatoryEnvVariableMock(); - mockRetrieveOptionalEnvVariable.mockReturnValue(undefined); - mockGetSecretValue.mockResolvedValue("nhs-private-key"); + mockRetrieveOptionalEnvVariable.mockImplementation( + (_key: string, defaultValue: string = "") => defaultValue, + ); + mockGetSecretValue.mockImplementation((secretName: string) => { + if (secretName === "auth-cookie-private-keys-secret") { + return Promise.resolve("auth-private-key"); + } + + if (secretName === "nhs-login-private-key-secret") { + return Promise.resolve("nhs-private-key"); + } + + return Promise.reject(new Error(`Unexpected secret: ${secretName}`)); + }); }); it("initializes and wires all dependencies with expected configuration", async () => { @@ -103,20 +116,21 @@ describe("login-lambda init", () => { const { AwsSecretsClient } = await import("../lib/secrets/secrets-manager-client"); const { AuthTokenService } = await import("../lib/auth/auth-token-service"); const { NhsLoginJwtHelper } = await import("../lib/login/nhs-login-jwt-helper"); - const { HttpClient } = await import("../lib/http/login-http-client"); + const { FetchHttpClient } = await import("../lib/http/http-client"); const { JwksClient } = await import("jwks-rsa"); const { NhsLoginClient } = await import("../lib/login/nhs-login-client"); const { TokenService } = await import("../lib/login/token-service"); const { LoginService } = await import("./login-service"); expect(AwsSecretsClient).toHaveBeenCalledWith("eu-west-2"); + expect(mockGetSecretValue).toHaveBeenCalledWith("auth-cookie-private-keys-secret"); expect(mockGetSecretValue).toHaveBeenCalledWith("nhs-login-private-key-secret"); expect(AuthTokenService).toHaveBeenCalledWith({ sessionMaxDurationMinutes: 120, accessTokenExpiryDurationMinutes: 10, refreshTokenExpiryDurationMinutes: 30, - privateKeys: { key: "nhs-private-key" }, + privateKeys: { key: "auth-private-key" }, publicKeys: {}, }); @@ -129,7 +143,7 @@ describe("login-lambda init", () => { }; expect(NhsLoginJwtHelper).toHaveBeenCalledWith(expectedNhsConfig); - expect(HttpClient).toHaveBeenCalledTimes(1); + expect(FetchHttpClient).toHaveBeenCalledTimes(1); expect(JwksClient).toHaveBeenCalledWith({ cache: true, rateLimit: true, @@ -156,8 +170,35 @@ describe("login-lambda init", () => { }); }); + it("parses JSON auth-cookie private-key secrets", async () => { + mockGetSecretValue.mockImplementation((secretName: string) => { + if (secretName === "auth-cookie-private-keys-secret") { + return Promise.resolve('{"key":"auth-private-key","next":"rotated-private-key"}'); + } + + if (secretName === "nhs-login-private-key-secret") { + return Promise.resolve("nhs-private-key"); + } + + return Promise.reject(new Error(`Unexpected secret: ${secretName}`)); + }); + + const { buildEnvironment: init } = await import("./init"); + await init(); + + const { AuthTokenService } = await import("../lib/auth/auth-token-service"); + expect(AuthTokenService).toHaveBeenCalledWith({ + sessionMaxDurationMinutes: 120, + accessTokenExpiryDurationMinutes: 10, + refreshTokenExpiryDurationMinutes: 30, + privateKeys: { key: "auth-private-key", next: "rotated-private-key" }, + publicKeys: {}, + }); + }); + it("uses the AWS_REGION value from mandatory env variable", async () => { setMandatoryEnvVariableMock({ AWS_REGION: "eu-central-1" }); + process.env.AWS_DEFAULT_REGION = "us-east-1"; const { buildEnvironment: init } = await import("./init"); await init(); @@ -169,8 +210,36 @@ describe("login-lambda init", () => { it("throws when AWS_REGION is missing", async () => { setMandatoryEnvVariableMock({ AWS_REGION: "" }); - const { buildEnvironment: init } = await import("./init"); - await expect(init()).rejects.toThrow("Missing environment variable: AWS_REGION"); + const { buildEnvironment } = await import("./init"); + await expect(buildEnvironment()).rejects.toThrow("Missing environment variable: AWS_REGION"); + }); + + it("throws when the auth-cookie private-key secret is an empty string", async () => { + mockGetSecretValue.mockImplementation((secretName: string) => { + if (secretName === "auth-cookie-private-keys-secret") { + return Promise.resolve(""); + } + return Promise.resolve("nhs-private-key"); + }); + + const { buildEnvironment } = await import("./init"); + await expect(buildEnvironment()).rejects.toThrow( + "AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME secret value must not be empty.", + ); + }); + + it("throws when the auth-cookie private-key secret is whitespace only", async () => { + mockGetSecretValue.mockImplementation((secretName: string) => { + if (secretName === "auth-cookie-private-keys-secret") { + return Promise.resolve(" "); + } + return Promise.resolve("nhs-private-key"); + }); + + const { buildEnvironment } = await import("./init"); + await expect(buildEnvironment()).rejects.toThrow( + "AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME secret value must not be empty.", + ); }); describe("singleton protection", () => { @@ -190,7 +259,7 @@ describe("login-lambda init", () => { it("should clear the cached environment on rejection so subsequent calls can retry", async () => { mockGetSecretValue .mockRejectedValueOnce(new Error("Secrets Manager unavailable")) - .mockResolvedValue("nhs-private-key"); + .mockResolvedValue("auth-private-key"); const { init: singletonInit } = await import("./init"); diff --git a/lambdas/src/login-lambda/init.ts b/lambdas/src/login-lambda/init.ts index 61dba5a3f..39a1b3cad 100644 --- a/lambdas/src/login-lambda/init.ts +++ b/lambdas/src/login-lambda/init.ts @@ -1,7 +1,7 @@ import { JwksClient } from "jwks-rsa"; import { AuthTokenService } from "../lib/auth/auth-token-service"; -import { HttpClient } from "../lib/http/login-http-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 { TokenService } from "../lib/login/token-service"; @@ -14,40 +14,14 @@ import { } from "../lib/utils/utils"; import { type ILoginService, LoginService, type LoginServiceParams } from "./login-service"; -// ALPHA: This file will need revisiting. -const envVars: LoginEnvVariables = { - // ALPHA: Uncomment when environment variables are properly set up. Currently using hardcoded values for development and testing. - 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"), - // ALPHA: Uncomment when authCookiePrivateKeySecret is properly retrieved. - // authCookiePrivateKeysSecretName: retrieveMandatoryEnvVariable( - // 'AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME' - // ), - authSessionMaxDurationMinutes: Number.parseInt( - retrieveMandatoryEnvVariable("AUTH_SESSION_MAX_DURATION_MINUTES"), - ), - authAccessTokenExpiryDurationMinutes: Number.parseInt( - retrieveMandatoryEnvVariable("AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES"), - ), - authRefreshTokenExpiryDurationMinutes: Number.parseInt( - retrieveMandatoryEnvVariable("AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES"), - ), - authCookieSameSite: retrieveMandatoryEnvVariable("AUTH_COOKIE_SAME_SITE"), - authCookieSecure: - retrieveOptionalEnvVariableWithDefault("AUTH_COOKIE_SECURE", "true").toLowerCase() === "true", -}; - interface LoginEnvVariables { + awsRegion: string; nhsLoginBaseEndpointUrl: string; nhsLoginJwksUri: string | undefined; nhsLoginClientId: string; nhsLoginRedirectUrl: string; nhsLoginPrivateKeySecretName: string; - // ALPHA: Uncomment when authCookiePrivateKeySecret is properly retrieved. - // authCookiePrivateKeysSecretName: string; + authCookiePrivateKeysSecretName: string; authSessionMaxDurationMinutes: number; authAccessTokenExpiryDurationMinutes: number; authRefreshTokenExpiryDurationMinutes: number; @@ -55,6 +29,42 @@ interface LoginEnvVariables { authCookieSecure: boolean; } +function parseAuthCookiePrivateKeys(secretValue: string): Record { + try { + const parsedSecret = JSON.parse(secretValue) as Record; + const privateKeys = Object.entries(parsedSecret).reduce>( + (accumulator, [key, value]) => { + if (typeof value === "string" && value.length > 0) { + accumulator[key] = value; + } + + return accumulator; + }, + {}, + ); + + if (typeof privateKeys.key === "string" && privateKeys.key.length > 0) { + return privateKeys; + } + 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; + } + // Non-JSON values are treated as a single key. + } + + if (secretValue.trim().length === 0) { + throw new Error("AUTH_COOKIE_PRIVATE_KEYS_SECRET_NAME secret value must not be empty."); + } + + return { + key: secretValue, + }; +} + export interface LoginLambdaDependencies { loginService: ILoginService; authTokenService: AuthTokenService; @@ -62,27 +72,47 @@ export interface LoginLambdaDependencies { authCookieSecure: boolean; } -// ALPHA: Removed commons temporarily. -export async function buildEnvironment(): Promise { - const secretManagerClient = new AwsSecretsClient(retrieveMandatoryEnvVariable("AWS_REGION")); - const authCookiePrivateKeySecret = { key: "" } as Record; - // ALPHA: Requires a proper private key for cookie signing. Currently reusing nhs login key. Fix soon. - // await secretManagerClient.getSecretKeyValuePair( - // envVars.authCookiePrivateKeysSecretName - // ); +function loadEnv(): LoginEnvVariables { + 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: Number.parseInt( + retrieveMandatoryEnvVariable("AUTH_SESSION_MAX_DURATION_MINUTES"), + ), + authAccessTokenExpiryDurationMinutes: Number.parseInt( + retrieveMandatoryEnvVariable("AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES"), + ), + authRefreshTokenExpiryDurationMinutes: Number.parseInt( + 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 secretManagerClient = new AwsSecretsClient(envVars.awsRegion); const nhsLoginPrivateKey = await secretManagerClient.getSecretValue( envVars.nhsLoginPrivateKeySecretName, ); - - // ALPHA: Remove when authCookiePrivateKeySecret is properly retrieved. - authCookiePrivateKeySecret["key"] = nhsLoginPrivateKey; + const authCookiePrivateKeySecret = await secretManagerClient.getSecretValue( + envVars.authCookiePrivateKeysSecretName, + ); const authTokenService = new AuthTokenService({ sessionMaxDurationMinutes: envVars.authSessionMaxDurationMinutes, accessTokenExpiryDurationMinutes: envVars.authAccessTokenExpiryDurationMinutes, refreshTokenExpiryDurationMinutes: envVars.authRefreshTokenExpiryDurationMinutes, - privateKeys: authCookiePrivateKeySecret, + privateKeys: parseAuthCookiePrivateKeys(authCookiePrivateKeySecret), publicKeys: {}, }); @@ -97,7 +127,7 @@ export async function buildEnvironment(): Promise { const nhsLoginJwtHelper: NhsLoginJwtHelper = new NhsLoginJwtHelper(nhsLoginConfig); - const httpClient = new HttpClient(); + const httpClient = new FetchHttpClient(); const jwksClient = new JwksClient({ cache: true, rateLimit: true, diff --git a/lambdas/src/login-lambda/login-lambda.test.ts b/lambdas/src/login-lambda/login-lambda.test.ts deleted file mode 100644 index 667b94abb..000000000 --- a/lambdas/src/login-lambda/login-lambda.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -import type { APIGatewayProxyEvent } from "aws-lambda"; - -const mockInit = jest.fn(); - -jest.mock("./init", () => ({ - init: () => mockInit(), -})); - -describe("login-lambda", () => { - beforeEach(() => { - jest.resetModules(); - process.env.AUTH_COOKIE_SAME_SITE = "Lax"; - mockInit.mockReset(); - }); - - it("returns 400 when body is missing", async () => { - mockInit.mockImplementation(async () => ({ - authTokenService: { - generateAuthAccessToken: jest.fn(), - generateAuthRefreshToken: jest.fn(), - }, - loginService: { - performLogin: jest.fn(), - }, - })); - - const { lambdaHandler } = await import("./index"); - - const event = { body: null } as APIGatewayProxyEvent; - - const res = await lambdaHandler(event); - - expect(res.statusCode).toBe(400); - expect(res.body).toBe("Invalid request, missing body"); - }); - - it("returns 200, user info, and Set-Cookie headers on success", async () => { - const loginOutput = { - nhsLoginAccessToken: "access-token", - nhsLoginRefreshToken: "refresh-token", - userInfoResponse: { - sub: "user-123", - nhs_number: "9686368973", - birthdate: "1968-02-12", - family_name: "MILLAR", - email: "testuser@example.com", - phone_number: "+447887510886", - }, - }; - - mockInit.mockImplementation(async () => ({ - authTokenService: { - generateAuthAccessToken: jest.fn().mockReturnValue("signed-access"), - generateAuthRefreshToken: jest.fn().mockReturnValue("signed-refresh"), - }, - loginService: { - performLogin: jest.fn().mockImplementation(async () => loginOutput), - }, - })); - - const { lambdaHandler } = await import("./index"); - - const event = { body: JSON.stringify({ code: "abc" }) } as APIGatewayProxyEvent; - const res = await lambdaHandler(event); - - expect(res.statusCode).toBe(200); - }); -}); diff --git a/lambdas/src/login-lambda/login-service.ts b/lambdas/src/login-lambda/login-service.ts index 54fddac72..6e4554b69 100644 --- a/lambdas/src/login-lambda/login-service.ts +++ b/lambdas/src/login-lambda/login-service.ts @@ -1,11 +1,11 @@ import { type JwtPayload } from "jsonwebtoken"; + +import { type LoginBody } from "."; import { type INhsLoginClient } from "../lib/login/nhs-login-client"; import { type ITokenService } from "../lib/login/token-service"; import { type INhsTokenResponseModel } from "../lib/models/nhs-login/nhs-login-token-response-model"; import { type INhsUserInfoResponseModel } from "../lib/models/nhs-login/nhs-login-user-info-response-model"; -import { type LoginBody } from "."; -// ALPHA: This file will need revisiting. export interface ILoginService { performLogin: (loginBody: LoginBody) => Promise; } @@ -26,7 +26,6 @@ export class LoginService { readonly tokenService: ITokenService; readonly nhsLoginClient: INhsLoginClient; - // ALPHA: Removed commons use. To be reintroduced for logging later. constructor(params: LoginServiceParams) { this.tokenService = params.tokenService; this.nhsLoginClient = params.nhsLoginClient; diff --git a/lambdas/src/session-lambda/init.test.ts b/lambdas/src/session-lambda/init.test.ts index 1630ff7e1..5d00fdea2 100644 --- a/lambdas/src/session-lambda/init.test.ts +++ b/lambdas/src/session-lambda/init.test.ts @@ -28,8 +28,8 @@ jest.mock("../lib/login/nhs-login-client", () => ({ NhsLoginClient: jest.fn().mockImplementation(() => mockNhsLoginClientInstance), })); -jest.mock("../lib/http/login-http-client", () => ({ - HttpClient: jest.fn().mockImplementation(() => ({})), +jest.mock("../lib/http/http-client", () => ({ + FetchHttpClient: jest.fn().mockImplementation(() => ({})), })); jest.mock("jwks-rsa", () => ({ diff --git a/lambdas/src/session-lambda/init.ts b/lambdas/src/session-lambda/init.ts index 7a3fbef44..7f3e213eb 100644 --- a/lambdas/src/session-lambda/init.ts +++ b/lambdas/src/session-lambda/init.ts @@ -1,7 +1,7 @@ import { JwksClient } from "jwks-rsa"; import { AuthTokenVerifier } from "../lib/auth/auth-token-verifier"; -import { HttpClient } from "../lib/http/login-http-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 { INhsLoginConfig } from "../lib/models/nhs-login/nhs-login-config"; @@ -49,7 +49,7 @@ export async function buildEnvironment(): Promise { baseUri: envVars.nhsLoginBaseEndpointUrl, } as INhsLoginConfig, {} as NhsLoginJwtHelper, - new HttpClient(), + new FetchHttpClient(), {} as JwksClient, ); diff --git a/local-environment/infra/main.tf b/local-environment/infra/main.tf index 6dff99cf4..01fc6c64e 100644 --- a/local-environment/infra/main.tf +++ b/local-environment/infra/main.tf @@ -244,14 +244,15 @@ module "login_lambda" { 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 = "hometest", - NHS_LOGIN_REDIRECT_URL = "http://localhost:3000/callback", - NHS_LOGIN_PRIVATE_KEY_SECRET_NAME = "nhs-login-private-key", - AUTH_SESSION_MAX_DURATION_MINUTES = "60", - AUTH_ACCESS_TOKEN_EXPIRY_DURATION_MINUTES = "60", - AUTH_REFRESH_TOKEN_EXPIRY_DURATION_MINUTES = "60", - AUTH_COOKIE_SAME_SITE = "Lax" - AUTH_COOKIE_SECURE = "false" + 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 } } @@ -279,8 +280,8 @@ module "session_lambda" { environment_variables = { NODE_OPTIONS = "--enable-source-maps" ALLOW_ORIGIN = "http://localhost:3000" - AUTH_COOKIE_KEY_ID = "key" - AUTH_COOKIE_PUBLIC_KEY_SECRET_NAME = "nhs-login-private-key" + AUTH_COOKIE_KEY_ID = var.local_auth_cookie_key_id + AUTH_COOKIE_PUBLIC_KEY_SECRET_NAME = var.local_auth_cookie_public_key_secret_name NHS_LOGIN_BASE_ENDPOINT_URL = local.resolved_nhs_login_base_url, } } diff --git a/local-environment/infra/outputs.tf b/local-environment/infra/outputs.tf index 29437bee9..a856beab5 100644 --- a/local-environment/infra/outputs.tf +++ b/local-environment/infra/outputs.tf @@ -63,6 +63,16 @@ output "nhs_login_authorize_url" { value = local.resolved_nhs_login_authorize_url } +output "nhs_login_client_id" { + description = "NHS Login client id for local UI authorize requests" + value = var.local_nhs_login_client_id +} + +output "nhs_login_scope" { + description = "NHS Login scope for local UI authorize requests" + value = var.local_nhs_login_scope +} + output "local_service_mode" { description = "Resolved local integration mode" value = var.local_service_mode diff --git a/local-environment/infra/variables.tf b/local-environment/infra/variables.tf index e4b40da2c..ad3963fa9 100644 --- a/local-environment/infra/variables.tf +++ b/local-environment/infra/variables.tf @@ -44,3 +44,75 @@ variable "local_use_ui_auth_url_override" { type = string default = null } + +variable "local_nhs_login_client_id" { + description = "NHS Login client id for local login/session lambda configuration." + type = string + default = "hometest" +} + +variable "local_nhs_login_scope" { + description = "NHS Login scope for local UI authorize requests." + type = string + default = "openid profile email phone" +} + +variable "local_nhs_login_redirect_url" { + description = "NHS Login redirect URL used by local login lambda configuration." + type = string + default = "http://localhost:3000/callback" +} + +variable "local_nhs_login_private_key_secret_name" { + description = "Secrets Manager secret name containing the NHS Login private key for login lambda signing." + type = string + default = "nhs-login-private-key" +} + +variable "local_auth_cookie_private_keys_secret_name" { + description = "Secrets Manager secret name containing auth cookie private keys for login lambda." + type = string + default = "nhs-login-private-key" +} + +variable "local_auth_cookie_public_key_secret_name" { + description = "Secrets Manager secret name containing auth cookie public key for session lambda verification." + type = string + default = "nhs-login-private-key" +} + +variable "local_auth_cookie_key_id" { + description = "Key id used for auth cookies in local session lambda configuration." + type = string + default = "key" +} + +variable "local_auth_session_max_duration_minutes" { + description = "Maximum auth session duration in minutes for local login lambda configuration." + type = string + default = "60" +} + +variable "local_auth_access_token_expiry_duration_minutes" { + description = "Access token expiry duration in minutes for local login lambda configuration." + type = string + default = "60" +} + +variable "local_auth_refresh_token_expiry_duration_minutes" { + description = "Refresh token expiry duration in minutes for local login lambda configuration." + type = string + default = "60" +} + +variable "local_auth_cookie_same_site" { + description = "SameSite policy used for auth cookie generation in local login lambda configuration." + type = string + default = "Lax" +} + +variable "local_auth_cookie_secure" { + description = "Whether auth cookie generation should set the Secure flag in local login lambda configuration." + type = string + default = "false" +} diff --git a/scripts/terraform/post-apply-env-update.sh b/scripts/terraform/post-apply-env-update.sh index f524210d4..f432f1d48 100644 --- a/scripts/terraform/post-apply-env-update.sh +++ b/scripts/terraform/post-apply-env-update.sh @@ -59,6 +59,8 @@ LOCALHOST_BACKEND_BASE_URL=${BACKEND_BASE_URL/127.0.0.1/localhost} NHS_LOGIN_AUTHORIZE_URL=$(terraform -chdir=local-environment/infra output -raw nhs_login_authorize_url) USE_WIREMOCK_AUTH=$(terraform -chdir=local-environment/infra output -raw use_wiremock_auth) SUPPLIER_SERVICE_URL=$(terraform -chdir=local-environment/infra output -raw supplier_service_url) +NHS_LOGIN_CLIENT_ID=$(terraform -chdir=local-environment/infra output -raw nhs_login_client_id) +NHS_LOGIN_SCOPE=$(terraform -chdir=local-environment/infra output -raw nhs_login_scope) case "$USE_WIREMOCK_AUTH" in true) TESTS_AUTH_TYPE="wiremock" ;; @@ -69,7 +71,7 @@ case "$USE_WIREMOCK_AUTH" in ;; esac -printf 'NEXT_PUBLIC_BACKEND_URL=%s\nNEXT_PUBLIC_NHS_LOGIN_AUTHORIZE_URL=%s\nNEXT_PUBLIC_USE_WIREMOCK_AUTH=%s\n' "$LOCALHOST_BACKEND_BASE_URL" "$NHS_LOGIN_AUTHORIZE_URL" "$USE_WIREMOCK_AUTH" > ./ui/.env.local +printf 'NEXT_PUBLIC_BACKEND_URL=%s\nNEXT_PUBLIC_NHS_LOGIN_AUTHORIZE_URL=%s\nNEXT_PUBLIC_USE_WIREMOCK_AUTH=%s\nNEXT_PUBLIC_NHS_LOGIN_CLIENT_ID=%s\nNEXT_PUBLIC_NHS_LOGIN_SCOPE=%s\n' "$LOCALHOST_BACKEND_BASE_URL" "$NHS_LOGIN_AUTHORIZE_URL" "$USE_WIREMOCK_AUTH" "$NHS_LOGIN_CLIENT_ID" "$NHS_LOGIN_SCOPE" > ./ui/.env.local TESTS_ENV_FILE=./tests/configuration/.env.local mkdir -p "$(dirname "$TESTS_ENV_FILE")" diff --git a/ui/package-lock.json b/ui/package-lock.json index 329cc3eb8..03b5821c8 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -96,6 +96,7 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -683,6 +684,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=18" }, @@ -706,6 +708,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=18" } @@ -2365,6 +2368,7 @@ "integrity": "sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.10.4", "@babel/runtime": "^7.12.5", @@ -2644,6 +2648,7 @@ "integrity": "sha512-A1sre26ke7HDIuY/M23nd9gfB+nrmhtYyMINbjI1zHJxYteKR6qSMX56FsmjMcDb3SMcjJg5BiRRgOCC/yBD0g==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -2654,6 +2659,7 @@ "integrity": "sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -2664,6 +2670,7 @@ "integrity": "sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==", "dev": true, "license": "MIT", + "peer": true, "peerDependencies": { "@types/react": "^19.2.0" } @@ -2744,6 +2751,7 @@ "integrity": "sha512-rLoGZIf9afaRBYsPUMtvkDWykwXwUPL60HebR4JgTI8mxfFe2cQTu3AGitANp4b9B2QlVru6WzjgB2IzJKiCSA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.58.0", "@typescript-eslint/types": "8.58.0", @@ -3224,6 +3232,7 @@ "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -3651,6 +3660,7 @@ "integrity": "sha512-Ixm8tFfoKKIPYdCCKYTsqv+Fd4IJ0DQqMyEimo+pxUOMUR9cVPlwTrFt9Avu+3cb6Zp3mAzl+t1MrG2fxxKsxw==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/types": "^7.26.0" } @@ -3767,6 +3777,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.10.12", "caniuse-lite": "^1.0.30001782", @@ -4659,6 +4670,7 @@ "integrity": "sha512-XoMjdBOwe/esVgEvLmNsD3IRHkm7fbKIUGvrleloJXUZgDHig2IPWNniv+GwjyJXzuNqVjlr5+4yVUZjycJwfQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -4809,6 +4821,7 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -7604,6 +7617,7 @@ "integrity": "sha512-Cvc9WUhxSMEo4McES3P7oK3QaXldCfNWp7pl2NNeiIFlCoLr3kfq9kb1fxftiwk1FLV7CvpvDfonxtzUDeSOPg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "cssstyle": "^4.2.1", "data-urls": "^5.0.0", @@ -8100,6 +8114,7 @@ "resolved": "https://registry.npmjs.org/nhsuk-frontend/-/nhsuk-frontend-10.4.2.tgz", "integrity": "sha512-DYa7E/jwWtQPKqzeF9eB9nVcTKHpjMYf+SydKao379qQapIkblfS2BNvKsVKuWpI0w+QgI8XSDNNOUTQEGRb1w==", "license": "MIT", + "peer": true, "engines": { "node": "^20.9.0 || ^22.11.0 || >= 24.11.0" }, @@ -8556,6 +8571,7 @@ "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -8790,6 +8806,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.5.tgz", "integrity": "sha512-llUJLzz1zTUBrskt2pwZgLq59AemifIftw4aB7JxOqf1HY2FDaGDxgwpAPVzHU1kdWabH7FauP4i1oEeer2WCA==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -8799,6 +8816,7 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.5.tgz", "integrity": "sha512-J5bAZz+DXMMwW/wV3xzKke59Af6CHY7G4uYLN1OvBcKEsWOs4pQExj86BBKamxl/Ik5bx9whOrvBlSDfWzgSag==", "license": "MIT", + "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -9947,6 +9965,7 @@ "integrity": "sha512-f0FFpIdcHgn8zcPSbf1dRevwt047YMnaiJM3u2w2RewrB+fob/zePZcrOyQoLMMO7aBIddLcQIEK5dYjkLnGrQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@cspotcode/source-map-support": "^0.8.0", "@tsconfig/node10": "^1.0.7", @@ -10025,7 +10044,8 @@ "version": "2.8.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", - "license": "0BSD" + "license": "0BSD", + "peer": true }, "node_modules/type-check": { "version": "0.4.0", @@ -10147,6 +10167,7 @@ "integrity": "sha512-bGdAIrZ0wiGDo5l8c++HWtbaNCWTS4UTv7RaTH/ThVIgjkveJt83m74bBHMJkuCbslY8ixgLBVZJIOiQlQTjfQ==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -10760,6 +10781,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-4.3.6.tgz", "integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/ui/src/__tests__/lib/services/login-service.test.ts b/ui/src/__tests__/lib/services/login-service.test.ts new file mode 100644 index 000000000..eda40ea74 --- /dev/null +++ b/ui/src/__tests__/lib/services/login-service.test.ts @@ -0,0 +1,96 @@ +import { mapAuthUser } from "@/lib/auth/mapAuthUser"; +import loginService from "@/lib/services/login-service"; +import type { AuthUser } from "@/state"; + +jest.mock("@/settings", () => ({ backendUrl: "http://mock-backend" })); +jest.mock("@/lib/auth/mapAuthUser", () => ({ mapAuthUser: jest.fn() })); + +const mockFetch = jest.fn(); +globalThis.fetch = mockFetch as typeof fetch; + +const mockedMapAuthUser = jest.mocked(mapAuthUser); + +describe("LoginService", () => { + const apiUrl = "http://mock-backend/login"; + + const apiUser = { + sub: "user-123", + nhs_number: "9686368973", + birthdate: "1968-02-12", + email: "test@example.com", + identity_proofing_level: "P9", + phone_number: "+447887510886", + family_name: "MILLAR", + given_name: "Alice", + }; + + const mappedUser: AuthUser = { + sub: "user-123", + nhsNumber: "9686368973", + birthdate: "1968-02-12", + email: "test@example.com", + identityProofingLevel: "P9", + phoneNumber: "+447887510886", + familyName: "MILLAR", + givenName: "Alice", + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockedMapAuthUser.mockReturnValue(mappedUser); + }); + + it("calls the login endpoint with the auth code and returns the mapped user", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: async () => apiUser, + }); + + const result = await loginService.login("abc123"); + + expect(result).toEqual(mappedUser); + expect(mockFetch).toHaveBeenCalledWith( + apiUrl, + expect.objectContaining({ + method: "POST", + headers: expect.objectContaining({ + "Content-Type": "application/json", + }), + body: JSON.stringify({ code: "abc123" }), + credentials: "include", + }), + ); + expect(mockedMapAuthUser).toHaveBeenCalledWith(apiUser); + }); + + it("throws when the login endpoint returns a non-OK response", async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 401, + text: async () => "Unauthorized", + }); + + await expect(loginService.login("bad-code")).rejects.toThrow("HTTP 401: Unauthorized"); + }); + + it("throws when the login endpoint returns a 500 response", async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 500, + text: async () => "Internal Server Error", + }); + + await expect(loginService.login("any-code")).rejects.toThrow("HTTP 500: Internal Server Error"); + }); + + it("throws when NEXT_PUBLIC_BACKEND_URL is not configured", async () => { + jest.resetModules(); + jest.doMock("@/settings", () => ({ backendUrl: "" })); + + const { default: freshLoginService } = await import("@/lib/services/login-service"); + + await expect(freshLoginService.login("abc123")).rejects.toThrow( + "Missing NEXT_PUBLIC_BACKEND_URL", + ); + }); +}); diff --git a/ui/src/__tests__/routes/CallbackPage.test.tsx b/ui/src/__tests__/routes/CallbackPage.test.tsx new file mode 100644 index 000000000..d2d3c1220 --- /dev/null +++ b/ui/src/__tests__/routes/CallbackPage.test.tsx @@ -0,0 +1,168 @@ +import "@testing-library/jest-dom"; +import { act, render, waitFor } from "@testing-library/react"; + +import { consumeLoginCsrf, verifyState } from "@/lib/auth/loginState"; +import { RoutePath } from "@/lib/models/route-paths"; +import loginService from "@/lib/services/login-service"; +import CallbackPage from "@/routes/CallbackPage"; +import { useAuth } from "@/state"; + +const mockNavigate = jest.fn(); +const mockSetUser = jest.fn(); +const mockHandleAsyncError = jest.fn(); + +jest.mock("react-router-dom", () => ({ + useNavigate: () => mockNavigate, +})); + +jest.mock("@/state", () => ({ + useAuth: jest.fn(), +})); + +jest.mock("@/lib/auth/loginState", () => ({ + consumeLoginCsrf: jest.fn(), + verifyState: jest.fn(), +})); + +jest.mock("@/lib/services/login-service", () => ({ + __esModule: true, + default: { login: jest.fn() }, +})); + +// Expose the handler directly so effects under test remain synchronous where possible. +jest.mock("@/hooks", () => ({ + useAsyncErrorHandler: (fn: (...args: unknown[]) => unknown) => { + return (...args: unknown[]) => { + try { + const result = fn(...args); + + if (result instanceof Promise) { + void result.catch((error) => { + mockHandleAsyncError(error); + }); + } + + return result; + } catch (error) { + mockHandleAsyncError(error); + return undefined; + } + }; + }, +})); + +const mockedConsumeLoginCsrf = jest.mocked(consumeLoginCsrf); +const mockedVerifyState = jest.mocked(verifyState); +const mockedLoginService = jest.mocked(loginService); +const mockedUseAuth = jest.mocked(useAuth); + +describe("CallbackPage", () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedUseAuth.mockReturnValue({ user: null, setUser: mockSetUser }); + }); + + afterEach(() => { + globalThis.history.replaceState({}, "", "/"); + jest.clearAllMocks(); + }); + + it("does nothing when code is absent from search params", async () => { + globalThis.history.replaceState({}, "", "/callback"); + + await act(async () => { + render(); + }); + + expect(mockedLoginService.login).not.toHaveBeenCalled(); + expect(mockSetUser).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it("navigates to LoginPage when CSRF is missing from session storage", async () => { + globalThis.history.replaceState({}, "", "/callback?code=abc123&state=some-state"); + mockedConsumeLoginCsrf.mockReturnValue(null); + + render(); + + await waitFor(() => expect(mockNavigate).toHaveBeenCalledWith(RoutePath.LoginPage)); + expect(mockSetUser).not.toHaveBeenCalled(); + }); + + it("navigates to LoginPage when verifyState throws", async () => { + globalThis.history.replaceState({}, "", "/callback?code=abc123&state=bad-state"); + mockedConsumeLoginCsrf.mockReturnValue("csrf-token"); + mockedVerifyState.mockImplementation(() => { + throw new Error("Invalid state"); + }); + + render(); + + await waitFor(() => expect(mockNavigate).toHaveBeenCalledWith(RoutePath.LoginPage)); + expect(mockSetUser).not.toHaveBeenCalled(); + }); + + it("calls setUser and navigates to returnTo path on successful auth", async () => { + globalThis.history.replaceState({}, "", "/callback?code=abc123&state=valid-state"); + mockedConsumeLoginCsrf.mockReturnValue("csrf-token"); + mockedVerifyState.mockReturnValue("/get-self-test-kit-for-HIV"); + + const mappedUser = { + sub: "user-123", + nhsNumber: "9686368973", + birthdate: "1968-02-12", + email: "test@example.com", + identityProofingLevel: "P9", + phoneNumber: "+447887510886", + familyName: "MILLAR", + givenName: "Alice", + }; + mockedLoginService.login.mockResolvedValue(mappedUser); + + render(); + + await waitFor(() => expect(mockSetUser).toHaveBeenCalledWith(mappedUser)); + expect(mockNavigate).toHaveBeenCalledWith("/get-self-test-kit-for-HIV"); + expect(mockedLoginService.login).toHaveBeenCalledWith("abc123"); + }); + + it("navigates to GetSelfTestKitPage when returnTo resolves to null", async () => { + globalThis.history.replaceState({}, "", "/callback?code=abc123&state=valid-state"); + mockedConsumeLoginCsrf.mockReturnValue("csrf-token"); + mockedVerifyState.mockReturnValue(null); + + const mappedUser = { + sub: "u", + nhsNumber: "n", + birthdate: "b", + email: "e", + identityProofingLevel: "P9", + phoneNumber: "p", + familyName: "F", + givenName: "G", + }; + mockedLoginService.login.mockResolvedValue(mappedUser); + + render(); + + await waitFor(() => expect(mockNavigate).toHaveBeenCalledWith(RoutePath.GetSelfTestKitPage)); + }); + + it("captures an async error when loginService.login throws", async () => { + globalThis.history.replaceState({}, "", "/callback?code=abc123&state=valid-state"); + mockedConsumeLoginCsrf.mockReturnValue("csrf-token"); + mockedVerifyState.mockReturnValue("/get-self-test-kit-for-HIV"); + mockedLoginService.login.mockRejectedValue(new Error("HTTP 500: backend failed")); + + render(); + + await waitFor(() => expect(mockHandleAsyncError).toHaveBeenCalledTimes(1)); + + expect(mockHandleAsyncError.mock.calls[0][0]).toBeInstanceOf(Error); + expect((mockHandleAsyncError.mock.calls[0][0] as Error).message).toBe( + "HTTP 500: backend failed", + ); + expect(mockSetUser).not.toHaveBeenCalled(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/src/__tests__/routes/LoginPage.test.tsx b/ui/src/__tests__/routes/LoginPage.test.tsx index a5fd277c9..7aa06f590 100644 --- a/ui/src/__tests__/routes/LoginPage.test.tsx +++ b/ui/src/__tests__/routes/LoginPage.test.tsx @@ -2,7 +2,9 @@ import { render, waitFor } from "@testing-library/react"; import { getAuthorizeLoginHintFragment } from "@/lib/auth/loginHint"; import { generateState, persistLoginCsrf } from "@/lib/auth/loginState"; -import LoginPage from "@/routes/LoginPage"; +import { RoutePath } from "@/lib/models/route-paths"; + +const mockNavigate = jest.fn(); jest.mock("@/lib/auth/loginHint", () => ({ getAuthorizeLoginHintFragment: jest.fn(), @@ -13,20 +15,59 @@ jest.mock("@/lib/auth/loginState", () => ({ persistLoginCsrf: jest.fn(), })); -jest.mock("@/settings", () => ({ - nhsLoginAuthorizeUrl: "https://auth.example.test/authorize", +jest.mock("@/settings", () => { + const values = { + nhsLoginAuthorizeUrl: "https://auth.example.test/authorize", + nhsLoginClientId: "hometest", + nhsLoginScope: "openid profile email phone", + }; + + return { + __esModule: true, + get nhsLoginAuthorizeUrl() { + return values.nhsLoginAuthorizeUrl; + }, + get nhsLoginClientId() { + return values.nhsLoginClientId; + }, + get nhsLoginScope() { + return values.nhsLoginScope; + }, + __setMockSettings( + nextValues: Partial<{ + nhsLoginAuthorizeUrl: string; + nhsLoginClientId: string; + nhsLoginScope: string; + }>, + ) { + Object.assign(values, nextValues); + }, + }; +}); + +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useNavigate: () => mockNavigate, })); const mockedGetAuthorizeLoginHintFragment = jest.mocked(getAuthorizeLoginHintFragment); const mockedGenerateState = jest.mocked(generateState); const mockedPersistLoginCsrf = jest.mocked(persistLoginCsrf); -describe("LoginPage", () => { - let randomSpy: jest.SpyInstance; +interface MockedSettingsModule { + __setMockSettings: ( + nextValues: Partial<{ + nhsLoginAuthorizeUrl: string; + nhsLoginClientId: string; + nhsLoginScope: string; + }>, + ) => void; +} + +const mockedSettings = jest.requireMock("@/settings") as MockedSettingsModule; +describe("LoginPage", () => { beforeEach(() => { - // Stabilise the nonce so the useEffect is deterministic under JSDOM. - randomSpy = jest.spyOn(Math, "random").mockReturnValue(0.1234); mockedGetAuthorizeLoginHintFragment.mockReturnValue("&login_hint=stored-hint"); mockedGenerateState.mockReturnValue({ csrf: "csrf-token", @@ -40,12 +81,17 @@ describe("LoginPage", () => { }); afterEach(() => { - randomSpy.mockRestore(); globalThis.history.replaceState({}, "", "/"); + mockedSettings.__setMockSettings({ + nhsLoginAuthorizeUrl: "https://auth.example.test/authorize", + nhsLoginClientId: "hometest", + nhsLoginScope: "openid profile email phone", + }); jest.clearAllMocks(); }); it("builds an NHS Login redirect URL with the encoded return target", async () => { + const { default: LoginPage } = await import("@/routes/LoginPage"); render(); await waitFor(() => { @@ -59,6 +105,7 @@ describe("LoginPage", () => { it("defaults returnTo to the root path when it is absent", async () => { mockedGetAuthorizeLoginHintFragment.mockReturnValue(""); globalThis.history.replaceState({}, "", "/login"); + const { default: LoginPage } = await import("@/routes/LoginPage"); render(); @@ -68,4 +115,27 @@ describe("LoginPage", () => { expect(mockedGetAuthorizeLoginHintFragment).toHaveBeenCalledWith(null); }); + + it("navigates to service error and skips redirect URL generation when config is missing", async () => { + mockedSettings.__setMockSettings({ nhsLoginClientId: "" }); + const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => undefined); + const { default: LoginPage } = await import("@/routes/LoginPage"); + + render(); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith(RoutePath.ServiceErrorPage, { + replace: true, + state: { + errorMessage: + "Missing NHS Login configuration. Ensure NEXT_PUBLIC_NHS_LOGIN_AUTHORIZE_URL, NEXT_PUBLIC_NHS_LOGIN_CLIENT_ID, and NEXT_PUBLIC_NHS_LOGIN_SCOPE are set.", + }, + }); + }); + + expect(mockedGetAuthorizeLoginHintFragment).not.toHaveBeenCalled(); + expect(mockedGenerateState).not.toHaveBeenCalled(); + expect(mockedPersistLoginCsrf).not.toHaveBeenCalled(); + consoleErrorSpy.mockRestore(); + }); }); diff --git a/ui/src/lib/services/login-service.ts b/ui/src/lib/services/login-service.ts new file mode 100644 index 000000000..f9fc3acac --- /dev/null +++ b/ui/src/lib/services/login-service.ts @@ -0,0 +1,31 @@ +import { mapAuthUser } from "@/lib/auth/mapAuthUser"; +import { backendUrl } from "@/settings"; +import type { AuthUser } from "@/state"; + +class LoginService { + async login(code: string): Promise { + if (!backendUrl || backendUrl.trim() === "") { + throw new Error("Missing NEXT_PUBLIC_BACKEND_URL"); + } + + const response = await fetch(`${backendUrl}/login`, { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify({ code }), + credentials: "include", + }); + + if (!response.ok) { + const text = await response.text(); + throw new Error(`HTTP ${response.status}: ${text}`); + } + + const data = await response.json(); + return mapAuthUser(data); + } +} + +const loginService = new LoginService(); +export default loginService; diff --git a/ui/src/routes/CallbackPage.tsx b/ui/src/routes/CallbackPage.tsx index 25addc74e..21253e1d9 100644 --- a/ui/src/routes/CallbackPage.tsx +++ b/ui/src/routes/CallbackPage.tsx @@ -1,14 +1,13 @@ "use client"; -import { useAuth } from "@/state"; -import { mapAuthUser } from "@/lib/auth/mapAuthUser"; -import { consumeLoginCsrf, verifyState } from "@/lib/auth/loginState"; import { useEffect, useRef } from "react"; - -import { RoutePath } from "@/lib/models/route-paths"; -import { backendUrl } from "@/settings"; import { useNavigate } from "react-router-dom"; + import { useAsyncErrorHandler } from "@/hooks"; +import { consumeLoginCsrf, verifyState } from "@/lib/auth/loginState"; +import { RoutePath } from "@/lib/models/route-paths"; +import loginService from "@/lib/services/login-service"; +import { useAuth } from "@/state"; function safeReturnTo(value: string | null | undefined) { if (!value) return null; @@ -38,10 +37,6 @@ export default function CallbackPage() { const navigate = useNavigate(); const didRun = useRef(false); const handleCallback = useAsyncErrorHandler(async () => { - if (!backendUrl || backendUrl.trim() === "") { - console.error("Missing NEXT_PUBLIC_BACKEND_URL"); - throw new Error("Missing NEXT_PUBLIC_BACKEND_URL"); - } const params = new URLSearchParams(globalThis.location.search); const code = params.get("code"); const stateParam = params.get("state"); @@ -58,20 +53,7 @@ export default function CallbackPage() { return; } - const response = await fetch(`${backendUrl}/login`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ code }), - credentials: "include", - }); - - if (!response.ok) { - const text = await response.text(); - throw new Error(`HTTP ${response.status}: ${text}`); - } - - const data = await response.json(); - const userData = mapAuthUser(data); + const userData = await loginService.login(code); setUser(userData); navigate(returnTo ?? RoutePath.GetSelfTestKitPage); diff --git a/ui/src/routes/LoginPage.tsx b/ui/src/routes/LoginPage.tsx index 8c4f8de87..c1b303a9c 100644 --- a/ui/src/routes/LoginPage.tsx +++ b/ui/src/routes/LoginPage.tsx @@ -1,13 +1,32 @@ "use client"; import { useEffect } from "react"; +import { useNavigate } from "react-router-dom"; import { getAuthorizeLoginHintFragment } from "@/lib/auth/loginHint"; import { generateState, persistLoginCsrf } from "@/lib/auth/loginState"; -import { nhsLoginAuthorizeUrl } from "@/settings"; +import { RoutePath } from "@/lib/models/route-paths"; +import * as settings from "@/settings"; export default function RedirectPage() { + const navigate = useNavigate(); + useEffect(() => { + const authorizeUrl = settings.nhsLoginAuthorizeUrl?.trim(); + const clientId = settings.nhsLoginClientId?.trim(); + const scope = settings.nhsLoginScope?.trim(); + + if (!authorizeUrl || !clientId || !scope) { + const missingConfigError = + "Missing NHS Login configuration. Ensure NEXT_PUBLIC_NHS_LOGIN_AUTHORIZE_URL, NEXT_PUBLIC_NHS_LOGIN_CLIENT_ID, and NEXT_PUBLIC_NHS_LOGIN_SCOPE are set."; + console.error(missingConfigError); + navigate(RoutePath.ServiceErrorPage, { + replace: true, + state: { errorMessage: missingConfigError }, + }); + return; + } + // Generate your URL client-side const params = new URLSearchParams(globalThis.location.search); const returnTo = params.get("returnTo") ?? "/"; @@ -16,18 +35,17 @@ export default function RedirectPage() { const { csrf, encoded: state } = generateState(returnTo); persistLoginCsrf(csrf); - // ALPHA: Improve this to use proper values and env variables. - const nonce = Math.floor(1000 + Math.random() * 9000); + const nonce = globalThis.crypto.randomUUID(); const callbackUrl = encodeURIComponent(`${globalThis.location.origin}/callback`); globalThis.location.href = - `${nhsLoginAuthorizeUrl}` + + `${authorizeUrl}` + `?response_type=code` + - `&client_id=hometest` + + `&client_id=${clientId}` + `&redirect_uri=${callbackUrl}` + - `&scope=${encodeURIComponent("openid profile email phone")}` + + `&scope=${encodeURIComponent(scope)}` + `&state=${encodeURIComponent(state)}` + loginHintQuery + `&nonce=${nonce}`; - }, []); + }, [navigate]); return null; } diff --git a/ui/src/settings.ts b/ui/src/settings.ts index 8fac54bf6..2eef3ef11 100644 --- a/ui/src/settings.ts +++ b/ui/src/settings.ts @@ -1,5 +1,7 @@ const backendUrl = process.env.NEXT_PUBLIC_BACKEND_URL; const nhsLoginAuthorizeUrl = process.env.NEXT_PUBLIC_NHS_LOGIN_AUTHORIZE_URL; const useWiremockAuth = process.env.NEXT_PUBLIC_USE_WIREMOCK_AUTH === "true"; +const nhsLoginClientId = process.env.NEXT_PUBLIC_NHS_LOGIN_CLIENT_ID; +const nhsLoginScope = process.env.NEXT_PUBLIC_NHS_LOGIN_SCOPE; -export { backendUrl, nhsLoginAuthorizeUrl, useWiremockAuth }; +export { backendUrl, nhsLoginAuthorizeUrl, useWiremockAuth, nhsLoginClientId, nhsLoginScope };