From 388b0672c092f65887bdb86c9806abd5bdc3f823 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 8 Sep 2026 13:13:35 -0400 Subject: [PATCH 1/2] feat(audit): record a bearer refused for the wrong token type verifyBearerAuth refuses a request before any handler runs and wrote nothing durable, so a caller presenting the wrong kind of token at a protected route left only an application log line. Moving passkey enrollment behind an access session made that specific: an ephemeral token offered at /webauthn/register/start is the account takeover probe the gate exists to stop. The refusal now writes bearer_token_failed when the presented token verifies against this issuer's keys but its typ is not the one the route requires, carrying the expected and presented types, the matched route pattern and the token's subject. Narrower than any 401 on purpose. A missing, malformed, expired or unsigned credential costs a caller nothing to produce, and recording those would let one scanner, or one signing key rotation, bury the rows that name a real attempt. Widening waits on audit retention (#173). Closes #286 --- .changeset/quick-moths-record.md | 26 +++++ docs/security-posture.md | 46 +++++++++ openapi.json | 2 + resources/coverage-badge.svg | 14 +-- src/controllers/internalSecurity.ts | 6 +- src/generated/api.ts | 2 + src/middleware/verifyBearerAuth.ts | 38 ++++++++ src/schemas/authEvent.types.ts | 9 +- src/services/bearerRefusal.ts | 72 ++++++++++++++ .../webauthn/enrollmentAuth.spec.ts | 31 ++++++ .../unit/middleware/verifyBearerAuth.spec.ts | 97 ++++++++++++++++++- tests/unit/services/bearerRefusal.spec.ts | 78 +++++++++++++++ 12 files changed, 406 insertions(+), 15 deletions(-) create mode 100644 .changeset/quick-moths-record.md create mode 100644 src/services/bearerRefusal.ts create mode 100644 tests/unit/services/bearerRefusal.spec.ts diff --git a/.changeset/quick-moths-record.md b/.changeset/quick-moths-record.md new file mode 100644 index 0000000..08a7fc2 --- /dev/null +++ b/.changeset/quick-moths-record.md @@ -0,0 +1,26 @@ +--- +'seamless-auth-api': patch +--- + +A bearer refused at the auth gate now leaves an audit record when the token was one this +server issued. + +`verifyBearerAuth` refuses a request before any handler runs, and that refusal reached +the application log and nothing else, so a caller presenting the wrong kind of token at a +protected route left no durable trace. Moving passkey enrollment behind an access session +made that specific: an ephemeral token offered at `/webauthn/register/start` is the +account takeover probe the gate exists to stop, and refusing it was invisible. + +The new `bearer_token_failed` event is written when the presented token verifies against +this issuer's keys but its `typ` is not the one the route requires. It carries the +expected and presented types, the matched route pattern and the token's subject, with +`userId` left null because a refused token has established no principal. + +Deliberately narrower than any 401. A missing, malformed, expired or unsigned credential +costs a caller nothing to produce, and recording those would let one scanner, or one +signing key rotation, bury the rows that name a real attempt. Widening it waits on audit +retention (#173). + +Nothing changes on the wire. The refusal answers the same 401 in the same place, and the +event is only visible to operators through the admin and internal event views, where +`bearer_token_failed` is a new value of the auth event type. diff --git a/docs/security-posture.md b/docs/security-posture.md index 37da833..22168ce 100644 --- a/docs/security-posture.md +++ b/docs/security-posture.md @@ -451,6 +451,52 @@ option today. `auth_failures` rows are not pruned, which matches `auth_events`. Retention is [issue #173](https://github.com/fells-code/seamless-auth-api/issues/173). +## Refusals at the auth gate + +**Posture: only a credential this server issued is worth a row.** + +`verifyBearerAuth` refuses a request before any handler runs, and that refusal used to +leave an application log line and nothing durable. Failed OTP codes, failed assertions +and locked accounts all reach `auth_events`; a caller presenting the wrong kind of token +at a protected route did not. + +The gap became a specific one when passkey enrollment moved to `auth: 'access'`. An +ephemeral token proves possession of an address and nothing more, so offering one at +`/webauthn/register/start` is the account takeover probe that gate exists to stop. +Refusing it is the right answer. Refusing it invisibly is not. + +A refused bearer now writes `bearer_token_failed` when, and only when, the token +verifies against this issuer's keys and its `typ` is not the one the gate requires. The +row carries the expected and presented types, the matched route pattern, and the token's +subject. + +### Why not every 401 + +The refusals left out are the ones anybody can manufacture. A missing header, a malformed +string, an unknown `kid`, a bad signature and an expired token all cost a caller nothing +to produce, and one scanner, or one signing key rotation retiring every outstanding token +at once, would fill the window with rows that name nobody. A token of the wrong type has +to have been minted here first, which bounds the volume to real flows and keeps the rows +worth reading. + +Widening this waits on retention and bulk export +([issue #173](https://github.com/fells-code/seamless-auth-api/issues/173)). Until a window +can be pruned, a high-volume event type costs the trail more than it adds. + +### The subject is metadata, not `user_id` + +A refused token has established no principal, so the event records `userId: null`. The +subject is kept in metadata, where it says whose flow token is being offered without +asserting that the caller is that user. It also could not be a foreign key: an ephemeral +subject may be the decoy `/login` mints for an address with no usable account, which +resolves to no row at all. + +### It changes no response + +The event is written server side and never reflected to the caller. The refusal answers +the same `401 { "error": "unauthorized" }` whether or not a row is written, so the decoy +rules above are untouched. + ## Static analysis triage **Posture: gated on a zero baseline, deliberately.** CodeQL runs on every pull request, diff --git a/openapi.json b/openapi.json index fb9fcd2..b6aac3d 100644 --- a/openapi.json +++ b/openapi.json @@ -1687,6 +1687,7 @@ "auth_action_incremented", "admin_device_replacement_recovery", "admin_session_revoked", + "bearer_token_failed", "credentials_deleted", "informational", "internal_user_updated_by_owner", @@ -1757,6 +1758,7 @@ "auth_action_incremented", "admin_device_replacement_recovery", "admin_session_revoked", + "bearer_token_failed", "credentials_deleted", "informational", "internal_user_updated_by_owner", diff --git a/resources/coverage-badge.svg b/resources/coverage-badge.svg index bfd39a2..8bd6c88 100644 --- a/resources/coverage-badge.svg +++ b/resources/coverage-badge.svg @@ -1,5 +1,5 @@ - - coverage: 98.9% + + coverage: 99% @@ -7,17 +7,17 @@ - + - - + + coverage coverage - 98.9% - 98.9% + 99% + 99% diff --git a/src/controllers/internalSecurity.ts b/src/controllers/internalSecurity.ts index 96bf6eb..da8c1e4 100644 --- a/src/controllers/internalSecurity.ts +++ b/src/controllers/internalSecurity.ts @@ -29,9 +29,9 @@ export const getSecurityAnomalies = async (_req: Request, res: Response) => { try { // Derived from AUTH_EVENT_TYPES. The hand-maintained list searched for five names - // nothing emitted (bearer_token_failed, jwks_failed, otp_failed, - // recovery_otp_failed, user_data_failed) while missing verify_otp_failed, - // totp_failed, magic_link_failed, and logout_failed, which are emitted. + // nothing emitted (jwks_failed, otp_failed, recovery_otp_failed, user_data_failed, + // and bearer_token_failed, which the auth gate now does emit) while missing + // verify_otp_failed, totp_failed, magic_link_failed, and logout_failed. const FAILURE_TYPES = FAILURE_EVENT_TYPES; const events = await AuthEvent.findAll({ diff --git a/src/generated/api.ts b/src/generated/api.ts index dd18961..131ae0a 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -1430,6 +1430,7 @@ export interface paths { | 'auth_action_incremented' | 'admin_device_replacement_recovery' | 'admin_session_revoked' + | 'bearer_token_failed' | 'credentials_deleted' | 'informational' | 'internal_user_updated_by_owner' @@ -1494,6 +1495,7 @@ export interface paths { | 'auth_action_incremented' | 'admin_device_replacement_recovery' | 'admin_session_revoked' + | 'bearer_token_failed' | 'credentials_deleted' | 'informational' | 'internal_user_updated_by_owner' diff --git a/src/middleware/verifyBearerAuth.ts b/src/middleware/verifyBearerAuth.ts index 4d699fe..14d4928 100644 --- a/src/middleware/verifyBearerAuth.ts +++ b/src/middleware/verifyBearerAuth.ts @@ -6,12 +6,49 @@ import { NextFunction, Request, Response } from 'express'; +import { AuthEventService } from '../services/authEventService.js'; +import { findMisusedBearer } from '../services/bearerRefusal.js'; import { AuthTokenType, validateBearerToken } from '../services/sessionService.js'; import { AuthenticatedRequest } from '../types/types.js'; import getLogger from '../utils/logger.js'; const logger = getLogger('verifyBearerAuth'); +/** The matched route pattern, so path parameters do not turn one route into many. */ +function routeLabel(req: Request): string { + const pattern = typeof req.route?.path === 'string' ? req.route.path : req.path; + + return `${req.baseUrl ?? ''}${pattern ?? ''}`; +} + +/** + * Records a refusal that a token this server issued caused. + * + * The subject is metadata rather than `userId`: a refused token has established no + * principal, and an ephemeral subject may be the decoy `/login` mints for an address + * with no account, which resolves to no row at all. + * + * Written server side and never reflected to the caller, so the refusal it describes + * answers exactly as it did before. + */ +async function recordBearerMisuse(req: Request, token: string, expectedType: AuthTokenType) { + const misuse = await findMisusedBearer(token, expectedType); + + if (!misuse) return; + + await AuthEventService.log({ + type: 'bearer_token_failed', + req, + metadata: { + reason: 'wrong_token_type', + expected: expectedType, + presented: misuse.presentedType, + route: routeLabel(req), + subject: misuse.subject, + }, + }); +} + export async function verifyBearerAuth( req: Request, res: Response, @@ -29,6 +66,7 @@ export async function verifyBearerAuth( const result = await validateBearerToken(token, authType); if (!result) { logger.error(`Invalid ${authType} bearer token`); + await recordBearerMisuse(req, token, authType); return res.status(401).json({ error: 'unauthorized' }); } (req as AuthenticatedRequest).user = result.user; diff --git a/src/schemas/authEvent.types.ts b/src/schemas/authEvent.types.ts index 189600b..f5509ba 100644 --- a/src/schemas/authEvent.types.ts +++ b/src/schemas/authEvent.types.ts @@ -11,6 +11,7 @@ export const AUTH_EVENT_TYPES = [ 'auth_action_incremented', 'admin_device_replacement_recovery', 'admin_session_revoked', + 'bearer_token_failed', 'credentials_deleted', 'informational', 'internal_user_updated_by_owner', @@ -78,10 +79,10 @@ export type AuthEventType = z.infer; * Types grouped by outcome, derived rather than hand-listed. * * Consumers used to keep their own copies of these groupings, which drifted: the - * anomaly detector searched for `otp_failed`, `bearer_token_failed`, and three other - * names nothing emitted, so those failures were invisible, while `verify_otp_failed` - * and `magic_link_failed` were emitted and never searched for. Deriving the groups - * means adding an event type puts it in the right bucket automatically. + * anomaly detector searched for five names nothing emitted, such as `otp_failed` and + * `jwks_failed`, so those failures were invisible, while `verify_otp_failed` and + * `magic_link_failed` were emitted and never searched for. Deriving the groups means + * adding an event type puts it in the right bucket automatically. */ export const FAILURE_EVENT_TYPES = AUTH_EVENT_TYPES.filter((type) => type.endsWith('_failed'), diff --git a/src/services/bearerRefusal.ts b/src/services/bearerRefusal.ts new file mode 100644 index 0000000..ab98282 --- /dev/null +++ b/src/services/bearerRefusal.ts @@ -0,0 +1,72 @@ +/* + * Copyright © 2026 Fells Code, LLC + * Licensed under the GNU Affero General Public License v3.0 + * See LICENSE file in the project root for full license information + */ + +import { AuthTokenType, verifyJwtWithKid } from './sessionService.js'; + +export interface MisusedBearer { + presentedType: string; + subject: string | null; +} + +/** + * The `typ` a bearer claims, read without verifying anything. + * + * This only decides whether a refusal is worth a second verification. A token whose + * claimed type is the one the gate asked for cannot be a type mismatch however it is + * signed, and that covers the ordinary refusals (an expired access token, a rotated + * session), which would otherwise pay for a verification that can only conclude there + * is nothing to record. The event itself is built from the verified payload. + */ +function peekTokenType(token: string): string | null { + const segment = token.split('.')[1]; + + if (!segment) return null; + + try { + const claims: unknown = JSON.parse(Buffer.from(segment, 'base64url').toString('utf8')); + + if (!claims || typeof claims !== 'object') return null; + + const typ = (claims as Record).typ; + + return typeof typ === 'string' ? typ : null; + } catch { + return null; + } +} + +/** + * Identifies a bearer this issuer minted that was presented at a gate it does not open. + * + * Deliberately narrower than "the request was refused". A missing, malformed, expired + * or unsigned credential is something any caller can produce for free, so recording + * those would let one scanner, or one signing key rotation, fill `auth_events` with + * rows that name no attacker and bury the ones that do. A token of the wrong type had + * to be issued by this server first, which bounds the volume to real flows and makes + * the row worth reading: it is an ephemeral token, which proves possession of an + * address and nothing else, being offered where an access session is required. + */ +export async function findMisusedBearer( + token: string, + expectedType: AuthTokenType, +): Promise { + const claimedType = peekTokenType(token); + + if (claimedType === null || claimedType === expectedType) { + return null; + } + + const payload = await verifyJwtWithKid(token); + + if (!payload || typeof payload.typ !== 'string' || payload.typ === expectedType) { + return null; + } + + return { + presentedType: payload.typ, + subject: typeof payload.sub === 'string' ? payload.sub : null, + }; +} diff --git a/tests/integration/webauthn/enrollmentAuth.spec.ts b/tests/integration/webauthn/enrollmentAuth.spec.ts index c71410f..658ec01 100644 --- a/tests/integration/webauthn/enrollmentAuth.spec.ts +++ b/tests/integration/webauthn/enrollmentAuth.spec.ts @@ -81,6 +81,37 @@ describe('passkey enrollment requires an access session', () => { expect(generateRegistrationOptions).not.toHaveBeenCalled(); }); + it.each([ + ['get', '/webauthn/register/start'], + ['post', '/webauthn/register/finish'], + ])('records the refusal on %s %s so enrollment probing is visible', async (method, path) => { + const { verifyJwtWithKid } = await import('../../../src/services/sessionService.js'); + const { AuthEventService } = await import('../../../src/services/authEventService.js'); + + (verifyJwtWithKid as any).mockResolvedValue({ typ: 'ephemeral', sub: 'user-1' }); + + const claims = Buffer.from(JSON.stringify({ typ: 'ephemeral', sub: 'user-1' })).toString( + 'base64url', + ); + + await (request(app) as any) + [method](path) + .set('Authorization', `Bearer header.${claims}.signature`); + + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'bearer_token_failed', + metadata: expect.objectContaining({ + reason: 'wrong_token_type', + expected: 'access', + presented: 'ephemeral', + route: path, + subject: 'user-1', + }), + }), + ); + }); + it.each([ ['get', '/webauthn/register/start'], ['post', '/webauthn/register/finish'], diff --git a/tests/unit/middleware/verifyBearerAuth.spec.ts b/tests/unit/middleware/verifyBearerAuth.spec.ts index ebdb08c..e71a233 100644 --- a/tests/unit/middleware/verifyBearerAuth.spec.ts +++ b/tests/unit/middleware/verifyBearerAuth.spec.ts @@ -1,12 +1,26 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { verifyBearerAuth } from '../../../src/middleware/verifyBearerAuth'; -import { validateBearerToken } from '../../../src/services/sessionService'; +import { AuthEventService } from '../../../src/services/authEventService'; +import { validateBearerToken, verifyJwtWithKid } from '../../../src/services/sessionService'; vi.mock('../../../src/services/sessionService', () => ({ validateBearerToken: vi.fn(), + verifyJwtWithKid: vi.fn(), })); +vi.mock('../../../src/services/authEventService', () => ({ + AuthEventService: { + log: vi.fn(), + }, +})); + +function bearerOfType(typ: string, sub = 'user-1') { + const claims = Buffer.from(JSON.stringify({ typ, sub })).toString('base64url'); + + return `Bearer header.${claims}.signature`; +} + describe('verifyBearerAuth', () => { let req: any; let res: any; @@ -127,4 +141,85 @@ describe('verifyBearerAuth', () => { }); expect(next).not.toHaveBeenCalled(); }); + + describe('auditing a refused bearer', () => { + it('records the misuse when a token this issuer minted is presented at another gate', async () => { + req.headers.authorization = bearerOfType('ephemeral'); + req.baseUrl = '/webauthn'; + req.route = { path: '/register/start' }; + + (validateBearerToken as any).mockResolvedValue(null); + (verifyJwtWithKid as any).mockResolvedValue({ typ: 'ephemeral', sub: 'user-1' }); + + await verifyBearerAuth(req, res, next); + + expect(AuthEventService.log).toHaveBeenCalledWith({ + type: 'bearer_token_failed', + req, + metadata: { + reason: 'wrong_token_type', + expected: 'access', + presented: 'ephemeral', + route: '/webauthn/register/start', + subject: 'user-1', + }, + }); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ error: 'unauthorized' }); + }); + + it('falls back to the request path when no route pattern matched', async () => { + req.headers.authorization = bearerOfType('access'); + req.path = '/users/me'; + + (validateBearerToken as any).mockResolvedValue(null); + (verifyJwtWithKid as any).mockResolvedValue({ typ: 'access', sub: 'user-1' }); + + await verifyBearerAuth(req, res, next, 'ephemeral'); + + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + metadata: expect.objectContaining({ route: '/users/me', presented: 'access' }), + }), + ); + }); + + it('records nothing for an ordinary refusal of the expected token type', async () => { + req.headers.authorization = bearerOfType('access'); + + (validateBearerToken as any).mockResolvedValue(null); + + await verifyBearerAuth(req, res, next); + + expect(AuthEventService.log).not.toHaveBeenCalled(); + expect(verifyJwtWithKid).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + }); + + it('records nothing when the refused token does not verify', async () => { + req.headers.authorization = bearerOfType('ephemeral'); + + (validateBearerToken as any).mockResolvedValue(null); + (verifyJwtWithKid as any).mockResolvedValue(null); + + await verifyBearerAuth(req, res, next); + + expect(AuthEventService.log).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + }); + + it('still answers 401 when the audit write throws', async () => { + req.headers.authorization = bearerOfType('ephemeral'); + + (validateBearerToken as any).mockResolvedValue(null); + (verifyJwtWithKid as any).mockResolvedValue({ typ: 'ephemeral', sub: 'user-1' }); + (AuthEventService.log as any).mockRejectedValue(new Error('audit down')); + + await verifyBearerAuth(req, res, next); + + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ error: 'unauthorized' }); + expect(next).not.toHaveBeenCalled(); + }); + }); }); diff --git a/tests/unit/services/bearerRefusal.spec.ts b/tests/unit/services/bearerRefusal.spec.ts new file mode 100644 index 0000000..3bbbadf --- /dev/null +++ b/tests/unit/services/bearerRefusal.spec.ts @@ -0,0 +1,78 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { findMisusedBearer } from '../../../src/services/bearerRefusal'; +import { verifyJwtWithKid } from '../../../src/services/sessionService'; + +function tokenWithClaims(claims: unknown) { + const segment = Buffer.from(JSON.stringify(claims)).toString('base64url'); + + return `header.${segment}.signature`; +} + +describe('findMisusedBearer', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('reports a verified token presented at a gate of another type', async () => { + (verifyJwtWithKid as any).mockResolvedValue({ typ: 'ephemeral', sub: 'user-1' }); + + const result = await findMisusedBearer(tokenWithClaims({ typ: 'ephemeral' }), 'access'); + + expect(result).toEqual({ presentedType: 'ephemeral', subject: 'user-1' }); + }); + + it('reports no subject when the verified token carries none', async () => { + (verifyJwtWithKid as any).mockResolvedValue({ typ: 'access' }); + + const result = await findMisusedBearer(tokenWithClaims({ typ: 'access' }), 'ephemeral'); + + expect(result).toEqual({ presentedType: 'access', subject: null }); + }); + + it('does not verify a token that claims the type the gate asked for', async () => { + const result = await findMisusedBearer(tokenWithClaims({ typ: 'access' }), 'access'); + + expect(result).toBeNull(); + expect(verifyJwtWithKid).not.toHaveBeenCalled(); + }); + + it('ignores a token whose signature does not verify', async () => { + (verifyJwtWithKid as any).mockResolvedValue(null); + + const result = await findMisusedBearer(tokenWithClaims({ typ: 'ephemeral' }), 'access'); + + expect(result).toBeNull(); + }); + + it('ignores a verified payload whose type disagrees with the claimed one', async () => { + (verifyJwtWithKid as any).mockResolvedValue({ typ: 'access', sub: 'user-1' }); + + const result = await findMisusedBearer(tokenWithClaims({ typ: 'ephemeral' }), 'access'); + + expect(result).toBeNull(); + }); + + it('ignores a verified payload with no type at all', async () => { + (verifyJwtWithKid as any).mockResolvedValue({ sub: 'user-1' }); + + const result = await findMisusedBearer(tokenWithClaims({ typ: 'ephemeral' }), 'access'); + + expect(result).toBeNull(); + }); + + it.each([ + ['no payload segment', 'not-a-jwt'], + ['an empty payload segment', 'header..signature'], + ['an undecodable payload segment', 'header.%%%.signature'], + ['a payload that is not an object', `header.${Buffer.from('"nope"').toString('base64url')}.s`], + ['a null payload', `header.${Buffer.from('null').toString('base64url')}.s`], + ['no type claim', tokenWithClaims({ sub: 'user-1' })], + ['a non-string type claim', tokenWithClaims({ typ: 7 })], + ])('spends no verification on %s', async (_case, token) => { + const result = await findMisusedBearer(token, 'access'); + + expect(result).toBeNull(); + expect(verifyJwtWithKid).not.toHaveBeenCalled(); + }); +}); From eb55b55d31871b8f812c6a23cff792495ba3169c Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 8 Sep 2026 13:18:25 -0400 Subject: [PATCH 2/2] docs(security): say what a bearer_token_failed subject does not prove --- docs/security-posture.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/security-posture.md b/docs/security-posture.md index 22168ce..8dcff5f 100644 --- a/docs/security-posture.md +++ b/docs/security-posture.md @@ -491,6 +491,12 @@ asserting that the caller is that user. It also could not be a foreign key: an e subject may be the decoy `/login` mints for an address with no usable account, which resolves to no row at all. +Read the row as "this account's flow token was offered here", not as "this account did +it". `/login` mints an ephemeral token from an address alone, so anyone who knows an +address can produce a row naming its owner. That is the same reason `userId` is null: +the subject is what the token claims, and the token proves possession of an address, +not of the account. + ### It changes no response The event is written server side and never reflected to the caller. The refusal answers