fix(site/src): stop blocking post-login redirect on CSRF-protected authcheck#26601
Draft
jakehwll wants to merge 1 commit into
Draft
fix(site/src): stop blocking post-login redirect on CSRF-protected authcheck#26601jakehwll wants to merge 1 commit into
jakehwll wants to merge 1 commit into
Conversation
…authcheck The DEVEX-334 e2e flake is two latent bugs stacked on top of each other: 1. `loginFn` fires POST /api/v2/authcheck via Promise.all immediately after the login response sets the session cookie, which makes authcheck CSRF-protected. Any failure here rejects the login mutation and the `location.href` hard reload in LoginPage never runs, leaving the page on /login. 2. `X-CSRF-TOKEN` was cached on the axios default headers exactly once, at module init, from `<meta property="csrf-token">`. If the bundle evaluated before the meta tag was populated (or the cookie was rotated later by a document refetch), the cached header drifted from the `csrf_token` cookie and every CSRF-protected POST 400d. Drop `checkAuthorization` from `loginFn` and let AuthProviders `permissionsQuery` fetch it on the next render. The post-login hard reload re-runs that query against a fresh document anyway, so this only costs one extra request on the dashboard load. Replace the cached header with a per-request axios interceptor that re-reads the meta tag on every call. Module-init still surfaces the "CSRF token not found" warning so misconfigured pages remain obvious. Refs: DEVEX-334
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.
The
test-e2e/test-e2e-premiumflake tracked in DEVEX-334 (internal#461) is two latent SPA bugs lined up on the post-login redirect path:loginFninsite/src/api/queries/users.tsfiresPOST /api/v2/authcheckin aPromise.allimmediately after the login response sets the session cookie, which makes that authcheck CSRF-protected. Any failure there rejects the login mutation, so thelocation.href = sanitizeRedirect(redirectTo)hard reload inLoginPagenever runs and the page stays on/login.X-CSRF-TOKENwas captured on the axios default headers exactly once, at module init, from<meta property="csrf-token">. If the bundle evaluated before the meta tag was populated (or a later document refetch rotated thecsrf_tokencookie) the cached header drifted from the cookie and every CSRF-protected POST 400'd.Drop
checkAuthorizationfromloginFnand letAuthProvider.permissionsQueryfetch authorization on the next render. The post-login hard reload re-runs that query against a fresh document anyway, so the only cost is one extra request on the dashboard load.Replace the cached header with a per-request axios interceptor that reads the meta tag on every call, so the header always reflects whatever the browser most recently received. The module-init
CSRF token not foundwarning is preserved.Refs: DEVEX-334, coder/internal#461
Decision log
/api/v2/authcheckfrom CSRF on the server (mirroring/api/v2/users/first) and rejected it: authcheck is authoritative for permissions and exempting it would weaken the CSRF posture without addressing the broader header-staleness bug.checkAuthorizationinsideloginFnbut reordering it after the hard reload point. Rejected: same race window, and the hard reload already gets a fresh permissions fetch for free on the new document.csrf_tokencookie, but it isHttpOnly, so the client cannot read it.EmbedContext.tsx(VS Code embed flow) uses the samedata.permissionspattern but authenticates viasetSessionToken(header, not cookie), which exempts it from CSRF percoderd/httpmw/csrf.go. Out of scope.