From a41b704322850c9c13a85a54a71b1e8900d7e5ae Mon Sep 17 00:00:00 2001 From: Ezekiel146 <320391431+Ezekiel146@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:16:28 -0700 Subject: [PATCH] feat(auth): JWT key rotation with key ids and an overlap window Access and refresh tokens are signed with the current key and carry its kid. Verification selects the key by kid and accepts a previous key during an overlap window, so rotation does not log users out. Tokens with an unknown kid are rejected. Adds the rotation runbook. Refs #106 Co-Authored-By: Claude Haiku 5.5 --- backend/src/config/index.ts | 39 +++++++++++++- backend/src/modules/auth/services/tokens.ts | 43 ++++++++++++++-- backend/tests/unit/tokens.test.ts | 55 ++++++++++++++++++++ changelog.d/106-jwt-key-rotation.md | 8 +++ docs/runbooks/secret-rotation.md | 57 +++++++++++++++++++++ 5 files changed, 195 insertions(+), 7 deletions(-) create mode 100644 changelog.d/106-jwt-key-rotation.md create mode 100644 docs/runbooks/secret-rotation.md diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index 67b177d4..1b317b83 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -112,12 +112,47 @@ function parseCorsOrigins(raw: string | undefined, isProduction: boolean): strin .filter((origin) => origin.length > 0); } +export interface JwtSigningKey { + /** Key id written into the token header so verification picks the right secret. */ + kid: string; + secret: string; +} + +/** + * Build the signing keyring for one token type. The first entry is the + * current key (new tokens are signed with it). An optional `_PREVIOUS` secret + * stays in the keyring during a rotation overlap, so tokens minted before the + * rotation keep verifying until they expire. Remove `_PREVIOUS` once the + * overlap window (the token TTL) has passed. See docs/runbooks/secret-rotation.md. + */ +function jwtSigningKeys(prefix: 'ACCESS' | 'REFRESH'): JwtSigningKey[] { + const keys: JwtSigningKey[] = [ + { + kid: process.env[`JWT_${prefix}_KID`] || `${prefix.toLowerCase()}-current`, + secret: requireEnv(`JWT_${prefix}_SECRET`), + }, + ]; + const previous = process.env[`JWT_${prefix}_SECRET_PREVIOUS`]; + if (previous) { + keys.push({ + kid: process.env[`JWT_${prefix}_KID_PREVIOUS`] || `${prefix.toLowerCase()}-previous`, + secret: previous, + }); + } + return keys; +} + +const accessKeys = jwtSigningKeys('ACCESS'); +const refreshKeys = jwtSigningKeys('REFRESH'); + export const config = { port: Number(process.env.PORT ?? 4000), databaseUrl: requireEnv('DATABASE_URL'), jwt: { - accessSecret: requireEnv('JWT_ACCESS_SECRET'), - refreshSecret: requireEnv('JWT_REFRESH_SECRET'), + accessSecret: accessKeys[0].secret, + refreshSecret: refreshKeys[0].secret, + accessKeys, + refreshKeys, // Short-lived access token; refresh token is the long-lived one. // No revocation list yet for either — see ARCHITECTURE_ESSENTIALS.md // "Known unresolved design gaps" (JWT refresh rotation without a diff --git a/backend/src/modules/auth/services/tokens.ts b/backend/src/modules/auth/services/tokens.ts index 5a0c1c0c..cba09ca6 100644 --- a/backend/src/modules/auth/services/tokens.ts +++ b/backend/src/modules/auth/services/tokens.ts @@ -3,7 +3,7 @@ // invalidates all outstanding tokens without a per-request DB read — the // check happens where the token is actually exchanged (refresh). import jwt from 'jsonwebtoken'; -import { config } from '../../../config'; +import { config, type JwtSigningKey } from '../../../config'; import type { Role } from '@prisma/client'; export interface AccessTokenPayload { @@ -35,18 +35,51 @@ export interface RefreshTokenPayload extends AccessTokenPayload { csrf: string; } +/** + * Sign with the current key (first in the keyring) and record its `kid` in + * the header, so verification can pick the matching secret during rotation. + */ +function signWithKeyring(keys: JwtSigningKey[], payload: object, expiresIn: string): string { + const [current] = keys; + return jwt.sign(payload, current.secret, { expiresIn, keyid: current.kid }); +} + +/** + * Verify against the key named by the token's `kid`. A token with an unknown + * `kid` is rejected, not checked against every key. A token without a `kid` + * (minted before rotation support) is tried against each key in order. + */ +function verifyWithKeyring(keys: JwtSigningKey[], token: string): T { + const decoded = jwt.decode(token, { complete: true }); + const kid = typeof decoded?.header.kid === 'string' ? decoded.header.kid : undefined; + const candidates = kid === undefined ? keys : keys.filter((key) => key.kid === kid); + if (candidates.length === 0) { + throw new jwt.JsonWebTokenError('Unknown signing key'); + } + + let lastError: unknown; + for (const key of candidates) { + try { + return jwt.verify(token, key.secret) as T; + } catch (error) { + lastError = error; + } + } + throw lastError; +} + export function signAccessToken(payload: AccessTokenPayload): string { - return jwt.sign(payload, config.jwt.accessSecret, { expiresIn: config.jwt.accessTtl }); + return signWithKeyring(config.jwt.accessKeys, payload, config.jwt.accessTtl); } export function signRefreshToken(payload: RefreshTokenPayload): string { - return jwt.sign(payload, config.jwt.refreshSecret, { expiresIn: config.jwt.refreshTtl }); + return signWithKeyring(config.jwt.refreshKeys, payload, config.jwt.refreshTtl); } export function verifyAccessToken(token: string): AccessTokenPayload { - return jwt.verify(token, config.jwt.accessSecret) as AccessTokenPayload; + return verifyWithKeyring(config.jwt.accessKeys, token); } export function verifyRefreshToken(token: string): RefreshTokenPayload { - return jwt.verify(token, config.jwt.refreshSecret) as RefreshTokenPayload; + return verifyWithKeyring(config.jwt.refreshKeys, token); } diff --git a/backend/tests/unit/tokens.test.ts b/backend/tests/unit/tokens.test.ts index 962d52c7..08dba5b0 100644 --- a/backend/tests/unit/tokens.test.ts +++ b/backend/tests/unit/tokens.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from 'vitest'; +import jwt from 'jsonwebtoken'; +import { config } from '../../src/config'; import { signAccessToken, signRefreshToken, @@ -46,3 +48,56 @@ describe('refresh tokens', () => { expect(() => verifyRefreshToken(accessToken)).toThrow(); }); }); + +// Key rotation (issue #106): new tokens are signed with the current key and +// carry its kid; tokens from the previous key verify only while that key is +// still in the keyring. +describe('access token key rotation', () => { + const originalKeys = config.jwt.accessKeys; + + function withKeyring(keys: typeof originalKeys) { + config.jwt.accessKeys = keys; + } + + it('signs with the current key and records its kid in the header', () => { + const token = signAccessToken({ sub: 'user-r1', role: 'patient' }); + const header = jwt.decode(token, { complete: true })?.header; + expect(header?.kid).toBe(originalKeys[0].kid); + }); + + it('accepts a token from the previous key during the overlap window', () => { + const previous = { kid: 'access-old', secret: 'previous-secret-value-0123456789' }; + const oldToken = jwt.sign({ sub: 'user-r2', role: 'patient' }, previous.secret, { + expiresIn: '5m', + keyid: previous.kid, + }); + withKeyring([originalKeys[0], previous]); + try { + expect(verifyAccessToken(oldToken).sub).toBe('user-r2'); + } finally { + withKeyring(originalKeys); + } + }); + + it('rejects the previous key once it has been removed from the keyring', () => { + const previous = { kid: 'access-old', secret: 'previous-secret-value-0123456789' }; + const oldToken = jwt.sign({ sub: 'user-r3', role: 'patient' }, previous.secret, { + expiresIn: '5m', + keyid: previous.kid, + }); + expect(() => verifyAccessToken(oldToken)).toThrow(); + }); + + it('rejects a token whose kid is not in the keyring', () => { + const token = jwt.sign({ sub: 'user-r4', role: 'patient' }, originalKeys[0].secret, { + expiresIn: '5m', + keyid: 'unknown-kid', + }); + expect(() => verifyAccessToken(token)).toThrow(); + }); + + it('still verifies a legacy token that has no kid header', () => { + const legacy = jwt.sign({ sub: 'user-r5', role: 'patient' }, originalKeys[0].secret, { expiresIn: '5m' }); + expect(verifyAccessToken(legacy).sub).toBe('user-r5'); + }); +}); diff --git a/changelog.d/106-jwt-key-rotation.md b/changelog.d/106-jwt-key-rotation.md new file mode 100644 index 00000000..e0343c76 --- /dev/null +++ b/changelog.d/106-jwt-key-rotation.md @@ -0,0 +1,8 @@ +Phase 9: JWT key rotation with key ids +([issue #106](https://github.com/DocForum/docforum-core/issues/106)). +Access and refresh tokens are signed with the current key and carry its +`kid`. Verification accepts a previous key during an overlap window +(`JWT_*_SECRET_PREVIOUS`), so rotation does not log users out. Tokens with an +unknown `kid` are rejected. The rotation procedure is in +`docs/runbooks/secret-rotation.md`. Loading secrets from a secrets manager and +the staging rotation are still open. diff --git a/docs/runbooks/secret-rotation.md b/docs/runbooks/secret-rotation.md new file mode 100644 index 00000000..935d321c --- /dev/null +++ b/docs/runbooks/secret-rotation.md @@ -0,0 +1,57 @@ +# Secret rotation runbook + +Scope: the JWT signing keys for access and refresh tokens (issue #106). The +other secret classes named in issue #106 (field-encryption keys, Stellar +releaser and sponsor keys) are not covered here yet. Stellar key rotation +must follow the custody decision in issue #25. + +## How JWT key rotation works + +Each token type has a keyring. The first entry signs new tokens and its +`kid` is written into the token header. Verification uses the key the +header names. A token whose `kid` is not in the keyring is rejected. A token +with no `kid` (minted before rotation support) is tried against each key. + +| Env var | Purpose | +|---|---| +| `JWT_ACCESS_SECRET` / `JWT_REFRESH_SECRET` | Current key. Signs new tokens. | +| `JWT_ACCESS_KID` / `JWT_REFRESH_KID` | Optional `kid` for the current key. Defaults to `access-current` / `refresh-current`. | +| `JWT_ACCESS_SECRET_PREVIOUS` / `JWT_REFRESH_SECRET_PREVIOUS` | Previous key, kept during the overlap window. | +| `JWT_ACCESS_KID_PREVIOUS` / `JWT_REFRESH_KID_PREVIOUS` | Optional `kid` for the previous key. Defaults to `access-previous` / `refresh-previous`. | + +Use a `kid` that changes at every rotation, for example `access-2026-10`. +Reusing a `kid` for a different secret would make verification pick the +wrong key. + +## Rotating a JWT key without logging users out + +Access tokens live 15 minutes and refresh tokens 30 days. The overlap window +must be at least the longer of the two, so the refresh token length (30 days) +bounds it. + +1. Generate a new secret with at least 32 random bytes, for example + `openssl rand -base64 48`. Don't reuse a previous value. +2. Set `JWT__SECRET_PREVIOUS` to the current secret and + `JWT__KID_PREVIOUS` to the current `kid`. Set `JWT__SECRET` to + the new secret and `JWT__KID` to a new `kid`. Redeploy. +3. Confirm that new logins work and that a token minted before the change + still refreshes. +4. After the overlap window (30 days for refresh tokens) has passed, remove + the `_PREVIOUS` variables and redeploy. Tokens still signed by the previous + key are now rejected, which is the intended end of the overlap. + +Rotating without an overlap window (removing `_PREVIOUS` immediately) logs out +every user whose token was signed by the old key. Use it only as an +emergency response to a leaked key. + +## Who can read each secret + +To be filled in with the secrets manager chosen under issue #22. Until then, +these values are set as deployment environment variables and must not be +committed to the repository or baked into an image. + +## Staging rotation record + +Not yet performed. Record the date, the `kid` values before and after, and +whether a test user's refresh still worked. Issue #106 asks for one staging +rotation to be carried out and recorded here.