feat(auth): add generic OpenID Connect (OIDC) single sign-on by holden093 · Pull Request #5752 · odysseus-dev/odysseus

holden093 · 2026-07-25T16:37:12Z

Note: Re-opening of #3508, auto-closed 2026-07-23 during the odysseus-dev org transfer (the dev history rewrite force-closed all open PRs). The full 24-commit OIDC feature has been cleanly rebased onto the current dev — all 162 OIDC tests pass against it, with no regressions to existing auth/session tests. Original review thread (RaresKeY, multi-model security reviews, etc.): #3508.

Summary

Adds generic OpenID Connect (OIDC) single sign-on as an alternative login
method. Users can sign in via any standard OIDC provider (Authentik,
Keycloak, Authelia, Google, etc.) while existing password authentication
remains fully functional. New OIDC users are auto-created on first login,
and admin status can be driven by IdP group membership.

Uses authlib for provider discovery (.well-known), JWT/JWKS
signature verification, and the authorization code flow — avoiding the
security risk of hand-rolling OIDC logic.

Built with AI assistance (see the Co-Authored-By trailer).

Target branch

This PR targets dev, not main.

Linked Issue

Closes #806

Type of Change

Bug fix (non-breaking — fixes a confirmed issue)
New feature (non-breaking — adds new behaviour)
Breaking change (changes or removes existing behaviour)
Refactor / cleanup (behaviour unchanged)
Documentation only
CI / tooling / configuration

Checklist

I searched open issues and open PRs — this is not a duplicate.
This PR targets dev
My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
I actually ran the app and verified the change works end-to-end (tested with a live Authentik instance).
I added new dependencies to requirements.txt and wired corresponding env vars through docker-compose.yml.

How to Test

Run the unit tests:
```
python -m pytest tests/test_oidc_auth.py tests/test_oidc_manager.py tests/test_oidc_routes.py -v
```
85 tests covering OidcManager (mocked HTTP + real RSA JWT signing), OIDC route handlers (login redirect, callback success/failure, error paths), AuthManager OIDC methods (user creation, lookup, idempotency, username collision, password rejection), and admin group mapping (promotion, demotion, no-op).

Verify existing auth tests pass:

python -m pytest tests/test_auth_regressions.py tests/test_auth_session_revocation.py tests/test_totp_failclosed.py tests/test_reserved_username_admin_escalation.py tests/test_gpu_compose_standalone.py -v

Verify the app starts without OIDC configured (default):

DATABASE_URL=sqlite:///:memory: python -c "
import os; os.environ['OIDC_ENABLED']='false'
from core.auth import AuthManager
from core.oidc import init_oidc_manager
assert init_oidc_manager() is None
print('OK')
"

End-to-end with a real OIDC provider:
- Set OIDC_ENABLED=true, OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET in .env
- docker compose up -d --build
- Visit http://localhost:7000/login — a "Sign in with …" button appears below the password form
- Click it, authenticate at the IdP, verify you are redirected back and logged in
- Open Settings → Account: "Change Password" and "Two-Factor Authentication" cards are hidden
- Log out, log in with password credentials: still works

Visual / UI changes

Login page — SSO button below the password form when OIDC is configured, with a divider. Styled to match the existing login form using theme variables.

Settings → Account — "Change Password" and "Two-Factor Authentication" cards hidden for OIDC users (they have no password and authenticate through the IdP).

Configuration reference

Variable	Default	Description
`OIDC_ENABLED`	`false`	Master toggle
`OIDC_ISSUER`	—	Provider issuer URL
`OIDC_CLIENT_ID`	—	Client ID
`OIDC_CLIENT_SECRET`	—	Client secret
`OIDC_SCOPES`	`openid profile email`	Requested scopes
`OIDC_ADMIN_GROUPS`	—	Comma-separated group names that grant admin

🤖 Generated with Claude Code

