PKCE frontend authentication#11528
Draft
nbudin wants to merge 53 commits into
Draft
Conversation
Replaces the Devise session cookie + CSRF token approach for the frontend SPA with an OpenID Connect PKCE flow using JWT bearer tokens. The Doorkeeper/doorkeeper-openid_connect backend was already in place from the jwt-backend-auth branch; this wires up the frontend side. Key changes: - Add AuthenticationManager, openid.ts, OAuthCallback to manage PKCE state, discovery, token exchange, and localStorage JWT storage - Update Apollo client to send Authorization: Bearer header when JWT present - Update SignInForm to redirect through PKCE rather than posting credentials - Update SignOutButton to call end_session_endpoint - Add /oauth/callback route in AppRouter - Expose oauth_frontend_application_uid and oidc_issuer_url via ClientConfiguration GraphQL type and app component props - Fix OAuthApplication#redirect_uri to generate URLs for both the app server port (from ActionMailer defaults) and the assets server port, covering development and production origins - Fix SessionsController to render server-side Devise sign-in form when inside an OAuth authorize flow, preserving the existing React modal flow for all other sign-in scenarios - Remove binding.pry left in skip_authorization block Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rather than embedding config (OAuth client ID, OIDC issuer URL, etc.) in HTML data attributes from the Rails app_component_props helper, the frontend now fetches ClientConfiguration directly via Apollo at startup using React's `use()` hook with a Suspense wrapper. This removes the tight coupling between server-rendered props and the React app entry point, and eliminates the need to pass Rails props through Liquid tag rendering helpers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Allow convention subdomains of INTERCODE_HOST to make CORS requests to the root site, so the OIDC discovery endpoint can be reached during sign- out. Also allow GET on /users/sign_out and switch Devise sign_out_via to :get so the end_session_endpoint browser redirect works. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
afterSessionChange was always called after signOut(), overriding the window.location.href navigation to the end_session_endpoint. Only navigate via afterSessionChange when there is no endSessionEndpoint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Override after_sign_out_path_for to return the referer URL when it comes from a trusted origin (INTERCODE_HOST, its subdomains, or a known convention domain), so users land back on the convention page after signing out from the root site. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rails 7 raises OpenRedirectError for cross-host redirects. Override respond_to_on_destroy to pass allow_other_host: true so the post-sign-out redirect back to a convention subdomain is permitted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Devise calls respond_to_on_destroy with a non_navigational_status keyword argument; match the correct signature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create a cms_devise layout that wraps the Devise sign-in form inside the site's Liquid CMS layout (with the React navbar), and switch SessionsController to use it for the OAuth authorization flow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the server-rendered Devise sign-in view with a React form component that renders inside the AppRoot React tree, allowing the CMS navigation bar to work correctly. The form still POSTs natively to /users/sign_in via Devise. Removes the now-unused cms_devise layout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ports SignUpForm and ForgotPasswordForm from the authentication modal into standalone page components (DeviseSignUpPage, DeviseForgotPasswordPage) so these flows work on the root site. Updates RegistrationsController and PasswordsController to render the React app shell at GET /users/sign_up and GET /users/password/new respectively. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces <a href> with <Link to> on the sign-in, sign-up, and forgot-password page components so navigating between them uses client-side routing instead of full-page reloads. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Nav bar sign-in/sign-up buttons now redirect straight to the root site's Devise forms (/users/sign_in, /users/sign_up) with user_return_to set to the current URL, so Devise sends the user back after authentication. - Add rootSiteHost to AppRootContext (from rootSite.host GraphQL field) - Rewrite SignInButton/SignUpButton as plain <a> links to the root site - Simplify SessionsController#new to always render the React app shell - Override SessionsController#create to allow cross-host redirect to trusted origins after sign-in (same pattern as sign-out) - Rename trusted_sign_out_origin? -> trusted_origin? since it now covers both sign-in and sign-out redirects Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously, profile creation and the needs_update redirect were handled by server-side before_actions that don't fire in the PKCE OAuth flow. Now AppRoot detects a missing or stale profile and handles it client-side via a new setupMyProfile GraphQL mutation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously these redirects happened in useEffect, causing a flash of page content before navigating. Moving the logic into the loader means the redirect happens before rendering, so the user lands directly on their final destination in one step. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace all literal strings flagged by i18n linting with t() calls. New keys added under authentication.oauthCallback, authentication.signOut, and authentication.signUp in en.json/es.json. The registrations controller alert now uses a Rails I18n key. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… level Moves the unauthenticated handler from AppRoot's useEffect to module-level in packs/application.tsx, eliminating a race condition where loaders running in parallel with AppRoot mounting could dispatch the event before the listener was installed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- All sign-in, sign-up, forgot-password, and reset-password pages (both full-page and modal variants) now display the convention name in the header when on a convention subdomain. - Created a custom Doorkeeper OAuth authorization consent page that shows the convention name and which OAuth application is requesting access. - Made recaptcha_site_key nullable in the GraphQL schema and updated the sign-up pages to skip the reCAPTCHA widget when no key is configured (fixes system test failures in environments without reCAPTCHA configured). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds `conventionByOauthReturnIfPresent` to the Query type. When the user lands on the root-domain sign-in page as part of a PKCE flow, the session's `user_return_to` OAuth URL contains a `redirect_uri` that identifies the convention subdomain. This resolver parses that to return the right convention even when the request host has no convention. AppRootQuery now fetches this as `signInConvention` and `buildAppRootContextValue` uses it as a fallback for `conventionName`, so auth page headers correctly show "Log in to ConventionName". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`session[:user_return_to]` is only written when going through the sign-in page, but the sign-up and forgot-password pages receive `user_return_to` as a query param directly. Fall back to `params[:user_return_to]` so those pages can also resolve the convention name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes signInConvention from AppRootQuery (which runs on every page) and moves it into a dedicated SignInConventionQuery. A new useSignInConventionName hook runs this query only when conventionName isn't already available from AppRootContext, so it fires only on the root-domain auth pages during an OAuth flow. All seven auth components now use the hook. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a new `oauthApplicationByCurrentRequest` GraphQL field that looks up
the Doorkeeper application by `client_id` from the OAuth return URL in the
session or params. Auth pages now show "Log in to use {{appName}}" when
signing in via a third-party OAuth app with no associated convention.
Also renames `useSignInConventionName` → `useSignInContext` (returns
`{ conventionName, oauthAppName }`) and the GraphQL query to
`SignInContextQuery` now that both concerns are covered.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1bafc6b to
e911356
Compare
Replace NullResourceOwner hack with proper unauthenticated redirect in doorkeeper.rb so Doorkeeper stores the OAuth URL in session[:user_return_to] and redirects to the sign-in page. Also add http:// redirect URI variants for localhost in OAuthApplication so Doorkeeper accepts the authorization request from the frontend (which sends http://localhost:3000/oauth/callback in development). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The http:// variants aren't needed — dev testing uses https://intercode.test:5050. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Avoids interference from old _intercode_session cookies that were set with domain: :all (matching all subdomains). The PKCE branch removed domain: :all since JWTs handle cross-domain auth, but browsers holding old broad-domain cookies would send two conflicting _intercode_session values, causing CSRF validation failures on sign-in. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Password reset links were generated using request.host, so if a user requested a reset from a convention subdomain the email would link to that subdomain. Now that /users/password/edit is root-site-only, those links would 404. Use ActionMailer::Base.default_url_options instead, which is always configured to the root INTERCODE_HOST. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
/users/edit is now root-site-only. Build the URL from authenticationManager.issuerUrl so it always points to the root site, and open it in a new tab so the user doesn't lose their place on the convention site. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Like the sign-in/sign-up pages, /users/edit needs to serve the React app shell so EditUserForm renders within the SPA rather than falling back to the default Devise view. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Now that the page lives on the root site and opens in a new tab, navigating to / after save doesn't make sense. Show a Saved! message inline instead, matching the pattern used by EditRootSite. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wraps the form in the same container/row/card structure used by DeviseSignInPage and DeviseSignUpPage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an optional CMS layout specifically for authentication pages (/users/sign_in, /users/sign_up, /users/password/*, /users/edit, /oauth/authorize). When set, CmsContentFinder returns it instead of the default layout for those paths on the root site. Also removes the leftover require for the deleted DynamicCookieDomain file from config/application.rb, which was preventing Rails from booting. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Code Coverage Report: Only Changed Files listed
Minimum allowed coverage is |
- Move auth routes (/users/*) to rootSiteRoutes so they're root-site-only - Extend normalizePathForLayout to treat /users/* and /oauth/authorize paths as layout-sensitive, so the auth CMS layout is applied correctly - Add is_intercode_frontend to AuthorizedApplicationType and suppress the OAuth app name on sign-in pages when it's the Intercode frontend itself - Allow site admins to manage root site and view OAuth applications even when authenticated via doorkeeper token (removes !doorkeeper_token guard) - Replace useState/setState-in-effect with useRef for cachedCmsLayoutId in AppRootLayout, fixing an ESLint cascading-renders warning Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Anywhere the app was popping the sign-in modal now initiates the PKCE OAuth flow and redirects instead, consistent with the sign-in nav item: - useLoginRequired: initiate OAuth with returnPath instead of opening modal - RunCard "log in to sign up" button: same - AppWrapper: remove dead openSignIn/onUnauthenticatedRef code (the GraphQLNotAuthenticatedErrorEvent listener in application.tsx already handles that path) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The sign-in modal is now completely unused — all auth triggers redirect via PKCE OAuth flow instead. This deletes the modal and all its child forms, the context, and cleans up the last consumers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… app Overrides the scopes method so the frontend app doesn't need a DB update when new scopes are added to the Doorkeeper config. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the previous blanket doorkeeper_token check with a specific manage_intercode scope requirement, so the Intercode frontend can manage OAuth applications and root site settings via its bearer token without opening that access to tokens from other OAuth apps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`Doorkeeper.application` is not a method on the Doorkeeper module, so the override was raising `NoMethodError` whenever Doorkeeper called `client.scopes` during `PreAuthorization#validate_scopes` — which made the `/oauth/authorize` flow 500 for the intercode frontend app. Replaces it with `Doorkeeper.config.scopes`, which returns the `Doorkeeper::OAuth::Scopes` object containing `default_scopes + optional_scopes` (including `manage_intercode`). Same return type as the inherited `super`, drop-in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces `access_token_expires_in nil` (eternal tokens) with the Doorkeeper default plus a scope-aware override: tokens carrying any `manage_*` scope expire in 30 minutes; everything else in 2 weeks. Enables `use_refresh_token` so the SPA can transparently extend its session via the existing `previous_refresh_token` rotation path. This caps the impact of any JWT exfiltrated from `localStorage` (e.g. via XSS): a stolen `manage_*` token is unusable after 30 minutes rather than forever. The refresh-token-in-`localStorage` problem still exists and is addressed separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Overrides `SessionsController#destroy` to revoke every non-revoked `Doorkeeper::AccessToken` row owned by `current_user` and issued by the intercode-frontend OAuth app, before letting Devise tear down the session. Revoking the row also kills the refresh token sharing it (Doorkeeper's `RefreshTokenRequest` validates against `revoked?`). The effect: a JWT exfiltrated via XSS stops working as soon as the user signs out, even if the attacker had already forked the refresh chain. This intentionally revokes the user's frontend tokens across every device, since sign-out is a "secure my account" action. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a `prepend_before_action` on `GraphqlController` that mirrors Devise's Doorkeeper-strategy `acceptable?` check. If the request carries an `Authorization: Bearer` whose token is expired, revoked, or unknown, the header is stripped before any `before_action` reads `current_user` — so instead of `DoorkeeperFailureApp` short-circuiting the whole request with 401, the SPA gets a normal anonymous response and can decide what to do. Also sets `X-Bearer-Token-Rejected: true` on those responses so the SPA can distinguish "I was never authenticated" from "my access token was rejected" and trigger a refresh, which matters when the client's clock skew makes a JWT look fresh locally but the server has already considered it expired. Drive-by: freeze METHODS and silence the unrelated MutableConstant rubocop warnings the file's pre-existing constants triggered when modifying it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the SPA up to actually use the refresh token it has been storing: - `AuthenticationManager.ensureFreshAccessToken` decodes the JWT `exp` claim and, if the token is within 30s of expiry, exchanges the refresh token for a fresh pair. Concurrent callers share a single in-flight request via a deduped promise. If refresh fails, local auth state is cleared so the caller falls through to an anonymous request. - The Apollo auth-headers link now awaits `ensureFreshAccessToken` instead of reading `jwtToken` directly, so every outbound GraphQL request gets a proactively-refreshed token when needed. - New `buildRefreshOnRejectedBearerLink` sits at the top of the Apollo link chain and reacts to the server's `X-Bearer-Token-Rejected: true` signal: it clears the (apparently still-valid) access token, forces a refresh, and re-subscribes to the chain so the auth-headers link runs again with the new token. This catches the case where the JWT exp claim looks fresh locally (skewed clock) but the server has already considered the token expired. - Adds a `refreshTokens` wrapper around openid-client's `refreshTokenGrant` so the manager doesn't import openid-client directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Serves the small bundle of values the SPA needs *before* it can fire any authenticated GraphQL — chiefly the OAuth client UID and OIDC issuer URL, without which `ensureFreshAccessToken` can't construct a refresh request on cold boot. Replaces the previously-used server-rendered `data-react-props` blob on the `AppRoot` div (and, in the next commit, the GraphQL `clientConfiguration` query). Same data the GraphQL field exposed today, in the same snake_case shape, so SPA consumers can swap with minimal changes. Drive-by: silence the pre-existing BlockLength / FrozenStringLiteralComment rubocop warnings on `config/routes.rb` triggered by adding the new route. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Module-load now fetches `/client_configuration` once, then constructs the authenticity-tokens manager, AuthenticationManager (with clientId and issuerUrl already populated), and Apollo client. React only renders after that promise resolves. This removes the chicken-and-egg where the AuthenticationManager booted without a clientId, couldn't refresh, and silently logged the user out on the next reload. Consequences: - `clientConfigurationQuery` is no longer fired (the GraphQL field is removed in the next commit). - The `useEffect` that set `clientId`/`issuerUrl` from the GraphQL result is gone — bootstrap handles it synchronously now. - The `data-react-props` blob is no longer read from the AppRoot div, so index.html can become a pure shell over time. - `clientConfigurationDataContext` renamed to `clientConfigurationContext` and re-typed with a flat `ClientConfiguration` shape (snake_case, no GraphQL wrapping object). `root.tsx` and `DeviseSignUpPage` updated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The same data is now served from `GET /client_configuration` and read by the SPA at module load, so the GraphQL field, type, and the lone query that consumed it (`ClientConfigurationQuery`) are no longer referenced. Deletes the GraphQL type and the SPA-side query file, removes the resolver from `QueryType`, and regenerates the schema / generated TS artifacts via `rails graphql:update`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mediates the SPA's OAuth session through an HttpOnly `__Host-` cookie so the long-lived refresh token never has to live in JavaScript-readable storage. The cookie is `Secure`, `HttpOnly`, `SameSite=Strict`, `Path=/`, no `Domain` — the `__Host-` prefix enforces those last three. * `exchange` finalizes the PKCE auth-code grant by running Doorkeeper's `AuthorizationCodeRequest` internally, stuffs the refresh token in the cookie, and returns just the access token to the SPA. * `refresh` re-runs `RefreshTokenRequest` against the cookie, rotates it with the new refresh token, returns a new access token. * `sign_out` revokes the underlying Doorkeeper row (which kills both the access token and its refresh chain) and clears the cookie. CSRF defense for the controller is `SameSite=Strict` on the cookie: the browser will never attach it to a cross-origin request, including top-level navigation from an attacker's page, so a CSRF token would be redundant. `verify_authenticity_token` is skipped accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stops storing the refresh token (and the access token) in localStorage and routes the auth-code exchange, refresh, and sign-out through the new `/oauth_session/*` endpoints. The refresh token lives only in the HttpOnly `__Host-intercode_refresh` cookie; the access token lives only in memory on the `AuthenticationManager`. Effect on an XSS attacker: the worst they can exfiltrate is the access token (already short-lived for `manage_*` scopes; gone the moment the user signs out), not the refresh chain. * `handleOauthCallback` POSTs `code + verifier` to `/oauth_session/exchange` instead of calling openid-client's `authorizationCodeGrant` directly. * `performRefresh` POSTs `/oauth_session/refresh` instead of `refreshTokenGrant` — the cookie carries the refresh token, the SPA never sees it. * `signOut` POSTs `/oauth_session/sign_out` to revoke the row + clear the cookie, then still redirects to `end_session_endpoint` to tear down the Devise session on `INTERCODE_HOST`. * New `bootstrapFromCookie()` runs on cold boot — silently 401-tolerant so logged-out users see no error. Apollo client construction is now deferred until after this resolves, so the first GraphQL query goes out authenticated when the user already has a session. * `deserializeFromBrowser` also clears the legacy `intercode.jwtToken` / `intercode.jwtRefreshToken` keys from localStorage so upgrading users don't leave dead tokens lying around. * `openid.ts` drops `exchangeCodeForToken` and `refreshTokens`; those POSTs are now server-internal in `OAuthSessionsController`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the `code_challenge` and `code_challenge_method` columns to `oauth_access_grants` (originally generated by `rails generate doorkeeper:pkce`, then rewritten as a bulk change_table). Until now `Doorkeeper::AccessGrant.pkce_supported?` returned false, which made `validate_code_verifier` a no-op — the SPA went through the motions of PKCE (challenge in the auth URL, verifier on the token exchange) but the server never bound them, leaving the authorization code redeemable by anyone who could intercept it. With the columns in place Doorkeeper now persists the challenge on each grant and enforces `verifier == sha256(challenge)` at token exchange. Verified end-to-end: wrong verifier → 400 invalid_grant, right verifier → 200. In-flight authorization codes at deploy time won't have a stored challenge, so they'll continue to validate as before during the brief window between rollout and the next user re-auth. Grants created after the migration enforce PKCE. No backfill needed. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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.
Summary
before_actions that don't fire in the PKCE flow); profile creation and destination routing now happen in the React Router loader before any rendering, eliminating flash-of-contentKey changes
authenticationManager: fetches client configuration (OIDC issuer, client ID) via GraphQL; manages PKCE state and token refresh via JWT bearer tokensSessionsController/RegistrationsController/PasswordsController: render app shell for Devise form routes;SessionsController#createallows cross-host redirect back to trusted convention domains after sign-inSignInButton/SignUpButton: initiate OAuth PKCE flow rather than opening a modal; sign-up navigates to/users/sign_up?user_return_to=<oauthUrl>so the user lands back at the convention after registrationappRootLoader: callssetupMyProfilemutation when no profile exists, then redirects to clickwrap agreement or profile setup page as needed — no flash of contentSetupMyProfileGraphQL mutation: creates aUserConProfilefor the current user viaSetupUserConProfileService, idempotent (returns existing profile if one already exists)Test plan
needs_update: trueprofile — should redirect to profile setup🤖 Generated with Claude Code