From ec7a4da9bced17bf14d7cc6d4f1c3c0ec4f8a8f6 Mon Sep 17 00:00:00 2001 From: David Neal Date: Tue, 5 May 2026 11:13:28 -0400 Subject: [PATCH 1/9] feat(api): add FDX error helper, asyncHandler, scope+auth utilities Introduce small per-app helpers that the route refactors depend on: - utils/errors.ts: FDX-compliant error builder + curated set of reusable error responses ({ code, message, debugMessage? }). - utils/asyncHandler.ts: wraps async route handlers so rejected promises propagate to Express's error handler. - utils/auth.ts: AuthenticatedRequest type, getSubject helper, and requireScope middleware for selectively enforcing OAuth scopes per-route. Augments Express's Request via global declaration so req.user is typed everywhere without per-route casts. --- apps/api/src/utils/asyncHandler.ts | 19 ++++++++ apps/api/src/utils/auth.ts | 75 ++++++++++++++++++++++++++++++ apps/api/src/utils/errors.ts | 53 +++++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 apps/api/src/utils/asyncHandler.ts create mode 100644 apps/api/src/utils/auth.ts create mode 100644 apps/api/src/utils/errors.ts diff --git a/apps/api/src/utils/asyncHandler.ts b/apps/api/src/utils/asyncHandler.ts new file mode 100644 index 0000000..2edff05 --- /dev/null +++ b/apps/api/src/utils/asyncHandler.ts @@ -0,0 +1,19 @@ +import type { Request, Response, NextFunction, RequestHandler } from "express"; + +/** + * Wrap an async Express route handler so any rejected promise is forwarded + * to Express's error handling middleware via `next(err)` instead of bubbling + * up as an unhandled rejection. + * + * The generic parameters allow callers to use narrower request/response + * types (for example `AuthenticatedRequest<{ accountId: string }>`) while + * still producing a standard Express `RequestHandler`. + */ +export function asyncHandler( + // eslint-disable-next-line no-unused-vars + fn: ( req: R, res: S, next: NextFunction ) => Promise +): RequestHandler { + return ( req, res, next ) => { + Promise.resolve( fn( req as R, res as S, next ) ).catch( next ); + }; +} diff --git a/apps/api/src/utils/auth.ts b/apps/api/src/utils/auth.ts new file mode 100644 index 0000000..b4f9aa2 --- /dev/null +++ b/apps/api/src/utils/auth.ts @@ -0,0 +1,75 @@ +import type { Request, Response, NextFunction, RequestHandler } from "express"; +import type { JWTPayload } from "jose"; +import { FDXErrors } from "./errors.js"; + +/** + * Augment Express's `Request` so route handlers can read the verified JWT + * payload at `req.user` without per-route casts. Populated by the JWT + * verification middleware in apps/api/src/index.ts. + */ +declare global { + namespace Express { + interface Request { + user?: JWTPayload; + } + } +} + +/** + * Convenience alias for handlers that require an authenticated request. + * Uses Express's standard generics so it remains assignable to plain + * `RequestHandler` parameters and to `asyncHandler`. + */ +export type AuthenticatedRequest< + P = Record, + ResBody = unknown, + ReqBody = unknown, + // Use `any` (matching express-serve-static-core's defaults) so this type + // remains assignable wherever a plain `Request` is expected without + // triggering ParsedQs incompatibilities. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ReqQuery = any +> = Request & { user?: JWTPayload }; + +/** + * Extract the authenticated subject (user id) from the verified JWT payload. + * Returns null when no user is attached or `sub` is missing. + */ +export function getSubject( req: Request ): string | null { + const user = req.user; + return typeof user?.sub === "string" && user.sub.length > 0 ? user.sub : null; +} + +/** + * Parse the space-separated `scope` claim from a verified JWT into a Set. + */ +function parseScopes( scopeClaim: unknown ): Set { + if ( typeof scopeClaim !== "string" ) return new Set(); + return new Set( scopeClaim.split( " " ).filter( Boolean ) ); +} + +/** + * Express middleware factory that requires the authenticated token's `scope` + * claim to contain every scope in `requiredScopes`. Responds with HTTP 403 + * and an FDX error body when scopes are missing. + * + * Must run after the JWT-verification middleware that attaches `req.user`. + */ +export function requireScope( ...requiredScopes: string[] ): RequestHandler { + return ( req: Request, res: Response, next: NextFunction ) => { + const user = req.user; + if ( !user ) { + return res.status( 401 ).json( FDXErrors.insufficientScope( "Missing authentication context" ) ); + } + + const grantedScopes = parseScopes( user.scope ); + const missing = requiredScopes.filter( ( s ) => !grantedScopes.has( s ) ); + if ( missing.length > 0 ) { + return res.status( 403 ).json( + FDXErrors.forbidden( `Missing required scope(s): ${ missing.join( " " ) }` ) + ); + } + + return next(); + }; +} diff --git a/apps/api/src/utils/errors.ts b/apps/api/src/utils/errors.ts new file mode 100644 index 0000000..816a9c5 --- /dev/null +++ b/apps/api/src/utils/errors.ts @@ -0,0 +1,53 @@ +/** + * FDX-compliant error response helpers. + * + * Per the FDX spec error responses must contain a numeric `code` and a + * `message`, and may optionally include a `debugMessage`. The `code` is the + * FDX error code (long-term persistent identifier) and is allowed to differ + * from the HTTP status code. + */ + +export interface FDXError { + code: number; + message: string; + debugMessage?: string; +} + +/** + * Build an FDX-compliant error body. + */ +export function fdxError( code: number, message: string, debugMessage?: string ): FDXError { + const body: FDXError = { code, message }; + if ( debugMessage ) body.debugMessage = debugMessage; + return body; +} + +/** + * Common FDX errors used across routes. + * + * Codes loosely follow the FDX spec ranges: + * - 6xx: customer / authorization-related errors + * - 7xx: account / data-related errors + */ +export const FDXErrors = { + insufficientScope: ( debugMessage?: string ): FDXError => + fdxError( 401, "Insufficient scope for this operation", debugMessage ), + + forbidden: ( debugMessage?: string ): FDXError => + fdxError( 403, "Forbidden", debugMessage ), + + customerNotFound: ( debugMessage?: string ): FDXError => + fdxError( 601, "A customer with the provided customer ID could not be found", debugMessage ), + + accountNotFound: ( debugMessage?: string ): FDXError => + fdxError( 701, "An account with the provided account ID could not be found", debugMessage ), + + statementNotFound: ( debugMessage?: string ): FDXError => + fdxError( 702, "A statement with the provided statement ID could not be found", debugMessage ), + + validationFailed: ( debugMessage?: string ): FDXError => + fdxError( 400, "Validation failed", debugMessage ), + + internalError: ( debugMessage?: string ): FDXError => + fdxError( 500, "Internal server error", debugMessage ) +}; From eaee56b6c0781049735a5f73c778567c775b139b Mon Sep 17 00:00:00 2001 From: David Neal Date: Tue, 5 May 2026 11:13:38 -0400 Subject: [PATCH 2/9] fix(api): replace customer-123 hardcode with authenticated subject lookup getCurrentCustomer now requires the authenticated user's id (the JWT sub claim) instead of always returning the first record. This removes the cross-customer data leak where any authenticated user resolved to "customer-123" regardless of identity. The first customer fixture is updated so its customerId matches the demo user's sub ("user_123" - see apps/auth/src/index.ts USERS map), allowing the demo flow to continue working end-to-end after the lookup becomes identity-driven. --- apps/api/src/data/customers.ts | 13 +++++++++---- apps/api/src/data/customersRepository.ts | 13 +++++++------ 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/apps/api/src/data/customers.ts b/apps/api/src/data/customers.ts index 7b78bec..5484b4d 100644 --- a/apps/api/src/data/customers.ts +++ b/apps/api/src/data/customers.ts @@ -1,9 +1,14 @@ -// Mock customer data +// Mock customer data. +// +// The first record's `customerId` is intentionally set to the demo user's +// OIDC `sub` claim ("user_123" — see apps/auth/src/index.ts USERS map) so the +// resource server can resolve an authenticated user to their customer record +// using the JWT subject directly. export const customers = [ { - customerId: "customer-123", - name: "Current Customer", - email: "customer@example.com", + customerId: "user_123", + name: "Dev User", + email: "user@example.test", status: "active", createdDate: "2021-05-15", preferences: { diff --git a/apps/api/src/data/customersRepository.ts b/apps/api/src/data/customersRepository.ts index 518750e..9bf6d09 100644 --- a/apps/api/src/data/customersRepository.ts +++ b/apps/api/src/data/customersRepository.ts @@ -20,16 +20,17 @@ interface CustomerFilters { } /** - * Get the current customer (simulating a logged-in user) + * Get the current customer for an authenticated user. + * + * The `userId` argument should be the verified JWT subject (`sub` claim) of + * the authenticated request. The repository looks up the customer record + * keyed on this id; never trust unauthenticated input here. */ -export async function getCurrentCustomer(): Promise { - // In a real implementation, this would use authentication context - // For now, we'll just return the first customer as the "current" one - +export async function getCurrentCustomer( userId: string ): Promise { // Simulate database query delay return new Promise( ( resolve ) => { setTimeout( () => { - const currentCustomer = customers.find( ( c: Customer ) => c.customerId === "customer-123" ); + const currentCustomer = customers.find( ( c: Customer ) => c.customerId === userId ); resolve( currentCustomer || null ); }, 75 ); // Simulate 75ms delay } ); From a8f4c3d2c185b011a3c4ea9deef4c06cf25d4cff Mon Sep 17 00:00:00 2001 From: David Neal Date: Tue, 5 May 2026 11:13:48 -0400 Subject: [PATCH 3/9] fix(api): scope account lookups by owning customer (IDOR fix) Add a customerId field to every account record and update the repository so account access is always scoped by the authenticated user. getAccounts now requires a customerId and filters to that customer; getAccountForCustomer pairs a lookup with an ownership check and treats not-owned identically to not-found so we don't leak the existence of other customers' accounts. All demo accounts are assigned to the demo user ("user_123") so the existing happy-path flow keeps working after the route handlers move to the ownership-aware repository helpers. --- apps/api/src/data/accounts.ts | 20 +++++++++++- apps/api/src/data/accountsRepository.ts | 41 ++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/apps/api/src/data/accounts.ts b/apps/api/src/data/accounts.ts index 3633fca..2a38bc0 100644 --- a/apps/api/src/data/accounts.ts +++ b/apps/api/src/data/accounts.ts @@ -1,7 +1,15 @@ -// Mock data for accounts +// Mock data for accounts. +// +// Every account carries a `customerId` referencing the owning customer. This +// is used by the resource server to enforce account-ownership checks on +// authenticated requests so a user can only access accounts they own. +// +// For the demo flow all accounts are assigned to the demo user (sub +// "user_123"; see apps/auth/src/index.ts USERS map and apps/api/src/data/customers.ts). export const accounts = [ { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-123", accountNumberDisplay: "0123", nickname: "My Checking", @@ -16,6 +24,7 @@ export const accounts = [ }, { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-456", accountNumberDisplay: "0456", nickname: "Emergency Fund", @@ -30,6 +39,7 @@ export const accounts = [ }, { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-789", accountNumberDisplay: "0789", nickname: "House Down Payment", @@ -44,6 +54,7 @@ export const accounts = [ }, { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-101", accountNumberDisplay: "0101", nickname: "Home Escrow", @@ -58,6 +69,7 @@ export const accounts = [ }, { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-202", accountNumberDisplay: "0202", nickname: "Investment Buffer", @@ -72,6 +84,7 @@ export const accounts = [ }, { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-303", accountNumberDisplay: "0303", nickname: "Rainy Day Fund", @@ -86,6 +99,7 @@ export const accounts = [ }, { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-404", accountNumberDisplay: "0404", nickname: "Dream Home", @@ -100,6 +114,7 @@ export const accounts = [ }, { accountCategory: "DEPOSIT_ACCOUNT", + customerId: "user_123", accountId: "account-505", accountNumberDisplay: "0505", nickname: "Vacation Club", @@ -114,6 +129,7 @@ export const accounts = [ }, { accountCategory: "LOC_ACCOUNT", + customerId: "user_123", accountId: "account-601", accountNumberDisplay: "4532", nickname: "Rewards Card", @@ -129,6 +145,7 @@ export const accounts = [ }, { accountCategory: "LOAN_ACCOUNT", + customerId: "user_123", accountId: "account-602", accountNumberDisplay: "9876", nickname: "Home Loan", @@ -148,6 +165,7 @@ export const accounts = [ }, { accountCategory: "LOAN_ACCOUNT", + customerId: "user_123", accountId: "account-603", accountNumberDisplay: "1234", nickname: "Car Payment", diff --git a/apps/api/src/data/accountsRepository.ts b/apps/api/src/data/accountsRepository.ts index 819ebfa..0414ed8 100644 --- a/apps/api/src/data/accountsRepository.ts +++ b/apps/api/src/data/accountsRepository.ts @@ -8,6 +8,9 @@ interface Currency { interface Account { accountCategory: string; accountId: string; + // Owning customer - used for authorization checks. Must equal the + // authenticated user's `sub` claim for the user to access this account. + customerId: string; accountNumberDisplay: string; productName: string; status: string; @@ -136,21 +139,33 @@ interface PaginatedAssetTransferNetworksResult { // Simulating async database operations with promises /** - * Get all accounts with pagination support + * Get all accounts owned by a specific customer with pagination support. + * + * Always scope account lookups by the authenticated user's customer id to + * prevent cross-customer data leakage. */ -export async function getAccounts( offset = 0, limit = 10 ): Promise { +export async function getAccounts( customerId: string, offset = 0, limit = 10 ): Promise { // Simulate database query delay return new Promise( ( resolve ) => { setTimeout( () => { - const paginatedAccounts = accounts.slice( offset, offset + limit ); + const owned = accounts.filter( ( acc: Account ) => acc.customerId === customerId ); + const paginatedAccounts = owned.slice( offset, offset + limit ); resolve( { accounts: paginatedAccounts, - total: accounts.length + total: owned.length } ); }, 100 ); // Simulate 100ms delay } ); } +/** + * Look up an account by id without applying ownership filtering. + * + * Callers in route handlers MUST verify `account.customerId` matches the + * authenticated user before returning the record. Prefer + * `getAccountForCustomer` when you have a customer id available - it + * collapses the lookup + ownership check into a single repository call. + */ export async function getAccountById( accountId: string ): Promise { // Simulate database query delay return new Promise( ( resolve ) => { @@ -161,6 +176,24 @@ export async function getAccountById( accountId: string ): Promise { + const account = await getAccountById( accountId ); + if ( !account ) return null; + if ( account.customerId !== customerId ) return null; + return account; +} + /** * Get account contact information by account ID */ From 8a22e06dd4dfe669f8c15e5e43a66188b9223db4 Mon Sep 17 00:00:00 2001 From: David Neal Date: Tue, 5 May 2026 11:14:11 -0400 Subject: [PATCH 4/9] fix(auth): add CSRF, login rate limiting, and remove `any` types P0: protect all interaction POST routes (login/confirm/cancel) with the maintained `csrf-csrf` double-submit cookie middleware and embed a hidden CSRF token in every interaction form. The CSRF session is bound to the interaction UID so each login flow gets a uniquely-scoped token. P1: rate-limit POST /interaction/:uid/login at 10 attempts per 15 minutes per IP via `express-rate-limit`. The limiter renders a friendly EJS error in production and exposes `LOGIN_RATE_LIMIT_*` env vars for tuning. P1: replace the five eslint-disabled `any` annotations in apps/auth with narrow types from a new minimal `oidc-provider` module declaration, and swap the response interception body/chunk over to `unknown` with a typed parser. P1: hide the development credential block in views/interaction.ejs unless NODE_ENV !== 'production' (exposed via `app.locals.isProduction`). Co-Authored-By: Claude Opus 4.7 --- apps/auth/package.json | 4 + apps/auth/public/styles.css | 5 +- apps/auth/src/index.ts | 289 +++++++++++++++++++++++++------ apps/auth/src/oidc-provider.d.ts | 78 ++++++++- apps/auth/views/interaction.ejs | 5 + pnpm-lock.yaml | 36 ++++ 6 files changed, 363 insertions(+), 54 deletions(-) diff --git a/apps/auth/package.json b/apps/auth/package.json index a231386..402d49e 100644 --- a/apps/auth/package.json +++ b/apps/auth/package.json @@ -12,15 +12,19 @@ }, "dependencies": { "@apps/shared": "workspace:*", + "cookie-parser": "^1.4.7", + "csrf-csrf": "^4.0.3", "dotenv": "^17.4.2", "ejs": "^5.0.2", "express": "^5.2.1", + "express-rate-limit": "^8.5.0", "oidc-provider": "^9.8.3", "pino": "^10.3.1", "pino-pretty": "^13.1.3" }, "devDependencies": { "@tailwindcss/cli": "^4.2.4", + "@types/cookie-parser": "^1.4.10", "@types/ejs": "^3.1.5", "@types/express": "^5.0.6", "@types/node": "^25.6.0", diff --git a/apps/auth/public/styles.css b/apps/auth/public/styles.css index 69d0929..923e1c2 100644 --- a/apps/auth/public/styles.css +++ b/apps/auth/public/styles.css @@ -1,4 +1,4 @@ -/*! tailwindcss v4.2.2 | MIT License | https://tailwindcss.com */ +/*! tailwindcss v4.2.4 | MIT License | https://tailwindcss.com */ @layer properties; @layer theme, base, components, utilities; @layer theme { @@ -231,6 +231,9 @@ .flex { display: flex; } + .hidden { + display: none; + } .h-5 { height: calc(var(--spacing) * 5); } diff --git a/apps/auth/src/index.ts b/apps/auth/src/index.ts index 57b0ec1..9f06a69 100644 --- a/apps/auth/src/index.ts +++ b/apps/auth/src/index.ts @@ -1,6 +1,14 @@ import "dotenv/config"; -import express, { Request, Response } from "express"; -import { Provider, errors } from "oidc-provider"; +import express, { Request, Response, NextFunction } from "express"; +import { + Provider, + errors, + type OIDCAuthorizationCode, + type OIDCClient +} from "oidc-provider"; +import cookieParser from "cookie-parser"; +import { doubleCsrf } from "csrf-csrf"; +import rateLimit from "express-rate-limit"; import { readFileSync, existsSync } from "fs"; import { resolve } from "path"; import { @@ -155,6 +163,12 @@ setupBasicExpress( app ); // Security headers app.use( createWebSecurityHeaders() ); +// Cookie parser - required for csrf-csrf double-submit cookie pattern. +// The `COOKIE_SECRET` is also reused below as the CSRF HMAC secret when +// `CSRF_SECRET` is not separately configured. +const COOKIE_SECRET = getRequiredEnv( "COOKIE_SECRET", "dev-cookie-secret-CHANGE-ME" ); +app.use( cookieParser( COOKIE_SECRET ) ); + // Body parsers (needed to log token request parameters) app.use( express.urlencoded( { extended: false } ) ); app.use( express.json() ); @@ -162,9 +176,148 @@ app.use( express.json() ); // Template engine setupEJSTemplates( app, new URL( "../views", import.meta.url ).pathname ); +// Expose NODE_ENV-derived flags to all templates so views can gate +// development-only UI (e.g. demo credentials) without re-checking env vars. +app.locals.isProduction = process.env.NODE_ENV === "production"; + // Serve static files (CSS, etc.) app.use( "/public", express.static( new URL( "../public", import.meta.url ).pathname ) ); +// --------------------------------------------------------------------------- +// CSRF protection (double-submit cookie via csrf-csrf, the maintained +// replacement for the deprecated `csurf` middleware). +// +// Tokens are bound to the interaction UID when present so each user's +// interaction has its own CSRF binding; for non-interaction routes the +// session identifier falls back to a stable cookie value or the request IP. +// --------------------------------------------------------------------------- +const CSRF_SECRET = process.env.CSRF_SECRET || COOKIE_SECRET; +const IS_PRODUCTION = process.env.NODE_ENV === "production"; + +const { + generateCsrfToken, + doubleCsrfProtection +} = doubleCsrf( { + getSecret: () => CSRF_SECRET, + getSessionIdentifier: ( req ) => { + // Prefer the per-interaction UID so tokens are scoped to a single login flow. + const uidParam = ( req.params as { uid?: string } | undefined )?.uid; + if ( typeof uidParam === "string" && uidParam.length > 0 ) { + return uidParam; + } + // Fallback to a stable per-client identifier so non-interaction routes + // still receive a deterministic session id. + return req.ip ?? "anonymous"; + }, + cookieName: IS_PRODUCTION ? "__Host-psifi.x-csrf-token" : "x-csrf-token", + cookieOptions: { + httpOnly: true, + sameSite: "lax", + secure: IS_PRODUCTION, + path: "/" + }, + getCsrfTokenFromRequest: ( req ) => { + const headerToken = req.headers[ "x-csrf-token" ]; + if ( typeof headerToken === "string" && headerToken.length > 0 ) { + return headerToken; + } + const bodyToken = ( req.body as { _csrf?: unknown } | undefined )?._csrf; + return typeof bodyToken === "string" ? bodyToken : undefined; + } +} ); + +// --------------------------------------------------------------------------- +// Login rate limiting +// +// 10 attempts per 15 minutes per IP balances normal user retries (typo'd +// password, autofill mishaps) against brute-force probing. Adjust via the +// `LOGIN_RATE_LIMIT_*` env vars if a deployment needs a different ceiling. +// --------------------------------------------------------------------------- +const LOGIN_RATE_LIMIT_WINDOW_MS = Number( process.env.LOGIN_RATE_LIMIT_WINDOW_MS ?? 15 * 60 * 1000 ); +const LOGIN_RATE_LIMIT_MAX = Number( process.env.LOGIN_RATE_LIMIT_MAX ?? 10 ); + +const loginRateLimiter = rateLimit( { + windowMs: LOGIN_RATE_LIMIT_WINDOW_MS, + max: LOGIN_RATE_LIMIT_MAX, + standardHeaders: true, + legacyHeaders: false, + handler: ( req: Request, res: Response ) => { + logger.warn( { + path: req.path, + ip: req.ip, + uid: req.params?.uid + }, "Login rate limit exceeded" ); + + const uidParam = req.params?.uid; + const uid = typeof uidParam === "string" ? uidParam : ""; + + let csrfToken = ""; + try { + csrfToken = generateCsrfToken( req, res ); + } catch { + // non-fatal; render without a fresh token + } + + try { + return res.status( 429 ).render( "interaction", { + uid, + prompt: "login", + scopes: [], + error: "Too many login attempts. Please wait a few minutes and try again.", + email: undefined, + csrfToken + } ); + } catch { + return res.status( 429 ).json( { + error: "too_many_requests", + error_description: "Too many login attempts. Please try again later." + } ); + } + } +} ); + +/** + * Wraps `doubleCsrfProtection` with a friendly EJS error page when the token + * fails to validate. Without this users would see a raw 403 JSON/HTML body. + */ +function csrfProtection( req: Request, res: Response, next: NextFunction ): void { + doubleCsrfProtection( req, res, ( err ) => { + if ( err ) { + logger.warn( { + path: req.path, + ip: req.ip, + error: err instanceof Error ? err.message : String( err ) + }, "CSRF validation failed" ); + + const uidParam = req.params?.uid; + const uid = typeof uidParam === "string" ? uidParam : ""; + + // Try to render the interaction page with a friendly error. If the + // view fails to render (e.g. on non-interaction routes), fall back + // to a plain 403. + try { + let csrfToken = ""; + try { + csrfToken = generateCsrfToken( req, res ); + } catch { + // ignore - we'll render without a fresh token + } + return res.status( 403 ).render( "interaction", { + uid, + prompt: "login", + scopes: [], + error: "Your session expired or the request could not be verified. Please try again.", + email: undefined, + csrfToken + } ); + } catch { + return res.status( 403 ).json( { error: "invalid_csrf_token" } ); + } + } + return next(); + } ); +} + // Very minimal in-memory user store const USERS = new Map< string, @@ -225,8 +378,10 @@ function validateInteractionUid( uid: string | string[] ): { success: true; data return { success: true, data: result.data }; } -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const configuration: any = { +// `oidc-provider` does not ship a complete public Configuration type, so we +// type our config object structurally. The provider validates the shape at +// runtime; we only need TypeScript to keep the values we set well-typed. +const configuration = { clients: SANITIZED_CLIENTS, // Use custom JWKS if provided, otherwise oidc-provider generates ephemeral keys ...( JWKS ? { jwks: JWKS } : {} ), @@ -245,8 +400,7 @@ const configuration: any = { IdToken: 60 * 60, // 1 hour RefreshToken: 14 * 24 * 60 * 60 // 14 days }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - issueRefreshToken: async ( _ctx: unknown, client: any, code: any ) => { + issueRefreshToken: async ( _ctx: unknown, client: OIDCClient, code: OIDCAuthorizationCode ) => { // Issue refresh token if client supports refresh_token grant and either: // - offline_access scope is requested (standard behavior), or // - client has force_refresh_token flag set in .env.clients.json @@ -367,8 +521,7 @@ const configuration: any = { }; }, interactions: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - url: ( _ctx: unknown, interaction: any ) => `/interaction/${ interaction.uid }` + url: ( _ctx: unknown, interaction: { uid: string } ) => `/interaction/${ interaction.uid }` } }; @@ -472,12 +625,15 @@ async function main() { session: details.session }, "GET /interaction/:uid - Interaction details loaded" ); + const csrfToken = generateCsrfToken( req, res ); + res.render( "interaction", { uid, prompt, scopes: requestedScopes, error: undefined, - email: undefined + email: undefined, + csrfToken } ); } catch ( error ) { logError( logger, error, { context: "GET /interaction/:uid" } ); @@ -501,6 +657,8 @@ async function main() { app.post( "/interaction/:uid/login", express.urlencoded( { extended: false } ), + loginRateLimiter, + csrfProtection, async ( req: Request, res: Response ) => { try { // Validate interaction UID path parameter @@ -523,12 +681,14 @@ async function main() { .filter( Boolean ); // Re-render login form with validation error + const csrfToken = generateCsrfToken( req, res ); return res.render( "interaction", { uid, prompt: "login", scopes: requestedScopes, error: "Invalid email or password format.", - email: String( req.body?.email || "" ).slice( 0, 254 ) // Preserve truncated email + email: String( req.body?.email || "" ).slice( 0, 254 ), // Preserve truncated email + csrfToken } ); } @@ -552,12 +712,14 @@ async function main() { .filter( Boolean ); // Re-render login form with error message + const csrfToken = generateCsrfToken( req, res ); return res.render( "interaction", { uid, prompt: "login", scopes: requestedScopes, error: "Invalid email or password. Please try again.", - email // Preserve the email field + email, // Preserve the email field + csrfToken } ); } @@ -633,6 +795,7 @@ async function main() { app.post( "/interaction/:uid/confirm", express.urlencoded( { extended: false } ), + csrfProtection, async ( req: Request, res: Response ) => { try { // Validate interaction UID path parameter @@ -750,6 +913,7 @@ async function main() { app.post( "/interaction/:uid/cancel", express.urlencoded( { extended: false } ), + csrfProtection, async ( req: Request, res: Response ) => { // Validate interaction UID path parameter const uidResult = validateInteractionUid( req.params.uid ); @@ -814,15 +978,38 @@ async function main() { }, "GET /auth - Authorization request received" ); } - // Intercept response to log token response - const originalSend = res.send.bind( res ); - const originalEnd = res.end.bind( res ); + // Intercept response to log token response. Use Response['send']/['end'] + // directly so the wrappers preserve Express's overloaded signatures. + const originalSend = res.send.bind( res ) as Response[ "send" ]; + const originalEnd = res.end.bind( res ) as Response[ "end" ]; + + interface TokenResponseBody { + access_token?: string; + id_token?: string; + refresh_token?: string; + token_type?: string; + expires_in?: number; + scope?: string; + } + + const parseTokenBody = ( value: unknown ): TokenResponseBody | null => { + if ( typeof value === "string" ) { + try { + return JSON.parse( value ) as TokenResponseBody; + } catch { + return null; + } + } + if ( value && typeof value === "object" ) { + return value as TokenResponseBody; + } + return null; + }; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - res.send = function ( body: any ) { + res.send = function ( body: unknown ) { if ( req.path === "/token" && req.method === "POST" ) { - try { - const parsed = typeof body === "string" ? JSON.parse( body ) : body; + const parsed = parseTokenBody( body ); + if ( parsed ) { logger.debug( { path: req.path, accessTokenIssued: !!parsed.access_token, @@ -832,48 +1019,46 @@ async function main() { expiresIn: parsed.expires_in, scope: parsed.scope }, "POST /token - Token response sent (via send)" ); - } catch { + } else { logger.debug( { path: req.path }, "POST /token - Response sent (could not parse)" ); } } return originalSend( body ); - }; + } as Response[ "send" ]; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - res.end = function ( chunk?: any, ...args: any[] ) { + res.end = function ( chunk?: unknown, ...args: unknown[] ) { if ( req.path === "/token" && req.method === "POST" && chunk ) { - try { - const parsed = typeof chunk === "string" ? JSON.parse( chunk ) : chunk; - if ( parsed.access_token ) { - // Count dots to determine JWT format (should have 2 dots = 3 parts) - const accessTokenParts = parsed.access_token ? parsed.access_token.split( "." ).length : 0; - const idTokenParts = parsed.id_token ? parsed.id_token.split( "." ).length : 0; - const refreshTokenParts = parsed.refresh_token ? parsed.refresh_token.split( "." ).length : 0; - - logger.debug( { - path: req.path, - accessTokenIssued: !!parsed.access_token, - accessTokenLength: parsed.access_token ? parsed.access_token.length : 0, - accessTokenParts, - accessTokenPrefix: parsed.access_token ? parsed.access_token.substring( 0, 20 ) : "", - idTokenIssued: !!parsed.id_token, - idTokenLength: parsed.id_token ? parsed.id_token.length : 0, - idTokenParts, - refreshTokenIssued: !!parsed.refresh_token, - refreshTokenLength: parsed.refresh_token ? parsed.refresh_token.length : 0, - refreshTokenParts, - refreshTokenPrefix: parsed.refresh_token ? parsed.refresh_token.substring( 0, 20 ) : "", - tokenType: parsed.token_type, - expiresIn: parsed.expires_in, - scope: parsed.scope - }, "POST /token - Token response sent (via end)" ); - } - } catch { - // Ignore parsing errors + const parsed = parseTokenBody( chunk ); + if ( parsed?.access_token ) { + // Count dots to determine JWT format (should have 2 dots = 3 parts) + const accessTokenParts = parsed.access_token.split( "." ).length; + const idTokenParts = parsed.id_token ? parsed.id_token.split( "." ).length : 0; + const refreshTokenParts = parsed.refresh_token ? parsed.refresh_token.split( "." ).length : 0; + + logger.debug( { + path: req.path, + accessTokenIssued: !!parsed.access_token, + accessTokenLength: parsed.access_token.length, + accessTokenParts, + accessTokenPrefix: parsed.access_token.substring( 0, 20 ), + idTokenIssued: !!parsed.id_token, + idTokenLength: parsed.id_token ? parsed.id_token.length : 0, + idTokenParts, + refreshTokenIssued: !!parsed.refresh_token, + refreshTokenLength: parsed.refresh_token ? parsed.refresh_token.length : 0, + refreshTokenParts, + refreshTokenPrefix: parsed.refresh_token ? parsed.refresh_token.substring( 0, 20 ) : "", + tokenType: parsed.token_type, + expiresIn: parsed.expires_in, + scope: parsed.scope + }, "POST /token - Token response sent (via end)" ); } } - return originalEnd( chunk, ...args ); - }; + // Express's `end` is heavily overloaded; we delegate to the original + // implementation via Reflect.apply so we don't have to enumerate + // every overload in our types. + return Reflect.apply( originalEnd, res, [ chunk, ...args ] ); + } as Response[ "end" ]; next(); } ); diff --git a/apps/auth/src/oidc-provider.d.ts b/apps/auth/src/oidc-provider.d.ts index ffee34c..1f2cfce 100644 --- a/apps/auth/src/oidc-provider.d.ts +++ b/apps/auth/src/oidc-provider.d.ts @@ -1 +1,77 @@ -declare module "oidc-provider"; +/** + * Minimal type declarations for `oidc-provider` v9.x. + * + * The upstream package does not currently ship its own types, and the + * community-maintained `@types/oidc-provider` only covers v8 APIs that no + * longer match what we use. We declare narrow shapes for just the pieces of + * the provider surface that this app actually touches so we can avoid + * `any` while keeping the declaration footprint small. + * + * Parameter names below are documentation-only (declarations have no body), + * so silence the unused-name lint that would otherwise fire on every method. + */ +/* eslint-disable no-unused-vars */ +declare module "oidc-provider" { + import type { IncomingMessage, ServerResponse } from "http"; + + export interface OIDCInteractionPrompt { + name: string; + details?: Record; + } + + export interface OIDCInteractionDetails { + uid: string; + prompt: OIDCInteractionPrompt; + params: Record; + session?: { accountId?: string }; + grantId?: string; + } + + export interface OIDCInteractionResult { + login?: { accountId: string }; + consent?: { grantId: string }; + } + + export interface OIDCGrantInstance { + addOIDCScope( scope: string ): void; + addOIDCClaims( claims: string[] ): void; + addResourceScope( resource: string, scope: string ): void; + save(): Promise; + } + + export interface OIDCGrantConstructor { + new ( props: { accountId: string; clientId: string } ): OIDCGrantInstance; + find( grantId: string ): Promise; + } + + export interface OIDCClient { + clientId: string; + client_id?: string; + grantTypeAllowed( grantType: string ): boolean; + } + + export interface OIDCAuthorizationCode { + scopes: Set; + } + + export class Provider { + constructor( issuer: string, configuration?: unknown ); + proxy: boolean; + // `callback()` returns a Node.js style request handler that works both + // as Express middleware and when invoked directly with (req, res). + callback(): ( req: IncomingMessage, res: ServerResponse, next?: ( err?: unknown ) => void ) => void; + interactionDetails( req: IncomingMessage, res: ServerResponse ): Promise; + interactionResult( + req: IncomingMessage, + res: ServerResponse, + result: OIDCInteractionResult, + options?: { mergeWithLastSubmission?: boolean } + ): Promise; + Grant: OIDCGrantConstructor; + } + + export const errors: { + OIDCProviderError: ErrorConstructor; + [key: string]: ErrorConstructor; + }; +} diff --git a/apps/auth/views/interaction.ejs b/apps/auth/views/interaction.ejs index 9c73e0c..43ee48e 100644 --- a/apps/auth/views/interaction.ejs +++ b/apps/auth/views/interaction.ejs @@ -43,6 +43,7 @@ <% } %>
+