From 58218ff5f6c5e501c36ead87da0699751d9b2fc8 Mon Sep 17 00:00:00 2001 From: "Julien G." <66769052+jubnl@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:13:35 +0200 Subject: [PATCH] fix(oidc,ui): restore Authentik login and fix mobile delete dialog (#845) OIDC: when OIDC_DISCOVERY_URL is explicitly set, trust the discovery doc's issuer for id_token comparison instead of rejecting a path mismatch as an error. Authentik (and similar realm-path providers) return a canonical issuer like /application/o// that differs from the operator's base OIDC_ISSUER. Strict equality blocked login in 3.x despite working in v2. Default discovery (no custom URL) keeps the strict check. Adds OIDC-SVC-037/038/039. UI: ConfirmDialog and CopyTripDialog lacked the --bottom-nav-h paddingBottom offset that other overlays already use. On mobile portrait the action buttons were hidden behind the sticky bottom nav bar. Closes #843 Closes #844 --- .../src/components/shared/ConfirmDialog.tsx | 2 +- .../src/components/shared/CopyTripDialog.tsx | 2 +- server/src/routes/oidc.ts | 2 +- server/src/services/oidcService.ts | 20 +++++-- .../tests/unit/services/oidcService.test.ts | 53 +++++++++++++++++++ 5 files changed, 71 insertions(+), 8 deletions(-) diff --git a/client/src/components/shared/ConfirmDialog.tsx b/client/src/components/shared/ConfirmDialog.tsx index 31cd9295..d75ad190 100644 --- a/client/src/components/shared/ConfirmDialog.tsx +++ b/client/src/components/shared/ConfirmDialog.tsx @@ -41,7 +41,7 @@ export default function ConfirmDialog({ return (
{ tokenData.id_token, doc, config.clientId, - config.issuer, + (doc.issuer ?? '').replace(/\/+$/, '') || config.issuer, ); if (idVerify.ok !== true) { const reason = 'error' in idVerify ? idVerify.error : 'unknown'; diff --git a/server/src/services/oidcService.ts b/server/src/services/oidcService.ts index 1a8b660f..42c0c5cd 100644 --- a/server/src/services/oidcService.ts +++ b/server/src/services/oidcService.ts @@ -140,11 +140,21 @@ export async function discover(issuer: string, discoveryUrl?: string | null): Pr const res = await fetch(url); if (!res.ok) throw new Error('Failed to fetch OIDC discovery document'); const doc = (await res.json()) as OidcDiscoveryDoc; - // Validate that the discovery doc's issuer matches the operator-configured - // one. A MITM or compromised doc could otherwise supply a crafted issuer - // that passes jwt.verify() because we used doc.issuer as the expected value. - if (doc.issuer && doc.issuer.replace(/\/+$/, '') !== issuer) { - throw new Error(`OIDC discovery issuer mismatch: expected "${issuer}", got "${doc.issuer}"`); + // Validate that the discovery doc's issuer matches the operator-configured one. + // When no custom discoveryUrl is set, a mismatch signals a MITM or misconfiguration + // and we reject. When the operator explicitly overrides the discovery URL (e.g. + // Authentik realm paths), the discovery doc's issuer is the canonical value — + // trust it and warn rather than blocking login. + const docIssuer = doc.issuer?.replace(/\/+$/, '') ?? ''; + if (docIssuer && docIssuer !== issuer) { + if (discoveryUrl) { + console.warn( + `[OIDC] Discovery doc issuer "${doc.issuer}" differs from configured OIDC_ISSUER "${issuer}". ` + + `Using discovery doc issuer for id_token verification (custom OIDC_DISCOVERY_URL is set).`, + ); + } else { + throw new Error(`OIDC discovery issuer mismatch: expected "${issuer}", got "${doc.issuer}"`); + } } doc._issuer = url; discoveryCache = doc; diff --git a/server/tests/unit/services/oidcService.test.ts b/server/tests/unit/services/oidcService.test.ts index 5e27b5fb..5de82a4b 100644 --- a/server/tests/unit/services/oidcService.test.ts +++ b/server/tests/unit/services/oidcService.test.ts @@ -219,6 +219,59 @@ describe('discover', () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false })); await expect(discover('https://bad-issuer.example.com')).rejects.toThrow(); }); + + it('OIDC-SVC-037: accepts mismatched doc issuer when discoveryUrl is explicit', async () => { + const doc = { + issuer: 'https://auth.example.com/application/o/myapp/', + authorization_endpoint: 'https://auth.example.com/application/o/myapp/authorize/', + token_endpoint: 'https://auth.example.com/application/o/token/', + userinfo_endpoint: 'https://auth.example.com/application/o/userinfo/', + }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const result = await discover( + 'https://auth.example.com', + 'https://auth.example.com/application/o/myapp/.well-known/openid-configuration', + ); + + expect(result.issuer).toBe(doc.issuer); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('differs from configured OIDC_ISSUER')); + warnSpy.mockRestore(); + }); + + it('OIDC-SVC-038: throws on mismatched doc issuer when discoveryUrl is omitted', async () => { + const doc = { + issuer: 'https://evil.example.com', + authorization_endpoint: 'https://unique-2.example.com/auth', + token_endpoint: 'https://unique-2.example.com/token', + userinfo_endpoint: 'https://unique-2.example.com/userinfo', + }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); + + await expect(discover('https://unique-2.example.com')).rejects.toThrow( + 'OIDC discovery issuer mismatch', + ); + }); + + it('OIDC-SVC-039: trailing-slash-only mismatch with explicit discoveryUrl does not warn', async () => { + const doc = { + issuer: 'https://auth.example.com/', + authorization_endpoint: 'https://auth.example.com/auth', + token_endpoint: 'https://auth.example.com/token', + userinfo_endpoint: 'https://auth.example.com/userinfo', + }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => doc })); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await discover( + 'https://auth.example.com', + 'https://auth.example.com/.well-known/openid-configuration', + ); + + expect(warnSpy).not.toHaveBeenCalled(); + warnSpy.mockRestore(); + }); }); // ── issuer trailing-slash regex (ReDoS guard) ─────────────────────────────────