diff --git a/apps/web/src/account.tsx b/apps/web/src/account.tsx index 91976fb6..341679de 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 { useState, type FormEvent } from "react"; +import { useEffect, useState, type FormEvent } from "react"; import { ErrorState } from "@/components/error-state"; import { IconButton } from "@/components/icon-button"; import { Button } from "@/components/ui/button"; @@ -31,27 +31,133 @@ export function AccountButton({ variant }: { variant: "rail" | "shell" }) { ); } +/** Members sign in with Google; accounts come only from invites. */ export function SignIn() { - const { state, establish } = useIdentity(); - const [signup, setSignup] = useState(false); + return ( +
+ + diffx + +

Sign in

+

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

+
+ ); +} + +/** Password sign-in for the admin only, at the unlinked /admin/login. */ +export function AdminSignIn() { + return ( +
+ + diffx + +

Admin sign in

+

+ Manage this server and review your collected changes. +

+ +
+ ); +} + +/** One-time admin creation from the setup link the server logged at startup. */ +export function AdminSetup() { + const [token] = useState( + () => new URLSearchParams(window.location.hash.slice(1)).get("t") ?? "", + ); + const [setup, setSetup] = useState<"loading" | "required" | "complete">( + "loading", + ); + const [error, setError] = useState(""); + useEffect(() => { + // The secret stays out of history once read. + window.history.replaceState(null, "", "/setup"); + const controller = new AbortController(); + void api + .GET("/api/v2/setup", { signal: controller.signal }) + .then(({ data, error }) => { + if (controller.signal.aborted) return; + if (!data) + throw new Error(errorDetail(error, "Unable to load setup status")); + setSetup(data.required ? "required" : "complete"); + }) + .catch((failure: unknown) => { + if (!controller.signal.aborted) + setError(errorDetail(failure, "Unable to load setup status")); + }); + return () => controller.abort(); + }, []); + return ( +
+ + diffx + +

Set up diffx

+ {error && } + {setup === "complete" && ( +

+ This server already has an admin.{" "} + Sign in as admin. +

+ )} + {setup === "required" && !token && ( +

+ Open the setup link from the server log to create the admin account. +

+ )} + {setup === "required" && token && ( + <> +

+ Create the admin account. It signs in with this password and invites + everyone else. +

+ + + )} +
+ ); +} + +function CredentialForm({ + path, + token, + submitLabel, + failure, +}: { + path: "/api/v2/auth/login" | "/api/v2/setup"; + token: string; + submitLabel: string; + failure: string; +}) { + const { establish } = useIdentity(); const [username, setUsername] = useState(""); const [password, setPassword] = useState(""); const [busy, setBusy] = useState(false); const [error, setError] = useState(""); - const signupEnabled = - state.status === "ready" && state.bootstrap.signupEnabled; + const creating = path === "/api/v2/setup"; async function submit(event: FormEvent) { event.preventDefault(); if (busy) return; setBusy(true); setError(""); try { - const { data, error } = await api.POST( - signup ? "/api/v2/auth/signup" : "/api/v2/auth/login", - { body: { username, password } }, - ); - if (!data) - throw new Error(errorDetail(error, "Unable to sign in. Try again.")); + const { data, error } = creating + ? await api.POST(path, { body: { token, username, password } }) + : await api.POST(path, { body: { username, password } }); + if (!data) throw new Error(errorDetail(error, failure)); const bootstrap = readBootstrap(data); if ( bootstrap.mode !== "remote" || @@ -63,82 +169,57 @@ export function SignIn() { ); establish(bootstrap); setPassword(""); - } catch (failure: unknown) { - setError(errorDetail(failure, "Unable to sign in. Try again.")); + } catch (failed: unknown) { + setError(errorDetail(failed, failure)); } finally { setBusy(false); } } return ( -
- - diffx - -

- {signup ? "Create your account" : "Sign in"} -

-

- Review your collected changes on this server. -

-
- - setUsername(event.target.value)} - required - minLength={3} - maxLength={32} - disabled={busy} - /> - - setPassword(event.target.value)} - required - minLength={signup ? 12 : undefined} - maxLength={128} - disabled={busy} - /> - {signup && ( -

- Use at least 12 characters. -

- )} - {error && } - - - {signupEnabled && ( - +
+ + setUsername(event.target.value)} + required + minLength={3} + maxLength={32} + disabled={busy} + /> + + setPassword(event.target.value)} + required + minLength={creating ? 12 : undefined} + maxLength={128} + disabled={busy} + /> + {creating && ( +

+ Use at least 12 characters. +

)} -
+ {error && } + + ); } @@ -176,7 +257,7 @@ export function AccountSettings() {

- + {state.bootstrap.user.admin && }
window.removeEventListener("popstate", changed); }, []); - const localSettings = - state.status === "ready" && - state.bootstrap.mode === "local" && - path === "/settings"; + 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 misplaced = + ready && + ((!remote && path === "/settings") || + (signedIn && (path === "/setup" || path === "/admin/login"))); useEffect(() => { - if (!localSettings) return; + if (!misplaced) return; window.history.replaceState(null, "", "/"); setPath("/"); - }, [localSettings]); + }, [misplaced]); if (state.status !== "ready") return ; - if (state.bootstrap.mode === "remote" && !state.bootstrap.user) + if (state.bootstrap.mode === "remote" && !state.bootstrap.user) { + if (path === "/setup") return ; + if (path === "/admin/login") return ; return ; + } if (path === "/settings" && state.bootstrap.mode === "remote") return ; return ( diff --git a/apps/web/test/browser/fixtures.ts b/apps/web/test/browser/fixtures.ts index 79499490..d2032e29 100644 --- a/apps/web/test/browser/fixtures.ts +++ b/apps/web/test/browser/fixtures.ts @@ -88,7 +88,7 @@ export async function withReviewNavigation( route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect, diff --git a/apps/web/test/browser/identity.spec.ts b/apps/web/test/browser/identity.spec.ts index 579eb231..68547f2a 100644 --- a/apps/web/test/browser/identity.spec.ts +++ b/apps/web/test/browser/identity.spec.ts @@ -1,18 +1,17 @@ import { type Page } from "@playwright/test"; import { expect, test } from "./fixtures.ts"; -const account = { id: "account-a", username: "alice" }; -function bootstrap(signedIn: boolean, signupEnabled = false) { +function bootstrap(signedIn: boolean, admin = false) { return { mode: "remote", - signupEnabled, - user: signedIn ? account : null, + googleEnabled: false, + user: signedIn ? { id: "account-a", username: "alice", admin } : null, csrfToken: signedIn ? "session-csrf" : null, }; } -async function remote(page: Page, signedIn = true, signupEnabled = false) { +async function remote(page: Page, signedIn = true, admin = false) { await page.route("**/api/v2/bootstrap", (route) => - route.fulfill({ json: bootstrap(signedIn, signupEnabled) }), + route.fulfill({ json: bootstrap(signedIn, admin) }), ); await page.route("**/api/v2/contexts?*", (route) => route.fulfill({ json: { contexts: [], nextCursor: null } }), @@ -59,58 +58,81 @@ test("bootstrap failure stays an error and retries without inferring local mode" await expect(page.getByRole("heading", { name: "Sign in" })).toBeVisible(); }); -test("disabled signup and invalid login preserve recovery", async ({ +test("members get no password form; invalid admin login preserves recovery", async ({ page, }) => { await remote(page, false); await page.route("**/api/v2/auth/login", (route) => route.fulfill({ status: 401, - json: { detail: "Invalid username or password" }, + json: { detail: "Authentication required" }, }), ); await page.goto("/"); - await expect( - page.getByRole("button", { name: "Create an account" }), - ).toHaveCount(0); + await expect(page.getByRole("heading", { name: "Sign in" })).toBeVisible(); + await expect(page.getByLabel("Password", { exact: true })).toHaveCount(0); + await page.goto("/admin/login"); await page.getByLabel("Username", { exact: true }).fill("alice"); await page.getByLabel("Password", { exact: true }).fill("incorrect-password"); await page.getByRole("button", { name: "Sign in", exact: true }).click(); - await expect(page.getByRole("alert")).toHaveText( - "Invalid username or password", - ); + await expect(page.getByRole("alert")).toHaveText("Authentication required"); await expect(page.getByLabel("Username", { exact: true })).toHaveValue( "alice", ); }); -test("signup opens an empty account whose settings work before any review", async ({ +test("the setup link creates the admin once and keeps its secret out of the URL", async ({ page, }) => { - await remote(page, false, true); - await page.route("**/api/v2/auth/signup", (route) => - route.fulfill({ json: bootstrap(true, true) }), - ); - await page.goto("/"); - await page.getByRole("button", { name: "Create an account" }).click(); + await remote(page, false); + await page.route("**/api/v2/setup", (route) => { + if (route.request().method() === "GET") + return route.fulfill({ json: { required: true } }); + expect(route.request().postDataJSON()).toEqual({ + token: "setup-secret", + username: "alice", + password: "a-secure-password", + }); + return route.fulfill({ json: bootstrap(true, true) }); + }); + await page.goto("/setup#t=setup-secret"); + await expect( + page.getByRole("heading", { name: "Set up diffx" }), + ).toBeVisible(); + expect(new URL(page.url()).hash).toBe(""); await page.getByLabel("Username", { exact: true }).fill("alice"); await page.getByLabel("Password", { exact: true }).fill("a-secure-password"); - await page - .getByRole("button", { name: "Create account", exact: true }) - .click(); + await page.getByRole("button", { name: "Create admin", exact: true }).click(); await expect( page.getByRole("heading", { name: "No review contexts yet" }), ).toBeVisible(); + expect(new URL(page.url()).pathname).toBe("/"); await page.getByRole("button", { name: "Account settings" }).click(); - await expect(page).toHaveURL(/\/settings$/); - await expect( - page.getByRole("heading", { name: "Account settings" }), - ).toBeVisible(); + await expect(page.getByRole("heading", { name: "Password" })).toBeVisible(); await expect( page.getByText("No tokens yet.", { exact: false }), ).toBeVisible(); }); +test("a completed setup points to admin sign in", async ({ page }) => { + await remote(page, false); + await page.route("**/api/v2/setup", (route) => + route.fulfill({ json: { required: false } }), + ); + await page.goto("/setup#t=stale-secret"); + await expect( + page.getByRole("link", { name: "Sign in as admin" }), + ).toHaveAttribute("href", "/admin/login"); + await expect(page.getByLabel("Password", { exact: true })).toHaveCount(0); +}); + +test("member settings have no password section", async ({ page }) => { + await remote(page); + await page.goto("/settings"); + await expect(page.getByRole("heading", { name: "API tokens" })).toBeVisible(); + await expect(page.getByRole("heading", { name: "Password" })).toHaveCount(0); +}); + test("collector token secret appears once, with owner CSRF and revocation", async ({ page, }) => { @@ -283,7 +305,7 @@ test("expired account requests remove workspace identity and return to sign in", test("password change sends CSRF and signs out after revoking browser sessions", async ({ page, }) => { - await remote(page); + await remote(page, true, true); await page.route("**/api/v2/account/password", (route) => { expect(route.request().headers()["x-csrf-token"]).toBe("session-csrf"); expect(route.request().postDataJSON()).toEqual({ @@ -303,7 +325,7 @@ test("password change sends CSRF and signs out after revoking browser sessions", .getByRole("button", { name: "Change password and sign out" }) .click(); await expect(page.getByRole("heading", { name: "Sign in" })).toBeVisible(); - await expect(page.getByLabel("Password", { exact: true })).toHaveValue(""); + await expect(page.getByLabel("Password", { exact: true })).toHaveCount(0); }); test("logout failure stays recoverable and success disposes account UI", async ({ diff --git a/apps/web/test/browser/local-workspace.spec.ts b/apps/web/test/browser/local-workspace.spec.ts index 8eaf194a..4b63e232 100644 --- a/apps/web/test/browser/local-workspace.spec.ts +++ b/apps/web/test/browser/local-workspace.spec.ts @@ -1793,9 +1793,11 @@ for (const mode of ["local", "remote"]) { route.fulfill({ json: { mode, - signupEnabled: false, + googleEnabled: false, user: - mode === "remote" ? { id: "account-a", username: "alice" } : null, + mode === "remote" + ? { id: "account-a", username: "alice", admin: false } + : null, csrfToken: mode === "remote" ? "csrf" : null, collect: mode === "local", input: "checkouts", @@ -1925,7 +1927,7 @@ test("local first launch waits for collection with an empty catalog", async ({ route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2033,7 +2035,7 @@ for (const initialHeads of ["stored", "arriving"]) { route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2112,7 +2114,7 @@ test("local root waits for collection to open its default over earlier heads", a route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2175,7 +2177,7 @@ test("unresolved navigation joins collection without reporting a missing stream" route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2280,7 +2282,7 @@ for (const path of ["/", "/streams/local-stream"]) { route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2337,7 +2339,7 @@ test("local startup reports failed checkouts while opening collected changes", a route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2371,7 +2373,7 @@ test("local live URLs sync the checkouts on every load", async ({ page }) => { route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2478,7 +2480,7 @@ test("local sync collects every checkout without replacing the open review", asy route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2571,8 +2573,8 @@ test("each remote account baselines its own seen heads", async ({ page }) => { route.fulfill({ json: { mode: "remote", - signupEnabled: false, - user: { id: "account-b", username: "bob" }, + googleEnabled: false, + user: { id: "account-b", username: "bob", admin: false }, csrfToken: "csrf", collect: false, }, @@ -2680,7 +2682,7 @@ test("scope history keeps live snapshot and draft through newer heads, then foll route.fulfill({ json: { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -2917,8 +2919,8 @@ test("saved comparison preferences belong to the active account", async ({ route.fulfill({ json: { mode: "remote", - signupEnabled: false, - user: { id: account, username: account }, + googleEnabled: false, + user: { id: account, username: account, admin: false }, csrfToken: "test", collect: false, }, @@ -2944,8 +2946,8 @@ test("pull request reviews show GitHub identity and a sanitized description; clo route.fulfill({ json: { mode: "remote", - signupEnabled: false, - user: { id: "account-a", username: "alice" }, + googleEnabled: false, + user: { id: "account-a", username: "alice", admin: false }, csrfToken: "csrf", collect: false, }, diff --git a/apps/web/test/identity.test.ts b/apps/web/test/identity.test.ts index 998487eb..ef3dad65 100644 --- a/apps/web/test/identity.test.ts +++ b/apps/web/test/identity.test.ts @@ -6,7 +6,7 @@ test("server mode requires explicit bootstrap and consistent session identity", assert.deepEqual( readBootstrap({ mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -14,7 +14,7 @@ test("server mode requires explicit bootstrap and consistent session identity", }), { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: true, @@ -24,34 +24,48 @@ test("server mode requires explicit bootstrap and consistent session identity", assert.deepEqual( readBootstrap({ mode: "remote", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, }), - { mode: "remote", signupEnabled: false, user: null, csrfToken: null }, + { mode: "remote", googleEnabled: false, user: null, csrfToken: null }, + ); + assert.deepEqual( + readBootstrap({ + mode: "remote", + googleEnabled: true, + user: { id: "owner", username: "alice", admin: true }, + csrfToken: "secret", + }), + { + mode: "remote", + googleEnabled: true, + user: { id: "owner", username: "alice", admin: true }, + csrfToken: "secret", + }, ); for (const invalid of [ null, {}, { mode: "local", - signupEnabled: true, + googleEnabled: true, user: null, csrfToken: null, collect: false, input: "capture", }, - { mode: "local", signupEnabled: false, user: null, csrfToken: null }, + { mode: "local", googleEnabled: false, user: null, csrfToken: null }, { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: false, }, { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: false, @@ -59,11 +73,17 @@ test("server mode requires explicit bootstrap and consistent session identity", }, { mode: "remote", - signupEnabled: false, - user: { id: "owner", username: "alice" }, + googleEnabled: false, + user: { id: "owner", username: "alice", admin: false }, csrfToken: null, }, - { mode: "remote", signupEnabled: false, user: null, csrfToken: "secret" }, + { + mode: "remote", + googleEnabled: false, + user: { id: "owner", username: "alice" }, + csrfToken: "secret", + }, + { mode: "remote", googleEnabled: false, user: null, csrfToken: "secret" }, ]) assert.throws( () => readBootstrap(invalid), diff --git a/cmd/diffx-server/main.go b/cmd/diffx-server/main.go index 93f73393..a61e6515 100644 --- a/cmd/diffx-server/main.go +++ b/cmd/diffx-server/main.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "flag" "fmt" "io" @@ -20,13 +21,12 @@ import ( ) func parseSettings(arguments []string, stderr io.Writer) (remoteserver.Settings, bool, error) { - settings := remoteserver.Settings{Password: os.Getenv("DIFFX_SERVER_PASSWORD")} + var settings remoteserver.Settings flags := flag.NewFlagSet("diffx-server", flag.ContinueOnError) flags.SetOutput(stderr) flags.StringVar(&settings.Listen, "listen", net.JoinHostPort("127.0.0.1", strconv.Itoa(config.DefaultServerPort)), "HTTP listen address; terminate TLS at the deployment boundary") flags.StringVar(&settings.State, "state", "", "SQLite database path") - flags.StringVar(&settings.Username, "username", "", "bootstrap username; password comes from DIFFX_SERVER_PASSWORD") - flags.BoolVar(&settings.SignupEnabled, "signup-enabled", false, "allow public signup") + flags.StringVar(&settings.PublicURL, "public-url", "", "browser-facing https origin used in the logged admin setup link") flags.BoolVar(&settings.InsecureCookies, "insecure-cookies", false, "allow browser sessions over HTTP for development") flags.IntVar(&settings.RetentionDays, "retention-days", 7, "snapshot retention in days") path := flags.String("config-file", "", "optional read-only server JSON config path; defaults to DIFFX_SERVER_CONFIG") @@ -55,11 +55,8 @@ func parseSettings(arguments []string, stderr io.Writer) (remoteserver.Settings, if !visited["state"] || settings.State == "" { settings.State = values.State } - if !visited["username"] { - settings.Username = values.Username - } - if !visited["signup-enabled"] { - settings.SignupEnabled = values.SignupEnabled + if !visited["public-url"] { + settings.PublicURL = values.PublicURL } if !visited["insecure-cookies"] { settings.InsecureCookies = values.InsecureCookies @@ -73,8 +70,9 @@ func parseSettings(arguments []string, stderr io.Writer) (remoteserver.Settings, if settings.State == "" { return settings, false, fmt.Errorf("state must be a database path or memory") } - if (settings.Username == "") != (settings.Password == "") { - return settings, false, fmt.Errorf("DIFFX_SERVER_USERNAME and DIFFX_SERVER_PASSWORD must be supplied together") + values.PublicURL, values.InsecureCookies = settings.PublicURL, settings.InsecureCookies + if err := values.ValidatePublicURL(); err != nil { + return settings, false, err } if _, err := remoteserver.Retention(settings.RetentionDays); err != nil { return settings, false, err @@ -124,17 +122,18 @@ func main() { } } -// Offline administration shares the server's exclusive database lock. -// Passwords are read from an explicitly selected environment variable or stdin; -// they are never command arguments or printed output. +// Offline admin recovery shares the server's exclusive database lock. Only the +// admin has a password; the setup link creates it. Passwords are read from an +// explicitly selected environment variable or stdin; they are never command +// arguments or printed output. func manageUser(arguments []string, stdin io.Reader, stdout, stderr io.Writer) error { - if len(arguments) == 0 || (arguments[0] != "create" && arguments[0] != "reset-password") { - return fmt.Errorf("usage: diffx-server user create|reset-password --username NAME --password-env VAR|--password-stdin [--state PATH]") + if len(arguments) == 0 || arguments[0] != "reset-password" { + return fmt.Errorf("usage: diffx-server user reset-password --username ADMIN --password-env VAR|--password-stdin [--state PATH]") } operation := arguments[0] flags := flag.NewFlagSet("diffx-server user "+operation, flag.ContinueOnError) flags.SetOutput(stderr) - username := flags.String("username", "", "account username") + username := flags.String("username", "", "admin username") state := flags.String("state", "", "SQLite database path; server must be stopped") configPath := flags.String("config-file", "", "optional server JSON config path") passwordEnv := flags.String("password-env", "", "read password from this environment variable") @@ -173,11 +172,9 @@ func manageUser(arguments []string, stdin io.Reader, stdout, stderr io.Writer) e return err } defer store.Close() - identity := identityservice.New(store) - if operation == "create" { - _, err = identity.Provision(context.Background(), *username, password) - } else { - err = identity.ResetPassword(context.Background(), *username, password) + err = identityservice.New(store).ResetPassword(context.Background(), *username, password) + if errors.Is(err, identityservice.ErrNotFound) { + return fmt.Errorf("no admin account named %s", *username) } if err != nil { return err diff --git a/cmd/diffx-server/main_test.go b/cmd/diffx-server/main_test.go index 67c49a36..d2bf33cd 100644 --- a/cmd/diffx-server/main_test.go +++ b/cmd/diffx-server/main_test.go @@ -11,6 +11,7 @@ import ( "github.com/flexdinesh/diffx/internal/identityservice" "github.com/flexdinesh/diffx/internal/reviewstore" + "github.com/flexdinesh/diffx/internal/testsupport" ) func TestRemoteConfigMappingAndPrecedence(t *testing.T) { @@ -47,45 +48,38 @@ func TestRemoteConfigMappingAndPrecedence(t *testing.T) { } } -func TestUserCommandsPasswordLifecycleAndLock(t *testing.T) { +func TestResetPasswordRecoversOnlyTheAdmin(t *testing.T) { path := filepath.Join(t.TempDir(), "state.db") - var stdout, stderr bytes.Buffer - t.Setenv("TEST_ACCOUNT_PASSWORD", "initial-password-123") - args := []string{"create", "--username", "alice", "--state", path, "--password-env", "TEST_ACCOUNT_PASSWORD"} - if err := manageUser(args, strings.NewReader(""), &stdout, &stderr); err != nil { - t.Fatal(err) - } - if strings.Contains(stdout.String(), "password-123") { - t.Fatal("password printed") - } store, err := reviewstore.Open(path) if err != nil { t.Fatal(err) } - identity := identityservice.New(store) - session, err := identity.Login(t.Context(), "alice", "initial-password-123") - if err != nil { - t.Fatal(err) - } - stdout.Reset() - if err := manageUser(args, strings.NewReader(""), &stdout, &stderr); err == nil || !strings.Contains(err.Error(), "already in use") { + session := testsupport.RemoteAdmin(t, store, "alice", "initial-password-123") + testsupport.RemoteAccount(t, store, "member") + var stdout, stderr bytes.Buffer + args := []string{"reset-password", "--username", "alice", "--state", path, "--password-stdin"} + if err := manageUser(args, strings.NewReader("changed-password-123\n"), &stdout, &stderr); err == nil || !strings.Contains(err.Error(), "already in use") { t.Fatalf("operator bypassed lock: %v", err) } if err := store.Close(); err != nil { t.Fatal(err) } - if err := manageUser(args, strings.NewReader(""), &stdout, &stderr); err == nil { - t.Fatal("duplicate account") + if err := manageUser([]string{"reset-password", "--username", "member", "--state", path, "--password-stdin"}, strings.NewReader("member-password-123\n"), &stdout, &stderr); err == nil || !strings.Contains(err.Error(), "no admin account named member") { + t.Fatalf("member gained a password: %v", err) } - if err := manageUser([]string{"reset-password", "--username", "alice", "--state", path, "--password-stdin"}, strings.NewReader("changed-password-123\n"), &stdout, &stderr); err != nil { + t.Setenv("TEST_ACCOUNT_PASSWORD", "changed-password-123") + if err := manageUser([]string{"reset-password", "--username", "alice", "--state", path, "--password-env", "TEST_ACCOUNT_PASSWORD"}, strings.NewReader(""), &stdout, &stderr); err != nil { t.Fatal(err) } + if strings.Contains(stdout.String(), "password-123") { + t.Fatal("password printed") + } store, err = reviewstore.Open(path) if err != nil { t.Fatal(err) } defer store.Close() - identity = identityservice.New(store) + identity := identityservice.New(store) if _, err := identity.Login(t.Context(), "alice", "initial-password-123"); err == nil { t.Fatal("old password accepted") } @@ -97,7 +91,7 @@ func TestUserCommandsPasswordLifecycleAndLock(t *testing.T) { } } func TestUserCommandValidation(t *testing.T) { - for _, args := range [][]string{nil, {"delete"}, {"create"}, {"create", "--username", "alice"}, {"create", "--username", "alice", "--password-env", "P", "--password-stdin"}, {"create", "--username", "alice", "--state", ":memory:", "--password-stdin"}} { + for _, args := range [][]string{nil, {"delete"}, {"create", "--username", "alice", "--password-stdin"}, {"reset-password"}, {"reset-password", "--username", "alice"}, {"reset-password", "--username", "alice", "--password-env", "P", "--password-stdin"}, {"reset-password", "--username", "alice", "--state", ":memory:", "--password-stdin"}} { if err := manageUser(args, strings.NewReader("password-123456"), io.Discard, io.Discard); err == nil { t.Fatalf("accepted invalid args: %v", args) } @@ -116,25 +110,37 @@ func TestRemoteConfigDefaultsAndVersion(t *testing.T) { } func TestRemoteIdentityConfigPrecedence(t *testing.T) { - t.Setenv("DIFFX_SERVER_PASSWORD", "password-123456") path := filepath.Join(t.TempDir(), "server.json") - if err := os.WriteFile(path, []byte(`{"username":"file-admin","signupEnabled":true,"insecureCookies":true}`), 0o400); err != nil { + if err := os.WriteFile(path, []byte(`{"publicUrl":"https://file.example.com","insecureCookies":true}`), 0o400); err != nil { t.Fatal(err) } settings, _, err := parseSettings([]string{"--config-file", path}, io.Discard) - if err != nil || settings.Username != "file-admin" || !settings.SignupEnabled || !settings.InsecureCookies { - t.Fatalf("file account: %+v %v", settings, err) + if err != nil || settings.PublicURL != "https://file.example.com" || !settings.InsecureCookies { + t.Fatalf("file identity: %+v %v", settings, err) } - t.Setenv("DIFFX_SERVER_USERNAME", "env-admin") - t.Setenv("DIFFX_SERVER_SIGNUP_ENABLED", "false") + t.Setenv("DIFFX_SERVER_PUBLIC_URL", "https://env.example.com") t.Setenv("DIFFX_SERVER_INSECURE_COOKIES", "false") settings, _, err = parseSettings([]string{"--config-file", path}, io.Discard) - if err != nil || settings.Username != "env-admin" || settings.SignupEnabled || settings.InsecureCookies { - t.Fatalf("environment account: %+v %v", settings, err) + if err != nil || settings.PublicURL != "https://env.example.com" || settings.InsecureCookies { + t.Fatalf("environment identity: %+v %v", settings, err) } - settings, _, err = parseSettings([]string{"--config-file", path, "--username", "flag-admin", "--signup-enabled", "--insecure-cookies"}, io.Discard) - if err != nil || settings.Username != "flag-admin" || !settings.SignupEnabled || !settings.InsecureCookies { - t.Fatalf("flag account: %+v %v", settings, err) + settings, _, err = parseSettings([]string{"--config-file", path, "--public-url", "http://127.0.0.1:7980", "--insecure-cookies"}, io.Discard) + if err != nil || settings.PublicURL != "http://127.0.0.1:7980" || !settings.InsecureCookies { + t.Fatalf("flag identity: %+v %v", settings, err) + } + for _, document := range []string{`{"username":"admin"}`, `{"signupEnabled":true}`} { + retired := filepath.Join(t.TempDir(), "retired.json") + if err := os.WriteFile(retired, []byte(document), 0o600); err != nil { + t.Fatal(err) + } + if _, _, err := parseSettings([]string{"--config-file", retired}, io.Discard); err == nil { + t.Fatalf("retired setting accepted: %s", document) + } + } + for _, flag := range []string{"--username", "--signup-enabled"} { + if _, _, err := parseSettings([]string{flag, "x"}, io.Discard); err == nil { + t.Fatalf("retired flag accepted: %s", flag) + } } } @@ -146,8 +152,9 @@ func TestRemoteRejectsInvalidDeploymentSettings(t *testing.T) { {"DIFFX_SERVER_STATE", ""}, {"DIFFX_SERVER_RETENTION_DAYS", "0"}, {"DIFFX_SERVER_RETENTION_DAYS", "-1"}, - {"DIFFX_SERVER_SIGNUP_ENABLED", "bad"}, - {"DIFFX_SERVER_PASSWORD", "partial-password"}, + {"DIFFX_SERVER_INSECURE_COOKIES", "bad"}, + {"DIFFX_SERVER_PUBLIC_URL", "http://diffx.example.com"}, + {"DIFFX_SERVER_PUBLIC_URL", "https://diffx.example.com/path"}, } { t.Run(tc.key+"="+tc.value, func(t *testing.T) { t.Setenv("DIFFX_SERVER_CONFIG", "") @@ -206,7 +213,7 @@ func TestServerNeverResetsState(t *testing.T) { t.Fatal(err) } db.Close() - err = manageUser([]string{"create", "--username", "alice", "--state", path, "--password-stdin"}, strings.NewReader("correct horse battery staple"), io.Discard, io.Discard) + err = manageUser([]string{"reset-password", "--username", "alice", "--state", path, "--password-stdin"}, strings.NewReader("correct horse battery staple"), io.Discard, io.Discard) if err == nil || !strings.Contains(err.Error(), "schema 10 is older") || strings.Contains(err.Error(), "--reset-state") { t.Fatalf("older state must be refused without a local reset hint: %v", err) } diff --git a/cmd/diffx/identity_test.go b/cmd/diffx/identity_test.go index e24f1bab..009003a7 100644 --- a/cmd/diffx/identity_test.go +++ b/cmd/diffx/identity_test.go @@ -6,15 +6,13 @@ import ( "github.com/flexdinesh/diffx/internal/identityservice" "github.com/flexdinesh/diffx/internal/reviewstore" + "github.com/flexdinesh/diffx/internal/testsupport" ) func provisionCollector(t *testing.T, store *reviewstore.Store) (reviewstore.User, string) { t.Helper() identity := identityservice.New(store) - account, err := identity.Provision(t.Context(), "admin", "test-password-at-least-twelve-characters") - if err != nil { - t.Fatal(err) - } + account := testsupport.RemoteAccount(t, store, "admin") token, err := identity.CreateToken(t.Context(), account.ID, "collector", identityservice.PurposeCollector, time.Time{}) if err != nil { t.Fatal(err) diff --git a/cmd/diffx/sync_test.go b/cmd/diffx/sync_test.go index 2a93e6bb..ce1f2497 100644 --- a/cmd/diffx/sync_test.go +++ b/cmd/diffx/sync_test.go @@ -25,6 +25,7 @@ import ( "github.com/flexdinesh/diffx/internal/review" "github.com/flexdinesh/diffx/internal/reviewstore" "github.com/flexdinesh/diffx/internal/submission" + "github.com/flexdinesh/diffx/internal/testsupport" "github.com/flexdinesh/diffx/internal/webui" ) @@ -445,10 +446,7 @@ func TestSyncRotatedTokenRecoversOnlyItsOwnersAttemptedPayload(t *testing.T) { if err != nil { t.Fatal(err) } - foreign, err := identity.Provision(t.Context(), "other", "other-password-at-least-twelve-characters") - if err != nil { - t.Fatal(err) - } + foreign := testsupport.RemoteAccount(t, fixture.store, "other") foreignToken, err := identity.CreateToken(t.Context(), foreign.ID, "foreign", identityservice.PurposeCollector, time.Time{}) if err != nil { t.Fatal(err) @@ -586,10 +584,7 @@ func TestHookRotatedEnvironmentTokenRecoversOwnersRemovedCheckout(t *testing.T) if err != nil { t.Fatal(err) } - foreign, err := identity.Provision(t.Context(), "other", "other-password-at-least-twelve-characters") - if err != nil { - t.Fatal(err) - } + foreign := testsupport.RemoteAccount(t, fixture.store, "other") foreignToken, err := identity.CreateToken(t.Context(), foreign.ID, "foreign", identityservice.PurposeCollector, time.Time{}) if err != nil { t.Fatal(err) diff --git a/compose.yaml b/compose.yaml index 589b9e1a..b3888107 100644 --- a/compose.yaml +++ b/compose.yaml @@ -10,10 +10,9 @@ services: DIFFX_SERVER_PORT: "7980" DIFFX_SERVER_STATE: /data/state.db DIFFX_SERVER_RETENTION_DAYS: "${DIFFX_SERVER_RETENTION_DAYS:-7}" - # Supply both for the first account; no default credentials are generated. - DIFFX_SERVER_USERNAME: "${DIFFX_SERVER_USERNAME:-}" - DIFFX_SERVER_PASSWORD: "${DIFFX_SERVER_PASSWORD:-}" - DIFFX_SERVER_SIGNUP_ENABLED: "${DIFFX_SERVER_SIGNUP_ENABLED:-false}" + # Until the admin exists, each start logs a one-time setup link on this + # origin; no default credentials are generated. + DIFFX_SERVER_PUBLIC_URL: "${DIFFX_SERVER_PUBLIC_URL:-}" # Loopback HTTP development only. Use false behind production HTTPS. DIFFX_SERVER_INSECURE_COOKIES: "${DIFFX_SERVER_INSECURE_COOKIES:-true}" # GitHub App integration: set all or none (empty disables it), then start diff --git a/docs-website/src/pages/agent-hooks.md b/docs-website/src/pages/agent-hooks.md index d896037b..a94cc0d6 100644 --- a/docs-website/src/pages/agent-hooks.md +++ b/docs-website/src/pages/agent-hooks.md @@ -66,11 +66,10 @@ mise run docker:down Compose publishes HTTP on `127.0.0.1:7980`, outside the local 7981–7990 range, and preserves the named SQLite volume when stopped or recreated. Set `DIFFX_SERVER_PORT=7979` to publish another host port. The project is named -`diffx`, so its volume is shared by every checkout. Before first start, put -`DIFFX_SERVER_USERNAME=alice` and `DIFFX_SERVER_PASSWORD=…` in a git-ignored -`.env` beside `compose.yaml`. No default account/password is generated. These -bootstrap credentials create the first account once; restarts preserve password -changes. Signup is disabled by default. Compose explicitly enables insecure +`diffx`, so its volume is shared by every checkout. On first start, find the +setup link in `docker compose logs server` and open it within 24 hours to create +the admin account; each restart before setup logs a new link. No default +account/password is generated and there is no public signup. Compose explicitly enables insecure cookies for loopback HTTP development; set `DIFFX_SERVER_INSECURE_COOKIES=false` and deploy behind HTTPS for remote access. See [server development](https://github.com/flexdinesh/diffx/blob/main/docs/development.md#server-development) for configuration @@ -85,8 +84,8 @@ docker run --name diffx-server --read-only -p 127.0.0.1:7980:7980 \ --mount source=diffx-data,target=/data diffx-server:local ``` -The env file supplies bootstrap username/password; plain HTTP development also -requires `DIFFX_SERVER_INSECURE_COOKIES=true`. The volume contains observations, +Plain HTTP development requires `DIFFX_SERVER_INSECURE_COOKIES=true` in the env +file; behind HTTPS, set `DIFFX_SERVER_PUBLIC_URL` so the logged setup link uses it. The volume contains observations, jobs, accounts and credential hashes. The image runs without Git or a repository mount, as an unprivileged user. SIGTERM drains HTTP and stops the worker; durable unfinished jobs recover after @@ -96,11 +95,12 @@ Build/install the remote server with Go: ```sh go install github.com/flexdinesh/diffx/cmd/diffx-server@edge -DIFFX_SERVER_USERNAME=alice DIFFX_SERVER_PASSWORD=… \ - DIFFX_SERVER_INSECURE_COOKIES=true DIFFX_SERVER_STATE=./test-data/state.db diffx-server +DIFFX_SERVER_INSECURE_COOKIES=true DIFFX_SERVER_STATE=./test-data/state.db diffx-server ``` -Sign in with username/password, then open the top-right account settings to create +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. Then open the top-right account settings to create a named API token. `collector` tokens support sync and plugins; `api` tokens also support the user's review REST/MCP APIs. Both expire after 90 days by default (selectable 1–365 days), can be revoked, and show their secret only once. Browser @@ -108,18 +108,16 @@ sessions use secure cookies; API/MCP and collectors use bearer tokens. Multiple users share one SQLite database with private data per account. Local `diffx` automatically supplies its owner and has no login or account settings. -Set `DIFFX_SERVER_SIGNUP_ENABLED=true` to allow signup; invitations are not yet -supported. For operator provisioning or password recovery, stop the server, run: +The admin can disable or delete accounts from the admin page; disabling revokes +the account's sessions and tokens, and deleting removes everything it owns. If +the admin loses its password, stop the server and run: ```sh -diffx-server user create --username bob --state /data/state.db --password-stdin < /path/to/private-password diffx-server user reset-password --username alice --state /data/state.db --password-stdin < /path/to/private-password ``` Then restart the server. Password reset revokes browser sessions; API token -ownership is independent of passwords. Bootstrap requires both -`DIFFX_SERVER_USERNAME` and `DIFFX_SERVER_PASSWORD`; it never overwrites an existing -account's password. Deploy behind TLS with secure cookies. +ownership is independent of passwords. Deploy behind TLS with secure cookies. Use `--retention-days 14` or config/environment to change server retention. The server defaults to `127.0.0.1:7980` and `./data/state.db`, independent of personal CLI settings. Its image defaults to `0.0.0.0:7980` and `/data/state.db`; diff --git a/docs-website/src/pages/api.md b/docs-website/src/pages/api.md index b57feaf0..cde77800 100644 --- a/docs-website/src/pages/api.md +++ b/docs-website/src/pages/api.md @@ -35,7 +35,7 @@ Local data lives in `$XDG_STATE_HOME/diffx/state.db`, or `~/.local/state/diffx/state.db`. An isolated `DIFFX_RUNTIME_DIR` moves the default database, manual sync and hook state, and config into that directory. `--state memory` disables persistence. -Current schema 12 state persists across restarts; schema 11 state upgrades in place. When local state comes from +Current schema 13 state persists across restarts; schema 11 and 12 state upgrades in place. When local state comes from an older diffx, `diffx` asks before moving it to a backup beside the database and starting fresh; scripts pass `--reset-state`, which backs up and resets current state too. Only the newest backup is kept. Newer or unrecognized diff --git a/docs/architecture.md b/docs/architecture.md index 898f8be4..1dad6f90 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -71,7 +71,8 @@ personal lifecycle discovery. - `reviewservice`: shared snapshot reads, comments, marks and capability checks. - `reviewdata`: storage-independent identities, results and errors. - `reviewsession`: a resolved review and its source/policy capabilities. -- `identityservice`: account, password, session and API-token policies. +- `identityservice`: account, admin setup, password, session, API-token and + account administration policies. - `github`: GitHub wire adapter: app JWT, installation tokens, OAuth web flow, REST reads, webhook signatures and payloads. No storage or diffx policy. - `githubsync`: GitHub integration rules: which accounts may follow which @@ -111,8 +112,10 @@ management routes. Remote composition resolves sessions or API tokens to opaque account IDs before application access. Review operations receive an owner without mode branches. `remoteserver` activates owner-scoped services on demand, including for accounts created after startup; requests, events and workers share that owner -registry and its shutdown lifetime. Identity service owns credential rules; -transports own cookies, CSRF and route permissions. +registry and its shutdown lifetime; disabling or deleting an account stops its +services. Identity service owns credential and administration rules, including +the single admin; transports own cookies, CSRF and route permissions, such as +admin routes requiring the admin's browser session. The frontend identity owner loads the explicit server mode before mounting review state. Distributed login and settings work without any observations. Account @@ -198,13 +201,14 @@ uses `X-Diffx-State` to pin submission and polling to the destination identity; older protocols are rejected. Release artifacts use the explicit `diffx` project name, independent of the checkout directory. -Schema 12 preserves current state across starts. Schema 11 moved review state +Schema 13 preserves current state across starts. It adds administration and +Google sign-in tables; schema 12 added GitHub integration tables. Schema 11 moved review state from each snapshot scope to its stream and scope, because comments and reviewed marks must survive new collections and snapshot deletion. Schema 11 and later upgrade in place: later schemas only add tables and indexes, so opening runs the idempotent creation statements and records the new version in one transaction, -because durable server volumes must gain GitHub integration state without an -operator reset. A schema change that alters existing tables needs a new +because durable server volumes must gain GitHub integration and administration +state without an operator reset. A schema change that alters existing tables needs a new compatibility floor instead. Older-than-11, future and unrecognized schemas are refused; opening a database never resets it. `reviewstore.Reset` is the only reset: under the ownership lock it moves current or older diffx state and its SQLite sidecars to diff --git a/docs/development.md b/docs/development.md index 3c452d2e..1c312e89 100644 --- a/docs/development.md +++ b/docs/development.md @@ -142,8 +142,8 @@ runtime directories (`DIFFX_RUNTIME_DIR`) and in-memory state for lifecycle test must not register inputs in the personal service. `DIFFX_EXIT_ON_STDIN_CLOSE` is a foreground development-process lifecycle hook. -Stop running processes before upgrading. Schema 11 databases upgrade in place to -schema 12 and survive restart; incompatible schemas are refused without resetting data. Local +Stop running processes before upgrading. Schema 11 and 12 databases upgrade in place to +schema 13 and survive restart; incompatible schemas are refused without resetting data. Local `diffx` offers to back up older state and start fresh on a terminal; `--reset-state` does so without asking, keeping only the newest backup beside the database. Server state never resets: back up the state directory while @@ -151,9 +151,10 @@ stopped and explicitly choose a new database path. In-memory state does not surv submissions apply the current setting, while exact retries preserve expiry. For isolated production-user tests, start `diffx-server` with a temporary -persistent database and bootstrap username/password. Login and create API tokens -through Settings, which works before any upload. Signup activates accounts without -a restart when enabled. Operator commands require stopping the database owner. +persistent database and open the setup link it logs to create the admin. Sign in +and create API tokens through Settings, which works before any upload. New +accounts activate without a restart. Operator commands require stopping the +database owner. Credentials scope REST, MCP, jobs and events. Local mode automatically supplies its stable owner and exposes no account UI or management routes. Do not provision against the running personal service. @@ -163,14 +164,13 @@ against the running personal service. Build both binaries with `mise run build`, then run server mode directly: ```sh -DIFFX_SERVER_USERNAME=alice DIFFX_SERVER_PASSWORD=… \ - DIFFX_SERVER_INSECURE_COOKIES=true DIFFX_SERVER_STATE=./test-data/state.db ./dist/diffx-server +DIFFX_SERVER_INSECURE_COOKIES=true DIFFX_SERVER_STATE=./test-data/state.db ./dist/diffx-server ``` Or use the checked-in Compose setup: ```sh -DIFFX_SERVER_USERNAME=alice DIFFX_SERVER_PASSWORD=… mise run docker:up +mise run docker:up docker compose logs server mise run docker:down ``` @@ -185,9 +185,7 @@ building the image. | `DIFFX_SERVER_PORT` | `7980` | `7980` | `--listen HOST:PORT` | | `DIFFX_SERVER_STATE` | `data/state.db` | `/data/state.db` | `--state PATH` | | `DIFFX_SERVER_RETENTION_DAYS` | `7` | `7` | `--retention-days DAYS` | -| `DIFFX_SERVER_USERNAME` | Unset | Unset | `--username NAME` | -| `DIFFX_SERVER_PASSWORD` | Unset | Unset | None | -| `DIFFX_SERVER_SIGNUP_ENABLED` | `false` | `false` | `--signup-enabled` | +| `DIFFX_SERVER_PUBLIC_URL` | Unset | Unset | `--public-url URL` | | `DIFFX_SERVER_INSECURE_COOKIES` | `false` | Compose: `true` | `--insecure-cookies` | The server listens on 7980 so a local `diffx [PATH]` (7981–7990) and the server @@ -195,31 +193,35 @@ run side by side. The `DIFFX_SERVER_` prefix keeps deployment variables from configuring the CLI, and the CLI's `DIFFX_LOCAL_*`/`DIFFX_REMOTE_*` from configuring the server. -Bootstrap requires both username and password. It creates the first account once -and never replaces passwords on restart. No default credentials or admin-token -file is generated. Signup remains disabled unless explicitly enabled. `memory` -and `:memory:` disable persistence. +While no admin exists, each start logs a one-time setup link, +`/setup#t=…`, valid for 24 hours; a restart replaces it. The link +creates the single admin with a username and password. No default credentials +are generated and there is no public signup. `DIFFX_SERVER_PUBLIC_URL` is the +browser-facing https origin used in that link (http only with insecure cookies); +unset, the link uses the listen address. `memory` and `:memory:` disable +persistence. -Sign in with username/password and create a named token in Settings. Use the +The admin signs in at `/admin/login`; every other account signs in with Google +and comes from an admin invite. Create a named token in Settings. Use the `collector` preset for producers, or `api` for review APIs/MCP. Token secrets are shown once; expiry defaults to 90 days (selectable 1–365). Tokens cannot manage accounts. Browser sessions are seven-day opaque cookies with CSRF protection on unsafe requests. Secure cookies are the binary/image default; explicitly enable insecure cookies only for HTTP development. Production requires HTTPS. -With the server stopped, operator commands read passwords from stdin: +If the admin loses its password, stop the server and reset it; the command reads +the password from stdin and refuses non-admin accounts: ```sh -diffx-server user create --username bob --state ./test-data/state.db --password-stdin < /path/to/private-password diffx-server user reset-password --username alice --state ./test-data/state.db --password-stdin < /path/to/private-password ``` -Password reset revokes browser sessions. Restart the server after provisioning. +Password reset revokes browser sessions. Restart the server afterwards. Precedence is defaults → explicit JSON file → env vars → explicit flags. Use `--config-file FILE` or `DIFFX_SERVER_CONFIG` to select a server config. The file -supports `host`, `port`, `state`, `username`, `retentionDays`, `signupEnabled` and -`insecureCookies`. Bootstrap passwords are supplied through env vars. The file +supports `host`, `port`, `state`, `publicUrl`, `retentionDays` and +`insecureCookies`. The file must exist and can be mounted read-only; startup never creates it or a config lock. Personal CLI config, `DIFFX_RUNTIME_DIR`, producer URL settings and external web assets @@ -235,7 +237,7 @@ starting with a fresh database; a volume is not a backup. Set `DIFFX_SERVER_PORT=7979 mise run docker:up` for a different published host port; the container always listens on 7980. Compose also accepts -bootstrap username/password, retention, signup and insecure-cookie overrides, +public URL, retention and insecure-cookie overrides, from the shell or a git-ignored `.env` beside `compose.yaml`. Compose defaults to insecure cookies because its HTTP listener is published on loopback; explicitly set `DIFFX_SERVER_INSECURE_COOKIES=false` behind production HTTPS. The fixed diff --git a/docs/release.md b/docs/release.md index da7bc6c3..495a48bb 100644 --- a/docs/release.md +++ b/docs/release.md @@ -41,8 +41,8 @@ arm64. Windows archives are also published. Archives include the native binary, README, and license. The embedded web application needs no installed Node.js runtime. -Stop running binaries before upgrading. Schema 11 state upgrades in place to -schema 12 and survives subsequent starts; older, future or unrecognized schemas are refused without +Stop running binaries before upgrading. Schema 11 and 12 state upgrades in place to +schema 13 and survives subsequent starts; older, future or unrecognized schemas are refused without resetting data. Local `diffx` offers to back up older state beside it and start fresh, or does so with `--reset-state`. For servers, back up the state directory and explicitly choose a fresh database path if discarding incompatible state. Upgrade server and collectors diff --git a/docs/system.md b/docs/system.md index 2da108e1..acd1d161 100644 --- a/docs/system.md +++ b/docs/system.md @@ -353,15 +353,28 @@ records; producer paths do not identify server files. Accounts have stable opaque IDs independent of username, passwords, browser sessions and API tokens. Username matching is case-insensitive and usernames are fixed in this version. Local mode automatically resolves one stable OS-backed -owner; users never manage local identity. Remote mode uses username/password -login. Signup is disabled unless explicitly enabled; operator provisioning -remains available. Bootstrap username/password creates the first account once, -never replaces changed passwords, and never generates default credentials. +owner; users never manage local identity. A remote server has exactly one admin, +the only account with a password; there is no public signup and no default +credential. While no admin exists, each start stores a hashed one-time setup +secret for 24 hours, replacing the previous one, and logs it as +`/setup#t=…`. The fragment keeps the secret out of request logs. +Setup consumes the secret and creates the admin in one transaction, so +concurrent attempts yield one admin. Only the admin signs in with a password, at +`/admin/login`; every other account signs in with Google, and a refused password +login is indistinguishable from a wrong password. Offline `user reset-password` +recovers only the admin. + +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 +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 +ingestion worker refuses jobs for unavailable accounts. Passwords use salted Argon2id hashes. Browser login creates a revocable opaque seven-day server-side session in an HttpOnly, Secure, SameSite=Lax cookie. Unsafe browser requests require a session CSRF token, including account management. -Password changes revoke existing sessions. Login/signup admission is bounded and +Password changes revoke existing sessions. Login and setup admission is bounded and login failures do not disclose whether a username exists. Plain HTTP development must explicitly opt into insecure cookies; production uses HTTPS. diff --git a/internal/config/server.go b/internal/config/server.go index 3feb801e..c1b1a968 100644 --- a/internal/config/server.go +++ b/internal/config/server.go @@ -26,8 +26,7 @@ type ServerValues struct { Host string `json:"host"` Port int `json:"port"` State string `json:"state"` - Username string `json:"username"` - SignupEnabled bool `json:"signupEnabled"` + PublicURL string `json:"publicUrl"` InsecureCookies bool `json:"insecureCookies"` RetentionDays int `json:"retentionDays"` // InternalPort listens beside Port, on the same host, when GitHub is enabled. @@ -81,9 +80,7 @@ func (github GitHub) Validate(insecureCookies bool) error { if len(missing) > 0 { return fmt.Errorf("incomplete GitHub integration; also set %s", strings.Join(missing, ", ")) } - public, err := url.Parse(github.PublicURL) - if err != nil || public.Host == "" || public.User != nil || public.RawQuery != "" || public.Fragment != "" || strings.TrimSuffix(public.Path, "/") != "" || - public.Scheme != "https" && (public.Scheme != "http" || !insecureCookies) { + if !validOrigin(github.PublicURL, insecureCookies) { return errors.New("GitHub publicUrl must be an https origin such as https://diffx.example.com (http only with insecure cookies)") } key, err := base64.StdEncoding.DecodeString(github.TokenKey) @@ -96,6 +93,21 @@ func (github GitHub) Validate(insecureCookies bool) error { return nil } +// validOrigin accepts a browser-facing origin; plain HTTP is development only. +func validOrigin(raw string, insecureCookies bool) bool { + public, err := url.Parse(raw) + return err == nil && public.Host != "" && public.User == nil && public.RawQuery == "" && public.Fragment == "" && strings.TrimSuffix(public.Path, "/") == "" && + (public.Scheme == "https" || public.Scheme == "http" && insecureCookies) +} + +// ValidatePublicURL checks the optional origin used in links the server issues. +func (values ServerValues) ValidatePublicURL() error { + if values.PublicURL != "" && !validOrigin(values.PublicURL, values.InsecureCookies) { + return errors.New("publicUrl must be an https origin such as https://diffx.example.com (http only with insecure cookies)") + } + return nil +} + // LoadServerFile resolves defaults, an explicitly selected file, then environment. // It never discovers personal config, creates files, or acquires a config lock. // Variables use the DIFFX_SERVER_ prefix so they never configure the local CLI. @@ -128,7 +140,7 @@ func LoadServerFile(path string) (ServerValues, error) { }{ {"DIFFX_SERVER_HOST", &values.Host}, {"DIFFX_SERVER_STATE", &values.State}, - {"DIFFX_SERVER_USERNAME", &values.Username}, + {"DIFFX_SERVER_PUBLIC_URL", &values.PublicURL}, {"DIFFX_SERVER_GITHUB_CLIENT_ID", &values.GitHub.ClientID}, {"DIFFX_SERVER_GITHUB_PRIVATE_KEY_FILE", &values.GitHub.PrivateKeyFile}, {"DIFFX_SERVER_GITHUB_PUBLIC_URL", &values.GitHub.PublicURL}, @@ -169,7 +181,6 @@ func LoadServerFile(path string) (ServerValues, error) { name string target *bool }{ - {"DIFFX_SERVER_SIGNUP_ENABLED", &values.SignupEnabled}, {"DIFFX_SERVER_INSECURE_COOKIES", &values.InsecureCookies}, } { if raw, exists := os.LookupEnv(field.name); exists { diff --git a/internal/identityservice/identity.go b/internal/identityservice/identity.go index 6e95307a..177a3090 100644 --- a/internal/identityservice/identity.go +++ b/internal/identityservice/identity.go @@ -18,6 +18,8 @@ var ( ErrInvalidInput = errors.New("invalid identity input") ErrConflict = errors.New("username already exists") ErrNotFound = errors.New("identity not found") + // ErrForbidden refuses an administrative action on the admin account itself. + ErrForbidden = errors.New("identity operation not permitted") ) type Purpose string @@ -28,11 +30,23 @@ const ( SessionLifetime = 7 * 24 * time.Hour DefaultTokenLifetime = 90 * 24 * time.Hour MaximumTokenLifetime = 365 * 24 * time.Hour + SetupLifetime = 24 * time.Hour ) +// Account is an authenticated owner. Admin marks the single account that +// signs in with a password and manages other accounts. type Account struct { ID string Username string + Admin bool +} + +// AccountSummary is the administrative view of an account. +type AccountSummary struct { + Account + Email string + Disabled bool + CreatedAt time.Time } type PasswordCredential struct { @@ -68,9 +82,21 @@ type TokenIdentity struct { } // Store operations preserve credential races atomically. Password replacement -// revokes sessions in its transaction; session creation compares the verified hash. +// revokes sessions in its transaction; session creation compares the verified hash +// and refuses disabled accounts. type Store interface { - CreateAccount(context.Context, Account, string, bool, time.Time) (Account, error) + // PutAdminSetup replaces the pending setup secret while no admin exists and + // reports whether it was stored. + PutAdminSetup(context.Context, string, time.Time) (bool, error) + AdminExists(context.Context) (bool, error) + // CreateAdmin consumes the unexpired setup secret and creates the admin in + // one transaction; a missing, expired or used secret is ErrNotFound. + CreateAdmin(ctx context.Context, account Account, passwordHash, setupHash string, now time.Time) error + AccountSummaries(context.Context) ([]AccountSummary, error) + // SetAccountDisabled revokes sessions and tokens when disabling. Both it and + // DeleteAccount refuse the admin with ErrForbidden. + SetAccountDisabled(ctx context.Context, accountID string, disabled bool, at time.Time) error + DeleteAccount(context.Context, string) error AccountPassword(context.Context, string) (PasswordCredential, error) AccountPasswordByID(context.Context, string) (PasswordCredential, error) ReplacePassword(context.Context, string, string, string) error @@ -130,33 +156,55 @@ func credentialHash(secret string) string { return hex.EncodeToString(hash[:]) } -func (service *Service) Provision(ctx context.Context, username, password string) (Account, error) { - return service.provision(ctx, username, password, false) +// PrepareSetup issues a one-time admin setup secret while no admin exists, +// replacing any earlier one. It returns "" once an admin exists. The secret is +// stored hashed, so each start issues a new one. +func (service *Service) PrepareSetup(ctx context.Context) (string, error) { + secret, err := opaqueSecret() + if err != nil { + return "", err + } + stored, err := service.store.PutAdminSetup(ctx, credentialHash(secret), service.now().Add(SetupLifetime)) + if err != nil || !stored { + return "", err + } + return secret, nil } -func (service *Service) Bootstrap(ctx context.Context, username, password string) (Account, error) { - return service.provision(ctx, username, password, true) +func (service *Service) SetupRequired(ctx context.Context) (bool, error) { + exists, err := service.store.AdminExists(ctx) + return !exists, err } -func (service *Service) provision(ctx context.Context, username, password string, bootstrap bool) (Account, error) { +// CompleteSetup creates the admin from a valid setup secret and signs it in. +func (service *Service) CompleteSetup(ctx context.Context, secret, username, password string) (Session, error) { username, err := canonicalUsername(username) if err != nil { - return Account{}, err + return Session{}, err } if err := validatePassword(password); err != nil { - return Account{}, err + return Session{}, err + } + if len(secret) != 43 { + return Session{}, ErrNotFound } hash, err := hashPassword(password) if err != nil { - return Account{}, err + return Session{}, err } id, err := opaqueSecret() if err != nil { - return Account{}, err + return Session{}, err } - return service.store.CreateAccount(ctx, Account{ID: id, Username: username}, hash, bootstrap, service.now()) + account := Account{ID: id, Username: username, Admin: true} + if err := service.store.CreateAdmin(ctx, account, hash, credentialHash(secret), service.now()); err != nil { + return Session{}, err + } + return service.issueSession(ctx, account, hash) } +// Login accepts passwords only for the admin; every other failure is the same +// ErrAuthentication, so it never reveals which accounts exist or use Google. func (service *Service) Login(ctx context.Context, username, password string) (Session, error) { username, invalid := canonicalUsername(username) if len(password) > 128 { @@ -170,9 +218,15 @@ func (service *Service) Login(ctx context.Context, username, password string) (S if err != nil { return Session{}, err } - if !verifyPassword(credential.Hash, password) { + if !verifyPassword(credential.Hash, password) || !credential.Account.Admin { return Session{}, ErrAuthentication } + return service.issueSession(ctx, credential.Account, credential.Hash) +} + +// issueSession fences on the credential the caller verified: a concurrent +// password change or disable makes issuance fail. +func (service *Service) issueSession(ctx context.Context, account Account, expected string) (Session, error) { secret, err := opaqueSecret() if err != nil { return Session{}, err @@ -182,10 +236,10 @@ func (service *Service) Login(ctx context.Context, username, password string) (S return Session{}, err } expires := service.now().Add(SessionLifetime) - if err := service.store.CreateSession(ctx, credential.Account.ID, credential.Hash, credentialHash(secret), csrf, expires); err != nil { + if err := service.store.CreateSession(ctx, account.ID, expected, credentialHash(secret), csrf, expires); err != nil { return Session{}, err } - return Session{Account: credential.Account, Secret: secret, CSRFToken: csrf, ExpiresAt: expires}, nil + return Session{Account: account, Secret: secret, CSRFToken: csrf, ExpiresAt: expires}, nil } func (service *Service) AuthenticateSession(ctx context.Context, secret string) (Session, error) { @@ -210,7 +264,7 @@ func (service *Service) ChangePassword(ctx context.Context, accountID, current, if err != nil { return err } - if len(current) > 128 || !verifyPassword(credential.Hash, current) { + if len(current) > 128 || !verifyPassword(credential.Hash, current) || !credential.Account.Admin { return ErrAuthentication } hash, err := hashPassword(password) @@ -220,6 +274,7 @@ func (service *Service) ChangePassword(ctx context.Context, accountID, current, return service.store.ReplacePassword(ctx, accountID, credential.Hash, hash) } +// ResetPassword recovers the admin offline; other accounts have no password. func (service *Service) ResetPassword(ctx context.Context, username, password string) error { username, err := canonicalUsername(username) if err != nil { @@ -232,6 +287,9 @@ func (service *Service) ResetPassword(ctx context.Context, username, password st if err != nil { return err } + if !credential.Account.Admin { + return ErrNotFound + } hash, err := hashPassword(password) if err != nil { return err @@ -284,6 +342,25 @@ func (service *Service) AuthenticateToken(ctx context.Context, secret string) (T return service.store.ResolveIdentityToken(ctx, credentialHash(secret), service.now()) } +// Accounts lists enabled accounts, the owners the ingestion worker may serve. func (service *Service) Accounts(ctx context.Context) ([]Account, error) { return service.store.IdentityAccounts(ctx) } + +func (service *Service) AccountSummaries(ctx context.Context) ([]AccountSummary, error) { + return service.store.AccountSummaries(ctx) +} + +func (service *Service) DisableAccount(ctx context.Context, accountID string) error { + return service.store.SetAccountDisabled(ctx, accountID, true, service.now()) +} + +func (service *Service) EnableAccount(ctx context.Context, accountID string) error { + return service.store.SetAccountDisabled(ctx, accountID, false, service.now()) +} + +// DeleteAccount removes the account and every review, token and integration +// it owns. +func (service *Service) DeleteAccount(ctx context.Context, accountID string) error { + return service.store.DeleteAccount(ctx, accountID) +} diff --git a/internal/remoteserver/admin.go b/internal/remoteserver/admin.go new file mode 100644 index 00000000..b2d421c3 --- /dev/null +++ b/internal/remoteserver/admin.go @@ -0,0 +1,80 @@ +package remoteserver + +import ( + "net/http" + "strings" + + "github.com/flexdinesh/diffx/internal/identityservice" +) + +type adminUserJSON struct { + ID string `json:"id"` + Username string `json:"username"` + Email *string `json:"email"` + Admin bool `json:"admin"` + Disabled bool `json:"disabled"` + CreatedAt int64 `json:"createdAt"` +} + +func projectAdminUser(account identityservice.AccountSummary) adminUserJSON { + result := adminUserJSON{ID: account.ID, Username: account.Username, Admin: account.Admin, Disabled: account.Disabled, CreatedAt: account.CreatedAt.UnixMilli()} + if account.Email != "" { + result.Email = &account.Email + } + return result +} + +// admin serves account administration to the admin's browser session. Disabling +// or deleting an account stops its active services; its credentials are already gone. +func (remote *transport) admin(w http.ResponseWriter, r *http.Request, p principal) { + if r.URL.Path == "/api/v2/admin/users" { + if r.Method != http.MethodGet { + problem(w, 405, "Method not allowed") + return + } + accounts, err := remote.identity.AccountSummaries(r.Context()) + if err != nil { + identityError(w, err) + return + } + users := make([]adminUserJSON, 0, len(accounts)) + for _, account := range accounts { + users = append(users, projectAdminUser(account)) + } + writeJSON(w, 200, struct { + Users []adminUserJSON `json:"users"` + }{users}) + return + } + rest, ok := strings.CutPrefix(r.URL.Path, "/api/v2/admin/users/") + if !ok || rest == "" { + http.NotFound(w, r) + return + } + id, action, _ := strings.Cut(rest, "/") + var err error + switch { + case action == "" && r.Method == http.MethodDelete: + err = remote.identity.DeleteAccount(r.Context(), id) + case (action == "disable" || action == "enable") && r.Method == http.MethodPost: + if action == "disable" { + err = remote.identity.DisableAccount(r.Context(), id) + } else { + err = remote.identity.EnableAccount(r.Context(), id) + } + case action == "" || action == "disable" || action == "enable": + problem(w, 405, "Method not allowed") + return + default: + http.NotFound(w, r) + return + } + if err == nil { + err = remote.applications.remove(id) + } + if err != nil { + identityError(w, err) + return + } + w.WriteHeader(204) +} diff --git a/internal/remoteserver/admin_test.go b/internal/remoteserver/admin_test.go new file mode 100644 index 00000000..b4afa87c --- /dev/null +++ b/internal/remoteserver/admin_test.go @@ -0,0 +1,109 @@ +package remoteserver + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/flexdinesh/diffx/internal/identityservice" + "github.com/flexdinesh/diffx/internal/testsupport" +) + +func TestAdminRoutesRequireAdminBrowserSession(t *testing.T) { + store, handler := identityHandler(t, Settings{}) + admin := testsupport.RemoteAdmin(t, store, "alice", testPassword) + cookie, csrf := loginIdentity(t, handler, "alice") + memberCookie, memberCSRF := memberSession(t, store, "member") + adminToken, err := identityservice.New(store).CreateToken(t.Context(), admin.Account.ID, "api", identityservice.PurposeAPI, time.Time{}) + if err != nil { + t.Fatal(err) + } + for _, request := range []struct { + cookie *http.Cookie + csrf, bearer string + }{{nil, "", ""}, {memberCookie, memberCSRF, ""}, {nil, "", "Bearer " + adminToken.Secret}} { + w := identityRequest(handler, "GET", "/api/v2/admin/users", "", request.cookie, request.csrf, request.bearer) + if w.Code != 401 && w.Code != 403 { + t.Fatalf("non-admin listed users: %d", w.Code) + } + } + w := identityRequest(handler, "GET", "/api/v2/admin/users", "", cookie, "", "") + var listed struct { + Users []adminUserJSON `json:"users"` + } + if err := json.Unmarshal(w.Body.Bytes(), &listed); err != nil || w.Code != 200 || len(listed.Users) != 2 { + t.Fatalf("list users: %d %s %v", w.Code, w.Body.String(), err) + } + var memberID string + for _, user := range listed.Users { + if user.Username == "member" { + memberID = user.ID + } + if user.Email != nil || user.CreatedAt <= 0 || user.Admin != (user.ID == admin.Account.ID) { + t.Fatalf("user projection: %+v", user) + } + } + if w := identityRequest(handler, "POST", "/api/v2/admin/users/"+memberID+"/disable", "", cookie, "", ""); w.Code != 403 { + t.Fatalf("disable without csrf: %d", w.Code) + } + for _, path := range []string{"/api/v2/admin/users/" + admin.Account.ID + "/disable", "/api/v2/admin/users/" + admin.Account.ID} { + method := "POST" + if path == "/api/v2/admin/users/"+admin.Account.ID { + method = "DELETE" + } + if w := identityRequest(handler, method, path, "", cookie, csrf, ""); w.Code != 403 { + t.Fatalf("admin changed itself %s: %d", path, w.Code) + } + } + for _, test := range []struct { + method, path string + want int + }{{"POST", "/api/v2/admin/users/missing/disable", 404}, {"GET", "/api/v2/admin/users/" + memberID + "/disable", 405}, {"POST", "/api/v2/admin/users/" + memberID + "/promote", 404}, {"POST", "/api/v2/admin/invites", 404}} { + if w := identityRequest(handler, test.method, test.path, "", cookie, csrf, ""); w.Code != test.want { + t.Fatalf("%s %s: %d want %d", test.method, test.path, w.Code, test.want) + } + } + + server := httptest.NewServer(handler) + defer server.Close() + r, err := http.NewRequestWithContext(t.Context(), "GET", server.URL+"/api/v2/events", nil) + if err != nil { + t.Fatal(err) + } + r.AddCookie(memberCookie) + stream, err := server.Client().Do(r) + if err != nil { + t.Fatal(err) + } + defer stream.Body.Close() + if stream.StatusCode != 200 { + t.Fatalf("member stream: %d", stream.StatusCode) + } + if w := identityRequest(handler, "POST", "/api/v2/admin/users/"+memberID+"/disable", "", cookie, csrf, ""); w.Code != 204 { + t.Fatalf("disable: %d %s", w.Code, w.Body.String()) + } + closed := make(chan error, 1) + go func() { _, err := io.Copy(&bytes.Buffer{}, stream.Body); closed <- err }() + select { + case <-closed: + case <-time.After(2 * time.Second): + t.Fatal("disabled member kept stream open") + } + if w := identityRequest(handler, "GET", "/api/v2/contexts", "", memberCookie, "", ""); w.Code != 401 { + t.Fatalf("disabled member session: %d", w.Code) + } + if w := identityRequest(handler, "POST", "/api/v2/admin/users/"+memberID+"/enable", "", cookie, csrf, ""); w.Code != 204 { + t.Fatalf("enable: %d", w.Code) + } + if w := identityRequest(handler, "DELETE", "/api/v2/admin/users/"+memberID, "", cookie, csrf, ""); w.Code != 204 { + t.Fatalf("delete: %d", w.Code) + } + w = identityRequest(handler, "GET", "/api/v2/admin/users", "", cookie, "", "") + if err := json.Unmarshal(w.Body.Bytes(), &listed); err != nil || len(listed.Users) != 1 { + t.Fatalf("deleted member listed: %s", w.Body.String()) + } +} diff --git a/internal/remoteserver/boundaries_users_test.go b/internal/remoteserver/boundaries_users_test.go index 34cb4326..35791b39 100644 --- a/internal/remoteserver/boundaries_users_test.go +++ b/internal/remoteserver/boundaries_users_test.go @@ -1,12 +1,16 @@ package remoteserver import ( + "bytes" "context" "encoding/json" + "log" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" + "sync" "testing" "testing/fstest" "time" @@ -19,14 +23,18 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) -func TestRunPasswordAccountPersistsAcrossRestart(t *testing.T) { +func TestRunSetupAdminPersistsAcrossRestart(t *testing.T) { state := filepath.Join(t.TempDir(), "state.db") + var logs syncBuffer + log.SetOutput(&logs) + t.Cleanup(func() { log.SetOutput(os.Stderr) }) var accountID string + client := &http.Client{Timeout: 3 * time.Second} for attempt := 0; attempt < 2; attempt++ { ctx, cancel := context.WithCancel(t.Context()) ready, done := make(chan string, 1), make(chan error, 1) go func() { - done <- Run(ctx, Settings{State: state, Listen: "127.0.0.1:0", Username: "admin", Password: testPassword, InsecureCookies: true}, func(address string) { ready <- address }) + done <- Run(ctx, Settings{State: state, Listen: "127.0.0.1:0", PublicURL: "https://diffx.example.com", InsecureCookies: true}, func(address string) { ready <- address }) }() var address string select { @@ -38,7 +46,24 @@ func TestRunPasswordAccountPersistsAcrossRestart(t *testing.T) { cancel() t.Fatal("startup timeout") } - response, err := (&http.Client{Timeout: 3 * time.Second}).Post("http://"+address+"/api/v2/auth/login", "application/json", strings.NewReader(`{"username":"admin","password":"`+testPassword+`"}`)) + path, body := "/api/v2/auth/login", `{"username":"admin","password":"`+testPassword+`"}` + if attempt == 0 { + // The link is logged just after ready reports the address. + var link string + found := false + for deadline := time.Now().Add(3 * time.Second); !found && time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) { + _, link, found = strings.Cut(logs.String(), "open https://diffx.example.com/setup#t=") + } + if !found { + cancel() + t.Fatalf("setup link not logged: %q", logs.String()) + } + path, body = "/api/v2/setup", `{"token":"`+strings.TrimSpace(link)+`","username":"admin","password":"`+testPassword+`"}` + } else if strings.Count(logs.String(), "setup#t=") != 1 { + cancel() + t.Fatal("setup link logged after admin exists") + } + response, err := client.Post("http://"+address+path, "application/json", strings.NewReader(body)) if err != nil { cancel() t.Fatal(err) @@ -46,9 +71,9 @@ func TestRunPasswordAccountPersistsAcrossRestart(t *testing.T) { var result bootstrapJSON err = json.NewDecoder(response.Body).Decode(&result) response.Body.Close() - if err != nil || response.StatusCode != 200 || result.User == nil { + if err != nil || response.StatusCode != 200 || result.User == nil || !result.User.Admin { cancel() - t.Fatalf("login: %v status%d", err, response.StatusCode) + t.Fatalf("%s: %v status%d", path, err, response.StatusCode) } if attempt == 0 { accountID = result.User.ID @@ -68,6 +93,24 @@ func TestRunPasswordAccountPersistsAcrossRestart(t *testing.T) { } } +// syncBuffer collects log output written by the server goroutine. +type syncBuffer struct { + mu sync.Mutex + buffer bytes.Buffer +} + +func (buffer *syncBuffer) Write(data []byte) (int, error) { + buffer.mu.Lock() + defer buffer.mu.Unlock() + return buffer.buffer.Write(data) +} + +func (buffer *syncBuffer) String() string { + buffer.mu.Lock() + defer buffer.mu.Unlock() + return buffer.buffer.String() +} + type credentialTransport struct { base http.RoundTripper token string diff --git a/internal/remoteserver/github_test.go b/internal/remoteserver/github_test.go index 578cafad..0bf41055 100644 --- a/internal/remoteserver/github_test.go +++ b/internal/remoteserver/github_test.go @@ -77,11 +77,8 @@ func webhook(handler http.Handler, delivery, signature, body string) int { } func TestGitHubRoutesAbsentWithoutConfiguration(t *testing.T) { - store, handler := identityHandler(t, Settings{SignupEnabled: true}) - if _, err := identityservice.New(store).Provision(t.Context(), "user", testPassword); err != nil { - t.Fatal(err) - } - cookie, _ := loginIdentity(t, handler, "user") + store, handler := identityHandler(t, Settings{}) + cookie, _ := memberSession(t, store, "user") if w := identityRequest(handler, "GET", "/api/v2/github/connection", "", cookie, "", ""); w.Code != 404 { t.Fatalf("unconfigured connection: %d", w.Code) } @@ -92,13 +89,9 @@ func TestGitHubRoutesAbsentWithoutConfiguration(t *testing.T) { func TestGitHubConnectFollowAndWebhookBoundaries(t *testing.T) { api := fakeGitHubAPI(t) - store, handler := identityHandler(t, Settings{SignupEnabled: true, GitHub: githubSettings(t, api.URL)}) - identity := identityservice.New(store) - if _, err := identity.Provision(t.Context(), "user", testPassword); err != nil { - t.Fatal(err) - } + store, handler := identityHandler(t, Settings{GitHub: githubSettings(t, api.URL)}) token := provisionToken(t, store, "automation", identityservice.PurposeAPI) - cookie, csrf := loginIdentity(t, handler, "user") + cookie, csrf := memberSession(t, store, "user") body := `{"action":"opened","number":1,"installation":{"id":5},"repository":{"id":10,"full_name":"octo/app"}}` if code := webhook(handler, "d1", "sha256=00", body); code != 401 { @@ -159,15 +152,9 @@ func TestGitHubConnectFollowAndWebhookBoundaries(t *testing.T) { func TestGitHubCallbackRefusesForeignAndMissingSessions(t *testing.T) { api := fakeGitHubAPI(t) - store, handler := identityHandler(t, Settings{SignupEnabled: true, GitHub: githubSettings(t, api.URL)}) - identity := identityservice.New(store) - for _, name := range []string{"alice", "bob"} { - if _, err := identity.Provision(t.Context(), name, testPassword); err != nil { - t.Fatal(err) - } - } - alice, csrf := loginIdentity(t, handler, "alice") - bob, _ := loginIdentity(t, handler, "bob") + store, handler := identityHandler(t, Settings{GitHub: githubSettings(t, api.URL)}) + alice, csrf := memberSession(t, store, "alice") + bob, _ := memberSession(t, store, "bob") w := identityRequest(handler, "POST", "/api/v2/github/connection", "", alice, csrf, "") var started struct { AuthorizeURL string `json:"authorizeUrl"` @@ -208,7 +195,7 @@ func TestFetchProtocolOnlyOnInternalListenerWithSecret(t *testing.T) { t.Cleanup(func() { store.Close() }) settings := githubSettings(t, fakeGitHubAPI(t).URL) settings.FetcherSecret = strings.Repeat("s", 32) - public, internal, closeServices, _, err := compose(t.Context(), store, fstest.MapFS{"index.html": {Data: []byte("app shell")}}, Settings{SignupEnabled: true, GitHub: settings}) + public, internal, closeServices, _, err := compose(t.Context(), store, fstest.MapFS{"index.html": {Data: []byte("app shell")}}, Settings{GitHub: settings}) if err != nil { t.Fatal(err) } diff --git a/internal/remoteserver/identity.go b/internal/remoteserver/identity.go index b7a4e383..738a0e70 100644 --- a/internal/remoteserver/identity.go +++ b/internal/remoteserver/identity.go @@ -41,18 +41,19 @@ type principal struct { type accountJSON struct { ID string `json:"id"` Username string `json:"username"` + Admin bool `json:"admin"` } type bootstrapJSON struct { Mode string `json:"mode"` - SignupEnabled bool `json:"signupEnabled"` + GoogleEnabled bool `json:"googleEnabled"` User *accountJSON `json:"user"` CSRFToken *string `json:"csrfToken"` } func (remote *transport) bootstrap(w http.ResponseWriter, p principal) { - result := bootstrapJSON{Mode: "remote", SignupEnabled: remote.settings.SignupEnabled} + result := bootstrapJSON{Mode: "remote"} if p.session != nil { - result.User = &accountJSON{p.account.ID, p.account.Username} + result.User = &accountJSON{p.account.ID, p.account.Username, p.account.Admin} result.CSRFToken = &p.session.CSRFToken } writeJSON(w, 200, result) @@ -89,7 +90,7 @@ func (remote *transport) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Login remains available with an expired browser cookie. Explicit bearer // credentials are always validated; never silently fall back to cookies. - authForm := r.URL.Path == "/api/v2/auth/login" || r.URL.Path == "/api/v2/auth/signup" + authForm := r.URL.Path == "/api/v2/auth/login" || r.URL.Path == "/api/v2/setup" public := r.URL.Path == "/api/v2/bootstrap" || (!strings.HasPrefix(r.URL.Path, "/api/") && !strings.HasPrefix(r.URL.Path, "/mcp") && !strings.HasPrefix(r.URL.Path, "/control/")) p, err := remote.authenticate(r) if errors.Is(err, identityservice.ErrAuthentication) && r.Header.Get("Authorization") == "" && (public || authForm) { @@ -133,6 +134,14 @@ func (remote *transport) ServeHTTP(w http.ResponseWriter, r *http.Request) { remote.githubAccount(w, r, p) return } + if strings.HasPrefix(r.URL.Path, "/api/v2/admin/") { + if p.session == nil || !p.account.Admin { + problem(w, 403, "Admin browser session required") + return + } + remote.admin(w, r, p) + return + } if strings.HasPrefix(r.URL.Path, "/api/v2/account/") || r.URL.Path == "/api/v2/auth/logout" { if p.session == nil { problem(w, 403, "Browser session required") @@ -246,13 +255,23 @@ func (remote *transport) static(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(raw) } } + +// auth serves the credential forms: admin password login and one-time admin +// setup. Both are bounded like login because each verifies a secret. func (remote *transport) auth(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost { - problem(w, 405, "Method not allowed") + if r.URL.Path == "/api/v2/setup" && r.Method == http.MethodGet { + required, err := remote.identity.SetupRequired(r.Context()) + if err != nil { + identityError(w, err) + return + } + writeJSON(w, 200, struct { + Required bool `json:"required"` + }{required}) return } - if r.URL.Path == "/api/v2/auth/signup" && !remote.settings.SignupEnabled { - problem(w, 403, "Signup is disabled") + if r.Method != http.MethodPost { + problem(w, 405, "Method not allowed") return } select { @@ -268,23 +287,32 @@ func (remote *transport) auth(w http.ResponseWriter, r *http.Request) { problem(w, 429, "Too many authentication attempts") return } - var input struct { - Username string `json:"username"` - Password string `json:"password"` - } - if !decodeJSON(w, r, &input) { - return - } var session identityservice.Session var err error - if r.URL.Path == "/api/v2/auth/signup" { - _, err = remote.identity.Provision(r.Context(), input.Username, input.Password) - if err != nil { - identityError(w, err) + if r.URL.Path == "/api/v2/setup" { + var input struct { + Token string `json:"token"` + Username string `json:"username"` + Password string `json:"password"` + } + if !decodeJSON(w, r, &input) { + return + } + session, err = remote.identity.CompleteSetup(r.Context(), input.Token, input.Username, input.Password) + if errors.Is(err, identityservice.ErrNotFound) { + problem(w, 404, "Setup link is invalid, expired or already used") + return + } + } else { + var input struct { + Username string `json:"username"` + Password string `json:"password"` + } + if !decodeJSON(w, r, &input) { return } + session, err = remote.identity.Login(r.Context(), input.Username, input.Password) } - session, err = remote.identity.Login(r.Context(), input.Username, input.Password) if err != nil { identityError(w, err) return @@ -450,6 +478,8 @@ func identityError(w http.ResponseWriter, err error) { problem(w, 409, "Username unavailable") case errors.Is(err, identityservice.ErrNotFound): problem(w, 404, "Not found") + case errors.Is(err, identityservice.ErrForbidden): + problem(w, 403, "The admin account cannot be changed here") default: problem(w, 500, "Server could not process request") } diff --git a/internal/remoteserver/identity_test.go b/internal/remoteserver/identity_test.go index 907eef20..f2d7bf26 100644 --- a/internal/remoteserver/identity_test.go +++ b/internal/remoteserver/identity_test.go @@ -15,6 +15,7 @@ import ( "github.com/flexdinesh/diffx/internal/identityservice" "github.com/flexdinesh/diffx/internal/ingestion" "github.com/flexdinesh/diffx/internal/reviewstore" + "github.com/flexdinesh/diffx/internal/testsupport" ) func identityHandler(t *testing.T, settings Settings) (*reviewstore.Store, http.Handler) { @@ -63,8 +64,16 @@ func loginIdentity(t *testing.T, handler http.Handler, name string) (*http.Cooki } return cookies[0], *result.CSRFToken } + +// memberSession signs in a passwordless member, as Google sign-in does. +func memberSession(t *testing.T, store *reviewstore.Store, name string) (*http.Cookie, string) { + t.Helper() + session := testsupport.RemoteSession(t, store, testsupport.RemoteAccount(t, store, name)) + return &http.Cookie{Name: sessionCookie, Value: session.Secret}, session.CSRFToken +} func TestRemoteSessionCSRFAndTokenLifecycle(t *testing.T) { - store, handler := identityHandler(t, Settings{Username: "alice", Password: testPassword}) + store, handler := identityHandler(t, Settings{}) + testsupport.RemoteAdmin(t, store, "alice", testPassword) cookie, csrf := loginIdentity(t, handler, "alice") if !cookie.HttpOnly || !cookie.Secure || cookie.SameSite != http.SameSiteLaxMode || cookie.Path != "/" { t.Fatalf("session cookie: %+v", cookie) @@ -116,7 +125,7 @@ func TestRemoteSessionCSRFAndTokenLifecycle(t *testing.T) { if w.Code != 403 { t.Fatal("api token managed account") } - otherCookie, otherCSRF := loginIdentity(t, handler, "other") + otherCookie, otherCSRF := memberSession(t, store, "member") w = identityRequest(handler, "DELETE", "/api/v2/account/tokens/"+created.Token.ID, "", otherCookie, otherCSRF, "") if w.Code != 404 { t.Fatal("foreign token revoked") @@ -147,11 +156,23 @@ func TestRemoteSessionCSRFAndTokenLifecycle(t *testing.T) { } } } -func TestSignupActivatesAccountWithoutRestart(t *testing.T) { - _, handler := identityHandler(t, Settings{SignupEnabled: true, InsecureCookies: true}) - w := identityRequest(handler, "POST", "/api/v2/auth/signup", `{"username":"newuser","password":"`+testPassword+`"}`, nil, "", "") +func TestSetupActivatesAdminWithoutRestart(t *testing.T) { + store, handler := identityHandler(t, Settings{InsecureCookies: true}) + setup, err := identityservice.New(store).PrepareSetup(t.Context()) + if err != nil { + t.Fatal(err) + } + w := identityRequest(handler, "GET", "/api/v2/setup", "", nil, "", "") + if w.Code != 200 || !strings.Contains(w.Body.String(), `"required":true`) { + t.Fatalf("setup required: %d %s", w.Code, w.Body.String()) + } + w = identityRequest(handler, "POST", "/api/v2/setup", `{"token":"`+strings.Repeat("x", 43)+`","username":"admin","password":"`+testPassword+`"}`, nil, "", "") + if w.Code != 404 { + t.Fatalf("forged setup: %d %s", w.Code, w.Body.String()) + } + w = identityRequest(handler, "POST", "/api/v2/setup", `{"token":"`+setup+`","username":"admin","password":"`+testPassword+`"}`, nil, "", "") if w.Code != 200 { - t.Fatalf("signup: %d %s", w.Code, w.Body.String()) + t.Fatalf("setup: %d %s", w.Code, w.Body.String()) } cookies := w.Result().Cookies() if len(cookies) != 1 || cookies[0].Secure { @@ -161,8 +182,16 @@ func TestSignupActivatesAccountWithoutRestart(t *testing.T) { if err := json.Unmarshal(w.Body.Bytes(), &result); err != nil { t.Fatal(err) } - if result.User == nil || result.CSRFToken == nil { - t.Fatal("signup identity absent") + if result.User == nil || !result.User.Admin || result.CSRFToken == nil { + t.Fatal("setup identity absent") + } + w = identityRequest(handler, "GET", "/api/v2/setup", "", nil, "", "") + if w.Code != 200 || !strings.Contains(w.Body.String(), `"required":false`) { + t.Fatalf("setup still required: %d %s", w.Code, w.Body.String()) + } + w = identityRequest(handler, "POST", "/api/v2/setup", `{"token":"`+setup+`","username":"second","password":"`+testPassword+`"}`, nil, "", "") + if w.Code != 404 { + t.Fatalf("setup reused: %d", w.Code) } w = identityRequest(handler, "POST", "/api/v2/account/tokens", `{"name":"collector","purpose":"collector","expiresInDays":1}`, cookies[0], *result.CSRFToken, "") if w.Code != 201 { @@ -177,7 +206,7 @@ func TestSignupActivatesAccountWithoutRestart(t *testing.T) { server := httptest.NewServer(handler) defer server.Close() patch := "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-before\n+after\n" - input, err := collector.CollectPatch(t.Context(), patch, t.TempDir(), collector.Options{SourceID: "signup", SubmissionID: "first"}) + input, err := collector.CollectPatch(t.Context(), patch, t.TempDir(), collector.Options{SourceID: "setup", SubmissionID: "first"}) if err != nil { t.Fatal(err) } @@ -195,12 +224,29 @@ func TestSignupActivatesAccountWithoutRestart(t *testing.T) { t.Fatalf("new owner handler did not activate: %d %s", w.Code, w.Body.String()) } } -func TestAuthFormsGuardAndRateLimit(t *testing.T) { - _, handler := identityHandler(t, Settings{Username: "alice", Password: testPassword}) - w := identityRequest(handler, "POST", "/api/v2/auth/signup", `{}`, nil, "", "") - if w.Code != 403 { - t.Fatal("signup enabled by default") +func TestPasswordLoginIsAdminOnly(t *testing.T) { + store, handler := identityHandler(t, Settings{}) + testsupport.RemoteAdmin(t, store, "alice", testPassword) + testsupport.RemoteAccount(t, store, "member") + for _, body := range []string{`{"username":"member","password":""}`, `{"username":"member","password":"` + testPassword + `"}`, `{"username":"nobody","password":"` + testPassword + `"}`, `{"username":"alice","password":"wrong-password-value"}`} { + w := identityRequest(handler, "POST", "/api/v2/auth/login", body, nil, "", "") + if w.Code != 401 || !strings.Contains(w.Body.String(), "Authentication required") { + t.Fatalf("login %s: %d %s", body, w.Code, w.Body.String()) + } } + cookie, csrf := loginIdentity(t, handler, "alice") + if w := identityRequest(handler, "POST", "/api/v2/auth/signup", `{"username":"new","password":"`+testPassword+`"}`, cookie, csrf, ""); w.Code != 404 { + 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`) { + t.Fatalf("bootstrap: %s", w.Body.String()) + } +} +func TestAuthFormsGuardAndRateLimit(t *testing.T) { + store, handler := identityHandler(t, Settings{}) + testsupport.RemoteAdmin(t, store, "alice", testPassword) + var w *httptest.ResponseRecorder for _, test := range []struct { header, value string want int @@ -224,7 +270,8 @@ func TestAuthFormsGuardAndRateLimit(t *testing.T) { func TestRevocationAndExpiryCloseActiveStreams(t *testing.T) { for _, action := range []string{"logout", "reset", "revoke", "expire"} { t.Run(action, func(t *testing.T) { - store, handler := identityHandler(t, Settings{Username: "alice", Password: testPassword, InsecureCookies: true}) + store, handler := identityHandler(t, Settings{InsecureCookies: true}) + testsupport.RemoteAdmin(t, store, "alice", testPassword) identity := identityservice.New(store) session, err := identity.Login(t.Context(), "alice", testPassword) if err != nil { diff --git a/internal/remoteserver/server.go b/internal/remoteserver/server.go index 766ee867..a4808734 100644 --- a/internal/remoteserver/server.go +++ b/internal/remoteserver/server.go @@ -5,8 +5,10 @@ import ( "context" "errors" "io/fs" + "log" "net" "net/http" + "strings" "sync" "time" @@ -21,11 +23,10 @@ import ( ) type Settings struct { - Listen string - State string - Username string - Password string - SignupEnabled bool + Listen string + State string + // PublicURL is the browser-facing origin used in the logged setup link. + PublicURL string InsecureCookies bool RetentionDays int GitHub *GitHubSettings @@ -58,6 +59,19 @@ func (registry *registry) get(account identityservice.Account) (application, err registry.applications[account.ID] = app return app, nil } + +// remove stops a deleted or disabled owner's services; a later request for an +// enabled account activates them again. +func (registry *registry) remove(accountID string) error { + registry.mu.Lock() + app, ok := registry.applications[accountID] + delete(registry.applications, accountID) + registry.mu.Unlock() + if !ok { + return nil + } + return app.service.Close() +} func (registry *registry) close() error { registry.mu.Lock() defer registry.mu.Unlock() @@ -106,9 +120,6 @@ func MultiHandler(ctx context.Context, store *reviewstore.Store, assets fs.FS, o // internal fetcher handler. func compose(ctx context.Context, store *reviewstore.Store, assets fs.FS, settings Settings) (http.Handler, http.Handler, func() error, []serverapp.Task, error) { identity := identityservice.New(store) - if err := bootstrap(ctx, identity, settings); err != nil { - return nil, nil, nil, nil, err - } github, err := composeGitHub(store, settings.GitHub) if err != nil { return nil, nil, nil, nil, err @@ -142,24 +153,6 @@ func compose(ctx context.Context, store *reviewstore.Store, assets fs.FS, settin } return remote, internal, applications.close, tasks, nil } -func bootstrap(ctx context.Context, identity *identityservice.Service, settings Settings) error { - if (settings.Username == "") != (settings.Password == "") { - return errors.New("DIFFX_SERVER_USERNAME and DIFFX_SERVER_PASSWORD must be supplied together") - } - if settings.Username != "" { - if _, err := identity.Bootstrap(ctx, settings.Username, settings.Password); err != nil { - return err - } - } - accounts, err := identity.Accounts(ctx) - if err != nil { - return err - } - if len(accounts) == 0 && !settings.SignupEnabled { - return errors.New("remote server needs a provisioned user, bootstrap username/password, or signup enabled") - } - return nil -} func Run(ctx context.Context, settings Settings, ready func(string)) error { if settings.State == "" { return errors.New("remote state database path required") @@ -203,10 +196,21 @@ func Run(ctx context.Context, settings Settings, ready func(string)) error { defer internalServer.Close() endpoints = append(endpoints, serverapp.Endpoint{Server: internalServer, Listener: internalListener}) } + setup, err := identityservice.New(store).PrepareSetup(ctx) + if err != nil { + return err + } return serverapp.Run(ctx, endpoints, append(tasks, serverapp.Prune(store.PruneExpired)), func() { if ready != nil { ready(listener.Addr().String()) } + if setup != "" { + origin := strings.TrimSuffix(settings.PublicURL, "/") + if origin == "" { + origin = "http://" + listener.Addr().String() + } + log.Printf("diffx-server setup: no admin yet; within %s open %s/setup#t=%s", identityservice.SetupLifetime, origin, setup) + } }) } func Retention(days int) (time.Duration, error) { diff --git a/internal/remoteserver/server_test.go b/internal/remoteserver/server_test.go index ef38fcd8..b9912cc6 100644 --- a/internal/remoteserver/server_test.go +++ b/internal/remoteserver/server_test.go @@ -26,10 +26,7 @@ const testPassword = "test-password-at-least-twelve-bytes" func provisionToken(t *testing.T, store *reviewstore.Store, name string, purpose identityservice.Purpose) string { t.Helper() identity := identityservice.New(store) - account, err := identity.Provision(t.Context(), name, testPassword) - if err != nil { - t.Fatal(err) - } + account := testsupport.RemoteAccount(t, store, name) token, err := identity.CreateToken(t.Context(), account.ID, "test", purpose, time.Time{}) if err != nil { t.Fatal(err) @@ -250,61 +247,6 @@ func TestMultiUserRESTMCPAndEventIsolation(t *testing.T) { } } -func TestPasswordBootstrapSurvivesRestart(t *testing.T) { - path := filepath.Join(t.TempDir(), "state.db") - store, err := reviewstore.Open(path) - if err != nil { - t.Fatal(err) - } - settings := Settings{Username: "admin", Password: testPassword} - identity := identityservice.New(store) - if err := bootstrap(t.Context(), identity, settings); err != nil { - t.Fatal(err) - } - session, err := identity.Login(t.Context(), "admin", testPassword) - if err != nil { - t.Fatal(err) - } - if err := identity.ChangePassword(t.Context(), session.Account.ID, testPassword, "changed-password-123"); err != nil { - t.Fatal(err) - } - if err := store.Close(); err != nil { - t.Fatal(err) - } - store, err = reviewstore.Open(path) - if err != nil { - t.Fatal(err) - } - defer store.Close() - identity = identityservice.New(store) - if err := bootstrap(t.Context(), identity, settings); err != nil { - t.Fatal(err) - } - if _, err := identity.Login(t.Context(), "admin", testPassword); err == nil { - t.Fatal("restart replaced changed password") - } - again, err := identity.Login(t.Context(), "admin", "changed-password-123") - if err != nil || again.Account.ID != session.Account.ID { - t.Fatalf("restart identity: %+v %v", again, err) - } -} -func TestEmptyServerRequiresExplicitIdentityPolicy(t *testing.T) { - store, err := reviewstore.Open("") - if err != nil { - t.Fatal(err) - } - defer store.Close() - identity := identityservice.New(store) - for _, settings := range []Settings{{}, {Username: "admin"}, {Password: testPassword}} { - if err := bootstrap(t.Context(), identity, settings); err == nil { - t.Fatal("missing policy accepted") - } - } - if err := bootstrap(t.Context(), identity, Settings{SignupEnabled: true}); err != nil { - t.Fatal(err) - } -} - func TestRetentionDurationValidation(t *testing.T) { for days, expected := range map[int]time.Duration{0: 7 * 24 * time.Hour, 7: 7 * 24 * time.Hour, 1: 24 * time.Hour, 30: 30 * 24 * time.Hour} { actual, err := Retention(days) @@ -325,8 +267,17 @@ func TestRemoteRunShutdownClosesActiveEventStream(t *testing.T) { defer cancel() ready := make(chan string, 1) done := make(chan error, 1) + state := filepath.Join(t.TempDir(), "reviews.sqlite") + store, err := reviewstore.Open(state) + if err != nil { + t.Fatal(err) + } + testsupport.RemoteAdmin(t, store, "account", testPassword) + if err := store.Close(); err != nil { + t.Fatal(err) + } go func() { - done <- Run(ctx, Settings{Listen: "127.0.0.1:0", State: filepath.Join(t.TempDir(), "reviews.sqlite"), Username: "account", Password: testPassword, InsecureCookies: true}, func(address string) { ready <- address }) + done <- Run(ctx, Settings{Listen: "127.0.0.1:0", State: state, InsecureCookies: true}, func(address string) { ready <- address }) }() var address string select { diff --git a/internal/reviewstore/authentication.go b/internal/reviewstore/authentication.go index 6ddf12b1..193cc2b2 100644 --- a/internal/reviewstore/authentication.go +++ b/internal/reviewstore/authentication.go @@ -26,6 +26,30 @@ func initializeAuthentication(transaction *sql.Tx) error { secret_hash TEXT NOT NULL UNIQUE, created_at INTEGER NOT NULL, last_used_at INTEGER, expires_at INTEGER NOT NULL)`, `CREATE INDEX IF NOT EXISTS identity_tokens_owner ON identity_tokens(user_id)`, + // Schema 13 adds administration and Google sign-in; every table is additive. + // The singleton keys allow at most one admin and one pending setup secret. + `CREATE TABLE IF NOT EXISTS admin_account ( + singleton INTEGER PRIMARY KEY CHECK(singleton=1), + user_id TEXT NOT NULL UNIQUE REFERENCES remote_accounts(user_id) ON DELETE CASCADE)`, + `CREATE TABLE IF NOT EXISTS admin_setup ( + singleton INTEGER PRIMARY KEY CHECK(singleton=1), + token_hash TEXT NOT NULL, expires_at INTEGER NOT NULL)`, + `CREATE TABLE IF NOT EXISTS disabled_accounts ( + user_id TEXT PRIMARY KEY REFERENCES remote_accounts(user_id) ON DELETE CASCADE, + disabled_at INTEGER NOT NULL)`, + // Google's subject identifies the person; email is the latest one Google reported. + `CREATE TABLE IF NOT EXISTS google_identities ( + sub TEXT PRIMARY KEY, user_id TEXT NOT NULL UNIQUE REFERENCES remote_accounts(user_id) ON DELETE CASCADE, + email TEXT NOT NULL, updated_at INTEGER NOT NULL)`, + `CREATE INDEX IF NOT EXISTS google_identities_email ON google_identities(email)`, + `CREATE TABLE IF NOT EXISTS account_invites ( + id TEXT PRIMARY KEY, token_hash TEXT NOT NULL UNIQUE, email TEXT NOT NULL, + created_at INTEGER NOT NULL, expires_at INTEGER NOT NULL, + used_at INTEGER, used_by TEXT REFERENCES remote_accounts(user_id) ON DELETE SET NULL, revoked_at INTEGER)`, + `CREATE INDEX IF NOT EXISTS account_invites_email ON account_invites(email)`, + `CREATE TABLE IF NOT EXISTS google_auth_states ( + state_hash TEXT PRIMARY KEY, binding_hash TEXT NOT NULL, nonce TEXT NOT NULL, pkce_verifier TEXT NOT NULL, + invite_id TEXT REFERENCES account_invites(id) ON DELETE CASCADE, expires_at INTEGER NOT NULL)`, } for _, statement := range statements { if _, err := transaction.Exec(statement); err != nil { @@ -35,45 +59,173 @@ func initializeAuthentication(transaction *sql.Tx) error { return nil } -// CreateAccount commits the owner and its initial credential together. Bootstrap -// checks all remote accounts in this transaction, so changed env never creates -// another owner or replaces credentials on restart. -func (store *Store) CreateAccount(ctx context.Context, account identityservice.Account, passwordHash string, bootstrap bool, created time.Time) (identityservice.Account, error) { +// accountColumns selects an Account from remote_accounts aliased as a. +const accountColumns = `a.user_id,a.username,EXISTS(SELECT 1 FROM admin_account WHERE user_id=a.user_id)` + +// enabled restricts remote_accounts aliased as a to accounts that may authenticate. +const enabled = `NOT EXISTS(SELECT 1 FROM disabled_accounts WHERE user_id=a.user_id)` + +func insertAccount(ctx context.Context, transaction *sql.Tx, account identityservice.Account, passwordHash string, created time.Time) error { + if _, err := transaction.ExecContext(ctx, `INSERT INTO users(id,os_uid,name,created_at) VALUES(?,?,?,?)`, account.ID, "remote:"+account.ID, account.Username, created.UnixMilli()); err != nil { + return err + } + result, err := transaction.ExecContext(ctx, `INSERT INTO remote_accounts(user_id,username,password_hash) VALUES(?,?,?) ON CONFLICT(username) DO NOTHING`, account.ID, account.Username, passwordHash) + if err != nil { + return err + } + count, err := result.RowsAffected() + if err != nil { + return err + } + if count == 0 { + return identityservice.ErrConflict + } + return nil +} + +// CreateAccount commits a non-admin owner and its credential together. An empty +// hash never verifies, so such an account has no password sign-in. +func (store *Store) CreateAccount(ctx context.Context, account identityservice.Account, passwordHash string, created time.Time) error { transaction, err := store.db.BeginTx(ctx, nil) if err != nil { - return identityservice.Account{}, err + return err } defer transaction.Rollback() - if bootstrap { - var existing identityservice.Account - err := transaction.QueryRowContext(ctx, `SELECT user_id,username FROM remote_accounts ORDER BY username LIMIT 1`).Scan(&existing.ID, &existing.Username) - if err == nil { - return existing, nil - } - if !errors.Is(err, sql.ErrNoRows) { - return identityservice.Account{}, err - } + if err := insertAccount(ctx, transaction, account, passwordHash, created); err != nil { + return err } - if _, err := transaction.ExecContext(ctx, `INSERT INTO users(id,os_uid,name,created_at) VALUES(?,?,?,?)`, account.ID, "remote:"+account.ID, account.Username, created.UnixMilli()); err != nil { - return identityservice.Account{}, err + return transaction.Commit() +} + +func (store *Store) PutAdminSetup(ctx context.Context, tokenHash string, expires time.Time) (bool, error) { + result, err := store.db.ExecContext(ctx, `INSERT INTO admin_setup(singleton,token_hash,expires_at) SELECT 1,?,? WHERE NOT EXISTS(SELECT 1 FROM admin_account) + ON CONFLICT(singleton) DO UPDATE SET token_hash=excluded.token_hash,expires_at=excluded.expires_at`, tokenHash, expires.UnixMilli()) + if err != nil { + return false, err } - result, err := transaction.ExecContext(ctx, `INSERT INTO remote_accounts(user_id,username,password_hash) VALUES(?,?,?) ON CONFLICT(username) DO NOTHING`, account.ID, account.Username, passwordHash) + count, err := result.RowsAffected() + return count == 1, err +} + +func (store *Store) AdminExists(ctx context.Context) (bool, error) { + var exists bool + err := store.db.QueryRowContext(ctx, `SELECT EXISTS(SELECT 1 FROM admin_account)`).Scan(&exists) + return exists, err +} + +// CreateAdmin consumes the setup secret, so concurrent setups create one admin. +func (store *Store) CreateAdmin(ctx context.Context, account identityservice.Account, passwordHash, setupHash string, now time.Time) error { + transaction, err := store.db.BeginTx(ctx, nil) if err != nil { - return identityservice.Account{}, err + return err + } + defer transaction.Rollback() + result, err := transaction.ExecContext(ctx, `DELETE FROM admin_setup WHERE token_hash=? AND expires_at>? AND NOT EXISTS(SELECT 1 FROM admin_account)`, setupHash, now.UnixMilli()) + if err != nil { + return err } count, err := result.RowsAffected() if err != nil { - return identityservice.Account{}, err + return err } - if count == 0 { - return identityservice.Account{}, identityservice.ErrConflict + if count != 1 { + return identityservice.ErrNotFound + } + if err := insertAccount(ctx, transaction, account, passwordHash, now); err != nil { + return err + } + if _, err := transaction.ExecContext(ctx, `INSERT INTO admin_account(singleton,user_id) VALUES(1,?)`, account.ID); err != nil { + return err } - return account, transaction.Commit() + return transaction.Commit() +} + +func (store *Store) AccountSummaries(ctx context.Context) ([]identityservice.AccountSummary, error) { + rows, err := store.db.QueryContext(ctx, `SELECT `+accountColumns+`,COALESCE(g.email,''),NOT `+enabled+`,u.created_at + FROM remote_accounts a JOIN users u ON u.id=a.user_id LEFT JOIN google_identities g ON g.user_id=a.user_id ORDER BY a.username`) + if err != nil { + return nil, err + } + defer rows.Close() + accounts := make([]identityservice.AccountSummary, 0) + for rows.Next() { + var account identityservice.AccountSummary + var created int64 + if err := rows.Scan(&account.ID, &account.Username, &account.Admin, &account.Email, &account.Disabled, &created); err != nil { + return nil, err + } + account.CreatedAt = time.UnixMilli(created) + accounts = append(accounts, account) + } + return accounts, rows.Err() +} + +// targetAccount locks in the account an admin action applies to, refusing the admin. +func targetAccount(ctx context.Context, transaction *sql.Tx, accountID string) error { + var admin bool + err := transaction.QueryRowContext(ctx, `SELECT EXISTS(SELECT 1 FROM admin_account WHERE user_id=a.user_id) FROM remote_accounts a WHERE a.user_id=?`, accountID).Scan(&admin) + if errors.Is(err, sql.ErrNoRows) { + return identityservice.ErrNotFound + } + if err != nil { + return err + } + if admin { + return identityservice.ErrForbidden + } + return nil +} + +// SetAccountDisabled revokes every browser session and API token at the commit +// that disables the account; enabling restores sign-in, not those credentials. +func (store *Store) SetAccountDisabled(ctx context.Context, accountID string, disabled bool, at time.Time) error { + transaction, err := store.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer transaction.Rollback() + if err := targetAccount(ctx, transaction, accountID); err != nil { + return err + } + statements := []string{`DELETE FROM disabled_accounts WHERE user_id=?`} + if disabled { + statements = []string{`INSERT INTO disabled_accounts(user_id,disabled_at) VALUES(?,?) ON CONFLICT(user_id) DO NOTHING`, `DELETE FROM browser_sessions WHERE user_id=?`, `DELETE FROM identity_tokens WHERE user_id=?`} + } + for index, statement := range statements { + arguments := []any{accountID} + if disabled && index == 0 { + arguments = append(arguments, at.UnixMilli()) + } + if _, err := transaction.ExecContext(ctx, statement, arguments...); err != nil { + return err + } + } + return transaction.Commit() +} + +// DeleteAccount removes the owner row; foreign keys cascade to everything it +// owns. Queued jobs reference ingestion records without a cascade, so they go first. +func (store *Store) DeleteAccount(ctx context.Context, accountID string) error { + transaction, err := store.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer transaction.Rollback() + if err := targetAccount(ctx, transaction, accountID); err != nil { + return err + } + if _, err := transaction.ExecContext(ctx, `DELETE FROM ingestion_jobs WHERE sequence IN (SELECT sequence FROM ingestion_records WHERE owner_id=?)`, accountID); err != nil { + return err + } + if _, err := transaction.ExecContext(ctx, `DELETE FROM users WHERE id=?`, accountID); err != nil { + return err + } + return transaction.Commit() } func (store *Store) AccountPassword(ctx context.Context, username string) (identityservice.PasswordCredential, error) { var result identityservice.PasswordCredential - err := store.db.QueryRowContext(ctx, `SELECT user_id,username,password_hash FROM remote_accounts WHERE username=?`, username).Scan(&result.Account.ID, &result.Account.Username, &result.Hash) + err := store.db.QueryRowContext(ctx, `SELECT `+accountColumns+`,a.password_hash FROM remote_accounts a WHERE a.username=? AND `+enabled, username).Scan(&result.Account.ID, &result.Account.Username, &result.Account.Admin, &result.Hash) if errors.Is(err, sql.ErrNoRows) { return result, identityservice.ErrNotFound } @@ -82,7 +234,7 @@ func (store *Store) AccountPassword(ctx context.Context, username string) (ident func (store *Store) AccountPasswordByID(ctx context.Context, accountID string) (identityservice.PasswordCredential, error) { var result identityservice.PasswordCredential - err := store.db.QueryRowContext(ctx, `SELECT user_id,username,password_hash FROM remote_accounts WHERE user_id=?`, accountID).Scan(&result.Account.ID, &result.Account.Username, &result.Hash) + err := store.db.QueryRowContext(ctx, `SELECT `+accountColumns+`,a.password_hash FROM remote_accounts a WHERE a.user_id=? AND `+enabled, accountID).Scan(&result.Account.ID, &result.Account.Username, &result.Account.Admin, &result.Hash) if errors.Is(err, sql.ErrNoRows) { return result, identityservice.ErrNotFound } @@ -124,7 +276,7 @@ func (store *Store) CreateSession(ctx context.Context, accountID, expected, secr if _, err := transaction.ExecContext(ctx, `DELETE FROM browser_sessions WHERE expires_at<=?`, expires.Add(-identityservice.SessionLifetime).UnixMilli()); err != nil { return err } - result, err := transaction.ExecContext(ctx, `INSERT INTO browser_sessions(secret_hash,user_id,csrf_token,expires_at) SELECT ?,user_id,?,? FROM remote_accounts WHERE user_id=? AND password_hash=?`, secretHash, csrf, expires.UnixMilli(), accountID, expected) + result, err := transaction.ExecContext(ctx, `INSERT INTO browser_sessions(secret_hash,user_id,csrf_token,expires_at) SELECT ?,a.user_id,?,? FROM remote_accounts a WHERE a.user_id=? AND a.password_hash=? AND `+enabled, secretHash, csrf, expires.UnixMilli(), accountID, expected) if err != nil { return err } @@ -141,7 +293,7 @@ func (store *Store) CreateSession(ctx context.Context, accountID, expected, secr func (store *Store) ResolveSession(ctx context.Context, secretHash string, now time.Time) (identityservice.Session, error) { var session identityservice.Session var expires int64 - err := store.db.QueryRowContext(ctx, `SELECT a.user_id,a.username,s.csrf_token,s.expires_at FROM browser_sessions s JOIN remote_accounts a ON a.user_id=s.user_id WHERE s.secret_hash=? AND s.expires_at>?`, secretHash, now.UnixMilli()).Scan(&session.Account.ID, &session.Account.Username, &session.CSRFToken, &expires) + err := store.db.QueryRowContext(ctx, `SELECT `+accountColumns+`,s.csrf_token,s.expires_at FROM browser_sessions s JOIN remote_accounts a ON a.user_id=s.user_id WHERE s.secret_hash=? AND s.expires_at>? AND `+enabled, secretHash, now.UnixMilli()).Scan(&session.Account.ID, &session.Account.Username, &session.Account.Admin, &session.CSRFToken, &expires) if errors.Is(err, sql.ErrNoRows) { return session, identityservice.ErrAuthentication } @@ -217,7 +369,7 @@ func (store *Store) ResolveIdentityToken(ctx context.Context, secretHash string, defer transaction.Rollback() var result identityservice.TokenIdentity var expires int64 - err = transaction.QueryRowContext(ctx, `SELECT a.user_id,a.username,t.purpose,t.expires_at FROM identity_tokens t JOIN remote_accounts a ON a.user_id=t.user_id WHERE t.secret_hash=? AND t.expires_at>?`, secretHash, now.UnixMilli()).Scan(&result.Account.ID, &result.Account.Username, &result.Purpose, &expires) + err = transaction.QueryRowContext(ctx, `SELECT `+accountColumns+`,t.purpose,t.expires_at FROM identity_tokens t JOIN remote_accounts a ON a.user_id=t.user_id WHERE t.secret_hash=? AND t.expires_at>? AND `+enabled, secretHash, now.UnixMilli()).Scan(&result.Account.ID, &result.Account.Username, &result.Account.Admin, &result.Purpose, &expires) if errors.Is(err, sql.ErrNoRows) { return result, identityservice.ErrAuthentication } @@ -232,7 +384,7 @@ func (store *Store) ResolveIdentityToken(ctx context.Context, secretHash string, } func (store *Store) IdentityAccounts(ctx context.Context) ([]identityservice.Account, error) { - rows, err := store.db.QueryContext(ctx, `SELECT user_id,username FROM remote_accounts ORDER BY username`) + rows, err := store.db.QueryContext(ctx, `SELECT `+accountColumns+` FROM remote_accounts a WHERE `+enabled+` ORDER BY a.username`) if err != nil { return nil, err } @@ -240,7 +392,7 @@ func (store *Store) IdentityAccounts(ctx context.Context) ([]identityservice.Acc accounts := make([]identityservice.Account, 0) for rows.Next() { var account identityservice.Account - if err := rows.Scan(&account.ID, &account.Username); err != nil { + if err := rows.Scan(&account.ID, &account.Username, &account.Admin); err != nil { return nil, err } accounts = append(accounts, account) diff --git a/internal/reviewstore/authentication_test.go b/internal/reviewstore/authentication_test.go index ac726927..0b37de71 100644 --- a/internal/reviewstore/authentication_test.go +++ b/internal/reviewstore/authentication_test.go @@ -5,7 +5,9 @@ import ( "crypto/sha256" "encoding/hex" "errors" + "fmt" "path/filepath" + "strings" "sync" "testing" "time" @@ -30,7 +32,34 @@ func secretHash(secret string) string { return hex.EncodeToString(hash[:]) } -func TestRemoteAccountPersistenceAndBootstrapOnce(t *testing.T) { +// member creates a passwordless account and a session for it, as Google sign-in does. +func member(t *testing.T, store *Store, username string) (identityservice.Account, identityservice.Session) { + t.Helper() + account := identityservice.Account{ID: "id-" + username, Username: username} + if err := store.CreateAccount(t.Context(), account, "", time.Now()); err != nil { + t.Fatal(err) + } + session := identityservice.Session{Account: account, Secret: strings.Repeat("s", 40) + username[:3], CSRFToken: "csrf-" + username, ExpiresAt: time.Now().Add(time.Hour)} + if err := store.CreateSession(t.Context(), account.ID, "", secretHash(session.Secret), session.CSRFToken, session.ExpiresAt); err != nil { + t.Fatal(err) + } + return account, session +} + +func setupAdmin(t *testing.T, service *identityservice.Service, username string) identityservice.Session { + t.Helper() + secret, err := service.PrepareSetup(t.Context()) + if err != nil || secret == "" { + t.Fatalf("prepare setup: %q %v", secret, err) + } + session, err := service.CompleteSetup(t.Context(), secret, username, testPassword) + if err != nil { + t.Fatal(err) + } + return session +} + +func TestAdminSetupPersistsAndRunsOnce(t *testing.T) { ctx := context.Background() path := filepath.Join(t.TempDir(), "state.db") store, err := Open(path) @@ -38,43 +67,55 @@ func TestRemoteAccountPersistenceAndBootstrapOnce(t *testing.T) { t.Fatal(err) } service := identityservice.New(store) - admin, err := service.Bootstrap(ctx, "Admin", testPassword) - if err != nil || admin.Username != "admin" { - t.Fatalf("bootstrap: %+v %v", admin, err) + if required, err := service.SetupRequired(ctx); err != nil || !required { + t.Fatalf("fresh state requires setup: %v %v", required, err) } - session, err := service.Login(ctx, "ADMIN", testPassword) + replaced, err := service.PrepareSetup(ctx) if err != nil { t.Fatal(err) } - token, err := service.CreateToken(ctx, admin.ID, "collector", identityservice.PurposeCollector, time.Time{}) + secret, err := service.PrepareSetup(ctx) + if err != nil || secret == "" || secret == replaced { + t.Fatalf("prepare setup: %q %v", secret, err) + } + var stored string + if err := store.db.QueryRow(`SELECT token_hash FROM admin_setup`).Scan(&stored); err != nil || stored != secretHash(secret) { + t.Fatalf("setup secret not hashed: %v", err) + } + if _, err := service.CompleteSetup(ctx, replaced, "admin", testPassword); !errors.Is(err, identityservice.ErrNotFound) { + t.Fatalf("replaced setup secret accepted: %v", err) + } + session, err := service.CompleteSetup(ctx, secret, "Admin", testPassword) + if err != nil || !session.Account.Admin || session.Account.Username != "admin" { + t.Fatalf("setup: %+v %v", session, err) + } + if _, err := service.CompleteSetup(ctx, secret, "second", testPassword); !errors.Is(err, identityservice.ErrNotFound) { + t.Fatalf("setup secret reused: %v", err) + } + token, err := service.CreateToken(ctx, session.Account.ID, "collector", identityservice.PurposeCollector, time.Time{}) if err != nil { t.Fatal(err) } - other, err := service.Provision(ctx, "other", testPassword) - if err != nil || other.ID == admin.ID { - t.Fatalf("provision: %+v %v", other, err) + other, _ := member(t, store, "other") + if _, err := service.Login(ctx, "other", ""); !errors.Is(err, identityservice.ErrAuthentication) { + t.Fatalf("member password login: %v", err) + } + if err := service.ChangePassword(ctx, other.ID, "", "member password is valid"); !errors.Is(err, identityservice.ErrAuthentication) { + t.Fatalf("member gained a password: %v", err) } - if _, err := service.Provision(ctx, "ADMIN", testPassword); !errors.Is(err, identityservice.ErrConflict) { - t.Fatalf("duplicate canonical username: %v", err) + if err := service.ResetPassword(ctx, "other", "member password is valid"); !errors.Is(err, identityservice.ErrNotFound) { + t.Fatalf("member password reset: %v", err) } - if err := service.ChangePassword(ctx, admin.ID, testPassword, "changed password is valid"); err != nil { + if err := service.ChangePassword(ctx, session.Account.ID, testPassword, "changed password is valid"); err != nil { t.Fatal(err) } - var password, tokenHash, sessionHash string - if err := store.db.QueryRow(`SELECT password_hash FROM remote_accounts WHERE user_id=?`, admin.ID).Scan(&password); err != nil || password == testPassword { + var password, tokenHash string + if err := store.db.QueryRow(`SELECT password_hash FROM remote_accounts WHERE user_id=?`, session.Account.ID).Scan(&password); err != nil || password == testPassword { t.Fatalf("password persisted unsafely: %v", err) } if err := store.db.QueryRow(`SELECT secret_hash FROM identity_tokens WHERE id=?`, token.ID).Scan(&tokenHash); err != nil || tokenHash != secretHash(token.Secret) { t.Fatalf("token not hashed: %v", err) } - // Issue a session for the second owner to inspect durable hashing independently. - otherSession, err := service.Login(ctx, "other", testPassword) - if err != nil { - t.Fatal(err) - } - if err := store.db.QueryRow(`SELECT secret_hash FROM browser_sessions WHERE user_id=?`, other.ID).Scan(&sessionHash); err != nil || sessionHash != secretHash(otherSession.Secret) { - t.Fatalf("session not hashed: %v", err) - } if err := store.Close(); err != nil { t.Fatal(err) } @@ -84,45 +125,119 @@ func TestRemoteAccountPersistenceAndBootstrapOnce(t *testing.T) { } defer store.Close() service = identityservice.New(store) - again, err := service.Bootstrap(ctx, "new-admin", "another valid password") - // Bootstrap is globally one-time, even when an env username is changed. - if err != nil || again != admin { - t.Fatalf("bootstrap changed owner: %+v %v", again, err) + if again, err := service.PrepareSetup(ctx); err != nil || again != "" { + t.Fatalf("setup issued after admin exists: %q %v", again, err) } - accounts, err := service.Accounts(ctx) - if err != nil || len(accounts) != 2 { - t.Fatalf("bootstrap created owner: %+v %v", accounts, err) + if required, err := service.SetupRequired(ctx); err != nil || required { + t.Fatalf("setup still required: %v %v", required, err) } if _, err := service.Login(ctx, "admin", testPassword); !errors.Is(err, identityservice.ErrAuthentication) { t.Fatalf("old password accepted: %v", err) } - if _, err := service.Login(ctx, "admin", "changed password is valid"); err != nil { + if _, err := service.Login(ctx, "ADMIN", "changed password is valid"); err != nil { t.Fatalf("restart replaced password: %v", err) } if _, err := service.AuthenticateSession(ctx, session.Secret); !errors.Is(err, identityservice.ErrAuthentication) { t.Fatalf("password change retained session: %v", err) } identity, err := service.AuthenticateToken(ctx, token.Secret) - if err != nil || identity.Account != admin || identity.Purpose != identityservice.PurposeCollector { + if err != nil || identity.Account != session.Account || identity.Purpose != identityservice.PurposeCollector { t.Fatalf("password change affected token: %+v %v", identity, err) } } -func TestSessionTokenIsolationExpiryAndRevocation(t *testing.T) { +func TestExpiredSetupSecretIsRefused(t *testing.T) { store, service := testIdentity(t) - ctx := context.Background() - first, err := service.Provision(ctx, "first", testPassword) + secret, err := service.PrepareSetup(t.Context()) if err != nil { t.Fatal(err) } - second, err := service.Provision(ctx, "second", testPassword) - if err != nil { + if _, err := store.db.Exec(`UPDATE admin_setup SET expires_at=?`, time.Now().UnixMilli()); err != nil { t.Fatal(err) } - session, err := service.Login(ctx, "first", testPassword) + if _, err := service.CompleteSetup(t.Context(), secret, "admin", testPassword); !errors.Is(err, identityservice.ErrNotFound) { + t.Fatalf("expired setup accepted: %v", err) + } +} + +func TestDisableAndDeleteAccount(t *testing.T) { + store, service := testIdentity(t) + ctx := context.Background() + admin := setupAdmin(t, service, "admin") + account, session := member(t, store, "member") + token, err := service.CreateToken(ctx, account.ID, "collector", identityservice.PurposeCollector, time.Time{}) if err != nil { t.Fatal(err) } + for _, err := range []error{service.DisableAccount(ctx, admin.Account.ID), service.DeleteAccount(ctx, admin.Account.ID)} { + if !errors.Is(err, identityservice.ErrForbidden) { + t.Fatalf("admin changed: %v", err) + } + } + if err := service.DisableAccount(ctx, "missing"); !errors.Is(err, identityservice.ErrNotFound) { + t.Fatalf("missing account: %v", err) + } + if err := service.DisableAccount(ctx, account.ID); err != nil { + t.Fatal(err) + } + if _, err := service.AuthenticateSession(ctx, session.Secret); !errors.Is(err, identityservice.ErrAuthentication) { + t.Fatalf("disabled session accepted: %v", err) + } + if _, err := service.AuthenticateToken(ctx, token.Secret); !errors.Is(err, identityservice.ErrAuthentication) { + t.Fatalf("disabled token accepted: %v", err) + } + if err := store.CreateSession(ctx, account.ID, "", "new-session", "csrf", time.Now().Add(time.Hour)); !errors.Is(err, identityservice.ErrAuthentication) { + t.Fatalf("disabled account signed in: %v", err) + } + accounts, err := service.Accounts(ctx) + if err != nil || len(accounts) != 1 || accounts[0] != admin.Account { + t.Fatalf("disabled account served: %+v %v", accounts, err) + } + summaries, err := service.AccountSummaries(ctx) + if err != nil || len(summaries) != 2 || !summaries[0].Admin || !summaries[1].Disabled || summaries[1].CreatedAt.IsZero() { + t.Fatalf("summaries: %+v %v", summaries, err) + } + if err := service.EnableAccount(ctx, account.ID); err != nil { + t.Fatal(err) + } + if tokens, err := service.Tokens(ctx, account.ID); err != nil || len(tokens) != 0 { + t.Fatalf("enable restored credentials: %+v %v", tokens, err) + } + if err := store.CreateSession(ctx, account.ID, "", "new-session", "csrf", time.Now().Add(time.Hour)); err != nil { + t.Fatalf("enabled account cannot sign in: %v", err) + } + if _, err := store.Ingest(account.ID, observationRequest("source", "published")); err != nil { + t.Fatal(err) + } + if _, err := store.Queue().Accept(ctx, account.ID, queuedRequest("queued")); err != nil { + t.Fatal(err) + } + if err := service.DeleteAccount(ctx, account.ID); err != nil { + t.Fatal(err) + } + for _, table := range []string{"users", "remote_accounts", "browser_sessions", "observations", "ingestion_records", "ingestion_jobs", "diffs"} { + var count int + if err := store.db.QueryRow(`SELECT COUNT(*) FROM ` + table).Scan(&count); err != nil { + t.Fatal(err) + } + want := 0 + if table == "users" || table == "remote_accounts" || table == "browser_sessions" { + want = 1 + } + if count != want { + t.Fatalf("%s rows after delete: %d", table, count) + } + } + if err := service.DeleteAccount(ctx, account.ID); !errors.Is(err, identityservice.ErrNotFound) { + t.Fatalf("delete twice: %v", err) + } +} + +func TestSessionTokenIsolationExpiryAndRevocation(t *testing.T) { + store, service := testIdentity(t) + ctx := context.Background() + first, session := member(t, store, "first") + second, _ := member(t, store, "second") authenticated, err := service.AuthenticateSession(ctx, session.Secret) if err != nil || authenticated.Account != first || authenticated.Secret != "" || authenticated.CSRFToken != session.CSRFToken { t.Fatalf("session: %+v %v", authenticated, err) @@ -193,14 +308,8 @@ func TestSessionTokenIsolationExpiryAndRevocation(t *testing.T) { func TestPasswordResetFencesConcurrentCredentialOperations(t *testing.T) { store, service := testIdentity(t) ctx := context.Background() - account, err := service.Provision(ctx, "owner", testPassword) - if err != nil { - t.Fatal(err) - } - session, err := service.Login(ctx, "owner", testPassword) - if err != nil { - t.Fatal(err) - } + session := setupAdmin(t, service, "owner") + account := session.Account stale, err := store.AccountPassword(ctx, "owner") if err != nil { t.Fatal(err) @@ -222,13 +331,15 @@ func TestPasswordResetFencesConcurrentCredentialOperations(t *testing.T) { } } -func TestConcurrentAccountProvisionHasOneOwner(t *testing.T) { - store, service := testIdentity(t) +func TestConcurrentAccountCreationHasOneOwner(t *testing.T) { + store, _ := testIdentity(t) ctx := context.Background() var workers sync.WaitGroup results := make(chan error, 6) - for range 6 { - workers.Go(func() { _, err := service.Provision(ctx, "Owner", testPassword); results <- err }) + for index := range 6 { + workers.Go(func() { + results <- store.CreateAccount(ctx, identityservice.Account{ID: fmt.Sprintf("id-%d", index), Username: "owner"}, "", time.Now()) + }) } workers.Wait() close(results) @@ -249,49 +360,41 @@ func TestConcurrentAccountProvisionHasOneOwner(t *testing.T) { } } -func TestConcurrentBootstrapCreatesOneAccount(t *testing.T) { +func TestConcurrentSetupCreatesOneAdmin(t *testing.T) { _, service := testIdentity(t) ctx := context.Background() + secret, err := service.PrepareSetup(ctx) + if err != nil { + t.Fatal(err) + } var workers sync.WaitGroup - accounts := make(chan identityservice.Account, 4) failures := make(chan error, 4) for _, username := range []string{"first", "second", "third", "fourth"} { workers.Go(func() { - account, err := service.Bootstrap(ctx, username, testPassword) - accounts <- account + _, err := service.CompleteSetup(ctx, secret, username, testPassword) failures <- err }) } workers.Wait() - close(accounts) close(failures) + var success int for err := range failures { - if err != nil { + if err == nil { + success++ + } else if !errors.Is(err, identityservice.ErrNotFound) { t.Fatal(err) } } - var owner string - for account := range accounts { - if owner == "" { - owner = account.ID - } - if account.ID != owner { - t.Fatal("concurrent bootstrap produced distinct owners") - } - } listed, err := service.Accounts(ctx) - if err != nil || len(listed) != 1 { - t.Fatalf("bootstrap accounts: %+v %v", listed, err) + if success != 1 || err != nil || len(listed) != 1 || !listed[0].Admin { + t.Fatalf("setup accounts: %d %+v %v", success, listed, err) } } func TestConcurrentPasswordReplacementOnlyOneWins(t *testing.T) { store, service := testIdentity(t) ctx := context.Background() - account, err := service.Provision(ctx, "owner", testPassword) - if err != nil { - t.Fatal(err) - } + account := setupAdmin(t, service, "owner").Account credential, err := store.AccountPassword(ctx, "owner") if err != nil { t.Fatal(err) diff --git a/internal/reviewstore/reset_test.go b/internal/reviewstore/reset_test.go index 453355f4..e829e8a8 100644 --- a/internal/reviewstore/reset_test.go +++ b/internal/reviewstore/reset_test.go @@ -27,7 +27,7 @@ func TestOlderSchemaRefusedWithoutReset(t *testing.T) { store.Close() t.Fatal("accepted older schema") } else { - for _, text := range []string{path, "schema 10 is older than supported schema 12", "supporting schema 10", "automatic reset is disabled", "back up", "SQLite sidecar files", "--state /path/to/new-state.db", "no saved reviews or server accounts"} { + for _, text := range []string{path, "schema 10 is older than supported schema 13", "supporting schema 10", "automatic reset is disabled", "back up", "SQLite sidecar files", "--state /path/to/new-state.db", "no saved reviews or server accounts"} { if !strings.Contains(err.Error(), text) { t.Fatalf("missing recovery guidance %q: %v", text, err) } @@ -50,35 +50,47 @@ func TestOlderSchemaRefusedWithoutReset(t *testing.T) { } func TestAdditiveSchemaUpgradesInPlace(t *testing.T) { - path := filepath.Join(t.TempDir(), "state.db") - store, err := Open(path) - if err != nil { - t.Fatal(err) - } - user, err := store.User("owner", "owner") - if err != nil { - t.Fatal(err) - } - // Schema 11 had no GitHub integration tables. - if _, err := store.db.Exec("DROP TABLE github_follows; DROP TABLE github_connections; DROP TABLE github_oauth_states; DROP TABLE github_deliveries; DROP TABLE github_pull_requests; PRAGMA user_version=11"); err != nil { - t.Fatal(err) - } - store.Close() - store, err = Open(path) - if err != nil { - t.Fatalf("schema 11 must upgrade in place: %v", err) - } - defer store.Close() - if _, err := store.GitHub().Follows(t.Context(), user.ID); err != nil { - t.Fatalf("upgrade did not create integration tables: %v", err) - } - var version int - if err := store.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != schemaVersion { - t.Fatalf("upgraded version %d %v", version, err) - } - again, err := store.User("owner", "owner") - if err != nil || again.ID != user.ID { - t.Fatalf("upgrade lost state: %+v %v", again, err) + // Schema 11 had no GitHub integration tables; neither it nor schema 12 had + // administration or Google sign-in tables. + administration := "DROP TABLE google_auth_states; DROP TABLE account_invites; DROP TABLE google_identities; DROP TABLE disabled_accounts; DROP TABLE admin_setup; DROP TABLE admin_account; " + for version, statements := range map[int]string{ + 11: "DROP TABLE github_follows; DROP TABLE github_connections; DROP TABLE github_oauth_states; DROP TABLE github_deliveries; DROP TABLE github_pull_requests; " + administration, + 12: administration, + } { + t.Run(fmt.Sprintf("schema %d", version), func(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.db") + store, err := Open(path) + if err != nil { + t.Fatal(err) + } + user, err := store.User("owner", "owner") + if err != nil { + t.Fatal(err) + } + if _, err := store.db.Exec(fmt.Sprintf("%sPRAGMA user_version=%d", statements, version)); err != nil { + t.Fatal(err) + } + store.Close() + store, err = Open(path) + if err != nil { + t.Fatalf("schema %d must upgrade in place: %v", version, err) + } + defer store.Close() + if _, err := store.GitHub().Follows(t.Context(), user.ID); err != nil { + t.Fatalf("upgrade did not create integration tables: %v", err) + } + if exists, err := store.AdminExists(t.Context()); err != nil || exists { + t.Fatalf("upgrade did not create administration tables: %v %v", exists, err) + } + var upgraded int + if err := store.db.QueryRow("PRAGMA user_version").Scan(&upgraded); err != nil || upgraded != schemaVersion { + t.Fatalf("upgraded version %d %v", upgraded, err) + } + again, err := store.User("owner", "owner") + if err != nil || again.ID != user.ID { + t.Fatalf("upgrade lost state: %+v %v", again, err) + } + }) } } @@ -88,7 +100,7 @@ func TestFailedUpgradeRollsBack(t *testing.T) { if err != nil { t.Fatal(err) } - // A conflicting object makes the schema 12 statements fail midway. + // A conflicting object makes the schema 13 statements fail midway. if _, err := store.db.Exec("DROP TABLE github_follows; DROP TABLE github_oauth_states; CREATE VIEW github_follows AS SELECT 1 AS owner_id; PRAGMA user_version=11"); err != nil { t.Fatal(err) } diff --git a/internal/reviewstore/store.go b/internal/reviewstore/store.go index 5747510d..c1431b59 100644 --- a/internal/reviewstore/store.go +++ b/internal/reviewstore/store.go @@ -17,7 +17,7 @@ import ( _ "modernc.org/sqlite" ) -const schemaVersion = 12 +const schemaVersion = 13 // additiveSchema is the oldest schema that opens by upgrading in place. Each // later schema only creates tables and indexes, so the idempotent initialization diff --git a/internal/serverapp/bootstrap.go b/internal/serverapp/bootstrap.go index 534bfefb..5649f7b9 100644 --- a/internal/serverapp/bootstrap.go +++ b/internal/serverapp/bootstrap.go @@ -16,7 +16,7 @@ func localBootstrap(capabilities Capabilities) http.HandlerFunc { w.Header().Set("Content-Type", "application/json; charset=utf-8") _ = json.NewEncoder(w).Encode(struct { Mode string `json:"mode"` - SignupEnabled bool `json:"signupEnabled"` + GoogleEnabled bool `json:"googleEnabled"` User *string `json:"user"` CSRFToken *string `json:"csrfToken"` Collect bool `json:"collect"` diff --git a/internal/serverapp/handler_test.go b/internal/serverapp/handler_test.go index 165e3f0d..90164228 100644 --- a/internal/serverapp/handler_test.go +++ b/internal/serverapp/handler_test.go @@ -61,10 +61,7 @@ func start(t *testing.T, state string, remote bool) deployment { } var account identityservice.Account if len(accounts) == 0 { - account, err = identity.Provision(t.Context(), "team", "test-password-at-least-twelve-characters") - if err != nil { - t.Fatal(err) - } + account = testsupport.RemoteAccount(t, store, "team") } else { for _, existing := range accounts { if existing.Username == "team" { diff --git a/internal/serverapp/identity_test.go b/internal/serverapp/identity_test.go index 41503c22..824a9d94 100644 --- a/internal/serverapp/identity_test.go +++ b/internal/serverapp/identity_test.go @@ -14,7 +14,7 @@ import ( ) func TestLocalIdentityIsAutomaticWithoutAccountManagement(t *testing.T) { - t.Setenv("DIFFX_SERVER_SIGNUP_ENABLED", "true") + t.Setenv("DIFFX_SERVER_PUBLIC_URL", "https://diffx.example.com") store, err := reviewstore.Open(":memory:") if err != nil { t.Fatal(err) @@ -33,7 +33,7 @@ func TestLocalIdentityIsAutomaticWithoutAccountManagement(t *testing.T) { handler.ServeHTTP(response, request) var bootstrap struct { Mode string `json:"mode"` - SignupEnabled bool `json:"signupEnabled"` + GoogleEnabled bool `json:"googleEnabled"` User json.RawMessage `json:"user"` CSRFToken json.RawMessage `json:"csrfToken"` Collect json.RawMessage `json:"collect"` @@ -42,12 +42,12 @@ func TestLocalIdentityIsAutomaticWithoutAccountManagement(t *testing.T) { if err := json.Unmarshal(response.Body.Bytes(), &bootstrap); err != nil { t.Fatal(err) } - if response.Code != http.StatusOK || bootstrap.Mode != "local" || bootstrap.SignupEnabled || string(bootstrap.User) != "null" || string(bootstrap.CSRFToken) != "null" || string(bootstrap.Collect) != "false" || bootstrap.Input != "capture" { + if response.Code != http.StatusOK || bootstrap.Mode != "local" || bootstrap.GoogleEnabled || string(bootstrap.User) != "null" || string(bootstrap.CSRFToken) != "null" || string(bootstrap.Collect) != "false" || bootstrap.Input != "capture" { t.Fatalf("local composition exposed remote identity: %d %s", response.Code, response.Body.String()) } for method, paths := range map[string][]string{ - http.MethodPost: {"/api/v2/auth/signup", "/api/v2/auth/login", "/api/v2/auth/logout", "/api/v2/account/tokens"}, - http.MethodGet: {"/api/v2/account/tokens"}, + http.MethodPost: {"/api/v2/setup", "/api/v2/auth/login", "/api/v2/auth/logout", "/api/v2/account/tokens"}, + http.MethodGet: {"/api/v2/account/tokens", "/api/v2/setup", "/api/v2/admin/users"}, http.MethodPut: {"/api/v2/account/password"}, } { for _, path := range paths { diff --git a/internal/testsupport/identity.go b/internal/testsupport/identity.go new file mode 100644 index 00000000..fec5d4a7 --- /dev/null +++ b/internal/testsupport/identity.go @@ -0,0 +1,58 @@ +package testsupport + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "testing" + "time" + + "github.com/flexdinesh/diffx/internal/identityservice" + "github.com/flexdinesh/diffx/internal/reviewstore" +) + +func secret(t testing.TB) string { + t.Helper() + bytes := make([]byte, 32) + if _, err := rand.Read(bytes); err != nil { + t.Fatal(err) + } + return base64.RawURLEncoding.EncodeToString(bytes) +} + +// RemoteAdmin completes one-time setup, as the logged setup link does. +func RemoteAdmin(t testing.TB, store *reviewstore.Store, username, password string) identityservice.Session { + t.Helper() + identity := identityservice.New(store) + setup, err := identity.PrepareSetup(t.Context()) + if err != nil || setup == "" { + t.Fatalf("prepare setup: %q %v", setup, err) + } + session, err := identity.CompleteSetup(t.Context(), setup, username, password) + if err != nil { + t.Fatal(err) + } + return session +} + +// RemoteAccount creates a passwordless member account, as an accepted invite does. +func RemoteAccount(t testing.TB, store *reviewstore.Store, username string) identityservice.Account { + t.Helper() + account := identityservice.Account{ID: secret(t), Username: username} + if err := store.CreateAccount(t.Context(), account, "", time.Now()); err != nil { + t.Fatal(err) + } + return account +} + +// RemoteSession signs a member in without Google, as a completed sign-in does. +func RemoteSession(t testing.TB, store *reviewstore.Store, account identityservice.Account) identityservice.Session { + t.Helper() + session := identityservice.Session{Account: account, Secret: secret(t), CSRFToken: secret(t), ExpiresAt: time.Now().Add(identityservice.SessionLifetime)} + hash := sha256.Sum256([]byte(session.Secret)) + if err := store.CreateSession(t.Context(), account.ID, "", hex.EncodeToString(hash[:]), session.CSRFToken, session.ExpiresAt); err != nil { + t.Fatal(err) + } + return session +} diff --git a/packages/api/openapi.yaml b/packages/api/openapi.yaml index fb99c416..2ad1625d 100644 --- a/packages/api/openapi.yaml +++ b/packages/api/openapi.yaml @@ -22,7 +22,7 @@ paths: /api/v2/auth/login: post: operationId: login - description: Distributed only. Same-origin JSON request; creates a browser session. Signup requires explicit server configuration. + description: Distributed only. Admin password sign-in; same-origin JSON request creating a browser session. Every other account signs in with Google, and any refusal is the same 401. requestBody: required: true content: @@ -42,35 +42,59 @@ paths: $ref: "#/components/responses/Problem" "403": $ref: "#/components/responses/Problem" - "409": - $ref: "#/components/responses/Problem" "429": $ref: "#/components/responses/Problem" default: $ref: "#/components/responses/Problem" - /api/v2/auth/signup: + /api/v2/setup: + get: + operationId: getSetup + description: Distributed only. Whether the server still needs its admin. Never exposes the setup secret. + responses: + "200": + description: Setup status. + content: + application/json: + schema: + type: object + additionalProperties: false + required: [required] + properties: + required: + type: boolean + default: + $ref: "#/components/responses/Problem" post: - operationId: signup - description: Distributed only. Same-origin JSON request; creates a browser session. Signup requires explicit server configuration. + operationId: completeSetup + description: Distributed only. Create the admin from the one-time secret the server logged at startup (the setup link's `#t=` fragment) and sign it in. The secret expires after 24 hours and is replaced on each start until used. requestBody: required: true content: application/json: schema: - $ref: "#/components/schemas/PasswordCredentials" + type: object + additionalProperties: false + required: [token, username, password] + properties: + token: + type: string + username: + type: string + password: + type: string responses: "200": - description: Authenticated browser identity. + description: Admin browser identity. content: application/json: schema: $ref: "#/components/schemas/Bootstrap" "400": $ref: "#/components/responses/Problem" - "401": - $ref: "#/components/responses/Problem" "403": $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" "409": $ref: "#/components/responses/Problem" "429": @@ -238,6 +262,95 @@ paths: $ref: "#/components/responses/Problem" default: $ref: "#/components/responses/Problem" + /api/v2/admin/users: + get: + security: + - browserSession: [] + operationId: listAdminUsers + description: Every account, including disabled ones. Admin browser session only. + responses: + "200": + description: Accounts. + content: + application/json: + schema: + type: object + additionalProperties: false + required: [users] + properties: + users: + type: array + items: + $ref: "#/components/schemas/AdminUser" + "401": + $ref: "#/components/responses/Problem" + "403": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + /api/v2/admin/users/{userId}: + parameters: + - $ref: "#/components/parameters/UserId" + delete: + security: + - browserSession: [] + operationId: deleteAdminUser + description: Delete the account and everything it owns. The admin account cannot be deleted. Admin browser session only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "204": + description: Deleted. + "401": + $ref: "#/components/responses/Problem" + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + /api/v2/admin/users/{userId}/disable: + parameters: + - $ref: "#/components/parameters/UserId" + post: + security: + - browserSession: [] + operationId: disableAdminUser + description: Block sign-in and revoke every browser session and API token. The admin account cannot be disabled. Admin browser session only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "204": + description: Disabled. + "401": + $ref: "#/components/responses/Problem" + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" + /api/v2/admin/users/{userId}/enable: + parameters: + - $ref: "#/components/parameters/UserId" + post: + security: + - browserSession: [] + operationId: enableAdminUser + description: Allow sign-in again. Revoked sessions and tokens stay revoked. Admin browser session only. + parameters: + - $ref: "#/components/parameters/CsrfToken" + responses: + "204": + description: Enabled. + "401": + $ref: "#/components/responses/Problem" + "403": + $ref: "#/components/responses/Problem" + "404": + $ref: "#/components/responses/Problem" + default: + $ref: "#/components/responses/Problem" /api/v2/github/webhooks: post: operationId: receiveGitHubWebhook @@ -1076,6 +1189,12 @@ components: description: Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. schema: type: string + UserId: + name: userId + in: path + required: true + schema: + type: string ContextId: name: contextId in: path @@ -1135,22 +1254,44 @@ components: AccountUser: type: object additionalProperties: false - required: [id, username] + required: [id, username, admin] properties: id: type: string username: type: string + admin: + type: boolean + description: The single account that signs in with a password and manages other accounts. + AdminUser: + type: object + additionalProperties: false + required: [id, username, email, admin, disabled, createdAt] + 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 + disabled: + type: boolean + createdAt: + type: integer + format: int64 Bootstrap: oneOf: - type: object additionalProperties: false - required: [mode, signupEnabled, user, csrfToken, collect, input] + required: [mode, googleEnabled, user, csrfToken, collect, input] properties: mode: type: string enum: [local] - signupEnabled: + googleEnabled: type: boolean enum: [false] user: @@ -1166,13 +1307,14 @@ components: description: What this process serves. `checkouts` reviews discovered checkouts; `capture` reviews one fixed patch (piped stdin or a fixture) and has nothing to switch to. - type: object additionalProperties: false - required: [mode, signupEnabled, user, csrfToken] + required: [mode, googleEnabled, user, csrfToken] properties: mode: type: string enum: [remote] - signupEnabled: + googleEnabled: type: boolean + description: Whether members can sign in with Google. user: oneOf: - $ref: "#/components/schemas/AccountUser" diff --git a/packages/api/src/client.ts b/packages/api/src/client.ts index d1aa5137..9bd53b09 100644 --- a/packages/api/src/client.ts +++ b/packages/api/src/client.ts @@ -47,7 +47,7 @@ export function readBootstrap(value: unknown): ApiBootstrap { if (!object(value)) throw new Error("Invalid server identity response"); if ( value.mode === "local" && - value.signupEnabled === false && + value.googleEnabled === false && value.user === null && value.csrfToken === null && typeof value.collect === "boolean" && @@ -55,18 +55,18 @@ export function readBootstrap(value: unknown): ApiBootstrap { ) return { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: value.collect, input: value.input, }; - if (value.mode !== "remote" || typeof value.signupEnabled !== "boolean") + if (value.mode !== "remote" || typeof value.googleEnabled !== "boolean") throw new Error("Invalid server identity response"); if (value.user === null && value.csrfToken === null) return { mode: "remote", - signupEnabled: value.signupEnabled, + googleEnabled: value.googleEnabled, user: null, csrfToken: null, }; @@ -76,13 +76,18 @@ export function readBootstrap(value: unknown): ApiBootstrap { value.user.id.length > 0 && typeof value.user.username === "string" && value.user.username.length > 0 && + typeof value.user.admin === "boolean" && typeof value.csrfToken === "string" && value.csrfToken.length > 0 ) return { mode: "remote", - signupEnabled: value.signupEnabled, - user: { id: value.user.id, username: value.user.username }, + googleEnabled: value.googleEnabled, + user: { + id: value.user.id, + username: value.user.username, + admin: value.user.admin, + }, csrfToken: value.csrfToken, }; throw new Error("Invalid server identity response"); diff --git a/packages/api/src/schema.d.ts b/packages/api/src/schema.d.ts index f0b3ea37..c90d4f29 100644 --- a/packages/api/src/schema.d.ts +++ b/packages/api/src/schema.d.ts @@ -30,7 +30,7 @@ export interface paths { }; get?: never; put?: never; - /** @description Distributed only. Same-origin JSON request; creates a browser session. Signup requires explicit server configuration. */ + /** @description Distributed only. Admin password sign-in; same-origin JSON request creating a browser session. Every other account signs in with Google, and any refusal is the same 401. */ post: operations["login"]; delete?: never; options?: never; @@ -38,17 +38,18 @@ export interface paths { patch?: never; trace?: never; }; - "/api/v2/auth/signup": { + "/api/v2/setup": { parameters: { query?: never; header?: never; path?: never; cookie?: never; }; - get?: never; + /** @description Distributed only. Whether the server still needs its admin. Never exposes the setup secret. */ + get: operations["getSetup"]; put?: never; - /** @description Distributed only. Same-origin JSON request; creates a browser session. Signup requires explicit server configuration. */ - post: operations["signup"]; + /** @description Distributed only. Create the admin from the one-time secret the server logged at startup (the setup link's `#t=` fragment) and sign it in. The secret expires after 24 hours and is replaced on each start until used. */ + post: operations["completeSetup"]; delete?: never; options?: never; head?: never; @@ -124,6 +125,80 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v2/admin/users": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** @description Every account, including disabled ones. Admin browser session only. */ + get: operations["listAdminUsers"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/admin/users/{userId}": { + parameters: { + query?: never; + header?: never; + path: { + userId: components["parameters"]["UserId"]; + }; + cookie?: never; + }; + get?: never; + put?: never; + post?: never; + /** @description Delete the account and everything it owns. The admin account cannot be deleted. Admin browser session only. */ + delete: operations["deleteAdminUser"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/admin/users/{userId}/disable": { + parameters: { + query?: never; + header?: never; + path: { + userId: components["parameters"]["UserId"]; + }; + cookie?: never; + }; + get?: never; + put?: never; + /** @description Block sign-in and revoke every browser session and API token. The admin account cannot be disabled. Admin browser session only. */ + post: operations["disableAdminUser"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v2/admin/users/{userId}/enable": { + parameters: { + query?: never; + header?: never; + path: { + userId: components["parameters"]["UserId"]; + }; + cookie?: never; + }; + get?: never; + put?: never; + /** @description Allow sign-in again. Revoked sessions and tokens stay revoked. Admin browser session only. */ + post: operations["enableAdminUser"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v2/github/webhooks": { parameters: { query?: never; @@ -627,13 +702,25 @@ export interface components { AccountUser: { id: string; username: string; + /** @description The single account that signs in with a password and manages other accounts. */ + admin: boolean; + }; + AdminUser: { + id: string; + username: string; + /** @description The latest email Google reported for the account; null for the admin. */ + email: string | null; + admin: boolean; + disabled: boolean; + /** Format: int64 */ + createdAt: number; }; Bootstrap: | { /** @enum {string} */ mode: "local"; /** @enum {boolean} */ - signupEnabled: false; + googleEnabled: false; user: null; csrfToken: null; /** @description Whether this process can collect its selected checkouts again through POST /api/v2/collections. */ @@ -647,7 +734,8 @@ export interface components { | { /** @enum {string} */ mode: "remote"; - signupEnabled: boolean; + /** @description Whether members can sign in with Google. */ + googleEnabled: boolean; user: components["schemas"]["AccountUser"] | null; csrfToken: string | null; }; @@ -1130,6 +1218,7 @@ export interface components { parameters: { /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ CsrfToken: string; + UserId: string; ContextId: string; Scope: components["schemas"]["DiffMode"]; DiffId: string; @@ -1191,12 +1280,34 @@ export interface operations { 400: components["responses"]["Problem"]; 401: components["responses"]["Problem"]; 403: components["responses"]["Problem"]; - 409: components["responses"]["Problem"]; 429: components["responses"]["Problem"]; default: components["responses"]["Problem"]; }; }; - signup: { + getSetup: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Setup status. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + required: boolean; + }; + }; + }; + default: components["responses"]["Problem"]; + }; + }; + completeSetup: { parameters: { query?: never; header?: never; @@ -1205,11 +1316,15 @@ export interface operations { }; requestBody: { content: { - "application/json": components["schemas"]["PasswordCredentials"]; + "application/json": { + token: string; + username: string; + password: string; + }; }; }; responses: { - /** @description Authenticated browser identity. */ + /** @description Admin browser identity. */ 200: { headers: { [name: string]: unknown; @@ -1219,8 +1334,8 @@ export interface operations { }; }; 400: components["responses"]["Problem"]; - 401: components["responses"]["Problem"]; 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; 409: components["responses"]["Problem"]; 429: components["responses"]["Problem"]; default: components["responses"]["Problem"]; @@ -1381,6 +1496,112 @@ export interface operations { default: components["responses"]["Problem"]; }; }; + listAdminUsers: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Accounts. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + users: components["schemas"]["AdminUser"][]; + }; + }; + }; + 401: components["responses"]["Problem"]; + 403: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + deleteAdminUser: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path: { + userId: components["parameters"]["UserId"]; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Deleted. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 401: components["responses"]["Problem"]; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + disableAdminUser: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path: { + userId: components["parameters"]["UserId"]; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Disabled. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 401: components["responses"]["Problem"]; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; + enableAdminUser: { + parameters: { + query?: never; + header?: { + /** @description Required for unsafe requests authenticated through a distributed browser session; attached by browser identity transport. Not applicable to local or bearer credentials. */ + "X-CSRF-Token"?: components["parameters"]["CsrfToken"]; + }; + path: { + userId: components["parameters"]["UserId"]; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Enabled. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 401: components["responses"]["Problem"]; + 403: components["responses"]["Problem"]; + 404: components["responses"]["Problem"]; + default: components["responses"]["Problem"]; + }; + }; receiveGitHubWebhook: { parameters: { query?: never; diff --git a/test/conformance/server.test.ts b/test/conformance/server.test.ts index 3f079466..a5032065 100644 --- a/test/conformance/server.test.ts +++ b/test/conformance/server.test.ts @@ -158,7 +158,7 @@ test("Go distribution implements the API contract", async (t) => { conforms("Bootstrap", localBootstrap.data); assert.deepEqual(localBootstrap.data, { mode: "local", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, collect: false, @@ -320,17 +320,18 @@ test("Go distribution implements the API contract", async (t) => { remoteBinary, ["--listen", `127.0.0.1:${remotePort}`, "--state", "memory"], { - stdio: "ignore", + stdio: ["ignore", "ignore", "pipe"], env: { ...process.env, - DIFFX_SERVER_USERNAME: "contract-user", - DIFFX_SERVER_PASSWORD: "contract-password-000001", - DIFFX_SERVER_SIGNUP_ENABLED: "false", + DIFFX_SERVER_PUBLIC_URL: "http://diffx.test", DIFFX_SERVER_INSECURE_COOKIES: "true", }, }, ); t.after(() => remoteServer.kill("SIGTERM")); + let serverLog = ""; + remoteServer.stderr.setEncoding("utf8"); + remoteServer.stderr.on("data", (chunk: string) => (serverLog += chunk)); const remoteUrl = `http://127.0.0.1:${remotePort}`; await waitForServer(remoteUrl); const remote = createApiClient({ baseUrl: remoteUrl }); @@ -338,14 +339,46 @@ test("Go distribution implements the API contract", async (t) => { conforms("Bootstrap", remoteBootstrap.data); assert.deepEqual(remoteBootstrap.data, { mode: "remote", - signupEnabled: false, + googleEnabled: false, user: null, csrfToken: null, }); - const disabledSignup = await remote.POST("/api/v2/auth/signup", { - body: { username: "second-user", password: "contract-password-000002" }, + const retiredSignup = await fetch(`${remoteUrl}/api/v2/auth/signup`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + username: "second-user", + password: "contract-password-000002", + }), }); - assert.equal(disabledSignup.response.status, 403); + assert.equal(retiredSignup.status, 401); + const pendingSetup = await remote.GET("/api/v2/setup"); + assert.deepEqual(pendingSetup.data, { required: true }); + let setupToken = ""; + for (let attempt = 0; !setupToken && attempt < 40; attempt++) { + setupToken = + /http:\/\/diffx\.test\/setup#t=(\S+)/.exec(serverLog)?.[1] ?? ""; + if (!setupToken) await new Promise((resolve) => setTimeout(resolve, 50)); + } + assert.ok(setupToken, `setup link not logged: ${serverLog}`); + const setup = await remote.POST("/api/v2/setup", { + body: { + token: setupToken, + username: "contract-user", + password: "contract-password-000001", + }, + }); + assert.equal(setup.response.status, 200); + conforms("Bootstrap", setup.data); + assert.equal(setup.data?.mode === "remote" && setup.data.user?.admin, true); + const reusedSetup = await remote.POST("/api/v2/setup", { + body: { + token: setupToken, + username: "second-user", + password: "contract-password-000002", + }, + }); + assert.equal(reusedSetup.response.status, 404); const login = await remote.POST("/api/v2/auth/login", { body: { username: "contract-user", @@ -361,6 +394,12 @@ test("Go distribution implements the API contract", async (t) => { const cookie = login.response.headers.get("set-cookie")?.split(";")[0]; assert.ok(cookie); const accountHeaders = { Cookie: cookie, "X-CSRF-Token": csrf }; + const users = await remote.GET("/api/v2/admin/users", { + headers: { Cookie: cookie }, + }); + assert.equal(users.response.status, 200); + assert.equal(users.data?.users.length, 1); + conforms("AdminUser", users.data?.users[0]); const missingCSRF = await remote.POST("/api/v2/account/tokens", { headers: { Cookie: cookie }, body: { name: "Denied", purpose: "api", expiresInDays: 90 }, diff --git a/test/conformance/state_errors_test.go b/test/conformance/state_errors_test.go index 9253a7cf..98a0ce59 100644 --- a/test/conformance/state_errors_test.go +++ b/test/conformance/state_errors_test.go @@ -20,7 +20,7 @@ func TestBothModesPreserveIncompatibleStateAndAcceptFreshPath(t *testing.T) { func(localserver.Status, *contextservice.Publication) { ready() }) }, "remote": func(ctx context.Context, path string, ready func()) error { - return remoteserver.Run(ctx, remoteserver.Settings{Listen: "127.0.0.1:0", State: path, SignupEnabled: true}, + return remoteserver.Run(ctx, remoteserver.Settings{Listen: "127.0.0.1:0", State: path}, func(string) { ready() }) }, } { diff --git a/tools/check-boundaries/main.go b/tools/check-boundaries/main.go index 6b3a38cb..92caf6f2 100644 --- a/tools/check-boundaries/main.go +++ b/tools/check-boundaries/main.go @@ -67,7 +67,7 @@ var allowed = map[string][]string{ "internal/submission": {"internal/ingestion", "internal/processlock"}, // Test helpers are classified, but production packages cannot import them. "internal/testsupport": { - "internal/contextservice", "internal/ingestion", "internal/ingestionqueue", "internal/review", + "internal/contextservice", "internal/identityservice", "internal/ingestion", "internal/ingestionqueue", "internal/review", "internal/reviewstore", "internal/serverapp", "internal/webui", }, "internal/version": {},