fix: four review-confirmed regressions in auth + OIDC flows

All found by the multi-angle /code-review over the branch and verified
against the installed libraries:

- 401 discriminator: the session-expiry redirect keyed off a path prefix
  (!/auth/*) and falsely logged users out on /profile/change-password
  with a wrong current password. requireAuth's no-session 401 now carries
  the distinct AUTH_REQUIRED code — the only signal api.ts dispatches
  auth:expired on; wrong-credential 401s keep AUTH_ERROR on any path.
  Test updated to pin the code-based contract on both directions.
- OIDC RFC 9207: exchangeAndVerifyTokens rebuilt the callback URL with
  only code+state, dropping the iss parameter openid-client v6 validates
  when the provider advertises support (authentik does) — every OIDC
  login would fail post-upgrade. The full callback query is now forwarded.
- OIDC http issuers: v6 enforces HTTPS on discovery/endpoints (v5
  didn't) — breaking internal http:// providers on Docker networks.
  OIDC_ALLOW_INSECURE_HTTP=true opts back in (documented in .env.example).
- 2FA downgrade: authService's webauthn fallback caught ANY challenge
  error and silently downgraded to password-only login; now only the
  stale-flag 'No registered WebAuthn credentials' case falls through,
  everything else rethrows.
- Burned-challenge retry: the server consumes the single-use 2FA
  challenge before verifying (anti-replay), so after a server rejection
  a retry can only ever return 'Challenge expired'. LoginPage now resets
  to sign-in on server failure (WebAuthn AND the pre-existing TOTP trap);
  a browser-prompt cancel keeps retry (token still live). Also lazy-loads
  @simplewebauthn/browser out of the entry bundle, matching ProfilePage.

Server 252/252, client 52/52, OIDC smoke 44/44, probe 33/33, e2e 27.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

.env.example

@@ -69,6 +69,12 @@ NODE_ENV=production
 #
 # WEBAUTHN_RP_ID=bills.example.com
 
+# openid-client v6 requires HTTPS for the OIDC issuer and all discovered
+# endpoints. If your identity provider is reached over an internal http://
+# address (e.g. http://authentik:9000 on a Docker network), opt back in:
+#
+# OIDC_ALLOW_INSECURE_HTTP=true
+
 # ── Bank Sync (SimpleFIN) ─────────────────────────────────────────────────────
 # Enable/disable bank sync from the Admin panel. Users connect their own
 # SimpleFIN Bridge from the Data page. No environment config required.

client/api.sessionExpiry.test.ts

@@ -1,22 +1,21 @@
 // @vitest-environment jsdom
 
-// 1e (QA follow-up) — a 401 outside /auth/* means the session died mid-use:
+// Session-expiry contract: the auth middleware is the only source of the
-// api.ts must dispatch 'auth:expired' (AuthProvider clears the user and
+// AUTH_REQUIRED code (no valid session) — api.ts dispatches 'auth:expired' on
-// RequireAuth redirects). A 401 under /auth/* is "wrong credential input"
+// it so AuthProvider clears the user and RequireAuth redirects. Any other 401
-// (e.g. change-password with a bad current password) and must NOT dispatch.
+// (code AUTH_ERROR — wrong credential input, e.g. change-password with a bad
+// current password, on ANY path) must NOT dispatch. A path prefix cannot make
+// that distinction — /profile/change-password proved it in review.
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { api } from '@/api';
 
-function mock401(): typeof fetch {
+function mock401(code: string): typeof fetch {
   return vi.fn(
     async () =>
-      new Response(
+      new Response(JSON.stringify({ error: code, message: 'Unauthorized', code }), {
-        JSON.stringify({ error: 'AUTH_ERROR', message: 'Unauthorized', code: 'AUTH_ERROR' }),
+        status: 401,
-        {
+        headers: { 'Content-Type': 'application/json' },
-          status: 401,
+      }),
-          headers: { 'Content-Type': 'application/json' },
-        },
-      ),
   ) as unknown as typeof fetch;
 }
 
@@ -29,7 +28,6 @@ describe('session-expiry dispatch on 401', () => {
   beforeEach(() => {
     expiredEvents = 0;
     window.addEventListener('auth:expired', onExpired);
-    vi.stubGlobal('fetch', mock401());
   });
 
   afterEach(() => {
@@ -37,12 +35,17 @@ describe('session-expiry dispatch on 401', () => {
     vi.unstubAllGlobals();
   });
 
-  it('dispatches auth:expired for a 401 on a data endpoint', async () => {
+  it('dispatches auth:expired for AUTH_REQUIRED (dead session) on a data endpoint', async () => {
+    vi.stubGlobal('fetch', mock401('AUTH_REQUIRED'));
     await expect(api.bills()).rejects.toMatchObject({ status: 401 });
     expect(expiredEvents).toBe(1);
   });
 
-  it('does NOT dispatch for a 401 under /auth/* (wrong credential input)', async () => {
+  it('does NOT dispatch for AUTH_ERROR (wrong credential) — even outside /auth/*', async () => {
+    vi.stubGlobal('fetch', mock401('AUTH_ERROR'));
+    await expect(
+      api.changeProfilePassword({ current_password: 'wrong', new_password: 'longenough1' }),
+    ).rejects.toMatchObject({ status: 401 });
     await expect(
       api.changePassword({ current_password: 'wrong', new_password: 'longenough1' }),
     ).rejects.toMatchObject({ status: 401 });

client/api.ts

@@ -147,11 +147,13 @@ async function _fetch<T = unknown>(
       _csrfFetch = null;
       return _fetch<T>(method, path, body, true);
     }
-    // Session expired mid-use: a 401 outside /auth/* means the session cookie
+    // Session expired mid-use: the auth middleware is the only source of
-    // died (under /auth/* a 401 is "wrong credential input" — e.g. bad current
+    // AUTH_REQUIRED (no valid session). Wrong-credential 401s (change-password,
-    // password — and must NOT log the user out). AuthProvider listens and
+    // TOTP disable, …) carry AUTH_ERROR and must NOT log the user out — a path
-    // clears the user, so RequireAuth redirects to /login preserving state.from.
+    // prefix can't make that distinction (/profile/change-password proved it).
-    if (res.status === 401 && !path.startsWith('/auth/')) {
+    // AuthProvider listens and clears the user; RequireAuth then redirects to
+    // /login preserving state.from.
+    if (res.status === 401 && data?.code === 'AUTH_REQUIRED') {
       window.dispatchEvent(new Event('auth:expired'));
     }
     throw toApiError(res, data);

client/pages/LoginPage.tsx

@@ -2,7 +2,6 @@ import { useState, useEffect, type ComponentType, type ReactNode } from 'react';
 import { Link, useNavigate, useLocation } from 'react-router-dom';
 import { Gauge, KeyRound, LockKeyhole, ShieldCheck } from 'lucide-react';
 import { toast } from 'sonner';
-import { startAuthentication } from '@simplewebauthn/browser';
 import { api } from '@/api';
 import { errMessage } from '@/lib/utils';
 import { useAuth, type User } from '@/hooks/useAuth';
@@ -186,19 +185,29 @@ export default function LoginPage() {
     setWebauthnError('');
     setWebauthnLoading(true);
     try {
+      // Lazy: keep the WebAuthn lib out of the login-critical entry bundle
+      // (only key-protected accounts ever reach this; ProfilePage does the same).
+      const { startAuthentication } = await import('@simplewebauthn/browser');
       const response = await startAuthentication({
         optionsJSON: options as Parameters<typeof startAuthentication>[0]['optionsJSON'],
       });
       const data = await api.webauthnChallenge({ challenge_token: challengeToken, response });
       handlePostLogin((data as { user?: User }).user!);
     } catch (err) {
-      // NotAllowedError = user cancelled / timed out at the browser prompt.
+      // NotAllowedError = user cancelled / timed out at the browser prompt —
+      // the endpoint was never called, so the single-use challenge token is
+      // still valid and retry works. Any SERVER failure has already consumed
+      // the token (anti-replay), so a retry can only ever return "Challenge
+      // expired" — send the user back to sign in instead of a dead button.
       const cancelled = (err as Error)?.name === 'NotAllowedError';
-      setWebauthnError(
+      if (cancelled) {
-        cancelled
+        setWebauthnError('Security key prompt was cancelled. Try again.');
-          ? 'Security key prompt was cancelled. Try again, or go back and sign in again.'
+      } else {
-          : errMessage(err, 'Security key verification failed.'),
+        setWebauthnChallenge(null);
-      );
+        setWebauthnOptions(null);
+        setWebauthnError('');
+        setError(errMessage(err, 'Security key verification failed.') + ' Please sign in again.');
+      }
     } finally {
       setWebauthnLoading(false);
     }
@@ -217,8 +226,14 @@ export default function LoginPage() {
       const data = await api.totpChallenge(payload);
       handlePostLogin(data.user!);
     } catch (err) {
-      setTotpError(errMessage(err, 'Invalid code.'));
+      // The server consumes the single-use challenge token BEFORE verifying the
+      // code (anti-replay), so after any server rejection a retry with the same
+      // token can only return "Challenge expired" — restart the sign-in instead
+      // of leaving a form that can never succeed. (Same semantics as WebAuthn.)
+      setTotpChallenge(null);
       setTotpCode('');
+      setTotpError('');
+      setError(errMessage(err, 'Invalid code.') + ' Please sign in again.');
     } finally {
       setTotpLoading(false);
     }

middleware/requireAuth.cts

@@ -64,7 +64,11 @@ function requireAuth(req: Req, res: Res, next: Next): any {
   }
 
   const user = getSessionUser(req.cookies?.[COOKIE_NAME]);
-  if (!user) return res.status(401).json(standardizeError('Not authenticated', 'AUTH_ERROR'));
+  // AUTH_REQUIRED (not AUTH_ERROR): the one code that means "no valid session".
+  // The client keys its session-expired redirect off this code — wrong-credential
+  // 401s elsewhere (change-password, TOTP disable, …) use AUTH_ERROR and must
+  // NOT log the user out.
+  if (!user) return res.status(401).json(standardizeError('Not authenticated', 'AUTH_REQUIRED'));
   req.user = user;
   next();
 }

routes/authOidc.cts

@@ -94,10 +94,12 @@ router.get('/callback', async (req: Req, res: Res) => {
   }
 
   try {
-    // Full token exchange + cryptographic ID token verification via openid-client@5.
+    // Full token exchange + cryptographic ID token verification via openid-client@6.
-    // Verifies: state, PKCE, JWT signature (JWKS), issuer, audience, expiry, nonce.
+    // Verifies: state, PKCE, JWT signature (JWKS), issuer, audience, expiry, nonce,
-    // Throws on any validation failure. Tokens are never logged.
+    // and the RFC 9207 `iss` response parameter — which is why the FULL callback
-    const claims = await exchangeAndVerifyTokens(config, code, stateId, savedState);
+    // query is forwarded, not just code/state. Throws on any validation failure.
+    // Tokens are never logged.
+    const claims = await exchangeAndVerifyTokens(config, code, stateId, savedState, req.query);
 
     // Map verified claims to a local user account
     const user = await findOrProvisionUser(claims, config);

services/authService.cts

@@ -94,10 +94,14 @@ async function login(username: any, password: any) {
       const loginToken = createLoginChallenge(getDb(), user.id, challengeId);
       return { requires_webauthn: true, challenge_token: loginToken, webauthn_options: options };
     } catch (err: any) {
-      // Stale flag (enabled but no usable credentials): a hard failure here would
+      // ONLY the stale-flag case (enabled but no usable credentials) may fall
-      // lock the account out entirely — fall through to password-only login instead.
+      // through to password-only login — a hard failure there would lock the
+      // account out entirely. Any other error (DB/crypto hiccup) rethrows:
+      // silently downgrading 2FA to password-only on a transient fault would
+      // let an attacker with the password ride out the second factor.
+      if (err.message !== 'No registered WebAuthn credentials') throw err;
       log.warn(
-        `[auth] WebAuthn enabled for user ${user.id} but challenge creation failed (${err.message}); proceeding with password login`,
+        `[auth] WebAuthn enabled for user ${user.id} but no usable credentials; proceeding with password login`,
       );
     }
   }

services/oidcService.cts

@@ -493,11 +493,17 @@ async function getOidcClient(config) {
     config.tokenEndpointAuthMethod === 'client_secret_post'
       ? oidc.ClientSecretPost(config.clientSecret)
       : oidc.ClientSecretBasic(config.clientSecret);
+  // openid-client v6 enforces HTTPS on the issuer and every discovered
+  // endpoint (v5 allowed http). Self-hosters commonly reach the provider over
+  // an internal http address (e.g. http://authentik:9000 on a Docker network),
+  // so OIDC_ALLOW_INSECURE_HTTP=true opts back in — deliberate and explicit.
+  const allowHttp = process.env.OIDC_ALLOW_INSECURE_HTTP === 'true';
   _cachedClient = await oidc.discovery(
     new URL(config.issuerUrl),
     config.clientId,
     { redirect_uris: [config.redirectUri], response_types: ['code'] },
     clientAuth,
+    allowHttp ? { execute: [oidc.allowInsecureRequests] } : undefined,
   );
   _cachedClientKey = clientKey;
   _cacheTs = now;
@@ -649,14 +655,19 @@ async function buildAuthorizationUrl(config, state) {
  *
  * Tokens are never logged. Returns verified claims on success.
  */
-async function exchangeAndVerifyTokens(config, code, stateId, savedState) {
+async function exchangeAndVerifyTokens(config, code, stateId, savedState, callbackQuery = {}) {
   const client = await getOidcClient(config);
 
   // authorizationCodeGrant() handles token exchange + full validation in one
   // step (signature via discovered JWKS, iss/aud/exp/nbf, PKCE, state, nonce).
-  // It throws on any problem. The "current URL" is the callback URL as the
+  // It throws on any problem. The "current URL" must carry EVERY parameter the
-  // provider redirected to it.
+  // provider actually returned — v6 validates the RFC 9207 `iss` parameter when
+  // the provider advertises authorization_response_iss_parameter_supported
+  // (authentik does), so rebuilding with only code/state breaks the exchange.
   const currentUrl = new URL(config.redirectUri);
+  for (const [key, value] of Object.entries(callbackQuery)) {
+    if (typeof value === 'string') currentUrl.searchParams.set(key, value);
+  }
   currentUrl.searchParams.set('code', code);
   currentUrl.searchParams.set('state', stateId);