From 3f71b2e13fc5da4899788cf8a21647009ebef6dd Mon Sep 17 00:00:00 2001 From: Hyeonsang Cho Date: Sun, 11 Oct 2026 20:05:03 +0900 Subject: [PATCH] fix(frontend): return to the opened workspace page after signing in (#6681) * fix(frontend): return to the opened workspace page after signing in Opening a workspace page without a valid session sent the user to a bare /login: the workspace layout called redirect("/login"), and a layout cannot read the request URL, so the path and query were both lost. A scheduled-task notification link (/workspace/scheduled-tasks?task_id=X) opened after the session expired therefore landed on the default workspace after sign-in. For an unauthenticated verdict the layout now renders WorkspaceLoginRedirect, which replaces the page with /login?next= from the browser. It renders nothing else, so the workspace pages under it never mount. AuthProvider.refreshUser and useRequireAuth kept the path but dropped the query. They now share currentReturnPath() with the fetcher, which already kept both. * docs(changelog): reference #6681 * fix(frontend): keep the fragment in the sign-in return path currentReturnPath() kept the path and query but not location.hash, so an anchored deep link (/workspace/chats/x#section) lost its anchor across the login round trip. It is now the single source for all four bounce sites, so include the fragment. The server never receives the fragment, so only the browser-side bounce can keep it. validateAuthNextPath, the backend's validate_next_param and its quote() into /auth/callback all carry a '#' through unchanged. The models API test stubbed window.location without hash; a real Location always has one, so the stub now carries it. --------- Co-authored-by: Willem Jiang --- CHANGELOG.md | 11 +++ CHANGELOG_zh.md | 7 ++ frontend/AGENTS.md | 4 + frontend/src/app/workspace/layout.tsx | 6 +- .../workspace/workspace-login-redirect.tsx | 25 ++++++ frontend/src/core/api/fetcher.ts | 8 +- frontend/src/core/auth/AuthProvider.tsx | 5 +- frontend/src/core/auth/types.ts | 15 ++++ .../e2e-auth/workspace-login-return.spec.ts | 81 +++++++++++++++++++ .../workspace-login-redirect.dom.test.tsx | 44 ++++++++++ .../unit/core/auth/AuthProvider.dom.test.tsx | 43 +++++++++- frontend/tests/unit/core/models/api.test.ts | 11 ++- 12 files changed, 245 insertions(+), 15 deletions(-) create mode 100644 frontend/src/components/workspace/workspace-login-redirect.tsx create mode 100644 frontend/tests/e2e-auth/workspace-login-return.spec.ts create mode 100644 frontend/tests/unit/components/workspace/workspace-login-redirect.dom.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 21517ccea9..833f214a9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4184,6 +4184,16 @@ This release closes that milestone with **439 merged pull requests**. word order cannot be recovered. Passing both `--print` and `--json` now exits 2 instead of running `--print` and dropping the `--json` message. ([#6636]) +- **frontend:** Signing in after a workspace session ends now returns to the + page that was open, query string and fragment included. Opening a workspace + page without a valid session (for example a scheduled-task notification link, + `/workspace/scheduled-tasks?task_id=...`, after the session expired) sent the + user to a bare `/login`, so signing in landed on the default workspace; the + workspace layout cannot read the request URL, so it now hands the redirect to + the browser, which keeps the full address in `next`. The client-side session + checks (`AuthProvider.refreshUser`, `useRequireAuth`) kept the path but + dropped the query string, and the shared fetcher dropped the fragment; all of + them now keep the full address. ([#6681]) - **sandbox:** The AIO, E2B and OpenSandbox providers and ACP agents no longer resolve `sandbox.environment` / `acp_agents..env` values a second time. `config.yaml` loading already replaces each `$VAR`, so a resolved secret that @@ -9605,3 +9615,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#6636]: https://github.com/bytedance/deer-flow/pull/6636 [#6666]: https://github.com/bytedance/deer-flow/pull/6666 [#6673]: https://github.com/bytedance/deer-flow/pull/6673 +[#6681]: https://github.com/bytedance/deer-flow/pull/6681 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index d49f494e0d..cbf1d07b39 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -3213,6 +3213,12 @@ 由于词序无法还原，会以退出码 2 退出并提示加引号。同时传入 `--print` 和 `--json` 现在会以退出码 2 退出，而不是运行 `--print` 并丢弃 `--json` 的消息。([#6636]) +- **前端：** 工作区会话失效后重新登录，现在会回到原来打开的页面，并保留查询参数和锚点。 + 此前在没有有效会话时打开工作区页面（例如会话过期后点开定时任务通知链接 + `/workspace/scheduled-tasks?task_id=...`）会跳到不带参数的 `/login`，登录后只能 + 回到默认工作区；工作区布局无法读取请求 URL，因此现在改由浏览器发起跳转，在 + `next` 中保留完整地址。客户端会话检查（`AuthProvider.refreshUser`、 + `useRequireAuth`）此前保留了路径但丢掉了查询参数，共享 fetcher 则丢掉了锚点；现在都保留完整地址。([#6681]) - **沙箱：** AIO、E2B、OpenSandbox 提供者和 ACP agent 不再对 `sandbox.environment` / `acp_agents..env` 的值做第二次解析。加载 `config.yaml` 时已替换每个 `$VAR`，因此本身以 `$` 开头的已解析密钥（如 @@ -7720,3 +7726,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#6636]: https://github.com/bytedance/deer-flow/pull/6636 [#6666]: https://github.com/bytedance/deer-flow/pull/6666 [#6673]: https://github.com/bytedance/deer-flow/pull/6673 +[#6681]: https://github.com/bytedance/deer-flow/pull/6681 diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 478d06ba29..fc4c1c5da8 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -318,6 +318,10 @@ Direct auth-page visits with a live suspended-session cookie get the SSR verdict via `AuthDisabledContext`, so the login form states the known reason without waiting for a deliberately generic login failure. +Other sign-in bounces keep path, query and fragment via `currentReturnPath()`. The +workspace layout cannot read the URL, so `unauthenticated` renders +`WorkspaceLoginRedirect` (never `children`), not `redirect()`. + ### Model reasoning capabilities `/api/models` projects a per-model `reasoning` contract (issue #5073) beside the diff --git a/frontend/src/app/workspace/layout.tsx b/frontend/src/app/workspace/layout.tsx index 4a15603d4b..344b0b6a27 100644 --- a/frontend/src/app/workspace/layout.tsx +++ b/frontend/src/app/workspace/layout.tsx @@ -4,6 +4,7 @@ import "streamdown/styles.css"; import { redirect } from "next/navigation"; import { GatewayOfflineFallback } from "@/components/workspace/gateway-offline-fallback"; +import { WorkspaceLoginRedirect } from "@/components/workspace/workspace-login-redirect"; import { AuthProvider } from "@/core/auth/AuthProvider"; import { getServerSideUser } from "@/core/auth/server"; import { ACCOUNT_DISABLED_LOGIN_URL, assertNever } from "@/core/auth/types"; @@ -35,7 +36,10 @@ export default async function WorkspaceLayout({ case "system_setup_required": redirect("/setup"); case "unauthenticated": - redirect("/login"); + // Not redirect(): a layout cannot read the request URL, so the browser + // carries the page it is on into `next`. + content = ; + break; case "account_disabled": // The login page renders the disabled-account reason from the error // param instead of a plain sign-in form the user cannot complete. diff --git a/frontend/src/components/workspace/workspace-login-redirect.tsx b/frontend/src/components/workspace/workspace-login-redirect.tsx new file mode 100644 index 0000000000..73a15e44af --- /dev/null +++ b/frontend/src/components/workspace/workspace-login-redirect.tsx @@ -0,0 +1,25 @@ +"use client"; + +import { useRouter } from "next/navigation"; +import { useEffect } from "react"; + +import { buildLoginUrl, currentReturnPath } from "@/core/auth/types"; + +/** + * Sends a signed-out visitor to the login page with this page as `next`. + * + * The workspace layout learns the session is missing on the server, but a + * layout cannot read the request URL, so a server redirect there would drop + * the page the user opened (e.g. a `?task_id=` link from a notification). + * The browser still has it. `replace` keeps this empty page out of history, + * as the server redirect did. + */ +export function WorkspaceLoginRedirect() { + const router = useRouter(); + + useEffect(() => { + router.replace(buildLoginUrl(currentReturnPath())); + }, [router]); + + return null; +} diff --git a/frontend/src/core/api/fetcher.ts b/frontend/src/core/api/fetcher.ts index bed24af915..9f3eb779e6 100644 --- a/frontend/src/core/api/fetcher.ts +++ b/frontend/src/core/api/fetcher.ts @@ -1,6 +1,7 @@ import { ACCOUNT_DISABLED_LOGIN_URL, buildLoginUrl, + currentReturnPath, parseAuthError, } from "@/core/auth/types"; import { isStaticWebsiteOnly } from "@/core/static-mode"; @@ -115,12 +116,7 @@ export async function fetch( window.location.href = ACCOUNT_DISABLED_LOGIN_URL; throw new UnauthorizedError(); } - // Include the search string: routes that carry their target in the query - // (e.g. the standalone artifact viewer) are otherwise unrecoverable after - // login, which lands on the default workspace instead. - window.location.href = buildLoginUrl( - `${window.location.pathname}${window.location.search}`, - ); + window.location.href = buildLoginUrl(currentReturnPath()); throw new UnauthorizedError(); } diff --git a/frontend/src/core/auth/AuthProvider.tsx b/frontend/src/core/auth/AuthProvider.tsx index 8c94e9d207..3fee616580 100644 --- a/frontend/src/core/auth/AuthProvider.tsx +++ b/frontend/src/core/auth/AuthProvider.tsx @@ -16,6 +16,7 @@ import { ACCOUNT_DISABLED_LOGIN_URL, type User, buildLoginUrl, + currentReturnPath, parseAuthError, } from "./types"; @@ -106,7 +107,7 @@ export function AuthProvider({ children, initialUser }: AuthProviderProps) { router.push(ACCOUNT_DISABLED_LOGIN_URL); } else if (pathname?.startsWith("/workspace")) { // Session expired or invalid — redirect on protected routes - router.push(buildLoginUrl(pathname)); + router.push(buildLoginUrl(currentReturnPath())); } } } catch (err) { @@ -221,7 +222,7 @@ export function useRequireAuth(): AuthContextType { // Only redirect if we're sure user is not authenticated (not just loading) if (!auth.isLoading && !auth.isAuthenticated) { - router.push(buildLoginUrl(pathname || "/workspace")); + router.push(buildLoginUrl(currentReturnPath())); } }, [auth.isAuthenticated, auth.isLoading, router, pathname]); diff --git a/frontend/src/core/auth/types.ts b/frontend/src/core/auth/types.ts index a837274161..01af671649 100644 --- a/frontend/src/core/auth/types.ts +++ b/frontend/src/core/auth/types.ts @@ -47,6 +47,21 @@ export function buildLoginUrl(returnPath: string): string { return `/login?next=${encodeURIComponent(returnPath)}`; } +/** + * The page the browser is on, query string and fragment included, as a + * `next` target. + * + * Every client-side bounce for an expired session uses this, so they agree: + * pages that carry their target in the query (a `?task_id=` deep link, the + * artifact viewer) are otherwise unrecoverable after login, which lands on the + * default workspace instead. Browser-only; the server cannot see the URL from + * a layout, and never sees the fragment at all. + */ +export function currentReturnPath(): string { + const { pathname, search, hash } = window.location; + return `${pathname}${search}${hash}`; +} + /** * Login-page URL that surfaces the account-disabled reason (#4063 gap 3). * A disabled account cannot sign back in, so unlike buildLoginUrl there is diff --git a/frontend/tests/e2e-auth/workspace-login-return.spec.ts b/frontend/tests/e2e-auth/workspace-login-return.spec.ts new file mode 100644 index 0000000000..7c403b2830 --- /dev/null +++ b/frontend/tests/e2e-auth/workspace-login-return.spec.ts @@ -0,0 +1,81 @@ +import { expect, test } from "@playwright/test"; + +/** + * A workspace page opened without a session — an expired cookie, or a + * notification link opened later — must come back to the same page, query + * string included, after the user signs in. The workspace layout cannot read + * the request URL on the server, so the bounce happens in the browser. + */ +test.describe("workspace login bounce", () => { + test.beforeEach(async ({ page }) => { + await page.route("**/api/v1/auth/providers", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ providers: [] }), + }), + ); + await page.route("**/api/v1/auth/setup-status", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ needs_setup: false }), + }), + ); + }); + + test("keeps the deep link's query string in next", async ({ page }) => { + await page.goto("/workspace/scheduled-tasks?task_id=task-1"); + + await expect(page).toHaveURL( + "/login?next=%2Fworkspace%2Fscheduled-tasks%3Ftask_id%3Dtask-1", + ); + await expect(page.getByRole("button", { name: "Sign In" })).toBeVisible(); + }); + + test("keeps a nested workspace path in next", async ({ page }) => { + await page.goto("/workspace/chats/thread-1"); + + await expect(page).toHaveURL("/login?next=%2Fworkspace%2Fchats%2Fthread-1"); + }); + + test("keeps the fragment in next", async ({ page }) => { + // The server never sees the fragment; only the browser bounce can keep it. + await page.goto("/workspace/chats/thread-1#message-3"); + + await expect(page).toHaveURL( + "/login?next=%2Fworkspace%2Fchats%2Fthread-1%23message-3", + ); + }); + + test("does not mount workspace pages before the bounce", async ({ page }) => { + // The layout renders only the redirect, so the chat page under it must + // never start loading data for a visitor without a session. + const workspaceRequests: string[] = []; + page.on("request", (request) => { + const { pathname } = new URL(request.url()); + if ( + pathname.startsWith("/api/") && + !pathname.startsWith("/api/v1/auth/") + ) { + workspaceRequests.push(pathname); + } + }); + + await page.goto("/workspace/chats/thread-1"); + await expect(page).toHaveURL(/\/login\?next=/); + await expect(page.getByRole("button", { name: "Sign In" })).toBeVisible(); + + expect(workspaceRequests).toEqual([]); + }); + + test("does not leave the bounce page in history", async ({ page }) => { + await page.goto("/login"); + await page.goto("/workspace/scheduled-tasks?task_id=task-1"); + await expect(page).toHaveURL(/\/login\?next=/); + + await page.goBack(); + + await expect(page).toHaveURL("/login"); + }); +}); diff --git a/frontend/tests/unit/components/workspace/workspace-login-redirect.dom.test.tsx b/frontend/tests/unit/components/workspace/workspace-login-redirect.dom.test.tsx new file mode 100644 index 0000000000..2687309044 --- /dev/null +++ b/frontend/tests/unit/components/workspace/workspace-login-redirect.dom.test.tsx @@ -0,0 +1,44 @@ +import { afterEach, expect, rs, test } from "@rstest/core"; +import { cleanup, render } from "@testing-library/react"; + +import { WorkspaceLoginRedirect } from "@/components/workspace/workspace-login-redirect"; + +const push = rs.fn(); +const replace = rs.fn(); +rs.mock("next/navigation", () => ({ + useRouter: () => ({ push, replace }), +})); + +afterEach(() => { + cleanup(); + push.mockReset(); + replace.mockReset(); +}); + +test("sends the visitor to login with the page they opened as next", () => { + window.history.replaceState({}, "", "/workspace/scheduled-tasks?task_id=t-1"); + + const { container } = render(); + + expect(replace).toHaveBeenCalledTimes(1); + expect(replace).toHaveBeenCalledWith( + "/login?next=%2Fworkspace%2Fscheduled-tasks%3Ftask_id%3Dt-1", + ); + // A replace, so Back does not return to this empty page. + expect(push).not.toHaveBeenCalled(); + expect(container.innerHTML).toBe(""); +}); + +test("keeps the fragment, so an anchored deep link survives the round trip", () => { + window.history.replaceState( + {}, + "", + "/workspace/chats/thread-1?x=1#message-3", + ); + + render(); + + expect(replace).toHaveBeenCalledWith( + "/login?next=%2Fworkspace%2Fchats%2Fthread-1%3Fx%3D1%23message-3", + ); +}); diff --git a/frontend/tests/unit/core/auth/AuthProvider.dom.test.tsx b/frontend/tests/unit/core/auth/AuthProvider.dom.test.tsx index b49e4fdbc0..d5a0657076 100644 --- a/frontend/tests/unit/core/auth/AuthProvider.dom.test.tsx +++ b/frontend/tests/unit/core/auth/AuthProvider.dom.test.tsx @@ -1,8 +1,12 @@ import { afterEach, beforeEach, expect, test, rs } from "@rstest/core"; -import { act, cleanup, render } from "@testing-library/react"; +import { act, cleanup, render, renderHook } from "@testing-library/react"; import type React from "react"; -import { AuthProvider, useAuth } from "@/core/auth/AuthProvider"; +import { + AuthProvider, + useAuth, + useRequireAuth, +} from "@/core/auth/AuthProvider"; import type { User } from "@/core/auth/types"; // refreshUser's 401 handling runs through next/navigation; capture the push @@ -10,7 +14,7 @@ import type { User } from "@/core/auth/types"; const push = rs.fn(); rs.mock("next/navigation", () => ({ useRouter: () => ({ push }), - usePathname: () => "/workspace", + usePathname: () => window.location.pathname, })); const initialUser: User = { @@ -87,3 +91,36 @@ test("every other 401 keeps the return-to-workspace redirect", async () => { expect(ctx!.user).toBeNull(); expect(push).toHaveBeenCalledWith("/login?next=%2Fworkspace"); }); + +test("a 401 returns to the page it happened on, query string included", async () => { + window.history.replaceState({}, "", "/workspace/scheduled-tasks?task_id=t-1"); + mount(); + globalThis.fetch = rs.fn(async () => + jsonResponse( + { detail: { code: "token_expired", message: "Token expired" } }, + 401, + ), + ) as unknown as typeof globalThis.fetch; + + await act(async () => { + await ctx!.refreshUser(); + }); + + expect(push).toHaveBeenCalledWith( + "/login?next=%2Fworkspace%2Fscheduled-tasks%3Ftask_id%3Dt-1", + ); +}); + +test("useRequireAuth returns to the page it guards, query string included", () => { + window.history.replaceState({}, "", "/workspace/scheduled-tasks?task_id=t-1"); + + renderHook(() => useRequireAuth(), { + wrapper: ({ children }: { children: React.ReactNode }) => ( + {children} + ), + }); + + expect(push).toHaveBeenCalledWith( + "/login?next=%2Fworkspace%2Fscheduled-tasks%3Ftask_id%3Dt-1", + ); +}); diff --git a/frontend/tests/unit/core/models/api.test.ts b/frontend/tests/unit/core/models/api.test.ts index 8d022e5f21..5ef4669157 100644 --- a/frontend/tests/unit/core/models/api.test.ts +++ b/frontend/tests/unit/core/models/api.test.ts @@ -56,9 +56,14 @@ test("loadModels rejects unsuccessful gateway responses", async () => { }); test("loadModels exposes the typed 401 redirect error", async () => { - // `search` is always a string on a real Location; the redirect target is - // built from pathname + search so the stub has to carry both. - const location = { href: "", pathname: "/workspace/chats", search: "" }; + // `search` and `hash` are always strings on a real Location; the redirect + // target is built from pathname + search + hash so the stub carries all three. + const location = { + href: "", + pathname: "/workspace/chats", + search: "", + hash: "", + }; rs.stubGlobal("window", { location }); rs.stubGlobal( "fetch",