fix(admin): accept same-origin first-run setup requests (#1280)

* fix(admin): accept same-origin first-run setup requests

A freshly generated config.yaml leaves allowed_origins commented out, so the
list is empty. The setup handler's CSRF guard assumed "no Origin header means
same-origin", but browsers send Origin on same-origin POSTs too — Chrome and
Edge always, Firefox since 70. The admin panel's own setup call is one of those
POSTs, so every new install hit "cross-origin setup request blocked" and could
never create an owner account.

The guard now accepts a request whose Origin names the same host:port as the
request's own Host header, falling back to the allowlist otherwise. That is what
the original comment intended. CSRF protection is unaffected: a cross-site
attacker cannot set Origin, the browser does, and a foreign origin still needs
an explicit allowlist entry.

Scheme is not compared. Nothing in this server derives the external scheme (no
r.TLS or X-Forwarded-Proto handling exists anywhere), so a scheme check would
reject legitimate requests behind a TLS-terminating proxy.

Tests: isSameOrigin table covering port/host/suffix/schemeless/opaque-origin
cases, plus two handler-level tests pinning both halves — same-origin succeeds
against an empty allowlist, a foreign origin still 403s and creates no user.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(identity): implement identity keypair caching and error handling

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Client/tauri-client/src/lib/identity.ts

@@ -119,12 +119,53 @@ export async function getIdentityPin(host: string, userId: string): Promise<stri
 
 // ── High-level lifecycle ───────────────────────────────────────────────────
 
+/**
+ * One identity keypair per host, shared by every caller in this process.
+ *
+ * The keypair has two independent consumers: the ready hook publishes its
+ * public half (`ensureIdentityKeyPublished`) and the voice session signs
+ * announces with its private half (`livekitSession.ensureIdentityKeyPair`).
+ * They must be the SAME pair — peers verify the announce signature against the
+ * published key. Without this memo each consumer calls the loader separately,
+ * so on a machine where the OS credential store does not round-trip (the write
+ * reports success, the next read returns nothing) each call mints a fresh
+ * keypair: the published key is then never the key that signed, every peer
+ * rejects the announce as a forged/MITM signature, and no amount of re-pinning
+ * helps because the pin records the published key, not the signer.
+ *
+ * The promise is cached (not the resolved value) so concurrent first callers
+ * share one generation instead of racing to create two.
+ */
+const identityKeyPairCache = new Map<string, Promise<CryptoKeyPair>>();
+
 /**
  * Load this host's identity keypair from the keyring, generating and saving a
  * fresh one on first login (or when the stored blob is corrupt). In non-Tauri
  * environments the keypair is in-memory only (not persisted).
+ *
+ * Stable for the lifetime of the process: repeat callers get the same keypair
+ * even when the keyring is unavailable (see `identityKeyPairCache`).
  */
-export async function getOrCreateIdentityKeyPair(host: string): Promise<CryptoKeyPair> {
+export function getOrCreateIdentityKeyPair(host: string): Promise<CryptoKeyPair> {
+  let pending = identityKeyPairCache.get(host);
+  if (pending === undefined) {
+    // A rejected load must not be cached, or the host is poisoned for the
+    // rest of the session; drop it so the next caller can retry.
+    pending = loadOrGenerateIdentityKeyPair(host).catch((err: unknown) => {
+      identityKeyPairCache.delete(host);
+      throw err;
+    });
+    identityKeyPairCache.set(host, pending);
+  }
+  return pending;
+}
+
+/** Test-only: drop the per-host keypair memo so each case starts clean. */
+export function resetIdentityKeyPairCache(): void {
+  identityKeyPairCache.clear();
+}
+
+async function loadOrGenerateIdentityKeyPair(host: string): Promise<CryptoKeyPair> {
   const stored = await loadIdentityKey(host);
   if (stored) {
     try {
@@ -134,7 +175,20 @@ export async function getOrCreateIdentityKeyPair(host: string): Promise<CryptoKe
     }
   }
   const keyPair = await generateIdentityKeyPair();
-  await saveIdentityKey(host, await exportIdentityKeyPair(keyPair.privateKey));
+  const blob = await exportIdentityKeyPair(keyPair.privateKey);
+  if (await saveIdentityKey(host, blob)) {
+    // The store reported success — verify it actually kept the value. Windows
+    // can accept a CredWrite and persist nothing (Credential Manager disabled,
+    // or the "do not allow storage of passwords and credentials" policy), which
+    // otherwise surfaces to the user only as peers flagging them as a MITM.
+    if ((await loadIdentityKey(host)) !== blob) {
+      log.error(
+        "Identity key did not persist — the credential store accepted the write but did not return it. " +
+          "This session works, but peers will see a new identity (and prompt to re-verify) every restart.",
+        { host },
+      );
+    }
+  }
   return keyPair;
 }
 

Client/tauri-client/tests/unit/identity.test.ts

@@ -1,11 +1,12 @@
 import { describe, it, expect, vi, beforeEach } from "vitest";
 
-const { invokeMock } = vi.hoisted(() => ({ invokeMock: vi.fn() }));
+const { invokeMock, logMock } = vi.hoisted(() => ({
+  invokeMock: vi.fn(),
+  logMock: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
+}));
 
 vi.mock("@tauri-apps/api/core", () => ({ invoke: invokeMock }));
-vi.mock("@lib/logger", () => ({
-  createLogger: () => ({ debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }),
-}));
+vi.mock("@lib/logger", () => ({ createLogger: () => logMock }));
 
 import {
   saveIdentityKey,
@@ -14,6 +15,7 @@ import {
   storeIdentityPin,
   getIdentityPin,
   getOrCreateIdentityKeyPair,
+  resetIdentityKeyPairCache,
   publishIdentityKey,
   ensureIdentityKeyPublished,
 } from "@lib/identity";
@@ -21,6 +23,11 @@ import { generateIdentityKeyPair, exportPublicKey } from "@lib/e2eeCrypto";
 
 beforeEach(() => {
   invokeMock.mockReset();
+  logMock.error.mockReset();
+  logMock.warn.mockReset();
+  // The keypair memo is process-wide by design; without this, one case's
+  // cached pair would satisfy the next case's keyring assertions.
+  resetIdentityKeyPairCache();
 });
 
 describe("identity keyring wrappers", () => {
@@ -110,6 +117,9 @@ describe("getOrCreateIdentityKeyPair", () => {
     const firstPub = await exportPublicKey(first.publicKey);
 
     // Second login: keyring returns the saved blob → same public key, no save.
+    // Drop the memo first, or this asserts nothing about the keyring — a new
+    // login is a new process, which is exactly what the reload path is for.
+    resetIdentityKeyPairCache();
     invokeMock.mockReset();
     invokeMock.mockImplementation((cmd: string) => {
       if (cmd === "load_identity_key") return Promise.resolve(savedBlob);
@@ -129,6 +139,68 @@ describe("getOrCreateIdentityKeyPair", () => {
     expect(kp.publicKey).toBeDefined();
     expect(invokeMock.mock.calls.some((c) => c[0] === "save_identity_key")).toBe(true);
   });
+
+  it("hands every caller the same keypair when the keyring never persists", async () => {
+    // A store that accepts the write and returns nothing on the next read.
+    // Before the memo, the ready hook (publishes the public half) and the voice
+    // session (signs announces with the private half) each generated their own
+    // keypair here — so the published key was never the key that signed, and
+    // peers rejected every announce as a forged signature.
+    invokeMock.mockImplementation((cmd: string) => {
+      if (cmd === "load_identity_key") return Promise.resolve(null);
+      return Promise.resolve(undefined);
+    });
+
+    const [publishPair, signingPair] = await Promise.all([
+      getOrCreateIdentityKeyPair("chat.example"),
+      getOrCreateIdentityKeyPair("chat.example"),
+    ]);
+    const laterPair = await getOrCreateIdentityKeyPair("chat.example");
+
+    expect(signingPair).toBe(publishPair);
+    expect(laterPair).toBe(publishPair);
+    // One generation, not one per caller.
+    expect(invokeMock.mock.calls.filter((c) => c[0] === "save_identity_key")).toHaveLength(1);
+  });
+
+  it("keeps the memo per host", async () => {
+    invokeMock.mockImplementation((cmd: string) => {
+      if (cmd === "load_identity_key") return Promise.resolve(null);
+      return Promise.resolve(undefined);
+    });
+    const a = await getOrCreateIdentityKeyPair("chat.example");
+    const b = await getOrCreateIdentityKeyPair("other.example");
+    expect(await exportPublicKey(b.publicKey)).not.toBe(await exportPublicKey(a.publicKey));
+  });
+
+  it("reports a credential store that accepts the write but drops the value", async () => {
+    invokeMock.mockImplementation((cmd: string) => {
+      if (cmd === "load_identity_key") return Promise.resolve(null);
+      return Promise.resolve(undefined); // save_identity_key "succeeds"
+    });
+
+    await getOrCreateIdentityKeyPair("chat.example");
+
+    expect(logMock.error).toHaveBeenCalledWith(expect.stringContaining("did not persist"), {
+      host: "chat.example",
+    });
+  });
+
+  it("stays quiet when the store round-trips the key", async () => {
+    let savedBlob: string | undefined;
+    invokeMock.mockImplementation((cmd: string, args?: Record<string, unknown>) => {
+      if (cmd === "load_identity_key") return Promise.resolve(savedBlob ?? null);
+      if (cmd === "save_identity_key") {
+        savedBlob = args!.key as string;
+        return Promise.resolve(undefined);
+      }
+      return Promise.resolve(undefined);
+    });
+
+    await getOrCreateIdentityKeyPair("chat.example");
+
+    expect(logMock.error).not.toHaveBeenCalled();
+  });
 });
 
 describe("publishIdentityKey", () => {

Server/admin/setup_handler.go

@@ -7,6 +7,7 @@ import (
 	"log/slog"
 	"net"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
@@ -57,10 +58,10 @@ func handleSetupStatus(database *db.DB) http.HandlerFunc {
 func handleSetup(database *db.DB, limiter *auth.RateLimiter, allowedOrigins []string) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		// CSRF protection: reject cross-origin requests (BUG-097).
-		// If Origin is present and doesn't match allowed origins, deny.
-		// No Origin header = same-origin or non-browser client (allow).
+		// A request is accepted when it is same-origin, or when its Origin is
+		// explicitly allowlisted. Absent Origin = non-browser client (allow).
 		if origin := r.Header.Get("Origin"); origin != "" {
-			if !isSetupOriginAllowed(origin, allowedOrigins) {
+			if !isSameOrigin(origin, r.Host) && !isSetupOriginAllowed(origin, allowedOrigins) {
 				writeErr(w, http.StatusForbidden, "FORBIDDEN", "cross-origin setup request blocked")
 				return
 			}
@@ -165,6 +166,34 @@ func handleSetup(database *db.DB, limiter *auth.RateLimiter, allowedOrigins []st
 	}
 }
 
+// isSameOrigin reports whether a browser-supplied Origin names this same
+// server, by comparing its host:port against the request's Host header.
+//
+// Browsers send Origin on same-origin POSTs too (Chrome and Edge always,
+// Firefox since 70), so the admin panel's own first-run setup call arrives
+// carrying one. Without this check it is measured against allowed_origins,
+// which is empty in a freshly generated config — so setup failed with
+// "cross-origin setup request blocked" on every new install.
+//
+// Scheme is deliberately not compared. Nothing in this server derives the
+// external scheme (there is no r.TLS or X-Forwarded-Proto handling anywhere),
+// so a TLS-terminating proxy in front would make a scheme check reject
+// legitimate requests. Matching host:port is enough: forging it requires
+// already serving content on this exact host and port, at which point the
+// origin is not the attacker's to borrow. Cross-site attackers cannot set
+// Origin at all — the browser does.
+func isSameOrigin(origin, host string) bool {
+	if host == "" {
+		return false
+	}
+	u, err := url.Parse(origin)
+	// Require a scheme so a schemeless "//host:port" cannot pass as same-origin.
+	if err != nil || u.Scheme == "" || u.Host == "" {
+		return false
+	}
+	return strings.EqualFold(u.Host, host)
+}
+
 // isSetupOriginAllowed checks if the given origin is permitted by the
 // configured allowed_origins list. Wildcard "*" allows any origin.
 // An empty list denies all cross-origin requests (safe default).

Server/admin/setup_handler_test.go

@@ -1,10 +1,12 @@
 package admin_test
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/owncord/server/admin"
@@ -194,3 +196,58 @@ func TestSetup_ConcurrentRace(t *testing.T) {
 		t.Errorf("user count = %d, want 1", count)
 	}
 }
+
+// A freshly generated config leaves allowed_origins empty, and browsers send an
+// Origin header on same-origin POSTs. Before isSameOrigin existed, that pairing
+// made first-run setup fail on every new install with "cross-origin setup
+// request blocked". These two tests pin both halves: same-origin gets through
+// on an empty allowlist, and a foreign origin still does not.
+func TestSetup_SameOriginAllowedWithEmptyAllowlist(t *testing.T) {
+	database := openAdminTestDB(t)
+	handler := admin.NewAdminAPI(database, "1.0.0", nil, nil, nil, nil, nil, newTestModService(database))
+
+	body, err := json.Marshal(map[string]string{"username": "owner", "password": "correct-horse"})
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	req := httptest.NewRequest("POST", "/setup", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	// httptest.NewRequest sets Host to example.com; the browser would send the
+	// matching Origin for a page served from this same server.
+	req.Header.Set("Origin", "https://"+req.Host)
+
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusCreated {
+		t.Fatalf("POST /setup with same-origin Origin = %d, want 201; body: %s", rr.Code, rr.Body.String())
+	}
+}
+
+func TestSetup_ForeignOriginStillBlocked(t *testing.T) {
+	database := openAdminTestDB(t)
+	handler := admin.NewAdminAPI(database, "1.0.0", nil, nil, nil, nil, nil, newTestModService(database))
+
+	body, err := json.Marshal(map[string]string{"username": "owner", "password": "correct-horse"})
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	req := httptest.NewRequest("POST", "/setup", bytes.NewReader(body))
+	req.Header.Set("Content-Type", "application/json")
+	req.Header.Set("Origin", "https://evil.example")
+
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	if rr.Code != http.StatusForbidden {
+		t.Fatalf("POST /setup from a foreign origin = %d, want 403", rr.Code)
+	}
+
+	count, err := database.UserCount(context.Background())
+	if err != nil {
+		t.Fatalf("UserCount: %v", err)
+	}
+	if count != 0 {
+		t.Fatalf("owner account created from a foreign origin: user count = %d, want 0", count)
+	}
+}

Server/admin/setup_origin_test.go

@@ -90,3 +90,91 @@ func TestIsSetupOriginAllowed(t *testing.T) {
 		})
 	}
 }
+
+// isSameOrigin is what lets the admin panel's own setup call through on a
+// default config, where allowed_origins is empty. It must not become a hole:
+// only an Origin naming this exact host:port may pass.
+func TestIsSameOrigin(t *testing.T) {
+	tests := []struct {
+		name   string
+		origin string
+		host   string
+		want   bool
+	}{
+		{
+			name:   "admin panel on the default port is same-origin",
+			origin: "https://localhost:8443",
+			host:   "localhost:8443",
+			want:   true,
+		},
+		{
+			name:   "host comparison is case-insensitive",
+			origin: "https://LocalHost:8443",
+			host:   "localhost:8443",
+			want:   true,
+		},
+		{
+			name:   "plain http against a proxied host still matches",
+			origin: "http://chat.example",
+			host:   "chat.example",
+			want:   true,
+		},
+		{
+			name:   "a different host is not same-origin",
+			origin: "https://evil.example",
+			host:   "localhost:8443",
+			want:   false,
+		},
+		{
+			name:   "a different port is not same-origin",
+			origin: "https://localhost:9999",
+			host:   "localhost:8443",
+			want:   false,
+		},
+		{
+			name:   "loopback by IP does not match loopback by name",
+			origin: "https://127.0.0.1:8443",
+			host:   "localhost:8443",
+			want:   false,
+		},
+		{
+			name:   "a suffix of the host is not same-origin",
+			origin: "https://evil-localhost:8443",
+			host:   "localhost:8443",
+			want:   false,
+		},
+		{
+			name:   "schemeless origin cannot pass as same-origin",
+			origin: "//localhost:8443",
+			host:   "localhost:8443",
+			want:   false,
+		},
+		{
+			name:   "opaque origin (sandboxed iframe) is denied",
+			origin: "null",
+			host:   "localhost:8443",
+			want:   false,
+		},
+		{
+			name:   "empty origin is denied",
+			origin: "",
+			host:   "localhost:8443",
+			want:   false,
+		},
+		{
+			name:   "empty host denies rather than matching an empty origin host",
+			origin: "https://localhost:8443",
+			host:   "",
+			want:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isSameOrigin(tt.origin, tt.host); got != tt.want {
+				t.Errorf("isSameOrigin(%q, %q) = %v, want %v",
+					tt.origin, tt.host, got, tt.want)
+			}
+		})
+	}
+}