feat(mailboxes): en tabel, så I kan have hver sin postkasse — og flere hver

Gmail lå i user_settings under en scope. Den tabel har user_id som hele
primærnøglen, så den rummer præcis én scope pr. person, og dens upsert skriver
scope-navnet om ved konflikt: at lægge IMAP ved siden af ville have slettet
Gmail-forbindelsen uden at sige det. Migration 0011 advarer ordret mod netop det,
og advarslen var tæt på at blive gået i for anden gang.

Formen var forkert alligevel. Man har postkasser, flertal — en arbejds-Gmail og
en privat iCloud er det almindelige tilfælde, ikke det sære. En række hver siger
det. Og den svarer på spørgsmålet, der satte det i gang: hver postkasse hører til
en bruger, så to mennesker på samme instans forbinder hver sin og ser kun sin
egen. Det eneste, der bliver fælles, er Googles OAuth-klient, og det er rigtigt:
den er appen, ikke kontoen.

Kodeord og tokens skrives forseglet. Forseglingen i notifications.go dækker kun
indstillingstabellerne, så denne tabels metoder kalder den i hånden — værd at
vide, før der kommer en tredje hemmelig kolonne.

Gmail bliver hvor den er indtil videre. Den virker, og at lægge en flytning af en
kørende forbindelse i samme ombæring som en ny integration gør begge dele sværere
at efterprøve. Skemaet har allerede plads til den.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

internal/store/mailboxes.go

