OIDC provider#1
Merged
Merged
Conversation
Adds discovery, authorize, token, userinfo, and end-session endpoints
so off-the-shelf OIDC libraries (next-auth, passport-openidconnect,
Spring Security, etc.) can authenticate against ManyRows by pointing
at the per-app issuer URL — no ManyRows SDK required. Coexists with
the existing AppKit SDK path; existing customers unaffected.
- Per-app issuer at {AuthDomain or BASE_URL}/x/{slug}/apps/{appId}
- PKCE required (S256 only; 43-char code_challenge length validated;
43-128 char code_verifier enforced per RFC 7636 §4.1)
- Exact-match redirect_uri + post_logout_redirect_uri allowlists
- Atomic code consume; replay-revoke per OIDC §3.1.3.2
- Cross-check session.user_id == code.user_id at /token
- access_token iss = host-only (SDK-compatible); id_token iss =
per-app path (matches discovery, spec MUST)
- Confidential + public client modes; constant-time secret compare
- email_verified always present alongside email (spec §5.1)
- /oidc/login and authorize error pages have X-Frame-Options + CSP
frame-ancestors to defend credential entry against clickjacking
- OIDC requires cookie transport mode (validated at admin config
time + defense-in-depth check at /authorize runtime)
- end-session accepts both GET and POST per OIDC Session Mgmt §5
Schema (00002_oidc_provider.sql, additive):
- oidc_auth_codes: short-lived authorization-code grants
- oidc_pending_authorize: carries the /authorize request across
the AppKit-sign-in round trip
- four columns on apps: oidc_enabled, oidc_client_secret_hash,
oidc_redirect_uris, oidc_post_logout_redirect_uris
Janitor sweeps both new tables; codes and pending rows linger past
use for a 1-hour grace window so replay detection sees them.
21 tests covering discovery, authorize parameter validation, both
grant types, PKCE mismatch, code reuse + session revoke, userinfo
bearer validation, end-session (GET + POST + allowlist), transport-
mode validation (admin + runtime), anti-clickjacking headers,
malformed code_challenge rejection, and a full RP end-to-end test
that drives discovery -> JWKS fetch -> authorize -> token ->
id_token signature verification against the JWKS.
Phase 2 — makes the OIDC provider actually configurable from the
admin UI. Without this, enabling OIDC on an app required raw SQL.
Admin API (PUT/GET /admin/.../apps/{id}/oidc-config):
- Server-generated client_secret with shown-once response shape
(matches the api_keys "generate -> show once -> hash" pattern)
- regenerateSecret + clearSecret are mutually exclusive
- Enabling with no redirect_uris rejected (error.oidcRedirectUrisRequired)
- Enabling on local-transport-mode app rejected via the existing
ErrOIDCRequiresCookieTransport from the repo layer
- Tri-state slice semantics: omit field = no change, [] = clear,
[...] = replace
Admin UI (new "OIDC" tab in AppAuthMethods):
- Enable toggle with cookie-transport-mode warning when not set
- Read-only Discovery URL / Issuer URL / Client ID with copy buttons
- Generate/rotate client_secret with show-once dialog ("I've saved it")
- Clear-secret downgrade-to-public with confirmation dialog
- Redirect URIs allowlist editor (chip list + add/remove)
- Post-logout redirect URIs allowlist editor (same shape)
- All values validated as http(s) URLs before adding
6 new admin handler tests + the earlier 21 OIDC protocol tests
still all pass (27 total). UI passes tsc + vite build.
UX fix: the Regenerate/Clear client_secret buttons each issued their own targeted PUT (with only the secret flag, no URI fields), which silently bypassed any unsaved redirect-URI edits in the form. Local state then carried changes the server never saw, with no save-or- discard prompt. Both buttons are now disabled when the form is dirty, with a Tooltip pointing at the save bar. Test fix: new TestAdminOIDCConfig_DisableAfterEnable proves the ordinary "turn it off" path AND that the client_secret hash and redirect URIs survive disable — so re-enabling later does not require re-running secret distribution. 7 admin tests pass (was 6). 28 OIDC tests total, full repo green.
Adds OIDC to the "What you get" feature list and a dedicated "Integrating via OpenID Connect" section between the AppKit section and Architecture. Covers: - Where to configure (App -> Auth methods -> OIDC) - The three values an RP library needs (discovery URL, client ID, client secret) and how to obtain them - A next-auth code snippet pointing at the discovery URL - The cookie-transport-mode requirement, called out as a note - That OIDC coexists with the AppKit SDK in parallel Documentation only — no behavioural changes.
Audit pass #3 caught: the custom clearOIDCSessionCookies hardcoded Secure=true, SameSite=Lax, and omitted Domain entirely. Browsers require clear-cookie attributes to MATCH the set-cookie attributes — so for apps with cookie_domain configured, or in dev mode, or with SameSite=Strict, the browser kept the stale cookie in its jar. Server-side the session was already revoked, so this wasn't a security hole; just a correctness/UX bug where dead cookies persisted and 401'd subsequent requests until natural expiry. Fix: use the existing handler.clearSessionCookies(w, ws, app) which threads resolveCookieDomain + resolveSameSite + IsDevMode-aware Secure. Drop the custom helper. Plus a regression test: sets cookie_domain on the app, drives /oidc/end-session, asserts the Set-Cookie has the matching Domain. The old end-session test didn't check Set-Cookie attributes at all — exactly the gap that let this slip through earlier audits. 22 OIDC protocol tests + 7 admin tests pass (was 21+7).
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
1 participant
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.
No description provided.