Phase 2a: embedded OIDC provider (authorize/token/userinfo/jwks/discovery)#148
Open
ClaydeCode wants to merge 8 commits into
Open
Phase 2a: embedded OIDC provider (authorize/token/userinfo/jwks/discovery)#148ClaydeCode wants to merge 8 commits into
ClaydeCode wants to merge 8 commits into
Conversation
…minal user binding Adds the users table (id doubles as the future OIDC sub) and binds every terminal session to a user. The owner user is derived from the default identity: same id (single source of truth for identity), email taken from the identity or synthesized as owner@<domain> since OIDC clients require an email-shaped identifier to auto-provision accounts. ensure_owner_user() runs on every startup after init_default_identity(): creates the owner row if missing and backfills user_id on any terminal that predates this migration. New pairings bind to the owner directly. No behavior or API change otherwise; existing sessions stay valid. Phase 1 of the multi-user identity rollout (embedded OIDC provider). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nding, User model - users.id is now BIGSERIAL: the owner user is its own entity, decoupled from the shard's identity hash id (which identifies the shard, not the person). The OIDC sub becomes the stringified numeric id in phase 2a. - role is a proper Postgres enum (user_role), mirrored by Role in the new data_model/user.py; ensure_owner_user returns a User model. - terminals.user_id is NOT NULL: the 0002 migration itself creates the owner user and binds existing terminals (existing shards have their identity at migration time; fresh shards have empty tables and get the owner at startup before any pairing). The Python-side backfill helper is gone; ensure_owner_user only creates the owner on fresh shards and backfills the synthesized email for migration-created owners. - pair.py drops the owner-missing fallback — the owner always exists once the app serves requests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ClaydeCode
force-pushed
the
feature/oidc-phase2a-provider
branch
from
July 11, 2026 10:58
0d462e4 to
6bd962a
Compare
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ClaydeCode
force-pushed
the
feature/oidc-phase2a-provider
branch
from
July 11, 2026 18:11
6bd962a to
7d3a362
Compare
The legacy-JSON migration inserted terminals without user_id, violating the new NOT NULL constraint (caught by CI). The owner user is created from the just-migrated default identity, mirroring the 0002 backfill. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… passed spike Five endpoints under /public/oidc (discovery, authorize, token, userinfo, jwks) on Authlib's framework-agnostic server classes: authorization-code + PKCE + refresh rotation, RS256 id_tokens signed with a key persisted in kv_store. First-party app clients only — no dynamic registration, no consent UI, no third-party exposure. The session is the existing terminal cookie: /authorize resolves it to the terminal's user (phase-1 users table), so every paired browser signs into an app with zero clicks. Anonymous browsers are redirected to the terminal UI with an oidc_rd return URL until the phase-3 login page exists. Authlib's core is synchronous; endpoints run it in asyncio.to_thread and the storage hooks bridge back to the main loop's psycopg pool via run_coroutine_threadsafe — single pool, unlike the spike's sync-psycopg sidestep. Hardening over the spike (per PoC verdict findings): access/refresh tokens and authorization codes stored as SHA-256 digests; scope saved from request.scope (client-allowed) not the raw request; null claims stripped from id_tokens (strict clients reject nonce:null); client_secret_basic AND _post accepted (Immich uses _post); token endpoint rate-limited in-process; authlib pinned <1.8 (grant param becomes mandatory there); FORWARDED_ALLOW_IPS=* so uvicorn trusts Traefik's X-Forwarded-Proto — Authlib refuses flows it sees as plain http. Client secrets deliberately stay plaintext: compose templates re-render them at every startup and they live in app compose envs on the same disk anyway. Phase 2a of the multi-user identity rollout; client registration at app install follows in phase 2b. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
user_sub columns are BIGINT FKs; ShardUser carries the numeric id and the OIDC sub claim is its stringified form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s now) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mic code redemption, reuse family revocation Applies the three findings from the 2026-07-11 security review (RFC 9700): - PKCE accepts S256 only. Authlib's default also allows 'plain', where the challenge travels as the cleartext verifier — useless against the authorization-request interception PKCE exists for. Discovery no longer advertises plain. - Authorization codes are consumed atomically (single UPDATE ... RETURNING on a redeemed flag), closing the TOCTOU race where two concurrent /token requests could both redeem one code. Any redemption attempt burns the code, including ones that later fail PKCE. Redeemed rows are kept until expiry: reuse of a redeemed code signals interception and revokes every token issued to that (client, user) grant. Side effect: the nonce-replay window now actually spans the code lifetime. - Replay of a rotated-out refresh token revokes the whole token family, so a thief who rotates first no longer keeps a live token while the legit client is silently rejected. Per-client rate-limit buckets (finding 4) deferred — the global in-process guard stays; noted as follow-up when it matters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ClaydeCode
force-pushed
the
feature/oidc-phase2a-provider
branch
from
July 11, 2026 18:43
7d3a362 to
be5293b
Compare
Contributor
Author
|
Security-hardening round from the OIDC review (RFC 9700), commit
Deferred (low, noted for later): per-client/IP rate-limit buckets — the coarse global 30/min guard stays. Tests: 20/20 in 🤖 Generated with Claude Code |
max-tet
force-pushed
the
feature/oidc-phase1-users
branch
from
July 26, 2026 20:36
703fa17 to
7d761f7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2a of the multi-user identity rollout: the embedded OIDC provider, productionized from the passed PoC (spike
spike/oidc-provider-poc, verdict 2026-06-12). Stacked on the phase-1 substrate PR.Five endpoints under
/public/oidc:/.well-known/openid-configuration,/authorize,/token,/userinfo,/jwks. Authorization-code + PKCE + refresh rotation, RS256 id_tokens. Deliberately not built: dynamic client registration, consent UI, third-party clients, logout channels — first-party app clients only.How it works
>=1.7.2,<1.8— 1.8 makes thegrantparam mandatory; pinned rather than chased tonight)./authorizeresolves theauthorizationJWT → terminal →usersrow (phase 1). Thesubclaim is the stringified numeric user id. A paired browser signs into an app with zero clicks; an anonymous browser is 302'd to the terminal UI with anoidc_rdreturn URL (login page lands in phase 3).asyncio.to_thread, storage hooks bridge back to the main loop's psycopg pool viarun_coroutine_threadsafe— one pool, no sidestep (unlike the spike's sync-psycopg shortcut).usersrows; when members arrive (phase 4) nothing here changes except the grant check in/authorize.Hardening vs. the spike (full list from the PoC verdict)
request.scope(client-allowed), not raw request — scope-escalation regression covered by test.nonce: null).client_secret_basicandclient_secret_postaccepted (Immich sends_post).kv_store.ENV FORWARDED_ALLOW_IPS=*in the Dockerfile:fastapi runalready enables uvicorn's proxy headers, but forwarded headers from the in-network Traefik must be trusted or Authlib seeshttp://and refuses flows (InsecureTransportError, hit in the PoC).Tests
17 integration tests against the real app + Postgres: discovery, JWKS stability, confidential + public (PKCE-only) flows, id_token claim correctness incl. nonce-absence, userinfo, refresh rotation (old access revoked, rotated refresh unusable), open-redirect rejection, wrong secret / unknown / expired / reused codes, anonymous redirect, scope narrowing, hashed-at-rest assertion, rate limit.
Not in this PR
{{ oidc.* }}template vars (phase 2b, next PR)Recommended reading order
migrations/shard-core-0003-oidc.sql— schemashard_core/database/oidc.py— storage (conn-first, hashed lookups)shard_core/service/oidc_provider.py— Authlib models, grants, key mgmt, sync→async bridgeshard_core/web/public/oidc.py— FastAPI adapter, lazy init, session resolution, rate limitshard_core/web/public/__init__.py,Dockerfile,pyproject.toml— wiringtests/test_oidc.py🤖 Generated with Claude Code
Test status
Full suite: 190 passed, 1 failed —
test_app_lifecycle.py::test_app_starts_and_stops, which fails identically on cleanmainon this machine (pre-existing, container-start timing), not a regression from this branch.Rebased on the reworked phase-1 branch:
user_subcolumns are BIGINT FKs,ShardUsercarries the numeric id.