Skip to content

Phase 2a: embedded OIDC provider (authorize/token/userinfo/jwks/discovery)#148

Open
ClaydeCode wants to merge 8 commits into
feature/oidc-phase1-usersfrom
feature/oidc-phase2a-provider
Open

Phase 2a: embedded OIDC provider (authorize/token/userinfo/jwks/discovery)#148
ClaydeCode wants to merge 8 commits into
feature/oidc-phase1-usersfrom
feature/oidc-phase2a-provider

Conversation

@ClaydeCode

@ClaydeCode ClaydeCode commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

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

  • Built on Authlib's framework-agnostic server classes (>=1.7.2,<1.8 — 1.8 makes the grant param mandatory; pinned rather than chased tonight).
  • Session = the existing terminal cookie. /authorize resolves the authorization JWT → terminal → users row (phase 1). The sub claim 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 an oidc_rd return URL (login page lands in phase 3).
  • Authlib's core is sync: request handling runs in asyncio.to_thread, storage hooks bridge back to the main loop's psycopg pool via run_coroutine_threadsafe — one pool, no sidestep (unlike the spike's sync-psycopg shortcut).
  • Multi-user-ready: the provider only knows users rows; when members arrive (phase 4) nothing here changes except the grant check in /authorize.

Hardening vs. the spike (full list from the PoC verdict)

  • Access/refresh tokens and authorization codes stored as SHA-256 digests — a DB read can no longer replay sessions. Client secrets stay plaintext deliberately: compose templates re-render them at every startup, and they sit in the app's compose env on the same disk anyway.
  • Scope stored from request.scope (client-allowed), not raw request — scope-escalation regression covered by test.
  • Null claims stripped from id_tokens (strict clients like Immich's openid-client v6 reject nonce: null).
  • Both client_secret_basic and client_secret_post accepted (Immich sends _post).
  • Token endpoint rate-limited (in-process, 30/min) → 429.
  • RS256 key generated once, persisted in kv_store.
  • ENV FORWARDED_ALLOW_IPS=* in the Dockerfile: fastapi run already enables uvicorn's proxy headers, but forwarded headers from the in-network Traefik must be trusted or Authlib sees http:// 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

  • Client registration at app install + {{ oidc.* }} template vars (phase 2b, next PR)
  • Login page / credentials (phase 3), members + grants (phase 4)
  • Cleanup job for expired codes/tokens (tables stay tiny at household scale; can ride along with phase 3)

Recommended reading order

  1. migrations/shard-core-0003-oidc.sql — schema
  2. shard_core/database/oidc.py — storage (conn-first, hashed lookups)
  3. shard_core/service/oidc_provider.py — Authlib models, grants, key mgmt, sync→async bridge
  4. shard_core/web/public/oidc.py — FastAPI adapter, lazy init, session resolution, rate limit
  5. shard_core/web/public/__init__.py, Dockerfile, pyproject.toml — wiring
  6. tests/test_oidc.py

🤖 Generated with Claude Code

Test status

Full suite: 190 passed, 1 failedtest_app_lifecycle.py::test_app_starts_and_stops, which fails identically on clean main on this machine (pre-existing, container-start timing), not a regression from this branch.


Rebased on the reworked phase-1 branch: user_sub columns are BIGINT FKs, ShardUser carries the numeric id.

…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
ClaydeCode force-pushed the feature/oidc-phase2a-provider branch from 0d462e4 to 6bd962a Compare July 11, 2026 10:58
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ClaydeCode
ClaydeCode force-pushed the feature/oidc-phase2a-provider branch from 6bd962a to 7d3a362 Compare July 11, 2026 18:11
ClaydeCode and others added 5 commits July 11, 2026 18:24
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
ClaydeCode force-pushed the feature/oidc-phase2a-provider branch from 7d3a362 to be5293b Compare July 11, 2026 18:43
@ClaydeCode

Copy link
Copy Markdown
Contributor Author

Security-hardening round from the OIDC review (RFC 9700), commit be5293b:

  • PKCE: S256 only. Authlib's default also accepted plain (challenge = cleartext verifier — no interception protection). Dropped from validation and discovery.
  • Authorization codes: atomic single-use. Redemption is now one UPDATE … RETURNING on a redeemed flag — the SELECT-then-DELETE TOCTOU where two concurrent /token calls could both redeem a code is gone. Any redemption attempt burns the code (even one that fails PKCE), and reuse of a redeemed code revokes every token of that (client, user) grant. Redeemed rows persist until expiry, which also makes the nonce-replay window real.
  • Refresh-token reuse detection. Replaying a rotated-out refresh token now revokes the whole token family — previously the thief who rotated first kept a live token while the legit client got a silent invalid_grant.

Deferred (low, noted for later): per-client/IP rate-limit buckets — the coarse global 30/min guard stays.

Tests: 20/20 in tests/test_oidc.py, incl. new coverage for plain-PKCE rejection, burn-on-failed-redemption, code-reuse revocation, and family revocation on refresh replay.

🤖 Generated with Claude Code

@max-tet
max-tet force-pushed the feature/oidc-phase1-users branch from 703fa17 to 7d761f7 Compare July 26, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant