🎯 fix: Narrow Public Share 401 Bypass to the Share Endpoint Only - #12905
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a client-side auth-interceptor edge case where public shared-link pages could be redirected to login due to unrelated unauthenticated 401s, by narrowing the “share page” exception to only the shared-messages data fetch endpoint.
Changes:
- Adds share-page and shared-link-request detection helpers (including support for base-prefixed deployments) to constrain the 401 refresh/redirect flow.
- Updates the 401 interceptor guard to only bypass the “no Authorization header” early-reject for
GET /api/share/:shareId. - Extends request-interceptor unit tests to cover base-prefixed share URLs and to ensure unrelated 401s don’t cause login redirects on share pages.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/data-provider/src/request.ts | Refines the Axios 401 interceptor guard by introducing share-page + shared-link request detection helpers. |
| packages/data-provider/specs/request-interceptor.spec.ts | Adds/updates tests to validate the narrowed guard behavior on share pages and base-prefixed routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isSharePage = () => | ||
| /(?:^|\/)share\/[^/]+\/?$/.test(stripBasePath(window.location.pathname)); | ||
|
|
||
| const getRequestPathname = (url?: string) => { | ||
| if (typeof url !== 'string') { | ||
| return ''; | ||
| } | ||
| try { | ||
| return new URL(url, window.location.origin).pathname; | ||
| } catch { | ||
| return url.split(/[?#]/)[0] ?? ''; | ||
| } | ||
| }; | ||
|
|
||
| const isSharedMessagesRequest = (url?: string, method?: string) => | ||
| method?.toLowerCase() === 'get' && | ||
| /(?:^|\/)api\/share\/[^/]+$/.test(getRequestPathname(url)); | ||
|
|
| it('recognizes base-prefixed shared link data requests', async () => { | ||
| expect.assertions(2); | ||
| setTokenHeader(undefined); | ||
|
|
||
| setWindowLocation({ | ||
| href: 'http://localhost/chat/share/abc123', | ||
| pathname: '/chat/share/abc123', | ||
| search: '', | ||
| hash: '', | ||
| origin: 'http://localhost', | ||
| } as Partial<Location>); | ||
|
|
||
| mockAdapter.mockRejectedValueOnce({ | ||
| response: { status: 401 }, | ||
| config: { url: '/chat/api/share/abc123', method: 'get', headers: {} }, | ||
| }); | ||
|
|
||
| mockAdapter.mockResolvedValueOnce({ | ||
| data: { token: 'new-token' }, | ||
| status: 200, | ||
| headers: {}, | ||
| config: {}, | ||
| }); | ||
|
|
||
| mockAdapter.mockResolvedValueOnce({ | ||
| data: { sharedLink: {} }, | ||
| status: 200, | ||
| headers: {}, | ||
| config: {}, | ||
| }); | ||
|
|
||
| try { | ||
| await axios.get('/chat/api/share/abc123'); | ||
| } catch { | ||
| // may reject depending on exact flow | ||
| } | ||
|
|
||
| expect(mockAdapter.mock.calls.length).toBe(3); | ||
|
|
||
| const refreshCall = mockAdapter.mock.calls[1]; | ||
| expect(refreshCall[0].url).toContain('api/auth/refresh'); | ||
| }); |
danny-avila
changed the title
|
@codex review |
GitNexus: 🚀 deployedThe |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
b80381d to
1207a6b
Compare
GitNexus: 🚀 deployedThe |
1207a6b to
0a4b673
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
danny-avila
changed the title
Summary
Fixes #12890 by preventing unrelated unauthenticated API 401s from forcing public shared-link pages to redirect to login.
Public share pages intentionally load
/api/share/:shareIdwithout an access token, but other app-level queries can still run on the page. In the reported flow,/api/share/:idreturned 200, then/api/mcp/serversreturned 401, and the global Axios interceptor attempted refresh + redirected to/login?redirect_to=/share/:id.This PR narrows the unauthenticated share-page exception to only
GET /api/share/:shareId, including base-prefixed deployments such as/chat/api/share/:shareId. Other unauthenticated 401s on public share pages now reject normally instead of triggering login navigation.Validation
git diff --cached --checkgit diff --checkI attempted
npx jest specs/request-interceptor.spec.ts --runInBand, but this worktree is missing the package dependency tree (jest-junit), so the focused spec could not run locally without reinstalling dependencies.