btlqql opened a new pull request, #5336:
URL: https://github.com/apache/rocketmq-dashboard/pull/5336
### Which Issue(s) This PR Fixes
- Fixes #5245
### Brief Description
Two places dropped the operator's page on session expiry:
- `web/src/api/client.ts` — `handleSessionUnauthorized()` cleared the
session and set
`window.location.href = '/login'`, discarding the current path and query
string.
- `web/src/pages/login/index.tsx` — `onFinish` always ran `navigate('/', {
replace: true })`, so
even a login page reached with a target in mind went home.
Now the 401 redirect carries the page the operator was on as an encoded
`redirect` parameter
(`sessionExpiryRedirectUrl`, which redirects plainly when the session
expires while `/login`
itself is open, so the login page never links to itself), and a successful
login navigates to
`sanitizeLoginRedirect(searchParams.get('redirect'))` with the same
`replace` navigation as before
(no back-button trap back into the login page). The target is only accepted
when it is a same-app
absolute path: protocol-relative values (`//evil.example`), absolute
external URLs, paths without a
leading slash and backslash variants all fall back to `/`.
### How Did You Test This Change?
New cases in `web/src/api/client.test.ts` (the redirect URL the 401 handler
produces, both through
`handleSessionUnauthorized()` and through a real 401 from a protected
request, with `window.location`
replaced because jsdom cannot navigate) and in
`web/src/pages/login/index.test.tsx` (return to the
carried page, home without a parameter, and each rejected redirect shape).
All were run first on the
unmodified tree (red) and then after the fix (green).
```
$ cd web && npx vitest run src/api/client.test.ts
src/pages/login/index.test.tsx
# before the fix
× sends an expired session to the login page carrying the page it was on
AssertionError: expected '/login' to be
'/login?redirect=%2Fops%2Falerts%3Flev…' // Object.is equality
× carries the current page when a protected request expires the session
AssertionError: expected '/login' to be
'/login?redirect=%2Finstance%2Fmessage…' // Object.is equality
× returns to the page the operator was on when the login page carries a
redirect
AssertionError: expected "vi.fn()" to be called with arguments: [
'/ops/alerts?level=error', …(1) ]
Test Files 2 failed (2)
Tests 3 failed | 24 passed (27)
# after the fix
✓ src/api/client.test.ts (17 tests) 52ms
✓ src/pages/login/index.test.tsx (10 tests) 2890ms
Test Files 2 passed (2)
Tests 27 passed (27)
$ cd web && npx tsc -b # exit 0, no diagnostics
$ cd web && npx eslint src/api/client.ts src/api/client.test.ts
src/pages/login/index.tsx src/pages/login/index.test.tsx
# exit 0, no problems
```
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits
- [x] Tests added or updated for non-trivial changes
- [ ] New UI text has both Chinese and English entries under `web/src/i18n/`
— not applicable: the
change adds no user-facing string (the existing login messages are
reused unchanged)
- [x] Architecture constraints stay green — no `server/**` file is touched,
so the ArchUnit checks
are unaffected; `npx tsc -b` and `npx eslint` are clean
- [ ] New source files carry the ASF license header — not applicable: no new
source file
- [ ] Documentation touched where behaviour changed — not applicable: the
session-expiry flow is not
described in README or `docs/`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
