diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 990ef3cd..1469b129 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -12,17 +12,17 @@ This drives the **whole happy path** through two mock UIs: Only the **LLM** and the **external SaaS HTTP boundaries** are faked. All agent code runs for real. -| Piece | Real or fake | -|---|---| -| Slack webhook → `process_slack_mention` → run dispatch | **real** (`agent.webapp`) | -| `get_agent`, deepagents loop, tools, middleware, prompt | **real** | -| `open_pull_request`, `slack_thread_reply` tools | **real** | -| Sandbox | **real** `local` provider, rooted in a throwaway temp dir | -| Git remote ("GitHub") | **real git**, a local bare repo the agent clones/pushes | -| The LLM | **fake** — a scripted model (`fake_llm.py`) emitting a fixed tool sequence | -| `api.github.com` REST (PR create) | **fake** (`/fake-gh/...`), state rendered at `/mock/github` | -| `slack.com/api` (post message, etc.) | **fake** (`/fake-slack/...`), thread rendered at `/mock/slack` | -| GitHub App token mint, `api.github.com/user` identity | stubbed (offline) | +| Piece | Real or fake | +| ---------------------------------------------------------------- | -------------------------------------------------------------------------- | +| Slack webhook → `process_slack_mention` → run dispatch | **real** (`agent.webapp`) | +| `get_agent`, deepagents loop, tools, middleware, prompt | **real** | +| `open_pull_request`, `slack_thread_reply` tools | **real** | +| Sandbox | **real** `local` provider, rooted in a throwaway temp dir | +| Git remote ("GitHub") | **real git**, a local bare repo the agent clones/pushes | +| The LLM | **fake** — a scripted model (`fake_llm.py`) emitting a fixed tool sequence | +| `api.github.com` REST (PR create) + dashboard GitHub OAuth login | **fake** (`/fake-gh/...`), state rendered at `/mock/github` | +| `slack.com/api` (post message, etc.) | **fake** (`/fake-slack/...`), thread rendered at `/mock/slack` | +| GitHub App token mint, `api.github.com/user` identity | stubbed (offline) | The fake GitHub/Slack stores are the single source of truth the mock UIs render, so what Playwright asserts on is exactly what the real agent produced. diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py index 1147300b..f774d546 100644 --- a/tests/e2e/harness.py +++ b/tests/e2e/harness.py @@ -18,8 +18,10 @@ import json import os import sys import time +from html import escape from pathlib import Path from typing import Any +from urllib.parse import quote sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) @@ -185,29 +187,43 @@ async def control_login_get(login: str = "", email: str = "", next_url: str = "" @app.get("/dashboard/api/auth/login") -async def mock_github_login(redirect_to: str = "", login: str = "") -> Response: - """Mock stand-in for GitHub OAuth: the dashboard's "Continue with GitHub" - button lands here. With no ``login``, render a picker of the fake GitHub - test users; once one is chosen, mint the real session cookie and redirect - back into the dashboard (``redirect_to``).""" +async def mock_github_login(redirect_to: str = "") -> Response: + """E2E stand-in for the dashboard OAuth start route. + + The real route would redirect to github.com. Keep the dashboard-facing URL + intact, then hand off to the fake GitHub simulator so Playwright exercises a + browser login flow instead of test code pre-minting a session cookie. + """ + ui = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/") + dest = redirect_to or (f"{ui}/agents" if ui else "/agents") + return RedirectResponse(f"/fake-gh/login/oauth/authorize?redirect_to={quote(dest)}", 302) + + +@app.get("/fake-gh/login/oauth/authorize") +async def fake_github_authorize(redirect_to: str = "", login: str = "") -> Response: + """Fake GitHub OAuth consent/login page for dashboard e2e tests.""" ui = os.environ.get("DASHBOARD_BASE_URL", "").rstrip("/") dest = redirect_to or (f"{ui}/agents" if ui else "/agents") if not login: options = "".join( - f'' for u in TEST_USERS + f'" + for u in TEST_USERS ) return HTMLResponse( - f"""Continue with GitHub (mock) + f"""GitHub · Authorize open-swe -

Continue with GitHub (mock)

-

Pick a fake GitHub account to sign in as.

-
- - - -
-

Tip: use a separate browser or profile per - user so their sessions don't overwrite each other.

+
+

Authorize open-swe

+

Pick a fake GitHub account to continue.

+
+ + + +
+
""" ) match = next((u for u in TEST_USERS if u["login"] == login), None) diff --git a/tests/e2e/tests/plan_review.spec.ts b/tests/e2e/tests/plan_review.spec.ts index 37d19ffa..e7f92199 100644 --- a/tests/e2e/tests/plan_review.spec.ts +++ b/tests/e2e/tests/plan_review.spec.ts @@ -38,9 +38,14 @@ test.describe("Plan review (HTTP comments)", () => { // 1. A user asks the bot to PLAN something in Slack. await request.post("/control/reset"); const send = await request.post("/mock/slack/send", { - data: { text: "<@U0BOT> plan how to add a greet() helper", mention_bot: true }, + data: { + text: "<@U0BOT> plan how to add a greet() helper", + mention_bot: true, + }, }); - const { thread_id: threadId } = (await send.json()) as { thread_id: string }; + const { thread_id: threadId } = (await send.json()) as { + thread_id: string; + }; expect(threadId).toBeTruthy(); const planPath = `/agents/${threadId}/plan`; @@ -57,7 +62,9 @@ test.describe("Plan review (HTTP comments)", () => { }; return (state.values?.messages ?? []) .map((m) => - typeof m.content === "string" ? m.content : JSON.stringify(m.content), + typeof m.content === "string" + ? m.content + : JSON.stringify(m.content), ) .some((c) => c.includes("Plan mode is active")); }, @@ -67,13 +74,45 @@ test.describe("Plan review (HTTP comments)", () => { // 2. The agent shares the plan-review link, then announces the plan is ready. await expect - .poll(async () => (await botMessages(request)).join("\n"), { timeout: 60_000 }) + .poll(async () => (await botMessages(request)).join("\n"), { + timeout: 60_000, + }) .toMatch(/\/agents\/[^/]+\/plan\b/); await expect - .poll(async () => (await botMessages(request)).join("\n"), { timeout: 60_000 }) + .poll(async () => (await botMessages(request)).join("\n"), { + timeout: 60_000, + }) .toMatch(/ready for review/i); - // 3. The OWNER opens the conversation, follows the "Review plan" banner, and + // 3. A logged-out user follows the plan deep link, signs in through the fake + // GitHub OAuth simulator, and lands back on the same plan page. + const loggedOutCtx = await browser.newContext(); + const loggedOut = await loggedOutCtx.newPage(); + await loggedOut.goto(planPath); + await expect(loggedOut).toHaveURL( + new RegExp(`/login\\?redirect=.*${threadId}.*plan`), + ); + await expect(loggedOut.getByText("Sign in to open-swe")).toBeVisible({ + timeout: 30_000, + }); + await loggedOut.getByRole("link", { name: "Continue with GitHub" }).click(); + await expect(loggedOut).toHaveURL(/\/fake-gh\/login\/oauth\/authorize/); + await expect(loggedOut.getByTestId("fake-github-login")).toBeVisible(); + await loggedOut.getByLabel("GitHub user").selectOption(OWNER.login); + await loggedOut.getByRole("button", { name: "Authorize open-swe" }).click(); + await expect(loggedOut).toHaveURL(new RegExp(`/agents/${threadId}/plan$`)); + await expect(loggedOut.getByTestId("plan-review")).toBeVisible({ + timeout: 30_000, + }); + await expect(loggedOut.getByTestId("plan-document")).toContainText( + "greet", + { + timeout: 30_000, + }, + ); + await loggedOutCtx.close(); + + // 4. The OWNER opens the conversation, follows the "Review plan" banner, and // sees the rendered plan. const ownerCtx = await browser.newContext({ permissions: ["clipboard-read", "clipboard-write"], @@ -85,7 +124,9 @@ test.describe("Plan review (HTTP comments)", () => { await expect(reviewLink).toBeVisible({ timeout: 30_000 }); await reviewLink.click(); await expect(owner).toHaveURL(new RegExp(`/agents/${threadId}/plan$`)); - await expect(owner.getByTestId("plan-review")).toBeVisible({ timeout: 30_000 }); + await expect(owner.getByTestId("plan-review")).toBeVisible({ + timeout: 30_000, + }); await expect(owner.getByText("Back to conversation")).toBeVisible(); await expect(owner.getByTestId("plan-document")).toContainText("greet", { timeout: 30_000, @@ -98,7 +139,9 @@ test.describe("Plan review (HTTP comments)", () => { // Copy the whole plan as markdown. await owner.getByTestId("copy-plan").click(); await expect(owner.getByTestId("copy-plan")).toContainText("Copied!"); - const clipboard = await owner.evaluate(() => navigator.clipboard.readText()); + const clipboard = await owner.evaluate(() => + navigator.clipboard.readText(), + ); expect(clipboard).toContain("## Plan: Add greet() helper"); expect(clipboard).toContain("### Verification"); @@ -107,18 +150,24 @@ test.describe("Plan review (HTTP comments)", () => { await expect(owner.getByTestId("plan-comment")).toHaveCount(1); await expect(owner.getByTestId("reject-plan")).toBeEnabled(); - // 4. A COLLABORATOR opens the same plan: sees it AND the owner's comment + // 5. A COLLABORATOR opens the same plan: sees it AND the owner's comment // (fetched over HTTP), but has NO approve button. const collabCtx = await browser.newContext(); await collabCtx.request.post("/control/login", { data: COLLABORATOR }); const collab = await collabCtx.newPage(); await collab.goto(planPath); - await expect(collab.getByTestId("plan-review")).toBeVisible({ timeout: 30_000 }); + await expect(collab.getByTestId("plan-review")).toBeVisible({ + timeout: 30_000, + }); await expect(collab.getByTestId("plan-document")).toContainText("greet", { timeout: 30_000, }); - await expect(collab.getByTestId("plan-comment")).toHaveCount(1, { timeout: 30_000 }); - await expect(collab.getByTestId("plan-comment")).toContainText("looks solid"); + await expect(collab.getByTestId("plan-comment")).toHaveCount(1, { + timeout: 30_000, + }); + await expect(collab.getByTestId("plan-comment")).toContainText( + "looks solid", + ); await expect(collab.getByTestId("approve-plan")).toHaveCount(0); await expect(collab.getByTestId("reject-plan")).toBeVisible(); @@ -126,21 +175,27 @@ test.describe("Plan review (HTTP comments)", () => { await addComment(collab, "Reviewer: please also add a docstring."); await expect(collab.getByTestId("plan-comment")).toHaveCount(2); - // 5. The owner sees the collaborator's comment (polled), then approves and + // 6. The owner sees the collaborator's comment (polled), then approves and // returns to the main conversation while implementation starts. - await expect(owner.getByTestId("plan-comment")).toHaveCount(2, { timeout: 30_000 }); + await expect(owner.getByTestId("plan-comment")).toHaveCount(2, { + timeout: 30_000, + }); await owner.getByTestId("approve-plan").click(); await expect(owner).toHaveURL(new RegExp(`/agents/${threadId}$`)); - // 6. The agent implements, opens a PR, and links it back in the Slack thread, + // 7. The agent implements, opens a PR, and links it back in the Slack thread, // echoing the reviewers' feedback — which proves the comments were stored // and harvested server-side on approve. await expect - .poll(async () => (await botMessages(request)).join("\n"), { timeout: 90_000 }) + .poll(async () => (await botMessages(request)).join("\n"), { + timeout: 90_000, + }) .toMatch(/\/pull\//); expect((await botMessages(request)).join("\n")).toMatch(/docstring/); - const prs = (await (await request.get("/mock/github/data")).json()) as Array; + const prs = (await ( + await request.get("/mock/github/data") + ).json()) as Array; expect(prs.length).toBeGreaterThan(0); await ownerCtx.close(); diff --git a/tests/test_dashboard_oauth_redirect.py b/tests/test_dashboard_oauth_redirect.py new file mode 100644 index 00000000..c19287ec --- /dev/null +++ b/tests/test_dashboard_oauth_redirect.py @@ -0,0 +1,30 @@ +from __future__ import annotations + +from agent.dashboard.oauth import sanitize_redirect_to + + +def test_sanitize_redirect_to_preserves_allowed_dashboard_target(monkeypatch) -> None: + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example") + monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example") + + target = "https://dashboard.example/agents/thread-1/plan?from=slack#review" + + assert sanitize_redirect_to(target) == target + + +def test_sanitize_redirect_to_preserves_allowed_preview_target(monkeypatch) -> None: + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example") + monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example") + + target = "https://preview.example/agents/thread-1/plan?from=slack#review" + + assert sanitize_redirect_to(target) == target + + +def test_sanitize_redirect_to_rejects_external_target(monkeypatch) -> None: + monkeypatch.setenv("DASHBOARD_BASE_URL", "https://dashboard.example") + monkeypatch.setenv("DASHBOARD_ALLOWED_ORIGINS", "https://preview.example") + + assert sanitize_redirect_to("https://evil.example/agents/thread-1/plan") == ( + "https://dashboard.example" + ) diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index 6d828464..e9fd7705 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -823,7 +823,7 @@ export const api = { export function loginUrl(redirectTo?: string): string { const target = - redirectTo ?? (typeof window !== "undefined" ? window.location.origin : "") + redirectTo ?? (typeof window !== "undefined" ? window.location.href : "") const qs = target ? `?redirect_to=${encodeURIComponent(target)}` : "" return `${API_BASE}/dashboard/api/auth/login${qs}` } diff --git a/ui/src/lib/auth-redirect-core.ts b/ui/src/lib/auth-redirect-core.ts new file mode 100644 index 00000000..91cc0e99 --- /dev/null +++ b/ui/src/lib/auth-redirect-core.ts @@ -0,0 +1,120 @@ +export const DEFAULT_AUTH_REDIRECT = "/agents" +export const AUTH_REDIRECT_STORAGE_KEY = "open-swe-auth-redirect" + +type LocationParts = { + pathname: string + search?: string + hash?: string +} + +function browserOrigin(): string | null { + return typeof window === "undefined" ? null : window.location.origin +} + +function storage(): Storage | null { + if (typeof window === "undefined") return null + try { + return window.sessionStorage + } catch { + return null + } +} + +function isBlockedRedirectPath(path: string): boolean { + return /^(?:\/login|\/dashboard\/api|\/_serverFn)(?:[/?#]|$)/.test(path) +} + +export function sanitizeAuthRedirect( + candidate: unknown, + fallback = DEFAULT_AUTH_REDIRECT +): string { + if (typeof candidate !== "string") return fallback + const trimmed = candidate.trim() + if (!trimmed) return fallback + + const origin = browserOrigin() + const isProtocolRelative = trimmed.startsWith("//") + const hasScheme = /^[a-zA-Z][a-zA-Z\d+.-]*:/.test(trimmed) + if (isProtocolRelative) return fallback + if (hasScheme && !origin) return fallback + + let parsed: URL + try { + parsed = new URL(trimmed, origin ?? "https://open-swe.invalid") + } catch { + return fallback + } + + if ((hasScheme || origin) && origin && parsed.origin !== origin) { + return fallback + } + + const path = `${parsed.pathname}${parsed.search}${parsed.hash}` + if (!path.startsWith("/") || isBlockedRedirectPath(path)) return fallback + return path +} + +export function authRedirectPathFromLocation(location: LocationParts): string { + const hash = location.hash + ? location.hash.startsWith("#") + ? location.hash + : `#${location.hash}` + : "" + return sanitizeAuthRedirect( + `${location.pathname}${location.search ?? ""}${hash}` + ) +} + +export function currentAuthRedirectPath(): string { + if (typeof window === "undefined") return DEFAULT_AUTH_REDIRECT + return authRedirectPathFromLocation(window.location) +} + +export function rememberAuthRedirect(candidate: unknown): string { + const path = sanitizeAuthRedirect(candidate) + const s = storage() + if (s) { + try { + s.setItem(AUTH_REDIRECT_STORAGE_KEY, path) + } catch {} + } + return path +} + +export function getRememberedAuthRedirect(): string | null { + const s = storage() + if (!s) return null + let raw: string | null = null + try { + raw = s.getItem(AUTH_REDIRECT_STORAGE_KEY) + } catch { + return null + } + if (!raw) return null + const path = sanitizeAuthRedirect(raw, "") + if (path) return path + clearRememberedAuthRedirect() + return null +} + +export function clearRememberedAuthRedirect(): void { + const s = storage() + if (!s) return + try { + s.removeItem(AUTH_REDIRECT_STORAGE_KEY) + } catch {} +} + +export function consumeAuthRedirect(candidate?: unknown): string { + const explicit = sanitizeAuthRedirect(candidate, "") + const path = explicit || getRememberedAuthRedirect() || DEFAULT_AUTH_REDIRECT + clearRememberedAuthRedirect() + return path +} + +export function authRedirectUrl(candidate?: unknown): string { + const path = sanitizeAuthRedirect(candidate) + const origin = browserOrigin() + if (!origin) return path + return new URL(path, origin).toString() +} diff --git a/ui/src/lib/auth-redirect.test.ts b/ui/src/lib/auth-redirect.test.ts new file mode 100644 index 00000000..eacc5063 --- /dev/null +++ b/ui/src/lib/auth-redirect.test.ts @@ -0,0 +1,76 @@ +/** @vitest-environment jsdom */ + +import { beforeEach, describe, expect, it } from "vitest" + +import { loginUrl } from "./api" +import { + AUTH_REDIRECT_STORAGE_KEY, + DEFAULT_AUTH_REDIRECT, + authRedirectPathFromLocation, + authRedirectUrl, + consumeAuthRedirect, + currentAuthRedirectPath, + getRememberedAuthRedirect, + rememberAuthRedirect, + sanitizeAuthRedirect, +} from "./auth-redirect-core" + +beforeEach(() => { + window.sessionStorage.clear() + window.history.pushState({}, "", "/") +}) + +describe("auth redirect helpers", () => { + it("captures protected route targets as relative paths", () => { + const path = authRedirectPathFromLocation({ + pathname: "/agents/thread-1/plan", + search: "?from=slack", + hash: "#review", + }) + + expect(path).toBe("/agents/thread-1/plan?from=slack#review") + expect(rememberAuthRedirect(path)).toBe(path) + expect(window.sessionStorage.getItem(AUTH_REDIRECT_STORAGE_KEY)).toBe(path) + }) + + it("resolves login targets to absolute same-origin URLs", () => { + const path = rememberAuthRedirect("/agents/thread-1/plan?from=slack#review") + + const target = `${window.location.origin}/agents/thread-1/plan?from=slack#review` + + expect(authRedirectUrl(path)).toBe(target) + expect(loginUrl(authRedirectUrl(path))).toContain( + encodeURIComponent(target) + ) + }) + + it("consumes remembered targets and clears session storage", () => { + rememberAuthRedirect("/agents/thread-1/plan") + + expect(consumeAuthRedirect()).toBe("/agents/thread-1/plan") + expect(getRememberedAuthRedirect()).toBeNull() + }) + + it("falls back for unsafe targets", () => { + expect( + sanitizeAuthRedirect("https://evil.example/agents/thread-1/plan") + ).toBe(DEFAULT_AUTH_REDIRECT) + expect(sanitizeAuthRedirect("//evil.example/agents/thread-1/plan")).toBe( + DEFAULT_AUTH_REDIRECT + ) + expect(sanitizeAuthRedirect("/login?redirect=/agents/thread-1/plan")).toBe( + DEFAULT_AUTH_REDIRECT + ) + }) + + it("builds a plan sign-in target for the current plan URL", () => { + window.history.pushState({}, "", "/agents/thread-1/plan?from=slack") + + expect(currentAuthRedirectPath()).toBe("/agents/thread-1/plan?from=slack") + expect(loginUrl(authRedirectUrl(currentAuthRedirectPath()))).toContain( + encodeURIComponent( + `${window.location.origin}/agents/thread-1/plan?from=slack` + ) + ) + }) +}) diff --git a/ui/src/lib/auth-redirect.tsx b/ui/src/lib/auth-redirect.tsx new file mode 100644 index 00000000..061786e4 --- /dev/null +++ b/ui/src/lib/auth-redirect.tsx @@ -0,0 +1,13 @@ +import { Navigate } from "@tanstack/react-router" + +import { + currentAuthRedirectPath, + rememberAuthRedirect, +} from "./auth-redirect-core" + +export * from "./auth-redirect-core" + +export function RequireLogin() { + const redirect = rememberAuthRedirect(currentAuthRedirectPath()) + return +} diff --git a/ui/src/routes/admin.tsx b/ui/src/routes/admin.tsx index f7cd1e8b..69b99d13 100644 --- a/ui/src/routes/admin.tsx +++ b/ui/src/routes/admin.tsx @@ -23,6 +23,7 @@ import { } from "@/components/ui/select" import { Skeleton } from "@/components/ui/skeleton" import { api } from "@/lib/api" +import { RequireLogin } from "@/lib/auth-redirect" import { useSession } from "@/lib/session" export const Route = createFileRoute("/admin")({ component: AdminPage }) @@ -43,7 +44,7 @@ function AdminPage() { ) } - if (!session.data) return + if (!session.data) return if (!session.data.is_admin) return return ( diff --git a/ui/src/routes/admin_.evals.tsx b/ui/src/routes/admin_.evals.tsx index 6c888f49..76309322 100644 --- a/ui/src/routes/admin_.evals.tsx +++ b/ui/src/routes/admin_.evals.tsx @@ -7,6 +7,7 @@ import { AppShell, SettingsSection } from "@/components/AppShell" import { Button } from "@/components/ui/button" import { Skeleton } from "@/components/ui/skeleton" import { api } from "@/lib/api" +import { RequireLogin } from "@/lib/auth-redirect" import { useSession } from "@/lib/session" export const Route = createFileRoute("/admin_/evals")({ component: ReviewerEvalPage }) @@ -21,7 +22,7 @@ function ReviewerEvalPage() { ) } - if (!session.data) return + if (!session.data) return if (!session.data.is_admin) return return ( diff --git a/ui/src/routes/agents.tsx b/ui/src/routes/agents.tsx index e93d3433..1a19a924 100644 --- a/ui/src/routes/agents.tsx +++ b/ui/src/routes/agents.tsx @@ -1,14 +1,10 @@ -import { - Navigate, - Outlet, - createFileRoute, - useRouterState, -} from "@tanstack/react-router" +import { Outlet, createFileRoute, useRouterState } from "@tanstack/react-router" import { AgentsShell } from "@/components/agents/AgentsSidebar" import { Skeleton } from "@/components/ui/skeleton" import agentsCss from "@/styles/agents.css?url" import { AgentThreadStreamProvider } from "@/lib/agents/AgentThreadStreamProvider" +import { RequireLogin } from "@/lib/auth-redirect" import { useSession } from "@/lib/session" export const Route = createFileRoute("/agents")({ @@ -41,7 +37,7 @@ function AgentsLayout() { ) } - if (!session.data) return + if (!session.data) return return ( diff --git a/ui/src/routes/agents/$threadId_.plan.tsx b/ui/src/routes/agents/$threadId_.plan.tsx index 23c94aba..5eeb0198 100644 --- a/ui/src/routes/agents/$threadId_.plan.tsx +++ b/ui/src/routes/agents/$threadId_.plan.tsx @@ -4,7 +4,10 @@ import { useEffect, useState } from "react" import { ArrowLeft } from "lucide-react" import { PlanReview } from "@/components/agents/PlanReview" +import { buttonVariants } from "@/components/ui/button" import { Skeleton } from "@/components/ui/skeleton" +import { loginUrl } from "@/lib/api" +import { authRedirectUrl, currentAuthRedirectPath } from "@/lib/auth-redirect" import { PlanApiError, getPlan } from "@/lib/plan" export const Route = createFileRoute("/agents/$threadId_/plan")({ @@ -32,6 +35,18 @@ function BackLink({ threadId }: { threadId: string }) { ) } +export function planSignInHref(): string { + return loginUrl(authRedirectUrl(currentAuthRedirectPath())) +} + +export function PlanSignInButton() { + return ( + + Sign in to view this plan + + ) +} + function PlanPage() { const { threadId } = Route.useParams() @@ -70,6 +85,7 @@ function PlanPage() { ? "Please sign in to view this plan." : "This plan could not be found."}

+ {status === 401 ? : null} diff --git a/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx b/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx index 1fe49b26..f2b424fc 100644 --- a/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx +++ b/ui/src/routes/agents/reviews/$owner.$repo.$number.tsx @@ -1,4 +1,4 @@ -import { Link, Navigate, createFileRoute } from "@tanstack/react-router" +import { Link, createFileRoute } from "@tanstack/react-router" import { useQuery, useQueryClient } from "@tanstack/react-query" import { useCallback, useEffect, useRef, useState } from "react" import { ArrowLeftIcon, GitPullRequestIcon } from "@phosphor-icons/react" @@ -9,6 +9,7 @@ import { ReviewMainBody } from "@/components/agents/ReviewMainBody" import { useSidebarControls } from "@/components/sidebar-layout" import { Skeleton } from "@/components/ui/skeleton" import { api } from "@/lib/api" +import { RequireLogin } from "@/lib/auth-redirect" import { useSession } from "@/lib/session" import { cn } from "@/lib/utils" @@ -70,7 +71,7 @@ function ReviewDetailPage() { ) } - if (!session.data) return + if (!session.data) return return (
diff --git a/ui/src/routes/agents_.instructions.tsx b/ui/src/routes/agents_.instructions.tsx index eb36d9d8..c12e5f25 100644 --- a/ui/src/routes/agents_.instructions.tsx +++ b/ui/src/routes/agents_.instructions.tsx @@ -1,8 +1,9 @@ -import { Navigate, createFileRoute } from "@tanstack/react-router"; +import { createFileRoute } from "@tanstack/react-router"; import { AgentInstructionsPanel } from "@/components/AgentInstructionsPanel"; import { AppShell } from "@/components/AppShell"; import { Skeleton } from "@/components/ui/skeleton"; +import { RequireLogin } from "@/lib/auth-redirect"; import { useSession } from "@/lib/session"; export const Route = createFileRoute("/agents_/instructions")({ @@ -19,7 +20,7 @@ function AgentInstructionsPage() { ); } - if (!session.data) return ; + if (!session.data) return ; return ( ) } - if (!session.data) return + if (!session.data) return if (!session.data.is_admin) return return ( diff --git a/ui/src/routes/cloud-agents.tsx b/ui/src/routes/cloud-agents.tsx index fc497049..0f570522 100644 --- a/ui/src/routes/cloud-agents.tsx +++ b/ui/src/routes/cloud-agents.tsx @@ -1,4 +1,4 @@ -import { Link, Navigate, createFileRoute } from "@tanstack/react-router" +import { Link, createFileRoute } from "@tanstack/react-router" import { CaretRightIcon } from "@phosphor-icons/react" import { useEffect, useRef, useState } from "react" @@ -23,6 +23,7 @@ import { useRepos, useSaveProfile, } from "@/lib/profile" +import { RequireLogin } from "@/lib/auth-redirect" import { useSession } from "@/lib/session" export const Route = createFileRoute("/cloud-agents")({ @@ -112,7 +113,7 @@ function CloudAgentsPage() { ) } - if (!session.data) return + if (!session.data) return const fallbackModel = defaultAgentModel const fallbackEffort = defaultAgentEffort diff --git a/ui/src/routes/login.tsx b/ui/src/routes/login.tsx index 0cefa50a..c3505bfc 100644 --- a/ui/src/routes/login.tsx +++ b/ui/src/routes/login.tsx @@ -1,16 +1,44 @@ -import { Navigate, createFileRoute } from "@tanstack/react-router"; +import { createFileRoute } from "@tanstack/react-router"; +import { useEffect, useMemo } from "react"; import { buttonVariants } from "@/components/ui/button"; import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card"; import { Skeleton } from "@/components/ui/skeleton"; import { loginUrl } from "@/lib/api"; +import { + DEFAULT_AUTH_REDIRECT, + authRedirectUrl, + consumeAuthRedirect, + getRememberedAuthRedirect, + rememberAuthRedirect, +} from "@/lib/auth-redirect"; import { useSession } from "@/lib/session"; import { cn } from "@/lib/utils"; -export const Route = createFileRoute("/login")({ component: Login }); +type LoginSearch = { redirect?: string }; + +export const Route = createFileRoute("/login")({ + validateSearch: (search: Record): LoginSearch => ({ + redirect: typeof search.redirect === "string" ? search.redirect : undefined, + }), + component: Login, +}); function Login() { const session = useSession(); + const search = Route.useSearch(); + const redirectParam = search.redirect; + const intendedPath = useMemo( + () => + redirectParam + ? rememberAuthRedirect(redirectParam) + : getRememberedAuthRedirect() ?? DEFAULT_AUTH_REDIRECT, + [redirectParam] + ); + const authenticatedRedirect = useMemo( + () => (session.data ? consumeAuthRedirect(redirectParam) : null), + [redirectParam, session.data] + ); if (session.isLoading) { return ( @@ -20,8 +48,8 @@ function Login() { ); } - if (session.data) { - return ; + if (authenticatedRedirect) { + return ; } return ( @@ -35,7 +63,10 @@ function Login() { - + Continue with GitHub @@ -43,3 +74,15 @@ function Login() { ); } + +function ClientRedirect({ path }: { path: string }) { + useEffect(() => { + if (typeof window !== "undefined") window.location.replace(path); + }, [path]); + + return ( +
+ +
+ ); +} diff --git a/ui/src/routes/my-settings.tsx b/ui/src/routes/my-settings.tsx index 2298421f..0ff3336f 100644 --- a/ui/src/routes/my-settings.tsx +++ b/ui/src/routes/my-settings.tsx @@ -1,4 +1,4 @@ -import { Navigate, createFileRoute, useNavigate } from "@tanstack/react-router" +import { createFileRoute, useNavigate } from "@tanstack/react-router" import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { useState } from "react" import { IoLogoSlack } from "react-icons/io5" @@ -24,6 +24,7 @@ import { useProfile, useSaveProfile, } from "@/lib/profile" +import { RequireLogin } from "@/lib/auth-redirect" import { useSession } from "@/lib/session" import { notificationsEnabled, @@ -366,7 +367,7 @@ function MySettingsPage() { ) } - if (!session.data) return + if (!session.data) return const handleLogout = async () => { await api.logout() diff --git a/ui/src/routes/review.tsx b/ui/src/routes/review.tsx index d01574ea..91ee968e 100644 --- a/ui/src/routes/review.tsx +++ b/ui/src/routes/review.tsx @@ -1,4 +1,4 @@ -import { Link, Navigate, createFileRoute } from "@tanstack/react-router"; +import { Link, createFileRoute } from "@tanstack/react-router"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { CaretRightIcon } from "@phosphor-icons/react"; import { useEffect, useMemo, useState } from "react"; @@ -11,6 +11,7 @@ import { Skeleton } from "@/components/ui/skeleton"; import { Switch } from "@/components/ui/switch"; import { Textarea } from "@/components/ui/textarea"; import { ApiError, api } from "@/lib/api"; +import { RequireLogin } from "@/lib/auth-redirect"; import { useSession } from "@/lib/session"; export const Route = createFileRoute("/review")({ component: ReviewPage }); @@ -66,7 +67,7 @@ function ReviewPage() { ); } - if (!session.data) return ; + if (!session.data) return ; const current: TeamSettings = local; const canEdit = session.data.is_admin; diff --git a/ui/src/routes/review_.repositories.$owner.tsx b/ui/src/routes/review_.repositories.$owner.tsx index 1cf611f8..64c8e797 100644 --- a/ui/src/routes/review_.repositories.$owner.tsx +++ b/ui/src/routes/review_.repositories.$owner.tsx @@ -1,4 +1,4 @@ -import { Navigate, createFileRoute } from "@tanstack/react-router"; +import { createFileRoute } from "@tanstack/react-router"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useEffect, useMemo, useState } from "react"; @@ -8,6 +8,7 @@ import { Button } from "@/components/ui/button"; import { Skeleton } from "@/components/ui/skeleton"; import { Switch } from "@/components/ui/switch"; import { ApiError, api } from "@/lib/api"; +import { RequireLogin } from "@/lib/auth-redirect"; import { useSession } from "@/lib/session"; const PAGE_SIZE = 20; @@ -78,7 +79,7 @@ function RepositoriesOwnerPage() { ); } - if (!session.data) return ; + if (!session.data) return ; const canEdit = session.data.is_admin; const enabledCount = ownerRepos.filter((r) => enabledSet.has(r.full_name)).length; diff --git a/ui/src/routes/review_.styles.tsx b/ui/src/routes/review_.styles.tsx index 2d72f49a..c17d9f82 100644 --- a/ui/src/routes/review_.styles.tsx +++ b/ui/src/routes/review_.styles.tsx @@ -1,8 +1,9 @@ -import { Navigate, createFileRoute } from "@tanstack/react-router"; +import { createFileRoute } from "@tanstack/react-router"; import { AppShell } from "@/components/AppShell"; import { ReviewStylesPanel } from "@/components/ReviewStylesPanel"; import { Skeleton } from "@/components/ui/skeleton"; +import { RequireLogin } from "@/lib/auth-redirect"; import { useSession } from "@/lib/session"; export const Route = createFileRoute("/review_/styles")({ component: ReviewStylesPage }); @@ -17,7 +18,7 @@ function ReviewStylesPage() { ); } - if (!session.data) return ; + if (!session.data) return ; return ( ) } - if (!session.data) return + if (!session.data) return return (