From 59d0f57a47955601d2c21ae4504568650e8d352d Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 20:17:20 -0400 Subject: [PATCH 1/2] Add Google Calendar OAuth connect and disconnect GET /api/v1/integrations/google/connect sends the browser to Google's consent screen (calendar.events and calendar.calendarlist.readonly, access_type=offline, prompt=consent, PKCE S256) with a state carried in a signed __Host-mealquest_google flow cookie bound to the session. GET /api/v1/integrations/google/callback checks the state and session, exchanges the code, refuses a response without a refresh token (reconnect_required) or without both scopes, seals the refresh token with secretbox into integrations_google and sets connected_at. DELETE /api/v1/integrations/google revokes the token at Google and clears it; a revoke failure keeps the connection so it can be retried. - internal/gcal: the connect service; Google endpoints are overridable and internal/gcal/gcaltest fakes the token and revoke endpoints. - internal/auth: the login cookie becomes the exported FlowCookie both flows use (same wire format); CheckRedirect and ExchangeError are exported for reuse. - internal/config: MEALQUEST_GOOGLE_CLIENT_ID and MEALQUEST_GOOGLE_CLIENT_SECRET_FILE, required at boot. - DeleteIntegrationsGoogle query (sqlc regenerated); no migration, the integrations_google table already exists. - deploy: compose mounts google_client_secret; .env.example and RUNBOOK document the Google OAuth client, its redirect URI and rotation. Maestro task 1e9d7850-3a09-4572-b8a1-cb18cd4774cb. Co-Authored-By: Claude Opus 5.5 --- cmd/server/boot_test.go | 6 + cmd/server/main.go | 15 +- deploy/.env.example | 15 +- deploy/RUNBOOK.md | 58 +++- deploy/compose.yaml | 4 + docs/ARCHITECTURE.md | 22 ++ internal/auth/auth.go | 26 +- internal/auth/auth_test.go | 4 +- internal/auth/cookie.go | 101 ++++-- internal/config/config.go | 12 + internal/config/config_test.go | 48 ++- internal/db/querier.go | 4 + internal/db/settings.sql.go | 16 + internal/gcal/connect.go | 357 ++++++++++++++++++++ internal/gcal/connect_test.go | 476 +++++++++++++++++++++++++++ internal/gcal/export_test.go | 6 + internal/gcal/gcaltest/gcaltest.go | 209 ++++++++++++ internal/server/aisettings_test.go | 2 +- internal/server/dinners_test.go | 2 +- internal/server/google.go | 105 ++++++ internal/server/google_test.go | 307 +++++++++++++++++ internal/server/login_test.go | 2 +- internal/server/media_test.go | 2 +- internal/server/origin_test.go | 2 +- internal/server/plans_test.go | 2 +- internal/server/recipesearch_test.go | 2 +- internal/server/server.go | 16 +- internal/server/server_test.go | 52 ++- internal/server/settings_test.go | 2 +- internal/server/suggestions_test.go | 2 +- migrations/queries/settings.sql | 7 + 31 files changed, 1817 insertions(+), 67 deletions(-) create mode 100644 internal/gcal/connect.go create mode 100644 internal/gcal/connect_test.go create mode 100644 internal/gcal/export_test.go create mode 100644 internal/gcal/gcaltest/gcaltest.go create mode 100644 internal/server/google.go create mode 100644 internal/server/google_test.go diff --git a/cmd/server/boot_test.go b/cmd/server/boot_test.go index 082b579..4e47b92 100644 --- a/cmd/server/boot_test.go +++ b/cmd/server/boot_test.go @@ -44,6 +44,10 @@ func loadConfig(t *testing.T, keyFile string) config.Config { if err := os.WriteFile(clientSecretPath, []byte("client-secret\n"), 0o600); err != nil { t.Fatal(err) } + googleSecretPath := filepath.Join(dir, "google_client_secret") + if err := os.WriteFile(googleSecretPath, []byte("google-secret\n"), 0o600); err != nil { + t.Fatal(err) + } t.Setenv(config.EnvHTTPAddr, "127.0.0.1:0") t.Setenv(config.EnvPublicURL, "http://localhost") t.Setenv(config.EnvDataDir, t.TempDir()) @@ -53,6 +57,8 @@ func loadConfig(t *testing.T, keyFile string) config.Config { t.Setenv(config.EnvOIDCClientID, "mealquest") t.Setenv(config.EnvOIDCClientSecretFile, clientSecretPath) t.Setenv(config.EnvOIDCAllowedGroups, "mealquest-admin") + t.Setenv(config.EnvGoogleClientID, "mealquest.apps.googleusercontent.com") + t.Setenv(config.EnvGoogleClientSecretFile, googleSecretPath) // MEALQUEST_DEFAULT_TZ is left unset; t.Setenv restores any outer value. t.Setenv(config.EnvDefaultTZ, "") if err := os.Unsetenv(config.EnvDefaultTZ); err != nil { diff --git a/cmd/server/main.go b/cmd/server/main.go index f5e4efb..7a38817 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -21,6 +21,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/plan" @@ -118,6 +119,18 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } + googleSvc, err := gcal.NewService(db, box, gcal.Config{ + ClientID: cfg.GoogleClientID, + ClientSecret: cfg.GoogleClientSecret.Reveal(), + PublicURL: cfg.PublicURL, + SecretKey: []byte(cfg.SecretKey.Reveal()), + Endpoint: gcal.GoogleEndpoint, + RevokeURL: gcal.GoogleRevokeURL, + }) + if err != nil { + return err + } + sessions, err := auth.NewSessionManager(db, cfg.SessionIdle, cfg.SessionMax) if err != nil { return err @@ -134,7 +147,7 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } - router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, search, ai, dinnerSvc, suggestSvc, planSvc, runner, photos, frontend.Assets()) + router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, search, ai, dinnerSvc, suggestSvc, planSvc, googleSvc, runner, photos, frontend.Assets()) if err != nil { return err } diff --git a/deploy/.env.example b/deploy/.env.example index a908b3f..6262452 100644 --- a/deploy/.env.example +++ b/deploy/.env.example @@ -8,6 +8,7 @@ # mkdir -p deploy/secrets && chmod 700 deploy/secrets # head -c 32 /dev/urandom | base64 > deploy/secrets/secret_key # printf '%s\n' '' > deploy/secrets/oidc_client_secret +# printf '%s\n' '' > deploy/secrets/google_client_secret # sudo chown 10001:10001 deploy/secrets/* && sudo chmod 600 deploy/secrets/* # # SearXNG's secret key is the exception: compose reads it itself, so it @@ -54,6 +55,14 @@ MEALQUEST_OIDC_CLIENT_ID=mealquest # Comma-separated Authentik groups whose members may sign in. MEALQUEST_OIDC_ALLOWED_GROUPS=mealquest-admin +# --- Google Calendar (Push to Calendar) ------------------------------------ + +# Client ID of the Google OAuth web client (deploy/RUNBOOK.md "Google +# Calendar OAuth client"). Required: the service will not start without it +# and the client secret file. Its authorized redirect URI is +# /api/v1/integrations/google/callback. +MEALQUEST_GOOGLE_CLIENT_ID=.apps.googleusercontent.com + # Optional session lifetimes. A session ends after IDLE unused and always MAX # after sign-in; MAX must be at least IDLE. Defaults: 720h and 4320h. MEALQUEST_SESSION_IDLE=720h @@ -67,6 +76,8 @@ MEALQUEST_SESSION_MAX=4320h # Base64 of 32 random bytes; encrypts secrets at rest (AES-256-GCM). # MEALQUEST_OIDC_CLIENT_SECRET_FILE=/run/secrets/oidc_client_secret # The Authentik provider's client secret. +# MEALQUEST_GOOGLE_CLIENT_SECRET_FILE=/run/secrets/google_client_secret +# The Google OAuth web client's secret. # MEALQUEST_SEARXNG_URL=http://searxng:8080 # The SearXNG recipe-search sidecar on the compose network. The Settings # screen pre-fills it as the SearXNG URL until one is saved; the saved @@ -81,7 +92,3 @@ MEALQUEST_SESSION_MAX=4320h # MEALQUEST_VAPID_PUBLIC_KEY= # MEALQUEST_VAPID_PRIVATE_KEY_FILE=/run/secrets/vapid_private_key # MEALQUEST_VAPID_SUBJECT=mailto: -# -# Google Calendar OAuth web client (Feature 16). -# MEALQUEST_GOOGLE_CLIENT_ID=.apps.googleusercontent.com -# MEALQUEST_GOOGLE_CLIENT_SECRET_FILE=/run/secrets/google_client_secret diff --git a/deploy/RUNBOOK.md b/deploy/RUNBOOK.md index edfc375..61137ae 100644 --- a/deploy/RUNBOOK.md +++ b/deploy/RUNBOOK.md @@ -33,6 +33,8 @@ On office-srvr (10.60.1.14), as recorded on 2026-10-05: Elsewhere: - Admin access to Authentik at . +- Owner access to the Google Cloud project that holds MealQuest's OAuth + client (see "Google Calendar OAuth client"). - SSH access to the Traefik host 10.60.1.33. - Cloudflare access for the `cubecraftlabs.com` zone, and admin access to both Pi-holes (10.60.1.101 `dns.ccl`, 10.60.1.85 `dns2.ccl`). @@ -129,6 +131,7 @@ Values in use: | `MEALQUEST_OIDC_ISSUER` | `https://auth.cubecraftlabs.com/application/o/mealquest/` | | `MEALQUEST_OIDC_CLIENT_ID` | `i6zraqmWsF552b8nmte070LXz0SI5pRdArBxFxeq` (not a secret) | | `MEALQUEST_OIDC_ALLOWED_GROUPS` | `mealquest-admin` | +| `MEALQUEST_GOOGLE_CLIENT_ID` | Client ID of the Google OAuth web client (not a secret; see "Google Calendar OAuth client") | | `MEALQUEST_SESSION_IDLE` / `MEALQUEST_SESSION_MAX` | `720h` / `4320h` | `MEALQUEST_HTTP_ADDR`, `MEALQUEST_DATA_DIR` and the `*_FILE` variables are @@ -146,6 +149,7 @@ service refuses to start. | --- | --- | --- | | `secret_key` | base64 of 32 random bytes; encrypts stored secrets (AES-256-GCM) | `/run/secrets/secret_key` | | `oidc_client_secret` | Client Secret of the Authentik provider `mealquest` | `/run/secrets/oidc_client_secret` | +| `google_client_secret` | Client secret of the Google OAuth web client | `/run/secrets/google_client_secret` | | `litestream_key` | dedicated SSH private key for the NAS (see "Backups") | `/etc/litestream/sftp_key` | | `searxng.env` | `SEARXNG_SECRET=<64 hex characters>`; SearXNG's secret key | not mounted; compose passes it as an environment variable | @@ -156,11 +160,12 @@ it (as `env_file`), so it is owned by `overseer`, mode 0600. mkdir -p deploy/secrets && chmod 700 deploy/secrets head -c 32 /dev/urandom | base64 > deploy/secrets/secret_key printf '%s\n' '' > deploy/secrets/oidc_client_secret +printf '%s\n' '' > deploy/secrets/google_client_secret # litestream_key: see "Backups" > "Database — Litestream" -sudo chown 10001:10001 deploy/secrets/secret_key deploy/secrets/oidc_client_secret deploy/secrets/litestream_key +sudo chown 10001:10001 deploy/secrets/secret_key deploy/secrets/oidc_client_secret deploy/secrets/google_client_secret deploy/secrets/litestream_key sudo chmod 600 deploy/secrets/* (umask 077; printf 'SEARXNG_SECRET=%s\n' "$(openssl rand -hex 32)" > deploy/secrets/searxng.env) -sudo ls -la deploy/secrets # three files mealquest mealquest, searxng.env overseer overseer; all -rw------- +sudo ls -la deploy/secrets # four files mealquest mealquest, searxng.env overseer overseer; all -rw------- ``` The `mealquest` host user (uid/gid 10001) is created in "Media — nightly @@ -238,6 +243,38 @@ On 2026-10-05 discovery returned 200 with issuer `https://auth.cubecraftlabs.com/application/o/mealquest/` and `id_token_signing_alg_values_supported: ["RS256"]`. +## Google Calendar OAuth client + +Push to Calendar connects the household's Google account with a Google +OAuth web client. MealQuest calls Google's OAuth and Calendar REST +endpoints directly. **The service will not start without +`MEALQUEST_GOOGLE_CLIENT_ID` and `deploy/secrets/google_client_secret`**, +so create the client before deploying a release that contains Feature 16. + +In the Google Cloud console, in the project that holds MealQuest: + +| Object | Setting | +| --- | --- | +| API | Google Calendar API enabled | +| OAuth consent screen | User type External; app name MealQuest; publishing status **In production** (a client left in Testing gets refresh tokens that expire after 7 days, so Push to Calendar would stop working a week after connecting) | +| Scopes | `https://www.googleapis.com/auth/calendar.events` and `https://www.googleapis.com/auth/calendar.calendarlist.readonly` | +| Credentials | OAuth client ID, application type **Web application**, name `mealquest` | +| Authorized redirect URI | `https://mealquest.cubecraftlabs.com/api/v1/integrations/google/callback` (exactly; it is `MEALQUEST_PUBLIC_URL`'s origin plus that path) | +| Client ID | in `deploy/.env` as `MEALQUEST_GOOGLE_CLIENT_ID` (not a secret) | +| Client secret | in `deploy/secrets/google_client_secret` only | + +The app is unverified, so Google shows an "unverified app" warning on the +consent screen; choose to continue. That is expected for a single-household +deployment. + +To connect, a signed-in household member opens +`https://mealquest.cubecraftlabs.com/api/v1/integrations/google/connect` +(the Settings screen links there once it ships). MealQuest asks for offline +access with consent always prompted, and lands back on +`/settings/calendar` once the refresh token is stored. Disconnecting +(`DELETE /api/v1/integrations/google`) revokes the token at Google and +forgets it. + ## Reverse proxy (Traefik) MealQuest is published at by the @@ -1313,6 +1350,23 @@ Sign in at to confirm; sign-ins fail between regenerating the secret in Authentik and recreating the service. +### Google OAuth client secret (`deploy/secrets/google_client_secret`) + +In the Google Cloud console, open the OAuth client `mealquest`, add a new +client secret, write it to the file (same owner and mode as above) and +recreate the service: + +```sh +printf '%s\n' '' | sudo tee deploy/secrets/google_client_secret >/dev/null +sudo chown 10001:10001 deploy/secrets/google_client_secret +sudo chmod 600 deploy/secrets/google_client_secret +docker compose -f deploy/compose.yaml up -d --force-recreate mealquest +``` + +Then disable and delete the old secret in the console. A new client secret +leaves the household's stored refresh token valid, so nothing needs +reconnecting. + ### LLM API key Stored encrypted in the database, not in `deploy/`. In the app, open diff --git a/deploy/compose.yaml b/deploy/compose.yaml index 4fe042a..f54b159 100644 --- a/deploy/compose.yaml +++ b/deploy/compose.yaml @@ -32,10 +32,12 @@ services: MEALQUEST_DATA_DIR: /data MEALQUEST_SECRET_KEY_FILE: /run/secrets/secret_key MEALQUEST_OIDC_CLIENT_SECRET_FILE: /run/secrets/oidc_client_secret + MEALQUEST_GOOGLE_CLIENT_SECRET_FILE: /run/secrets/google_client_secret MEALQUEST_SEARXNG_URL: http://searxng:8080 secrets: - secret_key - oidc_client_secret + - google_client_secret volumes: - mealquest-data:/data ports: @@ -128,6 +130,8 @@ secrets: file: ./secrets/secret_key oidc_client_secret: file: ./secrets/oidc_client_secret + google_client_secret: + file: ./secrets/google_client_secret volumes: mealquest-data: diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3d9cd7d..4df3f76 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -618,6 +618,28 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. not come back. - Connected once with Google OAuth in Settings; one event per planned dinner. +- `internal/gcal` owns the connection, as a confidential OAuth web client + (`MEALQUEST_GOOGLE_CLIENT_ID`, `MEALQUEST_GOOGLE_CLIENT_SECRET_FILE`, both + required at boot) over plain REST — no Google SDK. All three routes are + under the signed-in `/api/v1`: + - `GET /api/v1/integrations/google/connect` redirects to Google's consent + screen with scopes `calendar.events` and `calendar.calendarlist.readonly`, + `access_type=offline`, `prompt=consent` and PKCE. Its `state` travels in + the signed `__Host-mealquest_google` flow cookie (the same + `auth.FlowCookie` mechanism as the login cookie), bound to the session by + a hash of the session's CSRF token. + - `GET /api/v1/integrations/google/callback` (the redirect URI: + `MEALQUEST_PUBLIC_URL`'s origin plus this path) checks the state and + session, exchanges the code, refuses a response with no refresh token + (`reconnect_required`) or without both scopes, seals the refresh token + with `secretbox` into `integrations_google.refresh_token_ct`, sets + `connected_at` and redirects to `/settings/calendar`. A reconnect keeps + the chosen calendar and clears `last_sync_error`. + - `DELETE /api/v1/integrations/google` (CSRF-checked) revokes the token at + Google, then deletes the row; a token Google reports as `invalid_token` + counts as revoked, and any other revoke failure keeps the connection. + - No response, log line or error ever holds a token or Google's response + text. - **Deterministic event ids**; each plan day records its `calendar_event_id`. diff --git a/internal/auth/auth.go b/internal/auth/auth.go index d227ad8..2000308 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -107,7 +107,7 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger return nil, err } - client := &http.Client{Timeout: providerTimeout, CheckRedirect: checkRedirect} + client := &http.Client{Timeout: providerTimeout, CheckRedirect: CheckRedirect} discovery := *client recorder := &statusRecorder{next: http.DefaultTransport} discovery.Transport = recorder @@ -205,11 +205,12 @@ func checkEndpoint(name, raw string) error { // maxRedirects matches net/http's default redirect limit. const maxRedirects = 10 -// checkRedirect is the provider client's redirect policy. On a 307 or 308 to -// the same host Go resends the body and Authorization header, which carry the -// code and client secret, so every target must pass secureurl.Check and an -// https:// request never follows a redirect to plain http://, even loopback. -func checkRedirect(req *http.Request, via []*http.Request) error { +// CheckRedirect is the redirect policy of every client that talks to an +// OAuth provider. On a 307 or 308 to the same host Go resends the body and +// Authorization header, which carry the code and client secret, so every +// target must pass secureurl.Check and an https:// request never follows a +// redirect to plain http://, even loopback. +func CheckRedirect(req *http.Request, via []*http.Request) error { if len(via) >= maxRedirects { return fmt.Errorf("stopped after %d redirects", maxRedirects) } @@ -350,7 +351,7 @@ func (a *Auth) verify(ctx context.Context, code string, state loginState) (Claim token, err := a.oauth.Exchange(context.WithValue(ctx, oauth2.HTTPClient, a.client), code, oauth2.VerifierOption(state.Verifier)) if err != nil { - return Claims{}, exchangeError(err) + return Claims{}, ExchangeError(err) } raw, ok := token.Extra("id_token").(string) if !ok || raw == "" { @@ -400,7 +401,7 @@ func (a *Auth) userinfoGroups(ctx context.Context, token *oauth2.Token, subject } // userinfoError reduces a failed userinfo request to a fixed category, like -// exchangeError: go-oidc's error text can carry the response body. +// ExchangeError: go-oidc's error text can carry the response body. func userinfoError(err error) error { var transport *url.Error if errors.As(err, &transport) { @@ -425,10 +426,11 @@ func (a *Auth) member(groups []string) bool { return slices.ContainsFunc(groups, func(g string) bool { return slices.Contains(a.allowed, g) }) } -// exchangeError reduces a failed code exchange to a fixed category and safe -// metadata. The oauth2 error text can carry the token endpoint's response -// body or error_description, which may echo credentials, so it is dropped. -func exchangeError(err error) error { +// ExchangeError reduces a failed OAuth code exchange to a fixed category and +// safe metadata. The oauth2 error text can carry the token endpoint's +// response body or error_description, which may echo credentials, so it is +// dropped. +func ExchangeError(err error) error { var retrieve *oauth2.RetrieveError var transport *url.Error switch { diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 407bbb8..af9f53e 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -1060,7 +1060,7 @@ func TestCheckRedirect(t *testing.T) { {"https://auth.example.com/token", "ftp://auth.example.com/token", false}, } for _, tc := range tests { - err := checkRedirect(request(tc.to), []*http.Request{request(tc.from)}) + err := CheckRedirect(request(tc.to), []*http.Request{request(tc.from)}) if (err == nil) != tc.ok { t.Errorf("redirect %s -> %s: error = %v, want ok=%v", tc.from, tc.to, err, tc.ok) } @@ -1069,7 +1069,7 @@ func TestCheckRedirect(t *testing.T) { for i := range via { via[i] = request("https://auth.example.com/token") } - if err := checkRedirect(request("https://auth.example.com/token"), via); err == nil { + if err := CheckRedirect(request("https://auth.example.com/token"), via); err == nil { t.Errorf("redirect after %d hops: error = nil, want limit", maxRedirects) } } diff --git a/internal/auth/cookie.go b/internal/auth/cookie.go index 4be2d7d..722c0aa 100644 --- a/internal/auth/cookie.go +++ b/internal/auth/cookie.go @@ -27,38 +27,86 @@ type loginState struct { Nonce string `json:"n"` Verifier string `json:"v"` Return string `json:"r"` - Expires int64 `json:"e"` // Unix seconds + FlowExpiry } -// loginCookie signs and verifies the login cookie with HMAC-SHA256. +// loginCookie is the FlowCookie carrying loginState. type loginCookie struct { - key []byte + flow FlowCookie } func newLoginCookie(secretKey []byte) loginCookie { - mac := hmac.New(sha256.New, secretKey) - mac.Write([]byte(loginCookieKeyLabel)) - return loginCookie{key: mac.Sum(nil)} + return loginCookie{flow: NewFlowCookie(loginCookieName, loginCookieKeyLabel, loginCookieTTL, secretKey)} } -func (c loginCookie) sign(payload string) string { +func (c loginCookie) set(w http.ResponseWriter, s loginState, now time.Time) error { + return c.flow.Set(w, &s, now) +} + +func (c loginCookie) clear(w http.ResponseWriter) { c.flow.Clear(w) } + +func (c loginCookie) read(r *http.Request, now time.Time) (loginState, error) { + var s loginState + if err := c.flow.Read(r, &s, now); err != nil { + return loginState{}, err + } + return s, nil +} + +// FlowExpiry is embedded in every value a FlowCookie carries; Set fills it +// and Read enforces it. +type FlowExpiry struct { + Expires int64 `json:"e"` // Unix seconds +} + +func (e *FlowExpiry) flowExpiry() *FlowExpiry { return e } + +// flowValue is a pointer to a struct that embeds FlowExpiry. +type flowValue interface { + flowExpiry() *FlowExpiry +} + +// FlowCookie carries a redirect flow's state — the OAuth state, PKCE +// verifier and the like — from the request that sends the browser to a +// provider to the callback it returns to. The value is JSON, signed with +// HMAC-SHA256 under a key derived from the server secret and the cookie's +// own label, and expires after a fixed time. It is not encrypted: it must +// not hold anything the browser may not see. +type FlowCookie struct { + name string + ttl time.Duration + key []byte +} + +// NewFlowCookie returns the flow cookie name, valid for ttl, signed under a +// key derived from secretKey and keyLabel. Each flow uses its own keyLabel so +// one flow's cookie never verifies as another's. name must carry the __Host- +// prefix: the cookie is Secure, host-only and Path=/. +func NewFlowCookie(name, keyLabel string, ttl time.Duration, secretKey []byte) FlowCookie { + mac := hmac.New(sha256.New, secretKey) + mac.Write([]byte(keyLabel)) + return FlowCookie{name: name, ttl: ttl, key: mac.Sum(nil)} +} + +func (c FlowCookie) sign(payload string) string { mac := hmac.New(sha256.New, c.key) mac.Write([]byte(payload)) return base64.RawURLEncoding.EncodeToString(mac.Sum(nil)) } -func (c loginCookie) set(w http.ResponseWriter, s loginState, now time.Time) error { - s.Expires = now.Add(loginCookieTTL).Unix() - data, err := json.Marshal(s) +// Set stores v, which expires ttl after now, in the cookie. +func (c FlowCookie) Set(w http.ResponseWriter, v flowValue, now time.Time) error { + v.flowExpiry().Expires = now.Add(c.ttl).Unix() + data, err := json.Marshal(v) if err != nil { return err } payload := base64.RawURLEncoding.EncodeToString(data) http.SetCookie(w, &http.Cookie{ - Name: loginCookieName, + Name: c.name, Value: payload + "." + c.sign(payload), Path: "/", - MaxAge: int(loginCookieTTL / time.Second), + MaxAge: int(c.ttl / time.Second), Secure: true, HttpOnly: true, // Lax so the cookie rides the provider's top-level redirect back. @@ -67,9 +115,11 @@ func (c loginCookie) set(w http.ResponseWriter, s loginState, now time.Time) err return nil } -func (c loginCookie) clear(w http.ResponseWriter) { +// Clear removes the cookie. A callback clears it before reading, so it is +// single-use. +func (c FlowCookie) Clear(w http.ResponseWriter) { http.SetCookie(w, &http.Cookie{ - Name: loginCookieName, + Name: c.name, Path: "/", MaxAge: -1, Secure: true, @@ -78,27 +128,28 @@ func (c loginCookie) clear(w http.ResponseWriter) { }) } -func (c loginCookie) read(r *http.Request, now time.Time) (loginState, error) { - cookie, err := r.Cookie(loginCookieName) +// Read verifies the cookie r carries and decodes it into v. It fails when +// the cookie is missing, its signature is invalid or it has expired at now. +func (c FlowCookie) Read(r *http.Request, v flowValue, now time.Time) error { + cookie, err := r.Cookie(c.name) if err != nil { - return loginState{}, errors.New("login cookie missing") + return fmt.Errorf("%s cookie missing", c.name) } payload, sig, ok := strings.Cut(cookie.Value, ".") if !ok || !hmac.Equal([]byte(sig), []byte(c.sign(payload))) { - return loginState{}, errors.New("login cookie signature invalid") + return fmt.Errorf("%s cookie signature invalid", c.name) } data, err := base64.RawURLEncoding.DecodeString(payload) if err != nil { - return loginState{}, fmt.Errorf("login cookie encoding: %w", err) + return fmt.Errorf("%s cookie encoding: %w", c.name, err) } - var s loginState - if err := json.Unmarshal(data, &s); err != nil { - return loginState{}, fmt.Errorf("login cookie payload: %w", err) + if err := json.Unmarshal(data, v); err != nil { + return fmt.Errorf("%s cookie payload: %w", c.name, err) } - if now.Unix() >= s.Expires { - return loginState{}, errors.New("login cookie expired") + if now.Unix() >= v.flowExpiry().Expires { + return fmt.Errorf("%s cookie expired", c.name) } - return s, nil + return nil } // returnPath validates the optional post-login return path. Empty means "/". diff --git a/internal/config/config.go b/internal/config/config.go index c838255..ff2ecce 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -29,6 +29,9 @@ const ( EnvOIDCClientSecretFile = "MEALQUEST_OIDC_CLIENT_SECRET_FILE" EnvOIDCAllowedGroups = "MEALQUEST_OIDC_ALLOWED_GROUPS" + EnvGoogleClientID = "MEALQUEST_GOOGLE_CLIENT_ID" + EnvGoogleClientSecretFile = "MEALQUEST_GOOGLE_CLIENT_SECRET_FILE" + EnvSessionIdle = "MEALQUEST_SESSION_IDLE" EnvSessionMax = "MEALQUEST_SESSION_MAX" @@ -61,6 +64,12 @@ type Config struct { // OIDCAllowedGroups lists the groups whose members may sign in. OIDCAllowedGroups []string + // GoogleClientID and GoogleClientSecret are the Google OAuth web client + // Push to Calendar connects with. Its redirect URI is PublicURL's origin + // followed by /api/v1/integrations/google/callback. + GoogleClientID string + GoogleClientSecret Secret + // SessionIdle ends a session left unused this long. SessionIdle time.Duration // SessionMax ends every session this long after sign-in, used or not. @@ -110,6 +119,9 @@ func load(lookup func(string) (string, bool)) (Config, error) { OIDCClientSecret: r.secretFile(EnvOIDCClientSecretFile), OIDCAllowedGroups: r.list(EnvOIDCAllowedGroups), + GoogleClientID: r.str(EnvGoogleClientID), + GoogleClientSecret: r.secretFile(EnvGoogleClientSecretFile), + SessionIdle: r.sessionDuration(EnvSessionIdle, DefaultSessionIdle), SessionMax: r.sessionDuration(EnvSessionMax, DefaultSessionMax), diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 91e24c7..297bac6 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -29,6 +29,10 @@ func TestLoad(t *testing.T) { if err := os.WriteFile(clientSecretPath, []byte("client-s3cret\n"), 0o600); err != nil { t.Fatal(err) } + googleSecretPath := filepath.Join(dir, "google_client_secret") + if err := os.WriteFile(googleSecretPath, []byte("google-s3cret\n"), 0o600); err != nil { + t.Fatal(err) + } emptyPath := filepath.Join(dir, "empty_secret") if err := os.WriteFile(emptyPath, []byte("\n"), 0o600); err != nil { t.Fatal(err) @@ -46,6 +50,9 @@ func TestLoad(t *testing.T) { EnvOIDCClientID: "mealquest", EnvOIDCClientSecretFile: clientSecretPath, EnvOIDCAllowedGroups: "mealquest-admin", + + EnvGoogleClientID: "mealquest.apps.googleusercontent.com", + EnvGoogleClientSecretFile: googleSecretPath, } } with := func(changes map[string]string, unset ...string) map[string]string { @@ -94,6 +101,7 @@ func TestLoad(t *testing.T) { badVars: []string{ EnvHTTPAddr, EnvPublicURL, EnvDataDir, EnvShutdownTimeout, EnvSecretKeyFile, EnvOIDCIssuer, EnvOIDCClientID, EnvOIDCClientSecretFile, EnvOIDCAllowedGroups, + EnvGoogleClientID, EnvGoogleClientSecretFile, }, }, { @@ -211,6 +219,31 @@ func TestLoad(t *testing.T) { env: with(map[string]string{"MEALQUEST_SECRET_KEY": "inline"}), badVars: []string{"MEALQUEST_SECRET_KEY"}, }, + { + name: "Google client ID missing", + env: with(nil, EnvGoogleClientID), + badVars: []string{EnvGoogleClientID}, + }, + { + name: "Google client secret file missing", + env: with(nil, EnvGoogleClientSecretFile), + badVars: []string{EnvGoogleClientSecretFile}, + }, + { + name: "Google client secret file unreadable", + env: with(map[string]string{EnvGoogleClientSecretFile: unreadablePath}), + badVars: []string{EnvGoogleClientSecretFile}, + }, + { + name: "Google client secret file empty", + env: with(map[string]string{EnvGoogleClientSecretFile: emptyPath}), + badVars: []string{EnvGoogleClientSecretFile}, + }, + { + name: "inline Google client secret rejected", + env: with(map[string]string{"MEALQUEST_GOOGLE_CLIENT_SECRET": "inline"}), + badVars: []string{"MEALQUEST_GOOGLE_CLIENT_SECRET"}, + }, { name: "inline OIDC client secret rejected", env: with(map[string]string{"MEALQUEST_OIDC_CLIENT_SECRET": "inline"}), @@ -409,6 +442,10 @@ func TestLoadValues(t *testing.T) { if err := os.WriteFile(clientSecretPath, []byte("client-s3cret\n"), 0o600); err != nil { t.Fatal(err) } + googleSecretPath := filepath.Join(dir, "google_client_secret") + if err := os.WriteFile(googleSecretPath, []byte("google-s3cret\n"), 0o600); err != nil { + t.Fatal(err) + } env := map[string]string{ EnvHTTPAddr: "127.0.0.1:9000", EnvPublicURL: "http://localhost:9000", @@ -421,6 +458,9 @@ func TestLoadValues(t *testing.T) { EnvOIDCClientSecretFile: clientSecretPath, EnvOIDCAllowedGroups: " mealquest-admin ,family", + EnvGoogleClientID: "mealquest.apps.googleusercontent.com", + EnvGoogleClientSecretFile: googleSecretPath, + EnvDefaultTZ: "Europe/London", EnvSearXNGURL: "http://searxng:8080", @@ -454,6 +494,12 @@ func TestLoadValues(t *testing.T) { if cfg.OIDCClientSecret.Reveal() != "client-s3cret" { t.Errorf("OIDCClientSecret.Reveal() = %q", cfg.OIDCClientSecret.Reveal()) } + if cfg.GoogleClientID != "mealquest.apps.googleusercontent.com" { + t.Errorf("GoogleClientID = %q", cfg.GoogleClientID) + } + if cfg.GoogleClientSecret.Reveal() != "google-s3cret" { + t.Errorf("GoogleClientSecret.Reveal() = %q", cfg.GoogleClientSecret.Reveal()) + } if want := []string{"mealquest-admin", "family"}; !slices.Equal(cfg.OIDCAllowedGroups, want) { t.Errorf("OIDCAllowedGroups = %q, want %q", cfg.OIDCAllowedGroups, want) } @@ -492,7 +538,7 @@ func TestSecretRedacted(t *testing.T) { s := Secret{value: "s3cret-value"} for _, format := range []string{"%v", "%s", "%+v", "%#v"} { - cfg := Config{SecretKey: s, OIDCClientSecret: s} + cfg := Config{SecretKey: s, OIDCClientSecret: s, GoogleClientSecret: s} for _, out := range []string{fmt.Sprintf(format, s), fmt.Sprintf(format, cfg)} { if strings.Contains(out, "s3cret-value") { t.Errorf("format %s leaked secret: %s", format, out) diff --git a/internal/db/querier.go b/internal/db/querier.go index 88053ef..dd515f8 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -50,6 +50,10 @@ type Querier interface { // decided is left as it is (0 rows). DecideSuggestion(ctx context.Context, arg DecideSuggestionParams) (int64, error) DeleteAIRunsBefore(ctx context.Context, cutoff string) (int64, error) + // Deletes the row only while it still holds refresh_token_ct, so a + // connection made after the caller read the row is kept. Returns the number + // of rows deleted. + DeleteIntegrationsGoogle(ctx context.Context, refreshTokenCt []byte) (int64, error) DeleteRecipeIngredients(ctx context.Context, recipeID string) error DeleteRecipeSteps(ctx context.Context, recipeID string) error FailJob(ctx context.Context, arg FailJobParams) (int64, error) diff --git a/internal/db/settings.sql.go b/internal/db/settings.sql.go index 50a036e..835cc8e 100644 --- a/internal/db/settings.sql.go +++ b/internal/db/settings.sql.go @@ -10,6 +10,22 @@ import ( "database/sql" ) +const deleteIntegrationsGoogle = `-- name: DeleteIntegrationsGoogle :execrows +DELETE FROM integrations_google +WHERE id = 1 AND refresh_token_ct = ? +` + +// Deletes the row only while it still holds refresh_token_ct, so a +// connection made after the caller read the row is kept. Returns the number +// of rows deleted. +func (q *Queries) DeleteIntegrationsGoogle(ctx context.Context, refreshTokenCt []byte) (int64, error) { + result, err := q.db.ExecContext(ctx, deleteIntegrationsGoogle, refreshTokenCt) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + const getAISettings = `-- name: GetAISettings :one SELECT id, provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at, created_at, updated_at FROM ai_settings WHERE id = 1 diff --git a/internal/gcal/connect.go b/internal/gcal/connect.go new file mode 100644 index 0000000..01b4ede --- /dev/null +++ b/internal/gcal/connect.go @@ -0,0 +1,357 @@ +// Package gcal connects the household's Google account for Push to Calendar. +// The service is a confidential OAuth web client: the authorization code, +// tokens and client secret stay on the server. The refresh token is stored +// sealed by secretbox in integrations_google and is never logged or +// returned. +package gcal + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "database/sql" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "slices" + "strings" + "time" + + "golang.org/x/oauth2" + "golang.org/x/oauth2/endpoints" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// CallbackPath is where Google returns the browser after consent. It is +// served under the signed-in /api/v1 router, so the redirect URI registered +// with Google is MEALQUEST_PUBLIC_URL's origin followed by this path. +const CallbackPath = "/api/v1/integrations/google/callback" + +// Scopes requested at connect: create and change the events MealQuest pushes, +// and list the calendars the household can push to. +var Scopes = []string{ + "https://www.googleapis.com/auth/calendar.events", + "https://www.googleapis.com/auth/calendar.calendarlist.readonly", +} + +// GoogleRevokeURL is Google's OAuth token revocation endpoint. +const GoogleRevokeURL = "https://oauth2.googleapis.com/revoke" + +// GoogleEndpoint is Google's OAuth authorization and token endpoint. +var GoogleEndpoint = oauth2.Endpoint{ + AuthURL: endpoints.Google.AuthURL, + TokenURL: endpoints.Google.TokenURL, + AuthStyle: oauth2.AuthStyleInParams, +} + +const ( + connectCookieName = "__Host-mealquest_google" + connectCookieTTL = 10 * time.Minute + // connectCookieKeyLabel separates the connect cookie's signing key from + // the login cookie's and every other key derived from the server secret. + connectCookieKeyLabel = "mealquest google connect cookie v1" + // googleTimeout bounds every request to Google. + googleTimeout = 10 * time.Second + // maxRevokeBody caps how much of a revoke response is read. + maxRevokeBody = 4 << 10 +) + +var ( + // ErrBadState means the callback's state is missing, does not match the + // connect cookie, or the cookie is missing, expired or belongs to another + // session. + ErrBadState = errors.New("gcal: connect state is missing, expired or does not match this session") + // ErrDenied means Google returned an authorization error, usually because + // the household declined consent. + ErrDenied = errors.New("gcal: Google did not grant access") + // ErrNoCode means the callback carries no authorization code. + ErrNoCode = errors.New("gcal: callback has no authorization code") + // ErrNoRefreshToken means Google granted access without a refresh token, + // so MealQuest could not push later; the household must connect again. + ErrNoRefreshToken = errors.New("gcal: Google returned no refresh token") + // ErrScopes means the household did not grant every scope in Scopes. + ErrScopes = errors.New("gcal: Google did not grant every Calendar permission") + // ErrNotConnected means no Google account is connected. + ErrNotConnected = errors.New("gcal: Google Calendar is not connected") + // ErrChanged means the connection changed while it was being + // disconnected. + ErrChanged = errors.New("gcal: Google Calendar connection changed during disconnect") +) + +// Config is what NewService needs to talk to Google. +type Config struct { + ClientID string + ClientSecret string + // PublicURL is the service's public URL; its origin prefixes CallbackPath + // to form the redirect URI. + PublicURL *url.URL + // SecretKey is the server key the connect cookie's signing key derives + // from. + SecretKey []byte + // Endpoint is Google's authorization and token endpoint; production uses + // GoogleEndpoint and tests a fake. + Endpoint oauth2.Endpoint + // RevokeURL is Google's revocation endpoint; production uses + // GoogleRevokeURL and tests a fake. + RevokeURL string +} + +// Service connects and disconnects the household's Google account. +type Service struct { + st *store.Store + box *secretbox.Box + oauth oauth2.Config + revokeURL string + client *http.Client + cookie auth.FlowCookie + // now is the clock; tests replace it. + now func() time.Time +} + +// NewService returns the service over st, sealing refresh tokens with box. +func NewService(st *store.Store, box *secretbox.Box, cfg Config) (*Service, error) { + switch { + case st == nil: + return nil, errors.New("gcal: nil store") + case box == nil: + return nil, errors.New("gcal: nil secret box") + case cfg.ClientID == "", cfg.ClientSecret == "": + return nil, errors.New("gcal: client ID and client secret are required") + case cfg.PublicURL == nil: + return nil, errors.New("gcal: public URL is required") + case len(cfg.SecretKey) == 0: + return nil, errors.New("gcal: secret key is required") + case cfg.Endpoint.AuthURL == "", cfg.Endpoint.TokenURL == "", cfg.RevokeURL == "": + return nil, errors.New("gcal: authorization, token and revoke endpoints are required") + } + return &Service{ + st: st, + box: box, + oauth: oauth2.Config{ + ClientID: cfg.ClientID, + ClientSecret: cfg.ClientSecret, + Endpoint: cfg.Endpoint, + RedirectURL: cfg.PublicURL.Scheme + "://" + cfg.PublicURL.Host + CallbackPath, + Scopes: slices.Clone(Scopes), + }, + revokeURL: cfg.RevokeURL, + client: &http.Client{Timeout: googleTimeout, CheckRedirect: auth.CheckRedirect}, + cookie: auth.NewFlowCookie(connectCookieName, connectCookieKeyLabel, connectCookieTTL, cfg.SecretKey), + now: time.Now, + }, nil +} + +// connectState travels from Connect to Complete in the signed connect +// cookie. +type connectState struct { + State string `json:"s"` + Verifier string `json:"v"` + // Session binds the state to the session that started the connect: the + // SHA-256 of its CSRF token, which is unique to the session and never + // changes during it. + Session string `json:"b"` + auth.FlowExpiry +} + +// sessionBinding is the value connectState.Session must hold for s. +func sessionBinding(s auth.Session) string { + sum := sha256.Sum256([]byte(s.CSRFToken)) + return hex.EncodeToString(sum[:]) +} + +// Connect sets the connect cookie on w, bound to session, and returns +// Google's consent URL: offline access, consent always prompted so Google +// always issues a refresh token, and PKCE (S256). +func (s *Service) Connect(w http.ResponseWriter, session auth.Session) (string, error) { + if session.CSRFToken == "" { + return "", errors.New("gcal: connect needs a session") + } + state := connectState{ + State: rand.Text(), + Verifier: oauth2.GenerateVerifier(), + Session: sessionBinding(session), + } + if err := s.cookie.Set(w, &state, s.now()); err != nil { + return "", fmt.Errorf("gcal: set connect cookie: %w", err) + } + return s.oauth.AuthCodeURL(state.State, + oauth2.AccessTypeOffline, + oauth2.ApprovalForce, + oauth2.S256ChallengeOption(state.Verifier), + ), nil +} + +// Complete finishes the connect Google redirected r back with. It clears the +// connect cookie, checks the state against it and session, exchanges the +// code and stores the refresh token, sealed, with connected_at now. A +// reconnect replaces the stored token and keeps the chosen calendar. +func (s *Service) Complete(w http.ResponseWriter, r *http.Request, session auth.Session) error { + ctx := r.Context() + // The connect cookie is single-use. + s.cookie.Clear(w) + + var state connectState + if err := s.cookie.Read(r, &state, s.now()); err != nil { + return fmt.Errorf("%w: %v", ErrBadState, err) + } + q := r.URL.Query() + if subtle.ConstantTimeCompare([]byte(q.Get("state")), []byte(state.State)) != 1 { + return fmt.Errorf("%w: state mismatch", ErrBadState) + } + if subtle.ConstantTimeCompare([]byte(sessionBinding(session)), []byte(state.Session)) != 1 { + return fmt.Errorf("%w: started by another session", ErrBadState) + } + if q.Get("error") != "" { + // error and error_description are Google-supplied text; neither is + // kept. + return ErrDenied + } + code := q.Get("code") + if code == "" { + return ErrNoCode + } + + token, err := s.oauth.Exchange(context.WithValue(ctx, oauth2.HTTPClient, s.client), code, + oauth2.VerifierOption(state.Verifier)) + if err != nil { + return fmt.Errorf("gcal: %w", auth.ExchangeError(err)) + } + if token.RefreshToken == "" { + return ErrNoRefreshToken + } + if err := checkScopes(token); err != nil { + return err + } + sealed, err := s.box.Encrypt([]byte(token.RefreshToken)) + if err != nil { + return fmt.Errorf("gcal: seal refresh token: %w", err) + } + return s.store(ctx, sealed) +} + +// checkScopes fails with ErrScopes unless token's granted scopes include +// every scope in Scopes. Google lets the household untick a permission on +// the consent screen and reports what was granted in "scope". +func checkScopes(token *oauth2.Token) error { + granted, _ := token.Extra("scope").(string) + have := strings.Fields(granted) + for _, want := range Scopes { + if !slices.Contains(have, want) { + return ErrScopes + } + } + return nil +} + +// store saves sealed as the refresh token with connected_at now, clearing +// any earlier sync error. +func (s *Service) store(ctx context.Context, sealed []byte) error { + return s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + row, err := q.GetIntegrationsGoogle(ctx) + exists := err == nil + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("gcal: read Google integration: %w", err) + } + now := store.Timestamp{Time: s.now().UTC().Truncate(time.Millisecond)}.String() + created := now + if exists { + created = row.CreatedAt + } + if _, err := q.UpsertIntegrationsGoogle(ctx, db.UpsertIntegrationsGoogleParams{ + RefreshTokenCt: sealed, + CalendarID: row.CalendarID, + CalendarName: row.CalendarName, + ConnectedAt: now, + CreatedAt: created, + UpdatedAt: now, + }); err != nil { + return fmt.Errorf("gcal: save Google integration: %w", err) + } + return nil + }) +} + +// Disconnect revokes the stored refresh token at Google, then deletes the +// connection. A token Google no longer knows (the household already removed +// MealQuest's access in their Google account) counts as revoked. Any other +// revoke failure keeps the connection so the household can retry. +func (s *Service) Disconnect(ctx context.Context) error { + row, err := db.New(s.st.Reader).GetIntegrationsGoogle(ctx) + if errors.Is(err, sql.ErrNoRows) { + return ErrNotConnected + } + if err != nil { + return fmt.Errorf("gcal: read Google integration: %w", err) + } + token, err := s.box.Decrypt(row.RefreshTokenCt) + if err != nil { + return fmt.Errorf("gcal: open refresh token: %w", err) + } + if err := s.revoke(ctx, string(token)); err != nil { + return err + } + n, err := db.New(s.st.Writer).DeleteIntegrationsGoogle(ctx, row.RefreshTokenCt) + if err != nil { + return fmt.Errorf("gcal: delete Google integration: %w", err) + } + if n == 0 { + return ErrChanged + } + return nil +} + +// RevokeError is a revoke Google refused or that failed in transit. Its text +// never holds the token or Google's response body. +type RevokeError struct { + msg string +} + +func (e *RevokeError) Error() string { return e.msg } + +// revoke revokes token at Google. HTTP 200 and HTTP 400 invalid_token (the +// token is already revoked or expired) both leave it unusable. +func (s *Service) revoke(ctx context.Context, token string) error { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, s.revokeURL, + strings.NewReader(url.Values{"token": {token}}.Encode())) + if err != nil { + return fmt.Errorf("gcal: build revoke request: %w", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + resp, err := s.client.Do(req) + if err != nil { + var transport *url.Error + if errors.As(err, &transport) { + // The URL is dropped: after a refused redirect it is the + // Google-supplied Location. + return &RevokeError{msg: fmt.Sprintf("gcal: revoke request failed: %s: %v", transport.Op, transport.Err)} + } + return &RevokeError{msg: "gcal: revoke request failed"} + } + defer resp.Body.Close() + body, err := io.ReadAll(io.LimitReader(resp.Body, maxRevokeBody)) + if err != nil { + return &RevokeError{msg: "gcal: read revoke response failed"} + } + if resp.StatusCode == http.StatusOK { + return nil + } + if resp.StatusCode == http.StatusBadRequest { + var reply struct { + Error string `json:"error"` + } + if json.Unmarshal(body, &reply) == nil && reply.Error == "invalid_token" { + return nil + } + } + return &RevokeError{msg: fmt.Sprintf("gcal: Google refused the revoke (HTTP %d)", resp.StatusCode)} +} diff --git a/internal/gcal/connect_test.go b/internal/gcal/connect_test.go new file mode 100644 index 0000000..aa3f817 --- /dev/null +++ b/internal/gcal/connect_test.go @@ -0,0 +1,476 @@ +package gcal_test + +import ( + "bytes" + "context" + "crypto/rand" + "database/sql" + "encoding/base64" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "slices" + "strings" + "testing" + "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal/gcaltest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +const publicURL = "https://meals.example.com/app" + +var ( + redirectURL = "https://meals.example.com" + gcal.CallbackPath + session = auth.Session{Subject: "user-1", CSRFToken: "csrf-session-one"} + other = auth.Session{Subject: "user-1", CSRFToken: "csrf-session-two"} +) + +type harness struct { + svc *gcal.Service + google *gcaltest.Google + st *store.Store + box *secretbox.Box + clock time.Time +} + +func newHarness(t *testing.T) *harness { + t.Helper() + ctx := context.Background() + st, err := store.Open(ctx, t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := st.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(ctx, st.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + key := make([]byte, secretbox.KeySize) + rand.Read(key) + box, err := secretbox.New(base64.StdEncoding.EncodeToString(key)) + if err != nil { + t.Fatal(err) + } + pub, err := url.Parse(publicURL) + if err != nil { + t.Fatal(err) + } + google := gcaltest.New(t, redirectURL) + svc, err := gcal.NewService(st, box, gcal.Config{ + ClientID: gcaltest.ClientID, + ClientSecret: gcaltest.ClientSecret, + PublicURL: pub, + SecretKey: []byte("server-secret-key"), + Endpoint: google.Endpoint(), + RevokeURL: google.RevokeURL(), + }) + if err != nil { + t.Fatal(err) + } + h := &harness{svc: svc, google: google, st: st, box: box, clock: time.Date(2026, 10, 7, 18, 30, 0, 0, time.UTC)} + gcal.SetClock(svc, func() time.Time { return h.clock }) + return h +} + +// connect is a started connect: Google's consent URL and the connect +// cookie. +type connect struct { + consent *url.URL + cookie *http.Cookie +} + +func (h *harness) connect(t *testing.T, s auth.Session) connect { + t.Helper() + rec := httptest.NewRecorder() + raw, err := h.svc.Connect(rec, s) + if err != nil { + t.Fatal(err) + } + consent, err := url.Parse(raw) + if err != nil { + t.Fatal(err) + } + cookies := rec.Result().Cookies() + if len(cookies) != 1 { + t.Fatalf("Connect set %d cookies, want 1", len(cookies)) + } + return connect{consent: consent, cookie: cookies[0]} +} + +// callback completes c as session s with query, returning the error and the +// recorder Complete wrote to. +func (h *harness) callback(c connect, s auth.Session, query url.Values) (*httptest.ResponseRecorder, error) { + req := httptest.NewRequest(http.MethodGet, gcal.CallbackPath+"?"+query.Encode(), nil) + if c.cookie != nil { + req.AddCookie(c.cookie) + } + rec := httptest.NewRecorder() + return rec, h.svc.Complete(rec, req, s) +} + +// consented is the query Google sends back after consent to c. +func (h *harness) consented(c connect) url.Values { + q := c.consent.Query() + return url.Values{"state": {q.Get("state")}, "code": {h.google.Authorize(q.Get("code_challenge"))}} +} + +func (h *harness) row(t *testing.T) (db.IntegrationsGoogle, bool) { + t.Helper() + row, err := db.New(h.st.Reader).GetIntegrationsGoogle(context.Background()) + if errors.Is(err, sql.ErrNoRows) { + return db.IntegrationsGoogle{}, false + } + if err != nil { + t.Fatal(err) + } + return row, true +} + +func TestConnectRedirectsToConsent(t *testing.T) { + t.Parallel() + h := newHarness(t) + c := h.connect(t, session) + + if got, want := c.consent.Scheme+"://"+c.consent.Host+c.consent.Path, h.google.Endpoint().AuthURL; got != want { + t.Errorf("consent URL = %s, want %s", got, want) + } + q := c.consent.Query() + for name, want := range map[string]string{ + "client_id": gcaltest.ClientID, + "redirect_uri": redirectURL, + "response_type": "code", + "access_type": "offline", + "prompt": "consent", + "code_challenge_method": "S256", + } { + if got := q.Get(name); got != want { + t.Errorf("%s = %q, want %q", name, got, want) + } + } + if got := strings.Fields(q.Get("scope")); !slices.Equal(got, []string{ + "https://www.googleapis.com/auth/calendar.events", + "https://www.googleapis.com/auth/calendar.calendarlist.readonly", + }) { + t.Errorf("scope = %q", q.Get("scope")) + } + if q.Get("state") == "" || q.Get("code_challenge") == "" { + t.Errorf("state %q, code_challenge %q: want both set", q.Get("state"), q.Get("code_challenge")) + } + if q.Get("client_secret") != "" || strings.Contains(c.consent.String(), gcaltest.ClientSecret) { + t.Error("consent URL carries the client secret") + } + + ck := c.cookie + if ck.Name != "__Host-mealquest_google" || ck.Path != "/" || !ck.Secure || !ck.HttpOnly || + ck.SameSite != http.SameSiteLaxMode || ck.MaxAge != 600 || ck.Domain != "" { + t.Errorf("connect cookie = %+v, want __Host-, Path=/, Secure, HttpOnly, Lax, 10 minutes", ck) + } + // Two connects never share a state. + if again := h.connect(t, session); again.consent.Query().Get("state") == q.Get("state") { + t.Error("two connects share a state") + } +} + +func TestCompleteStoresSealedRefreshToken(t *testing.T) { + t.Parallel() + h := newHarness(t) + c := h.connect(t, session) + rec, err := h.callback(c, session, h.consented(c)) + if err != nil { + t.Fatalf("Complete: %v", err) + } + + row, ok := h.row(t) + if !ok { + t.Fatal("no integration stored") + } + if bytes.Contains(row.RefreshTokenCt, []byte(gcaltest.RefreshToken)) { + t.Error("refresh token stored in plain text") + } + plain, err := h.box.Decrypt(row.RefreshTokenCt) + if err != nil || string(plain) != gcaltest.RefreshToken { + t.Errorf("stored token opens to %q, %v; want the refresh token", plain, err) + } + if want := "2026-10-07T18:30:00.000Z"; row.ConnectedAt != want || row.CreatedAt != want || row.UpdatedAt != want { + t.Errorf("connected_at %q created_at %q updated_at %q, want %s", row.ConnectedAt, row.CreatedAt, row.UpdatedAt, want) + } + if row.CalendarID.Valid || row.CalendarName.Valid || row.LastSyncError.Valid { + t.Errorf("new connection has calendar or sync error: %+v", row) + } + assertCookieCleared(t, rec) + assertNoSecret(t, "response", rec.Body.String()+headerText(rec.Header())) +} + +func TestReconnectReplacesTokenAndKeepsCalendar(t *testing.T) { + t.Parallel() + h := newHarness(t) + ctx := context.Background() + if _, err := db.New(h.st.Writer).UpsertIntegrationsGoogle(ctx, db.UpsertIntegrationsGoogleParams{ + RefreshTokenCt: []byte("old sealed token"), + CalendarID: sql.NullString{String: "family@group.calendar.google.com", Valid: true}, + CalendarName: sql.NullString{String: "Family", Valid: true}, + ConnectedAt: "2026-09-01T00:00:00.000Z", + LastSyncError: sql.NullString{String: "token revoked", Valid: true}, + CreatedAt: "2026-09-01T00:00:00.000Z", + UpdatedAt: "2026-09-01T00:00:00.000Z", + }); err != nil { + t.Fatal(err) + } + + c := h.connect(t, session) + if _, err := h.callback(c, session, h.consented(c)); err != nil { + t.Fatalf("Complete: %v", err) + } + row, _ := h.row(t) + if plain, err := h.box.Decrypt(row.RefreshTokenCt); err != nil || string(plain) != gcaltest.RefreshToken { + t.Errorf("token after reconnect opens to %q, %v", plain, err) + } + if row.CalendarID.String != "family@group.calendar.google.com" || row.CalendarName.String != "Family" { + t.Errorf("calendar after reconnect = %v %v, want it kept", row.CalendarID, row.CalendarName) + } + if row.LastSyncError.Valid { + t.Errorf("last_sync_error = %q, want it cleared", row.LastSyncError.String) + } + if row.CreatedAt != "2026-09-01T00:00:00.000Z" || row.ConnectedAt != "2026-10-07T18:30:00.000Z" { + t.Errorf("created_at %q connected_at %q", row.CreatedAt, row.ConnectedAt) + } +} + +func TestCompleteRefusesBadState(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + // edit changes the connect, the callback's session or its query. + edit func(t *testing.T, h *harness, c *connect, s *auth.Session, q url.Values) + }{ + {"missing state", func(_ *testing.T, _ *harness, _ *connect, _ *auth.Session, q url.Values) { q.Del("state") }}, + {"wrong state", func(_ *testing.T, _ *harness, _ *connect, _ *auth.Session, q url.Values) { q.Set("state", "forged") }}, + {"no connect cookie", func(_ *testing.T, _ *harness, c *connect, _ *auth.Session, _ url.Values) { c.cookie = nil }}, + {"tampered cookie", func(_ *testing.T, _ *harness, c *connect, _ *auth.Session, _ url.Values) { + c.cookie = &http.Cookie{Name: c.cookie.Name, Value: "x" + c.cookie.Value} + }}, + {"another session", func(_ *testing.T, _ *harness, _ *connect, s *auth.Session, _ url.Values) { *s = other }}, + {"expired", func(_ *testing.T, h *harness, _ *connect, _ *auth.Session, _ url.Values) { + h.clock = h.clock.Add(10 * time.Minute) + }}, + {"state of another connect", func(t *testing.T, h *harness, _ *connect, _ *auth.Session, q url.Values) { + q.Set("state", h.connect(t, session).consent.Query().Get("state")) + }}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h := newHarness(t) + c := h.connect(t, session) + s := session + q := h.consented(c) + tc.edit(t, h, &c, &s, q) + + rec, err := h.callback(c, s, q) + if !errors.Is(err, gcal.ErrBadState) { + t.Fatalf("Complete error = %v, want ErrBadState", err) + } + if h.google.TokenCalls() != 0 { + t.Error("the code was exchanged despite the bad state") + } + if _, ok := h.row(t); ok { + t.Error("an integration was stored") + } + assertCookieCleared(t, rec) + }) + } +} + +func TestCompleteRefusesMissingRefreshToken(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.google.OmitRefreshToken() + c := h.connect(t, session) + _, err := h.callback(c, session, h.consented(c)) + if !errors.Is(err, gcal.ErrNoRefreshToken) { + t.Fatalf("Complete error = %v, want ErrNoRefreshToken", err) + } + if _, ok := h.row(t); ok { + t.Error("an integration was stored without a refresh token") + } + assertNoSecret(t, "error", err.Error()) +} + +func TestCompleteRefusesMissingScope(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.google.GrantScope("https://www.googleapis.com/auth/calendar.calendarlist.readonly") + c := h.connect(t, session) + _, err := h.callback(c, session, h.consented(c)) + if !errors.Is(err, gcal.ErrScopes) { + t.Fatalf("Complete error = %v, want ErrScopes", err) + } + if _, ok := h.row(t); ok { + t.Error("an integration was stored without calendar.events") + } +} + +func TestCompleteDenied(t *testing.T) { + t.Parallel() + h := newHarness(t) + c := h.connect(t, session) + q := url.Values{"state": {c.consent.Query().Get("state")}, "error": {"access_denied"}} + if _, err := h.callback(c, session, q); !errors.Is(err, gcal.ErrDenied) { + t.Fatalf("Complete error = %v, want ErrDenied", err) + } + if h.google.TokenCalls() != 0 { + t.Error("token endpoint called after a denial") + } +} + +func TestCompleteExchangeFailureIsRedacted(t *testing.T) { + t.Parallel() + h := newHarness(t) + h.google.FailToken(http.StatusBadRequest) + c := h.connect(t, session) + _, err := h.callback(c, session, h.consented(c)) + if err == nil { + t.Fatal("Complete succeeded despite the token endpoint failing") + } + if !strings.Contains(err.Error(), "HTTP 400") { + t.Errorf("error = %v, want the status", err) + } + assertNoSecret(t, "error", err.Error()) + if _, ok := h.row(t); ok { + t.Error("an integration was stored") + } +} + +func TestDisconnectRevokesAndClears(t *testing.T) { + t.Parallel() + h := newHarness(t) + c := h.connect(t, session) + if _, err := h.callback(c, session, h.consented(c)); err != nil { + t.Fatal(err) + } + if err := h.svc.Disconnect(context.Background()); err != nil { + t.Fatalf("Disconnect: %v", err) + } + if got := h.google.Revoked(); !slices.Equal(got, []string{gcaltest.RefreshToken}) { + t.Errorf("revoked %q, want the refresh token", got) + } + if _, ok := h.row(t); ok { + t.Error("integration still stored after disconnect") + } + if err := h.svc.Disconnect(context.Background()); !errors.Is(err, gcal.ErrNotConnected) { + t.Errorf("second Disconnect = %v, want ErrNotConnected", err) + } +} + +func TestDisconnectTokenAlreadyRevokedAtGoogle(t *testing.T) { + t.Parallel() + h := newHarness(t) + c := h.connect(t, session) + if _, err := h.callback(c, session, h.consented(c)); err != nil { + t.Fatal(err) + } + h.google.AnswerRevoke(http.StatusBadRequest, `{"error":"invalid_token","error_description":"Token expired or revoked"}`) + if err := h.svc.Disconnect(context.Background()); err != nil { + t.Fatalf("Disconnect: %v", err) + } + if _, ok := h.row(t); ok { + t.Error("integration still stored") + } +} + +func TestDisconnectRevokeFailureKeepsConnection(t *testing.T) { + t.Parallel() + for _, status := range []int{http.StatusBadRequest, http.StatusServiceUnavailable} { + h := newHarness(t) + c := h.connect(t, session) + if _, err := h.callback(c, session, h.consented(c)); err != nil { + t.Fatal(err) + } + h.google.AnswerRevoke(status, "") + err := h.svc.Disconnect(context.Background()) + var revoke *gcal.RevokeError + if !errors.As(err, &revoke) { + t.Fatalf("HTTP %d: Disconnect error = %v, want *RevokeError", status, err) + } + assertNoSecret(t, "error", err.Error()) + if _, ok := h.row(t); !ok { + t.Errorf("HTTP %d: connection cleared although Google did not revoke", status) + } + } +} + +func TestDisconnectNotConnected(t *testing.T) { + t.Parallel() + h := newHarness(t) + if err := h.svc.Disconnect(context.Background()); !errors.Is(err, gcal.ErrNotConnected) { + t.Fatalf("Disconnect = %v, want ErrNotConnected", err) + } + if len(h.google.Revoked()) != 0 { + t.Error("revoke called with nothing connected") + } +} + +func TestNewServiceRequiresConfig(t *testing.T) { + t.Parallel() + h := newHarness(t) + pub, _ := url.Parse(publicURL) + valid := gcal.Config{ + ClientID: "id", ClientSecret: "secret", PublicURL: pub, SecretKey: []byte("k"), + Endpoint: gcal.GoogleEndpoint, RevokeURL: gcal.GoogleRevokeURL, + } + if _, err := gcal.NewService(h.st, h.box, valid); err != nil { + t.Fatalf("valid config: %v", err) + } + for name, edit := range map[string]func(*gcal.Config){ + "client ID": func(c *gcal.Config) { c.ClientID = "" }, + "client secret": func(c *gcal.Config) { c.ClientSecret = "" }, + "public URL": func(c *gcal.Config) { c.PublicURL = nil }, + "secret key": func(c *gcal.Config) { c.SecretKey = nil }, + "token URL": func(c *gcal.Config) { c.Endpoint.TokenURL = "" }, + "revoke URL": func(c *gcal.Config) { c.RevokeURL = "" }, + } { + cfg := valid + edit(&cfg) + if _, err := gcal.NewService(h.st, h.box, cfg); err == nil { + t.Errorf("NewService accepted a config without the %s", name) + } + } +} + +func assertCookieCleared(t *testing.T, rec *httptest.ResponseRecorder) { + t.Helper() + for _, c := range rec.Result().Cookies() { + if c.Name == "__Host-mealquest_google" && c.MaxAge < 0 { + return + } + } + t.Error("connect cookie not cleared") +} + +// assertNoSecret fails when text holds the refresh token, the access token +// or the client secret. +func assertNoSecret(t *testing.T, what, text string) { + t.Helper() + for _, secret := range []string{gcaltest.RefreshToken, gcaltest.AccessToken, gcaltest.ClientSecret} { + if strings.Contains(text, secret) { + t.Errorf("%s contains a secret: %s", what, text) + } + } +} + +func headerText(h http.Header) string { + var b strings.Builder + for k, vs := range h { + b.WriteString(k + ": " + strings.Join(vs, ", ") + "\n") + } + return b.String() +} diff --git a/internal/gcal/export_test.go b/internal/gcal/export_test.go new file mode 100644 index 0000000..1bdec15 --- /dev/null +++ b/internal/gcal/export_test.go @@ -0,0 +1,6 @@ +package gcal + +import "time" + +// SetClock replaces s's clock. +func SetClock(s *Service, now func() time.Time) { s.now = now } diff --git a/internal/gcal/gcaltest/gcaltest.go b/internal/gcal/gcaltest/gcaltest.go new file mode 100644 index 0000000..4edb931 --- /dev/null +++ b/internal/gcal/gcaltest/gcaltest.go @@ -0,0 +1,209 @@ +// Package gcaltest provides a fake Google OAuth server for tests of the +// Google Calendar connection: a token endpoint that checks the client, the +// redirect URI and PKCE, and a revoke endpoint that records what it revoked. +// It never touches the network beyond its own httptest server. +package gcaltest + +import ( + "crypto/sha256" + "encoding/base64" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "golang.org/x/oauth2" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" +) + +// Fixed credentials and tokens the fake issues and expects. +const ( + ClientID = "mealquest-test.apps.googleusercontent.com" + ClientSecret = "google-client-s3cret-4f1c" + RefreshToken = "1//google-refresh-t0ken-9a8b7c" + AccessToken = "ya29.google-access-t0ken-5d6e" +) + +// Google is the fake. Its exported fields script the next responses; set +// them before the request they affect. +type Google struct { + Server *httptest.Server + // RedirectURL is the redirect URI the token endpoint requires. + RedirectURL string + + mu sync.Mutex + // omitRefresh leaves refresh_token out of the token response. + omitRefresh bool + // scope is the token response's granted scopes. + scope string + // tokenStatus, when not 200, fails the token endpoint with a body that + // quotes the refresh token and client secret. + tokenStatus int + // revokeStatus and revokeBody answer the revoke endpoint; revokeBody + // quotes the token when revokeStatus is not 200 or 400. + revokeStatus int + revokeBody string + codes map[string]string // code -> PKCE challenge + tokenCalls int + revoked []string +} + +// New starts a fake Google that requires redirectURL at its token endpoint. +// It grants every scope in gcal.Scopes with a refresh token by default. +func New(t *testing.T, redirectURL string) *Google { + t.Helper() + g := &Google{ + RedirectURL: redirectURL, + scope: strings.Join(gcal.Scopes, " "), + tokenStatus: http.StatusOK, + revokeStatus: http.StatusOK, + codes: map[string]string{}, + } + mux := http.NewServeMux() + mux.HandleFunc("POST /token", g.token) + mux.HandleFunc("POST /revoke", g.revoke) + g.Server = httptest.NewServer(mux) + t.Cleanup(g.Server.Close) + return g +} + +// Endpoint is the fake's authorization and token endpoint, with Google's +// client authentication style. +func (g *Google) Endpoint() oauth2.Endpoint { + return oauth2.Endpoint{ + AuthURL: g.Server.URL + "/auth", + TokenURL: g.Server.URL + "/token", + AuthStyle: gcal.GoogleEndpoint.AuthStyle, + } +} + +// RevokeURL is the fake's revocation endpoint. +func (g *Google) RevokeURL() string { return g.Server.URL + "/revoke" } + +// Authorize plays the consent screen for an authorization request with PKCE +// challenge and returns the code Google would send back. +func (g *Google) Authorize(challenge string) string { + g.mu.Lock() + defer g.mu.Unlock() + code := fmt.Sprintf("code-%d", len(g.codes)+1) + g.codes[code] = challenge + return code +} + +// OmitRefreshToken makes the token endpoint answer without a refresh token, +// as Google does when it reuses an earlier grant. +func (g *Google) OmitRefreshToken() { + g.mu.Lock() + defer g.mu.Unlock() + g.omitRefresh = true +} + +// GrantScope sets the scopes the token endpoint reports as granted. +func (g *Google) GrantScope(scope string) { + g.mu.Lock() + defer g.mu.Unlock() + g.scope = scope +} + +// FailToken makes the token endpoint answer status with a body that quotes +// the refresh token and client secret. +func (g *Google) FailToken(status int) { + g.mu.Lock() + defer g.mu.Unlock() + g.tokenStatus = status +} + +// AnswerRevoke makes the revoke endpoint answer status with body. A body of +// "" with a status other than 200 quotes the token it was sent. +func (g *Google) AnswerRevoke(status int, body string) { + g.mu.Lock() + defer g.mu.Unlock() + g.revokeStatus, g.revokeBody = status, body +} + +// TokenCalls is how many requests reached the token endpoint. +func (g *Google) TokenCalls() int { + g.mu.Lock() + defer g.mu.Unlock() + return g.tokenCalls +} + +// Revoked lists the tokens the revoke endpoint received, in order. +func (g *Google) Revoked() []string { + g.mu.Lock() + defer g.mu.Unlock() + return append([]string(nil), g.revoked...) +} + +func (g *Google) token(w http.ResponseWriter, r *http.Request) { + g.mu.Lock() + defer g.mu.Unlock() + g.tokenCalls++ + if err := r.ParseForm(); err != nil { + http.Error(w, "bad form", http.StatusBadRequest) + return + } + f := r.PostForm + challenge, known := g.codes[f.Get("code")] + delete(g.codes, f.Get("code")) + switch { + case f.Get("grant_type") != "authorization_code": + oauthError(w, http.StatusBadRequest, "unsupported_grant_type") + return + case f.Get("client_id") != ClientID || f.Get("client_secret") != ClientSecret: + oauthError(w, http.StatusUnauthorized, "invalid_client") + return + case !known, f.Get("redirect_uri") != g.RedirectURL, s256(f.Get("code_verifier")) != challenge: + oauthError(w, http.StatusBadRequest, "invalid_grant") + return + } + if g.tokenStatus != http.StatusOK { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(g.tokenStatus) + fmt.Fprintf(w, `{"error":"invalid_grant","error_description":"echo %s %s"}`, RefreshToken, ClientSecret) + return + } + refresh := "" + if !g.omitRefresh { + refresh = fmt.Sprintf(`"refresh_token":%q,`, RefreshToken) + } + w.Header().Set("Content-Type", "application/json") + fmt.Fprintf(w, `{"access_token":%q,%s"expires_in":3599,"scope":%q,"token_type":"Bearer"}`, + AccessToken, refresh, g.scope) +} + +func (g *Google) revoke(w http.ResponseWriter, r *http.Request) { + g.mu.Lock() + defer g.mu.Unlock() + if r.Header.Get("Content-Type") != "application/x-www-form-urlencoded" { + http.Error(w, "want a form", http.StatusUnsupportedMediaType) + return + } + if err := r.ParseForm(); err != nil { + http.Error(w, "bad form", http.StatusBadRequest) + return + } + token := r.PostForm.Get("token") + g.revoked = append(g.revoked, token) + body := g.revokeBody + if body == "" && g.revokeStatus != http.StatusOK { + body = "cannot revoke " + token + } + w.WriteHeader(g.revokeStatus) + fmt.Fprint(w, body) +} + +func oauthError(w http.ResponseWriter, status int, code string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + fmt.Fprintf(w, `{"error":%q}`, code) +} + +// s256 is the PKCE S256 challenge for verifier. +func s256(verifier string) string { + sum := sha256.Sum256([]byte(verifier)) + return base64.RawURLEncoding.EncodeToString(sum[:]) +} diff --git a/internal/server/aisettings_test.go b/internal/server/aisettings_test.go index 89832dd..cd11105 100644 --- a/internal/server/aisettings_test.go +++ b/internal/server/aisettings_test.go @@ -29,7 +29,7 @@ func newAIRouter(t *testing.T) http.Handler { t.Helper() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), - newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/dinners_test.go b/internal/server/dinners_test.go index fb978c8..1f7aef9 100644 --- a/internal/server/dinners_test.go +++ b/internal/server/dinners_test.go @@ -92,7 +92,7 @@ func newDinnersEnv(t *testing.T) *dinnersEnv { t.Fatal(err) } e.h, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), - svc, newTestSuggest(t), newTestPlan(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + svc, newTestSuggest(t), newTestPlan(t), newTestGoogle(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/google.go b/internal/server/google.go new file mode 100644 index 0000000..df5243c --- /dev/null +++ b/internal/server/google.go @@ -0,0 +1,105 @@ +package server + +import ( + "errors" + "log/slog" + "net/http" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/auth" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" +) + +// googleConnectedPath is where a completed connect sends the browser: the +// Google Calendar settings screen. +const googleConnectedPath = "/settings/calendar" + +// getGoogleConnect sends the browser to Google's consent screen with a state +// bound to its session. The household opens it as a top-level navigation; +// the session cookie is SameSite=Lax, so it rides along. +func getGoogleConnect(logger *slog.Logger, svc *gcal.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + session, ok := auth.SessionFrom(r.Context()) + if !ok { + logger.ErrorContext(r.Context(), "Google connect without a session") + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + consent, err := svc.Connect(w, session) + if err != nil { + logger.ErrorContext(r.Context(), "start Google connect", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + http.Redirect(w, r, consent, http.StatusFound) + } +} + +// getGoogleCallback completes the connect Google redirects back with, then +// sends the browser to the Google Calendar settings screen. Every failure is +// an API error; none holds a token or Google's response text. +func getGoogleCallback(logger *slog.Logger, svc *gcal.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + ctx := r.Context() + session, ok := auth.SessionFrom(ctx) + if !ok { + logger.ErrorContext(ctx, "Google callback without a session") + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + err := svc.Complete(w, r, session) + switch { + case err == nil: + http.Redirect(w, r, googleConnectedPath, http.StatusSeeOther) + case errors.Is(err, gcal.ErrBadState): + logger.WarnContext(ctx, "Google callback refused", "error", err) + writeError(w, r, logger, http.StatusBadRequest, "invalid_state", + "this Google Calendar connection expired or was not started here; connect again") + case errors.Is(err, gcal.ErrDenied): + logger.WarnContext(ctx, "Google returned an authorization error") + writeError(w, r, logger, http.StatusForbidden, "google_denied", + "Google did not grant access to your calendar; connect again to retry") + case errors.Is(err, gcal.ErrNoCode): + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", "missing authorization code") + case errors.Is(err, gcal.ErrNoRefreshToken): + logger.WarnContext(ctx, "Google connect refused", "error", err) + writeError(w, r, logger, http.StatusBadGateway, "reconnect_required", + "Google did not grant lasting access to your calendar; connect Google Calendar again") + case errors.Is(err, gcal.ErrScopes): + logger.WarnContext(ctx, "Google connect refused", "error", err) + writeError(w, r, logger, http.StatusForbidden, "scopes_missing", + "allow MealQuest to see your calendars and manage their events, then connect Google Calendar again") + default: + logger.WarnContext(ctx, "Google connect failed", "error", err) + writeError(w, r, logger, http.StatusBadGateway, "google_failed", + "Google Calendar could not be connected; connect again") + } + } +} + +// deleteGoogle revokes the connected account's access at Google and forgets +// it: 204 on success, 409 not_connected when nothing is connected, 502 when +// Google could not revoke (the connection is kept, so it can be retried). +func deleteGoogle(logger *slog.Logger, svc *gcal.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + err := svc.Disconnect(r.Context()) + var revoke *gcal.RevokeError + switch { + case err == nil: + w.WriteHeader(http.StatusNoContent) + case errors.Is(err, gcal.ErrNotConnected): + writeError(w, r, logger, http.StatusConflict, "not_connected", "Google Calendar is not connected") + case errors.Is(err, gcal.ErrChanged): + writeError(w, r, logger, http.StatusConflict, "conflict", + "the Google Calendar connection changed; reload and try again") + case errors.As(err, &revoke): + logger.WarnContext(r.Context(), "revoke Google access", "error", err) + writeError(w, r, logger, http.StatusBadGateway, "revoke_failed", + "Google could not revoke MealQuest's access; try again") + default: + logger.ErrorContext(r.Context(), "disconnect Google Calendar", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + } + } +} diff --git a/internal/server/google_test.go b/internal/server/google_test.go new file mode 100644 index 0000000..c954235 --- /dev/null +++ b/internal/server/google_test.go @@ -0,0 +1,307 @@ +package server + +import ( + "bytes" + "context" + "database/sql" + "encoding/json" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "slices" + "strings" + "testing" + "testing/fstest" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal/gcaltest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +const ( + googleConnectPath = "/api/v1/integrations/google/connect" + googleCallbackPath = "/api/v1/integrations/google/callback" + googlePath = "/api/v1/integrations/google" +) + +// googleEnv is a router whose Google Calendar service talks to a fake +// Google, with every log line it writes. +type googleEnv struct { + h http.Handler + google *gcaltest.Google + st *store.Store + box *secretbox.Box + logs *bytes.Buffer + // seen collects every response body and header, to check none holds a + // secret. + seen strings.Builder +} + +func newGoogleEnv(t *testing.T) *googleEnv { + t.Helper() + pub, err := url.Parse("https://mealquest.example") + if err != nil { + t.Fatal(err) + } + e := &googleEnv{ + google: gcaltest.New(t, "https://mealquest.example"+gcal.CallbackPath), + st: newTestStore(t), + box: newTestBox(t), + logs: &bytes.Buffer{}, + } + svc, err := gcal.NewService(e.st, e.box, gcal.Config{ + ClientID: gcaltest.ClientID, + ClientSecret: gcaltest.ClientSecret, + PublicURL: pub, + SecretKey: []byte("server-secret-key"), + Endpoint: e.google.Endpoint(), + RevokeURL: e.google.RevokeURL(), + }) + if err != nil { + t.Fatal(err) + } + logger := slog.New(slog.NewJSONHandler(e.logs, &slog.HandlerOptions{Level: slog.LevelDebug})) + e.h, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), + newTestDinners(t), newTestSuggest(t), newTestPlan(t), svc, newTestRunner(t), newTestPhotos(t), + fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + for _, secret := range []string{gcaltest.RefreshToken, gcaltest.AccessToken, gcaltest.ClientSecret} { + if strings.Contains(e.seen.String(), secret) { + t.Errorf("a response holds a secret: %s", e.seen.String()) + } + if strings.Contains(e.logs.String(), secret) { + t.Errorf("the logs hold a secret: %s", e.logs.String()) + } + } + }) + return e +} + +func (e *googleEnv) serve(req *http.Request) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + e.h.ServeHTTP(rec, req) + e.seen.WriteString(rec.Body.String()) + for k, vs := range rec.Header() { + e.seen.WriteString(k + ": " + strings.Join(vs, ", ") + "\n") + } + return rec +} + +// connect starts a connect as the test session and returns Google's consent +// URL and the connect cookie. +func (e *googleEnv) connect(t *testing.T) (*url.URL, *http.Cookie) { + t.Helper() + rec := e.serve(signedIn(http.MethodGet, googleConnectPath, nil)) + if rec.Code != http.StatusFound { + t.Fatalf("connect = %d %s, want 302", rec.Code, rec.Body) + } + if cc := rec.Header().Get("Cache-Control"); cc != "no-store" { + t.Errorf("connect Cache-Control = %q, want no-store", cc) + } + consent, err := url.Parse(rec.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + var cookie *http.Cookie + for _, c := range rec.Result().Cookies() { + if c.Name == "__Host-mealquest_google" { + cookie = c + } + } + if cookie == nil { + t.Fatal("connect set no connect cookie") + } + return consent, cookie +} + +// callback returns to the callback as the test session with the connect +// cookie and query. +func (e *googleEnv) callback(cookie *http.Cookie, query url.Values) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, googleCallbackPath+"?"+query.Encode(), nil) + req.AddCookie(validSessionCookie) + if cookie != nil { + req.AddCookie(cookie) + } + return e.serve(req) +} + +func (e *googleEnv) consented(consent *url.URL) url.Values { + q := consent.Query() + return url.Values{"state": {q.Get("state")}, "code": {e.google.Authorize(q.Get("code_challenge"))}} +} + +func (e *googleEnv) stored(t *testing.T) bool { + t.Helper() + _, err := db.New(e.st.Reader).GetIntegrationsGoogle(context.Background()) + if errors.Is(err, sql.ErrNoRows) { + return false + } + if err != nil { + t.Fatal(err) + } + return true +} + +func errorBodyCode(t *testing.T, rec *httptest.ResponseRecorder) (code, message string) { + t.Helper() + var body struct { + Error apiError `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode %q: %v", rec.Body.String(), err) + } + return body.Error.Code, body.Error.Message +} + +func TestGoogleConnectAndDisconnect(t *testing.T) { + t.Parallel() + e := newGoogleEnv(t) + + consent, cookie := e.connect(t) + q := consent.Query() + if consent.Host != strings.TrimPrefix(e.google.Server.URL, "http://") || consent.Path != "/auth" { + t.Errorf("consent URL = %s, want the fake's /auth", consent) + } + if q.Get("access_type") != "offline" || q.Get("prompt") != "consent" || + q.Get("scope") != strings.Join(gcal.Scopes, " ") || + q.Get("redirect_uri") != "https://mealquest.example"+gcal.CallbackPath { + t.Errorf("consent query = %v", q) + } + + rec := e.callback(cookie, e.consented(consent)) + if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/settings/calendar" { + t.Fatalf("callback = %d Location %q %s, want 303 to /settings/calendar", rec.Code, rec.Header().Get("Location"), rec.Body) + } + row, err := db.New(e.st.Reader).GetIntegrationsGoogle(context.Background()) + if err != nil { + t.Fatal(err) + } + if plain, err := e.box.Decrypt(row.RefreshTokenCt); err != nil || string(plain) != gcaltest.RefreshToken { + t.Errorf("stored token opens to %q, %v", plain, err) + } + if row.ConnectedAt == "" { + t.Error("connected_at not set") + } + + // DELETE needs the CSRF header like every other write. + req := httptest.NewRequest(http.MethodDelete, googlePath, nil) + req.AddCookie(validSessionCookie) + if rec := e.serve(req); rec.Code != http.StatusForbidden { + t.Errorf("DELETE without CSRF = %d, want 403", rec.Code) + } + if len(e.google.Revoked()) != 0 || !e.stored(t) { + t.Fatal("a DELETE without CSRF revoked or cleared the connection") + } + + rec = e.serve(signedIn(http.MethodDelete, googlePath, nil)) + if rec.Code != http.StatusNoContent || rec.Body.Len() != 0 { + t.Fatalf("DELETE = %d %q, want 204 with no body", rec.Code, rec.Body) + } + if got := e.google.Revoked(); !slices.Equal(got, []string{gcaltest.RefreshToken}) { + t.Errorf("revoked %q, want the refresh token", got) + } + if e.stored(t) { + t.Error("connection still stored after DELETE") + } + + rec = e.serve(signedIn(http.MethodDelete, googlePath, nil)) + if code, _ := errorBodyCode(t, rec); rec.Code != http.StatusConflict || code != "not_connected" { + t.Errorf("second DELETE = %d %s, want 409 not_connected", rec.Code, rec.Body) + } +} + +func TestGoogleCallbackBadState(t *testing.T) { + t.Parallel() + e := newGoogleEnv(t) + consent, cookie := e.connect(t) + good := e.consented(consent) + + for name, tc := range map[string]struct { + cookie *http.Cookie + query url.Values + }{ + "missing state": {cookie, url.Values{"code": good["code"]}}, + "wrong state": {cookie, url.Values{"state": {"forged"}, "code": good["code"]}}, + "missing cookie": {nil, good}, + } { + rec := e.callback(tc.cookie, tc.query) + if code, _ := errorBodyCode(t, rec); rec.Code != http.StatusBadRequest || code != "invalid_state" { + t.Errorf("%s: callback = %d %s, want 400 invalid_state", name, rec.Code, rec.Body) + } + } + if e.google.TokenCalls() != 0 { + t.Error("a code was exchanged despite a bad state") + } + if e.stored(t) { + t.Error("a connection was stored") + } +} + +func TestGoogleCallbackWithoutRefreshTokenAsksToReconnect(t *testing.T) { + t.Parallel() + e := newGoogleEnv(t) + e.google.OmitRefreshToken() + consent, cookie := e.connect(t) + rec := e.callback(cookie, e.consented(consent)) + code, message := errorBodyCode(t, rec) + if rec.Code != http.StatusBadGateway || code != "reconnect_required" || !strings.Contains(message, "connect Google Calendar again") { + t.Errorf("callback = %d %s, want 502 reconnect_required asking to connect again", rec.Code, rec.Body) + } + if e.stored(t) { + t.Error("a connection was stored without a refresh token") + } +} + +func TestGoogleCallbackFailuresAreRedacted(t *testing.T) { + t.Parallel() + e := newGoogleEnv(t) + e.google.FailToken(http.StatusBadRequest) + consent, cookie := e.connect(t) + rec := e.callback(cookie, e.consented(consent)) + if code, _ := errorBodyCode(t, rec); rec.Code != http.StatusBadGateway || code != "google_failed" { + t.Errorf("callback = %d %s, want 502 google_failed", rec.Code, rec.Body) + } + // newGoogleEnv's cleanup checks the response and logs hold no secret. +} + +func TestGoogleRevokeFailureKeepsConnection(t *testing.T) { + t.Parallel() + e := newGoogleEnv(t) + consent, cookie := e.connect(t) + if rec := e.callback(cookie, e.consented(consent)); rec.Code != http.StatusSeeOther { + t.Fatalf("callback = %d %s", rec.Code, rec.Body) + } + e.google.AnswerRevoke(http.StatusServiceUnavailable, "") + rec := e.serve(signedIn(http.MethodDelete, googlePath, nil)) + if code, _ := errorBodyCode(t, rec); rec.Code != http.StatusBadGateway || code != "revoke_failed" { + t.Errorf("DELETE = %d %s, want 502 revoke_failed", rec.Code, rec.Body) + } + if !e.stored(t) { + t.Error("connection cleared although Google did not revoke") + } +} + +func TestGoogleRoutesRequireSession(t *testing.T) { + t.Parallel() + e := newGoogleEnv(t) + for _, c := range []struct{ method, path string }{ + {http.MethodGet, googleConnectPath}, + {http.MethodGet, googleCallbackPath + "?state=s&code=c"}, + {http.MethodDelete, googlePath}, + } { + rec := e.serve(httptest.NewRequest(c.method, c.path, nil)) + if rec.Code != http.StatusUnauthorized || rec.Body.String() != unauthenticatedJSON { + t.Errorf("%s %s signed out = %d %s, want 401", c.method, c.path, rec.Code, rec.Body) + } + } + if e.google.TokenCalls() != 0 || len(e.google.Revoked()) != 0 { + t.Error("a signed-out request reached Google") + } +} diff --git a/internal/server/login_test.go b/internal/server/login_test.go index 16d854e..3555347 100644 --- a/internal/server/login_test.go +++ b/internal/server/login_test.go @@ -83,7 +83,7 @@ func newLoginRouter(t *testing.T, issuer string) http.Handler { t.Fatal(err) } r, err := NewRouter(logger, "dev", trusted, flow, sessions, newTestSettings(t), - newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/media_test.go b/internal/server/media_test.go index dad2c2d..d301ace 100644 --- a/internal/server/media_test.go +++ b/internal/server/media_test.go @@ -51,7 +51,7 @@ func newMediaRouter(t *testing.T) (r http.Handler, id, orphan string) { t.Fatal(err) } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - r, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), + r, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), photos, fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/origin_test.go b/internal/server/origin_test.go index 8c3f22b..dfe3b5c 100644 --- a/internal/server/origin_test.go +++ b/internal/server/origin_test.go @@ -45,7 +45,7 @@ func newOriginRouter(t *testing.T, logs io.Writer) http.Handler { } logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(logs, nil))) r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{}, newTestSettings(t), - newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/plans_test.go b/internal/server/plans_test.go index 9f6142c..b6e131a 100644 --- a/internal/server/plans_test.go +++ b/internal/server/plans_test.go @@ -56,7 +56,7 @@ func newPlansRouter(t *testing.T) plansEnv { t.Fatal(err) } h, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), - newTestDinners(t), newTestSuggest(t), svc, runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestDinners(t), newTestSuggest(t), svc, newTestGoogle(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/recipesearch_test.go b/internal/server/recipesearch_test.go index 9931bb6..34f2032 100644 --- a/internal/server/recipesearch_test.go +++ b/internal/server/recipesearch_test.go @@ -22,7 +22,7 @@ const ( func newRecipeSearchRouter(t *testing.T, search *recipes.SearchSettingsService) http.Handler { t.Helper() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), search, newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), + r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), search, newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/server.go b/internal/server/server.go index 5fbceb6..8a17ce5 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -14,6 +14,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/forwarded" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/plan" @@ -45,12 +46,14 @@ type RouteMounter interface { // /decline; planSvc serves GET /api/v1/plans/{weekStart}, POST // /api/v1/plans/{weekStart}/draft, PUT // /api/v1/plans/{weekStart}/days/{date}, GET /api/v1/rotation-reviews and -// POST /api/v1/dinners/{id}/rotation-review; runner serves +// POST /api/v1/dinners/{id}/rotation-review; googleSvc serves GET +// /api/v1/integrations/google/connect and /callback and DELETE +// /api/v1/integrations/google; runner serves // POST /api/v1/jobs and GET /api/v1/jobs/{id}; photos serves // GET /media/photos/{id}-{size}.jpg; assets is the web build // served for every other GET. /health, /auth and the web build are public. // Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader. -func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, search *recipes.SearchSettingsService, ai *aisettings.Service, dinnerSvc *dinners.Service, suggestSvc *suggest.Service, planSvc *plan.Service, runner *jobs.Runner, photos PhotoFiles, assets fs.FS) (http.Handler, error) { +func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, search *recipes.SearchSettingsService, ai *aisettings.Service, dinnerSvc *dinners.Service, suggestSvc *suggest.Service, planSvc *plan.Service, googleSvc *gcal.Service, runner *jobs.Runner, photos PhotoFiles, assets fs.FS) (http.Handler, error) { if settings == nil { return nil, errors.New("server: nil household settings service") } @@ -69,6 +72,9 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a if planSvc == nil { return nil, errors.New("server: nil plan service") } + if googleSvc == nil { + return nil, errors.New("server: nil Google Calendar service") + } if runner == nil { return nil, errors.New("server: nil job runner") } @@ -128,6 +134,12 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Put("/plans/{weekStart}/days/{date}", putPlanDay(logger, planSvc)) apiV1.Get("/rotation-reviews", getRotationReviews(logger, planSvc)) apiV1.Post("/dinners/{id}/rotation-review", postRotationReview(logger, planSvc)) + // connect and callback are top-level browser navigations, so they are + // GETs: the Lax session cookie rides both and CSRF applies to neither. + // The callback's state is bound to the session that started connect. + apiV1.Get("/integrations/google/connect", getGoogleConnect(logger, googleSvc)) + apiV1.Get("/integrations/google/callback", getGoogleCallback(logger, googleSvc)) + apiV1.Delete("/integrations/google", deleteGoogle(logger, googleSvc)) apiV1.Post("/jobs", postJob(logger, runner)) apiV1.Get("/jobs/{id}", getJob(logger, runner)) media.Get("/photos/{name}", getPhoto(logger, photos)) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index a7499a3..d588bc3 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -11,16 +11,19 @@ import ( "maps" "net/http" "net/http/httptest" + "net/url" "strings" "testing" "testing/fstest" "github.com/go-chi/chi/v5" + "golang.org/x/oauth2" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/aisettings" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/skills" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/plan" @@ -50,7 +53,7 @@ func newTestRouterWith(t *testing.T, version string, settings *household.Setting "index.html": {Data: []byte(fixtureIndex)}, "assets/index-AbC123.js": {Data: []byte(fixtureAsset)}, } - r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), runner, newTestPhotos(t), assets) + r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), runner, newTestPhotos(t), assets) if err != nil { t.Fatal(err) } @@ -218,6 +221,28 @@ func newTestBox(t *testing.T) *secretbox.Box { return box } +// newTestGoogle returns a Google Calendar service over a migrated temp +// database whose Google endpoints refuse every connection. +func newTestGoogle(t *testing.T) *gcal.Service { + t.Helper() + pub, err := url.Parse("https://mealquest.example") + if err != nil { + t.Fatal(err) + } + svc, err := gcal.NewService(newTestStore(t), newTestBox(t), gcal.Config{ + ClientID: "client-id", + ClientSecret: "client-secret", + PublicURL: pub, + SecretKey: []byte("server-secret-key"), + Endpoint: oauth2.Endpoint{AuthURL: "http://127.0.0.1:1/auth", TokenURL: "http://127.0.0.1:1/token"}, + RevokeURL: "http://127.0.0.1:1/revoke", + }) + if err != nil { + t.Fatal(err) + } + return svc +} + // newTestAI returns an AI settings service over a migrated temp database // with the seed AI settings and a random secret box. func newTestAI(t *testing.T) *aisettings.Service { @@ -237,7 +262,7 @@ func TestNewRouterRequiresAISettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), nil, newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), nil, newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil AI settings service") } } @@ -246,7 +271,7 @@ func TestNewRouterRequiresSearchSettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil recipe search settings service") } } @@ -255,7 +280,7 @@ func TestNewRouterRequiresPhotoFiles(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), nil, fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), nil, fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted nil photo files") } } @@ -264,7 +289,7 @@ func TestNewRouterRequiresSettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil settings service") } } @@ -273,7 +298,7 @@ func TestNewRouterRequiresDinners(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), nil, newTestSuggest(t), newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), nil, newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil dinners service") } } @@ -282,7 +307,7 @@ func TestNewRouterRequiresSuggestions(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), nil, newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), nil, newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil suggestions service") } } @@ -291,16 +316,25 @@ func TestNewRouterRequiresPlan(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), nil, newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), nil, newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil plan service") } } +func TestNewRouterRequiresGoogle(t *testing.T) { + t.Parallel() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), nil, newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + t.Fatal("NewRouter accepted a nil Google Calendar service") + } +} + func TestNewRouterRequiresJobRunner(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), nil, newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), nil, newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil job runner") } } diff --git a/internal/server/settings_test.go b/internal/server/settings_test.go index 09f1be7..8a4398f 100644 --- a/internal/server/settings_test.go +++ b/internal/server/settings_test.go @@ -277,7 +277,7 @@ func TestHouseholdSettingsWithRealSession(t *testing.T) { } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) router, err := NewRouter(logger, "dev", nil, stubAuth{}, sessions, household.NewSettingsService(st), - newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestAI(t), newTestDinners(t), newTestSuggest(t), newTestPlan(t), newTestGoogle(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/suggestions_test.go b/internal/server/suggestions_test.go index a0c6549..2595b13 100644 --- a/internal/server/suggestions_test.go +++ b/internal/server/suggestions_test.go @@ -45,7 +45,7 @@ func newSuggestionsRouter(t *testing.T, names ...string) (http.Handler, *store.S t.Fatal(err) } h, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), - newTestDinners(t), svc, newTestPlan(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestDinners(t), svc, newTestPlan(t), newTestGoogle(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/migrations/queries/settings.sql b/migrations/queries/settings.sql index 6a1c6a8..e2b7a36 100644 --- a/migrations/queries/settings.sql +++ b/migrations/queries/settings.sql @@ -106,6 +106,13 @@ ON CONFLICT (id) DO UPDATE SET updated_at = excluded.updated_at RETURNING *; +-- name: DeleteIntegrationsGoogle :execrows +-- Deletes the row only while it still holds refresh_token_ct, so a +-- connection made after the caller read the row is kept. Returns the number +-- of rows deleted. +DELETE FROM integrations_google +WHERE id = 1 AND refresh_token_ct = ?; + -- name: GetChatSettings :one -- Returns sql.ErrNoRows until the row exists. SELECT * FROM chat_settings -- 2.55.0 From 2cd2e5af322f37955ca42010e77ee58c06ed82e3 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Wed, 7 Oct 2026 20:46:35 -0400 Subject: [PATCH 2/2] Serialize Google connect and disconnect; consume connect state once A disconnect read the stored refresh token, revoked it at Google, then deleted the row only if it still held that token. Google revokes the whole grant, so a reconnect whose token was issued before that revoke was kept as connected with a dead token. Connect's exchange-and-store and disconnect's read-revoke-delete now hold one mutex across the Google request and the database write, and the delete is unconditional (the conditional delete and ErrChanged are gone: the lock is the one guard). The connect cookie was single-use only in that the browser was asked to drop it; a saved copy verified again until it expired. The service now keeps each started connect's state in memory until the cookie expires and the callback consumes it, so a replayed cookie is refused as bad state. The fake Google now issues a distinct refresh token per exchange, revokes the whole grant like Google, and can hold a token or revoke request so tests can interleave a reconnect and a disconnect. Maestro task 1e9d7850-3a09-4572-b8a1-cb18cd4774cb (MQ125-01, MQ125-02). Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 9 ++- internal/auth/cookie.go | 5 +- internal/db/querier.go | 5 +- internal/db/settings.sql.go | 16 ++--- internal/gcal/connect.go | 70 ++++++++++++++---- internal/gcal/connect_test.go | 110 +++++++++++++++++++++++++++++ internal/gcal/gcaltest/gcaltest.go | 94 ++++++++++++++++++++++-- internal/server/google.go | 3 - migrations/queries/settings.sql | 7 +- 9 files changed, 276 insertions(+), 43 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4df3f76..1635116 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -627,7 +627,10 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. `access_type=offline`, `prompt=consent` and PKCE. Its `state` travels in the signed `__Host-mealquest_google` flow cookie (the same `auth.FlowCookie` mechanism as the login cookie), bound to the session by - a hash of the session's CSRF token. + a hash of the session's CSRF token. The service also keeps each started + connect's `state` in memory until the cookie expires; the callback + consumes it, so a replayed cookie is refused (`invalid_state`), and a + restart mid-connect means connecting again. - `GET /api/v1/integrations/google/callback` (the redirect URI: `MEALQUEST_PUBLIC_URL`'s origin plus this path) checks the state and session, exchanges the code, refuses a response with no refresh token @@ -638,6 +641,10 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. - `DELETE /api/v1/integrations/google` (CSRF-checked) revokes the token at Google, then deletes the row; a token Google reports as `invalid_token` counts as revoked, and any other revoke failure keeps the connection. + - Connect's exchange-and-store and disconnect's read-revoke-delete hold + one lock across the Google request and the database write: Google + revokes the whole grant, so a revoke overlapping a reconnect would + otherwise leave a stored token Google has already revoked. - No response, log line or error ever holds a token or Google's response text. - **Deterministic event ids**; each plan day records its diff --git a/internal/auth/cookie.go b/internal/auth/cookie.go index 722c0aa..ed85350 100644 --- a/internal/auth/cookie.go +++ b/internal/auth/cookie.go @@ -115,8 +115,9 @@ func (c FlowCookie) Set(w http.ResponseWriter, v flowValue, now time.Time) error return nil } -// Clear removes the cookie. A callback clears it before reading, so it is -// single-use. +// Clear asks the browser to discard the cookie. A copy of it still verifies +// until it expires, so a flow that must be single-use also consumes its +// state on the server. func (c FlowCookie) Clear(w http.ResponseWriter) { http.SetCookie(w, &http.Cookie{ Name: c.name, diff --git a/internal/db/querier.go b/internal/db/querier.go index dd515f8..81dc22c 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -50,10 +50,7 @@ type Querier interface { // decided is left as it is (0 rows). DecideSuggestion(ctx context.Context, arg DecideSuggestionParams) (int64, error) DeleteAIRunsBefore(ctx context.Context, cutoff string) (int64, error) - // Deletes the row only while it still holds refresh_token_ct, so a - // connection made after the caller read the row is kept. Returns the number - // of rows deleted. - DeleteIntegrationsGoogle(ctx context.Context, refreshTokenCt []byte) (int64, error) + DeleteIntegrationsGoogle(ctx context.Context) error DeleteRecipeIngredients(ctx context.Context, recipeID string) error DeleteRecipeSteps(ctx context.Context, recipeID string) error FailJob(ctx context.Context, arg FailJobParams) (int64, error) diff --git a/internal/db/settings.sql.go b/internal/db/settings.sql.go index 835cc8e..05d40d3 100644 --- a/internal/db/settings.sql.go +++ b/internal/db/settings.sql.go @@ -10,20 +10,14 @@ import ( "database/sql" ) -const deleteIntegrationsGoogle = `-- name: DeleteIntegrationsGoogle :execrows +const deleteIntegrationsGoogle = `-- name: DeleteIntegrationsGoogle :exec DELETE FROM integrations_google -WHERE id = 1 AND refresh_token_ct = ? +WHERE id = 1 ` -// Deletes the row only while it still holds refresh_token_ct, so a -// connection made after the caller read the row is kept. Returns the number -// of rows deleted. -func (q *Queries) DeleteIntegrationsGoogle(ctx context.Context, refreshTokenCt []byte) (int64, error) { - result, err := q.db.ExecContext(ctx, deleteIntegrationsGoogle, refreshTokenCt) - if err != nil { - return 0, err - } - return result.RowsAffected() +func (q *Queries) DeleteIntegrationsGoogle(ctx context.Context) error { + _, err := q.db.ExecContext(ctx, deleteIntegrationsGoogle) + return err } const getAISettings = `-- name: GetAISettings :one diff --git a/internal/gcal/connect.go b/internal/gcal/connect.go index 01b4ede..63740e6 100644 --- a/internal/gcal/connect.go +++ b/internal/gcal/connect.go @@ -20,6 +20,7 @@ import ( "net/url" "slices" "strings" + "sync" "time" "golang.org/x/oauth2" @@ -82,9 +83,6 @@ var ( ErrScopes = errors.New("gcal: Google did not grant every Calendar permission") // ErrNotConnected means no Google account is connected. ErrNotConnected = errors.New("gcal: Google Calendar is not connected") - // ErrChanged means the connection changed while it was being - // disconnected. - ErrChanged = errors.New("gcal: Google Calendar connection changed during disconnect") ) // Config is what NewService needs to talk to Google. @@ -106,6 +104,8 @@ type Config struct { } // Service connects and disconnects the household's Google account. +// MealQuest runs as one process, so the service's in-memory lock and pending +// states see every request. type Service struct { st *store.Store box *secretbox.Box @@ -115,6 +115,19 @@ type Service struct { cookie auth.FlowCookie // now is the clock; tests replace it. now func() time.Time + + // account serializes every change to the connected account — a + // connect's exchange and store, a disconnect's read, revoke and delete — + // across the request to Google and the database write. Google revokes the + // whole grant, so a revoke overlapping a connect would also kill the + // token the connect stores. + account sync.Mutex + + // pending holds the state of every connect started and not yet + // completed, with when it expires. Complete consumes a state once, so a + // replayed cookie finds it gone. + pendingMu sync.Mutex + pending map[string]time.Time } // NewService returns the service over st, sealing refresh tokens with box. @@ -147,6 +160,7 @@ func NewService(st *store.Store, box *secretbox.Box, cfg Config) (*Service, erro client: &http.Client{Timeout: googleTimeout, CheckRedirect: auth.CheckRedirect}, cookie: auth.NewFlowCookie(connectCookieName, connectCookieKeyLabel, connectCookieTTL, cfg.SecretKey), now: time.Now, + pending: map[string]time.Time{}, }, nil } @@ -180,9 +194,11 @@ func (s *Service) Connect(w http.ResponseWriter, session auth.Session) (string, Verifier: oauth2.GenerateVerifier(), Session: sessionBinding(session), } - if err := s.cookie.Set(w, &state, s.now()); err != nil { + now := s.now() + if err := s.cookie.Set(w, &state, now); err != nil { return "", fmt.Errorf("gcal: set connect cookie: %w", err) } + s.addPending(state.State, now) return s.oauth.AuthCodeURL(state.State, oauth2.AccessTypeOffline, oauth2.ApprovalForce, @@ -191,12 +207,12 @@ func (s *Service) Connect(w http.ResponseWriter, session auth.Session) (string, } // Complete finishes the connect Google redirected r back with. It clears the -// connect cookie, checks the state against it and session, exchanges the -// code and stores the refresh token, sealed, with connected_at now. A -// reconnect replaces the stored token and keeps the chosen calendar. +// connect cookie, checks the state against it and session, consumes the +// state, exchanges the code and stores the refresh token, sealed, with +// connected_at now. A reconnect replaces the stored token and keeps the +// chosen calendar. func (s *Service) Complete(w http.ResponseWriter, r *http.Request, session auth.Session) error { ctx := r.Context() - // The connect cookie is single-use. s.cookie.Clear(w) var state connectState @@ -210,6 +226,11 @@ func (s *Service) Complete(w http.ResponseWriter, r *http.Request, session auth. if subtle.ConstantTimeCompare([]byte(sessionBinding(session)), []byte(state.Session)) != 1 { return fmt.Errorf("%w: started by another session", ErrBadState) } + // A copy of the cookie verifies until it expires, so the state is + // single-use only because it is consumed here. + if !s.consumePending(state.State) { + return fmt.Errorf("%w: already used", ErrBadState) + } if q.Get("error") != "" { // error and error_description are Google-supplied text; neither is // kept. @@ -220,6 +241,8 @@ func (s *Service) Complete(w http.ResponseWriter, r *http.Request, session auth. return ErrNoCode } + s.account.Lock() + defer s.account.Unlock() token, err := s.oauth.Exchange(context.WithValue(ctx, oauth2.HTTPClient, s.client), code, oauth2.VerifierOption(state.Verifier)) if err != nil { @@ -238,6 +261,29 @@ func (s *Service) Complete(w http.ResponseWriter, r *http.Request, session auth. return s.store(ctx, sealed) } +// addPending records state, started at now, as pending until the connect +// cookie expires, and forgets expired ones. +func (s *Service) addPending(state string, now time.Time) { + s.pendingMu.Lock() + defer s.pendingMu.Unlock() + for k, expires := range s.pending { + if !now.Before(expires) { + delete(s.pending, k) + } + } + s.pending[state] = now.Add(connectCookieTTL) +} + +// consumePending removes state from the pending states, reporting whether +// it was there. +func (s *Service) consumePending(state string) bool { + s.pendingMu.Lock() + defer s.pendingMu.Unlock() + _, ok := s.pending[state] + delete(s.pending, state) + return ok +} + // checkScopes fails with ErrScopes unless token's granted scopes include // every scope in Scopes. Google lets the household untick a permission on // the consent screen and reports what was granted in "scope". @@ -286,6 +332,8 @@ func (s *Service) store(ctx context.Context, sealed []byte) error { // MealQuest's access in their Google account) counts as revoked. Any other // revoke failure keeps the connection so the household can retry. func (s *Service) Disconnect(ctx context.Context) error { + s.account.Lock() + defer s.account.Unlock() row, err := db.New(s.st.Reader).GetIntegrationsGoogle(ctx) if errors.Is(err, sql.ErrNoRows) { return ErrNotConnected @@ -300,13 +348,9 @@ func (s *Service) Disconnect(ctx context.Context) error { if err := s.revoke(ctx, string(token)); err != nil { return err } - n, err := db.New(s.st.Writer).DeleteIntegrationsGoogle(ctx, row.RefreshTokenCt) - if err != nil { + if err := db.New(s.st.Writer).DeleteIntegrationsGoogle(ctx); err != nil { return fmt.Errorf("gcal: delete Google integration: %w", err) } - if n == 0 { - return ErrChanged - } return nil } diff --git a/internal/gcal/connect_test.go b/internal/gcal/connect_test.go index aa3f817..434078a 100644 --- a/internal/gcal/connect_test.go +++ b/internal/gcal/connect_test.go @@ -290,6 +290,31 @@ func TestCompleteRefusesBadState(t *testing.T) { } } +// TestCompleteRefusesReplayedState presents a connect's cookie and state a +// second time, with a second valid code from the same consent URL, as an +// attacker holding a copy of the cookie could. The server refuses it, +// whatever the browser did with the cleared cookie. +func TestCompleteRefusesReplayedState(t *testing.T) { + t.Parallel() + h := newHarness(t) + c := h.connect(t, session) + if _, err := h.callback(c, session, h.consented(c)); err != nil { + t.Fatalf("Complete: %v", err) + } + stored, _ := h.row(t) + + _, err := h.callback(c, session, h.consented(c)) + if !errors.Is(err, gcal.ErrBadState) { + t.Fatalf("replayed Complete error = %v, want ErrBadState", err) + } + if got := h.google.TokenCalls(); got != 1 { + t.Errorf("token endpoint called %d times, want once: the replayed code was exchanged", got) + } + if row, _ := h.row(t); !bytes.Equal(row.RefreshTokenCt, stored.RefreshTokenCt) { + t.Error("the replay replaced the stored token") + } +} + func TestCompleteRefusesMissingRefreshToken(t *testing.T) { t.Parallel() h := newHarness(t) @@ -371,6 +396,91 @@ func TestDisconnectRevokesAndClears(t *testing.T) { } } +// TestDisconnectRacingReconnect drives a disconnect and a reconnect into +// each other at Google, with distinct tokens. However they interleave, the +// end state is consistent: disconnected, or connected with a token Google +// has not revoked. +func TestDisconnectRacingReconnect(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + // hold stops the request the other operation must not slip past. + hold func(*gcaltest.Google) (<-chan struct{}, func()) + // first is held at Google while second runs. + first, second func(h *harness, c connect, q url.Values) error + }{ + { + // The disconnect has read the stored token and is revoking it + // when the reconnect would store a new one. + name: "reconnect during revoke", + hold: (*gcaltest.Google).HoldRevoke, + first: disconnect, + second: reconnect, + }, + { + // The reconnect's new token is issued but not yet stored when + // the disconnect would revoke the old one. + name: "disconnect during exchange", + hold: (*gcaltest.Google).HoldToken, + first: reconnect, + second: disconnect, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + h := newHarness(t) + first := h.connect(t, session) + if _, err := h.callback(first, session, h.consented(first)); err != nil { + t.Fatal(err) + } + c := h.connect(t, session) + q := h.consented(c) + + arrived, release := tc.hold(h.google) + defer release() + firstDone := make(chan error, 1) + go func() { firstDone <- tc.first(h, c, q) }() + <-arrived + secondDone := make(chan error, 1) + go func() { secondDone <- tc.second(h, c, q) }() + // Let the second operation run as far as it can while the first + // is held at Google: to the end when nothing serializes them. + select { + case err := <-secondDone: + secondDone <- err + case <-time.After(500 * time.Millisecond): + } + release() + for _, done := range []chan error{firstDone, secondDone} { + if err := <-done; err != nil { + t.Logf("operation: %v", err) + } + } + + row, ok := h.row(t) + if !ok { + return + } + plain, err := h.box.Decrypt(row.RefreshTokenCt) + if err != nil { + t.Fatal(err) + } + if !h.google.Live(string(plain)) { + t.Errorf("stored as connected with a revoked token (Google revoked %d tokens)", len(h.google.Revoked())) + } + }) + } +} + +func disconnect(h *harness, _ connect, _ url.Values) error { + return h.svc.Disconnect(context.Background()) +} + +func reconnect(h *harness, c connect, q url.Values) error { + _, err := h.callback(c, session, q) + return err +} + func TestDisconnectTokenAlreadyRevokedAtGoogle(t *testing.T) { t.Parallel() h := newHarness(t) diff --git a/internal/gcal/gcaltest/gcaltest.go b/internal/gcal/gcaltest/gcaltest.go index 4edb931..0d621ca 100644 --- a/internal/gcal/gcaltest/gcaltest.go +++ b/internal/gcal/gcaltest/gcaltest.go @@ -1,7 +1,9 @@ // Package gcaltest provides a fake Google OAuth server for tests of the // Google Calendar connection: a token endpoint that checks the client, the -// redirect URI and PKCE, and a revoke endpoint that records what it revoked. -// It never touches the network beyond its own httptest server. +// redirect URI and PKCE and issues a distinct refresh token per exchange, and +// a revoke endpoint that, like Google's, revokes the whole grant: every token +// issued so far. It never touches the network beyond its own httptest +// server. package gcaltest import ( @@ -10,6 +12,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "slices" "strings" "sync" "testing" @@ -19,7 +22,9 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/gcal" ) -// Fixed credentials and tokens the fake issues and expects. +// Fixed credentials and tokens the fake issues and expects. The first +// exchange issues RefreshToken; exchange n after it issues RefreshToken +// followed by "-n", so every issued refresh token contains RefreshToken. const ( ClientID = "mealquest-test.apps.googleusercontent.com" ClientSecret = "google-client-s3cret-4f1c" @@ -49,6 +54,19 @@ type Google struct { codes map[string]string // code -> PKCE challenge tokenCalls int revoked []string + // issued is every refresh token issued, in order; dead is those a + // revoke has invalidated. + issued []string + dead map[string]bool + // tokenHold and revokeHold, when set, stop the next request to that + // endpoint. + tokenHold, revokeHold *hold +} + +// hold stops one request: arrived is closed when it arrives, and it goes on +// once release is closed. +type hold struct { + arrived, release chan struct{} } // New starts a fake Google that requires redirectURL at its token endpoint. @@ -61,6 +79,7 @@ func New(t *testing.T, redirectURL string) *Google { tokenStatus: http.StatusOK, revokeStatus: http.StatusOK, codes: map[string]string{}, + dead: map[string]bool{}, } mux := http.NewServeMux() mux.HandleFunc("POST /token", g.token) @@ -138,6 +157,55 @@ func (g *Google) Revoked() []string { return append([]string(nil), g.revoked...) } +// Live reports whether token was issued by the fake and no revoke has +// invalidated it since. +func (g *Google) Live(token string) bool { + g.mu.Lock() + defer g.mu.Unlock() + return slices.Contains(g.issued, token) && !g.dead[token] +} + +// HoldToken stops the next token request after its refresh token is issued +// and before it is answered, so a revoke can land in between. It returns a +// channel closed when the request arrives and the function that lets it go +// on. +func (g *Google) HoldToken() (arrived <-chan struct{}, release func()) { + g.mu.Lock() + defer g.mu.Unlock() + g.tokenHold = newHold() + return g.tokenHold.arrived, g.tokenHold.releaseFunc() +} + +// HoldRevoke stops the next revoke request before Google acts on it. It +// returns a channel closed when the request arrives and the function that +// lets it go on. +func (g *Google) HoldRevoke() (arrived <-chan struct{}, release func()) { + g.mu.Lock() + defer g.mu.Unlock() + g.revokeHold = newHold() + return g.revokeHold.arrived, g.revokeHold.releaseFunc() +} + +func newHold() *hold { + return &hold{arrived: make(chan struct{}), release: make(chan struct{})} +} + +func (h *hold) releaseFunc() func() { + var once sync.Once + return func() { once.Do(func() { close(h.release) }) } +} + +// wait stops the request at h, if any, without holding g.mu. +func (g *Google) wait(h *hold) { + if h == nil { + return + } + g.mu.Unlock() + close(h.arrived) + <-h.release + g.mu.Lock() +} + func (g *Google) token(w http.ResponseWriter, r *http.Request) { g.mu.Lock() defer g.mu.Unlock() @@ -168,8 +236,16 @@ func (g *Google) token(w http.ResponseWriter, r *http.Request) { } refresh := "" if !g.omitRefresh { - refresh = fmt.Sprintf(`"refresh_token":%q,`, RefreshToken) + issue := RefreshToken + if n := len(g.issued); n > 0 { + issue = fmt.Sprintf("%s-%d", RefreshToken, n+1) + } + g.issued = append(g.issued, issue) + refresh = fmt.Sprintf(`"refresh_token":%q,`, issue) } + h := g.tokenHold + g.tokenHold = nil + g.wait(h) w.Header().Set("Content-Type", "application/json") fmt.Fprintf(w, `{"access_token":%q,%s"expires_in":3599,"scope":%q,"token_type":"Bearer"}`, AccessToken, refresh, g.scope) @@ -186,8 +262,18 @@ func (g *Google) revoke(w http.ResponseWriter, r *http.Request) { http.Error(w, "bad form", http.StatusBadRequest) return } + h := g.revokeHold + g.revokeHold = nil + g.wait(h) token := r.PostForm.Get("token") g.revoked = append(g.revoked, token) + if g.revokeStatus == http.StatusOK && slices.Contains(g.issued, token) { + // Google revokes the grant, not just the token: every token issued + // to it so far stops working. + for _, t := range g.issued { + g.dead[t] = true + } + } body := g.revokeBody if body == "" && g.revokeStatus != http.StatusOK { body = "cannot revoke " + token diff --git a/internal/server/google.go b/internal/server/google.go index df5243c..4b8fc77 100644 --- a/internal/server/google.go +++ b/internal/server/google.go @@ -90,9 +90,6 @@ func deleteGoogle(logger *slog.Logger, svc *gcal.Service) http.HandlerFunc { w.WriteHeader(http.StatusNoContent) case errors.Is(err, gcal.ErrNotConnected): writeError(w, r, logger, http.StatusConflict, "not_connected", "Google Calendar is not connected") - case errors.Is(err, gcal.ErrChanged): - writeError(w, r, logger, http.StatusConflict, "conflict", - "the Google Calendar connection changed; reload and try again") case errors.As(err, &revoke): logger.WarnContext(r.Context(), "revoke Google access", "error", err) writeError(w, r, logger, http.StatusBadGateway, "revoke_failed", diff --git a/migrations/queries/settings.sql b/migrations/queries/settings.sql index e2b7a36..4ff78ee 100644 --- a/migrations/queries/settings.sql +++ b/migrations/queries/settings.sql @@ -106,12 +106,9 @@ ON CONFLICT (id) DO UPDATE SET updated_at = excluded.updated_at RETURNING *; --- name: DeleteIntegrationsGoogle :execrows --- Deletes the row only while it still holds refresh_token_ct, so a --- connection made after the caller read the row is kept. Returns the number --- of rows deleted. +-- name: DeleteIntegrationsGoogle :exec DELETE FROM integrations_google -WHERE id = 1 AND refresh_token_ct = ?; +WHERE id = 1; -- name: GetChatSettings :one -- Returns sql.ErrNoRows until the row exists. -- 2.55.0