feat(gmail): Gmail flytter ind i postkasse-tabellen
Ét sted at spørge "hvilke postkasser har den her person". Gmails tokens, udløser, etiket, set-liste og målprojekt ligger nu som en række i mailboxes ved siden af dem, der læses over IMAP, og baggrundsjobbet fejer én gang i stedet for to og skifter vej efter rækkens kind. PKCE-værdierne bliver i user_settings. De hører til ét forsøg på at logge ind, ikke til en postkasse, og de skal væk igen når forsøget er ovre. Målprojektet fik sin egen kolonne, som IMAP-postkasser også får glæde af — en arbejdspostkasse, der filer i arbejdsprojektet, er det åbenlyse at ville have. Ikke en fremmednøgle: sletter man projektet, skal destinationen stille holde op med at gælde, ikke afvise sletningen eller tage postkassen med sig. Flytningen kunne først køre nu. Den tømmer indstillingsrækken, og havde den ligget i v0.14.0, mens handlerne stadig læste den, var en fungerende postkasse blevet koblet fra. Værdierne kopieres som chiffertekst — begge tabeller forsegler med samme nøgle — så migrationen behøver ikke nøglen, hvilket en migration heller ikke har noget at gøre med at holde. Der er en prøve for præcis dét: en Gmail forbundet før flytningen, med et forseglet token skrevet som indstillingstabellen skrev dem, kommer ud på den anden side med token, udløser, adresse, målprojekt og set-liste i behold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f3fecbd0b4
commit
b0f78d1549
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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 '';
|
||||
+6
-8
@@ -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()
|
||||
@@ -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
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user