diff --git a/docs/oauth-pkce.md b/docs/oauth-pkce.md index 7ffb4f0..d61c00b 100644 --- a/docs/oauth-pkce.md +++ b/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 diff --git a/internal/web/google_reauth_test.go b/internal/web/google_reauth_test.go new file mode 100644 index 0000000..5912e6b --- /dev/null +++ b/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) + } +} diff --git a/internal/web/server.go b/internal/web/server.go index d0c143e..ade4282 100644 --- a/internal/web/server.go +++ b/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,6 +1049,32 @@ 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 @@ -1048,11 +1082,9 @@ func (s *Server) handleConnectionReauth(w http.ResponseWriter, r *http.Request) // 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) diff --git a/internal/web/templates/connection_reauth.html b/internal/web/templates/connection_reauth.html new file mode 100644 index 0000000..6c5b1c6 --- /dev/null +++ b/internal/web/templates/connection_reauth.html @@ -0,0 +1,73 @@ +{{define "connection_reauth"}} + + + + + + Re-authenticate: {{.DisplayName}} - Sieve + + + + + + + {{template "nav" .}} + +
+
+
+ ← Back to connections +

Re-authenticate {{.DisplayName}}

+

+ ID: {{.ID}}{{if .Email}} · Account: {{.Email}}{{end}} +

+
+ + {{if .Error}} +
+

{{.Error}}

+
+ {{end}} + + {{if eq .Status "reauth_required"}} +
+

This connection needs re-authentication — its Google token expired or was revoked. Re-authenticate to restore access.

+
+ {{end}} + +
+

+ This re-runs Google sign-in for this connection. Its ID and IAM grants are preserved{{if .Email}}, and you must sign in with the same account ({{.Email}}){{end}} — Sieve rejects the re-auth if a different account is chosen. +

+ +
+ + +
+ Advanced: use a different OAuth client (optional) +
+

+ Only to move this account onto a different OAuth client for the same account — e.g. a new GCP project in the same Google Workspace org. 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. Provide both fields or neither. +

+
+ + +
+
+ + +
+
+
+
+
+
+
+ + +{{end}} diff --git a/internal/web/templates/connections.html b/internal/web/templates/connections.html index 7fb85ea..44e0818 100644 --- a/internal/web/templates/connections.html +++ b/internal/web/templates/connections.html @@ -70,28 +70,17 @@

Active {{timeAgo .CreatedAt}} {{if and (eq .Status "reauth_required") (eq .ConnectorType "google")}} -
- -
+ + Re-authenticate + {{else if and (eq .Status "active") (eq .ConnectorType "google")}} -
- Re-authenticate -
-

Re-runs OAuth against this connection's current client. To migrate it to a different Google Workspace org's client, paste that client below (blank = keep the current one).

- - - -
-
+ + Re-authenticate + {{end}} {{if or (eq .ConnectorType "http_proxy") (eq .ConnectorType "mcp_proxy") (eq .ConnectorType "github") (eq .ConnectorType "gitlab")}}