Skip to content

Commit cfc6283

Browse files
authored
Merge pull request #129 from EqualifyEverything/feat/optional-auth-layer-pr2-oidc
feat(auth): generic OIDC + Microsoft Entra preset (PR2)
2 parents dd4d0b1 + 2a84e15 commit cfc6283

15 files changed

Lines changed: 1590 additions & 45 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ tests/
9696
- [Add a new S3 operation](docs/how-to/add-s3-operations.md)
9797
- [Test rate limits locally](docs/how-to/test-rate-limits.md)
9898
- [Enable basic auth on the viewer](docs/how-to/enable-basic-auth.md)
99+
- [Configure SSO (Microsoft Entra and other OIDC providers)](docs/how-to/configure-sso.md)
99100

100101
## Conventions
101102

clients/viewer/src/auth/LoginPage.tsx

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,6 @@ export function LoginPage() {
3333
}
3434
}, [config, navigate])
3535

36-
// OIDC with a single provider → auto-redirect to its login_url.
37-
useEffect(() => {
38-
if (config && config.mode === 'oidc' && config.providers.length === 1) {
39-
const url = new URL(config.providers[0].login_url, window.location.origin)
40-
url.searchParams.set('next', next)
41-
window.location.href = url.toString()
42-
}
43-
}, [config, next])
44-
4536
const handleSubmit = async (event: FormEvent<HTMLFormElement>) => {
4637
event.preventDefault()
4738
if (submitting) return
@@ -80,12 +71,7 @@ export function LoginPage() {
8071
<main className="flex min-h-screen items-center justify-center bg-background px-4">
8172
<section className="w-full max-w-sm space-y-6 rounded-lg border border-border bg-card p-8 shadow-lg">
8273
<BrandHeader />
83-
<header>
84-
<h1 className="text-2xl font-bold">Sign in</h1>
85-
<p className="mt-1 text-sm text-muted-foreground">
86-
Choose a sign-in method to continue.
87-
</p>
88-
</header>
74+
<h1 className="sr-only">Sign in to Equalify Reflow</h1>
8975
<ul className="space-y-3">
9076
{config.providers.map((provider) => {
9177
const url = new URL(provider.login_url, window.location.origin)
@@ -114,12 +100,7 @@ export function LoginPage() {
114100
<main className="flex min-h-screen items-center justify-center bg-background px-4">
115101
<section className="w-full max-w-sm space-y-6 rounded-lg border border-border bg-card p-8 shadow-lg">
116102
<BrandHeader />
117-
<header>
118-
<h1 className="text-2xl font-bold">Sign in</h1>
119-
<p className="mt-1 text-sm text-muted-foreground">
120-
Enter your username and password to continue.
121-
</p>
122-
</header>
103+
<h1 className="sr-only">Sign in to Equalify Reflow</h1>
123104
<form className="space-y-4" onSubmit={handleSubmit} noValidate>
124105
<label className="block space-y-1">
125106
<span className="text-sm font-medium">Username</span>

docs/explanation/authentication-design.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,64 @@ SSO via OIDC redirects out to the IdP and back. The browser's first request afte
7474
Self-signup implies a user database, an email-verification flow (or an open-by-default surface), and an admin UI for moderation. That's an entire product surface that doesn't belong in an OSS tool deployed by small operators who want a *locked-down* viewer. Operator-provisioned users via env keep the threat model the same as API keys: the people with access are the people the operator deliberately gave access to.
7575

7676
For deployments that need self-signup, OIDC is the right answer — federate to an IdP that already handles signup, password reset, and account hygiene.
77+
78+
## How the OIDC redirect flow holds together
79+
80+
The browser drives a chain of redirects with state preserved across hops in two short-lived cookies. The high-level picture:
81+
82+
```
83+
User → SPA GET /login (anonymous; AuthProvider knows mode=oidc)
84+
SPA → Backend GET /api/v1/auth/login/entra (kickoff route)
85+
← 302 to IdP authz URL
86+
+ Set-Cookie: reflow_oauth_tx (signed: state, nonce, verifier, next_path)
87+
User → IdP (sign-in, MFA, consent — entirely on the IdP)
88+
IdP → User 302 to /api/v1/auth/callback/entra?code=…&state=…
89+
User → Backend GET /api/v1/auth/callback/entra (callback route)
90+
Reads reflow_oauth_tx, validates state matches.
91+
POSTs token endpoint with code + verifier.
92+
Validates id_token: signature (JWKS), iss, aud, exp, nonce.
93+
← 302 to next_path
94+
+ Set-Cookie: reflow_session (signed identity)
95+
+ Set-Cookie: reflow_session_csrf
96+
+ clears reflow_oauth_tx
97+
```
98+
99+
Several decisions are worth understanding because they're not obvious from reading the code alone.
100+
101+
### Why two cookies, not one
102+
103+
The OAuth `state` parameter is the canonical CSRF defence for the redirect-back. To validate it, we need to know what state we minted on kickoff — that's the kind of thing many implementations stash in a server-side session. Our session store doesn't exist yet at that point in the flow (the user is still anonymous), so a separate, short-lived `reflow_oauth_tx` cookie carries it instead. Same cookie also carries the PKCE verifier and the original `next` path so the callback finishes the round-trip without needing any other state.
104+
105+
The `reflow_oauth_tx` cookie is signed with a different `itsdangerous` salt from the session cookie, so a session value can never be replayed as a tx value or vice versa. TTL is 10 minutes — long enough that a user pausing on an MFA prompt or a password reset still completes the flow, short enough that a captured tx cookie can't be replayed against a future kickoff.
106+
107+
### Why PKCE even with a confidential client
108+
109+
Entra (and most enterprise IdPs) treat us as a "confidential client" because we have a `client_secret`. The OAuth 2.0 spec says PKCE is optional for confidential clients. We do it anyway because:
110+
111+
1. Defence in depth — if the `client_secret` ever leaks (CI logs, accidental commit, env-dump page), an attacker who also intercepts a single authorisation code can't redeem it without the verifier we never put on the wire.
112+
2. Future-proofing — if we ever break the deployment into a public client (e.g. a native desktop variant), nothing about the OIDC integration changes.
113+
114+
The `code_challenge` (S256 hash of a 64-char verifier) goes in the auth URL; the verifier rides home in the signed tx cookie and is sent in the token-exchange POST body.
115+
116+
### Why we validate every claim ourselves
117+
118+
`joserfc.jwt.decode` validates the JWT signature against the JWKS we fetched from the discovery doc. It does **not** validate `iss`, `aud`, `exp`, or `nonce` — those are application-level checks. We do them in `OIDCAuthProvider._validate_id_token`:
119+
120+
- `iss` must match the discovery doc's issuer. Catches "wrong tenant" misconfigurations.
121+
- `aud` must include our `client_id`. ID tokens issued for *another* client of the same IdP should not be redeemable here.
122+
- `exp` must not be in the past (with 60s leeway for clock skew).
123+
- `nonce` must match the value we minted on kickoff. Prevents replay of an ID token captured from a different login session — even from the same IdP, even within the token's `exp` window.
124+
125+
### Why JWKS rotation gets a force-refresh retry
126+
127+
IdPs rotate signing keys silently. Entra ≈ daily; some providers do it on demand. Our JWKS cache TTL (one hour) is an optimisation, not a correctness boundary — on a signature-validation failure we force-refresh JWKS once and retry. If validation still fails, the token really is bad. This avoids a class of "everything was fine yesterday and now nobody can log in" outages that would otherwise need a container restart to recover.
128+
129+
### Why the open-redirect sanitiser
130+
131+
A malicious link `https://reflow.example/login?next=https://evil.example/steal` would, without sanitisation, surface `https://evil.example/steal` as the post-login destination — turning our login flow into an unintentional open redirect. `_safe_next_path` accepts only values starting with a single `/` (no scheme, no `//`); anything else falls back to `AUTH_POST_LOGIN_REDIRECT`. Cheap defence, real value.
132+
133+
## Why no group/role gating in the first cut
134+
135+
The `Identity` model deliberately has no `groups` or `roles` field. Reading the IdP's group claim is straightforward (Entra emits group object IDs in the `groups` claim or via Graph API for large groups), but **policy** — what to do with the membership — is the messy part. "Members of group X may use the viewer" is a different policy from "members of group Y are admins" or "members of any of these N groups", and each of those wants a different config shape.
136+
137+
Shipping a half-baked policy mechanism in PR2 would be worse than shipping none. The clean extension point is there: add `groups: list[str]` to `Identity`, plus a `RequireGroups` FastAPI dependency, plus an env-driven allowlist. That's a Phase 4 ticket once the OIDC plumbing has lived in production for a while and we know which policy patterns operators actually need.

0 commit comments

Comments
 (0)