auth-providers: OIDC callback hardening

[MichaelUray]

Summary

OIDC robustness fixes that surfaced while integrating Huly with Authentik for a production deployment. All commits are standalone and have no dependency on the forthcoming WAC v1 stack. The branch grew beyond the original three commits; the sections below cover the current state.

Commits

1. fix(authProviders): instrument OIDC callback + redirect on fail (not 500)

Problem: Every OIDC callback (bogus or real code) returned 500 Internal Server Error, even with valid sessions. No log line was emitted between try auth via openid and the 500 — passport-strategy errors threw silently before handleProviderAuth could log them.

Fix:

• Switch to the explicit-callback passport.authenticate variant so strategy errors surface as err/info/status args instead of being swallowed.
• Privacy-safe diagnostic shape: no raw code / state / tokens / user. Logged fields are stage, errMessage/Name/Stack, hasUser, infoSummary, statusCode, hasSession + sessionKeys (names only), cookie + state + code presence (lengths only), host, forwardedProto.
• Permanent invalid-callback guard: ALL failures → 302 /login?error=oidc_callback_failed, never 500. Same wrap covers handleProviderAuth throws.

2. fix(account): admin.ts trim+lowercase on both env-set AND lookup + account(admin): keep non-email ADMIN_EMAILS entries by default; strict mode opt-in

Problem: server/account/src/admin.ts built the ADMIN_EMAILS set without normalizing. All callers pass an already lowercased/trimmed email (cleanEmail), so an env value such as ADMIN_EMAILS=" Michael@Example.com " never matched and silently produced a non-admin login.

Fix: Env split → trim → toLowerCase → drop empty entries; lookup normalizes the same way. Every non-empty entry that matched before still matches, and entries with uppercase letters or surrounding whitespace now match too.

Deliberate tightening — empty entries: the original new Set(process.env.ADMIN_EMAILS?.split(',') ?? []) put '' into the set for any empty entry — ADMIN_EMAILS='', a leading comma (,admin), a trailing comma (admin,, e.g. when PLATFORM_ADMIN_EMAILS is empty) or a double comma (admin,,ops@example.com). isAdminEmail('') was then true, and loginOrSignUpWithProvider passes '' when the provider returns no email, so such a login received an admin token. Empty entries are now dropped in every position and isAdminEmail('') is always false. A deployment that relied on this would lose admin for email-less provider logins; that is intended.

Entries without @ are kept by default — the repo's own dev/, tests/ and ws-tests/ compose files use ADMIN_EMAILS=admin,${PLATFORM_ADMIN_EMAILS}, so dropping them would revoke admin rights on upgrade. A startup warning lists such entries. Deployments that want the fail-closed behaviour opt in with ADMIN_EMAILS_STRICT=true. (An intermediate commit on this branch dropped them by default behind ADMIN_EMAILS_ALLOW_LOGIN_ID; that flag is gone, see the review thread.)

Tests cover whitespace, case-insensitive env and lookup, kept-with-warn for no-@ entries (including admin,…), the strict opt-in (only the exact value true; false/TRUE/1/empty keep entries), no warning for clean values, empty/unset env, empty entries in every position ('', ,admin, admin,, admin,,ops@example.com) and multi-entry. Each test clears and restores ADMIN_EMAILS/ADMIN_EMAILS_STRICT, so none depends on the outer environment.

3. chore(authProviders): drop cookieHeaderLen from OIDC diagnostics

Rationale: hasCookieHeader (boolean) is sufficient to diagnose missing session-cookie issues; the cookieHeaderLen byte count adds no operational value and is one more PII-adjacent surface. Removed from both diagnostic paths in pods/authProviders/src/openid.ts.

4. auth(oidc): gate verbose callback diagnostics behind OIDC_DEBUG

Every failed OIDC callback logged the full diagnostic block (errStack, sessionKeys, host, …) at error level, so repeated invalid callbacks from anonymous clients could flood the log. Normal operation now logs a single short error line; the full block only with OIDC_DEBUG=true.

5. auth(oidc): redirect to /login when provider auth yields no account

handleProviderAuth returns '' when the login cannot be resolved (e.g. no account and sign-up disabled). Previously no redirect happened and next() ran into nothing, leaving a hanging/empty response. Now fail-closed: explicit 302 /login?error=oidc_no_account.

6. auth(oidc): retry strategy registration with capped backoff

The account service registered the OIDC passport strategy once at startup via a single fire-and-forget Issuer.discover(). A transient issuer outage at that moment left the oidc strategy permanently unregistered (500 "Unknown authentication strategy" on every OpenID login until a restart; observed for ~10 days).

• registerOidcStrategyWithRetry() retries the whole chain (discover → Client → Strategy → passport.use) with capped exponential backoff (5 s, ×2, cap 60 s); discover and sleep are injectable for tests. It never rejects.
• Warn gating: first 5 attempts and every 60th thereafter; error stacks only under OIDC_DEBUG.
• Routes stay registered synchronously; while registration is pending, /auth/openid answers 503 with Retry-After: 5.
• First tests for the package: the retry helper in isolation and the registerOpenid wiring.

Out of scope

The OIDC name-split heuristic for IdPs that violate OIDC §5.1 (given_name = full_name) ships separately in #10919 — Authentik will fix this upstream in v2026.8 (PR #21544), so the Huly-side patch remains defense-in-depth.

DCO

All commits Signed-off-by Michael Uray. maintainer_can_modify=true.

[huly-github-staging]

Connected to Huly®: UBERF-16534

[ArtyomSavchenko]

I think it can affect existing instances where admin email is specified without valid email. Perhaps some configurations can use something like ADMIN_EMAILS=admin

[MichaelUray]

Correct — the repo's own compose files ship ADMIN_EMAILS=admin,${PLATFORM_ADMIN_EMAILS} (dev/, tests/, ws-tests/) and tests/prepare-pg.sh creates that admin account, so the fail-closed default would have revoked admin on upgrade.

Changed in a02b7f6: entries without @ are kept by default again (original semantics, now trimmed + lowercased), and a startup warning lists them. Dropping them is an explicit opt-in via ADMIN_EMAILS_STRICT=true. The ADMIN_EMAILS_ALLOW_LOGIN_ID flag from the earlier commit is removed, so there is a single switch. Tests cover both modes, including ADMIN_EMAILS=admin,… staying admin.

One deliberate difference from the original code remains: empty entries are dropped. The old split(',') put '' into the set for ADMIN_EMAILS='', ,admin, admin, or admin,,x, which made a provider login without an email an admin. isAdminEmail('') is now always false; this is covered by tests and described in the PR body.