@@ -0,0 +1,210 @@
+package store
+
+import (
+	"context"
+	"database/sql"
+	"fmt"
+	"time"
+)
+
+// Mailbox is one connected mailbox belonging to one person.
+type Mailbox struct {
+	ID     string `json:"id"`
+	UserID string `json:"-"`
+	Kind   string `json:"kind"` // "gmail" or "imap"
+	Name   string `json:"name"`
+
+	Host     string `json:"host,omitempty"`
+	Username string `json:"username,omitempty"`
+	Password string `json:"-"` // never leaves the server
+	Folder   string `json:"folder,omitempty"`
+
+	RefreshToken string    `json:"-"`
+	AccessToken  string    `json:"-"`
+	ExpiresAt    time.Time `json:"-"`
+	Label        string    `json:"label,omitempty"`
+
+	LastUID    uint32    `json:"last_uid,omitempty"`
+	LastSyncAt time.Time `json:"last_sync_at,omitempty"`
+	CreatedAt  time.Time `json:"created_at"`
+}
+
+const mailboxColumns = `id, user_id, kind, name, host, username, password, folder,
+	refresh_token, access_token, expires_at, label, last_uid, last_sync_at, created_at`
+
+// Mailboxes returns everything one person has connected, oldest first.
+func (db *DB) Mailboxes(ctx context.Context, userID string) ([]Mailbox, error) {
+	rows, err := db.QueryContext(ctx,
+		`SELECT `+mailboxColumns+` FROM mailboxes WHERE user_id = ? ORDER BY created_at`, userID)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+
+	var out []Mailbox
+	for rows.Next() {
+		m, err := db.scanMailbox(rows)
+		if err != nil {
+			return nil, err
+		}
+		out = append(out, m)
+	}
+	return out, rows.Err()
+}
+
+// Mailbox returns one, and only to the person it belongs to: the id is enough to
+// find a row, so the owner is part of the question rather than checked after.
+func (db *DB) Mailbox(ctx context.Context, userID, id string) (*Mailbox, error) {
+	row := db.QueryRowContext(ctx,
+		`SELECT `+mailboxColumns+` FROM mailboxes WHERE id = ? AND user_id = ?`, id, userID)
+	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.
+func (db *DB) SaveMailbox(ctx context.Context, m *Mailbox) error {
+	if m.ID == "" {
+		m.ID = NewID()
+	}
+	if m.CreatedAt.IsZero() {
+		m.CreatedAt = time.Now()
+	}
+	if m.Folder == "" {
+		m.Folder = "INBOX"
+	}
+
+	password, err := db.sealValue(m.Password)
+	if err != nil {
+		return fmt.Errorf("seal password: %w", err)
+	}
+	refresh, err := db.sealValue(m.RefreshToken)
+	if err != nil {
+		return fmt.Errorf("seal refresh_token: %w", err)
+	}
+	access, err := db.sealValue(m.AccessToken)
+	if err != nil {
+		return fmt.Errorf("seal access_token: %w", err)
+	}
+
+	_, err = db.ExecContext(ctx, `
+		INSERT INTO mailboxes (`+mailboxColumns+`)
+		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+		ON CONFLICT (id) DO UPDATE SET
+		    name = excluded.name,
+		    host = excluded.host,
+		    username = excluded.username,
+		    password = excluded.password,
+		    folder = excluded.folder,
+		    refresh_token = excluded.refresh_token,
+		    access_token = excluded.access_token,
+		    expires_at = excluded.expires_at,
+		    label = excluded.label,
+		    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.LastUID, unixOrZero(m.LastSyncAt), m.CreatedAt.Unix())
+	return err
+}
+
+// MarkMailboxRead records how far a run got. Written separately from the rest so
+// a sync never rewrites the credentials it is holding in memory.
+func (db *DB) MarkMailboxRead(ctx context.Context, id string, lastUID uint32, at time.Time) error {
+	_, err := db.ExecContext(ctx,
+		`UPDATE mailboxes SET last_uid = ?, last_sync_at = ? WHERE id = ?`,
+		lastUID, at.Unix(), id)
+	return err
+}
+
+// DeleteMailbox disconnects one. The tasks it made stay: they are the person's
+// work now, not the mailbox's.
+func (db *DB) DeleteMailbox(ctx context.Context, userID, id string) error {
+	_, err := db.ExecContext(ctx, `DELETE FROM mailboxes WHERE id = ? AND user_id = ?`, id, userID)
+	return err
+}
+
+// UsersWithMailboxes returns everybody the sweep has to visit.
+func (db *DB) UsersWithMailboxes(ctx context.Context) ([]string, error) {
+	rows, err := db.QueryContext(ctx, `SELECT DISTINCT user_id FROM mailboxes`)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+
+	var ids []string
+	for rows.Next() {
+		var id string
+		if err := rows.Scan(&id); err != nil {
+			return nil, err
+		}
+		ids = append(ids, id)
+	}
+	return ids, rows.Err()
+}
+
+type scanner interface {
+	Scan(dest ...any) error
+}
+
+func (db *DB) scanMailbox(row scanner) (Mailbox, error) {
+	var m Mailbox
+	var expires, lastSync, created int64
+	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.LastUID, &lastSync, &created)
+	if err != nil {
+		return m, err
+	}
+
+	for _, field := range []*string{&m.Password, &m.RefreshToken, &m.AccessToken} {
+		plain, err := db.unsealValue(*field)
+		if err != nil {
+			return m, err
+		}
+		*field = plain
+	}
+
+	m.ExpiresAt = timeOrZero(expires)
+	m.LastSyncAt = timeOrZero(lastSync)
+	m.CreatedAt = time.Unix(created, 0)
+	return m, nil
+}
+
+// sealValue and unsealValue are the single-string versions of the map helpers in
+// notifications.go. Without a key they pass the value through, which is what the
+// tests that never set one expect.
+func (db *DB) sealValue(v string) (string, error) {
+	if db.box == nil {
+		return v, nil
+	}
+	return db.box.Seal(v)
+}
+
+func (db *DB) unsealValue(v string) (string, error) {
+	if db.box == nil {
+		return v, nil
+	}
+	return db.box.Unseal(v)
+}
+
+func unixOrZero(t time.Time) int64 {
+	if t.IsZero() {
+		return 0
+	}
+	return t.Unix()
+}
+
+func timeOrZero(unix int64) time.Time {
+	if unix == 0 {
+		return time.Time{}
+	}
+	return time.Unix(unix, 0)
+}

internal/store/mailboxes_test.go

@@ -0,0 +1,173 @@
+package store
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestAMailboxIsStoredSealedAndComesBackWhole(t *testing.T) {
+	db, userID := sealedStore(t)
+	ctx := context.Background()
+
+	const password = "abcd-efgh-ijkl-mnop"
+	m := &Mailbox{
+		UserID:   userID,
+		Kind:     "imap",
+		Name:     "iCloud",
+		Host:     "imap.mail.me.com:993",
+		Username: "kw@icloud.com",
+		Password: password,
+	}
+	if err := db.SaveMailbox(ctx, m); err != nil {
+		t.Fatal(err)
+	}
+
+	var stored string
+	if err := db.QueryRowContext(ctx,
+		`SELECT password FROM mailboxes WHERE id = ?`, m.ID).Scan(&stored); err != nil {
+		t.Fatal(err)
+	}
+	if strings.Contains(stored, password) {
+		t.Fatalf("the password is in the row as typed: %s", stored)
+	}
+
+	back, err := db.Mailbox(ctx, userID, m.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if back == nil {
+		t.Fatal("the mailbox was not found")
+	}
+	if back.Password != password {
+		t.Errorf("got password %q", back.Password)
+	}
+	if back.Folder != "INBOX" {
+		t.Errorf("the folder defaulted to %q, want INBOX", back.Folder)
+	}
+}
+
+// The question that prompted the table: two people on one instance, each with
+// their own mailbox, neither seeing the other's.
+func TestAMailboxBelongsToOnePerson(t *testing.T) {
+	db, kristian := sealedStore(t)
+	ctx := context.Background()
+
+	andreas := &User{Email: "andreas@example.dk", Name: "Andreas"}
+	if err := db.CreateUser(ctx, andreas, "Indbakke"); err != nil {
+		t.Fatal(err)
+	}
+
+	mine := &Mailbox{UserID: kristian, Kind: "imap", Host: "imap.mail.me.com:993", Username: "kw"}
+	theirs := &Mailbox{UserID: andreas.ID, Kind: "imap", Host: "imap.fastmail.com:993", Username: "andreas"}
+	for _, m := range []*Mailbox{mine, theirs} {
+		if err := db.SaveMailbox(ctx, m); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	list, err := db.Mailboxes(ctx, kristian)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(list) != 1 || list[0].Username != "kw" {
+		t.Fatalf("got %d mailboxes for Kristian, want only his own", len(list))
+	}
+
+	// And the id alone is not enough to reach somebody else's.
+	other, err := db.Mailbox(ctx, kristian, theirs.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if other != nil {
+		t.Error("one person read another's mailbox by its id")
+	}
+}
+
+// Several mailboxes each is the ordinary case — a work Gmail and a private
+// iCloud — and is the whole reason this is a table rather than a setting.
+func TestOnePersonCanHaveSeveral(t *testing.T) {
+	db, userID := sealedStore(t)
+	ctx := context.Background()
+
+	for _, host := range []string{"imap.mail.me.com:993", "imap.fastmail.com:993"} {
+		if err := db.SaveMailbox(ctx, &Mailbox{
+			UserID: userID, Kind: "imap", Host: host, Username: "kw",
+		}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	list, err := db.Mailboxes(ctx, userID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(list) != 2 {
+		t.Fatalf("got %d mailboxes, want 2", len(list))
+	}
+}
+
+// The same mailbox twice would make two tasks out of every mail.
+func TestTheSameMailboxTwiceIsRefused(t *testing.T) {
+	db, userID := sealedStore(t)
+	ctx := context.Background()
+
+	first := &Mailbox{UserID: userID, Kind: "imap", Host: "imap.mail.me.com:993", Username: "kw"}
+	if err := db.SaveMailbox(ctx, first); err != nil {
+		t.Fatal(err)
+	}
+	again := &Mailbox{UserID: userID, Kind: "imap", Host: "imap.mail.me.com:993", Username: "kw"}
+	if err := db.SaveMailbox(ctx, again); err == nil {
+		t.Error("the same host and username was accepted a second time")
+	}
+}
+
+func TestReadingProgressIsRemembered(t *testing.T) {
+	db, userID := sealedStore(t)
+	ctx := context.Background()
+
+	m := &Mailbox{UserID: userID, Kind: "imap", Host: "imap.mail.me.com:993", Username: "kw"}
+	if err := db.SaveMailbox(ctx, m); err != nil {
+		t.Fatal(err)
+	}
+	at := time.Now().Truncate(time.Second)
+	if err := db.MarkMailboxRead(ctx, m.ID, 4711, at); err != nil {
+		t.Fatal(err)
+	}
+
+	back, err := db.Mailbox(ctx, userID, m.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if back.LastUID != 4711 {
+		t.Errorf("last uid is %d", back.LastUID)
+	}
+	if !back.LastSyncAt.Equal(at) {
+		t.Errorf("last sync is %v, want %v", back.LastSyncAt, at)
+	}
+	// The credentials it was holding must not have been rewritten as a side effect.
+	if back.Host != "imap.mail.me.com:993" {
+		t.Errorf("the host changed to %q", back.Host)
+	}
+}
+
+// Disconnecting takes the mailbox, not the work it produced.
+func TestDisconnectingKeepsTheTasks(t *testing.T) {
+	db, userID := sealedStore(t)
+	ctx := context.Background()
+
+	m := &Mailbox{UserID: userID, Kind: "imap", Host: "imap.mail.me.com:993", Username: "kw"}
+	if err := db.SaveMailbox(ctx, m); err != nil {
+		t.Fatal(err)
+	}
+	if err := db.DeleteMailbox(ctx, userID, m.ID); err != nil {
+		t.Fatal(err)
+	}
+	list, err := db.Mailboxes(ctx, userID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(list) != 0 {
+		t.Errorf("the mailbox is still connected")
+	}
+}

internal/store/migrations/0014_mailboxes.sql

@@ -0,0 +1,57 @@
+-- Mailboxes: one row per connected mailbox, not one setting per person.
+--
+-- Gmail lived in `user_settings` under scope 'gmail'. That table has `user_id` as
+-- its whole primary key, so it holds exactly one scope per person and its upsert
+-- rewrites the scope name on conflict — adding IMAP beside Gmail there would have
+-- deleted the Gmail connection without saying so. Migration 0011 warns about this
+-- in as many words, and the warning was nearly walked into a second time.
+--
+-- The shape is wrong for the thing anyway. A person has mailboxes, plural: a work
+-- Gmail and a private iCloud is the ordinary case, not the exotic one. A row each
+-- says that. It also answers the question that prompted this — every mailbox
+-- belongs to a user, so two people on the same instance connect their own and see
+-- only their own. The one piece that stays instance-wide is the Google OAuth
+-- client, which is a single registration everybody signs in through, and that is
+-- correct: it is the app, not the account.
+--
+-- `kind` rather than two tables. What differs is how the credential is obtained —
+-- OAuth for Gmail, a password for IMAP — and what differs is a nullable column,
+-- not a schema. What is the same is everything the sync loop touches: which
+-- mailbox, how far it has read, when it last ran.
+--
+-- `password` and `refresh_token` are written sealed; see internal/secret. The
+-- store's own seal/unseal only covers the settings tables, so this table's methods
+-- call them by hand — a fact worth knowing before adding a third secret column.
+--
+-- `last_uid` is IMAP's place-marker and means nothing for Gmail, which keeps its
+-- own by query. Zero is "nothing read yet" for both.
+CREATE TABLE mailboxes (
+    id            TEXT PRIMARY KEY,
+    user_id       TEXT NOT NULL REFERENCES users (id) ON DELETE CASCADE,
+    kind          TEXT NOT NULL CHECK (kind IN ('gmail', 'imap')),
+    name          TEXT NOT NULL DEFAULT '',
+
+    -- IMAP
+    host          TEXT NOT NULL DEFAULT '',
+    username      TEXT NOT NULL DEFAULT '',
+    password      TEXT NOT NULL DEFAULT '',
+    folder        TEXT NOT NULL DEFAULT 'INBOX',
+
+    -- Gmail
+    refresh_token TEXT NOT NULL DEFAULT '',
+    access_token  TEXT NOT NULL DEFAULT '',
+    expires_at    INTEGER NOT NULL DEFAULT 0,
+    label         TEXT NOT NULL DEFAULT '',
+
+    last_uid      INTEGER NOT NULL DEFAULT 0,
+    last_sync_at  INTEGER NOT NULL DEFAULT 0,
+    created_at    INTEGER NOT NULL
+);
+
+-- The sweep asks "whose mailboxes are due", so it reads by user.
+CREATE INDEX mailboxes_by_user ON mailboxes (user_id);
+
+-- The same mailbox twice is a mistake, not a choice: two rows would make two
+-- tasks out of every mail. Gmail's uniqueness is the account, IMAP's is the host
+-- and the username together, and `kind` keeps the two from colliding.
+CREATE UNIQUE INDEX mailboxes_once ON mailboxes (user_id, kind, host, username);