From bab44b96c13b0aade4cb2dd27167142ed1236c13 Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Mon, 15 Jun 2026 18:18:30 +0100 Subject: [PATCH 01/10] refactor: remove useless DB features from JWKS --- app/auth/authenticate.js | 17 +++-- app/data-sources/JWKS.js | 24 ++++++++ app/data-sources/mongo/JWKS.js | 44 ------------- app/graphql/context.js | 4 +- docs/architecture.md | 4 +- test/unit/app/context.test.js | 10 +-- test/unit/auth/authenticate.test.js | 49 ++++++++++----- test/unit/data-sources/jwks.test.js | 71 +++++++++++++++++++++ test/unit/data-sources/mongo/jwks.test.js | 75 ----------------------- 9 files changed, 150 insertions(+), 148 deletions(-) create mode 100644 app/data-sources/JWKS.js delete mode 100644 app/data-sources/mongo/JWKS.js create mode 100644 test/unit/data-sources/jwks.test.js delete mode 100644 test/unit/data-sources/mongo/jwks.test.js diff --git a/app/auth/authenticate.js b/app/auth/authenticate.js index 01beb1cc..7d317b9c 100644 --- a/app/auth/authenticate.js +++ b/app/auth/authenticate.js @@ -1,7 +1,7 @@ import { getDirective, MapperKind, mapSchema } from '@graphql-tools/utils' import { Unit } from 'aws-embedded-metrics' import { defaultFieldResolver } from 'graphql' -import jwt from 'jsonwebtoken' +import { decodeProtectedHeader, jwtVerify } from 'jose' import { config } from '../config.js' import { Unauthorized } from '../errors/graphql.js' import { DAL_REQUEST_AUTHENTICATION_001 } from '../logger/codes.js' @@ -20,11 +20,13 @@ export async function getAuth(request, jwkDatasource) { code: DAL_REQUEST_AUTHENTICATION_001, request: { remoteAddress: request?.info?.remoteAddress } }) - const decodedToken = jwt.decode(token, { complete: true }) + const decodedToken = decodeProtectedHeader(token) const requestStart = Date.now() - const signingKey = await jwkDatasource.getPublicKey(decodedToken.header.kid) + const signingKey = await jwkDatasource.getPublicKey(decodedToken.kid) const requestTimeMs = Date.now() - requestStart - const verified = jwt.verify(token, signingKey) + const { payload: verified } = await jwtVerify(token, signingKey, { + algorithms: ['RS256'] + }) sendMetric('RequestTime', requestTimeMs, Unit.Milliseconds, { code: DAL_REQUEST_AUTHENTICATION_001 }) @@ -56,9 +58,14 @@ export async function getAuth(request, jwkDatasource) { }) } }) + return verified } catch (error) { - if (error.name === 'TokenExpiredError') { + if ( + error.name === 'TokenExpiredError' || + error.code === 'ERR_JWT_EXPIRED' || + error.name === 'JWTExpired' + ) { logger.warn('#DAL - request authentication - token expired', { error, code: DAL_REQUEST_AUTHENTICATION_001, diff --git a/app/data-sources/JWKS.js b/app/data-sources/JWKS.js new file mode 100644 index 00000000..9b4e43bd --- /dev/null +++ b/app/data-sources/JWKS.js @@ -0,0 +1,24 @@ +import { createRemoteJWKSet } from 'jose' +import { config } from '../config.js' + +let jwksSet = null + +export class JWKS { + getRemoteJwksSet() { + if (!jwksSet) { + const jwksUri = config.get('oidc.jwksURI') + const url = new URL(jwksUri) + + jwksSet = createRemoteJWKSet(url, { + timeout: config.get('oidc.timeoutMs') + }) + } + + return jwksSet + } + + async getPublicKey(kid) { + const getKey = this.getRemoteJwksSet() + return await getKey({ kid, alg: 'RS256' }) + } +} diff --git a/app/data-sources/mongo/JWKS.js b/app/data-sources/mongo/JWKS.js deleted file mode 100644 index 0bbb3f57..00000000 --- a/app/data-sources/mongo/JWKS.js +++ /dev/null @@ -1,44 +0,0 @@ -import { MongoDataSource } from 'apollo-datasource-mongodb' -import { HttpsProxyAgent } from 'https-proxy-agent' -import jwksClient from 'jwks-rsa' -import { config } from '../../config.js' - -export class MongoJWKS extends MongoDataSource { - async retrievePublicKeyByKid(kid) { - const jwk = await this.findOneById(kid) - return jwk?.publicKey - } - - async insertPublicKey(kid, publicKey) { - return this.collection.insertOne({ - _id: kid, - publicKey, - createdAt: new Date(), - updatedAt: new Date() - }) - } - - async fetchPublicKey(kid) { - const clientConfig = { - jwksUri: config.get('oidc.jwksURI'), - timeout: config.get('oidc.timeoutMs') - } - - if (config.get('cdp.httpsProxy')) { - clientConfig.requestAgent = new HttpsProxyAgent(config.get('cdp.httpsProxy')) - } - - const client = jwksClient({ - ...clientConfig - }) - - const key = await client.getSigningKey(kid) - const publicKey = key.getPublicKey() - await this.insertPublicKey(kid, publicKey) - return publicKey - } - - async getPublicKey(kid) { - return (await this.retrievePublicKeyByKid(kid)) ?? this.fetchPublicKey(kid) - } -} diff --git a/app/graphql/context.js b/app/graphql/context.js index 960540c2..69d49894 100644 --- a/app/graphql/context.js +++ b/app/graphql/context.js @@ -1,9 +1,9 @@ import jwt from 'jsonwebtoken' import { getAuth, getRequestingGroup } from '../auth/authenticate.js' import { HitachiPayments } from '../data-sources/hitachi/HitachiPayments.js' +import { JWKS } from '../data-sources/JWKS.js' import { MongoBusiness } from '../data-sources/mongo/Business.js' import { MongoCustomer } from '../data-sources/mongo/Customer.js' -import { MongoJWKS } from '../data-sources/mongo/JWKS.js' import { RuralPaymentsBusiness } from '../data-sources/rural-payments/RuralPaymentsBusiness.js' import { RuralPaymentsCustomer } from '../data-sources/rural-payments/RuralPaymentsCustomer.js' import { RuralPaymentsReferenceData } from '../data-sources/rural-payments/RuralPaymentsReferenceData.js' @@ -29,7 +29,7 @@ export const extractOrgIdFromDefraIdToken = (sbi, token) => { } export async function context({ request }) { - const auth = await getAuth(request, new MongoJWKS({ modelOrCollection: db.collection('jwks') })) + const auth = await getAuth(request, new JWKS()) const requestLogger = logger.child({ transactionId: request.transactionId, diff --git a/docs/architecture.md b/docs/architecture.md index 83599655..1feb6343 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -25,9 +25,7 @@ C4Context Boundary(cdp, "Core Development Platform (CDP)", "AWS VPC") { System(dal, "FCP-DAL-API", "Data Access Layer (DAL) provides GraphQL API") - System_Ext(mongo, "Mongo DB", "Caches JWKS public keys, -
- relations between organisation IDs / SBIs, + System_Ext(mongo, "Mongo DB", "Caches relations between organisation IDs / SBIs,
and relations between person IDs / CRNs") System_Ext(squid, "Squid Proxy", "Routes all outbound requests") diff --git a/test/unit/app/context.test.js b/test/unit/app/context.test.js index 4307858a..eb676f0f 100644 --- a/test/unit/app/context.test.js +++ b/test/unit/app/context.test.js @@ -7,7 +7,7 @@ const RuralPaymentsBusinessMock = jest.fn() const RuralPaymentsCustomerMock = jest.fn() const MongoCustomerMock = jest.fn() const MongoBusinessMock = jest.fn() -const MongoJWKSMock = jest.fn() +const JWKSMock = jest.fn() const loggerChild = jest.fn() const loggerMock = { child: loggerChild } @@ -36,8 +36,8 @@ jest.unstable_mockModule('../../../app/data-sources/mongo/Business.js', () => ({ jest.unstable_mockModule('../../../app/data-sources/mongo/Customer.js', () => ({ MongoCustomer: MongoCustomerMock })) -jest.unstable_mockModule('../../../app/data-sources/mongo/JWKS.js', () => ({ - MongoJWKS: MongoJWKSMock +jest.unstable_mockModule('../../../app/data-sources/JWKS.js', () => ({ + JWKS: JWKSMock })) jest.unstable_mockModule('../../../app/logger/logger.js', () => ({ logger: loggerMock @@ -52,7 +52,7 @@ describe('context', () => { test('should build context with correct properties', async () => { getAuthMock.mockResolvedValue({ user: 'test-user' }) PermissionsMock.mockImplementation(() => ({ type: 'Permissions' })) - MongoJWKSMock.mockImplementation(() => ({})) + JWKSMock.mockImplementation(() => ({})) loggerChild.mockReturnValue({ log: jest.fn() }) const request = { headers: { @@ -65,7 +65,7 @@ describe('context', () => { const result = await context({ request }) - expect(getAuthMock).toHaveBeenCalledWith(request, MongoJWKSMock()) + expect(getAuthMock).toHaveBeenCalledWith(request, JWKSMock()) expect(loggerMock.child).toHaveBeenCalledWith({ transactionId: 'tx-1', traceId: 'trace-1' diff --git a/test/unit/auth/authenticate.test.js b/test/unit/auth/authenticate.test.js index 40420619..afdb29a7 100644 --- a/test/unit/auth/authenticate.test.js +++ b/test/unit/auth/authenticate.test.js @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, jest, test } from '@jest/globals' import { buildSchema, findBreakingChanges } from 'graphql' import jwt from 'jsonwebtoken' +import { generateKeyPairSync } from 'node:crypto' import { config } from '../../../app/config.js' import { Unauthorized } from '../../../app/errors/graphql.js' @@ -35,10 +36,23 @@ const tokenPayload = { azp: 'azp-id' } -const token = jwt.sign({ ...tokenPayload, email: 'pii@defra.gov.uk' }, 'secret', { - expiresIn: '1h' +const { publicKey, privateKey } = generateKeyPairSync('rsa', { + modulusLength: 2048 +}) +const { privateKey: wrongPrivateKey } = generateKeyPairSync('rsa', { + modulusLength: 2048 +}) + +const token = jwt.sign({ ...tokenPayload, email: 'pii@defra.gov.uk' }, privateKey, { + algorithm: 'RS256', + expiresIn: '1h', + keyid: 'mock-key-id-123' +}) +const tokenDiffSecret = jwt.sign(tokenPayload, wrongPrivateKey, { + algorithm: 'RS256', + expiresIn: '1h', + keyid: 'mock-key-id-123' }) -const tokenDiffSecret = jwt.sign(tokenPayload, 'secret2', { expiresIn: '1h' }) const requestInfo = { remoteAddress: '0.0.0.0' } const mockRequest = (token) => ({ headers: { @@ -46,7 +60,7 @@ const mockRequest = (token) => ({ }, info: requestInfo }) -const decodedToken = jwt.verify(token, 'secret') +const decodedToken = jwt.decode(token) const mockPublicKeyFunc = jest.fn() const mockJWKSDataSource = { getPublicKey: mockPublicKeyFunc } @@ -61,9 +75,11 @@ describe('getAuth', () => { describe('with a valid token', () => { test('should return decoded token, and log payload details', async () => { - mockPublicKeyFunc.mockReturnValue('secret') - expect(await getAuth(mockRequest(token), mockJWKSDataSource)).toEqual(decodedToken) - expect(mockPublicKeyFunc).toHaveBeenCalledWith(undefined) + mockPublicKeyFunc.mockResolvedValue(publicKey) + const tokenPayload = await getAuth(mockRequest(token), mockJWKSDataSource) + + expect(tokenPayload).toEqual(decodedToken) + expect(mockPublicKeyFunc).toHaveBeenCalledWith('mock-key-id-123') expect(info).toHaveBeenCalledTimes(1) expect(info.mock.calls[0]).toEqual([ '#DAL Request authentication - JWT verified', @@ -89,12 +105,17 @@ describe('getAuth', () => { }) test('should return decoded token, and log payload details (no email check)', async () => { - mockPublicKeyFunc.mockReturnValue('secret') - const tokenNoEmail = jwt.sign(tokenPayload, 'secret', { expiresIn: '1h' }) + mockPublicKeyFunc.mockResolvedValue(publicKey) + const tokenNoEmail = jwt.sign(tokenPayload, privateKey, { + algorithm: 'RS256', + expiresIn: '1h', + keyid: 'mock-key-id-123' + }) + expect(await getAuth(mockRequest(tokenNoEmail), mockJWKSDataSource)).toEqual( - jwt.decode(tokenNoEmail, 'secret') + jwt.decode(tokenNoEmail) ) - expect(mockPublicKeyFunc).toHaveBeenCalledWith(undefined) + expect(mockPublicKeyFunc).toHaveBeenCalledWith('mock-key-id-123') expect(info).toHaveBeenCalledTimes(1) expect(info.mock.calls[0]).toEqual([ '#DAL Request authentication - JWT verified', @@ -126,8 +147,9 @@ describe('getAuth', () => { }) test('should return an empty object when token verification fails, due to incorrect signing key', async () => { + mockPublicKeyFunc.mockResolvedValue(publicKey) expect(await getAuth(mockRequest(tokenDiffSecret), mockJWKSDataSource)).toEqual({}) - expect(mockPublicKeyFunc).toHaveBeenCalledWith(undefined) + expect(mockPublicKeyFunc).toHaveBeenCalledWith('mock-key-id-123') }) test('should return an empty object when token verification fails, due to token expiry', async () => { @@ -137,7 +159,7 @@ describe('getAuth', () => { throw error }) expect(await getAuth(mockRequest(token), mockJWKSDataSource)).toEqual({}) - expect(mockPublicKeyFunc).toHaveBeenCalledWith(undefined) + expect(mockPublicKeyFunc).toHaveBeenCalledWith('mock-key-id-123') }) }) @@ -249,7 +271,6 @@ describe('authDirectiveTransformer', () => { } directive @auth(requires: AuthRole = TEST) on OBJECT | FIELD_DEFINITION - `) const originalConfig = { ...config } diff --git a/test/unit/data-sources/jwks.test.js b/test/unit/data-sources/jwks.test.js new file mode 100644 index 00000000..cc6edbb2 --- /dev/null +++ b/test/unit/data-sources/jwks.test.js @@ -0,0 +1,71 @@ +import { beforeAll } from '@jest/globals' +import { generateKeyPairSync } from 'node:crypto' +import { jwtVerify } from 'jose' +import jwt from 'jsonwebtoken' +import nock from 'nock' +import { config } from '../../../app/config.js' +const { JWKS } = await import('../../../app/data-sources/JWKS.js') + +describe('getJwtPublicKey', () => { + const { publicKey, privateKey } = generateKeyPairSync('rsa', { + modulusLength: 2048 + }) + + beforeAll(() => { + nock.disableNetConnect() + }) + + beforeEach(() => { + nock(config.get('oidc.jwksURI')) + .get('/') + .reply(200, { + keys: [ + { + kty: 'RSA', + kid: 'mock-key-id-123', + alg: 'RS256', + use: 'sig', + n: publicKey.export({ format: 'jwk' }).n, + e: publicKey.export({ format: 'jwk' }).e + } + ] + }) + nock(config.get('oidc.jwksURI')) + .get('/') + .reply(200, { + keys: [ + { + kty: 'RSA', + kid: 'mock-key-id-123', + alg: 'RS256', + use: 'sig', + n: publicKey.export({ format: 'jwk' }).n, + e: publicKey.export({ format: 'jwk' }).e + } + ] + }) + }) + + afterAll(() => { + nock.cleanAll() + nock.enableNetConnect() + }) + + it('should return the public key specified by Key ID and algorithm', async () => { + const jwks = new JWKS() + const mockTokenPayload = { + iat: Math.floor(Date.now() / 1000) + } + const mockToken = jwt.sign(mockTokenPayload, privateKey, { + algorithm: 'RS256' + }) + + expect( + ( + await jwtVerify(mockToken, await jwks.getPublicKey('mock-key-id-123'), { + algorithms: ['RS256'] + }) + ).payload + ).toMatchObject(mockTokenPayload) + }) +}) diff --git a/test/unit/data-sources/mongo/jwks.test.js b/test/unit/data-sources/mongo/jwks.test.js deleted file mode 100644 index a591999d..00000000 --- a/test/unit/data-sources/mongo/jwks.test.js +++ /dev/null @@ -1,75 +0,0 @@ -import { beforeAll } from '@jest/globals' -import { generateKeyPairSync } from 'crypto' -import jwt from 'jsonwebtoken' -import { MongoClient } from 'mongodb' -import nock from 'nock' -import { config } from '../../../../app/config.js' - -const client = new MongoClient(config.get('mongo.mongoUrl')) -client.connect() -const db = client.db(config.get('mongo.databaseName')) -const { MongoJWKS } = await import('../../../../app/data-sources/mongo/JWKS.js') - -describe('getJwtPublicKey', () => { - const { publicKey, privateKey } = generateKeyPairSync('rsa', { - modulusLength: 2048 - }) - - beforeAll(() => { - nock.disableNetConnect() - }) - - beforeEach(() => { - nock(config.get('oidc.jwksURI')) - .get('/') - .reply(200, { - keys: [ - { - kty: 'RSA', - kid: 'mock-key-id-123', - alg: 'RS256', - use: 'sig', - n: publicKey.export({ format: 'jwk' }).n, - e: publicKey.export({ format: 'jwk' }).e - } - ] - }) - }) - - afterAll(() => { - nock.cleanAll() - nock.enableNetConnect() - }) - - it('should return the public key and proxy called', async () => { - const JWKS = new MongoJWKS({ modelOrCollection: db.collection('jwks') }) - - const mockTokenPayload = { - iat: Math.floor(Date.now() / 1000) - } - - const mockToken = jwt.sign(mockTokenPayload, privateKey, { - algorithm: 'RS256' - }) - - expect(jwt.verify(mockToken, await JWKS.getPublicKey('mock-key-id-123'))).toEqual( - mockTokenPayload - ) - }) - - it('should return the public key without proxy', async () => { - const JWKS = new MongoJWKS({ modelOrCollection: db.collection('jwks') }) - - const mockTokenPayload = { - iat: Math.floor(Date.now() / 1000) - } - - const mockToken = jwt.sign(mockTokenPayload, privateKey, { - algorithm: 'RS256' - }) - - expect(jwt.verify(mockToken, await JWKS.getPublicKey('mock-key-id-123'))).toEqual( - mockTokenPayload - ) - }) -}) From 941d375f5e07a72f94ac253ce62ca846453f7fab Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Mon, 15 Jun 2026 18:23:07 +0100 Subject: [PATCH 02/10] fix: hitachi API base URL cannot be null --- app/config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/config.js b/app/config.js index aa115ee4..691a3e92 100644 --- a/app/config.js +++ b/app/config.js @@ -228,7 +228,6 @@ export const config = convict({ doc: 'Hitachi base API URL', format: String, default: 'https://api.example.com', - nullable: true, env: 'HITACHI_BASE_URL' }, timeoutMs: { From eb9414e64b82c3f9b1029847e900561d5e3f326e Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Mon, 15 Jun 2026 18:24:26 +0100 Subject: [PATCH 03/10] chore: tidy dependencies inline with impl - switch `jwks-rsa` module for the underlying `jose`, and reduce deps - remove unused `https-proxy-agent`, use Node v24 auto-proxy feature --- package-lock.json | 188 +++++----------------------------------------- package.json | 3 +- 2 files changed, 21 insertions(+), 170 deletions(-) diff --git a/package-lock.json b/package-lock.json index 82e7c8e4..867c0ea2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28,9 +28,8 @@ "graphql": "^16.11.0", "graphql-scalars": "^1.25.0", "http-status-codes": "^2.3.0", - "https-proxy-agent": "^7.0.6", + "jose": "^6.2.3", "jsonwebtoken": "^9.0.2", - "jwks-rsa": "^3.1.0", "mongodb": "^7.1.1", "undici": "^7.28.0", "uuid": "^14.0.0", @@ -4858,6 +4857,9 @@ "arm64" ], "dev": true, + "libc": [ + "glibc" + ], "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -4876,6 +4878,9 @@ "arm64" ], "dev": true, + "libc": [ + "musl" + ], "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -4894,6 +4899,9 @@ "ppc64" ], "dev": true, + "libc": [ + "glibc" + ], "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -4912,6 +4920,9 @@ "s390x" ], "dev": true, + "libc": [ + "glibc" + ], "license": "Apache-2.0 AND MIT", "optional": true, "os": [ @@ -5112,49 +5123,6 @@ "@babel/types": "^7.28.2" } }, - "node_modules/@types/body-parser": { - "version": "1.19.6", - "resolved": "https://registry.npmjs.org/@types/body-parser/-/body-parser-1.19.6.tgz", - "integrity": "sha512-HLFeCYgz89uk22N5Qg3dvGvsv46B8GLvKKo1zKG4NybA8U2DiEO3w9lqGg29t/tfLRJpJ6iQxnVw4OnB7MoM9g==", - "license": "MIT", - "dependencies": { - "@types/connect": "*", - "@types/node": "*" - } - }, - "node_modules/@types/connect": { - "version": "3.4.38", - "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.38.tgz", - "integrity": "sha512-K6uROf1LD88uDQqJCktA4yzL1YYAK6NgfsI0v/mTgyPKWsX1CnJ0XPSDhViejru1GcRkLWb8RlzFYJRqGUbaug==", - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, - "node_modules/@types/express": { - "version": "4.17.25", - "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.25.tgz", - "integrity": "sha512-dVd04UKsfpINUnK0yBoYHDF3xu7xVH4BuDotC/xGuycx4CgbP48X/KF/586bcObxT0HENHXEU8Nqtu6NR+eKhw==", - "license": "MIT", - "dependencies": { - "@types/body-parser": "*", - "@types/express-serve-static-core": "^4.17.33", - "@types/qs": "*", - "@types/serve-static": "^1" - } - }, - "node_modules/@types/express-serve-static-core": { - "version": "4.19.7", - "resolved": "https://registry.npmjs.org/@types/express-serve-static-core/-/express-serve-static-core-4.19.7.tgz", - "integrity": "sha512-FvPtiIf1LfhzsaIXhv/PHan/2FeQBbtBDtfX2QfvPxdUelMDEckK08SM6nqo1MIZY3RUlfA+HV8+hFUSio78qg==", - "license": "MIT", - "dependencies": { - "@types/node": "*", - "@types/qs": "*", - "@types/range-parser": "*", - "@types/send": "*" - } - }, "node_modules/@types/graceful-fs": { "version": "4.1.9", "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.9.tgz", @@ -5171,12 +5139,6 @@ "integrity": "sha512-1m0bIFVc7eJWyve9S0RnuRgcQqF/Xd5QsUZAZeQFr1Q3/p9JWoQQEqmVy+DPTNpGXwhgIetAoYF8JSc33q29QA==", "license": "MIT" }, - "node_modules/@types/http-errors": { - "version": "2.0.5", - "resolved": "https://registry.npmjs.org/@types/http-errors/-/http-errors-2.0.5.tgz", - "integrity": "sha512-r8Tayk8HJnX0FztbZN7oVqGccWgw98T/0neJphO91KkmOzug1KkofZURD4UaD5uH8AqcFLfdPErnBod0u71/qg==", - "license": "MIT" - }, "node_modules/@types/istanbul-lib-coverage": { "version": "2.0.6", "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.6.tgz", @@ -5211,85 +5173,22 @@ "dev": true, "license": "MIT" }, - "node_modules/@types/jsonwebtoken": { - "version": "9.0.10", - "resolved": "https://registry.npmjs.org/@types/jsonwebtoken/-/jsonwebtoken-9.0.10.tgz", - "integrity": "sha512-asx5hIG9Qmf/1oStypjanR7iKTv0gXQ1Ov/jfrX6kS/EO0OFni8orbmGCn0672NHR3kXHwpAwR+B368ZGN/2rA==", - "license": "MIT", - "dependencies": { - "@types/ms": "*", - "@types/node": "*" - } - }, "node_modules/@types/long": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/@types/long/-/long-4.0.2.tgz", "integrity": "sha512-MqTGEo5bj5t157U6fA/BiDynNkn0YknVdh48CMPkTSpFTVmvao5UQmm7uEF6xBEo7qIMAlY/JSleYaE6VOdpaA==", "license": "MIT" }, - "node_modules/@types/mime": { - "version": "1.3.5", - "resolved": "https://registry.npmjs.org/@types/mime/-/mime-1.3.5.tgz", - "integrity": "sha512-/pyBZWSLD2n0dcHE3hq8s8ZvcETHtEuF+3E7XVt0Ig2nvsVQXdghHVcEkIWjy9A0wKfTn97a/PSDYohKIlnP/w==", - "license": "MIT" - }, - "node_modules/@types/ms": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/@types/ms/-/ms-2.1.0.tgz", - "integrity": "sha512-GsCCIZDE/p3i96vtEqx+7dBUGXrc7zeSK3wwPHIaRThS+9OhWIXRqzs4d6k1SVU8g91DrNRWxWUGhp5KXQb2VA==", - "license": "MIT" - }, "node_modules/@types/node": { "version": "25.0.2", "resolved": "https://registry.npmjs.org/@types/node/-/node-25.0.2.tgz", "integrity": "sha512-gWEkeiyYE4vqjON/+Obqcoeffmk0NF15WSBwSs7zwVA2bAbTaE0SJ7P0WNGoJn8uE7fiaV5a7dKYIJriEqOrmA==", + "dev": true, "license": "MIT", "dependencies": { "undici-types": "~7.16.0" } }, - "node_modules/@types/qs": { - "version": "6.14.0", - "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.14.0.tgz", - "integrity": "sha512-eOunJqu0K1923aExK6y8p6fsihYEn/BYuQ4g0CxAAgFc4b/ZLN4CrsRZ55srTdqoiLzU2B2evC+apEIxprEzkQ==", - "license": "MIT" - }, - "node_modules/@types/range-parser": { - "version": "1.2.7", - "resolved": "https://registry.npmjs.org/@types/range-parser/-/range-parser-1.2.7.tgz", - "integrity": "sha512-hKormJbkJqzQGhziax5PItDUTMAM9uE2XXQmM37dyd4hVM+5aVl7oVxMVUiVQn2oCQFN/LKCZdvSM0pFRqbSmQ==", - "license": "MIT" - }, - "node_modules/@types/send": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/@types/send/-/send-1.2.1.tgz", - "integrity": "sha512-arsCikDvlU99zl1g69TcAB3mzZPpxgw0UQnaHeC1Nwb015xp8bknZv5rIfri9xTOcMuaVgvabfIRA7PSZVuZIQ==", - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, - "node_modules/@types/serve-static": { - "version": "1.15.10", - "resolved": "https://registry.npmjs.org/@types/serve-static/-/serve-static-1.15.10.tgz", - "integrity": "sha512-tRs1dB+g8Itk72rlSI2ZrW6vZg0YrLI81iQSTkMmOqnqCaNr/8Ek4VwWcN5vZgCYWbg/JJSGBlUaYGAOP73qBw==", - "license": "MIT", - "dependencies": { - "@types/http-errors": "*", - "@types/node": "*", - "@types/send": "<1" - } - }, - "node_modules/@types/serve-static/node_modules/@types/send": { - "version": "0.17.6", - "resolved": "https://registry.npmjs.org/@types/send/-/send-0.17.6.tgz", - "integrity": "sha512-Uqt8rPBE8SY0RK8JB1EzVOIZ32uqy8HwdxCnoCOsYrvnswqmFZ/k+9Ikidlk/ImhsdvBsloHbAlewb2IEBV/Og==", - "license": "MIT", - "dependencies": { - "@types/mime": "^1", - "@types/node": "*" - } - }, "node_modules/@types/stack-utils": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz", @@ -5641,6 +5540,7 @@ "version": "7.1.4", "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-7.1.4.tgz", "integrity": "sha512-MnA+YT8fwfJPgBx3m60MNqakm30XOkyIoH1y6huTQvC0PwZG7ki8NacLBcrPbNoo8vEZy7Jpuk7+jMO+CUovTQ==", + "dev": true, "license": "MIT", "engines": { "node": ">= 14" @@ -8701,6 +8601,7 @@ "version": "7.0.6", "resolved": "https://registry.npmjs.org/https-proxy-agent/-/https-proxy-agent-7.0.6.tgz", "integrity": "sha512-vK9P5/iUfdl95AI+JVyUuIcVtd4ofvtrOr3HNtM2yxC9bnMbEdp3x01OhQNnjb8IJYi38VlTE3mBXwcfvywuSw==", + "dev": true, "license": "MIT", "dependencies": { "agent-base": "^7.1.2", @@ -11861,9 +11762,9 @@ } }, "node_modules/jose": { - "version": "4.15.9", - "resolved": "https://registry.npmjs.org/jose/-/jose-4.15.9.tgz", - "integrity": "sha512-1vUQX+IdDMVPj4k8kOxgUqlcK518yluMuGZwqlr44FS1ppZB/5GWh4rZG89erpOBOJjU/OBsnCVFfapsRz6nEA==", + "version": "6.2.3", + "resolved": "https://registry.npmjs.org/jose/-/jose-6.2.3.tgz", + "integrity": "sha512-YYVDInQKFJfR/xa3ojUTl8c2KoTwiL1R5Wg9YCydwH0x0B9grbzlg5HC7mMjCtUJjbQ/YnGEZIhI5tCgfTb4Hw==", "license": "MIT", "funding": { "url": "https://github.com/sponsors/panva" @@ -12012,23 +11913,6 @@ "safe-buffer": "^5.0.1" } }, - "node_modules/jwks-rsa": { - "version": "3.2.0", - "resolved": "https://registry.npmjs.org/jwks-rsa/-/jwks-rsa-3.2.0.tgz", - "integrity": "sha512-PwchfHcQK/5PSydeKCs1ylNym0w/SSv8a62DgHJ//7x2ZclCoinlsjAfDxAAbpoTPybOum/Jgy+vkvMmKz89Ww==", - "license": "MIT", - "dependencies": { - "@types/express": "^4.17.20", - "@types/jsonwebtoken": "^9.0.4", - "debug": "^4.3.4", - "jose": "^4.15.4", - "limiter": "^1.1.5", - "lru-memoizer": "^2.2.0" - }, - "engines": { - "node": ">=14" - } - }, "node_modules/jws": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/jws/-/jws-4.0.1.tgz", @@ -12089,11 +11973,6 @@ "node": ">= 0.8.0" } }, - "node_modules/limiter": { - "version": "1.1.5", - "resolved": "https://registry.npmjs.org/limiter/-/limiter-1.1.5.tgz", - "integrity": "sha512-FWWMIEOxz3GwUI4Ts/IvgVy6LPvoMPgjMdQ185nN6psJyBJ4yOpzqm695/h5umdLJg2vW3GR5iG11MAkR2AzJA==" - }, "node_modules/lines-and-columns": { "version": "1.2.4", "resolved": "https://registry.npmjs.org/lines-and-columns/-/lines-and-columns-1.2.4.tgz", @@ -12240,34 +12119,6 @@ "node": "20 || >=22" } }, - "node_modules/lru-memoizer": { - "version": "2.3.0", - "resolved": "https://registry.npmjs.org/lru-memoizer/-/lru-memoizer-2.3.0.tgz", - "integrity": "sha512-GXn7gyHAMhO13WSKrIiNfztwxodVsP8IoZ3XfrJV4yH2x0/OeTO/FIaAHTY5YekdGgW94njfuKmyyt1E0mR6Ug==", - "license": "MIT", - "dependencies": { - "lodash.clonedeep": "^4.5.0", - "lru-cache": "6.0.0" - } - }, - "node_modules/lru-memoizer/node_modules/lru-cache": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", - "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", - "license": "ISC", - "dependencies": { - "yallist": "^4.0.0" - }, - "engines": { - "node": ">=10" - } - }, - "node_modules/lru-memoizer/node_modules/yallist": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", - "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==", - "license": "ISC" - }, "node_modules/make-dir": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/make-dir/-/make-dir-4.0.0.tgz", @@ -14829,6 +14680,7 @@ "version": "7.16.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.16.0.tgz", "integrity": "sha512-Zz+aZWSj8LE6zoxD+xrjh4VfkIG8Ya6LvYkZqtUQGJPZjYl53ypCaUwWqo7eI0x66KBGeRo+mlBEkMSeSZ38Nw==", + "dev": true, "license": "MIT" }, "node_modules/unixify": { diff --git a/package.json b/package.json index 12dc8076..df4dbb33 100644 --- a/package.json +++ b/package.json @@ -51,9 +51,8 @@ "graphql": "^16.11.0", "graphql-scalars": "^1.25.0", "http-status-codes": "^2.3.0", - "https-proxy-agent": "^7.0.6", "jsonwebtoken": "^9.0.2", - "jwks-rsa": "^3.1.0", + "jose": "^6.2.3", "mongodb": "^7.1.1", "undici": "^7.28.0", "uuid": "^14.0.0", From 4ce1aa00ee387af7d4b708971e44469e14d8fb4d Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Mon, 15 Jun 2026 18:30:35 +0100 Subject: [PATCH 04/10] feat: add Oauth startup check --- app/logger/codes.js | 1 + app/utils/health/index.js | 5 +- app/utils/health/jwks.js | 34 +++++++++ test/unit/utils/health/index.test.js | 17 +++-- test/unit/utils/health/jwks.test.js | 102 +++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 11 deletions(-) create mode 100644 app/utils/health/jwks.js create mode 100644 test/unit/utils/health/jwks.test.js diff --git a/app/logger/codes.js b/app/logger/codes.js index b66fc0e8..3cdb4fdc 100644 --- a/app/logger/codes.js +++ b/app/logger/codes.js @@ -21,3 +21,4 @@ export const HITACHI_API_REQUEST_001 = 'HITACHI_API_REQUEST_001' export const HITACHI_API_NOT_FOUND_001 = 'HITACHI_API_NOT_FOUND_001' export const MONGO_DB_ERROR_001 = 'MONGO_DB_ERROR_001' +export const JWKS_FETCH_ERROR_001 = 'JWKS_FETCH_ERROR_001' diff --git a/app/utils/health/index.js b/app/utils/health/index.js index 487c1d99..bedb665b 100644 --- a/app/utils/health/index.js +++ b/app/utils/health/index.js @@ -1,8 +1,9 @@ +import { logger } from '../../logger/logger.js' +import { healthCheck as jwks } from './jwks.js' import { healthCheck as mongo } from './mongo.js' import { healthCheck as ruralPayments } from './rural-payments.js' -import { logger } from '../../logger/logger.js' -const healthChecks = [mongo, ruralPayments] +const healthChecks = [mongo, jwks, ruralPayments] /** * Runs all registered health checks. diff --git a/app/utils/health/jwks.js b/app/utils/health/jwks.js new file mode 100644 index 00000000..04c025e2 --- /dev/null +++ b/app/utils/health/jwks.js @@ -0,0 +1,34 @@ +import { config } from '../../config.js' +import { JWKS } from '../../data-sources/JWKS.js' +import { JWKS_FETCH_ERROR_001 } from '../../logger/codes.js' +import { logger } from '../../logger/logger.js' + +export const healthCheck = async () => { + try { + const res = await fetch(config.get('oidc.jwksURI')) + if (!res.ok) { + logger.error('#DAL - JWKS endpoint returned', { res }) + throw new Error('Problem fetching JWKS keys, status:', res.status) + } + + const json = await res.json() + if (!json.keys || !Array.isArray(json.keys)) { + logger.error('#DAL - Error fetching JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) + throw new Error('Problem inspecting JWKS keys response') + } + logger.info('Fetched', json.keys.length, 'JWKS keys') + + const [firstKey] = json.keys + if (!firstKey?.kid) { + logger.error('#DAL - Error fetching JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) + throw new Error('Missing JWKS keys') + } + + const jwks = new JWKS() + await jwks.getPublicKey(firstKey.kid) + logger.info('Resolved first JWKS key for kid:', firstKey.kid) + } catch (error) { + logger.error('#DAL - Error checking JWKS keys', { error, code: JWKS_FETCH_ERROR_001 }) + throw error + } +} diff --git a/test/unit/utils/health/index.test.js b/test/unit/utils/health/index.test.js index 3ffce47a..cf103bd6 100644 --- a/test/unit/utils/health/index.test.js +++ b/test/unit/utils/health/index.test.js @@ -2,10 +2,14 @@ import { expect, jest } from '@jest/globals' const mockMongoHealthCheck = jest.fn() const mockRuralPaymentsHealthCheck = jest.fn() +const mockJwksHealthCheck = jest.fn() jest.unstable_mockModule('../../../../app/utils/health/mongo.js', () => ({ healthCheck: mockMongoHealthCheck })) +jest.unstable_mockModule('../../../../app/utils/health/jwks.js', () => ({ + healthCheck: mockJwksHealthCheck +})) jest.unstable_mockModule('../../../../app/utils/health/rural-payments.js', () => ({ healthCheck: mockRuralPaymentsHealthCheck @@ -17,6 +21,7 @@ describe('runHealthChecks', () => { beforeEach(() => { mockMongoHealthCheck.mockResolvedValue(undefined) mockRuralPaymentsHealthCheck.mockResolvedValue(undefined) + mockJwksHealthCheck.mockResolvedValue(undefined) jest.spyOn(process, 'exit').mockReturnValue(1) }) @@ -28,20 +33,14 @@ describe('runHealthChecks', () => { await runHealthChecks() expect(mockMongoHealthCheck).toHaveBeenCalledTimes(1) + expect(mockJwksHealthCheck).toHaveBeenCalledTimes(1) expect(mockRuralPaymentsHealthCheck).toHaveBeenCalledTimes(1) }) - it('should stop and terminate process if the mongo health check fails', async () => { + it('should stop and terminate process if any health check fails', async () => { const error = new Error('Health check failed') mockMongoHealthCheck.mockRejectedValueOnce(error) - - await runHealthChecks() - - expect(process.exit).toHaveBeenCalledWith(1) - }) - - it('should stop and terminate process if the rural payments health check fails', async () => { - const error = new Error('Health check failed') + mockJwksHealthCheck.mockRejectedValueOnce(error) mockRuralPaymentsHealthCheck.mockRejectedValueOnce(error) await runHealthChecks() diff --git a/test/unit/utils/health/jwks.test.js b/test/unit/utils/health/jwks.test.js new file mode 100644 index 00000000..41816b83 --- /dev/null +++ b/test/unit/utils/health/jwks.test.js @@ -0,0 +1,102 @@ +import { expect, jest } from '@jest/globals' +import { config } from '../../../../app/config.js' + +const mockGetPublicKey = jest.fn() +const mockLogger = { + logger: { + error: jest.fn(), + info: jest.fn() + } +} + +jest.unstable_mockModule('../../../../app/logger/logger.js', () => mockLogger) +jest.unstable_mockModule('../../../../app/data-sources/JWKS.js', () => ({ + JWKS: class { + getPublicKey = mockGetPublicKey + } +})) + +const { healthCheck } = await import('../../../../app/utils/health/jwks.js') + +describe('JWKS health check', () => { + const originalFetch = global.fetch + + beforeEach(() => { + mockGetPublicKey.mockResolvedValue('public-key') + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: jest.fn().mockResolvedValue({ + keys: [{ kid: 'mock-key-id-123' }] + }) + }) + }) + + afterEach(() => { + mockGetPublicKey.mockReset() + mockLogger.logger.error.mockReset() + mockLogger.logger.info.mockReset() + global.fetch = originalFetch + jest.restoreAllMocks() + }) + + it('should fetch JWKS keys and resolve a public key', async () => { + await healthCheck() + + expect(global.fetch).toHaveBeenCalledWith(config.get('oidc.jwksURI')) + expect(mockGetPublicKey).toHaveBeenCalledWith('mock-key-id-123') + expect(mockLogger.logger.info).toHaveBeenCalledWith('Fetched', 1, 'JWKS keys') + }) + + it('should log and throw when JWKS request is unsuccessful, e.g. rate limiting', async () => { + global.fetch.mockResolvedValueOnce({ + ok: false, + status: 429, + json: jest.fn().mockResolvedValue('') + }) + + await expect(healthCheck()).rejects.toThrow('Problem fetching JWKS keys, status: 429') + expect(mockLogger.logger.error).toHaveBeenCalledWith('#DAL - Error fetching JWKS keys', { + code: expect.any(String), + res: expect.any(Object) + }) + }) + + it('should log and throw when no JWKS keys array is returned', async () => { + global.fetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: jest.fn().mockResolvedValue({}) + }) + + await expect(healthCheck()).rejects.toThrow('Problem inspecting JWKS keys response') + expect(mockLogger.logger.error).toHaveBeenCalledWith('#DAL - Error parsing JWKS keys', { + code: expect.any(String), + res: expect.any(Object) + }) + }) + + it('should log and throw when no JWKS keys are returned', async () => { + global.fetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: jest.fn().mockResolvedValue({ keys: [] }) + }) + + await expect(healthCheck()).rejects.toThrow('Missing JWKS keys') + expect(mockLogger.logger.error).toHaveBeenCalledWith('#DAL - Error checking JWKS keys', { + error: expect.any(Error), + code: expect.any(String) + }) + }) + + it('should log and throw when JWKS public key retrieval fails', async () => { + mockGetPublicKey.mockRejectedValueOnce(new Error('Unable to resolve key')) + + await expect(healthCheck()).rejects.toThrow('Unable to resolve key') + expect(mockLogger.logger.error).toHaveBeenCalledWith('#DAL - Error checking JWKS keys', { + error: expect.any(Error), + code: expect.any(String) + }) + }) +}) From b7561b9ce89745262d4bb4941b16d507a11b6225 Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Thu, 16 Jul 2026 08:24:36 +0100 Subject: [PATCH 05/10] chore: add sonarcloud setup --- .vscode/settings.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 803be37c..384f3275 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -50,5 +50,9 @@ } }, "jestrunner.jestCommand": "NODE_OPTIONS=--experimental-vm-modules DOTENV_CONFIG_PATH=./.env.test ./node_modules/jest/bin/jest.js", - "jestrunner.runOptions": ["--runInBand", "--coverage", "--setupFiles", "dotenv/config"] + "jestrunner.runOptions": ["--runInBand", "--coverage", "--setupFiles", "dotenv/config"], + "sonarlint.connectedMode.project": { + "connectionId": "defra", + "projectKey": "DEFRA_fcp-dal-api" + } } From 2603ed7a182ab7f05670bf5442450a855c16b016 Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Thu, 16 Jul 2026 18:23:49 +0100 Subject: [PATCH 06/10] feat: imporve startup healthcheck logs --- app/data-sources/JWKS.js | 2 +- app/utils/health/jwks.js | 12 ++++++------ app/utils/health/mongo.js | 4 +++- test/unit/utils/health/jwks.test.js | 4 +++- test/unit/utils/health/mongo.test.js | 2 +- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/data-sources/JWKS.js b/app/data-sources/JWKS.js index 9b4e43bd..19b54cbf 100644 --- a/app/data-sources/JWKS.js +++ b/app/data-sources/JWKS.js @@ -19,6 +19,6 @@ export class JWKS { async getPublicKey(kid) { const getKey = this.getRemoteJwksSet() - return await getKey({ kid, alg: 'RS256' }) + return getKey({ kid, alg: 'RS256' }) } } diff --git a/app/utils/health/jwks.js b/app/utils/health/jwks.js index 04c025e2..fe6971f9 100644 --- a/app/utils/health/jwks.js +++ b/app/utils/health/jwks.js @@ -5,28 +5,28 @@ import { logger } from '../../logger/logger.js' export const healthCheck = async () => { try { + logger.info(`Fetching JWKS keys from ${config.get('oidc.jwksURI')}`) const res = await fetch(config.get('oidc.jwksURI')) if (!res.ok) { - logger.error('#DAL - JWKS endpoint returned', { res }) - throw new Error('Problem fetching JWKS keys, status:', res.status) + logger.error('#DAL - Error fetching JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) + throw new Error(`Problem fetching JWKS keys, status: ${res.status}`) } const json = await res.json() if (!json.keys || !Array.isArray(json.keys)) { - logger.error('#DAL - Error fetching JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) + logger.error('#DAL - Error parsing JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) throw new Error('Problem inspecting JWKS keys response') } - logger.info('Fetched', json.keys.length, 'JWKS keys') const [firstKey] = json.keys if (!firstKey?.kid) { - logger.error('#DAL - Error fetching JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) + logger.error('#DAL - Error no matching JWKS key', { res, code: JWKS_FETCH_ERROR_001 }) throw new Error('Missing JWKS keys') } const jwks = new JWKS() await jwks.getPublicKey(firstKey.kid) - logger.info('Resolved first JWKS key for kid:', firstKey.kid) + logger.info(`SUCCESS: Resolved first JWKS key for kid: ${firstKey.kid}`) } catch (error) { logger.error('#DAL - Error checking JWKS keys', { error, code: JWKS_FETCH_ERROR_001 }) throw error diff --git a/app/utils/health/mongo.js b/app/utils/health/mongo.js index 7bd89271..99e1f9b4 100644 --- a/app/utils/health/mongo.js +++ b/app/utils/health/mongo.js @@ -1,11 +1,13 @@ +import { config } from '../../config.js' import { MONGO_DB_ERROR_001 } from '../../logger/codes.js' import { logger } from '../../logger/logger.js' import { mongoClient } from '../../mongo.js' export const healthCheck = async () => { try { + logger.info(`Connecting to MongoDB on ${config.get('mongo.mongoUrl')}`) await mongoClient.connect() - logger.info('Connected to MongoDB') + logger.info('SUCCESS: Connected to MongoDB') } catch (err) { logger.error('#DAL - Error connecting to MongoDB', { error: err, code: MONGO_DB_ERROR_001 }) throw err diff --git a/test/unit/utils/health/jwks.test.js b/test/unit/utils/health/jwks.test.js index 41816b83..3f022b50 100644 --- a/test/unit/utils/health/jwks.test.js +++ b/test/unit/utils/health/jwks.test.js @@ -45,7 +45,9 @@ describe('JWKS health check', () => { expect(global.fetch).toHaveBeenCalledWith(config.get('oidc.jwksURI')) expect(mockGetPublicKey).toHaveBeenCalledWith('mock-key-id-123') - expect(mockLogger.logger.info).toHaveBeenCalledWith('Fetched', 1, 'JWKS keys') + expect(mockLogger.logger.info).toHaveBeenCalledWith( + 'SUCCESS: Resolved first JWKS key for kid: mock-key-id-123' + ) }) it('should log and throw when JWKS request is unsuccessful, e.g. rate limiting', async () => { diff --git a/test/unit/utils/health/mongo.test.js b/test/unit/utils/health/mongo.test.js index acf1dacb..a19f86eb 100644 --- a/test/unit/utils/health/mongo.test.js +++ b/test/unit/utils/health/mongo.test.js @@ -25,7 +25,7 @@ describe('Mongo health check', () => { await healthCheck() expect(mongoClient.connect).toHaveBeenCalled() - expect(mockLogger.logger.info).toHaveBeenCalledWith('Connected to MongoDB') + expect(mockLogger.logger.info).toHaveBeenCalledWith('SUCCESS: Connected to MongoDB') }) it('should log error and throw when MongoDB fails to connect', async () => { From e7eedf0fdb821b6db1eece984f110bd170085f87 Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Thu, 16 Jul 2026 18:49:45 +0100 Subject: [PATCH 07/10] feat: only check JWKS setup when auth is enabled --- app/utils/health/jwks.js | 52 ++++++++++++++++------------- test/unit/utils/health/jwks.test.js | 13 ++++++-- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/app/utils/health/jwks.js b/app/utils/health/jwks.js index fe6971f9..edf175c4 100644 --- a/app/utils/health/jwks.js +++ b/app/utils/health/jwks.js @@ -4,31 +4,37 @@ import { JWKS_FETCH_ERROR_001 } from '../../logger/codes.js' import { logger } from '../../logger/logger.js' export const healthCheck = async () => { - try { - logger.info(`Fetching JWKS keys from ${config.get('oidc.jwksURI')}`) - const res = await fetch(config.get('oidc.jwksURI')) - if (!res.ok) { - logger.error('#DAL - Error fetching JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) - throw new Error(`Problem fetching JWKS keys, status: ${res.status}`) - } + if (!config.get('auth.disabled')) { + try { + logger.info(`Fetching JWKS keys from ${config.get('oidc.jwksURI')}`) + const res = await fetch(config.get('oidc.jwksURI')) + if (!res.ok) { + logger.error('#DAL - Error fetching JWKS keys', { + res, + code: JWKS_FETCH_ERROR_001, + tenant: { message: await res.text() } + }) + throw new Error(`Problem fetching JWKS keys, status: ${res.status}`) + } - const json = await res.json() - if (!json.keys || !Array.isArray(json.keys)) { - logger.error('#DAL - Error parsing JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) - throw new Error('Problem inspecting JWKS keys response') - } + const json = await res.json() + if (!json.keys || !Array.isArray(json.keys)) { + logger.error('#DAL - Error parsing JWKS keys', { res, code: JWKS_FETCH_ERROR_001 }) + throw new Error('Problem inspecting JWKS keys response') + } - const [firstKey] = json.keys - if (!firstKey?.kid) { - logger.error('#DAL - Error no matching JWKS key', { res, code: JWKS_FETCH_ERROR_001 }) - throw new Error('Missing JWKS keys') - } + const [firstKey] = json.keys + if (!firstKey?.kid) { + logger.error('#DAL - Error no matching JWKS key', { res, code: JWKS_FETCH_ERROR_001 }) + throw new Error('Missing JWKS keys') + } - const jwks = new JWKS() - await jwks.getPublicKey(firstKey.kid) - logger.info(`SUCCESS: Resolved first JWKS key for kid: ${firstKey.kid}`) - } catch (error) { - logger.error('#DAL - Error checking JWKS keys', { error, code: JWKS_FETCH_ERROR_001 }) - throw error + const jwks = new JWKS() + await jwks.getPublicKey(firstKey.kid) + logger.info(`SUCCESS: Resolved first JWKS key for kid: ${firstKey.kid}`) + } catch (error) { + logger.error('#DAL - Error checking JWKS keys', { error, code: JWKS_FETCH_ERROR_001 }) + throw error + } } } diff --git a/test/unit/utils/health/jwks.test.js b/test/unit/utils/health/jwks.test.js index 3f022b50..a792d561 100644 --- a/test/unit/utils/health/jwks.test.js +++ b/test/unit/utils/health/jwks.test.js @@ -30,6 +30,7 @@ describe('JWKS health check', () => { keys: [{ kid: 'mock-key-id-123' }] }) }) + config.set('auth.disabled', false) }) afterEach(() => { @@ -54,13 +55,14 @@ describe('JWKS health check', () => { global.fetch.mockResolvedValueOnce({ ok: false, status: 429, - json: jest.fn().mockResolvedValue('') + text: jest.fn().mockResolvedValue('some upstream error') }) await expect(healthCheck()).rejects.toThrow('Problem fetching JWKS keys, status: 429') expect(mockLogger.logger.error).toHaveBeenCalledWith('#DAL - Error fetching JWKS keys', { code: expect.any(String), - res: expect.any(Object) + res: expect.any(Object), + tenant: { message: 'some upstream error' } }) }) @@ -101,4 +103,11 @@ describe('JWKS health check', () => { code: expect.any(String) }) }) + + it('should skip JWKS health check when auth is disabled', async () => { + config.set('auth.disabled', true) + + expect(global.fetch).not.toHaveBeenCalled() + expect(await healthCheck()).toBeUndefined() + }) }) From 4b0b19ad5a6f0c8e726b9f8d1e7c530d1f775cdd Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Sat, 18 Jul 2026 14:41:28 +0100 Subject: [PATCH 08/10] ci: harden GH Actions, pin versions to commits --- .github/workflows/check-pull-request.yml | 12 +++++++----- .github/workflows/publish-hotfix.yml | 11 ++++++----- .github/workflows/publish.yml | 14 +++++++------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/.github/workflows/check-pull-request.yml b/.github/workflows/check-pull-request.yml index 1250ed02..45b142d8 100644 --- a/.github/workflows/check-pull-request.yml +++ b/.github/workflows/check-pull-request.yml @@ -23,12 +23,12 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v5 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 - name: Setup npm - uses: actions/setup-node@v5 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24.18 cache: npm @@ -36,7 +36,7 @@ jobs: - name: Install and audit run: | npm audit --omit=dev - npm ci + npm ci --ignore-scripts npm run postinstall - name: Verify semantic version bump @@ -71,6 +71,7 @@ jobs: - name: Jest Coverage Comment (with base snapshot) if: steps.base-cov.outputs.available == 'true' uses: ArtiomTr/jest-coverage-report-action@v2 + # NOTE: need to switch to vitest! with: test-script: npm test base-coverage-file: base-coverage/report.json @@ -78,6 +79,7 @@ jobs: - name: Jest Coverage Comment (no base snapshot) if: steps.base-cov.outputs.available != 'true' uses: ArtiomTr/jest-coverage-report-action@v2 + # NOTE: need to switch to vitest! with: test-script: npm test @@ -88,9 +90,9 @@ jobs: # TODO: re-enable mTLS tests when startup health check issue is resolved # && npm run test:acceptance:proxy:mtls - - name: Sonarqube Cloud Scan + - name: Sonar Cloud Scan if: github.actor != 'dependabot[bot]' - uses: SonarSource/sonarqube-scan-action@v6 + uses: SonarSource/sonarqube-scan-action@22918119ff8e1ca75a623e15c8296b6ea4fbe28f # v8.2.1 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/.github/workflows/publish-hotfix.yml b/.github/workflows/publish-hotfix.yml index 195836b7..f6ff6d7f 100644 --- a/.github/workflows/publish-hotfix.yml +++ b/.github/workflows/publish-hotfix.yml @@ -18,29 +18,30 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out code - uses: actions/checkout@v5 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 # Depth 0 is required for branch-based versioning - name: Test code and Create Test Coverage Reports - uses: actions/setup-node@v5 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24 cache: npm - run: | npm audit --omit=dev - npm ci + npm ci --ignore-scripts npm run postinstall npm run lint npm test - name: Publish Hot Fix - uses: DEFRA/cdp-build-action/build-hotfix@main + uses: DEFRA/cdp-build-action/build-hotfix@f13fc42004f99a1df2e21ce1ff488cc72c897e40 # 17 Jul 2026 with: github-token: ${{ secrets.GITHUB_TOKEN }} + # FIXME: this step should be run before the tag publishing in case there are problems found! - name: SonarCloud Scan - uses: SonarSource/sonarqube-scan-action@v6 + uses: SonarSource/sonarqube-scan-action@22918119ff8e1ca75a623e15c8296b6ea4fbe28f # v8.2.1 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 80122235..1d1a523e 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,17 +19,17 @@ jobs: name: Snapshot base coverage for PR comparison runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 - - uses: actions/setup-node@v5 + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24 cache: npm - run: | - npm ci + npm ci --ignore-scripts npm run postinstall - name: Run unit + graphql tests with coverage run: npm test -- --ci --json --testLocationInResults --outputFile=report.json - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: coverage-report-main path: report.json @@ -41,7 +41,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out code - uses: actions/checkout@v5 + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 @@ -56,7 +56,7 @@ jobs: echo "value=${VERSION}" >> $GITHUB_OUTPUT - name: Build and Publish - uses: DEFRA/cdp-build-action/build@main + uses: DEFRA/cdp-build-action/build@f13fc42004f99a1df2e21ce1ff488cc72c897e40 # 17 Jul 2026 with: github-token: ${{ secrets.GITHUB_TOKEN }} version: ${{ steps.version.outputs.value }} @@ -67,7 +67,7 @@ jobs: git push --tags - name: SonarCloud Scan - uses: SonarSource/sonarqube-scan-action@v6 + uses: SonarSource/sonarqube-scan-action@22918119ff8e1ca75a623e15c8296b6ea4fbe28f # v8.2.1 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} From 1adb1a7ff34f15c769695d6df0c84af0976bf536 Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Sat, 18 Jul 2026 17:08:09 +0100 Subject: [PATCH 09/10] ci: debug PR build, show DAL logs on test faliure --- .github/workflows/check-pull-request.yml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-pull-request.yml b/.github/workflows/check-pull-request.yml index 45b142d8..27312158 100644 --- a/.github/workflows/check-pull-request.yml +++ b/.github/workflows/check-pull-request.yml @@ -85,8 +85,24 @@ jobs: - name: Acceptance & environment emulation tests run: | - npm run test:acceptance:docker && \ - npm run test:acceptance:proxy + set +e + docker compose -f compose.yml -f test/acceptance/compose.yml \ + run --rm --build --pull always --quiet-pull test + if [ $? -ne 0 ]; then + echo -e '\n\n====> Dumping DAL logs...\n' + docker logs fcp-dal-api-fcp-dal-api-1 | jq + docker compose down --remove-orphans + exit 1 + fi + docker compose -f compose.yml -f test/acceptance/compose.yml -f test/acceptance/cdp-mock-check.yml \ + run --rm --quiet-pull test # no need to build or pull again + if [ $? -ne 0 ]; then + echo -e '\n\n====> Dumping DAL logs...\n' + docker logs fcp-dal-api-fcp-dal-api-1 | jq + docker compose down --remove-orphans + exit 1 + fi + docker compose down --remove-orphans # TODO: re-enable mTLS tests when startup health check issue is resolved # && npm run test:acceptance:proxy:mtls From 64cb4838d02e90c2ed00c91a040f43af28fa29b5 Mon Sep 17 00:00:00 2001 From: Robin Knipe Date: Fri, 17 Jul 2026 17:38:04 +0100 Subject: [PATCH 10/10] chore: bump version --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 867c0ea2..d84ab57b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "fcp-dal-api", - "version": "2.15.1", + "version": "2.15.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "fcp-dal-api", - "version": "2.15.1", + "version": "2.15.3", "hasInstallScript": true, "license": "OGL-UK-3.0", "dependencies": { diff --git a/package.json b/package.json index df4dbb33..1b9ad6c8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fcp-dal-api", - "version": "2.15.1", + "version": "2.15.3", "description": "Customer Registry GraphQL Service", "homepage": "https://github.com/DEFRA/fcp-data-access-layer-api", "main": "app/index.js",