fix(web): dedicated Google re-auth page + PR #39 review follow-ups

docs/oauth-pkce.md

@@ -213,10 +213,13 @@ use the server's global client (flags / env / BYO file). The chosen client is:
 - used for the authorize + token exchange,
 - stored (encrypted) with the connection and reused for token refresh, and
 - reused automatically on **reauth** by default — a connection re-authorizes
-  against the same org's client, never silently repointed at the global one. To
-  *migrate* a connection to a different client (e.g. off an old project whose
-  APIs were never enabled), expand the connection's **Re-authenticate** control
-  and paste the new client; a blank field keeps the current one.
+  against the same org's client, never silently repointed at the global one. The
+  **Re-authenticate** page has an optional "use a different OAuth client" field to
+  move a connection to *another client for the same account* — e.g. a new GCP
+  project **in the same Workspace org** (blank keeps the current client). Note
+  this can't cross orgs: re-auth requires signing in as the same account, and a
+  different org's Internal client won't admit that account (`org_internal`). To
+  move an account to a different org, delete and re-add the connection.
 
 **Setup, per org:** each Workspace admin creates a GCP project in their org,
 enables the APIs (Gmail/Drive/Docs/Sheets), and makes an **Internal** OAuth

internal/web/google_reauth_test.go

@@ -0,0 +1,133 @@
+package web
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	gmailconn "github.com/trilitech/Sieve/internal/connectors/gmail"
+	"github.com/trilitech/Sieve/internal/scriptgen"
+	"github.com/trilitech/Sieve/internal/testing/testenv"
+)
+
+// newGoogleReauthTestServer wires an admin web server with the gmail ("google")
+// connector registered so tests can seed and re-auth Google connections.
+func newGoogleReauthTestServer(t *testing.T) (*httptest.Server, *testenv.Env) {
+	t.Helper()
+	env := testenv.New(t).WithOperator("test-pass", "test-op")
+	env.Registry.Register(gmailconn.Meta, gmailconn.Factory)
+
+	scriptgenSvc := scriptgen.NewService(env.Connections, env.Settings)
+	srv := NewServer(
+		env.Tokens, env.Connections, env.Roles,
+		env.Registry, env.Approval, env.Audit,
+		"", env.Settings, scriptgenSvc,
+		env.Keyring, env.DB, "127.0.0.1:0",
+	)
+	srv.SetAuth(env.Operator, env.Session)
+	t.Cleanup(srv.Close)
+	ts := httptest.NewServer(srv.Handler())
+	t.Cleanup(ts.Close)
+	return ts, env
+}
+
+func seedGoogleConn(t *testing.T, env *testenv.Env, id string) {
+	t.Helper()
+	if err := env.Connections.Add(id, "google", "Work Google", map[string]any{
+		"email":         "user@example.com",
+		"oauth_token":   map[string]any{"access_token": "ya29.stub", "refresh_token": "r"},
+		"client_id":     "stored-cid.apps.googleusercontent.com",
+		"client_secret": "stored-secret",
+	}); err != nil {
+		t.Fatalf("seed google connection: %v", err)
+	}
+}
+
+// TestReauthPage_Renders proves the dedicated re-auth page renders for a Google
+// connection and shows the bound account (so operators know what they're re-authing).
+func TestReauthPage_Renders(t *testing.T) {
+	ts, env := newGoogleReauthTestServer(t)
+	seedGoogleConn(t, env, "g1")
+
+	resp, err := env.AdminClient().Get(ts.URL + "/connections/g1/reauth")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("reauth page: want 200, got %d", resp.StatusCode)
+	}
+	raw, _ := io.ReadAll(resp.Body)
+	body := string(raw)
+	if !strings.Contains(body, "Re-authenticate") || !strings.Contains(body, "user@example.com") {
+		t.Errorf("reauth page missing expected content (button / account email)")
+	}
+}
+
+// TestReauth_DefaultUsesStoredClient proves re-auth with no client fields
+// redirects to Google using the connection's STORED client (not a global one).
+func TestReauth_DefaultUsesStoredClient(t *testing.T) {
+	ts, env := newGoogleReauthTestServer(t)
+	seedGoogleConn(t, env, "g1")
+
+	req, _ := http.NewRequest(http.MethodPost, ts.URL+"/connections/g1/reauth", nil)
+	req.Header.Set("Origin", ts.URL)
+	resp, err := env.AdminClient().Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusFound {
+		t.Fatalf("reauth: want 302 redirect to Google, got %d", resp.StatusCode)
+	}
+	loc := resp.Header.Get("Location")
+	if !strings.Contains(loc, "client_id=stored-cid.apps.googleusercontent.com") {
+		t.Errorf("reauth must use the STORED client; Location=%s", loc)
+	}
+}
+
+// TestReauth_HalfSpecifiedClientRejected proves the both-or-neither guard: a
+// client_id with no secret (which would build a non-refreshing config) is a 400.
+func TestReauth_HalfSpecifiedClientRejected(t *testing.T) {
+	ts, env := newGoogleReauthTestServer(t)
+	seedGoogleConn(t, env, "g1")
+
+	form := url.Values{"google_client_id": {"new-cid.apps.googleusercontent.com"}} // no secret
+	req, _ := http.NewRequest(http.MethodPost, ts.URL+"/connections/g1/reauth", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.Header.Set("Origin", ts.URL)
+	resp, err := env.AdminClient().Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Fatalf("half-specified client on reauth must be 400, got %d", resp.StatusCode)
+	}
+}
+
+// TestAdd_HalfSpecifiedGoogleClientRejected proves the same guard on the add path.
+func TestAdd_HalfSpecifiedGoogleClientRejected(t *testing.T) {
+	ts, env := newGoogleReauthTestServer(t)
+
+	form := url.Values{
+		"connector_type":   {"google"},
+		"id":               {"g2"},
+		"display_name":     {"New Google"},
+		"google_client_id": {"new-cid.apps.googleusercontent.com"}, // no secret
+	}
+	req, _ := http.NewRequest(http.MethodPost, ts.URL+"/connections/add", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.Header.Set("Origin", ts.URL)
+	resp, err := env.AdminClient().Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Fatalf("half-specified Google client on add must be 400, got %d", resp.StatusCode)
+	}
+}

