fix(frontend): logout lands on /; 401 redirect preserves query safely - #774
Open
CH3CHO wants to merge 1 commit into
Open
fix(frontend): logout lands on /; 401 redirect preserves query safely#774CH3CHO wants to merge 1 commit into
CH3CHO wants to merge 1 commit into
Conversation
- AvatarDropdown no longer passes redirect=<pathname>; login page falls back to / - request.tsx 401 handler uses buildRedirectSearch so pathname+search round-trip - login page uses sanitizeRedirectValue to block open-redirect (//, http(s):, javascript:, /login self-loop, /\, control chars) - new frontend/src/utils/redirect.ts with sanitizeRedirectValue + buildRedirectSearch - node:test coverage for both pure functions (31 cases) Signed-off-by: CH3CHO <ch3cho@qq.com>
CH3CHO
force-pushed
the
fix/logout-redirect-root
branch
from
August 27, 2026 02:40
07dd830 to
e80b1ab
Compare
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.
Fixes #772
Problem
Clicking "logout" in the avatar dropdown navigates the user to
/login?redirect=<pathname>. This has two defects:redirectusespathnameonly, notpathname + search. On/users?tab=profile, after login the user lands on/usersand losestab=profile, breaking page state.User request: after logout, default to landing on the root page after re-login.
Solution
Extract two pure functions into
frontend/src/utils/redirect.ts:sanitizeRedirectValue(raw)— validates and sanitizes aredirectquery value. Rejects//host,http(s)://,javascript:, the/loginself-loop (case-insensitive, with or without query/hash suffix),/\host, and C0 control characters. All open-redirect defenses live here.buildRedirectSearch(pathname, search)— used in the 401 flow. Combinespathname + search, runsencodeURIComponentover the result, and returnsredirect=<encoded>(without the leading?). The caller decides whether to add?.Three call sites share the helpers:
AvatarDropdown/index.tsx— active logout now callshistory.push('/login'), noredirectparam.services/request.tsx— 401 interceptor usesbuildRedirectSearch(pathname, search)sopathname + searchround-trip cleanly.pages/login/index.tsx— after successful login,sanitizeRedirectValue(urlParams.get('redirect'))validates the value before navigation.Additional Security Hardening (from code review)
Bugs that code review caught before the fix landed:
/\evil.comprotocol-relative bypass. WHATWG URL parsing treats\as/inside the authority, so/\evil.comresolves to//evil.com. Closed by addingraw[1] === '\'to the leading-slash guard./\t/evil.com,/\n/evil.com,/\r/evil.comall become//evil.com. Closed by adding/[\x00-\x1F\x7F]/rejection./loginself-loop guard was exact-match. The original===check could be bypassed by/login?next=1,/LOGIN, etc. Tightened to a path-segment match.Files Changed
frontend/src/utils/redirect.tssanitizeRedirectValue+buildRedirectSearchfrontend/src/utils/redirect.test.tsnode:testcasesfrontend/src/components/AvatarDropdown/index.tsxredirect=<pathname>indoLogoutfrontend/src/services/request.tsxbuildRedirectSearchin 401 handlerfrontend/src/pages/login/index.tsxsanitizeRedirectValuepost-loginVerification
node:testcases all green (covers normal paths, all known bypass vectors).eslint-disable-next-linesuppressions are scoped to attack-vector test fixtures only).tsc --noEmitintroduces no new errors (only the pre-existingdompurifytyping issue remains)./is preserved.Test Plan (manual e2e)
/users?tab=profile→ click logout → after re-login land on/(not/users)/users?tab=profile→ clearlocalStorage.tokenand make any API call → after re-login return to/users?tab=profile/login?redirect=https://example.comdirectly → login → land on//login?redirect=//example.comdirectly → login → land on//login?redirect=/loginor/login?redirect=/LOGINdirectly → login → land on/