diff --git a/server/src/nest/auth/passkey.controller.ts b/server/src/nest/auth/passkey.controller.ts index 1b2a18b52..9aba3f5e0 100644 --- a/server/src/nest/auth/passkey.controller.ts +++ b/server/src/nest/auth/passkey.controller.ts @@ -49,7 +49,9 @@ export class PasskeyController { @UseGuards(PasskeyEnabledGuard, JwtAuthGuard) async registerOptions(@CurrentUser() user: User, @Body() body: PasskeyRegisterOptionsDto, @Req() req: Request) { this.limit('mfa', req, 5); - const result = await this.passkeys.passkeyRegisterOptions(user.id, body?.password); + // The Origin header is only a pre-ceremony sanity input (see PasskeyService) + // — the RP ID itself is never derived from request headers. + const result = await this.passkeys.passkeyRegisterOptions(user.id, body?.password, req.headers.origin); if (result.error) throw new HttpException({ error: result.error }, result.status!); return result.options; } @@ -72,7 +74,7 @@ export class PasskeyController { @UseGuards(PasskeyEnabledGuard) async loginOptions(@Req() req: Request) { this.limit('login', req, 10); - const result = await this.passkeys.passkeyLoginOptions(); + const result = await this.passkeys.passkeyLoginOptions(req.headers.origin); if (result.error) throw new HttpException({ error: result.error }, result.status!); return result.options; } diff --git a/server/src/nest/auth/passkey.service.ts b/server/src/nest/auth/passkey.service.ts index 880d7f89f..bb5fe04a1 100644 --- a/server/src/nest/auth/passkey.service.ts +++ b/server/src/nest/auth/passkey.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import bcrypt from 'bcryptjs'; import { generateRegistrationOptions, @@ -7,7 +7,7 @@ import { verifyAuthenticationResponse, type AuthenticatorTransportFuture, } from '@simplewebauthn/server'; -import { WebauthnConfigService } from './webauthn-config.service'; +import { WebauthnConfigService, originWithinRpScope, type WebauthnConfig } from './webauthn-config.service'; import { avatarUrl } from '../common/avatarUrl'; import { stripUserForClient } from './auth.helpers'; import { AuthService } from './auth.service'; @@ -52,17 +52,27 @@ interface CredentialRow { last_used_at: string | null; } -function challengeFromResponse(resp: unknown): string | null { +function clientDataFromResponse(resp: unknown): { challenge?: unknown; origin?: unknown } | null { try { const cdj = (resp as { response?: { clientDataJSON?: unknown } })?.response?.clientDataJSON; if (typeof cdj !== 'string') return null; - const parsed = JSON.parse(Buffer.from(cdj, 'base64url').toString('utf8')) as { challenge?: unknown }; - return typeof parsed.challenge === 'string' ? parsed.challenge : null; + return JSON.parse(Buffer.from(cdj, 'base64url').toString('utf8')) as { challenge?: unknown; origin?: unknown }; } catch { return null; } } +function challengeFromResponse(resp: unknown): string | null { + const challenge = clientDataFromResponse(resp)?.challenge; + return typeof challenge === 'string' ? challenge : null; +} + +/** The browser-asserted origin of the ceremony (clientDataJSON.origin). */ +function originFromResponse(resp: unknown): string | null { + const origin = clientDataFromResponse(resp)?.origin; + return typeof origin === 'string' ? origin : null; +} + function parseTransports(raw: string | null): AuthenticatorTransportFuture[] | undefined { if (!raw) return undefined; try { @@ -91,6 +101,8 @@ function defaultCredentialName(deviceType: string | undefined): string { */ @Injectable() export class PasskeyService { + private readonly logger = new Logger(PasskeyService.name); + constructor( private readonly db: DatabaseService, private readonly auth: AuthService, @@ -126,6 +138,55 @@ export class PasskeyService { return row ?? null; } + // ------------------------------------------------------------------------- + // Origin handling (#2147) + // ------------------------------------------------------------------------- + + /** + * The origins handed to the verifier for this ceremony. An explicit operator + * list is honored verbatim. A derived config (APP_URL fallback) often carries + * the wrong scheme or port (TLS-terminating proxy, first ALLOWED_ORIGINS + * entry), so the browser-asserted origin is added IF it falls within the RP + * ID's WebAuthn scope — the same suffix rule the browser enforces before it + * runs the ceremony at all. rpIdHash, challenge and signature checks are + * untouched, so this never accepts an origin the RP ID doesn't cover. + */ + private expectedOrigins(cfg: WebauthnConfig, resp: unknown): string[] { + if (cfg.explicitOrigins) return cfg.origins; + const clientOrigin = originFromResponse(resp); + if (!clientOrigin || cfg.origins.includes(clientOrigin) || !originWithinRpScope(clientOrigin, cfg.rpID)) { + return cfg.origins; + } + return [...cfg.origins, clientOrigin]; + } + + /** + * Pre-ceremony check of the request's Origin header against the resolved RP + * config. A browser on an origin the derived localhost fallback can never + * verify (nothing configured, accessed remotely) gets the actionable + * not-configured 400 BEFORE the ceremony instead of a cryptic verify failure + * after it. Any other mismatch is only logged: proxies may rewrite the + * Origin header while clientDataJSON still carries the real one, and an + * explicit origins list is the operator's contract. Requests without a + * parseable http(s) Origin (curl, 'null') skip the check entirely. + */ + private originCannotVerify(cfg: WebauthnConfig, requestOrigin: string | undefined): boolean { + if (!requestOrigin) return false; + let origin: string; + try { + const url = new URL(requestOrigin); + if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; + origin = url.origin; + } catch { + return false; + } + if (cfg.origins.includes(origin) || originWithinRpScope(origin, cfg.rpID)) return false; + this.logger.warn( + `Passkey ceremony requested from origin ${origin}, but the RP config resolves to rpID=${cfg.rpID}, origins=[${cfg.origins.join(', ')}] — set APP_URL or the webauthn_rp_id/webauthn_origins settings.`, + ); + return !cfg.explicitOrigins && cfg.rpID === 'localhost'; + } + // ------------------------------------------------------------------------- // Registration (authenticated — from Settings, password re-auth required) // ------------------------------------------------------------------------- @@ -133,9 +194,11 @@ export class PasskeyService { async passkeyRegisterOptions( userId: number, password: string | undefined, + requestOrigin?: string, ): Promise<{ error?: string; status?: number; options?: Awaited> }> { const cfg = this.webauthn.resolve(); if (!cfg) return { ...NOT_CONFIGURED }; + if (this.originCannotVerify(cfg, requestOrigin)) return { ...NOT_CONFIGURED }; const user = this.db.get('SELECT * FROM users WHERE id = ?', userId); if (!user) return { error: 'User not found', status: 404 }; @@ -190,16 +253,22 @@ export class PasskeyService { return { error: 'Registration challenge expired. Please try again.', status: 400 }; } + const expectedOrigin = this.expectedOrigins(cfg, resp); let verification; try { verification = await verifyRegistrationResponse({ response: resp as Parameters[0]['response'], expectedChallenge: challenge, - expectedOrigin: cfg.origins, + expectedOrigin, expectedRPID: cfg.rpID, requireUserVerification: true, }); - } catch { + } catch (err) { + // Body stays the generic 400 (parity), but the operator gets to see WHY — + // origin/RP mismatches used to be swallowed entirely. + this.logger.warn( + `Passkey registration rejected (expectedRPID=${cfg.rpID}, expectedOrigin=[${expectedOrigin.join(', ')}]): ${err instanceof Error ? err.message : String(err)}`, + ); return { error: 'Could not register this passkey.', status: 400 }; } @@ -253,13 +322,14 @@ export class PasskeyService { // Authentication (public — primary, discoverable-credential login) // ------------------------------------------------------------------------- - async passkeyLoginOptions(): Promise<{ + async passkeyLoginOptions(requestOrigin?: string): Promise<{ error?: string; status?: number; options?: Awaited>; }> { const cfg = this.webauthn.resolve(); if (!cfg) return { ...NOT_CONFIGURED }; + if (this.originCannotVerify(cfg, requestOrigin)) return { ...NOT_CONFIGURED }; const now = Date.now(); this.purgeExpiredChallenges(now); @@ -302,12 +372,13 @@ export class PasskeyService { const cred = this.db.get('SELECT * FROM webauthn_credentials WHERE credential_id = ?', credId); if (!cred) return { ...AUTH_FAILED }; + const expectedOrigin = this.expectedOrigins(cfg, resp); let verification; try { verification = await verifyAuthenticationResponse({ response: resp as Parameters[0]['response'], expectedChallenge: challenge, - expectedOrigin: cfg.origins, + expectedOrigin, expectedRPID: cfg.rpID, requireUserVerification: true, credential: { @@ -317,7 +388,10 @@ export class PasskeyService { transports: parseTransports(cred.transports), }, }); - } catch { + } catch (err) { + this.logger.warn( + `Passkey login rejected (expectedRPID=${cfg.rpID}, expectedOrigin=[${expectedOrigin.join(', ')}]): ${err instanceof Error ? err.message : String(err)}`, + ); return { ...AUTH_FAILED }; } diff --git a/server/src/nest/auth/webauthn-config.service.ts b/server/src/nest/auth/webauthn-config.service.ts index fed212fab..b9f918c1e 100644 --- a/server/src/nest/auth/webauthn-config.service.ts +++ b/server/src/nest/auth/webauthn-config.service.ts @@ -17,11 +17,38 @@ function isIpHost(host: string): boolean { return false; } +/** + * The scope rule WebAuthn itself applies when the browser runs a ceremony: the + * origin's host must equal the RP ID or be a subdomain of it, over https + * (plain http only for localhost loopback dev). Used to judge whether an + * origin could ever complete a ceremony against a given RP ID — never to + * derive the RP ID from anything request-controlled. + */ +export function originWithinRpScope(origin: string, rpID: string): boolean { + let url: URL; + try { + url = new URL(origin); + } catch { + return false; + } + const host = url.hostname; + const loopback = host === 'localhost' || host === '127.0.0.1'; + if (url.protocol !== 'https:' && !(url.protocol === 'http:' && loopback)) return false; + return host === rpID || host.endsWith(`.${rpID}`); +} + export interface WebauthnConfig { rpID: string; rpName: string; /** Exact allowed origins (scheme + host + port). One in prod; localhost dev adds the Vite/API ports. */ origins: string[]; + /** + * True when `origins` came from operator config (env / admin setting) — such + * a list is honored verbatim. False when derived from APP_URL, where the + * scheme/port may not match the real browsing origin (TLS-terminating proxy) + * and the verifier may widen within the RP ID's scope. + */ + explicitOrigins: boolean; } /** @@ -87,7 +114,7 @@ export class WebauthnConfigService { } if (origins.length === 0) return null; - return { rpID, rpName: 'TREK', origins }; + return { rpID, rpName: 'TREK', origins, explicitOrigins: explicitOrigins.length > 0 }; } /** True when a usable RP ID resolves for this deployment (exposed as a pure boolean on app-config). */ diff --git a/server/tests/e2e/auth.e2e.test.ts b/server/tests/e2e/auth.e2e.test.ts index 7ef0f1b49..abb0c8a61 100644 --- a/server/tests/e2e/auth.e2e.test.ts +++ b/server/tests/e2e/auth.e2e.test.ts @@ -36,9 +36,12 @@ vi.mock('../../src/websocket', () => ({ broadcastToUser: vi.fn(), broadcast: vi. // The audit domain is DI-native: writeAudit runs for real against the temp // db's audit_log table; only the file logger is silenced. vi.mock('../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logDebug: vi.fn(), logError: vi.fn(), logWarn: vi.fn() })); +// Switchable so the passkey cases can reproduce the real APP_URL-unset +// fallback (http://localhost:{PORT}) without disturbing the other cases. +const { appUrlRef } = vi.hoisted(() => ({ appUrlRef: { value: 'https://x' } })); vi.mock('../../src/app-config', async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, getAppUrl: () => 'https://x' }; + return { ...actual, getAppUrl: () => appUrlRef.value }; }); import { MailerService } from '../../src/nest/notifications/mailer/mailer.service'; @@ -326,6 +329,56 @@ describe('Auth e2e (real auth guard + real service + real cookie service + temp expect(await hasMapsKey(admin.user.id)).toBe(true); }); + describe('passkey origin gate (#2147)', () => { + beforeAll(() => { + db.prepare("INSERT INTO app_settings (key, value) VALUES ('passkey_login', 'true') ON CONFLICT(key) DO UPDATE SET value = 'true'").run(); + // What getAppUrl() really yields with APP_URL unset — the phantom config. + appUrlRef.value = 'http://localhost:3001'; + }); + + afterAll(() => { + db.prepare("DELETE FROM app_settings WHERE key = 'passkey_login'").run(); + appUrlRef.value = 'https://x'; + }); + + it('register/options answers the actionable not-configured 400 for a remote origin', async () => { + const res = await request(server) + .post('/api/auth/passkey/register/options') + .set('Cookie', sessionCookie(userId)) + .set('Origin', 'https://trip.example.org') + .send({ password: userPassword }); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'Passkey login is not configured for this server.' }); + }); + + it('register/options still serves the localhost dev flow (no Origin header, localhost origin)', async () => { + const bare = await request(server) + .post('/api/auth/passkey/register/options') + .set('Cookie', sessionCookie(userId)) + .send({ password: userPassword }); + expect(bare.status).toBe(200); + expect(bare.body.rp).toEqual({ name: 'TREK', id: 'localhost' }); + + const local = await request(server) + .post('/api/auth/passkey/register/options') + .set('Cookie', sessionCookie(userId)) + .set('Origin', 'http://localhost:5173') + .send({ password: userPassword }); + expect(local.status).toBe(200); + expect(local.body.rp.id).toBe('localhost'); + }); + + it('login/options gets the same gate on the same derivation chain', async () => { + resetRateLimits(app); // shares the per-ip 'login' bucket with the login cases above + const res = await request(server) + .post('/api/auth/passkey/login/options') + .set('Origin', 'https://trip.example.org') + .send({}); + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: 'Passkey login is not configured for this server.' }); + }); + }); + it('POST /logout clears the session cookie', async () => { const res = await request(server).post('/api/auth/logout'); expect(res.status).toBe(200); diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index f330b68e6..f3f9bbebe 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -1,11 +1,13 @@ /** * Unit tests for the DI-native PasskeyService — PASSKEY-SVC-001 through - * PASSKEY-SVC-032. Written fresh with the DI fold: the legacy + * PASSKEY-SVC-039. Written fresh with the DI fold: the legacy * services/passkeyService had no service-level suite, so these characterize * the relocated behavior (challenge store semantics, WebAuthn ceremony error * paths, clone detection, management CRUD) over a real in-memory SQLite DB; * 031–032 pin the post-fold `fix(server)` quirk fix (the transactional - * dup-check → INSERT with its 409-vs-400 sentinel split). + * dup-check → INSERT with its 409-vs-400 sentinel split); 033–039 pin the + * #2147 origin hardening (pre-ceremony Origin gate for the derived localhost + * fallback, in-scope widening of expectedOrigin for derived configs). * The @simplewebauthn/server ceremony functions are mocked — the library's * crypto is not under test, the service's handling of its verdicts is. * Constructed directly (no TestingModule, repo convention). @@ -118,7 +120,9 @@ const auth = new AuthService( ); const svc = new PasskeyService(new DatabaseService(testDb), auth, webauthn); -const CFG = { rpID: 'trek.example.com', rpName: 'TREK', origins: ['https://trek.example.com'] }; +const CFG = { rpID: 'trek.example.com', rpName: 'TREK', origins: ['https://trek.example.com'], explicitOrigins: false }; +// The unconfigured fallback resolve() yields when APP_URL is unset (#2147). +const LOCALHOST_CFG = { rpID: 'localhost', rpName: 'TREK', origins: ['http://localhost:5173', 'http://localhost:3001'], explicitOrigins: false }; const NOT_CONFIGURED_ERROR = 'Passkey login is not configured for this server.'; beforeAll(() => { @@ -138,10 +142,10 @@ afterAll(() => { // ── helpers ─────────────────────────────────────────────────────────────────── -/** A response payload whose clientDataJSON echoes the given challenge. */ -function ceremonyResponse(challenge: string, extra: Record = {}) { +/** A response payload whose clientDataJSON echoes the given challenge (plus optional extra client data, e.g. origin). */ +function ceremonyResponse(challenge: string, extra: Record = {}, clientData: Record = {}) { return { - response: { clientDataJSON: Buffer.from(JSON.stringify({ challenge }), 'utf8').toString('base64url') }, + response: { clientDataJSON: Buffer.from(JSON.stringify({ challenge, ...clientData }), 'utf8').toString('base64url') }, ...extra, }; } @@ -241,6 +245,45 @@ describe('passkeyRegisterOptions', () => { expect(stored[0]).toMatchObject({ challenge: 'reg-chal', user_id: user.id, type: 'registration' }); expect(Number(stored[0].expires_at)).toBeGreaterThan(Date.now()); }); + + it('PASSKEY-SVC-033: the derived localhost fallback fails fast for a foreign request origin (#2147)', async () => { + // APP_URL unset → resolve() yields the localhost phantom config. A browser + // on a real domain can never finish that ceremony, so the actionable + // not-configured 400 comes BEFORE the ceremony, not as a cryptic verify 400. + resolveWebauthnConfigMock.mockReturnValue(LOCALHOST_CFG); + const { user, password } = createUser(testDb); + expect(await svc.passkeyRegisterOptions(user.id, password, 'https://trip.example.org')) + .toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); + expect(swMock.generateRegistrationOptions).not.toHaveBeenCalled(); + expect(challengeCount()).toBe(0); + }); + + it('PASSKEY-SVC-034: localhost origins and headerless requests keep working against the dev fallback', async () => { + resolveWebauthnConfigMock.mockReturnValue(LOCALHOST_CFG); + const { user, password } = createUser(testDb); + let seq = 0; // distinct challenges — the store has a UNIQUE constraint + swMock.generateRegistrationOptions.mockImplementation(async () => ({ challenge: `reg-chal-${++seq}` })); + + // Listed origin, in-scope origin on another port, no header (curl) — all pass. + expect(await svc.passkeyRegisterOptions(user.id, password, 'http://localhost:5173')).toEqual({ options: { challenge: 'reg-chal-1' } }); + expect(await svc.passkeyRegisterOptions(user.id, password, 'http://localhost:8080')).toEqual({ options: { challenge: 'reg-chal-2' } }); + expect(await svc.passkeyRegisterOptions(user.id, password, undefined)).toEqual({ options: { challenge: 'reg-chal-3' } }); + }); + + it('PASSKEY-SVC-035: a mismatching origin never blocks a real or explicit config (proxy tolerance)', async () => { + // Proxies may rewrite the Origin header to an internal value while + // clientDataJSON still carries the real one — options must not hard-fail + // for a config that verify can still satisfy. + const { user, password } = createUser(testDb); + let seq = 0; + swMock.generateRegistrationOptions.mockImplementation(async () => ({ challenge: `reg-chal-${++seq}` })); + + resolveWebauthnConfigMock.mockReturnValue(CFG); // derived from a real APP_URL + expect(await svc.passkeyRegisterOptions(user.id, password, 'http://10.0.0.5:3001')).toEqual({ options: { challenge: 'reg-chal-1' } }); + + resolveWebauthnConfigMock.mockReturnValue({ ...LOCALHOST_CFG, explicitOrigins: true }); // operator contract + expect(await svc.passkeyRegisterOptions(user.id, password, 'https://trip.example.org')).toEqual({ options: { challenge: 'reg-chal-2' } }); + }); }); // ── passkeyRegisterVerify ───────────────────────────────────────────────────── @@ -362,6 +405,42 @@ describe('passkeyRegisterVerify', () => { expect(row).toEqual({ name: 'Original', user_id: user.id }); expect(testDb.prepare('SELECT COUNT(*) AS n FROM webauthn_credentials').get()).toEqual({ n: 1 }); }); + + it('PASSKEY-SVC-036: widens expectedOrigin with the in-scope browser origin for a derived config (#2147)', async () => { + // ALLOWED_ORIGINS-derived configs carry the wrong scheme behind a + // TLS-terminating proxy (http://… while the browser is on https://…). The + // browser-asserted origin is added because it falls within the RP scope; + // rpIdHash/challenge/signature checks still run in the verifier. + resolveWebauthnConfigMock.mockReturnValue({ rpID: 'trek.example.org', rpName: 'TREK', origins: ['http://trek.example.org'], explicitOrigins: false }); + const { user } = createUser(testDb); + seedChallenge('w1', user.id, 'registration'); + swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict()); + + const result = await svc.passkeyRegisterVerify(user.id, { + attestationResponse: ceremonyResponse('w1', {}, { origin: 'https://trek.example.org' }), + }); + expect(result.success).toBe(true); + expect(swMock.verifyRegistrationResponse.mock.calls[0][0].expectedOrigin) + .toEqual(['http://trek.example.org', 'https://trek.example.org']); + }); + + it('PASSKEY-SVC-037: never widens for an explicit origins list or an out-of-scope origin', async () => { + const { user } = createUser(testDb); + + // Operator-supplied list is verbatim — an in-scope but unlisted origin + // still has to fail verify (documented contract, options let it through). + resolveWebauthnConfigMock.mockReturnValue({ ...CFG, origins: ['https://trek.example.com'], explicitOrigins: true }); + seedChallenge('w2', user.id, 'registration'); + swMock.verifyRegistrationResponse.mockResolvedValue(registrationVerdict()); + await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w2', {}, { origin: 'https://app.trek.example.com' }) }); + expect(swMock.verifyRegistrationResponse.mock.calls[0][0].expectedOrigin).toEqual(['https://trek.example.com']); + + // Derived config, but the asserted origin is outside the RP scope. + resolveWebauthnConfigMock.mockReturnValue(CFG); + seedChallenge('w3', user.id, 'registration'); + await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w3', {}, { origin: 'https://evil.example.net' }) }); + expect(swMock.verifyRegistrationResponse.mock.calls[1][0].expectedOrigin).toEqual(['https://trek.example.com']); + }); }); // ── passkeyLoginOptions ─────────────────────────────────────────────────────── @@ -384,6 +463,16 @@ describe('passkeyLoginOptions', () => { expect(stored).toHaveLength(1); // the expired row was purged expect(stored[0]).toMatchObject({ challenge: 'auth-chal', user_id: null, type: 'authentication' }); }); + + it('PASSKEY-SVC-038: the derived localhost fallback fails fast for a foreign request origin (#2147)', async () => { + resolveWebauthnConfigMock.mockReturnValue(LOCALHOST_CFG); + expect(await svc.passkeyLoginOptions('https://trip.example.org')).toEqual({ error: NOT_CONFIGURED_ERROR, status: 400 }); + expect(swMock.generateAuthenticationOptions).not.toHaveBeenCalled(); + + // No Origin header (non-browser client) → unchanged legacy behavior. + swMock.generateAuthenticationOptions.mockResolvedValue({ challenge: 'auth-chal' }); + expect(await svc.passkeyLoginOptions()).toEqual({ options: { challenge: 'auth-chal' } }); + }); }); // ── passkeyLoginVerify ──────────────────────────────────────────────────────── @@ -508,6 +597,23 @@ describe('passkeyLoginVerify', () => { expect(userRow.last_login).not.toBeNull(); expect(userRow.login_count).toBe(1); }); + + it('PASSKEY-SVC-039: widens expectedOrigin for a derived config on login too (#2147)', async () => { + // Same derivation chain as registration: an instance whose APP_URL changes + // scheme after enrollment must not brick every passkey login. + resolveWebauthnConfigMock.mockReturnValue({ rpID: 'trek.example.org', rpName: 'TREK', origins: ['http://trek.example.org'], explicitOrigins: false }); + const { user } = createUser(testDb); + insertCredential(user.id, { credential_id: 'scheme-heal' }); + seedChallenge('a10', null, 'authentication'); + swMock.verifyAuthenticationResponse.mockResolvedValue({ verified: true, authenticationInfo: { newCounter: 1 } }); + + const result = await svc.passkeyLoginVerify({ + assertionResponse: ceremonyResponse('a10', { id: 'scheme-heal' }, { origin: 'https://trek.example.org' }), + }); + expect(result.token).toBeTruthy(); + expect(swMock.verifyAuthenticationResponse.mock.calls[0][0].expectedOrigin) + .toEqual(['http://trek.example.org', 'https://trek.example.org']); + }); }); // ── listPasskeys ────────────────────────────────────────────────────────────── diff --git a/server/tests/unit/nest/webauthn-config.service.test.ts b/server/tests/unit/nest/webauthn-config.service.test.ts index 090391b37..9637ff0b9 100644 --- a/server/tests/unit/nest/webauthn-config.service.test.ts +++ b/server/tests/unit/nest/webauthn-config.service.test.ts @@ -8,9 +8,12 @@ * resolver NEVER reads request headers. */ +// The real getAppUrl() NEVER returns an empty string — with nothing configured +// it falls back to `http://localhost:{PORT}` (app-url.ts). The mock default +// mirrors that; a '' default here once hid the #2147 phantom-config bug. const { settingsStore, appUrlRef } = vi.hoisted(() => ({ settingsStore: new Map(), - appUrlRef: { value: '' }, + appUrlRef: { value: 'http://localhost:3001' }, })); vi.mock('../../../src/app-config', async (importOriginal) => { @@ -20,7 +23,7 @@ vi.mock('../../../src/app-config', async (importOriginal) => { import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { DatabaseService } from '../../../src/nest/database/database.service'; -import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; +import { WebauthnConfigService, originWithinRpScope } from '../../../src/nest/auth/webauthn-config.service'; // Injected instead of vi.mocking src/db/database: the service reads exactly one // row shape (app_settings.value), so a stub connection keeps the suite free of @@ -38,7 +41,7 @@ const svc = new WebauthnConfigService(new DatabaseService(conn)); beforeEach(() => { settingsStore.clear(); - appUrlRef.value = ''; + appUrlRef.value = 'http://localhost:3001'; }); afterEach(() => { @@ -52,6 +55,7 @@ describe('WebauthnConfigService.resolve', () => { expect(cfg).not.toBeNull(); expect(cfg!.rpID).toBe('trek.example.org'); expect(cfg!.origins).toEqual(['https://trek.example.org']); + expect(cfg!.explicitOrigins).toBe(false); // derived, not operator-supplied expect(svc.isConfigured()).toBe(true); }); @@ -61,9 +65,14 @@ describe('WebauthnConfigService.resolve', () => { expect(svc.isConfigured()).toBe(false); }); - it('WAC-003: returns null when nothing is configured', () => { - expect(svc.resolve()).toBeNull(); - expect(svc.isConfigured()).toBe(false); + it('WAC-003: nothing configured resolves to the localhost dev fallback (#2147)', () => { + // getAppUrl() never yields '' — an unconfigured instance lands on + // http://localhost:{PORT}, so resolve() returns the localhost dev config. + // The remote-origin guard for that phantom config lives in PasskeyService. + const cfg = svc.resolve(); + expect(cfg!.rpID).toBe('localhost'); + expect(cfg!.explicitOrigins).toBe(false); + expect(svc.isConfigured()).toBe(true); }); it('WAC-004: localhost dev uses the browser (Vite :5173) origin, not just the API port', () => { @@ -81,6 +90,7 @@ describe('WebauthnConfigService.resolve', () => { const cfg = svc.resolve(); expect(cfg!.rpID).toBe('public.example.org'); expect(cfg!.origins).toEqual(['https://public.example.org']); + expect(cfg!.explicitOrigins).toBe(true); }); it('WAC-006: webauthn_origins is parsed as a comma-separated, trimmed list', () => { @@ -104,6 +114,7 @@ describe('WebauthnConfigService.resolve', () => { settingsStore.set('webauthn_rp_id', 'trek.example.org'); const cfg = svc.resolve(); expect(cfg!.origins).toEqual(['https://trek.example.org']); + expect(cfg!.explicitOrigins).toBe(false); // the origin list itself is still derived }); it('WAC-009: an operator flipping the RP ID at runtime takes effect on the next resolve', () => { @@ -114,4 +125,36 @@ describe('WebauthnConfigService.resolve', () => { expect(svc.resolve()!.rpID).toBe('moved.example.org'); }); + + it('WAC-010: the WEBAUTHN_ORIGINS env var also marks the origin list explicit', () => { + vi.stubEnv('WEBAUTHN_RP_ID', 'env.example.org'); + vi.stubEnv('WEBAUTHN_ORIGINS', 'https://env.example.org'); + expect(svc.resolve()!.explicitOrigins).toBe(true); + }); +}); + +// The scope rule mirrors what the browser enforces for a ceremony: https-only +// (http for loopback dev), hostname equal to the RP ID or a subdomain of it. +// Anything laxer would weaken origin binding, so each edge is pinned. +describe('originWithinRpScope', () => { + it('WAC-011: accepts the RP domain and its subdomains over https', () => { + expect(originWithinRpScope('https://trek.example.org', 'trek.example.org')).toBe(true); + expect(originWithinRpScope('https://app.trek.example.org', 'trek.example.org')).toBe(true); + expect(originWithinRpScope('https://trek.example.org:8443', 'trek.example.org')).toBe(true); + }); + + it('WAC-012: rejects http on a non-localhost host, but allows loopback dev', () => { + expect(originWithinRpScope('http://trek.example.org', 'trek.example.org')).toBe(false); + expect(originWithinRpScope('http://localhost:5173', 'localhost')).toBe(true); + expect(originWithinRpScope('http://127.0.0.1:3001', '127.0.0.1')).toBe(true); + }); + + it('WAC-013: rejects foreign hosts, suffix tricks and garbage', () => { + expect(originWithinRpScope('https://evil.example.net', 'trek.example.org')).toBe(false); + // "eviltrek.example.org" is NOT a subdomain of "trek.example.org". + expect(originWithinRpScope('https://eviltrek.example.org', 'trek.example.org')).toBe(false); + expect(originWithinRpScope('https://trip.example.org', 'localhost')).toBe(false); + expect(originWithinRpScope('null', 'trek.example.org')).toBe(false); + expect(originWithinRpScope('ftp://trek.example.org', 'trek.example.org')).toBe(false); + }); });