feat(auth): run the OAuth dance on the backend (RUK-291) - #8
Merged
Conversation
The backend becomes a confidential OAuth client, so it needs the provider's client_secret, this instance's external redirect_uri and the provider's two endpoints. None carries an in-code default: an endpoint baked into the binary is one an operator cannot see when they need to know where their sign-ins are going. OAuthDanceEnabled is both-or-neither. A config naming a client_secret but no endpoints would otherwise register the routes and then send the browser to a URL with no host — a failure that arrives as a 302 and reads as success in every access log. It never aborts startup: an instance that configures no dance boots exactly as before. app.oauth_cookie_path and app.oauth_callback_path are in the gate for a sharper reason than the rest. Both are empty-is-not-inert: an empty cookie path makes net/http omit the attribute, and the browser then scopes the cookie to the internal route behind the proxy — precisely the value that never comes back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A replayed state is a signal of an attack rather than a routine error, so it gets its own audit reason instead of sharing one with an expired dance. failedLoginDetails covers the case the renderer had no wording for: a refusal that never identified anyone. It used to render as "login failed for " with a dangling preposition and an empty actor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Redeems an authorization code at the provider's token endpoint with the client secret and the PKCE verifier, and returns the id_token. Outgoing HTTP goes through xhttp with the shared sanitizer, per project convention. Endpoints come from config with no fallback, so a stand can point the dance at a local fake and an operator can see where sign-ins go. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The code the callback hands the frontend is redeemed exactly once. GETDEL is what makes that true: two racing exchanges of the same code produce exactly one winner, which a GET followed by a DEL would not. Keys are the SHA-256 of the code, so a Valkey dump never carries a live one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every sign-in path built its own audit failure event. The dance adds three more callers, and copying the construction a fourth time is how the actor and the reason drift apart between paths. signInWithVerifiedClaims is extracted for the same reason: the dance resolves a user from verified id_token claims exactly as the BFF exchange does, and that resolution is the part worth sharing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DanceStart and DanceCallback cross the API/service boundary, so they live here rather than in either side. DanceProvider is the allow-list: the route takes a path segment, and github, email, stub and bootstrap must never open a dance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…gned cookies Nothing about a dance in flight is stored server-side. The provider is sent the plaintext state and the browser holds only its HMAC signature: whoever observes the redirect URL holds one half and not the other, and that asymmetry is the binding. The verifier never travels — only its S256 challenge does. The deadline is inside the signature rather than in the cookie's MaxAge, because MaxAge is a hint a browser may ignore and an attacker replaying a captured pair with curl honours nothing at all. Ordering in verifyDanceOrigin is load-bearing twice over. The emptiness checks sit AFTER signature verification, or a stranger with curl learns which half of their attempt was wrong. And the explicit empty-signature branch looks redundant against Verify — it is not: falling through would take the audited path and let anyone knocking fill the audit trail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dance puts an authorization code and a one-time code in query strings and request bodies that the access log would otherwise record verbatim. A log line is the wrong place for either: both are redeemable, and logs outlive the seconds they are valid for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/start redirects to the provider, /callback completes the dance and hands the frontend a one-time code, /code/exchange redeems that code for a token pair. The BFF path at /login/oauth/exchange/google stays live and untouched — the two routes coexist by design. The callback answers with a redirect carrying an error code, never a JSON error: the browser arrives here as a top-level navigation from the provider, and there is a frontend page to send it to. A code rather than the token pair itself crosses the redirect, because the frontend is a separate origin in production and a redirect can only carry things in the URL — where a 720h refresh token would outlive the session in logs, history and Referer headers. Cookie attributes come from the external redirect_uri and the configured cookie path, not from the mounted route: Caddy serves this backend under `handle_path /auth/*` and strips the prefix, so a cookie scoped to what the handler sees is never sent back — and no handler test would notice, because there is no proxy in one. Secure follows that URI's SCHEME rather than the environment name. IsDev() covers dev, local and performance_test, and the deployed dev stand runs environment: dev behind Caddy on https — so an environment-keyed flag shipped the state signature and the PKCE verifier without Secure on a live HTTPS stand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Output of `make swag` and the client codegen. Kept apart from the handwritten change so the diff that needs reading stays readable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Route registration, config predicates and logging resolved configuration back at the operator kept getting written and kept getting deleted by hand. Test the output — the pure functions — and log what was done, not what was said. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Makes the backend a confidential OAuth client: it now owns the authorization-code
flow with PKCE instead of delegating it to the BFF. This is the prerequisite for
configuring Google through the admin UI, where the client_secret lives in the
backend's encrypted store and must never reach the frontend. Backend-only — the
frontend is untouched, and the existing BFF route stays live, so this ships
independently and breaks nothing.
/login/oauth/{provider}/start,/callback, and/code/exchange;/login/oauth/exchange/googlekeeps working unchangedstate, the browserholds only its HMAC signature, and the PKCE verifier never travels at all
separate origin and a redirect can only carry things in the URL; the code is redeemed
exactly once via Valkey
GETDELSecurefrom the redirect_uri's scheme rather than the environment name,which is what a deployed HTTPS dev stand needs to not leak both halves of a dance
fill the audit trail; a replayed state gets its own reason as an attack signal
make swag); no migrationsVerified: every commit builds standalone (bisect-safe),
make lintclean,./internal/...green except a pre-existing
TestConflictScopeMatrixfailure that reproduces on main.