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 ──────────────────────────────────────────────────────────────