mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-19 13:21:46 +00:00
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/<slug>/ 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
This commit is contained in:
@@ -41,7 +41,7 @@ export default function ConfirmDialog({
|
|||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
className="fixed inset-0 z-[10000] flex items-center justify-center px-4 trek-backdrop-enter"
|
className="fixed inset-0 z-[10000] flex items-center justify-center px-4 trek-backdrop-enter"
|
||||||
style={{ backgroundColor: 'rgba(15, 23, 42, 0.5)' }}
|
style={{ backgroundColor: 'rgba(15, 23, 42, 0.5)', paddingBottom: 'var(--bottom-nav-h)' }}
|
||||||
onClick={onClose}
|
onClick={onClose}
|
||||||
>
|
>
|
||||||
<div
|
<div
|
||||||
|
|||||||
@@ -42,7 +42,7 @@ export default function CopyTripDialog({ isOpen, tripTitle, onClose, onConfirm }
|
|||||||
return (
|
return (
|
||||||
<div
|
<div
|
||||||
className="fixed inset-0 z-[10000] flex items-center justify-center px-4 trek-backdrop-enter"
|
className="fixed inset-0 z-[10000] flex items-center justify-center px-4 trek-backdrop-enter"
|
||||||
style={{ backgroundColor: 'rgba(15, 23, 42, 0.5)' }}
|
style={{ backgroundColor: 'rgba(15, 23, 42, 0.5)', paddingBottom: 'var(--bottom-nav-h)' }}
|
||||||
onClick={onClose}
|
onClick={onClose}
|
||||||
>
|
>
|
||||||
<div
|
<div
|
||||||
|
|||||||
@@ -112,7 +112,7 @@ router.get('/callback', async (req: Request, res: Response) => {
|
|||||||
tokenData.id_token,
|
tokenData.id_token,
|
||||||
doc,
|
doc,
|
||||||
config.clientId,
|
config.clientId,
|
||||||
config.issuer,
|
(doc.issuer ?? '').replace(/\/+$/, '') || config.issuer,
|
||||||
);
|
);
|
||||||
if (idVerify.ok !== true) {
|
if (idVerify.ok !== true) {
|
||||||
const reason = 'error' in idVerify ? idVerify.error : 'unknown';
|
const reason = 'error' in idVerify ? idVerify.error : 'unknown';
|
||||||
|
|||||||
@@ -140,12 +140,22 @@ export async function discover(issuer: string, discoveryUrl?: string | null): Pr
|
|||||||
const res = await fetch(url);
|
const res = await fetch(url);
|
||||||
if (!res.ok) throw new Error('Failed to fetch OIDC discovery document');
|
if (!res.ok) throw new Error('Failed to fetch OIDC discovery document');
|
||||||
const doc = (await res.json()) as OidcDiscoveryDoc;
|
const doc = (await res.json()) as OidcDiscoveryDoc;
|
||||||
// Validate that the discovery doc's issuer matches the operator-configured
|
// Validate that the discovery doc's issuer matches the operator-configured one.
|
||||||
// one. A MITM or compromised doc could otherwise supply a crafted issuer
|
// When no custom discoveryUrl is set, a mismatch signals a MITM or misconfiguration
|
||||||
// that passes jwt.verify() because we used doc.issuer as the expected value.
|
// and we reject. When the operator explicitly overrides the discovery URL (e.g.
|
||||||
if (doc.issuer && doc.issuer.replace(/\/+$/, '') !== issuer) {
|
// 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}"`);
|
throw new Error(`OIDC discovery issuer mismatch: expected "${issuer}", got "${doc.issuer}"`);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
doc._issuer = url;
|
doc._issuer = url;
|
||||||
discoveryCache = doc;
|
discoveryCache = doc;
|
||||||
discoveryCacheTime = Date.now();
|
discoveryCacheTime = Date.now();
|
||||||
|
|||||||
@@ -219,6 +219,59 @@ describe('discover', () => {
|
|||||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false }));
|
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false }));
|
||||||
await expect(discover('https://bad-issuer.example.com')).rejects.toThrow();
|
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) ─────────────────────────────────
|
// ── issuer trailing-slash regex (ReDoS guard) ─────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user