diff --git a/apps/web/src/account.tsx b/apps/web/src/account.tsx index 341679de..2d51582d 100644 --- a/apps/web/src/account.tsx +++ b/apps/web/src/account.tsx @@ -1,6 +1,6 @@ import { api, errorDetail, readBootstrap } from "@diffx/api"; import { ArrowLeftIcon, UserRoundIcon } from "lucide-react"; -import { useEffect, useState, type FormEvent } from "react"; +import { useEffect, useState, type FormEvent, type ReactNode } from "react"; import { ErrorState } from "@/components/error-state"; import { IconButton } from "@/components/icon-button"; import { Button } from "@/components/ui/button"; @@ -31,18 +31,206 @@ export function AccountButton({ variant }: { variant: "rail" | "shell" }) { ); } +const prose = "mb-4 leading-copy text-secondary"; + +/** Failed outcomes the Google callback reports through `/?signin=`. */ +const signInFailures: Record = { + "no-account": + "No diffx account uses that Google account. Ask the admin for an invite.", + "invite-mismatch": + "That Google account does not match the invited email, or Google has not verified it. Open the invite link again and choose the invited account.", + "invite-invalid": + "The invite link is invalid, expired or already used. Ask the admin for a new one.", + failed: "Google sign-in failed. Try again.", +}; + +/** Reads and clears the callback outcome so a reload does not repeat it. */ +function takeSignInOutcome() { + const url = new URL(window.location.href); + const outcome = url.searchParams.get("signin"); + if (outcome === null) return ""; + url.searchParams.delete("signin"); + window.history.replaceState(window.history.state, "", url); + return outcome; +} + /** Members sign in with Google; accounts come only from invites. */ export function SignIn() { + const { state } = useIdentity(); + const [outcome] = useState(takeSignInOutcome); + const google = + state.status === "ready" && + state.bootstrap.mode === "remote" && + state.bootstrap.googleEnabled; + const failure = signInFailures[outcome]; return (
diffx

Sign in

-

- Sign-in is not available on this server yet. Ask its admin for an - invite. -

+ {outcome === "cancelled" && ( +

+ Google sign-in was cancelled. +

+ )} + {failure && } + {google ? ( + <> +

+ Use the Google account your invite was sent to. +

+ + + ) : ( +

+ Sign-in is not available on this server yet. Ask its admin for an + invite. +

+ )} +
+ ); +} + +/** Starts Google sign-in, accepting the invite when one is given. */ +function ContinueWithGoogle({ invite }: { invite?: string }) { + const [busy, setBusy] = useState(false); + const [error, setError] = useState(""); + async function start() { + setBusy(true); + setError(""); + try { + const { data, error } = + invite === undefined + ? await api.POST("/api/v2/auth/google", { body: {} }) + : await api.POST("/api/v2/invites/google", { + body: { token: invite }, + }); + if (!data) + throw new Error( + errorDetail(error, "Unable to start Google sign-in. Try again."), + ); + window.location.assign(data.authorizeUrl); + } catch (failure: unknown) { + setError( + errorDetail(failure, "Unable to start Google sign-in. Try again."), + ); + setBusy(false); + } + } + return ( + <> + {error && } + + + ); +} + +function fragmentToken() { + return new URLSearchParams(window.location.hash.slice(1)).get("t") ?? ""; +} + +type InviteState = + | { status: "loading" } + | { status: "invalid" } + | { status: "error"; message: string } + | { status: "ready"; email: string; expiresAt: number }; + +/** Accepts an invite link: its secret stays in the fragment, never a request URL. */ +export function InviteSignUp() { + const [token, setToken] = useState(fragmentToken); + const [invite, setInvite] = useState({ status: "loading" }); + const [attempt, setAttempt] = useState(0); + useEffect(() => { + // Another link opened in this tab changes only the fragment. + const changed = () => { + const next = fragmentToken(); + window.history.replaceState(null, "", "/invite"); + if (next) setToken(next); + }; + window.addEventListener("hashchange", changed); + return () => window.removeEventListener("hashchange", changed); + }, []); + useEffect(() => { + // The secret stays out of history once read. + window.history.replaceState(null, "", "/invite"); + if (!token) { + setInvite({ status: "invalid" }); + return; + } + const controller = new AbortController(); + setInvite({ status: "loading" }); + void api + .POST("/api/v2/invites/lookup", { + body: { token }, + signal: controller.signal, + }) + .then(({ data, error, response }) => { + if (controller.signal.aborted) return; + if (response.status === 404) { + setInvite({ status: "invalid" }); + return; + } + if (!data) + throw new Error(errorDetail(error, "Unable to load the invite")); + setInvite({ + status: "ready", + email: data.email, + expiresAt: data.expiresAt, + }); + }) + .catch((failure: unknown) => { + if (!controller.signal.aborted) + setInvite({ + status: "error", + message: errorDetail(failure, "Unable to load the invite"), + }); + }); + return () => controller.abort(); + }, [token, attempt]); + return ( +
+ + diffx + +

Join diffx

+ {invite.status === "loading" && ( +

+ Checking your invite… +

+ )} + {invite.status === "error" && ( + setAttempt((value) => value + 1)} + /> + )} + {invite.status === "invalid" && ( +

+ This invite link is invalid, expired or already used. Ask the admin + for a new one. +

+ )} + {invite.status === "ready" && ( + <> +

+ You are invited as {invite.email}. Continue with + the Google account for that email. The invite expires{" "} + {new Date(invite.expiresAt).toLocaleString()}. +

+ + + )}
); } @@ -223,11 +411,33 @@ function CredentialForm({ ); } +/** A signed-in account page: the way back to reviews, then its content. */ +export function AccountPage({ + username, + children, +}: { + username: string; + children: ReactNode; +}) { + return ( +
+
+ + {username} +
+
{children}
+
+ ); +} + export function AccountSettings() { const { state, signedOut } = useIdentity(); const [error, setError] = useState(""); const [busy, setBusy] = useState(false); if (state.status !== "ready" || !state.bootstrap.user) return null; + const user = state.bootstrap.user; async function logout() { setBusy(true); setError(""); @@ -243,41 +453,44 @@ export function AccountSettings() { } } return ( -
-
- - {state.bootstrap.user.username} -
-
-

Account settings

-

- Signed in as {state.bootstrap.user.username}. -

- - - {state.bootstrap.user.admin && } -
+ + {user.admin && } +
+

+ Browser session +

+ {error && } + -
-
-
+ Sign out + + + ); } diff --git a/apps/web/src/admin.tsx b/apps/web/src/admin.tsx new file mode 100644 index 00000000..31f82fbd --- /dev/null +++ b/apps/web/src/admin.tsx @@ -0,0 +1,459 @@ +import { + api, + errorDetail, + type ApiAdminUser as AdminUser, + type ApiInvite as Invite, +} from "@diffx/api"; +import { useEffect, useState, type FormEvent } from "react"; +import { ConfirmDialog } from "@/components/confirm-dialog"; +import { ErrorState } from "@/components/error-state"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { AccountPage } from "./account.tsx"; +import { useIdentity } from "./identity-context.tsx"; + +const prose = "mb-4 leading-copy text-secondary"; +const row = + "flex flex-wrap items-center justify-between gap-4 border-b border-separator py-3"; + +function date(value: number) { + return new Date(value).toLocaleDateString(); +} + +/** Loads a list and reloads it on demand, keeping the last list meanwhile. */ +function useAdminList(load: (signal: AbortSignal) => Promise) { + const [items, setItems] = useState(null); + const [error, setError] = useState(""); + const [attempt, setAttempt] = useState(0); + useEffect(() => { + const controller = new AbortController(); + setError(""); + load(controller.signal) + .then((value) => { + if (!controller.signal.aborted) setItems(value); + }) + .catch((failure: unknown) => { + if (!controller.signal.aborted) + setError(errorDetail(failure, "Unable to load. Try again.")); + }); + return () => controller.abort(); + }, [load, attempt]); + return { items, error, reload: () => setAttempt((value) => value + 1) }; +} + +async function loadUsers(signal: AbortSignal) { + const { data, error } = await api.GET("/api/v2/admin/users", { signal }); + if (!data) + throw new Error(errorDetail(error, "Unable to load accounts. Try again.")); + return data.users; +} + +async function loadInvites(signal: AbortSignal) { + const { data, error } = await api.GET("/api/v2/admin/invites", { signal }); + if (!data) + throw new Error(errorDetail(error, "Unable to load invites. Try again.")); + return data.invites; +} + +/** The admin's page: invite people and manage every account. */ +export function AdminPage() { + const { state } = useIdentity(); + if ( + state.status !== "ready" || + state.bootstrap.mode !== "remote" || + !state.bootstrap.user + ) + return null; + return ( + +

Admin

+

Invite people and manage accounts on this server.

+ + + + ); +} + +const inviteStatus: Record< + Invite["status"], + { label: string; tone: "accent" | "success" | "neutral" } +> = { + pending: { label: "Pending", tone: "accent" }, + used: { label: "Used", tone: "success" }, + expired: { label: "Expired", tone: "neutral" }, + revoked: { label: "Revoked", tone: "neutral" }, +}; + +function inviteDetail(invite: Invite) { + switch (invite.status) { + case "pending": + return `Expires ${new Date(invite.expiresAt).toLocaleString()}`; + case "used": + return invite.usedBy === null + ? "Account deleted" + : `Joined as ${invite.usedBy}`; + case "expired": + return `Expired ${date(invite.expiresAt)}`; + case "revoked": + return `Created ${date(invite.createdAt)}`; + } +} + +function AdminInvites({ google }: { google: boolean }) { + const invites = useAdminList(loadInvites); + const [email, setEmail] = useState(""); + const [created, setCreated] = useState<{ + invite: Invite; + url: string; + } | null>(null); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(""); + const [copyStatus, setCopyStatus] = useState(""); + const [revoking, setRevoking] = useState(null); + const [revokeError, setRevokeError] = useState(""); + async function create(event: FormEvent) { + event.preventDefault(); + if (busy) return; + setBusy(true); + setError(""); + setCreated(null); + setCopyStatus(""); + try { + const { data, error } = await api.POST("/api/v2/admin/invites", { + body: { email }, + }); + if (!data) + throw new Error( + errorDetail(error, "Unable to create invite. Try again."), + ); + setCreated(data); + setEmail(""); + invites.reload(); + } catch (failure: unknown) { + setError(errorDetail(failure, "Unable to create invite. Try again.")); + } finally { + setBusy(false); + } + } + async function revoke(id: string) { + setBusy(true); + setRevokeError(""); + try { + const { response, error } = await api.DELETE( + "/api/v2/admin/invites/{inviteId}", + { params: { path: { inviteId: id } } }, + ); + if (!response.ok) + throw new Error( + errorDetail(error, "Unable to revoke invite. Try again."), + ); + if (created?.invite.id === id) setCreated(null); + setRevoking(null); + invites.reload(); + } catch (failure: unknown) { + setRevokeError( + errorDetail(failure, "Unable to revoke invite. Try again."), + ); + } finally { + setBusy(false); + } + } + async function copy() { + try { + await navigator.clipboard.writeText(created?.url ?? ""); + setCopyStatus("Copied"); + } catch { + setCopyStatus("Copy unavailable. Select and copy the link below."); + } + } + return ( +
+

+ Invites +

+ {google ? ( +

+ An invite lets one person join with the Google account for that email. + Links work once and expire after 48 hours; inviting an email again + replaces its pending link. +

+ ) : ( +

+ Invites need Google sign-in. Set DIFFX_SERVER_GOOGLE_CLIENT_ID, + DIFFX_SERVER_GOOGLE_CLIENT_SECRET and DIFFX_SERVER_PUBLIC_URL, then + restart the server. +

+ )} + {invites.error && ( + + )} + {invites.items === null && !invites.error && ( +

+ Loading invites… +

+ )} + {invites.items !== null && invites.items.length > 0 && ( +
    + {invites.items.map((invite) => ( +
  • +
    + {invite.email} + + + {inviteStatus[invite.status].label} + + {inviteDetail(invite)} + +
    + {invite.status === "pending" && ( + + )} +
    • + ))} +
+ )} + { + if (!open) setRevoking(null); + }} + title={`Revoke invite for ${revoking?.email ?? "this email"}?`} + description="The invite link stops working immediately. You can invite this email again." + error={revokeError} + confirmLabel="Revoke invite" + busyLabel="Revoking…" + busy={busy} + onConfirm={() => { + if (revoking) void revoke(revoking.id); + }} + /> + {google && ( +
+ + setEmail(event.target.value)} + disabled={busy} + /> + {error && } + + + )} + {created && ( +
+

Send this invite link

+

+ It works once, for {created.invite.email}, until{" "} + {new Date(created.invite.expiresAt).toLocaleString()}. The link will + disappear when you leave this page. +

+ 
+            {created.url}
+
+
+ + +
+ {copyStatus &&

{copyStatus}

} +
+ )} +
+ ); +} + +type AccountAction = { kind: "disable" | "delete"; user: AdminUser }; + +function AdminAccounts() { + const users = useAdminList(loadUsers); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(""); + const [confirming, setConfirming] = useState(null); + const [confirmError, setConfirmError] = useState(""); + async function apply(kind: "disable" | "enable" | "delete", user: AdminUser) { + setBusy(true); + setError(""); + setConfirmError(""); + const failure = `Unable to ${kind} ${user.username}. Try again.`; + try { + const params = { params: { path: { userId: user.id } } }; + const { response, error } = + kind === "delete" + ? await api.DELETE("/api/v2/admin/users/{userId}", params) + : kind === "disable" + ? await api.POST("/api/v2/admin/users/{userId}/disable", params) + : await api.POST("/api/v2/admin/users/{userId}/enable", params); + if (!response.ok) throw new Error(errorDetail(error, failure)); + setConfirming(null); + users.reload(); + } catch (failed: unknown) { + if (kind === "enable") setError(errorDetail(failed, failure)); + else setConfirmError(errorDetail(failed, failure)); + } finally { + setBusy(false); + } + } + return ( +
+

+ Accounts +

+

+ Disabling signs an account out everywhere and revokes its API tokens. + Deleting also removes its reviews. +

+ {users.error && ( + + )} + {error && } + {users.items === null && !users.error && ( +

+ Loading accounts… +

+ )} + {users.items !== null && ( +
    + {users.items.map((user) => ( +
  • +
    + + {user.username} + {user.admin && Admin} + {user.disabled && ( + + Disabled + + )} + + + {user.email ?? "Password sign-in"} · Joined{" "} + {date(user.createdAt)} + +
    + {!user.admin && ( +
    + {user.disabled ? ( + + ) : ( + + )} + +
    + )} +
    • + ))} +
+ )} + { + if (!open) setConfirming(null); + }} + title={ + confirming?.kind === "delete" + ? `Delete ${confirming.user.username}?` + : `Disable ${confirming?.user.username ?? "account"}?` + } + description={ + confirming?.kind === "delete" + ? "The account, its reviews, comments and API tokens are removed. This cannot be undone." + : "The account is signed out everywhere and its API tokens are revoked. Enabling it later restores sign-in, not those tokens." + } + error={confirmError} + confirmLabel={ + confirming?.kind === "delete" ? "Delete account" : "Disable account" + } + busyLabel={confirming?.kind === "delete" ? "Deleting…" : "Disabling…"} + busy={busy} + onConfirm={() => { + if (confirming) void apply(confirming.kind, confirming.user); + }} + /> +
+ ); +} diff --git a/apps/web/src/identity-context.tsx b/apps/web/src/identity-context.tsx index 6b5e02c2..d2d85293 100644 --- a/apps/web/src/identity-context.tsx +++ b/apps/web/src/identity-context.tsx @@ -28,6 +28,15 @@ interface Identity { signedOut: () => void; } const IdentityContext = createContext(null); +/** Requests made without a browser session; their 401s are not sign-outs. */ +const anonymous = new Set([ + "/api/v2/bootstrap", + "/api/v2/auth/login", + "/api/v2/setup", + "/api/v2/auth/google", + "/api/v2/invites/lookup", + "/api/v2/invites/google", +]); export function navigate(path: string) { window.history.pushState(null, "", path); @@ -58,13 +67,7 @@ export function IdentityProvider({ children }: { children: ReactNode }) { useEffect(() => { const middleware: ApiMiddleware = { onRequest({ request }) { - const path = new URL(request.url).pathname; - if ( - path === "/api/v2/bootstrap" || - path === "/api/v2/auth/login" || - path === "/api/v2/setup" - ) - return request; + if (anonymous.has(new URL(request.url).pathname)) return request; const csrf = current.current?.csrfToken; if (csrf && request.method !== "GET" && request.method !== "HEAD") request.headers.set("X-CSRF-Token", csrf); @@ -76,8 +79,7 @@ export function IdentityProvider({ children }: { children: ReactNode }) { if ( response.status === 401 && !request.signal.aborted && - new URL(request.url).pathname !== "/api/v2/auth/login" && - new URL(request.url).pathname !== "/api/v2/setup" + !anonymous.has(new URL(request.url).pathname) ) signedOut(); }, diff --git a/apps/web/src/main.tsx b/apps/web/src/main.tsx index fb1a05b7..65811dd3 100644 --- a/apps/web/src/main.tsx +++ b/apps/web/src/main.tsx @@ -25,8 +25,10 @@ import { AccountSettings, AdminSetup, AdminSignIn, + InviteSignUp, SignIn, } from "./account.tsx"; +import { AdminPage } from "./admin.tsx"; import { TooltipProvider } from "@/components/ui/tooltip"; const root = document.getElementById("root"); @@ -42,11 +44,17 @@ function ApplicationRoutes() { const ready = state.status === "ready"; const remote = ready && state.bootstrap.mode === "remote"; const signedIn = ready && state.bootstrap.user !== null; - // Local mode has no account pages; signed-in users have no sign-in pages. + const admin = signedIn && state.bootstrap.user?.admin === true; + // Local mode has no account pages; signed-in users have no sign-in pages; + // only the admin has the admin page. const misplaced = ready && - ((!remote && path === "/settings") || - (signedIn && (path === "/setup" || path === "/admin/login"))); + ((!remote && (path === "/settings" || path === "/admin")) || + (signedIn && + (path === "/setup" || + path === "/admin/login" || + path === "/invite" || + (path === "/admin" && !admin)))); useEffect(() => { if (!misplaced) return; window.history.replaceState(null, "", "/"); @@ -56,10 +64,12 @@ function ApplicationRoutes() { if (state.bootstrap.mode === "remote" && !state.bootstrap.user) { if (path === "/setup") return ; if (path === "/admin/login") return ; + if (path === "/invite") return ; return ; } if (path === "/settings" && state.bootstrap.mode === "remote") return ; + if (path === "/admin" && admin) return ; return ( diff --git a/apps/web/test/browser/identity.spec.ts b/apps/web/test/browser/identity.spec.ts index 68547f2a..b77ee75e 100644 --- a/apps/web/test/browser/identity.spec.ts +++ b/apps/web/test/browser/identity.spec.ts @@ -1,17 +1,32 @@ import { type Page } from "@playwright/test"; import { expect, test } from "./fixtures.ts"; -function bootstrap(signedIn: boolean, admin = false) { +interface RemoteOptions { + googleEnabled?: boolean; + email?: string | null; +} +function bootstrap( + signedIn: boolean, + admin = false, + { googleEnabled = false, email = null }: RemoteOptions = {}, +) { return { mode: "remote", - googleEnabled: false, - user: signedIn ? { id: "account-a", username: "alice", admin } : null, + googleEnabled, + user: signedIn + ? { id: "account-a", username: "alice", email, admin } + : null, csrfToken: signedIn ? "session-csrf" : null, }; } -async function remote(page: Page, signedIn = true, admin = false) { +async function remote( + page: Page, + signedIn = true, + admin = false, + options: RemoteOptions = {}, +) { await page.route("**/api/v2/bootstrap", (route) => - route.fulfill({ json: bootstrap(signedIn, admin) }), + route.fulfill({ json: bootstrap(signedIn, admin, options) }), ); await page.route("**/api/v2/contexts?*", (route) => route.fulfill({ json: { contexts: [], nextCursor: null } }), @@ -515,3 +530,307 @@ test("expired GitHub authorization asks to connect again and disconnect confirms page.getByRole("button", { name: "Connect GitHub", exact: true }), ).toBeVisible(); }); + +test("Google sign-in starts without CSRF and reports the callback outcome once", async ({ + page, +}) => { + await remote(page, false, false, { googleEnabled: true }); + let started: unknown; + await page.route("**/api/v2/auth/google", (route) => { + expect(route.request().headers()["x-csrf-token"]).toBeUndefined(); + started = route.request().postDataJSON(); + return route.fulfill({ + json: { authorizeUrl: "https://accounts.google.test/auth?state=s" }, + }); + }); + await page.route("https://accounts.google.test/**", (route) => + route.fulfill({ contentType: "text/html", body: "

Google

" }), + ); + await page.goto("/?signin=no-account"); + await expect(page.getByRole("alert")).toHaveText( + "No diffx account uses that Google account. Ask the admin for an invite.", + ); + expect(new URL(page.url()).search).toBe(""); + await expect(page.getByLabel("Password", { exact: true })).toHaveCount(0); + await page.getByRole("button", { name: "Continue with Google" }).click(); + await expect(page.getByRole("heading", { name: "Google" })).toBeVisible(); + expect(started).toEqual({}); + expect(page.url()).toBe("https://accounts.google.test/auth?state=s"); +}); + +test("without Google configuration sign in offers no Google action", async ({ + page, +}) => { + await remote(page, false); + await page.goto("/?signin=cancelled"); + await expect( + page.getByText("Sign-in is not available on this server yet.", { + exact: false, + }), + ).toBeVisible(); + await expect( + page.getByRole("button", { name: "Continue with Google" }), + ).toHaveCount(0); +}); + +test("an invite link shows its email, hides its secret and starts Google sign-in with it", async ({ + page, +}) => { + await remote(page, false, false, { googleEnabled: true }); + await page.route("**/api/v2/invites/lookup", (route) => { + expect(route.request().postDataJSON()).toEqual({ token: "invite-secret" }); + return route.fulfill({ + json: { email: "bob@example.com", expiresAt: Date.now() + 86_400_000 }, + }); + }); + let started: unknown; + await page.route("**/api/v2/invites/google", (route) => { + started = route.request().postDataJSON(); + return route.fulfill({ + json: { authorizeUrl: "https://accounts.google.test/auth?state=i" }, + }); + }); + await page.route("https://accounts.google.test/**", (route) => + route.fulfill({ contentType: "text/html", body: "

Google

" }), + ); + await page.goto("/invite#t=invite-secret"); + await expect(page.getByRole("heading", { name: "Join diffx" })).toBeVisible(); + await expect(page.getByText("bob@example.com")).toBeVisible(); + expect(new URL(page.url()).hash).toBe(""); + expect(page.url()).not.toContain("invite-secret"); + await page.getByRole("button", { name: "Continue with Google" }).click(); + await expect(page.getByRole("heading", { name: "Google" })).toBeVisible(); + expect(started).toEqual({ token: "invite-secret" }); +}); + +test("an unusable invite link says so without a Google action", async ({ + page, +}) => { + await remote(page, false, false, { googleEnabled: true }); + await page.route("**/api/v2/invites/lookup", (route) => + route.fulfill({ + status: 404, + json: { detail: "Invite link is invalid, expired or already used" }, + }), + ); + await page.goto("/invite#t=used-secret"); + await expect( + page.getByText("This invite link is invalid, expired or already used.", { + exact: false, + }), + ).toBeVisible(); + await expect( + page.getByRole("button", { name: "Continue with Google" }), + ).toHaveCount(0); +}); + +test("members see their Google email and cannot open the admin page", async ({ + page, +}) => { + await remote(page, true, false, { email: "alice@example.com" }); + await page.goto("/settings"); + await expect( + page.getByText("with Google as alice@example.com", { exact: false }), + ).toBeVisible(); + await expect( + page.getByRole("button", { name: "Manage accounts and invites" }), + ).toHaveCount(0); + await page.goto("/admin"); + await expect( + page.getByRole("heading", { name: "No review contexts yet" }), + ).toBeVisible(); + expect(new URL(page.url()).pathname).toBe("/"); +}); + +test("the admin invites by email, sees the link once and revokes it", async ({ + page, +}) => { + await remote(page, true, true, { googleEnabled: true }); + const now = Date.now(); + const pending = { + id: "invite-a", + email: "bob@example.com", + status: "pending", + createdAt: now, + expiresAt: now + 48 * 3_600_000, + usedBy: null, + }; + let invites: unknown[] = [ + { + ...pending, + id: "invite-old", + email: "carol@example.com", + status: "used", + usedBy: "carol", + }, + ]; + await page.route("**/api/v2/admin/users", (route) => + route.fulfill({ + json: { + users: [ + { + id: "account-a", + username: "alice", + email: null, + admin: true, + disabled: false, + createdAt: now, + }, + ], + }, + }), + ); + await page.route("**/api/v2/admin/invites", (route) => { + if (route.request().method() === "POST") { + expect(route.request().headers()["x-csrf-token"]).toBe("session-csrf"); + expect(route.request().postDataJSON()).toEqual({ + email: "bob@example.com", + }); + invites = [pending, ...invites]; + return route.fulfill({ + status: 201, + json: { + invite: pending, + url: "https://diffx.example.com/invite#t=one-time", + }, + }); + } + return route.fulfill({ json: { invites } }); + }); + await page.route("**/api/v2/admin/invites/invite-a", (route) => { + expect(route.request().method()).toBe("DELETE"); + expect(route.request().headers()["x-csrf-token"]).toBe("session-csrf"); + invites = invites.map((invite) => + invite === pending ? { ...pending, status: "revoked" } : invite, + ); + return route.fulfill({ status: 204 }); + }); + await page.goto("/settings"); + await page + .getByRole("button", { name: "Manage accounts and invites" }) + .click(); + await expect(page.getByRole("heading", { name: "Admin" })).toBeVisible(); + expect(new URL(page.url()).pathname).toBe("/admin"); + await expect(page.getByText("Joined as carol")).toBeVisible(); + await expect(page.getByRole("button", { name: "Delete alice" })).toHaveCount( + 0, + ); + await page.getByLabel("Email", { exact: true }).fill("bob@example.com"); + await page.getByRole("button", { name: "Create invite" }).click(); + await expect( + page.getByRole("heading", { name: "Send this invite link" }), + ).toBeVisible(); + await expect(page.locator("pre")).toHaveText( + "https://diffx.example.com/invite#t=one-time", + ); + await page + .getByRole("button", { name: "Revoke invite for bob@example.com" }) + .click(); + const confirm = page.getByRole("dialog", { + name: "Revoke invite for bob@example.com?", + }); + await confirm.getByRole("button", { name: "Revoke invite" }).click(); + await expect(confirm).toBeHidden(); + await expect( + page.getByRole("heading", { name: "Send this invite link" }), + ).toHaveCount(0); + await expect(page.getByText("Revoked", { exact: true })).toBeVisible(); + await expect( + page.getByRole("button", { name: "Revoke invite for bob@example.com" }), + ).toHaveCount(0); +}); + +test("the admin disables, enables and deletes accounts after confirmation", async ({ + page, +}) => { + await remote(page, true, true); + let member = { + id: "account-b", + username: "bob", + email: "bob@example.com", + admin: false, + disabled: false, + createdAt: Date.now(), + }; + let deleted = false; + await page.route("**/api/v2/admin/invites", (route) => + route.fulfill({ json: { invites: [] } }), + ); + await page.route("**/api/v2/admin/users", (route) => + route.fulfill({ + json: { + users: [ + { + id: "account-a", + username: "alice", + email: null, + admin: true, + disabled: false, + createdAt: Date.now(), + }, + ...(deleted ? [] : [member]), + ], + }, + }), + ); + const calls: string[] = []; + await page.route("**/api/v2/admin/users/account-b**", (route) => { + expect(route.request().headers()["x-csrf-token"]).toBe("session-csrf"); + const path = new URL(route.request().url()).pathname; + calls.push(`${route.request().method()} ${path}`); + if (path.endsWith("/disable")) member = { ...member, disabled: true }; + else if (path.endsWith("/enable")) member = { ...member, disabled: false }; + else deleted = true; + return route.fulfill({ status: 204 }); + }); + await page.goto("/admin"); + await expect( + page.getByText("Invites need Google sign-in.", { exact: false }), + ).toBeVisible(); + await expect(page.getByLabel("Email", { exact: true })).toHaveCount(0); + await expect( + page.getByText("bob@example.com", { exact: false }), + ).toBeVisible(); + await page.getByRole("button", { name: "Disable bob" }).click(); + const disable = page.getByRole("dialog", { name: "Disable bob?" }); + await expect(disable.getByRole("button", { name: "Cancel" })).toBeFocused(); + await disable.getByRole("button", { name: "Disable account" }).click(); + await expect(disable).toBeHidden(); + await expect(page.getByText("Disabled", { exact: true })).toBeVisible(); + await page.getByRole("button", { name: "Enable bob" }).click(); + await expect(page.getByRole("button", { name: "Disable bob" })).toBeVisible(); + await page.getByRole("button", { name: "Delete bob" }).click(); + const remove = page.getByRole("dialog", { name: "Delete bob?" }); + await remove.getByRole("button", { name: "Delete account" }).click(); + await expect(remove).toBeHidden(); + await expect(page.getByText("bob@example.com", { exact: false })).toHaveCount( + 0, + ); + expect(calls).toEqual([ + "POST /api/v2/admin/users/account-b/disable", + "POST /api/v2/admin/users/account-b/enable", + "DELETE /api/v2/admin/users/account-b", + ]); +}); + +test("another invite link opened in the same tab loads that invite", async ({ + page, +}) => { + await remote(page, false, false, { googleEnabled: true }); + await page.route("**/api/v2/invites/lookup", (route) => { + const { token } = route.request().postDataJSON(); + return route.fulfill({ + json: { + email: token === "second" ? "carol@example.com" : "bob@example.com", + expiresAt: Date.now() + 86_400_000, + }, + }); + }); + await page.goto("/invite#t=first"); + await expect(page.getByText("bob@example.com")).toBeVisible(); + await page.evaluate(() => { + window.location.hash = "t=second"; + }); + await expect(page.getByText("carol@example.com")).toBeVisible(); + expect(new URL(page.url()).hash).toBe(""); +}); diff --git a/apps/web/test/browser/local-workspace.spec.ts b/apps/web/test/browser/local-workspace.spec.ts index 4b63e232..30a9249a 100644 --- a/apps/web/test/browser/local-workspace.spec.ts +++ b/apps/web/test/browser/local-workspace.spec.ts @@ -1796,7 +1796,12 @@ for (const mode of ["local", "remote"]) { googleEnabled: false, user: mode === "remote" - ? { id: "account-a", username: "alice", admin: false } + ? { + id: "account-a", + username: "alice", + email: null, + admin: false, + } : null, csrfToken: mode === "remote" ? "csrf" : null, collect: mode === "local", @@ -2574,7 +2579,7 @@ test("each remote account baselines its own seen heads", async ({ page }) => { json: { mode: "remote", googleEnabled: false, - user: { id: "account-b", username: "bob", admin: false }, + user: { id: "account-b", username: "bob", email: null, admin: false }, csrfToken: "csrf", collect: false, }, @@ -2920,7 +2925,7 @@ test("saved comparison preferences belong to the active account", async ({ json: { mode: "remote", googleEnabled: false, - user: { id: account, username: account, admin: false }, + user: { id: account, username: account, email: null, admin: false }, csrfToken: "test", collect: false, }, @@ -2947,7 +2952,7 @@ test("pull request reviews show GitHub identity and a sanitized description; clo json: { mode: "remote", googleEnabled: false, - user: { id: "account-a", username: "alice", admin: false }, + user: { id: "account-a", username: "alice", email: null, admin: false }, csrfToken: "csrf", collect: false, }, diff --git a/apps/web/test/identity.test.ts b/apps/web/test/identity.test.ts index ef3dad65..dee7bb98 100644 --- a/apps/web/test/identity.test.ts +++ b/apps/web/test/identity.test.ts @@ -34,13 +34,13 @@ test("server mode requires explicit bootstrap and consistent session identity", readBootstrap({ mode: "remote", googleEnabled: true, - user: { id: "owner", username: "alice", admin: true }, + user: { id: "owner", username: "alice", email: null, admin: true }, csrfToken: "secret", }), { mode: "remote", googleEnabled: true, - user: { id: "owner", username: "alice", admin: true }, + user: { id: "owner", username: "alice", email: null, admin: true }, csrfToken: "secret", }, ); @@ -74,7 +74,7 @@ test("server mode requires explicit bootstrap and consistent session identity", { mode: "remote", googleEnabled: false, - user: { id: "owner", username: "alice", admin: false }, + user: { id: "owner", username: "alice", email: null, admin: false }, csrfToken: null, }, { @@ -83,6 +83,12 @@ test("server mode requires explicit bootstrap and consistent session identity", user: { id: "owner", username: "alice" }, csrfToken: "secret", }, + { + mode: "remote", + googleEnabled: true, + user: { id: "owner", username: "alice", admin: false }, + csrfToken: "secret", + }, { mode: "remote", googleEnabled: false, user: null, csrfToken: "secret" }, ]) assert.throws( diff --git a/docs-website/src/pages/agent-hooks.md b/docs-website/src/pages/agent-hooks.md index a967098f..47a31a92 100644 --- a/docs-website/src/pages/agent-hooks.md +++ b/docs-website/src/pages/agent-hooks.md @@ -100,7 +100,7 @@ DIFFX_SERVER_INSECURE_COOKIES=true DIFFX_SERVER_STATE=./test-data/state.db diffx Open the setup link the server logs to create the admin account. The admin signs in with that password at `/admin/login`; everyone else signs in with Google from -an admin invite. Invites need a Google OAuth web client: set +an admin invite created at `/admin`. Invites need a Google OAuth web client: set `DIFFX_SERVER_GOOGLE_CLIENT_ID`, `DIFFX_SERVER_GOOGLE_CLIENT_SECRET` and `DIFFX_SERVER_PUBLIC_URL`, and register `/api/v2/auth/google/callback` as its redirect URI. Then open the top-right account settings to create diff --git a/docs/development.md b/docs/development.md index 64202543..ce2c498a 100644 --- a/docs/development.md +++ b/docs/development.md @@ -302,7 +302,10 @@ providers around identity; `SessionProvider` owns context selection, routes keys workspace owners by context. Lint rejects import cycles, Hook-rule violations and type assertions. The identity owner loads server mode and the current account before mounting review state; distributed login and `/settings` work with -zero observations. Account changes dispose workspace requests and event streams. +zero observations. Signed out, `/setup`, `/admin/login` and `/invite` render +their pages and every other path renders Google sign-in; `/admin` renders only +for the admin. Setup and invite pages read their secret from the fragment and +drop it from history before any request. Account changes dispose workspace requests and event streams. Local mode bypasses account flows. Consumers subscribe through domain hooks for diff source, navigation, draft, review, reviewed files, and collapse state. Collapsed diff files (per stream and scope), closed tree folders and Hide reviewed files (per diff --git a/docs/system.md b/docs/system.md index 172c7449..de49792b 100644 --- a/docs/system.md +++ b/docs/system.md @@ -377,8 +377,8 @@ once. A Google account without an account or invite cannot sign up. Without Goog configuration the server runs admin-only: Google routes are absent and invites cannot be created. -The admin lists accounts and disables, enables or deletes any account except -itself. Disabling revokes the account's browser sessions and API tokens in the +The admin manages accounts and invites at `/admin`, linked from its Settings. +It lists accounts and disables, enables or deletes any account except itself. Disabling revokes the account's browser sessions and API tokens in the same transaction and refuses new ones; enabling restores sign-in, not those credentials. Deleting removes the account and everything it owns, including queued ingestion jobs. Both stop the account's active services, and the diff --git a/internal/identityservice/google.go b/internal/identityservice/google.go index a7d37490..7341c250 100644 --- a/internal/identityservice/google.go +++ b/internal/identityservice/google.go @@ -95,6 +95,9 @@ type GoogleStore interface { AcceptInvite(ctx context.Context, inviteID string, account Account, identity google.Identity, now time.Time) error // GoogleAccount resolves a linked subject, recording the latest email. GoogleAccount(ctx context.Context, identity google.Identity, now time.Time) (account Account, disabled bool, err error) + // GoogleEmail is the latest email Google reported for the account; "" for + // accounts without a Google identity. + GoogleEmail(ctx context.Context, accountID string) (string, error) PutGoogleFlow(ctx context.Context, stateHash string, flow GoogleFlow) error // TakeGoogleFlow consumes an unexpired flow exactly once. TakeGoogleFlow(ctx context.Context, stateHash string, now time.Time) (GoogleFlow, error) @@ -102,6 +105,11 @@ type GoogleStore interface { func (service *Service) GoogleEnabled() bool { return service.google != nil } +// GoogleEmail is the account's latest Google email, or "" for the admin. +func (service *Service) GoogleEmail(ctx context.Context, accountID string) (string, error) { + return service.store.GoogleEmail(ctx, accountID) +} + func canonicalEmail(email string) (string, error) { email = strings.ToLower(strings.TrimSpace(email)) local, domain, found := strings.Cut(email, "@") diff --git a/internal/remoteserver/google_test.go b/internal/remoteserver/google_test.go index 6ad068af..c2de6524 100644 --- a/internal/remoteserver/google_test.go +++ b/internal/remoteserver/google_test.go @@ -177,7 +177,7 @@ func TestInviteSignupThenGoogleSignInByStableSubject(t *testing.T) { t.Fatalf("state reused: %s", w.Header().Get("Location")) } w = identityRequest(handler, "GET", "/api/v2/bootstrap", "", member, "", "") - if !strings.Contains(w.Body.String(), `"username":"alice.smith","admin":false`) { + if !strings.Contains(w.Body.String(), `"username":"alice.smith","email":"alice.smith@example.com","admin":false`) { t.Fatalf("member identity: %s", w.Body.String()) } if w := identityRequest(handler, "POST", "/api/v2/invites/lookup", `{"token":"`+secret+`"}`, nil, "", ""); w.Code != 404 { @@ -192,7 +192,7 @@ func TestInviteSignupThenGoogleSignInByStableSubject(t *testing.T) { state = fake.authorize(t, authorize, "renamed", "subject-a", "alice@new.example.com", true) again := sessionFrom(t, callback(handler, state, "renamed", flow)) w = identityRequest(handler, "GET", "/api/v2/bootstrap", "", again, "", "") - if !strings.Contains(w.Body.String(), `"username":"alice.smith"`) { + if !strings.Contains(w.Body.String(), `"username":"alice.smith","email":"alice@new.example.com"`) { t.Fatalf("email change changed account: %s", w.Body.String()) } w = identityRequest(handler, "GET", "/api/v2/admin/users", "", admin, "", "") diff --git a/internal/remoteserver/identity.go b/internal/remoteserver/identity.go index c8bdc5ff..b0c4abee 100644 --- a/internal/remoteserver/identity.go +++ b/internal/remoteserver/identity.go @@ -39,9 +39,10 @@ type principal struct { purpose identityservice.Purpose } type accountJSON struct { - ID string `json:"id"` - Username string `json:"username"` - Admin bool `json:"admin"` + ID string `json:"id"` + Username string `json:"username"` + Email *string `json:"email"` + Admin bool `json:"admin"` } type bootstrapJSON struct { Mode string `json:"mode"` @@ -50,10 +51,18 @@ type bootstrapJSON struct { CSRFToken *string `json:"csrfToken"` } -func (remote *transport) bootstrap(w http.ResponseWriter, p principal) { +func (remote *transport) bootstrap(w http.ResponseWriter, r *http.Request, p principal) { result := bootstrapJSON{Mode: "remote", GoogleEnabled: remote.identity.GoogleEnabled()} if p.session != nil { - result.User = &accountJSON{p.account.ID, p.account.Username, p.account.Admin} + email, err := remote.identity.GoogleEmail(r.Context(), p.account.ID) + if err != nil { + identityError(w, err) + return + } + result.User = &accountJSON{ID: p.account.ID, Username: p.account.Username, Admin: p.account.Admin} + if email != "" { + result.User.Email = &email + } result.CSRFToken = &p.session.CSRFToken } writeJSON(w, 200, result) @@ -116,7 +125,7 @@ func (remote *transport) ServeHTTP(w http.ResponseWriter, r *http.Request) { problem(w, 405, "Method not allowed") return } - remote.bootstrap(w, p) + remote.bootstrap(w, r, p) return } if !strings.HasPrefix(r.URL.Path, "/api/") && !strings.HasPrefix(r.URL.Path, "/mcp") && !strings.HasPrefix(r.URL.Path, "/control/") { @@ -332,7 +341,7 @@ func (remote *transport) auth(w http.ResponseWriter, r *http.Request) { return } remote.cookie(w, session.Secret, session.ExpiresAt) - remote.bootstrap(w, principal{account: session.Account, session: &session}) + remote.bootstrap(w, r, principal{account: session.Account, session: &session}) } func (remote *transport) cookie(w http.ResponseWriter, secret string, expiry time.Time) { cookie := &http.Cookie{Name: sessionCookie, Value: secret, Path: "/", HttpOnly: true, Secure: !remote.settings.InsecureCookies, SameSite: http.SameSiteLaxMode, Expires: expiry} diff --git a/internal/remoteserver/identity_test.go b/internal/remoteserver/identity_test.go index f2d7bf26..e7ef602f 100644 --- a/internal/remoteserver/identity_test.go +++ b/internal/remoteserver/identity_test.go @@ -239,7 +239,7 @@ func TestPasswordLoginIsAdminOnly(t *testing.T) { t.Fatalf("public signup route: %d", w.Code) } w := identityRequest(handler, "GET", "/api/v2/bootstrap", "", cookie, "", "") - if strings.Contains(w.Body.String(), "signupEnabled") || !strings.Contains(w.Body.String(), `"googleEnabled":false`) || !strings.Contains(w.Body.String(), `"admin":true`) { + if strings.Contains(w.Body.String(), "signupEnabled") || !strings.Contains(w.Body.String(), `"googleEnabled":false`) || !strings.Contains(w.Body.String(), `"email":null,"admin":true`) { t.Fatalf("bootstrap: %s", w.Body.String()) } } diff --git a/internal/reviewstore/invites.go b/internal/reviewstore/invites.go index 64199c85..486fef86 100644 --- a/internal/reviewstore/invites.go +++ b/internal/reviewstore/invites.go @@ -151,6 +151,15 @@ func (store *Store) GoogleAccount(ctx context.Context, identity google.Identity, return account, disabled, err } +func (store *Store) GoogleEmail(ctx context.Context, accountID string) (string, error) { + var email string + err := store.db.QueryRowContext(ctx, `SELECT email FROM google_identities WHERE user_id=?`, accountID).Scan(&email) + if errors.Is(err, sql.ErrNoRows) { + return "", nil + } + return email, err +} + func (store *Store) PutGoogleFlow(ctx context.Context, stateHash string, flow identityservice.GoogleFlow) error { // Expired flows are removed when new ones start; consumed flows are deleted. if _, err := store.db.ExecContext(ctx, `DELETE FROM google_auth_states WHERE expires_at<=?`, time.Now().UnixMilli()); err != nil { diff --git a/packages/api/openapi.yaml b/packages/api/openapi.yaml index f80a3530..7af85778 100644 --- a/packages/api/openapi.yaml +++ b/packages/api/openapi.yaml @@ -1489,12 +1489,15 @@ components: AccountUser: type: object additionalProperties: false - required: [id, username, admin] + required: [id, username, email, admin] properties: id: type: string username: type: string + email: + type: [string, "null"] + description: The latest email Google reported for the account; null for the admin. admin: type: boolean description: The single account that signs in with a password and manages other accounts. diff --git a/packages/api/src/client.ts b/packages/api/src/client.ts index 9bd53b09..10888593 100644 --- a/packages/api/src/client.ts +++ b/packages/api/src/client.ts @@ -25,6 +25,8 @@ export type ApiCollectionFailure = components["schemas"]["CollectionFailure"]; export type ApiPullRequest = components["schemas"]["PullRequest"]; export type ApiGitHubConnection = components["schemas"]["GitHubConnection"]; export type ApiGitHubRepository = components["schemas"]["GitHubRepository"]; +export type ApiAdminUser = components["schemas"]["AdminUser"]; +export type ApiInvite = components["schemas"]["Invite"]; export function errorDetail(error: unknown, fallback: string) { if (error instanceof Error) return error.message; @@ -76,6 +78,7 @@ export function readBootstrap(value: unknown): ApiBootstrap { value.user.id.length > 0 && typeof value.user.username === "string" && value.user.username.length > 0 && + (typeof value.user.email === "string" || value.user.email === null) && typeof value.user.admin === "boolean" && typeof value.csrfToken === "string" && value.csrfToken.length > 0 @@ -86,6 +89,7 @@ export function readBootstrap(value: unknown): ApiBootstrap { user: { id: value.user.id, username: value.user.username, + email: value.user.email, admin: value.user.admin, }, csrfToken: value.csrfToken, diff --git a/packages/api/src/schema.d.ts b/packages/api/src/schema.d.ts index 56473bd8..6cad1643 100644 --- a/packages/api/src/schema.d.ts +++ b/packages/api/src/schema.d.ts @@ -823,6 +823,8 @@ export interface components { AccountUser: { id: string; username: string; + /** @description The latest email Google reported for the account; null for the admin. */ + email: string | null; /** @description The single account that signs in with a password and manages other accounts. */ admin: boolean; };