diff --git a/web/e2e/auth-redirect.spec.ts b/web/e2e/auth-redirect.spec.ts index aaf581805..6be503eb5 100644 --- a/web/e2e/auth-redirect.spec.ts +++ b/web/e2e/auth-redirect.spec.ts @@ -1,5 +1,11 @@ import { expect, test } from "@playwright/test"; -import { getAuthCallbackUrl } from "../src/lib/utils"; +import { + authCallbackDestination, + getAuthCallbackUrl, + safeRedirectPath, + signInCallbackQuery, + signInHrefFor, +} from "../src/lib/utils"; test("uses the configured site URL for production auth callbacks", () => { expect( @@ -22,3 +28,98 @@ test("uses the browser origin while developing locally", () => { }), ).toBe("http://localhost:3001/auth/callback?type=recovery"); }); + +test.describe("safeRedirectPath", () => { + for (const path of [ + "/dashboard", + "/dashboard?tab=integrations", + "/integrations/github/callback?installation_id=1&state=abc", + "/docs#install", + ]) { + test(`keeps the same-origin path ${path}`, () => { + expect(safeRedirectPath(path)).toBe(path); + }); + } + + for (const target of [ + null, + undefined, + "", + "dashboard", + "//evil.com", + "//evil.com/dashboard", + "/\\evil.com", + "\\\\evil.com", + "/\t/evil.com", + "/\n/evil.com", + "https://evil.com", + "https://treq.dev/dashboard", + "javascript:alert(1)", + " /dashboard", + ]) { + test(`falls back to /dashboard for ${JSON.stringify(target)}`, () => { + expect(safeRedirectPath(target)).toBe("/dashboard"); + }); + } +}); + +test.describe("signInCallbackQuery", () => { + test("carries a same-origin redirect to the callback", () => { + expect(signInCallbackQuery("", "/dashboard?tab=integrations")).toBe( + `redirect=${encodeURIComponent("/dashboard?tab=integrations")}`, + ); + }); + + for (const target of ["//evil.com", "https://evil.com"]) { + test(`replaces the unsafe redirect ${target} with /dashboard`, () => { + expect(signInCallbackQuery("", target)).toBe("redirect=%2Fdashboard"); + }); + } + + test("keeps the desktop callback query unchanged", () => { + expect(signInCallbackQuery("desktop", null)).toBe("source=desktop"); + }); + + test("adds nothing when no redirect was requested", () => { + expect(signInCallbackQuery("", null)).toBeUndefined(); + }); +}); + +test.describe("authCallbackDestination", () => { + test("sends web sign-ins to the redirect target", () => { + const params = new URLSearchParams({ redirect: "/dashboard?tab=integrations" }); + expect(authCallbackDestination(params)).toEqual({ + kind: "web", + path: "/dashboard?tab=integrations", + }); + }); + + test("ignores an off-site redirect", () => { + const params = new URLSearchParams({ redirect: "//evil.com" }); + expect(authCallbackDestination(params)).toEqual({ kind: "web", path: "/dashboard" }); + }); + + test("hands desktop sign-ins a token", () => { + const params = new URLSearchParams({ source: "desktop", redirect: "/dashboard" }); + expect(authCallbackDestination(params)).toEqual({ kind: "desktop" }); + }); +}); + +test("signInHrefFor returns the user to the page they were on", () => { + expect(signInHrefFor("/integrations/github/callback?installation_id=1&state=abc")).toBe( + `/sign-in?redirect=${encodeURIComponent("/integrations/github/callback?installation_id=1&state=abc")}`, + ); +}); + +test("the navbar sign-in link sends GitHub OAuth back to /auth/callback", async ({ page }) => { + await page.route("**/auth/v1/authorize**", (route) => route.abort()); + await page.goto("/"); + await page.getByRole("link", { name: "Sign in" }).first().click(); + const authorize = page.waitForRequest("**/auth/v1/authorize**"); + await page.getByRole("button", { name: "Continue with GitHub" }).click(); + const redirectTo = new URL((await authorize).url()).searchParams.get("redirect_to"); + expect(redirectTo).not.toBeNull(); + const url = new URL(redirectTo!); + expect(url.pathname).toBe("/auth/callback"); + expect(url.search).toBe(""); +}); diff --git a/web/src/lib/utils.ts b/web/src/lib/utils.ts index 7b785a56f..e30d54472 100644 --- a/web/src/lib/utils.ts +++ b/web/src/lib/utils.ts @@ -22,3 +22,60 @@ export function getAuthCallbackUrl({ const origin = (isProduction ? siteUrl : browserOrigin).replace(/\/$/, ""); return `${origin}/auth/callback${query ? `?${query}` : ""}`; } + +const DEFAULT_REDIRECT_PATH = "/dashboard"; + +const REDIRECT_CHECK_ORIGIN = "https://redirect-check.invalid"; + +/** + * Return `target` as a path on this site, or `/dashboard` when it is not one. + * Only a path with one leading `/` passes, so `//host`, `https:` and + * `javascript:` targets cannot send a user off the site after sign-in. + */ +export function safeRedirectPath(target: string | null | undefined): string { + if ( + !target || + !target.startsWith("/") || + target.startsWith("//") || + target.includes("\\") || + // Browsers drop tabs and newlines from URLs, so "/\t/evil.com" would + // become "//evil.com". + /[\u0000-\u001f\u007f]/.test(target) + ) { + return DEFAULT_REDIRECT_PATH; + } + // Resolve against a fixed origin as a final check that the host cannot change. + const url = new URL(target, REDIRECT_CHECK_ORIGIN); + if (url.origin !== REDIRECT_CHECK_ORIGIN) return DEFAULT_REDIRECT_PATH; + return `${url.pathname}${url.search}${url.hash}`; +} + +/** + * Query string /sign-in adds to the OAuth callback URL. `redirect` is only + * carried when the caller asked for one, and it is always the checked path. + */ +export function signInCallbackQuery( + source: string, + requestedRedirect: string | null, +): string | undefined { + const query = new URLSearchParams(); + if (source) query.set("source", source); + if (requestedRedirect !== null) query.set("redirect", safeRedirectPath(requestedRedirect)); + return query.toString() || undefined; +} + +/** + * Where /auth/callback sends a signed-in user. Only the desktop app needs a + * one-time token, so web sign-ins go straight to the checked redirect path. + */ +export function authCallbackDestination( + params: URLSearchParams, +): { kind: "desktop" } | { kind: "web"; path: string } { + if (params.get("source") === "desktop") return { kind: "desktop" }; + return { kind: "web", path: safeRedirectPath(params.get("redirect")) }; +} + +/** Sign-in link that returns the user to `pathAndSearch` afterwards. */ +export function signInHrefFor(pathAndSearch: string): string { + return `/sign-in?redirect=${encodeURIComponent(pathAndSearch)}`; +} diff --git a/web/src/pages/auth/callback.tsx b/web/src/pages/auth/callback.tsx index 243706412..5845fb25a 100644 --- a/web/src/pages/auth/callback.tsx +++ b/web/src/pages/auth/callback.tsx @@ -4,6 +4,7 @@ import Head from "@docusaurus/Head"; import BrowserOnly from "@docusaurus/BrowserOnly"; import useDocusaurusContext from "@docusaurus/useDocusaurusContext"; import { supabase } from "../../lib/supabase"; +import { authCallbackDestination } from "../../lib/utils"; function CallbackContent() { const [status, setStatus] = useState<"loading" | "desktop" | "error">("loading"); @@ -70,6 +71,12 @@ function CallbackContent() { throw new Error("Failed to establish session"); } + const destination = authCallbackDestination(searchParams); + if (destination.kind === "web") { + window.location.replace(destination.path); + return; + } + // Create a one-time token for desktop auth handoff const { data: token, error: tokenError } = await supabase.rpc("create_desktop_token"); if (tokenError || !token) { diff --git a/web/src/pages/integrations/github/callback.tsx b/web/src/pages/integrations/github/callback.tsx index 93c56be98..15af8831f 100644 --- a/web/src/pages/integrations/github/callback.tsx +++ b/web/src/pages/integrations/github/callback.tsx @@ -3,6 +3,7 @@ import Layout from "@theme/Layout"; import BrowserOnly from "@docusaurus/BrowserOnly"; import useDocusaurusContext from "@docusaurus/useDocusaurusContext"; import { supabase } from "../../../lib/supabase"; +import { signInHrefFor } from "../../../lib/utils"; type State = "loading" | "success" | "error" | "unauthenticated"; @@ -105,7 +106,7 @@ function CallbackContent() { <>

You need to be signed in to link the GitHub App.

Sign in diff --git a/web/src/pages/integrations/linear/callback.tsx b/web/src/pages/integrations/linear/callback.tsx index 4e8ea7c26..6b6b64cee 100644 --- a/web/src/pages/integrations/linear/callback.tsx +++ b/web/src/pages/integrations/linear/callback.tsx @@ -3,6 +3,7 @@ import Layout from "@theme/Layout"; import BrowserOnly from "@docusaurus/BrowserOnly"; import useDocusaurusContext from "@docusaurus/useDocusaurusContext"; import { supabase } from "../../../lib/supabase"; +import { signInHrefFor } from "../../../lib/utils"; type State = "loading" | "success" | "error" | "unauthenticated"; @@ -101,7 +102,7 @@ function CallbackContent() { <>

You need to be signed in to link Linear.

 Sign in diff --git a/web/src/pages/sign-in.tsx b/web/src/pages/sign-in.tsx index b8689bfea..ec09bf5a1 100644 --- a/web/src/pages/sign-in.tsx +++ b/web/src/pages/sign-in.tsx @@ -3,7 +3,7 @@ import Layout from "@theme/Layout"; import BrowserOnly from "@docusaurus/BrowserOnly"; import useDocusaurusContext from "@docusaurus/useDocusaurusContext"; import { supabase } from "../lib/supabase"; -import { getAuthCallbackUrl } from "../lib/utils"; +import { getAuthCallbackUrl, safeRedirectPath, signInCallbackQuery } from "../lib/utils"; type View = "sign-in" | "sign-up" | "forgot-password"; @@ -23,6 +23,13 @@ function LoginContent() { const params = new URLSearchParams(window.location.search); const source = params.get("source") || ""; + const requestedRedirect = params.get("redirect"); + const redirectPath = safeRedirectPath(requestedRedirect); + + const goToNextPage = () => { + window.location.href = + source === "desktop" ? "/auth/callback?source=desktop" : redirectPath; + }; // Keep users on sign-in when email signup is disabled useEffect(() => { @@ -34,15 +41,9 @@ function LoginContent() { // Check if already logged in useEffect(() => { supabase.auth.getSession().then(({ data: { session } }) => { - if (session) { - if (source === "desktop") { - window.location.href = `/auth/callback?source=desktop`; - } else { - window.location.href = "/dashboard"; - } - } + if (session) goToNextPage(); }); - }, [source]); + }, [source, redirectPath]); const callbackUrl = (query?: string) => getAuthCallbackUrl({ @@ -51,7 +52,7 @@ function LoginContent() { isProduction: process.env.NODE_ENV === "production", query, }); - const redirectTo = callbackUrl(source ? `source=${encodeURIComponent(source)}` : undefined); + const redirectTo = callbackUrl(signInCallbackQuery(source, requestedRedirect)); const switchView = (v: View) => { if (v === "sign-up" && !emailSignupEnabled) return; @@ -94,11 +95,7 @@ function LoginContent() { if (error) { setError(error.message); } else { - if (source === "desktop") { - window.location.href = `/auth/callback?source=desktop`; - } else { - window.location.href = "/dashboard"; - } + goToNextPage(); } setLoading(false); }; @@ -115,11 +112,7 @@ function LoginContent() { setError(error.message); } else if (data.session) { // Email confirmation disabled — user is signed in immediately - if (source === "desktop") { - window.location.href = `/auth/callback?source=desktop`; - } else { - window.location.href = "/dashboard"; - } + goToNextPage(); } else { // Email confirmation required setMessage("Account created! Check your email to confirm your account, then sign in.");