From 31bf0557dfe86bb4988bb67288e74a2973406d31 Mon Sep 17 00:00:00 2001 From: Kristian Wind Date: Tue, 18 Aug 2026 09:05:18 +0200 Subject: [PATCH] feat(gmail): the OAuth client can be entered instead of deployed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one click that opens Google's consent screen already existed. What it needs behind it is a registered client, and no app can conjure one: Google issues no Gmail access to an unregistered client, and `gmail.readonly` is a restricted scope, so an id shipped inside a public image would be both extractable from it and unusable without Google's security review of every deployment using it. The registration is a one-off. Having to edit a Rune's manifest and recreate the container to paste the result was not. `VERDANDE_GMAIL_CLIENT_ID` and `_SECRET` still win when set. A value in the environment is a deliberate deployment decision, applied again on every recreate, and a form that could quietly override it would make the manifest a lie — so the page says where the value came from, and PUT answers 409. The redirect URI is spelled out on the page rather than left to be reconstructed. It is derived from VERDANDE_BASE_URL, it has to match Google's copy exactly, and the error Google returns when it does not names neither value. Its own table rather than `user_settings`: an OAuth client is the instance's, not one person's — and `user_settings` has `user_id` as its whole primary key, so a second scope there would silently overwrite the AI provider's key. Co-Authored-By: Claude Opus 5 --- internal/httpapi/gmail_handlers.go | 135 +++++++++++++++++- internal/httpapi/integrations_test.go | 127 ++++++++++++++++ .../migrations/0011_instance_settings.sql | 24 ++++ internal/store/notifications.go | 41 ++++++ .../indstillinger/integrationer/+page.svelte | 134 ++++++++++++++++- 5 files changed, 450 insertions(+), 11 deletions(-) create mode 100644 internal/store/migrations/0011_instance_settings.sql diff --git a/internal/httpapi/gmail_handlers.go b/internal/httpapi/gmail_handlers.go index e468a1d..4979029 100644 --- a/internal/httpapi/gmail_handlers.go +++ b/internal/httpapi/gmail_handlers.go @@ -10,12 +10,44 @@ import ( "github.com/kristianwind/verdande/internal/store" ) -func (s *Server) gmailConfig() gmail.Config { - return gmail.Config{ +// gmailConfig is the OAuth client this instance authorises with. +// +// The environment wins when it is set. A value in the environment is a deliberate +// deployment decision — it is in the Rune's manifest, under version control, +// applied on every recreate — and a form that could quietly override it would make +// the manifest a lie. The stored pair is for the ordinary case, where nobody wants +// to redeploy a container to paste a client id. +func (s *Server) gmailConfig(ctx context.Context) gmail.Config { + cfg := gmail.Config{ ClientID: s.cfg.GmailClientID, ClientSecret: s.cfg.GmailClientSecret, RedirectURL: s.cfg.GmailRedirectURL(), } + if cfg.ClientID != "" && cfg.ClientSecret != "" { + return cfg + } + + stored, err := s.db.InstanceSettings(ctx, "gmail") + if err != nil { + // Not fatal: an unconfigured Gmail and a database that could not answer + // look the same from here, and the caller's next step is the same either + // way — say it is not set up. + return cfg + } + if id, _ := stored["client_id"].(string); cfg.ClientID == "" { + cfg.ClientID = id + } + if secret, _ := stored["client_secret"].(string); cfg.ClientSecret == "" { + cfg.ClientSecret = secret + } + return cfg +} + +// gmailFromEnv says whether the deployment set the client, which the settings page +// needs so it can explain why the fields are not editable rather than accepting +// changes that will never take effect. +func (s *Server) gmailFromEnv() bool { + return s.cfg.GmailClientID != "" && s.cfg.GmailClientSecret != "" } // handleGmailAuthorize starts the flow: it mints a PKCE pair, remembers it against @@ -27,7 +59,7 @@ func (s *Server) gmailConfig() gmail.Config { // defaults. func (s *Server) handleGmailAuthorize(w http.ResponseWriter, r *http.Request) { user := userFrom(r.Context()) - cfg := s.gmailConfig() + cfg := s.gmailConfig(r.Context()) if !cfg.Configured() { writeError(w, http.StatusConflict, CodeGmailNotConfigured, @@ -102,7 +134,7 @@ func (s *Server) handleGmailCallback(w http.ResponseWriter, r *http.Request) { return } - token, err := s.gmailConfig().Exchange(r.Context(), code, gmail.PKCE{Verifier: verifier}) + token, err := s.gmailConfig(r.Context()).Exchange(r.Context(), code, gmail.PKCE{Verifier: verifier}) if err != nil { s.log.Error("gmail exchange", "err", err, "user", user.ID) http.Redirect(w, r, settingsURL+"?gmail=failed", http.StatusFound) @@ -185,7 +217,7 @@ func (s *Server) SyncGmail(ctx context.Context, user *store.User) (int, error) { access := str("access_token") expires, _ := settings["expires_at"].(float64) if access == "" || int64(expires) <= time.Now().Unix() { - token, err := s.gmailConfig().Refresh(ctx, refresh) + token, err := s.gmailConfig(ctx).Refresh(ctx, refresh) if err != nil { return 0, err } @@ -300,3 +332,96 @@ func (s *Server) handleVersion(w http.ResponseWriter, r *http.Request) { } writeJSON(w, http.StatusOK, status) } + +// --- the OAuth client itself ------------------------------------------------------ + +// The registration Google requires, enterable rather than deployed. +// +// The one click that opens Google's consent screen already exists; what it needs +// behind it is a registered client, and that is not something any app can conjure: +// Google issues no Gmail access to an unregistered client, and `gmail.readonly` is +// a restricted scope, so an id shipped inside a public image would be both +// extractable from it and unusable without Google's security review of every +// deployment. The registration is a one-off. Having to edit a Rune's manifest and +// recreate the container to paste the result was not. +// +// Administrators only, and sessions only: this is the instance's identity to +// Google, and a leaked API token must not be able to read or replace it. + +type gmailClientJSON struct { + ClientID string `json:"client_id"` + // Secret is written and never read back, like the AI key: a settings page that + // repopulates a password field is one that will eventually leak into a + // screenshot. + ClientSecret string `json:"client_secret,omitempty"` + HasSecret bool `json:"has_secret"` + // FromEnv says the deployment set this, in which case the stored pair is + // ignored and the fields are not worth editing. Sent so the page can say why + // rather than accepting changes that will never take effect. + FromEnv bool `json:"from_env"` + // RedirectURI is what has to be registered with Google, spelled out. It is + // derived from VERDANDE_BASE_URL and must match exactly — it is the single + // most likely thing to be wrong, and the error Google gives for it names + // neither value. + RedirectURI string `json:"redirect_uri"` +} + +func (s *Server) handleGetGmailClient(w http.ResponseWriter, r *http.Request) { + stored, err := s.db.InstanceSettings(r.Context(), "gmail") + if err != nil { + s.internal(w, r, "gmail client", err) + return + } + out := gmailClientJSON{ + FromEnv: s.gmailFromEnv(), + RedirectURI: s.cfg.GmailRedirectURL(), + } + if out.FromEnv { + out.ClientID = s.cfg.GmailClientID + out.HasSecret = true + } else { + out.ClientID, _ = stored["client_id"].(string) + secret, _ := stored["client_secret"].(string) + out.HasSecret = secret != "" + } + writeJSON(w, http.StatusOK, out) +} + +func (s *Server) handleSetGmailClient(w http.ResponseWriter, r *http.Request) { + var req gmailClientJSON + if err := decodeJSON(w, r, &req); err != nil { + return + } + if s.gmailFromEnv() { + writeError(w, http.StatusConflict, CodeConflict, + "the Gmail client is set in the environment and cannot be changed here") + return + } + + stored, err := s.db.InstanceSettings(r.Context(), "gmail") + if err != nil { + s.internal(w, r, "gmail client", err) + return + } + // An empty secret means "leave it alone", not "delete it" — otherwise + // correcting a typo in the client id would silently clear the secret. + secret, _ := stored["client_secret"].(string) + if req.ClientSecret != "" { + secret = strings.TrimSpace(req.ClientSecret) + } + id := strings.TrimSpace(req.ClientID) + + if fields := map[string]string{}; id == "" && secret != "" { + fields["client_id"] = "required" + writeFieldErrors(w, fields) + return + } + + if err := s.db.SetInstanceSettings(r.Context(), "gmail", map[string]any{ + "client_id": id, "client_secret": secret, + }); err != nil { + s.internal(w, r, "save gmail client", err) + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/internal/httpapi/integrations_test.go b/internal/httpapi/integrations_test.go index 24f2209..afb1734 100644 --- a/internal/httpapi/integrations_test.go +++ b/internal/httpapi/integrations_test.go @@ -1153,3 +1153,130 @@ func TestMCPCreateRefusesASecondLevelOfSubtask(t *testing.T) { t.Errorf("a sub-task of a sub-task was accepted: %v", response) } } + +// The Gmail OAuth client, enterable rather than deployed. +// +// The registration itself cannot be avoided — Google issues no Gmail access to an +// unregistered client — but having to edit a Rune's manifest and recreate the +// container to paste the result can be, and that is what this covers. +func TestTheGmailClientCanBeSetFromTheInterface(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + _, empty := ts.do(t, "GET", "/api/v1/gmail/client", nil) + if empty["client_id"] != "" || empty["has_secret"] != false { + t.Errorf("a fresh instance already has a client: %v", empty) + } + if empty["from_env"] != false { + t.Errorf("from_env = %v with nothing in the environment", empty["from_env"]) + } + // The single most likely thing to be wrong, so it is spelled out rather than + // left for somebody to reconstruct. + if empty["redirect_uri"] != "http://127.0.0.1/oauth/gmail/callback" { + t.Errorf("redirect_uri = %v", empty["redirect_uri"]) + } + + resp, _ := ts.do(t, "PUT", "/api/v1/gmail/client", map[string]any{ + "client_id": "1234.apps.googleusercontent.com", "client_secret": "hemmelig", + }) + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("save: status %d", resp.StatusCode) + } + + _, saved := ts.do(t, "GET", "/api/v1/gmail/client", nil) + if saved["client_id"] != "1234.apps.googleusercontent.com" { + t.Errorf("client_id = %v", saved["client_id"]) + } + if saved["has_secret"] != true { + t.Error("has_secret is false after saving one") + } + // Never read back. A settings page that repopulates a password field is one + // that will eventually leak into a screenshot. + if _, present := saved["client_secret"]; present { + t.Error("the secret came back in the response") + } + + // Saving only the id must not clear the secret — otherwise correcting a typo + // silently breaks the connection. + ts.do(t, "PUT", "/api/v1/gmail/client", map[string]any{ + "client_id": "5678.apps.googleusercontent.com", + }) + _, again := ts.do(t, "GET", "/api/v1/gmail/client", nil) + if again["client_id"] != "5678.apps.googleusercontent.com" || again["has_secret"] != true { + t.Errorf("saving the id alone lost the secret: %v", again) + } + + // And the authorise call now has something to build a URL with, which is the + // whole point: one click, and Google's consent screen. + resp, body := ts.do(t, "POST", "/api/v1/gmail/authorize", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("authorize with a stored client: status %d, body %v", resp.StatusCode, body) + } + if url, _ := body["url"].(string); !strings.Contains(url, "5678.apps.googleusercontent.com") { + t.Errorf("the consent URL does not carry the stored client: %v", body["url"]) + } +} + +// The environment wins, and says so. A value in the Rune's manifest is applied on +// every recreate, so a form that could quietly override it would make the manifest +// a lie. +func TestTheEnvironmentsGmailClientCannotBeOverwritten(t *testing.T) { + ts := newTestServerWith(t, func(c *config.Config) { + c.GmailClientID = "fra-miljøet.apps.googleusercontent.com" + c.GmailClientSecret = "også-fra-miljøet" + }) + ts.bootstrap(t) + + _, shown := ts.do(t, "GET", "/api/v1/gmail/client", nil) + if shown["from_env"] != true { + t.Errorf("from_env = %v with the environment set", shown["from_env"]) + } + if shown["client_id"] != "fra-miljøet.apps.googleusercontent.com" { + t.Errorf("client_id = %v", shown["client_id"]) + } + + resp, _ := ts.do(t, "PUT", "/api/v1/gmail/client", map[string]any{ + "client_id": "min-egen", "client_secret": "min-egen", + }) + if resp.StatusCode != http.StatusConflict { + t.Fatalf("overwriting the environment: status %d, want 409", resp.StatusCode) + } + + _, after := ts.do(t, "GET", "/api/v1/gmail/client", nil) + if after["client_id"] != "fra-miljøet.apps.googleusercontent.com" { + t.Errorf("the environment's client was replaced: %v", after["client_id"]) + } +} + +// Administrators only, and sessions only: this is the instance's identity to +// Google, and a leaked token must not be able to read or replace it. +func TestTheGmailClientIsAdminsOnlyAndSessionsOnly(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + other := ts.newUser(t, "anden@example.dk", "Anden") + + for _, c := range []struct { + method string + body any + }{{"GET", nil}, {"PUT", map[string]any{"client_id": "x"}}} { + if resp, _ := other.do(t, c.method, "/api/v1/gmail/client", c.body); resp.StatusCode != http.StatusForbidden { + t.Errorf("%s as an ordinary user: status %d, want 403", c.method, resp.StatusCode) + } + } + + admin, err := ts.db.UserByEmail(t.Context(), "kristian@example.dk") + if err != nil { + t.Fatal(err) + } + token := ts.apiToken(t, admin.ID) + req, _ := http.NewRequest("GET", ts.URL+"/api/v1/gmail/client", nil) + req.Header.Set("Authorization", "Bearer "+token) + resp, err := (&http.Client{}).Do(req) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Errorf("with an admin's API token: status %d, want 403", resp.StatusCode) + } +} diff --git a/internal/store/migrations/0011_instance_settings.sql b/internal/store/migrations/0011_instance_settings.sql new file mode 100644 index 0000000..b6ee8cb --- /dev/null +++ b/internal/store/migrations/0011_instance_settings.sql @@ -0,0 +1,24 @@ +-- Settings that belong to the instance rather than to a person. +-- +-- The first of them is the Gmail OAuth client. It was configuration only — +-- `VERDANDE_GMAIL_CLIENT_ID` and `_SECRET` — which on a Rune means editing the +-- manifest and recreating the container to change a value you paste once and +-- never touch again. The registration itself cannot be avoided (Google issues no +-- Gmail access to an unregistered client, and `gmail.readonly` is a restricted +-- scope, so a client id shipped inside a public image would be both extractable +-- and unusable), but having to redeploy to enter it can. +-- +-- Its own table rather than `user_settings`, for two reasons. An OAuth client is +-- the instance's, not one person's — everybody connecting their mailbox uses the +-- same registration. And `user_settings` has `user_id` as its whole primary key, +-- so it can hold exactly one scope per person and its upsert rewrites the scope +-- name on conflict; adding a second scope there would silently overwrite the AI +-- provider's key. +-- +-- JSON for the same reason templates are: written whole, read whole, and it would +-- otherwise be a migration every time a setting is added. +CREATE TABLE instance_settings ( + scope TEXT PRIMARY KEY, + values_json TEXT NOT NULL DEFAULT '{}', + updated_at INTEGER NOT NULL +); diff --git a/internal/store/notifications.go b/internal/store/notifications.go index 05d7ce4..918c94b 100644 --- a/internal/store/notifications.go +++ b/internal/store/notifications.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "time" "github.com/kristianwind/verdande/internal/auth" @@ -346,3 +347,43 @@ func (db *DB) SetUserSettings(ctx context.Context, userID, scope string, values userID, scope, string(raw), time.Now().Unix()) return err } + +// --- instance-wide settings ------------------------------------------------------------- + +// InstanceSettings reads one scope of the instance's own settings. A missing row +// is an empty map, not an error. +// +// Separate from UserSettings because some configuration is the server's rather +// than a person's — the Gmail OAuth client is one registration that everybody +// connecting a mailbox goes through, not a preference each of them holds. +func (db *DB) InstanceSettings(ctx context.Context, scope string) (map[string]any, error) { + var raw string + err := db.QueryRowContext(ctx, + `SELECT values_json FROM instance_settings WHERE scope = ?`, scope).Scan(&raw) + if errors.Is(err, sql.ErrNoRows) { + return map[string]any{}, nil + } + if err != nil { + return nil, err + } + values := map[string]any{} + if err := json.Unmarshal([]byte(raw), &values); err != nil { + return map[string]any{}, nil + } + return values, nil +} + +func (db *DB) SetInstanceSettings(ctx context.Context, scope string, values map[string]any) error { + raw, err := json.Marshal(values) + if err != nil { + return err + } + _, err = db.ExecContext(ctx, ` + INSERT INTO instance_settings (scope, values_json, updated_at) + VALUES (?, ?, ?) + ON CONFLICT (scope) DO UPDATE SET + values_json = excluded.values_json, + updated_at = excluded.updated_at`, + scope, string(raw), time.Now().Unix()) + return err +} diff --git a/web/src/routes/indstillinger/integrationer/+page.svelte b/web/src/routes/indstillinger/integrationer/+page.svelte index 8fc8b42..9fd50e3 100644 --- a/web/src/routes/indstillinger/integrationer/+page.svelte +++ b/web/src/routes/indstillinger/integrationer/+page.svelte @@ -11,6 +11,47 @@ let gmailSaved = $state(false); let syncing = $state(false); + // The OAuth client is the instance's registration with Google, not anybody's + // mailbox — so it is read only for administrators, and only when they are the + // ones who could act on it. + let client = $state(null); + let clientId = $state(''); + let clientSecret = $state(''); + let savingClient = $state(false); + let clientSaved = $state(false); + + $effect(() => { + if (!app.user?.is_admin) return; + api + .gmailClient() + .then((c) => { + client = c; + clientId = c.client_id ?? ''; + }) + .catch(() => { + // Not worth a toast: the panel simply does not offer the fields, which + // is the same thing it does for everybody who is not an administrator. + }); + }); + + async function saveClient(event) { + event.preventDefault(); + savingClient = true; + clientSaved = false; + try { + await api.setGmailClient({ client_id: clientId, client_secret: clientSecret }); + // Cleared rather than kept: the field is a password field and the value is + // now stored. Leaving it filled is how a secret ends up in a screenshot. + clientSecret = ''; + client = await api.gmailClient(); + clientSaved = true; + } catch (e) { + app.toast(humanMessage(e)); + } finally { + savingClient = false; + } + } + /** * What the OAuth callback redirected back with. It arrives as a query parameter * because the callback is a top-level navigation from Google — there is no fetch @@ -157,13 +198,71 @@

