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 @@