Adds OIDC authentication as an alternative to password login, enabling sign-in via any standard provider (Authentik, Keycloak, Authelia, etc.). New features: - Generic OIDC provider support via .well-known discovery (authlib) - Coexists with existing password auth — users choose at login - Auto-creates local users on first OIDC login - Admin group mapping: OIDC_ADMIN_GROUPS grants admin based on IdP groups - Admin status syncs on every login (follows IdP membership) - OIDC users cannot use password login or set up 2FA - UI hides change-password and 2FA cards for OIDC users New env vars: - OIDC_ENABLED, OIDC_ISSUER, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET - OIDC_SCOPES, OIDC_ADMIN_GROUPS New files: - core/oidc.py — OidcManager (discovery, auth URL, code exchange, id_token verification with JWT/JWKS) - routes/oidc_routes.py — /api/auth/oidc/{login,callback,config} Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The base docker-compose.yml gained 6 OIDC_* env vars; the standalone GPU files must mirror them or test_gpu_compose_standalone fails. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…, redirect_uri, stateless state, first-user-admin 1. aud validation: handle JSON array audiences per OIDC spec (check membership, validate azp for multi-audience tokens). 2. JWKS caching: cache keys after first fetch; refresh only on unknown kid — avoids live IdP round-trip on every login. 3. Algorithm pinning: restrict to RS256/ES256/etc from discovery doc, never allow HS256 or 'none'. 4. OIDC_REDIRECT_URI: support a fixed redirect URI (proxy-safe, doesn't trust Host header). 5. Stateless state: replace in-memory dict with Fernet-encrypted state tokens — works across multiple uvicorn workers/processes. 6. OIDC_FIRST_USER_IS_ADMIN: bootstrap first OIDC user as admin when no users exist and OIDC_ADMIN_GROUPS is unset — prevents zero-admin lockout.

…JWKS cooldown, secure cookies, dead code 1. Fix bootstrap admin demotion: skip set_oidc_user_admin when OIDC_ADMIN_GROUPS is unset so bootstrap/manual admin survives subsequent logins. 2. Login CSRF: bind state to an HttpOnly cookie set at /login and verified with constant-time compare at /callback. 3. JWKS cooldown: throttle refresh to once per 60s to prevent attacker-triggered unbounded IdP fetches via random kid values. 4. Secure cookies: derive secure flag from request scheme when SECURE_COOKIES is not explicitly set; OIDC session defaults to secure on HTTPS connections. 5. Remove dead validation-shaped code: the jwt.decode block with None key that swallowed all exceptions and discarded output.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…b protection 1. Suppress first-user bootstrap admin when OIDC_ADMIN_GROUPS is configured — a non-admin IdP user must not get admin just by being the first to log in. Group membership is the only path when groups are set. 2. Reject mismatched UserInfo sub: the UserInfo endpoint MUST NOT overwrite the verified id_token subject. Also guard all verified identity claims (sub, iss, aud, exp, iat, nonce, azp) from being overwritten by UserInfo.

… to compose files Both vars were documented in .env.example and read by the application code, but never forwarded to the container by any docker-compose file. OIDC_REDIRECT_URI is essential for deployments behind a reverse proxy to avoid deriving the wrong scheme from the inbound request.

…-audience azp, OIDC 2FA guards 1. Discovery issuer mismatch now raises OidcError instead of logging a warning (OIDC Discovery §1.1 requires mismatch abort). 2. Multi-audience ID tokens without azp are now rejected (OIDC Core §2 requires azp when aud has multiple values). 3. /change-password, /2fa/setup, /2fa/confirm, and /2fa/disable now reject OIDC users with a clear message. The frontend already hides these cards, but the backend must also enforce the policy. 113 passing (76 OIDC + 37 regression), 0 failures.

…ed redirect_uri 1. JWKS fetch/parse errors now wrapped as OidcError instead of escaping as raw exceptions. Transient network failures, HTTP errors, and bad JSON responses from the JWKS endpoint all produce a controlled OidcError, which the callback route already redirects to /login?error=oidc_failed instead of a 500. 2. exchange_code() now extracts the stored redirect_uri from the Fernet-encrypted state token and rejects any callback-derived redirect_uri that differs. The token exchange POSTs the stored value — the one the IdP saw in the authorization request — removing the last callback dependence on request-derived URI behaviour. Tests: 5 new (3 JWKS error wrapping + 2 redirect_uri binding) Total: 130 passing (81 OIDC + 49 regression), 0 failures.

Wrap oidc_manager.exchange_code() in asyncio.to_thread() so slow provider I/O (token exchange, JWKS refresh, UserInfo) doesn't block the async worker's event loop. Moves import asyncio to top level. Add regression test proving a 0.3s blocking exchange completes quickly without blocking concurrent async work.

…s workers Two fixes from review feedback: 1. (security/availability) Record _last_jwks_refresh timestamp BEFORE calling _refresh_jwks() so a failed JWKS fetch is also throttled by the 60-second cooldown. Previously the timestamp was only set after success, so an outage or key rotation would retry the IdP on every login attempt. 2. (auth) Use the shared persistent app key from secret_storage (_get_fernet) for OIDC state encryption instead of reading data/.app_key directly and falling back to a per-process key. This guarantees all uvicorn workers share the same Fernet key, even on a fresh data directory — worker A's state is always decryptable by worker B on the callback. Add regression tests for both fixes.

…nst silent demotion Three fixes from second review pass: 1. (security) Serialize the first-OIDC-user admin bootstrap inside _config_lock. Previously the check and username collision resolution happened outside the lock, so two concurrent first-login callbacks could both observe an empty user map and both persist as admin. Now idempotent lookup, bootstrap decision, and collision resolution are all inside one critical section. 2. (auth) Make data/.app_key creation atomic via O_EXCL open so two racing workers on a fresh deployment cannot generate different keys. The loser reads the winner's key, guaranteeing every worker shares the same Fernet key for OIDC state encryption. 3. (auth) Track whether UserInfo was successfully fetched (_userinfo_available flag in claims). The callback now skips admin group sync for existing users when UserInfo is unavailable AND the id_token lacks a groups claim — a transient provider failure no longer silently demotes existing OIDC admins. When UserInfo succeeds or the id_token carries groups, admin status syncs as before. Regression tests: concurrent admin bootstrap, key-creation race, UserInfo-unavailable preserves admin, UserInfo-available demotes, id_token groups authoritative without UserInfo.

…omic key creation - Add fcntl.flock inter-process file lock shared by setup() and create_user_oidc() so multi-worker first-admin bootstrap is serialised across processes, not just threads within one worker. Both methods reload auth.json inside the lock so the loser sees the winner's write. - _fetch_userinfo() now returns None (not {}) when discovery has no userinfo_endpoint, and exchange_code() only sets _userinfo_available=True when a live endpoint was reached. Prevents the callback from treating 'no endpoint' as authoritative group non-membership evidence. - Rewrite _load_or_create_key() to write the Fernet key to a temp file, fsync, then atomically os.link() into place. No reader ever sees the final path before the complete key bytes are available — a racing worker either sees no file or a complete one, never an empty/partial file. 105 tests pass (97 existing + 8 new regressions covering the three fixes). Co-Authored-By: Kevin <holden093@users.noreply.github.com>

fcntl.flock serialises across processes (uvicorn workers) but does NOT block threads within the same process — two threads calling flock(LOCK_EX) on the same file both succeed immediately. This caused a rare race in concurrent first-admin bootstrap where both create_user_oidc() and setup() could enter the critical section simultaneously, resulting in a lost write and zero admins. Add a module-level threading.Lock acquired before the file lock so the critical section is serialised across both threads and workers. Fixes the flaky test: TestInterprocessFirstAdminSerialisation::test_two_managers_single_first_admin ("Expected exactly 1 admin after concurrent bootstrap; found 0")

1. Serialize set_oidc_user_admin() across workers via _interprocess_auth_lock() with reload-recheck-save, preventing stale in-memory snapshots from overwriting users concurrently created by another worker. 2. Accept single-element aud arrays without azp (OIDC Core §2 only requires azp for multi-audience tokens). Normalize aud to a list and only enforce azp when len > 1. 3. Add threading.Lock around _get_fernet() key creation to prevent two threads in the same process from racing on the shared temp-file path and destroying each other's work. 4. Guard import fcntl with try/except so AuthManager imports on native Windows. _interprocess_auth_lock degrades to intra-process-only when fcntl is unavailable. Tests: +4 regression tests (108 total, 0 failures).

create_user() (password-user creation) now takes _interprocess_auth_lock with reload-before-save, preventing a concurrent set_oidc_user_admin() from losing a newly-created password user. Introduces _create_user_locked() internal helper so setup() (which already holds the inter-process lock) can create users without a nested fcntl.flock deadlock. Also changed _auth_intraprocess_lock to threading.RLock to support safe nesting patterns across mutation methods. Regression: test_create_user_survives_concurrent_set_oidc_user_admin (109 total, 0 failures).

The reserved-username guard was only in create_user(), but setup() now calls _create_user_locked() directly (to avoid nested flock deadlock). Move the check into the shared helper so it applies regardless of which path creates the user. Fixes CI failure in test_setup_rejects_reserved_admin_username.

All auth.json mutation methods now acquire _interprocess_auth_lock (flock-based) + _config_lock with reload-before-save, matching the pattern already used by create_user / create_user_oidc / set_oidc_user_admin. This prevents stale-write clobbers when an OIDC callback races a concurrent set_privileges, delete_user, rename_user, set_admin, change_password, or TOTP mutation from another uvicorn worker. Also: - Normalize setup() username (strip + lower) to match every other user-creation path, preventing "Alice" vs "alice" lockout. - Normalize in _create_user_locked() as defense-in-depth. - Remove dead _setup_lock — its serialisation was subsumed by _interprocess_auth_lock + _config_lock. - Move pre-lock validation inside the critical section for change_password, totp_generate_secret, and totp_confirm_enable. - Re-read backup codes from disk inside the lock in totp_verify() to prevent dual-consumption across workers. Co-Authored-By: antigravity <agy@antigravity>

Fixes 11 security/robustness issues identified across a 4-model review (deepseek, gpt-5.5, claude-fable-5, gpt-5.6-sol) of the OIDC SSO implementation: UserInfo & claims integrity: - Require non-empty matching sub before trusting UserInfo (P2) - Reject non-dict/malformed UserInfo responses as unavailable - Validate NumericDate strictly: reject bool, NaN, Inf - Add iat future-token verification (60s tolerance) - Initialize userinfo safely before try block Callback hardening: - Clear CSRF cookie on ALL 8 failure branches + 503 unconfigured - Echo state parameter on IdP error redirects (OIDC Core §3.1.2.6) - Check create_session_trusted() return value before setting cookie Defense-in-depth: - OIDC_MAX_AGE env var with auth_time verification (+60s skew) - AuthManager.check_oidc_totp() with disk-reloaded TOTP enforcement - SameSite=Lax proxy documentation in .env.example UI fix: - Preserve OIDC callback error after setMode() initialization Tests: 164 passed (123 OIDC + 41 regression), +21 new regression tests

13 additional hardening fixes based on gpt-5.6-sol final judgment: Spec compliance & validation: - Validate azp == client_id whenever present (not just multi-audience) - Enforce HTTPS on token/JWKS/UserInfo endpoints - Validate IdP claim types before use (sub must be non-empty string) - Enforce 'openid' in OIDC_SCOPES - Validate state payload shape after decryption Hardening: - Last-admin guard on OIDC group demotion (refuse to demote sole admin) - Cookie Secure: trust X-Forwarded-Proto only when TRUST_PROXY_HEADERS=true - OIDC_MAX_AGE parse error now caught during init (no app crash) - Nonce comparison uses secrets.compare_digest (constant-time) - Algorithm validated against allow-list before jwt.decode (not after) - JWKS cooldown marker now lock-protected against concurrent threads Operational: - Config endpoint returns generic error, logs details server-side - Pin authlib>=1.3.0,<2 in requirements.txt Tests: 164 passed (123 OIDC + 41 regression), 0 failures

Verify set_oidc_user_admin() refuses to demote the sole admin when an IdP group change would otherwise lock out all admin access.

…ssions Addresses RaresKeY's review (5 findings) and follow-up Basic RP validation comment on PR odysseus-dev#3508: - Add PKCE (RFC 7636, S256): code_challenge in the authorization request, verifier carried in the Fernet-encrypted state, and code_verifier sent to the token endpoint. - Require the iat claim in id_tokens (OIDC Core §2); tokens without iat are now rejected. - Prefer client_secret_basic at the token endpoint per discovery (OIDC default), falling back to client_secret_post only when the provider excludes basic. - Require HTTPS for the issuer and authorization endpoint, not just the back-channel endpoints. - Preserve OIDC subs exactly (no strip) so distinct whitespace-bearing subjects can never collapse into one local account; same for the UserInfo sub-binding comparison. - Sync admin state only on a well-formed groups claim; UserInfo availability alone (or a malformed groups value) no longer demotes an existing admin. - OIDC session/CSRF cookies are Secure by default regardless of SECURE_COOKIES; explicit OIDC_ALLOW_INSECURE_COOKIES=true is the only (documented, dev-only) opt-out. - Make sessions issued by one uvicorn worker validate on others via an mtime-gated read-through reload of sessions.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GRiLb12nnLnBnYsg14oSWd

… cookie policy Follow-up hardening beyond the explicit review findings: - Propagate session revocation across uvicorn workers: token validation now syncs issuance AND revocation from sessions.json (mtime-gated), _save_sessions merges on-disk state under an inter-process flock so concurrent workers can't lose each other's sessions, and revocation tombstones prevent a just-revoked token from being re-merged. - Restrict sessions.json and auth.json to 0600 (bearer tokens and password hashes; same policy as data/app.db, odysseus-dev#4420), applied atomically at write time and retroactively at load. - Password-login session cookie: SECURE_COOKIES=false can no longer downgrade the cookie when the request arrived over HTTPS (spoofable X-Forwarded-Proto still requires TRUST_PROXY_HEADERS opt-in). - Document why OIDC state tokens are deliberately not single-use and which mechanisms bound the replay window. - Warn once per process (not twice per login) when OIDC_ALLOW_INSECURE_COOKIES is enabled; pass the variable through the Compose files so the documented dev override actually reaches containers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GRiLb12nnLnBnYsg14oSWd

holden093 · 2026-07-25T16:50:17Z

Heads-up: the failing `Python tests (pytest)` check is a pre-existing `dev` bug, not this PR

The pytest job fails at test collection, before any of this PR's changes are exercised:

NameError: name 'Any' is not defined

Root cause is on dev itself: src/agent_loop.py annotates with dict[str, Any] (lines 1503 and 1877) but only imports AsyncGenerator, List, Dict, Optional, Set — no Any, and no from __future__ import annotations. So importing the module raises NameError, which breaks collection for every test that transitively imports agent_loop (hence the repo-wide failure).

This PR doesn't touch agent_loop.py; every other check (syntax, CodeQL, lint, security scans) passes here, and this PR's own tests pass locally.

One-line fix (add Any to the typing import) is included in #5751. Once dev carries that fix, this PR's pytest will go green with no changes needed here.

holden093 and others added 24 commits July 25, 2026 18:32

fix: add OIDC env vars to standalone GPU compose files

d1d2b67

The base docker-compose.yml gained 6 OIDC_* env vars; the standalone GPU files must mirror them or test_gpu_compose_standalone fails. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Potential fix for pull request finding

879a1fe

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

fix(oidc): surface callback errors on login page from URL query param

e1e82d5

test(oidc): add regression test for last-admin demotion guard

bcef54a

Verify set_oidc_user_admin() refuses to demote the sole admin when an IdP group change would otherwise lock out all admin access.

github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(auth): add generic OpenID Connect (OIDC) single sign-on#5752

feat(auth): add generic OpenID Connect (OIDC) single sign-on#5752
holden093 wants to merge 24 commits into
odysseus-dev:devfrom
holden093:feat/oidc-sso-reopen

holden093 commented Jul 25, 2026

Uh oh!

holden093 commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

holden093 commented Jul 25, 2026

Summary

Target branch

Linked Issue

Type of Change

Checklist

How to Test

Visual / UI changes

Configuration reference

Uh oh!

holden093 commented Jul 25, 2026

Heads-up: the failing Python tests (pytest) check is a pre-existing dev bug, not this PR

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Heads-up: the failing `Python tests (pytest)` check is a pre-existing `dev` bug, not this PR