…

{:else if !gmail.connected}
- +
-

- Kræver at administratoren har registreret en OAuth-klient hos Google og sat - VERDANDE_GMAIL_CLIENT_ID og - _SECRET. -

+ + + {#if app.user?.is_admin && client} + {#if client.from_env} +

+ Klienten er sat i miljøet (VERDANDE_GMAIL_CLIENT_ID), + så den kan ikke ændres her. Det er med vilje: en værdi i runens manifest er + en beslutning, der bliver anvendt igen ved hver genstart, og en formular, + der kunne overskrive den, ville gøre manifestet til en løgn. +

+ {:else} +
+ OAuth-klient hos Google + +

+ Registrér en klient i Google Cloud Console → APIs & + Services → Credentials → OAuth client ID → Web application, slå + Gmail API til, og indsæt de to værdier her. Din egen konto skal stå som + testbruger, så længe appen ikke er verificeret hos Google. +

+ +
+ + + +
+ +
+
+ + +
+
+ + +
+
+ + {#if clientSaved}Gemt.{/if} +
+
+
+ {/if} + {:else if client && !client.client_id} +

+ Der er ikke registreret en OAuth-klient hos Google endnu. Det skal en + administrator gøre, før knappen her kan bruges. +

+ {/if} {:else}

Forbundet som {gmail.email || 'ukendt konto'}.

@@ -269,6 +368,29 @@