internal/web/server.go

@@ -342,7 +342,7 @@ func NewServer(
 	// given partial just ignore it. The ops picker partial is included so
 	// policies.html and policy_edit.html resolve to the same scope-aware
 	// markup — making create/edit divergence structurally impossible.
-	pages := []string{"connections", "connection_edit", "tokens", "tokens_edit", "approvals", "audit", "settings", "iam", "iam_edit", "iam_filter_edit", "docs"}
+	pages := []string{"connections", "connection_edit", "connection_reauth", "tokens", "tokens_edit", "approvals", "audit", "settings", "iam", "iam_edit", "iam_filter_edit", "docs"}
 	for _, page := range pages {
 		t := template.Must(
 			template.New("").Funcs(funcMap()).ParseFS(templateFS,
@@ -446,6 +446,7 @@ func (s *Server) Handler() http.Handler {
 	mux.HandleFunc("GET /connections", s.handleConnections)
 	mux.HandleFunc("POST /connections/add", s.handleConnectionAdd)
 	mux.HandleFunc("POST /connections/{id}/delete", s.handleConnectionDelete)
+	mux.HandleFunc("GET /connections/{id}/reauth", s.handleConnectionReauthPage)
 	mux.HandleFunc("POST /connections/{id}/reauth", s.handleConnectionReauth)
 	mux.HandleFunc("GET /connections/{id}/edit", s.handleConnectionEditPage)
 	mux.HandleFunc("POST /connections/{id}/edit", s.handleConnectionEditSave)
@@ -978,6 +979,13 @@ func (s *Server) handleConnectionAdd(w http.ResponseWriter, r *http.Request) {
 		// global client. Lets one instance serve multiple Workspace orgs.
 		gClientID := strings.TrimSpace(r.FormValue("google_client_id"))
 		gClientSecret := strings.TrimSpace(r.FormValue("google_client_secret"))
+		// Both-or-neither: a client_id without a secret builds a config that can't
+		// refresh tokens (the gmail connector needs both), so the connection would
+		// authorize but then die at expiry. Reject the half-specified input.
+		if (gClientID == "") != (gClientSecret == "") {
+			http.Error(w, "provide both a Google client_id and client_secret, or leave both blank to use the server default", http.StatusBadRequest)
+			return
+		}
 
 		conf, err := s.googleOAuthConfigFor(r, gClientID, gClientSecret)
 		if err != nil {
@@ -1041,18 +1049,42 @@ func (s *Server) handleConnectionDelete(w http.ResponseWriter, r *http.Request)
 // Limited to Google connections today; GitHub PAT/App connections have their
 // own setup flow and would need a separate re-auth surface if their tokens
 // expire.
+// handleConnectionReauthPage renders the dedicated re-authentication page for a
+// Google connection (GET). A clear single-purpose page beats an inline table
+// control: the primary action is one obvious button (re-auth with the stored
+// client), with an optional advanced disclosure for same-account client
+// migration. Admin-listener-only via requireOperatorSession.
+func (s *Server) handleConnectionReauthPage(w http.ResponseWriter, r *http.Request) {
+	id := r.PathValue("id")
+	conn, err := s.connections.GetWithConfig(id)
+	if err != nil {
+		// Locked/rotating keyring is transient (503), not a missing resource.
+		s.writeConnectionError(w, http.StatusNotFound, "connection not found", err)
+		return
+	}
+	if conn.ConnectorType != "google" {
+		http.Error(w, "re-auth not supported for connector type "+conn.ConnectorType, http.StatusBadRequest)
+		return
+	}
+	email, _ := conn.Config["email"].(string)
+	s.render(w, r, "connection_reauth", map[string]any{
+		"ID":          conn.ID,
+		"DisplayName": conn.DisplayName,
+		"Email":       email,
+		"Status":      conn.Status,
+	})
+}
+
 func (s *Server) handleConnectionReauth(w http.ResponseWriter, r *http.Request) {
 	id := r.PathValue("id")
 	// Read the stored config so we can re-authorize against the SAME OAuth client
 	// the connection was created with — critical for multi-org: a connection from
 	// one Workspace org must reauth via that org's client, not the global one.
 	conn, err := s.connections.GetWithConfig(id)
 	if err != nil {
-		if errors.Is(err, secrets.ErrKeyringNotLoaded) {
-			http.Error(w, "service locked: passphrase required", http.StatusServiceUnavailable)
-			return
-		}
-		http.Error(w, "connection not found", http.StatusNotFound)
+		// Locked/rotating keyring is a transient service state (503), not a
+		// missing resource — writeConnectionError covers both sentinels.
+		s.writeConnectionError(w, http.StatusNotFound, "connection not found", err)
 		return
 	}
 	if conn.ConnectorType != "google" {
@@ -1063,11 +1095,18 @@ func (s *Server) handleConnectionReauth(w http.ResponseWriter, r *http.Request)
 	// Reauth reuses the connection's stored client by DEFAULT so it stays on the
 	// same org's OAuth client — a connection from one org must not silently jump
 	// to the global client of another org (that would re-trigger org_internal).
-	// To deliberately MIGRATE a connection to a different client (e.g. off an old
-	// project whose APIs were never enabled), the operator can supply a new client
-	// on the reauth form; a non-empty client_id there overrides the stored one.
+	// To deliberately MIGRATE a connection to a different client (same account,
+	// e.g. a new GCP project in the same org), the operator can supply a new
+	// client here; a non-empty client_id overrides the stored one.
 	gClientID := strings.TrimSpace(r.FormValue("google_client_id"))
 	gClientSecret := strings.TrimSpace(r.FormValue("google_client_secret"))
+	// Both-or-neither: a client_id without a secret builds a config that can't
+	// refresh (the gmail connector needs both), so the connection would die at
+	// token expiry. Reject the half-specified case instead of silently accepting.
+	if (gClientID == "") != (gClientSecret == "") {
+		http.Error(w, "provide both client_id and client_secret, or neither", http.StatusBadRequest)
+		return
+	}
 	if gClientID == "" {
 		gClientID, _ = conn.Config["client_id"].(string)
 		gClientSecret, _ = conn.Config["client_secret"].(string)

internal/web/templates/connection_reauth.html

@@ -0,0 +1,73 @@
+{{define "connection_reauth"}}
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <title>Re-authenticate: {{.DisplayName}} - Sieve</title>
+  <script src="https://cdn.tailwindcss.com"></script>
+  <link rel="preconnect" href="https://fonts.googleapis.com">
+  <link href="https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600;700&display=swap" rel="stylesheet">
+  <style>body { font-family: 'Inter', sans-serif; }</style>
+</head>
+<body class="bg-slate-900 text-slate-100 min-h-screen">
+  {{template "nav" .}}
+
+  <main class="ml-64 p-8">
+    <div class="max-w-2xl">
+      <div class="mb-6">
+        <a href="/connections" class="text-sm text-slate-400 hover:text-slate-200">← Back to connections</a>
+        <h1 class="text-2xl font-bold text-white mt-2">Re-authenticate {{.DisplayName}}</h1>
+        <p class="text-sm text-slate-400 mt-1">
+          ID: <code class="text-slate-300">{{.ID}}</code>{{if .Email}} · Account: <code class="text-slate-300">{{.Email}}</code>{{end}}
+        </p>
+      </div>
+
+      {{if .Error}}
+      <div class="mb-6 rounded-lg bg-red-500/10 border border-red-500/20 px-4 py-3">
+        <p class="text-sm text-red-400">{{.Error}}</p>
+      </div>
+      {{end}}
+
+      {{if eq .Status "reauth_required"}}
+      <div class="mb-6 rounded-lg bg-amber-500/10 border border-amber-500/20 px-4 py-3">
+        <p class="text-sm text-amber-300">This connection needs re-authentication — its Google token expired or was revoked. Re-authenticate to restore access.</p>
+      </div>
+      {{end}}
+
+      <div class="rounded-lg border border-slate-700 bg-slate-800/40 p-6 space-y-5">
+        <p class="text-sm text-slate-300">
+          This re-runs Google sign-in for this connection. Its <strong>ID and IAM grants are preserved</strong>{{if .Email}}, and you must sign in with the same account (<code class="text-slate-300">{{.Email}}</code>){{end}} — Sieve rejects the re-auth if a different account is chosen.
+        </p>
+
+        <form method="POST" action="/connections/{{.ID}}/reauth" class="space-y-4">
+          <button type="submit"
+                  class="w-full rounded-lg bg-indigo-600 px-4 py-2.5 text-sm font-semibold text-white hover:bg-indigo-500 transition-colors">
+            Re-authenticate with Google
+          </button>
+
+          <details class="rounded-lg border border-slate-700 bg-slate-900/60 px-4 py-3">
+            <summary class="cursor-pointer text-xs font-medium text-slate-300">Advanced: use a different OAuth client <span class="text-slate-500">(optional)</span></summary>
+            <div class="mt-3 space-y-3">
+              <p class="text-xs text-slate-500">
+                Only to move this account onto a <em>different OAuth client for the same account</em> — e.g. a new GCP project <strong>in the same Google Workspace org</strong>. Cross-org migration isn't possible here (a different org's client won't admit this account); to switch orgs, delete and re-add the connection. Leave both blank to keep the current client. <strong>Provide both fields or neither.</strong>
+              </p>
+              <div>
+                <label class="block text-xs font-medium text-slate-400 mb-1">New OAuth client_id</label>
+                <input type="text" name="google_client_id" placeholder="…apps.googleusercontent.com"
+                       class="w-full rounded-lg bg-slate-900 border border-slate-600 px-3 py-1.5 text-sm text-white placeholder-slate-500 focus:outline-none focus:ring-2 focus:ring-indigo-500">
+              </div>
+              <div>
+                <label class="block text-xs font-medium text-slate-400 mb-1">New OAuth client_secret</label>
+                <input type="password" name="google_client_secret" placeholder="GOCSPX-…"
+                       class="w-full rounded-lg bg-slate-900 border border-slate-600 px-3 py-1.5 text-sm text-white placeholder-slate-500 focus:outline-none focus:ring-2 focus:ring-indigo-500">
+              </div>
+            </div>
+          </details>
+        </form>
+      </div>
+    </div>
+  </main>
+</body>
+</html>
+{{end}}

internal/web/templates/connections.html

@@ -70,28 +70,17 @@ <h2 class="text-sm font-semibold text-slate-300 uppercase tracking-wider">Active
               <td class="px-6 py-4 text-sm text-slate-400">{{timeAgo .CreatedAt}}</td>
               <td class="px-6 py-4 text-right whitespace-nowrap">
                 {{if and (eq .Status "reauth_required") (eq .ConnectorType "google")}}
-                <form method="POST" action="/connections/{{.ID}}/reauth" class="inline mr-3">
-                  <button type="submit"
-                          class="text-sm text-amber-300 hover:text-amber-200 font-medium transition-colors">
-                    Re-authenticate
-                  </button>
-                </form>
+                <a href="/connections/{{.ID}}/reauth"
+                   title="This connection's Google token expired or was revoked — re-authenticate to restore access."
+                   class="inline-block mr-3 text-sm text-amber-300 hover:text-amber-200 font-medium transition-colors">
+                  Re-authenticate
+                </a>
                 {{else if and (eq .Status "active") (eq .ConnectorType "google")}}
-                <details class="inline-block mr-3 text-left align-middle">
-                  <summary title="Re-run Google OAuth for this connection to refresh scopes/tokens, or migrate it to a different OAuth client. Its ID and IAM grants are preserved."
-                           class="cursor-pointer text-sm text-slate-400 hover:text-indigo-300 font-medium transition-colors">Re-authenticate</summary>
-                  <form method="POST" action="/connections/{{.ID}}/reauth" class="mt-2 space-y-2 rounded-lg border border-slate-700 bg-slate-900/80 p-3 w-72">
-                    <p class="text-xs text-slate-500">Re-runs OAuth against this connection's current client. To <em>migrate</em> it to a different Google Workspace org's client, paste that client below (blank = keep the current one).</p>
-                    <input type="text" name="google_client_id" placeholder="new OAuth client_id (optional)"
-                           class="w-full rounded-lg bg-slate-900 border border-slate-600 px-3 py-1.5 text-xs text-white placeholder-slate-500 focus:outline-none focus:ring-2 focus:ring-indigo-500">
-                    <input type="password" name="google_client_secret" placeholder="new OAuth client_secret (optional)"
-                           class="w-full rounded-lg bg-slate-900 border border-slate-600 px-3 py-1.5 text-xs text-white placeholder-slate-500 focus:outline-none focus:ring-2 focus:ring-indigo-500">
-                    <button type="submit"
-                            class="w-full rounded-lg bg-indigo-600 px-3 py-1.5 text-xs font-medium text-white hover:bg-indigo-500 transition-colors">
-                      Re-authenticate
-                    </button>
-                  </form>
-                </details>
+                <a href="/connections/{{.ID}}/reauth"
+                   title="Re-run Google sign-in for this connection — refresh scopes/tokens, or move it to a different OAuth client for the same account. ID and IAM grants are preserved."
+                   class="inline-block mr-3 text-sm text-slate-400 hover:text-indigo-300 font-medium transition-colors">
+                  Re-authenticate
+                </a>
                 {{end}}
                 {{if or (eq .ConnectorType "http_proxy") (eq .ConnectorType "mcp_proxy") (eq .ConnectorType "github") (eq .ConnectorType "gitlab")}}
                 <a href="/connections/{{.ID}}/edit"