From c026c5b1c90809d82e19abc477eff94baa4d07a6 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 2 Sep 2026 19:01:03 +0200 Subject: [PATCH] fix(auth): passkey_configured stops advertising the phantom localhost config (#2147) Without any configuration getAppUrl() invents http://localhost as the relying party, and the UI advertised passkeys that every click answered with not configured. A localhost RP only counts as configured now when the operator declared one. The origin check also binds to the expected origins again instead of accepting every subdomain of the RP id; that widening never shipped in a release. --- server/src/nest/auth/auth.service.ts | 22 +++++++++- server/src/nest/auth/passkey.service.ts | 15 +++++-- server/tests/unit/nest/auth.service.test.ts | 32 +++++++++++++++ .../tests/unit/nest/passkey.service.test.ts | 40 +++++++++++++++++-- 4 files changed, 101 insertions(+), 8 deletions(-) diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index a00d1b6eb..c393096a9 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -230,6 +230,26 @@ export class AuthService { // App config (public) // ------------------------------------------------------------------------- + /** + * Whether a passkey ceremony can actually complete on this deployment. With + * nothing configured at all, getAppUrl() invents `http://localhost:{PORT}` and + * the resolver returns a localhost RP for it: a phantom config that every real + * browser is turned away from at the options step (#2147), so advertising it + * as configured only produces a button whose every click 400s. A localhost RP + * the operator pointed here themselves (APP_URL, ALLOWED_ORIGINS or the + * webauthn settings) is a real single-machine install and still counts. + */ + private passkeyConfigured(): boolean { + const cfg = this.webauthn.resolve(); + if (!cfg) return false; + if (cfg.rpID !== 'localhost' || cfg.explicitOrigins) return true; + const env = readEnv(); + const declaredRpId = ( + env.webauthn.rpId || this.db.get<{ value: string }>("SELECT value FROM app_settings WHERE key = 'webauthn_rp_id'")?.value + )?.trim(); + return !!(declaredRpId || env.app.appUrl || env.http.allowedOriginsRaw); + } + getAppConfig(authenticatedUser: User | undefined | null) { const userCount = this.db.get<{ count: number }>('SELECT COUNT(*) as count FROM users WHERE COALESCE(is_guest, 0) = 0')!.count; const isDemo = readEnv().demo.enabled; @@ -281,7 +301,7 @@ export class AuthService { // are true. `passkey_configured` stays a pure boolean — it never leaks the // resolved RP ID / origin / APP_URL on this unauthenticated endpoint. passkey_login: toggles.passkey_login, - passkey_configured: this.webauthn.isConfigured(), + passkey_configured: this.passkeyConfigured(), env_override_oidc_only: readEnv().oidc.only, has_users: userCount > 0, setup_complete: setupComplete, diff --git a/server/src/nest/auth/passkey.service.ts b/server/src/nest/auth/passkey.service.ts index bb5fe04a1..c0e14e886 100644 --- a/server/src/nest/auth/passkey.service.ts +++ b/server/src/nest/auth/passkey.service.ts @@ -146,10 +146,15 @@ export class PasskeyService { * 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. + * entry), so the browser-asserted origin is added when it is the RP ID's own + * host and only the scheme or port drifted. + * + * The HOST is never widened, even though the browser's rule would run a + * ceremony from any subdomain of the RP ID: on an apex RP ID that exact-origin + * list is the only thing between a taken-over sibling subdomain and an + * assertion this server would accept, and the browser cannot supply it. A + * deployment that really spans several hosts under one RP ID lists them in + * webauthn_origins / WEBAUTHN_ORIGINS, which is honored verbatim above. */ private expectedOrigins(cfg: WebauthnConfig, resp: unknown): string[] { if (cfg.explicitOrigins) return cfg.origins; @@ -157,6 +162,8 @@ export class PasskeyService { if (!clientOrigin || cfg.origins.includes(clientOrigin) || !originWithinRpScope(clientOrigin, cfg.rpID)) { return cfg.origins; } + // Parsing cannot throw here: originWithinRpScope only says yes on a URL it parsed. + if (new URL(clientOrigin).hostname !== cfg.rpID) return cfg.origins; return [...cfg.origins, clientOrigin]; } diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 0f0c4c5e5..5e570429f 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -614,6 +614,38 @@ describe('getAppConfig', () => { expect(cfg.password_registration).toBe(true); vi.unstubAllEnvs(); }); + + it('AUTH-DB-052d: an instance that declared nothing reports passkeys as not configured (#2147)', () => { + // APP_URL unset, so getAppUrl() invents http://localhost:{PORT} and the + // resolver hands back a localhost RP. No browser on the real domain can + // finish a ceremony against it, and the options step says so, so the client + // must not advertise a button whose every click 400s. + createUser(testDb); + expect(svc.getAppConfig(null).passkey_configured).toBe(false); + }); + + it('AUTH-DB-052e: a localhost RP the operator declared themselves still counts', () => { + createUser(testDb); + vi.stubEnv('APP_URL', 'http://localhost:5173'); + expect(svc.getAppConfig(null).passkey_configured).toBe(true); + vi.unstubAllEnvs(); + + vi.stubEnv('ALLOWED_ORIGINS', 'http://localhost:5173'); + expect(svc.getAppConfig(null).passkey_configured).toBe(true); + vi.unstubAllEnvs(); + + testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('webauthn_rp_id', 'localhost')").run(); + expect(svc.getAppConfig(null).passkey_configured).toBe(true); + }); + + it('AUTH-DB-052f: a real domain is configured, a bare IP host is not', () => { + createUser(testDb); + vi.stubEnv('APP_URL', 'https://trek.example.org'); + expect(svc.getAppConfig(null).passkey_configured).toBe(true); + vi.stubEnv('APP_URL', 'http://192.168.1.50:3001'); + expect(svc.getAppConfig(null).passkey_configured).toBe(false); + vi.unstubAllEnvs(); + }); }); describe('demoLogin', () => { diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index f3f9bbebe..e60cca554 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -1,13 +1,14 @@ /** * Unit tests for the DI-native PasskeyService — PASSKEY-SVC-001 through - * PASSKEY-SVC-039. Written fresh with the DI fold: the legacy + * PASSKEY-SVC-041. 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); 033–039 pin the + * dup-check → INSERT with its 409-vs-400 sentinel split); 033 to 041 pin the * #2147 origin hardening (pre-ceremony Origin gate for the derived localhost - * fallback, in-scope widening of expectedOrigin for derived configs). + * fallback, scheme/port widening of expectedOrigin for derived configs, and + * that the host itself is never widened). * 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). @@ -441,6 +442,25 @@ describe('passkeyRegisterVerify', () => { 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']); }); + + it('PASSKEY-SVC-040: a derived config heals scheme and port, never the host (#2147)', async () => { + // With the RP ID on an apex domain the browser runs a ceremony from any + // subdomain of it, so a taken-over sibling could hand over an assertion + // nothing else in the chain rejects. The exact-origin list is the only + // defence there, so only scheme/port drift on the RP ID's host is healed. + resolveWebauthnConfigMock.mockReturnValue({ rpID: 'example.com', rpName: 'TREK', origins: ['https://example.com'], explicitOrigins: false }); + const { user } = createUser(testDb); + + seedChallenge('w4', user.id, 'registration'); + swMock.verifyRegistrationResponse.mockResolvedValueOnce(registrationVerdict({}, { id: 'sibling-cred' })); + await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w4', {}, { origin: 'https://blog.example.com' }) }); + expect(swMock.verifyRegistrationResponse.mock.calls[0][0].expectedOrigin).toEqual(['https://example.com']); + + seedChallenge('w5', user.id, 'registration'); + swMock.verifyRegistrationResponse.mockResolvedValueOnce(registrationVerdict({}, { id: 'port-cred' })); + await svc.passkeyRegisterVerify(user.id, { attestationResponse: ceremonyResponse('w5', {}, { origin: 'https://example.com:8443' }) }); + expect(swMock.verifyRegistrationResponse.mock.calls[1][0].expectedOrigin).toEqual(['https://example.com', 'https://example.com:8443']); + }); }); // ── passkeyLoginOptions ─────────────────────────────────────────────────────── @@ -614,6 +634,20 @@ describe('passkeyLoginVerify', () => { expect(swMock.verifyAuthenticationResponse.mock.calls[0][0].expectedOrigin) .toEqual(['http://trek.example.org', 'https://trek.example.org']); }); + + it('PASSKEY-SVC-041: login never widens to a sibling subdomain of the RP ID (#2147)', async () => { + // The relay path the exact-origin list exists for: a challenge fetched from + // the public options endpoint, spent by a ceremony a compromised sibling + // subdomain ran against the apex RP ID. + resolveWebauthnConfigMock.mockReturnValue({ rpID: 'example.com', rpName: 'TREK', origins: ['https://example.com'], explicitOrigins: false }); + const { user } = createUser(testDb); + insertCredential(user.id, { credential_id: 'apex-cred' }); + seedChallenge('a11', null, 'authentication'); + swMock.verifyAuthenticationResponse.mockResolvedValue({ verified: true, authenticationInfo: { newCounter: 1 } }); + + await svc.passkeyLoginVerify({ assertionResponse: ceremonyResponse('a11', { id: 'apex-cred' }, { origin: 'https://blog.example.com' }) }); + expect(swMock.verifyAuthenticationResponse.mock.calls[0][0].expectedOrigin).toEqual(['https://example.com']); + }); }); // ── listPasskeys ──────────────────────────────────────────────────────────────