fix(frontend): preserve returnTo through the onboarding interstitial (#14353)

Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: OpenHands Bot <contact@all-hands.dev>

enterprise/server/routes/auth.py

@@ -5,7 +5,7 @@ import warnings
 from datetime import datetime, timedelta, timezone
 from types import MappingProxyType
 from typing import Annotated, Optional, cast
-from urllib.parse import quote, urlencode
+from urllib.parse import parse_qs, quote, urlencode, urlparse
 from uuid import UUID as parse_uuid
 
 from fastapi import (
@@ -586,7 +586,10 @@ async def keycloak_callback(
         # Only redirect to onboarding if user has a valid offline token,
         # otherwise they need to complete the Keycloak offline token flow first
         if valid_offline_token and await _should_redirect_to_onboarding(user_id, user):
-            redirect_url = f'{web_url}/onboarding'
+            # Preserve the user's originally requested destination as
+            # ``?returnTo=...`` so the frontend ``OnboardingForm`` can
+            # restore it after the user finishes the form.
+            redirect_url = _build_onboarding_redirect(redirect_url, web_url)
             logger.info(
                 'Redirecting returning user to onboarding',
                 extra={'user_id': user_id, 'deployment_mode': DEPLOYMENT_MODE},
@@ -690,6 +693,97 @@ async def authenticate(request: Request):
         return response
 
 
+def _extract_login_inner_return_to(relative_url: str) -> str | None:
+    """Extract the inner ``returnTo`` from a ``/login?returnTo=...`` URL.
+
+    Returns the decoded inner ``returnTo`` value, or ``None`` if
+    ``relative_url`` is not a login URL or has no inner ``returnTo``.
+
+    The OAuth flow's ``state`` is set to the full URL of the page that
+    triggered the login (see ``generateAuthUrl`` in the frontend).
+    For an unauthenticated deep-link visit, that page is itself
+    ``/login?returnTo=<actual destination>``, so the OAuth callback's
+    ``redirect_url`` ends up *wrapping* the user's true destination
+    inside a login URL. Sending the user back through ``/login`` after
+    onboarding works in principle (``LoginPage`` re-redirects authed
+    users to its own ``returnTo``), but the round-trip adds extra
+    state and is brittle when query-string layering goes wrong.
+
+    Unwrapping here keeps the post-onboarding navigation a single
+    direct step, e.g. ``/onboarding?returnTo=%2Fsettings%2Fuser``
+    rather than the doubly-nested
+    ``/onboarding?returnTo=%2Flogin%3FreturnTo%3D%252Fsettings...``.
+    """
+    parsed = urlparse(relative_url)
+    if parsed.path != '/login':
+        return None
+    inner = parse_qs(parsed.query).get('returnTo')
+    if not inner:
+        return None
+    value = inner[0]
+    if not value.startswith('/'):
+        return None
+    return value
+
+
+def _build_onboarding_redirect(original_url: str, web_url: str) -> str:
+    """Build the ``/onboarding`` redirect URL preserving ``returnTo``.
+
+    The user's originally requested destination is preserved as a
+    ``returnTo`` query parameter on ``/onboarding``.
+
+    Without this, any deep link the user clicked while logged out
+    (e.g. ``/conversations/abc?foo=bar``) is silently dropped at the
+    onboarding interstitial because the OAuth callback would clobber
+    its working ``redirect_url`` with a bare ``f'{web_url}/onboarding'``.
+    The frontend ``OnboardingForm`` reads this ``returnTo`` query
+    parameter and restores it after the user finishes the form.
+
+    The trivial home-page case (``original_url`` empty, equal to
+    ``web_url``, or pointing at ``web_url/``) returns the bare
+    ``/onboarding`` URL to keep the URL bar clean — that is already
+    the default landing page once onboarding completes.
+
+    The ``returnTo`` value is always a *relative* path (``/foo?bar``)
+    rather than an absolute URL: that keeps the URL short, avoids
+    leaking the deployment origin into the browser bar a second time,
+    and lets the frontend use ``navigate(returnTo)`` directly.
+
+    When ``original_url`` is itself a ``/login?returnTo=...`` URL —
+    which is the common case for unauthenticated deep-link visits,
+    because the OAuth flow's ``state`` carries the full login page
+    URL — the *inner* ``returnTo`` is extracted so the user lands at
+    their real destination in a single navigation rather than
+    bouncing through ``/login`` after onboarding.
+    """
+    onboarding_url = f'{web_url}/onboarding'
+    if not original_url:
+        return onboarding_url
+
+    # Compute the path-and-query portion of the original URL. We try
+    # to strip the deployment origin first so we end up with a
+    # relative path; if the URL points at a different host we fall
+    # back to the URL as-is. The ``OnboardingForm`` component's
+    # ``sanitizeReturnTo`` helper rejects absolute/protocol-relative
+    # URLs before use, so any unexpected absolute value here is safe.
+    relative = original_url
+    if web_url and original_url.startswith(web_url):
+        relative = original_url[len(web_url) :] or '/'
+
+    # If we ended up with a login-page URL, unwrap its inner
+    # ``returnTo`` so post-onboarding navigation goes straight to the
+    # user's real destination instead of bouncing through ``/login``.
+    inner_return_to = _extract_login_inner_return_to(relative)
+    if inner_return_to is not None:
+        relative = inner_return_to
+
+    # Skip the trivial home-page case to keep the URL clean.
+    if relative in ('', '/'):
+        return onboarding_url
+
+    return f'{onboarding_url}?returnTo={quote(relative, safe="")}'
+
+
 async def _should_redirect_to_onboarding(user_id: str, user: User) -> bool:
     """Check if user should be redirected to onboarding after TOS acceptance.
     Backend always redirects applicable users to /onboarding.
@@ -749,7 +843,10 @@ async def _get_post_auth_redirect(
             'Redirecting user to onboarding',
             extra={'user_id': user_id, 'deployment_mode': DEPLOYMENT_MODE},
         )
-        return f'{web_url}/onboarding'
+        # Preserve the user's originally requested destination as
+        # ``?returnTo=...`` so the frontend ``OnboardingForm`` can
+        # restore it after the user finishes the form.
+        return _build_onboarding_redirect(default_url, web_url)
     return default_url
 
 

enterprise/tests/unit/server/routes/test_auth_onboarding.py

@@ -17,6 +17,7 @@ from fastapi.responses import JSONResponse
 from server.auth.saas_user_auth import SaasUserAuth
 from server.routes.auth import (
     OnboardingSubmission,
+    _build_onboarding_redirect,
     _get_post_auth_redirect,
     _should_redirect_to_onboarding,
     complete_onboarding,
@@ -237,6 +238,210 @@ class TestGetPostAuthRedirect:
         assert call_args[0][0] == 'Redirecting user to onboarding'
         assert call_args[1]['extra']['user_id'] == user_id
 
+    @pytest.mark.asyncio
+    async def test_preserves_deep_link_path_as_returnTo(self, mock_user):
+        """Regression: deep-link path is preserved as ``returnTo``.
+
+        When an onboarding-needing user originally requested a deep
+        link (e.g. ``/conversations/abc``), the backend must redirect
+        to ``/onboarding?returnTo=...`` so the frontend
+        ``OnboardingForm`` can restore that destination after the
+        user finishes the form. Without this, deep links clicked
+        while logged out are silently dropped at the onboarding
+        interstitial.
+        """
+        mock_user.onboarding_completed = False
+        user_id = str(mock_user.id)
+
+        with (
+            patch('server.routes.auth.DEPLOYMENT_MODE', 'cloud'),
+            patch(
+                'server.routes.auth.UserStore.get_user_by_id',
+                new_callable=AsyncMock,
+                return_value=mock_user,
+            ),
+        ):
+            result = await _get_post_auth_redirect(
+                user_id,
+                'https://example.com/conversations/abc',
+                'https://example.com',
+            )
+
+        assert result == (
+            'https://example.com/onboarding?returnTo=%2Fconversations%2Fabc'
+        )
+
+    @pytest.mark.asyncio
+    async def test_preserves_query_string_as_returnTo(self, mock_user):
+        """Regression: query-string destinations are preserved fully.
+
+        Destinations like ``/conversations/abc?foo=bar`` must be
+        preserved in the encoded ``returnTo`` rather than truncated.
+        """
+        mock_user.onboarding_completed = False
+        user_id = str(mock_user.id)
+
+        with (
+            patch('server.routes.auth.DEPLOYMENT_MODE', 'cloud'),
+            patch(
+                'server.routes.auth.UserStore.get_user_by_id',
+                new_callable=AsyncMock,
+                return_value=mock_user,
+            ),
+        ):
+            result = await _get_post_auth_redirect(
+                user_id,
+                'https://example.com/conversations/abc?foo=bar',
+                'https://example.com',
+            )
+
+        assert result == (
+            'https://example.com/onboarding'
+            '?returnTo=%2Fconversations%2Fabc%3Ffoo%3Dbar'
+        )
+
+
+# --- Tests for _build_onboarding_redirect ---
+
+
+class TestBuildOnboardingRedirect:
+    """Tests for the ``_build_onboarding_redirect`` helper.
+
+    These exercise the unit directly — the integration coverage
+    through ``_get_post_auth_redirect`` lives in
+    ``TestGetPostAuthRedirect`` above.
+    """
+
+    def test_strips_web_url_prefix_to_relative_path(self):
+        result = _build_onboarding_redirect(
+            'https://example.com/foo/bar', 'https://example.com'
+        )
+        assert result == ('https://example.com/onboarding?returnTo=%2Ffoo%2Fbar')
+
+    def test_preserves_query_string_in_returnTo(self):
+        result = _build_onboarding_redirect(
+            'https://example.com/foo?bar=baz&qux=1', 'https://example.com'
+        )
+        assert result == (
+            'https://example.com/onboarding' '?returnTo=%2Ffoo%3Fbar%3Dbaz%26qux%3D1'
+        )
+
+    def test_skips_returnTo_for_bare_home_with_trailing_slash(self):
+        # ``https://example.com/`` is the home page; appending a
+        # ``returnTo=%2F`` would be noise since ``/`` is the default
+        # post-onboarding landing page anyway.
+        result = _build_onboarding_redirect(
+            'https://example.com/', 'https://example.com'
+        )
+        assert result == 'https://example.com/onboarding'
+
+    def test_skips_returnTo_for_bare_home_without_trailing_slash(self):
+        result = _build_onboarding_redirect(
+            'https://example.com', 'https://example.com'
+        )
+        assert result == 'https://example.com/onboarding'
+
+    def test_skips_returnTo_when_original_url_is_empty(self):
+        result = _build_onboarding_redirect('', 'https://example.com')
+        assert result == 'https://example.com/onboarding'
+
+    def test_passes_through_absolute_url_when_origin_does_not_match(self):
+        # Defensive: if the original_url doesn't share the deployment
+        # origin, the helper preserves it as-is so the frontend can
+        # decide whether to navigate within the SPA or do a hard
+        # window.location redirect.
+        result = _build_onboarding_redirect(
+            'https://other.example.com/foo', 'https://example.com'
+        )
+        assert result == (
+            'https://example.com/onboarding'
+            '?returnTo=https%3A%2F%2Fother.example.com%2Ffoo'
+        )
+
+    def test_unwraps_cross_origin_login_returnTo(self):
+        """Cross-origin login URLs still have their inner ``returnTo`` extracted.
+
+        ``_extract_login_inner_return_to`` matches on the path only
+        (``parsed.path == '/login'``); it does not check the host.  A
+        login URL at a different origin therefore has its inner
+        destination unwrapped to a safe relative path rather than being
+        preserved as a cross-origin absolute URL.
+
+        This differs from ``test_passes_through_absolute_url_when_origin_does_not_match``
+        (non-login cross-origin path → absolute URL preserved as-is) and
+        the behaviour is intentionally *safer*: the attacker-controlled
+        origin is discarded and only the relative ``returnTo`` value from
+        the query string is used.
+        """
+        result = _build_onboarding_redirect(
+            'https://other.example.com/login?returnTo=%2Ffoo', 'https://example.com'
+        )
+        assert result == 'https://example.com/onboarding?returnTo=%2Ffoo'
+
+    def test_unwraps_login_returnTo_to_inner_destination(self):
+        """Regression: login-wrapped destinations are unwrapped.
+
+        An unauthenticated deep-link visit creates an OAuth ``state``
+        that wraps the user's real destination inside a
+        ``/login?returnTo=...`` URL. The onboarding redirect must
+        unwrap that so the user lands directly at their destination
+        after finishing onboarding instead of bouncing through
+        ``/login`` (which adds round-trips and is brittle when
+        query-string layering goes wrong).
+        """
+        result = _build_onboarding_redirect(
+            'https://example.com/login?returnTo=%2Fsettings%2Fuser'
+            '&login_method=github',
+            'https://example.com',
+        )
+        assert result == (
+            'https://example.com/onboarding' '?returnTo=%2Fsettings%2Fuser'
+        )
+
+    def test_unwraps_login_returnTo_with_inner_query_string(self):
+        """Inner destinations with their own query string survive unwrap.
+
+        Destinations like ``/conversations/abc?foo=bar`` must keep
+        their query string when the outer login URL is unwrapped.
+        """
+        result = _build_onboarding_redirect(
+            'https://example.com/login'
+            '?returnTo=%2Fconversations%2Fabc%3Ffoo%3Dbar'
+            '&login_method=github',
+            'https://example.com',
+        )
+        assert result == (
+            'https://example.com/onboarding'
+            '?returnTo=%2Fconversations%2Fabc%3Ffoo%3Dbar'
+        )
+
+    def test_unwraps_login_returnTo_to_bare_home_skips_returnTo(self):
+        """Home-page short-circuit applies after the unwrap step.
+
+        If the unwrapped destination is the bare home page, the
+        helper should still emit the clean ``/onboarding`` URL with
+        no ``returnTo`` query parameter.
+        """
+        result = _build_onboarding_redirect(
+            'https://example.com/login?returnTo=%2F&login_method=github',
+            'https://example.com',
+        )
+        assert result == 'https://example.com/onboarding'
+
+    def test_does_not_unwrap_non_login_path(self):
+        """Non-login paths with a ``returnTo`` are preserved verbatim.
+
+        Only ``/login`` URLs get the inner-returnTo unwrap; other
+        paths that happen to carry a ``returnTo`` query parameter
+        must be preserved as-is.
+        """
+        result = _build_onboarding_redirect(
+            'https://example.com/foo?returnTo=%2Fbar', 'https://example.com'
+        )
+        assert result == (
+            'https://example.com/onboarding' '?returnTo=%2Ffoo%3FreturnTo%3D%252Fbar'
+        )
+
 
 # --- Tests for /complete_onboarding endpoint ---
 

frontend/__tests__/components/features/guards/onboarding-guard.test.tsx

@@ -0,0 +1,151 @@
+import { render } from "@testing-library/react";
+import { createRoutesStub } from "react-router";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { OnboardingGuard } from "#/components/features/guards/onboarding-guard";
+
+const mockNavigate = vi.fn();
+
+vi.mock("react-router", async (importOriginal) => {
+  const original = await importOriginal<typeof import("react-router")>();
+  return {
+    ...original,
+    useNavigate: () => mockNavigate,
+  };
+});
+
+const mockUseConfig = vi.fn();
+const mockUseOnboardingStatus = vi.fn();
+
+vi.mock("#/hooks/query/use-config", () => ({
+  useConfig: () => mockUseConfig(),
+}));
+
+vi.mock("#/hooks/query/use-onboarding-status", () => ({
+  useOnboardingStatus: () => mockUseOnboardingStatus(),
+}));
+
+const renderGuardAt = (initialEntry: string) => {
+  const queryClient = new QueryClient({
+    defaultOptions: { queries: { retry: false } },
+  });
+
+  const Stub = createRoutesStub([
+    {
+      path: "*",
+      Component: () => (
+        <OnboardingGuard>
+          <div data-testid="children" />
+        </OnboardingGuard>
+      ),
+    },
+  ]);
+
+  return render(
+    <QueryClientProvider client={queryClient}>
+      <Stub initialEntries={[initialEntry]} />
+    </QueryClientProvider>,
+  );
+};
+
+describe("OnboardingGuard returnTo preservation", () => {
+  beforeEach(() => {
+    mockNavigate.mockReset();
+    mockUseConfig.mockReturnValue({
+      data: { feature_flags: { enable_onboarding: true } },
+    });
+    mockUseOnboardingStatus.mockReturnValue({
+      data: { should_complete_onboarding: true },
+      isLoading: false,
+    });
+  });
+
+  it("preserves the originally requested path as a returnTo query parameter", async () => {
+    renderGuardAt("/conversations/abc-123");
+
+    await vi.waitFor(() => {
+      expect(mockNavigate).toHaveBeenCalledWith(
+        `/onboarding?returnTo=${encodeURIComponent("/conversations/abc-123")}`,
+        { replace: true },
+      );
+    });
+  });
+
+  it("preserves search params alongside the path in returnTo", async () => {
+    renderGuardAt("/conversations/abc?foo=bar&baz=qux");
+
+    await vi.waitFor(() => {
+      expect(mockNavigate).toHaveBeenCalledWith(
+        `/onboarding?returnTo=${encodeURIComponent("/conversations/abc?foo=bar&baz=qux")}`,
+        { replace: true },
+      );
+    });
+  });
+
+  it("does not double-encode pre-encoded characters in search params", async () => {
+    // ``search`` from useLocation() is already percent-encoded. Without
+    // decoding it first, ``encodeURIComponent`` would re-encode the ``%``
+    // as ``%25``, causing double-encoding (e.g. ``%20`` → ``%2520``).
+    // With the fix, ``%20`` in the original search is decoded to a space
+    // and then re-encoded once, yielding ``%20`` (not ``%2520``) in the
+    // ``returnTo`` parameter.
+    renderGuardAt("/conversations/abc?tab=user%20profile");
+
+    await vi.waitFor(() => {
+      expect(mockNavigate).toHaveBeenCalledWith(
+        `/onboarding?returnTo=${encodeURIComponent("/conversations/abc?tab=user profile")}`,
+        { replace: true },
+      );
+    });
+  });
+
+  it("does not append a returnTo when the originally requested path is /", async () => {
+    renderGuardAt("/");
+
+    await vi.waitFor(() => {
+      expect(mockNavigate).toHaveBeenCalledWith("/onboarding", {
+        replace: true,
+      });
+    });
+  });
+
+  it("does not redirect when already on /onboarding", async () => {
+    renderGuardAt("/onboarding");
+
+    // Allow the effect to settle.
+    await new Promise((resolve) => {
+      setTimeout(resolve, 0);
+    });
+
+    expect(mockNavigate).not.toHaveBeenCalled();
+  });
+
+  it("does not redirect when the enable_onboarding feature flag is off", async () => {
+    mockUseConfig.mockReturnValue({
+      data: { feature_flags: { enable_onboarding: false } },
+    });
+
+    renderGuardAt("/conversations/abc");
+
+    await new Promise((resolve) => {
+      setTimeout(resolve, 0);
+    });
+
+    expect(mockNavigate).not.toHaveBeenCalled();
+  });
+
+  it("does not redirect when onboarding is already complete", async () => {
+    mockUseOnboardingStatus.mockReturnValue({
+      data: { should_complete_onboarding: false },
+      isLoading: false,
+    });
+
+    renderGuardAt("/conversations/abc");
+
+    await new Promise((resolve) => {
+      setTimeout(resolve, 0);
+    });
+
+    expect(mockNavigate).not.toHaveBeenCalled();
+  });
+});

frontend/__tests__/components/features/onboarding/onboarding-form.test.tsx

@@ -5,7 +5,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
 import { I18nextProvider } from "react-i18next";
 import i18n from "i18next";
-import OnboardingForm, { clientLoader } from "#/routes/onboarding-form";
+import OnboardingForm, {
+  clientLoader,
+  sanitizeReturnTo,
+} from "#/routes/onboarding-form";
 import AuthService from "#/api/auth-service/auth-service.api";
 import { onboardingService } from "#/api/onboarding-service/onboarding-service.api";
 
@@ -14,7 +17,9 @@ const mockNavigate = vi.fn();
 const mockUseMe = vi.fn();
 
 // Loader data set in beforeEach for each test suite
-let loaderData: { config: { app_mode: string; feature_flags: { deployment_mode: string } } };
+let loaderData: {
+  config: { app_mode: string; feature_flags: { deployment_mode: string } };
+};
 
 vi.mock("react-router", async (importOriginal) => {
   const original = await importOriginal<typeof import("react-router")>();
@@ -51,9 +56,7 @@ vi.mock("#/api/option-service/option-service.api", () => ({
   },
 }));
 
-
-
-const renderOnboardingForm = async () => {
+const renderOnboardingForm = async (initialEntry: string = "/") => {
   const queryClient = new QueryClient({
     defaultOptions: { queries: { retry: false } },
   });
@@ -69,7 +72,7 @@ const renderOnboardingForm = async () => {
   const result = render(
     <I18nextProvider i18n={i18n}>
       <QueryClientProvider client={queryClient}>
-        <RouterStub initialEntries={["/"]} />
+        <RouterStub initialEntries={[initialEntry]} />
       </QueryClientProvider>
     </I18nextProvider>,
   );
@@ -177,13 +180,15 @@ describe("OnboardingForm - Cloud Mode", () => {
     await user.click(screen.getByRole("button", { name: /finish/i }));
 
     expect(mockMutate).toHaveBeenCalledTimes(1);
-    expect(mockMutate).toHaveBeenCalledWith({
-      selections: {
-        org_size: "org_2_10",
-        use_case: ["new_features"],
-        role: "software_engineer",
-      },
-    });
+    expect(mockMutate).toHaveBeenCalledWith(
+      expect.objectContaining({
+        selections: {
+          org_size: "org_2_10",
+          use_case: ["new_features"],
+          role: "software_engineer",
+        },
+      }),
+    );
   });
 
   it("should render 5 options on step 1 (org size question)", async () => {
@@ -214,13 +219,15 @@ describe("OnboardingForm - Cloud Mode", () => {
     await user.click(screen.getByRole("button", { name: /finish/i }));
 
     // Verify all selections were preserved
-    expect(mockMutate).toHaveBeenCalledWith({
-      selections: {
-        org_size: "solo",
-        use_case: ["fixing_bugs"],
-        role: "cto_founder",
-      },
-    });
+    expect(mockMutate).toHaveBeenCalledWith(
+      expect.objectContaining({
+        selections: {
+          org_size: "solo",
+          use_case: ["fixing_bugs"],
+          role: "cto_founder",
+        },
+      }),
+    );
   });
 
   it("should allow selecting multiple options on multi-select steps", async () => {
@@ -241,13 +248,15 @@ describe("OnboardingForm - Cloud Mode", () => {
     await user.click(screen.getByTestId("step-option-software_engineer"));
     await user.click(screen.getByRole("button", { name: /finish/i }));
 
-    expect(mockMutate).toHaveBeenCalledWith({
-      selections: {
-        org_size: "solo",
-        use_case: ["new_features", "fixing_bugs", "refactoring"],
-        role: "software_engineer",
-      },
-    });
+    expect(mockMutate).toHaveBeenCalledWith(
+      expect.objectContaining({
+        selections: {
+          org_size: "solo",
+          use_case: ["new_features", "fixing_bugs", "refactoring"],
+          role: "software_engineer",
+        },
+      }),
+    );
   });
 
   it("should allow deselecting options on multi-select steps", async () => {
@@ -269,13 +278,15 @@ describe("OnboardingForm - Cloud Mode", () => {
     await user.click(screen.getByTestId("step-option-software_engineer"));
     await user.click(screen.getByRole("button", { name: /finish/i }));
 
-    expect(mockMutate).toHaveBeenCalledWith({
-      selections: {
-        org_size: "solo",
-        use_case: ["fixing_bugs"],
-        role: "software_engineer",
-      },
-    });
+    expect(mockMutate).toHaveBeenCalledWith(
+      expect.objectContaining({
+        selections: {
+          org_size: "solo",
+          use_case: ["fixing_bugs"],
+          role: "software_engineer",
+        },
+      }),
+    );
   });
 
   it("should show all progress bars filled on the last step", async () => {
@@ -397,14 +408,16 @@ describe("OnboardingForm - Self-Hosted Mode", () => {
     await user.click(screen.getByRole("button", { name: /finish/i }));
 
     expect(mockMutate).toHaveBeenCalledTimes(1);
-    expect(mockMutate).toHaveBeenCalledWith({
-      selections: {
-        org_name: "Acme Corp",
-        org_domain: "acme.com",
-        org_size: "org_2_10",
-        use_case: ["new_features"],
-      },
-    });
+    expect(mockMutate).toHaveBeenCalledWith(
+      expect.objectContaining({
+        selections: {
+          org_name: "Acme Corp",
+          org_domain: "acme.com",
+          org_size: "org_2_10",
+          use_case: ["new_features"],
+        },
+      }),
+    );
   });
 
   it("should show all 3 progress bars filled on the last step", async () => {
@@ -446,7 +459,6 @@ describe("OnboardingForm - Self-Hosted Mode", () => {
     const nextButton = screen.getByRole("button", { name: /next/i });
     expect(nextButton).not.toBeDisabled();
   });
-
 });
 
 describe("OnboardingForm - redirect when already onboarded", () => {
@@ -481,9 +493,98 @@ describe("OnboardingForm - redirect when already onboarded", () => {
       expect(mockNavigate).toHaveBeenCalledWith("/", { replace: true });
     });
   });
+
+  it("should restore the returnTo destination when onboarding is already complete", async () => {
+    // Regression: a stale ``/onboarding`` link must still respect
+    // a ``returnTo`` query param so post-login deep links survive.
+    vi.spyOn(onboardingService, "getStatus").mockResolvedValue({
+      should_complete_onboarding: false,
+    });
+
+    await renderOnboardingForm(
+      `/?returnTo=${encodeURIComponent("/conversations/abc?foo=bar")}`,
+    );
+
+    await vi.waitFor(() => {
+      expect(mockNavigate).toHaveBeenCalledWith("/conversations/abc?foo=bar", {
+        replace: true,
+      });
+    });
+    expect(mockNavigate).not.toHaveBeenCalledWith("/", { replace: true });
+  });
+
+  it("should reject absolute URL returnTo and redirect to / when onboarding is already complete", async () => {
+    // Security: a hand-crafted ``?returnTo=https://evil.example`` must
+    // never turn the component redirect into an open-redirect vector.
+    vi.spyOn(onboardingService, "getStatus").mockResolvedValue({
+      should_complete_onboarding: false,
+    });
+
+    await renderOnboardingForm(
+      `/?returnTo=${encodeURIComponent("https://evil.example.com/pwn")}`,
+    );
+
+    await vi.waitFor(() => {
+      expect(mockNavigate).toHaveBeenCalledWith("/", { replace: true });
+    });
+  });
+
+  it("should reject protocol-relative returnTo and redirect to / when onboarding is already complete", async () => {
+    // Security: protocol-relative URLs (``//evil.example.com``) are
+    // also open-redirect vectors and must be rejected.
+    vi.spyOn(onboardingService, "getStatus").mockResolvedValue({
+      should_complete_onboarding: false,
+    });
+
+    await renderOnboardingForm(
+      `/?returnTo=${encodeURIComponent("//evil.example.com/pwn")}`,
+    );
+
+    await vi.waitFor(() => {
+      expect(mockNavigate).toHaveBeenCalledWith("/", { replace: true });
+    });
+  });
+
+  it("should forward returnTo to submitOnboarding so the post-submit redirect respects it", async () => {
+    // Regression: ``OnboardingGuard`` saves the originally requested
+    // URL as ``?returnTo=...``. ``OnboardingForm`` must thread that
+    // value through the submit mutation so the post-submit fallback
+    // (when the server response has no ``redirect_url``) sends the
+    // user back to where they started.
+    const user = userEvent.setup();
+    await renderOnboardingForm(
+      `/?returnTo=${encodeURIComponent("/conversations/abc?foo=bar")}`,
+    );
+
+    // Step 1 - org size
+    await user.click(screen.getByTestId("step-option-solo"));
+    await user.click(screen.getByRole("button", { name: /next/i }));
+
+    // Step 2 - use case
+    await user.click(screen.getByTestId("step-option-new_features"));
+    await user.click(screen.getByRole("button", { name: /next/i }));
+
+    // Step 3 - role
+    await user.click(screen.getByTestId("step-option-software_engineer"));
+    await user.click(screen.getByRole("button", { name: /finish/i }));
+
+    expect(mockMutate).toHaveBeenCalledWith(
+      expect.objectContaining({
+        returnTo: "/conversations/abc?foo=bar",
+      }),
+    );
+  });
 });
 
 describe("onboarding-form clientLoader", () => {
+  // The loader takes a ``{ request }`` arg from react-router. Most of
+  // the existing tests don't care about the URL, so build a default
+  // request pointing at bare ``/onboarding`` and let returnTo-aware
+  // tests override it.
+  const makeArgs = (url = "https://app.example.com/onboarding") => ({
+    request: new Request(url),
+  });
+
   beforeEach(() => {
     mockQueryClientGetData.mockReset();
     mockQueryClientSetData.mockReset();
@@ -498,7 +599,7 @@ describe("onboarding-form clientLoader", () => {
       };
       mockQueryClientGetData.mockReturnValue(saasConfig);
 
-      const result = await clientLoader();
+      const result = await clientLoader(makeArgs());
 
       expect(result).toBeDefined();
       expect((result as Response).status).toBe(302);
@@ -512,7 +613,7 @@ describe("onboarding-form clientLoader", () => {
       };
       mockQueryClientGetData.mockReturnValue(ossConfig);
 
-      const result = await clientLoader();
+      const result = await clientLoader(makeArgs());
 
       expect(result).toBeDefined();
       expect((result as Response).status).toBe(302);
@@ -526,7 +627,7 @@ describe("onboarding-form clientLoader", () => {
       };
       mockQueryClientGetData.mockReturnValue(undefinedConfig);
 
-      const result = await clientLoader();
+      const result = await clientLoader(makeArgs());
 
       expect(result).toBeDefined();
       expect((result as Response).status).toBe(302);
@@ -537,7 +638,7 @@ describe("onboarding-form clientLoader", () => {
       mockQueryClientGetData.mockReturnValue(null);
       mockGetConfig.mockResolvedValue(null);
 
-      const result = await clientLoader();
+      const result = await clientLoader(makeArgs());
 
       expect(result).toBeDefined();
       expect((result as Response).status).toBe(302);
@@ -551,7 +652,7 @@ describe("onboarding-form clientLoader", () => {
       };
       mockQueryClientGetData.mockReturnValue(saasCloudConfig);
 
-      const result = await clientLoader();
+      const result = await clientLoader(makeArgs());
 
       expect(result).toEqual({ config: saasCloudConfig });
     });
@@ -559,16 +660,116 @@ describe("onboarding-form clientLoader", () => {
     it("should allow access and return config when app_mode is saas with self_hosted deployment and enable_onboarding is true", async () => {
       const saasSelfHostedConfig = {
         app_mode: "saas",
-        feature_flags: { deployment_mode: "self_hosted", enable_onboarding: true },
+        feature_flags: {
+          deployment_mode: "self_hosted",
+          enable_onboarding: true,
+        },
       };
       mockQueryClientGetData.mockReturnValue(saasSelfHostedConfig);
 
-      const result = await clientLoader();
+      const result = await clientLoader(makeArgs());
 
       expect(result).toEqual({ config: saasSelfHostedConfig });
     });
   });
 
+  describe("returnTo handling on redirect", () => {
+    // The frontend can disagree with the backend about whether
+    // onboarding applies (e.g. the backend gates on
+    // ``DEPLOYMENT_MODE='cloud'`` while the frontend gates on
+    // ``feature_flags.enable_onboarding``). When the frontend
+    // redirects users away from /onboarding because the flag is
+    // off, it must still honor the ``?returnTo=`` query parameter
+    // so deep links survive the disagreement.
+    it("should honor returnTo when enable_onboarding is false", async () => {
+      const saasConfig = {
+        app_mode: "saas",
+        feature_flags: { deployment_mode: "cloud", enable_onboarding: false },
+      };
+      mockQueryClientGetData.mockReturnValue(saasConfig);
+
+      const result = await clientLoader(
+        makeArgs(
+          "https://app.example.com/onboarding?returnTo=%2Fsettings%2Fuser",
+        ),
+      );
+
+      expect((result as Response).status).toBe(302);
+      expect((result as Response).headers.get("Location")).toBe(
+        "/settings/user",
+      );
+    });
+
+    it("should honor returnTo with query string when app_mode is oss", async () => {
+      const ossConfig = {
+        app_mode: "oss",
+        feature_flags: { deployment_mode: undefined, enable_onboarding: true },
+      };
+      mockQueryClientGetData.mockReturnValue(ossConfig);
+
+      const result = await clientLoader(
+        makeArgs(
+          "https://app.example.com/onboarding" +
+            "?returnTo=%2Fconversations%2Fabc%3Ffoo%3Dbar",
+        ),
+      );
+
+      expect((result as Response).headers.get("Location")).toBe(
+        "/conversations/abc?foo=bar",
+      );
+    });
+
+    it("should reject absolute URL returnTo and fall back to /", async () => {
+      // Safety: never let a hand-crafted ``?returnTo=https://evil.example``
+      // turn the loader's redirect into an open-redirect vector.
+      const ossConfig = {
+        app_mode: "oss",
+        feature_flags: { deployment_mode: undefined, enable_onboarding: true },
+      };
+      mockQueryClientGetData.mockReturnValue(ossConfig);
+
+      const result = await clientLoader(
+        makeArgs(
+          "https://app.example.com/onboarding" +
+            "?returnTo=https%3A%2F%2Fevil.example.com%2Fpwn",
+        ),
+      );
+
+      expect((result as Response).headers.get("Location")).toBe("/");
+    });
+
+    it("should reject protocol-relative returnTo and fall back to /", async () => {
+      const ossConfig = {
+        app_mode: "oss",
+        feature_flags: { deployment_mode: undefined, enable_onboarding: true },
+      };
+      mockQueryClientGetData.mockReturnValue(ossConfig);
+
+      const result = await clientLoader(
+        makeArgs(
+          "https://app.example.com/onboarding" +
+            "?returnTo=%2F%2Fevil.example.com%2Fpwn",
+        ),
+      );
+
+      expect((result as Response).headers.get("Location")).toBe("/");
+    });
+
+    it("should fall back to / when returnTo is missing", async () => {
+      const ossConfig = {
+        app_mode: "oss",
+        feature_flags: { deployment_mode: undefined, enable_onboarding: true },
+      };
+      mockQueryClientGetData.mockReturnValue(ossConfig);
+
+      const result = await clientLoader(
+        makeArgs("https://app.example.com/onboarding"),
+      );
+
+      expect((result as Response).headers.get("Location")).toBe("/");
+    });
+  });
+
   describe("config fetching", () => {
     it("should use cached config from queryClient when available", async () => {
       const cachedConfig = {
@@ -577,9 +778,11 @@ describe("onboarding-form clientLoader", () => {
       };
       mockQueryClientGetData.mockReturnValue(cachedConfig);
 
-      await clientLoader();
+      await clientLoader(makeArgs());
 
-      expect(mockQueryClientGetData).toHaveBeenCalledWith(["web-client-config"]);
+      expect(mockQueryClientGetData).toHaveBeenCalledWith([
+        "web-client-config",
+      ]);
       expect(mockGetConfig).not.toHaveBeenCalled();
     });
 
@@ -591,7 +794,7 @@ describe("onboarding-form clientLoader", () => {
       mockQueryClientGetData.mockReturnValue(null);
       mockGetConfig.mockResolvedValue(fetchedConfig);
 
-      const result = await clientLoader();
+      const result = await clientLoader(makeArgs());
 
       expect(mockGetConfig).toHaveBeenCalled();
       expect(mockQueryClientSetData).toHaveBeenCalledWith(
@@ -602,3 +805,39 @@ describe("onboarding-form clientLoader", () => {
     });
   });
 });
+
+describe("sanitizeReturnTo", () => {
+  it("should return / for null", () => {
+    expect(sanitizeReturnTo(null)).toBe("/");
+  });
+
+  it("should return / for empty string", () => {
+    expect(sanitizeReturnTo("")).toBe("/");
+  });
+
+  it("should allow same-origin absolute paths", () => {
+    expect(sanitizeReturnTo("/conversations/abc")).toBe("/conversations/abc");
+  });
+
+  it("should allow paths with query strings", () => {
+    expect(sanitizeReturnTo("/conversations/abc?foo=bar")).toBe(
+      "/conversations/abc?foo=bar",
+    );
+  });
+
+  it("should prepend / to relative paths that lack one", () => {
+    expect(sanitizeReturnTo("conversations/abc")).toBe("/conversations/abc");
+  });
+
+  it("should reject http:// URLs and fall back to /", () => {
+    expect(sanitizeReturnTo("http://evil.example.com/pwn")).toBe("/");
+  });
+
+  it("should reject https:// URLs and fall back to /", () => {
+    expect(sanitizeReturnTo("https://evil.example.com/pwn")).toBe("/");
+  });
+
+  it("should reject protocol-relative URLs and fall back to /", () => {
+    expect(sanitizeReturnTo("//evil.example.com/pwn")).toBe("/");
+  });
+});

frontend/src/components/features/guards/onboarding-guard.tsx

@@ -6,12 +6,19 @@ import { useConfig } from "#/hooks/query/use-config";
 /**
  * Forces SaaS users with incomplete onboarding to /onboarding before they can
  * access any protected route. Mirrors EmailVerificationGuard.
+ *
+ * The originally requested URL is preserved as a ``returnTo`` query
+ * parameter on ``/onboarding`` so it can be restored once the user
+ * completes the onboarding flow. Without this, post-login deep links
+ * (set up by ``root-layout.tsx`` as ``/login?returnTo=...``) get
+ * dropped at the onboarding interstitial and the user always lands
+ * back on ``/``.
  */
 export function OnboardingGuard({ children }: { children: React.ReactNode }) {
   const { data, isLoading } = useOnboardingStatus();
   const { data: config } = useConfig();
   const navigate = useNavigate();
-  const { pathname } = useLocation();
+  const { pathname, search } = useLocation();
 
   React.useEffect(() => {
     if (isLoading) return;
@@ -21,13 +28,33 @@ export function OnboardingGuard({ children }: { children: React.ReactNode }) {
       data?.should_complete_onboarding &&
       pathname !== "/onboarding"
     ) {
-      navigate("/onboarding", { replace: true });
+      // Preserve the user's originally requested URL (path + query)
+      // so OnboardingForm can restore it after the user finishes.
+      // Skip the trivial ``/`` case to keep the URL clean — that is
+      // already the default landing page after onboarding.
+      let destination = "/onboarding";
+      if (pathname !== "/") {
+        // Decode ``search`` first so already-encoded characters (e.g.
+        // ``%20``) are not double-encoded by ``encodeURIComponent``.
+        // The try/catch handles malformed percent-encoding sequences by
+        // falling back to the raw ``search`` string.
+        let decodedSearch: string;
+        try {
+          decodedSearch = search ? decodeURIComponent(search) : "";
+        } catch {
+          decodedSearch = search;
+        }
+        const returnTo = `${pathname}${decodedSearch}`;
+        destination = `/onboarding?returnTo=${encodeURIComponent(returnTo)}`;
+      }
+      navigate(destination, { replace: true });
     }
   }, [
     config?.feature_flags?.enable_onboarding,
     data?.should_complete_onboarding,
     isLoading,
     pathname,
+    search,
     navigate,
   ]);
 

frontend/src/hooks/mutation/use-submit-onboarding.ts

@@ -6,6 +6,13 @@ import { displayErrorToast } from "#/utils/custom-toast-handlers";
 
 type SubmitOnboardingArgs = {
   selections: Record<string, string | string[]>;
+  /**
+   * Fallback destination to navigate to when the server response does
+   * not include a ``redirect_url``. ``OnboardingForm`` passes the
+   * caller's ``returnTo`` here so deep links survive the onboarding
+   * interstitial. Defaults to ``/`` when omitted.
+   */
+  returnTo?: string;
 };
 
 interface OnboardingResponse {
@@ -25,11 +32,11 @@ export const useSubmitOnboarding = () => {
       );
       return data;
     },
-    onSuccess: (data) => {
+    onSuccess: (data, { returnTo }) => {
       queryClient.invalidateQueries({ queryKey: SETTINGS_QUERY_KEYS.all });
       queryClient.invalidateQueries({ queryKey: ["onboarding-status"] });
 
-      const finalRedirectUrl = data.redirect_url || "/";
+      const finalRedirectUrl = data.redirect_url || returnTo || "/";
       // Check if the redirect URL is an external URL (starts with http or https)
       if (
         finalRedirectUrl.startsWith("http://") ||

frontend/src/routes/onboarding-form.tsx

@@ -1,6 +1,11 @@
 import React from "react";
 import { useTranslation } from "react-i18next";
-import { useNavigate, redirect, useLoaderData } from "react-router";
+import {
+  useNavigate,
+  redirect,
+  useLoaderData,
+  useSearchParams,
+} from "react-router";
 import StepHeader from "#/components/features/onboarding/step-header";
 import { StepContent } from "#/components/features/onboarding/step-content";
 import { BrandButton } from "#/components/features/settings/brand-button";
@@ -21,22 +26,68 @@ import {
 import { queryClient } from "#/query-client-config";
 import OptionService from "#/api/option-service/option-service.api";
 
-export const clientLoader = async () => {
+/**
+ * Sanitize a raw ``returnTo`` value, returning a safe same-origin path.
+ *
+ * Absolute URLs and protocol-relative URLs fall back to ``"/"`` to
+ * prevent open-redirect attacks. Relative paths that don't start with
+ * ``"/"`` are prepended with one.
+ */
+export function sanitizeReturnTo(raw: string | null): string {
+  if (!raw) return "/";
+  // Same-origin paths only — reject protocol-bearing or
+  // protocol-relative targets.
+  if (
+    raw.startsWith("http://") ||
+    raw.startsWith("https://") ||
+    raw.startsWith("//")
+  ) {
+    return "/";
+  }
+  return raw.startsWith("/") ? raw : `/${raw}`;
+}
+
+/**
+ * Compute a safe redirect target from the URL's ``returnTo`` query
+ * parameter, defaulting to ``"/"``.
+ *
+ * This loader intentionally redirects same-origin only — absolute URLs
+ * are dropped to ``"/"`` rather than being followed, since the loader
+ * This loader intentionally redirects to same-origin paths only. Absolute
+ * URLs are silently dropped to "/" because a loader's `redirect()` performs
+ * a client-side navigation — following an external URL would break the SPA.
+ * URL here would break the SPA.
+ */
+function safeReturnToFromRequest(request: Request): string {
+  const url = new URL(request.url);
+  return sanitizeReturnTo(url.searchParams.get("returnTo"));
+}
+
+export const clientLoader = async ({ request }: { request: Request }) => {
   let config = queryClient.getQueryData<WebClientConfig>(["web-client-config"]);
   if (!config) {
     config = await OptionService.getConfig();
     queryClient.setQueryData<WebClientConfig>(["web-client-config"], config);
   }
 
+  // The deployment's frontend may have ``enable_onboarding=false`` or
+  // a non-SaaS ``app_mode`` even when the backend OAuth callback has
+  // just sent the user here (the backend gates on ``DEPLOYMENT_MODE``,
+  // not on the frontend feature flag, so the two can disagree). When
+  // we redirect away in those cases, honor the user's deep-link
+  // ``?returnTo=`` so they don't lose their original destination at
+  // the onboarding interstitial.
+  const fallback = safeReturnToFromRequest(request);
+
   // Check server feature flag to block access
   if (!config?.feature_flags?.enable_onboarding) {
-    return redirect("/");
+    return redirect(fallback);
   }
 
   // Only allow access to onboarding for SaaS mode (cloud or self-hosted)
   // OSS users should never reach /onboarding
   if (config?.app_mode !== "saas") {
-    return redirect("/");
+    return redirect(fallback);
   }
 
   return { config };
@@ -85,6 +136,12 @@ function OnboardingForm() {
   const navigate = useNavigate();
   const loaderData = useLoaderData<typeof clientLoader>();
   const config = loaderData?.config;
+  const [searchParams] = useSearchParams();
+  // ``OnboardingGuard`` forwards the user's originally requested URL
+  // here so we can restore it after they finish the form. Sanitize to
+  // prevent open-redirect attacks — absolute/protocol-relative URLs
+  // fall back to ``"/"``.
+  const returnTo = sanitizeReturnTo(searchParams.get("returnTo"));
   const { data: onboardingStatus, isLoading: isOnboardingStatusLoading } =
     useOnboardingStatus();
   const { mutate: submitOnboarding } = useSubmitOnboarding();
@@ -92,12 +149,17 @@ function OnboardingForm() {
   React.useEffect(() => {
     if (isOnboardingStatusLoading) return;
     if (onboardingStatus?.should_complete_onboarding === false) {
-      navigate("/", { replace: true });
+      // Honor returnTo if the user already completed onboarding so a
+      // stale ``/onboarding`` link still respects their deep-link
+      // destination. ``returnTo`` is already sanitized above so it is
+      // always a safe same-origin path.
+      navigate(returnTo, { replace: true });
     }
   }, [
     onboardingStatus?.should_complete_onboarding,
     isOnboardingStatusLoading,
     navigate,
+    returnTo,
   ]);
 
   const onboardingAppMode: OnboardingAppMode = getOnboardingAppMode(
@@ -181,7 +243,7 @@ function OnboardingForm() {
 
   const handleNext = () => {
     if (isLastStep) {
-      submitOnboarding({ selections: answers });
+      submitOnboarding({ selections: answers, returnTo });
     } else {
       setCurrentStepIndex((prev) => prev + 1);
     }