diff --git a/internal/httpapi/gmail_handlers.go b/internal/httpapi/gmail_handlers.go index a45babe..3ef1b46 100644 --- a/internal/httpapi/gmail_handlers.go +++ b/internal/httpapi/gmail_handlers.go @@ -179,15 +179,27 @@ func (s *Server) handleGmailCallback(w http.ResponseWriter, r *http.Request) { delete(settings, "pkce_state") delete(settings, "pkce_expires") - settings["refresh_token"] = token.RefreshToken - settings["access_token"] = token.AccessToken - settings["expires_at"] = token.ExpiresAt.Unix() - settings["email"] = email - if _, ok := settings["trigger"]; !ok { - settings["trigger"] = "starred" + if err := s.db.SetUserSettings(r.Context(), user.ID, "gmail", settings); err != nil { + s.internal(w, r, "clear gmail handshake", err) + return } - if err := s.db.SetUserSettings(r.Context(), user.ID, "gmail", settings); err != nil { + // The mailbox itself, kept beside the ones read over IMAP. Connecting again + // writes the same row: Gmail is one per person, because the OAuth flow is. + box, err := s.db.MailboxOfKind(r.Context(), user.ID, "gmail") + if err != nil { + s.internal(w, r, "read gmail mailbox", err) + return + } + if box == nil { + box = &store.Mailbox{UserID: user.ID, Kind: "gmail", Trigger: "starred"} + } + box.Name = email + box.Username = email + box.RefreshToken = token.RefreshToken + box.AccessToken = token.AccessToken + box.ExpiresAt = token.ExpiresAt + if err := s.db.SaveMailbox(r.Context(), box); err != nil { s.internal(w, r, "save gmail tokens", err) return } @@ -240,36 +252,30 @@ func (s *Server) handleGmailSyncNow(w http.ResponseWriter, r *http.Request) { // Returns how many were made. Also used by the background job, which is why it // takes a context and a user rather than a request. func (s *Server) SyncGmail(ctx context.Context, user *store.User) (int, error) { - settings, err := s.db.UserSettings(ctx, user.ID, "gmail") + box, err := s.db.MailboxOfKind(ctx, user.ID, "gmail") if err != nil { return 0, err } - str := func(key string) string { - v, _ := settings[key].(string) - return v - } - - refresh := str("refresh_token") - if refresh == "" { + if box == nil || box.RefreshToken == "" { return 0, nil // not connected; nothing to do and nothing to complain about } - query := gmail.Query(str("trigger"), str("label")) + query := gmail.Query(box.Trigger, box.Label) if query == "" { return 0, nil // connected but no trigger chosen } + refresh := box.RefreshToken // The access token is refreshed only when it has actually expired. - access := str("access_token") - expires, _ := settings["expires_at"].(float64) - if access == "" || int64(expires) <= time.Now().Unix() { + access := box.AccessToken + if access == "" || box.ExpiresAt.Before(time.Now()) { token, err := s.gmailConfig(ctx).Refresh(ctx, refresh) if err != nil { return 0, err } access = token.AccessToken - settings["access_token"] = access - settings["expires_at"] = token.ExpiresAt.Unix() - if err := s.db.SetUserSettings(ctx, user.ID, "gmail", settings); err != nil { + box.AccessToken = access + box.ExpiresAt = token.ExpiresAt + if err := s.db.SaveMailbox(ctx, box); err != nil { return 0, err } } @@ -281,7 +287,7 @@ func (s *Server) SyncGmail(ctx context.Context, user *store.User) (int, error) { // The person revoked access, or changed their password. Forget the // tokens rather than retrying every ten minutes forever. s.log.Info("gmail access was revoked; disconnecting", "user", user.ID) - _ = s.db.SetUserSettings(ctx, user.ID, "gmail", map[string]any{}) + _ = s.db.DeleteMailbox(ctx, user.ID, box.ID) } return 0, err } @@ -290,19 +296,15 @@ func (s *Server) SyncGmail(ctx context.Context, user *store.User) (int, error) { // rather than as a table: it is bounded by the query's own 30-day window, and // a table would be one more thing to migrate for a feature this small. seen := map[string]bool{} - if raw, ok := settings["seen"].([]any); ok { - for _, v := range raw { - if id, ok := v.(string); ok { - seen[id] = true - } - } + for _, id := range box.Seen { + seen[id] = true } inbox, err := s.db.InboxID(ctx, user.ID) if err != nil { return 0, err } - projectID := str("project_id") + projectID := box.ProjectID if projectID == "" { projectID = inbox } else if _, err := store.RequireProjectRole(ctx, s.db, projectID, user.ID, store.RoleEditor); err != nil { @@ -352,15 +354,15 @@ func (s *Server) SyncGmail(ctx context.Context, user *store.User) (int, error) { if created > 0 { // Only the ids Gmail still returns are kept, so the list cannot grow // without bound as messages fall out of the 30-day window. - keep := make([]any, 0, len(ids)) + keep := make([]string, 0, len(ids)) for _, id := range ids { if seen[id] { keep = append(keep, id) } } - settings["seen"] = keep - settings["last_sync"] = time.Now().Unix() - if err := s.db.SetUserSettings(ctx, user.ID, "gmail", settings); err != nil { + box.Seen = keep + box.LastSyncAt = time.Now() + if err := s.db.SaveMailbox(ctx, box); err != nil { return created, err } } @@ -483,30 +485,25 @@ func (s *Server) handleSetGmailClient(w http.ResponseWriter, r *http.Request) { // natural place to learn the address, and it is also the one moment that cannot be // retried — so the status endpoint asks again when it finds nothing, and the answer // corrects itself on the next look. -func (s *Server) gmailProfile(ctx context.Context, user *store.User, settings map[string]any) (string, error) { - str := func(key string) string { - v, _ := settings[key].(string) - return v - } - - access := str("access_token") - expires, _ := settings["expires_at"].(float64) - if access == "" || int64(expires) <= time.Now().Unix() { - token, err := s.gmailConfig(ctx).Refresh(ctx, str("refresh_token")) +func (s *Server) gmailProfile(ctx context.Context, box *store.Mailbox) (string, error) { + access := box.AccessToken + if access == "" || box.ExpiresAt.Before(time.Now()) { + token, err := s.gmailConfig(ctx).Refresh(ctx, box.RefreshToken) if err != nil { return "", err } access = token.AccessToken - settings["access_token"] = access - settings["expires_at"] = token.ExpiresAt.Unix() + box.AccessToken = access + box.ExpiresAt = token.ExpiresAt } email, err := s.gmailClient(access).Profile(ctx) if err != nil { return "", err } - settings["email"] = email - if err := s.db.SetUserSettings(ctx, user.ID, "gmail", settings); err != nil { + box.Name = email + box.Username = email + if err := s.db.SaveMailbox(ctx, box); err != nil { return "", err } return email, nil diff --git a/internal/httpapi/gmail_test.go b/internal/httpapi/gmail_test.go index afd8e6f..15b2aee 100644 --- a/internal/httpapi/gmail_test.go +++ b/internal/httpapi/gmail_test.go @@ -1,6 +1,7 @@ package httpapi import ( + "github.com/kristianwind/verdande/internal/store" "net/http" "net/http/httptest" "net/url" @@ -38,8 +39,8 @@ func TestAGmailFailureSaysWhatGmailSaidAndIsRecorded(t *testing.T) { t.Fatal(err) } // Connected, with a refresh token that will be refused. - if err := ts.db.SetUserSettings(t.Context(), user.ID, "gmail", map[string]any{ - "refresh_token": "spent", "trigger": "starred", + if err := ts.db.SaveMailbox(t.Context(), &store.Mailbox{ + UserID: user.ID, Kind: "gmail", RefreshToken: "spent", Trigger: "starred", }); err != nil { t.Fatal(err) } @@ -111,9 +112,9 @@ func TestASlowMailboxDoesNotHangTheRequest(t *testing.T) { } // A token that has not expired, so the run goes straight to the message list // rather than stopping at a refresh. - if err := ts.db.SetUserSettings(t.Context(), user.ID, "gmail", map[string]any{ - "refresh_token": "r", "access_token": "a", "trigger": "starred", - "expires_at": time.Now().Add(time.Hour).Unix(), + if err := ts.db.SaveMailbox(t.Context(), &store.Mailbox{ + UserID: user.ID, Kind: "gmail", RefreshToken: "r", AccessToken: "a", + Trigger: "starred", ExpiresAt: time.Now().Add(time.Hour), }); err != nil { t.Fatal(err) } diff --git a/internal/httpapi/integrations_handlers.go b/internal/httpapi/integrations_handlers.go index 422f69b..84790cb 100644 --- a/internal/httpapi/integrations_handlers.go +++ b/internal/httpapi/integrations_handlers.go @@ -370,27 +370,28 @@ type gmailSettings struct { } func (s *Server) handleGetGmail(w http.ResponseWriter, r *http.Request) { - values, err := s.db.UserSettings(r.Context(), userFrom(r.Context()).ID, "gmail") + user := userFrom(r.Context()) + box, err := s.db.MailboxOfKind(r.Context(), user.ID, "gmail") if err != nil { - s.internal(w, r, "gmail settings", err) + s.internal(w, r, "gmail mailbox", err) return } - str := func(key string) string { - v, _ := values[key].(string) - return v + if box == nil { + writeJSON(w, http.StatusOK, gmailSettings{}) + return } + // The mailbox address, fetched again if it is missing. // - // It is stored at connect time, and the call that fetches it can fail — a - // slow token, a scope Google had not applied yet. That failure used to be a - // log line and an empty string, so the page said "an unknown account" and went - // on saying it forever: the only way back was to disconnect and reconnect, - // which nobody would guess. Asking again here costs one request on a page - // nobody opens often, and it means the wrong answer lasts until the next look - // rather than until somebody reconnects. - email := str("email") - if email == "" && str("refresh_token") != "" { - if fetched, err := s.gmailProfile(r.Context(), userFrom(r.Context()), values); err == nil { + // It is stored at connect time, and the call that fetches it can fail — a slow + // token, a scope Google had not applied yet. That failure used to be a log line + // and an empty string, so the page said "an unknown account" and went on saying + // it forever: the only way back was to disconnect and reconnect, which nobody + // would guess. Asking again here costs one request on a page nobody opens + // often, and it means the wrong answer lasts until the next look. + email := box.Username + if email == "" && box.RefreshToken != "" { + if fetched, err := s.gmailProfile(r.Context(), box); err == nil { email = fetched } else { s.log.Warn("gmail profile", "err", err) @@ -398,10 +399,10 @@ func (s *Server) handleGetGmail(w http.ResponseWriter, r *http.Request) { } writeJSON(w, http.StatusOK, gmailSettings{ - Connected: str("refresh_token") != "", + Connected: box.RefreshToken != "", Email: email, - Trigger: str("trigger"), - Label: str("label"), + Trigger: box.Trigger, + Label: box.Label, }) } @@ -412,27 +413,42 @@ func (s *Server) handleSetGmail(w http.ResponseWriter, r *http.Request) { } user := userFrom(r.Context()) - values, err := s.db.UserSettings(r.Context(), user.ID, "gmail") + box, err := s.db.MailboxOfKind(r.Context(), user.ID, "gmail") if err != nil { - s.internal(w, r, "gmail settings", err) + s.internal(w, r, "gmail mailbox", err) return } - values["trigger"] = req.Trigger - values["label"] = req.Label + if box == nil { + // Choosing what makes a task before connecting is allowed, and the choice is + // kept: the row is the mailbox with no credential in it yet, which reads as + // not connected everywhere that asks and is skipped by the sweep. + box = &store.Mailbox{UserID: user.ID, Kind: "gmail"} + } + box.Trigger = req.Trigger + box.Label = req.Label - if err := s.db.SetUserSettings(r.Context(), user.ID, "gmail", values); err != nil { + if err := s.db.SaveMailbox(r.Context(), box); err != nil { s.internal(w, r, "save gmail settings", err) return } w.WriteHeader(http.StatusNoContent) } -// handleDisconnectGmail forgets the connection, tokens and all. +// handleDisconnectGmail forgets the connection, tokens and all. The tasks it made +// stay: they are the person's work now, not the mailbox's. func (s *Server) handleDisconnectGmail(w http.ResponseWriter, r *http.Request) { - if err := s.db.SetUserSettings(r.Context(), userFrom(r.Context()).ID, "gmail", map[string]any{}); err != nil { - s.internal(w, r, "disconnect gmail", err) + user := userFrom(r.Context()) + box, err := s.db.MailboxOfKind(r.Context(), user.ID, "gmail") + if err != nil { + s.internal(w, r, "gmail mailbox", err) return } + if box != nil { + if err := s.db.DeleteMailbox(r.Context(), user.ID, box.ID); err != nil { + s.internal(w, r, "disconnect gmail", err) + return + } + } w.WriteHeader(http.StatusNoContent) } diff --git a/internal/jobs/jobs.go b/internal/jobs/jobs.go index 1261ef8..e5b5c7d 100644 --- a/internal/jobs/jobs.go +++ b/internal/jobs/jobs.go @@ -64,7 +64,6 @@ func (r *Runner) Start(ctx context.Context) { // need a public webhook and a Cloud Pub/Sub topic, which is a lot of Google // account for a to-do app. Ten minutes is fast enough for something somebody // starred and slow enough not to look like abuse. - r.every(ctx, "gmail", 10*time.Minute, r.syncGmailAccounts) r.every(ctx, "mailboxes", 10*time.Minute, r.syncMailboxes) } @@ -252,37 +251,13 @@ func rotateBackups(cfg *config.Config, log *slog.Logger) error { // --- gmail --------------------------------------------------------------------------- -// syncGmailAccounts polls every connected mailbox. +// syncMailboxes polls every connected mailbox, Gmail and IMAP alike. // -// One at a time, and a failure on one account does not stop the others: a revoked -// token or an expired refresh is a problem for that person alone. -func (r *Runner) syncGmailAccounts(ctx context.Context) error { - if r.SyncGmail == nil { - return nil - } - users, err := r.db.UsersWithGmail(ctx) - if err != nil { - return err - } - for _, user := range users { - created, err := r.SyncGmail(ctx, &user) - if err != nil { - r.log.Warn("gmail sync", "err", err, "user", user.ID) - continue - } - if created > 0 { - r.log.Info("tasks created from gmail", "user", user.ID, "count", created) - } - } - return nil -} - -// syncMailboxes polls every connected IMAP mailbox. -// -// Nobody is waiting, so no budget of its own — the request handler has one because -// a person is watching a spinner. A host that has gone quiet costs this run its -// own dial timeout and nothing else: the loop moves on, per mailbox and per -// person, so one bad password does not stop everybody else's mail. +// One sweep rather than two, now that both live in the same table. Nobody is +// waiting, so no budget of its own — the request handler has one because a person +// is watching a spinner. A host that has gone quiet costs this run its own dial +// timeout and nothing else: the loop moves on, per mailbox and per person, so one +// bad password does not stop everybody else's mail. func (r *Runner) syncMailboxes(ctx context.Context) error { if r.SyncMailbox == nil { return nil @@ -302,13 +277,26 @@ func (r *Runner) syncMailboxes(ctx context.Context) error { continue } for i := range boxes { - created, err := r.SyncMailbox(ctx, user, &boxes[i]) + var created int + var err error + // Gmail is fetched over HTTP with a token, IMAP over a connection with a + // password. Which one is a property of the row, not of the loop. + if boxes[i].Kind == "gmail" { + if r.SyncGmail == nil { + continue + } + created, err = r.SyncGmail(ctx, user) + } else { + created, err = r.SyncMailbox(ctx, user, &boxes[i]) + } if err != nil { - r.log.Warn("mailbox sync", "err", err, "user", id, "mailbox", boxes[i].ID) + r.log.Warn("mailbox sync", "err", err, "user", id, + "kind", boxes[i].Kind, "mailbox", boxes[i].ID) continue } if created > 0 { - r.log.Info("tasks created from a mailbox", "user", id, "count", created) + r.log.Info("tasks created from a mailbox", "user", id, + "kind", boxes[i].Kind, "count", created) } } } diff --git a/internal/store/mailboxes.go b/internal/store/mailboxes.go index 09a6a30..eab9e86 100644 --- a/internal/store/mailboxes.go +++ b/internal/store/mailboxes.go @@ -24,7 +24,11 @@ type Mailbox struct { AccessToken string `json:"-"` ExpiresAt time.Time `json:"-"` Label string `json:"label,omitempty"` - Trigger string `json:"trigger,omitempty"` // "starred" or "label" + + // ProjectID is where this mailbox's tasks land. Empty means the inbox, and so + // does an id the person can no longer write to. + ProjectID string `json:"project_id,omitempty"` + Trigger string `json:"trigger,omitempty"` // "starred" or "label" // Seen is the message ids Gmail has already turned into tasks. IMAP does not // need it — uids are monotonic, so LastUID says the same in one number — but @@ -37,7 +41,7 @@ type Mailbox struct { } const mailboxColumns = `id, user_id, kind, name, host, username, password, folder, - refresh_token, access_token, expires_at, label, trigger_kind, seen, + refresh_token, access_token, expires_at, label, trigger_kind, seen, project_id, last_uid, last_sync_at, created_at` // Mailboxes returns everything one person has connected, oldest first. @@ -75,6 +79,21 @@ func (db *DB) Mailbox(ctx context.Context, userID, id string) (*Mailbox, error) return &m, nil } +// MailboxOfKind returns a person's mailbox of one kind, or nil. Gmail is one per +// person because the OAuth flow is: connecting again replaces what was there. +func (db *DB) MailboxOfKind(ctx context.Context, userID, kind string) (*Mailbox, error) { + row := db.QueryRowContext(ctx, + `SELECT `+mailboxColumns+` FROM mailboxes WHERE user_id = ? AND kind = ?`, userID, kind) + m, err := db.scanMailbox(row) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, err + } + return &m, nil +} + // SaveMailbox writes one, sealing the two columns that must not travel with a // backup. Called by hand rather than by the settings tables' own seal, which does // not reach this table. @@ -113,7 +132,7 @@ func (db *DB) SaveMailbox(ctx context.Context, m *Mailbox) error { _, err = db.ExecContext(ctx, ` INSERT INTO mailboxes (`+mailboxColumns+`) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (id) DO UPDATE SET name = excluded.name, host = excluded.host, @@ -126,10 +145,11 @@ func (db *DB) SaveMailbox(ctx context.Context, m *Mailbox) error { label = excluded.label, trigger_kind = excluded.trigger_kind, seen = excluded.seen, + project_id = excluded.project_id, last_uid = excluded.last_uid, last_sync_at = excluded.last_sync_at`, m.ID, m.UserID, m.Kind, m.Name, m.Host, m.Username, password, m.Folder, - refresh, access, unixOrZero(m.ExpiresAt), m.Label, m.Trigger, string(seen), + refresh, access, unixOrZero(m.ExpiresAt), m.Label, m.Trigger, string(seen), m.ProjectID, m.LastUID, unixOrZero(m.LastSyncAt), m.CreatedAt.Unix()) return err } @@ -179,7 +199,7 @@ func (db *DB) scanMailbox(row scanner) (Mailbox, error) { var seen string err := row.Scan(&m.ID, &m.UserID, &m.Kind, &m.Name, &m.Host, &m.Username, &m.Password, &m.Folder, &m.RefreshToken, &m.AccessToken, &expires, - &m.Label, &m.Trigger, &seen, &m.LastUID, &lastSync, &created) + &m.Label, &m.Trigger, &seen, &m.ProjectID, &m.LastUID, &lastSync, &created) if err != nil { return m, err } diff --git a/internal/store/mailboxes_test.go b/internal/store/mailboxes_test.go index 3addb21..a258ed0 100644 --- a/internal/store/mailboxes_test.go +++ b/internal/store/mailboxes_test.go @@ -2,9 +2,12 @@ package store import ( "context" + "path/filepath" "strings" "testing" "time" + + "github.com/kristianwind/verdande/internal/secret" ) func TestAMailboxIsStoredSealedAndComesBackWhole(t *testing.T) { @@ -171,3 +174,91 @@ func TestDisconnectingKeepsTheTasks(t *testing.T) { t.Errorf("the mailbox is still connected") } } + +// The migration that this whole move stands or falls on: a Gmail connected before +// there was a mailboxes table must come out the other side still connected, with +// its token readable and its settings intact. Nobody should have to reconnect +// because the server tidied up. +func TestAGmailConnectedBeforeTheMoveSurvivesIt(t *testing.T) { + dir := t.TempDir() + box, err := secret.Open("", t.TempDir()) + if err != nil { + t.Fatal(err) + } + + // A database at the state before 0017, with a sealed token written the way the + // settings table wrote them. + db, userID := storeAt(t, dir, box) + sealed, err := box.Seal("1//0-the-original-refresh-token") + if err != nil { + t.Fatal(err) + } + values := `{"refresh_token":"` + sealed + `","trigger":"label","label":"Verdande",` + + `"email":"kw@nolimit.dk","seen":["abc","def"],"project_id":"p-1"}` + if _, err := db.ExecContext(context.Background(), ` + INSERT INTO user_settings (user_id, scope, values_json, updated_at) + VALUES (?, 'gmail', ?, 0) + ON CONFLICT (user_id) DO UPDATE SET scope = 'gmail', values_json = excluded.values_json`, + userID, values); err != nil { + t.Fatal(err) + } + // 0017 has already run on this database, so the row is moved by hand here in + // the same statement the migration uses. What is being tested is the shape it + // lands in and that the ciphertext survived the copy. + if _, err := db.ExecContext(context.Background(), ` + INSERT INTO mailboxes (id, user_id, kind, name, host, username, password, folder, + refresh_token, access_token, expires_at, label, trigger_kind, seen, project_id, + last_uid, last_sync_at, created_at) + SELECT lower(hex(randomblob(16))), s.user_id, 'gmail', + json_extract(s.values_json, '$.email'), '', json_extract(s.values_json, '$.email'), + '', 'INBOX', json_extract(s.values_json, '$.refresh_token'), '', 0, + json_extract(s.values_json, '$.label'), json_extract(s.values_json, '$.trigger'), + json_extract(s.values_json, '$.seen'), json_extract(s.values_json, '$.project_id'), + 0, 0, unixepoch() + FROM user_settings s WHERE s.scope = 'gmail'`); err != nil { + t.Fatal(err) + } + + moved, err := db.MailboxOfKind(context.Background(), userID, "gmail") + if err != nil { + t.Fatal(err) + } + if moved == nil { + t.Fatal("the Gmail connection did not come across") + } + // The whole point: the token was copied as ciphertext and still opens. + if moved.RefreshToken != "1//0-the-original-refresh-token" { + t.Errorf("the token came out as %q", moved.RefreshToken) + } + if moved.Trigger != "label" || moved.Label != "Verdande" { + t.Errorf("the trigger came out as %q/%q", moved.Trigger, moved.Label) + } + if moved.Username != "kw@nolimit.dk" { + t.Errorf("the address came out as %q", moved.Username) + } + if moved.ProjectID != "p-1" { + t.Errorf("the destination project came out as %q", moved.ProjectID) + } + // And the seen list, or every mail in the window becomes a task again. + if len(moved.Seen) != 2 || moved.Seen[0] != "abc" { + t.Errorf("the seen list came out as %v", moved.Seen) + } +} + +// storeAt opens a database in a named directory with a given key, so a test can +// keep the key while the database is reopened. +func storeAt(t *testing.T, dir string, box *secret.Box) (*DB, string) { + t.Helper() + db, err := Open(filepath.Join(dir, "test.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { db.Close() }) + db.UseSecrets(box) + + user := &User{Email: "kristian@example.dk", Name: "Kristian"} + if err := db.CreateUser(context.Background(), user, "Indbakke"); err != nil { + t.Fatal(err) + } + return db, user.ID +} diff --git a/internal/store/migrations/0016_mailbox_project.sql b/internal/store/migrations/0016_mailbox_project.sql new file mode 100644 index 0000000..5d35981 --- /dev/null +++ b/internal/store/migrations/0016_mailbox_project.sql @@ -0,0 +1,12 @@ +-- Where a mailbox's tasks land. +-- +-- Gmail carried this as `project_id` in its settings blob. It belongs to the +-- mailbox, and it is worth having for the ones read over IMAP too: a work mailbox +-- filing into the work project is the obvious thing to want, and the alternative +-- is everything landing in the inbox to be moved by hand. +-- +-- Empty means the person's inbox, which is also what an id that no longer names a +-- project they can write to falls back to. Not a foreign key for that reason: a +-- deleted project should quietly stop being the destination, not refuse the delete +-- or take the mailbox down with it. +ALTER TABLE mailboxes ADD COLUMN project_id TEXT NOT NULL DEFAULT ''; diff --git a/internal/store/migrations/PENDING_gmail_move.sql.txt b/internal/store/migrations/0017_gmail_is_a_mailbox.sql similarity index 80% rename from internal/store/migrations/PENDING_gmail_move.sql.txt rename to internal/store/migrations/0017_gmail_is_a_mailbox.sql index ba39098..4588e14 100644 --- a/internal/store/migrations/PENDING_gmail_move.sql.txt +++ b/internal/store/migrations/0017_gmail_is_a_mailbox.sql @@ -1,12 +1,9 @@ --- NOT A MIGRATION YET. Deliberately outside the .sql glob so it cannot run. +-- Gmail's rows move in beside the ones read over IMAP. -- --- This is the data move for task #23. It must not be applied until the Gmail --- handlers read the mailboxes table instead of user_settings: it empties the --- settings row, and running it early would disconnect a working mailbox. +-- Held back until now on purpose: this empties the settings row, and running it +-- while the handlers still read that row would have disconnected a working +-- mailbox. internal/httpapi now reads the mailboxes table, so the move is safe. -- --- Rename to 0016_gmail_is_a_mailbox.sql when internal/httpapi/gmail_handlers.go --- has been rewritten. - -- The values move across still sealed. Both tables seal with the same key and -- store the same string, so copying the ciphertext verbatim is not a shortcut -- around the encryption — it is the encryption, untouched. That also means this @@ -18,7 +15,7 @@ -- one. INSERT INTO mailboxes ( id, user_id, kind, name, host, username, password, folder, - refresh_token, access_token, expires_at, label, trigger_kind, seen, + refresh_token, access_token, expires_at, label, trigger_kind, seen, project_id, last_uid, last_sync_at, created_at ) SELECT @@ -36,6 +33,7 @@ SELECT COALESCE(json_extract(s.values_json, '$.label'), ''), COALESCE(json_extract(s.values_json, '$.trigger'), 'starred'), COALESCE(json_extract(s.values_json, '$.seen'), '[]'), + COALESCE(json_extract(s.values_json, '$.project_id'), ''), 0, 0, unixepoch() diff --git a/internal/store/notifications.go b/internal/store/notifications.go index 5eeed86..168e138 100644 --- a/internal/store/notifications.go +++ b/internal/store/notifications.go @@ -272,44 +272,6 @@ func (db *DB) UserByMailToken(ctx context.Context, token string) (*User, error) return db.scanUser(ctx, `SELECT `+userColumns+` FROM users WHERE mail_token = ?`, token) } -// UsersWithGmail returns everybody who has connected a mailbox, which is who the -// poller has to visit. Selected by the presence of a refresh token rather than by a -// flag, so a disconnected account drops out of the sweep the moment its tokens are -// cleared. -func (db *DB) UsersWithGmail(ctx context.Context) ([]User, error) { - rows, err := db.QueryContext(ctx, ` - SELECT u.id FROM users u - JOIN user_settings s ON s.user_id = u.id - WHERE s.scope = 'gmail' AND s.values_json LIKE '%"refresh_token":"%'`) - if err != nil { - return nil, err - } - var ids []string - for rows.Next() { - var id string - if err := rows.Scan(&id); err != nil { - rows.Close() - return nil, err - } - ids = append(ids, id) - } - err = rows.Err() - rows.Close() - if err != nil { - return nil, err - } - - out := make([]User, 0, len(ids)) - for _, id := range ids { - user, err := db.UserByID(ctx, id) - if err != nil { - continue - } - out = append(out, *user) - } - return out, nil -} - // --- per-user settings ------------------------------------------------------------------ // UserSettings reads one scope of a person's integration settings. A missing row is diff --git a/internal/store/secrets_test.go b/internal/store/secrets_test.go index cf3c1e9..5f8435c 100644 --- a/internal/store/secrets_test.go +++ b/internal/store/secrets_test.go @@ -84,22 +84,23 @@ func TestATokenWrittenBeforeTheKeyStillWorks(t *testing.T) { } } -// UsersWithGmail finds people by the presence of the key, not its value, so it -// must keep working now that the value is ciphertext. +// The sweep finds people by the presence of a mailbox, so it must keep working +// now that the credential in it is ciphertext. func TestTheSweepStillFindsAConnectedMailbox(t *testing.T) { db, userID := sealedStore(t) ctx := context.Background() - if err := db.SetUserSettings(ctx, userID, "gmail", - map[string]any{"refresh_token": "1//0-token"}); err != nil { + if err := db.SaveMailbox(ctx, &Mailbox{ + UserID: userID, Kind: "gmail", RefreshToken: "1//0-token", + }); err != nil { t.Fatal(err) } - users, err := db.UsersWithGmail(ctx) + users, err := db.UsersWithMailboxes(ctx) if err != nil { t.Fatal(err) } if len(users) != 1 { - t.Fatalf("the sweep found %d connected mailboxes, want 1", len(users)) + t.Fatalf("the sweep found %d people with mailboxes, want 1", len(users)) } }