amadan

oficina / sheets Public

feat: an address becomes a stable actor id

Every authored column is a uint64 — Sheet.CreatedBy, Cell.UpdatedBy,
Event.ActorID — and the session carries an email address.
sessions.UserID cannot bridge the two, so members does.

The row is minted on first sight rather than at boot from
SHEETS_OWNERS: admission has already happened by then, and boot-time
rows would go stale the moment the allowlist changed. The write is an
upsert-then-read because two tabs arriving on one magic link is the
realistic case, and read-then-create loses it.

Address is plaintext per spec 2.2, which is also what makes the unique
index possible: sealing uses a fresh nonce per call, so no index could
ever match one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018RN4HhHR4rHH3Fxh4LMds5
Paul Campbell pushed by paul@keymail.dev 56e72fbfb8d64133041a5b7bfc9f5383a0558f9d parent 6c47bf9
7 files changed, +265 −3
  • internal/sheets/helpers_test.go +58 −0
  • internal/sheets/members.go +76 −0
  • internal/sheets/members_test.go +98 −0
  • internal/sheets/migrations.go +3 −3
  • internal/sheets/migrations/0002_create_members.sql +4 −0
  • internal/sheets/migrations/schema.sql +4 −0
  • internal/sheets/models.go +22 −0
diff --git a/internal/sheets/helpers_test.go b/internal/sheets/helpers_test.go
new file mode 100644
index 0000000..8bb1cf1
--- /dev/null
+++ b/internal/sheets/helpers_test.go
@@ -0,0 +1,58 @@
+package sheets
+
+import (
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "path/filepath"
+ "testing"
+
+ rdb "github.com/carlosframework/rastrillo/db"
+ "github.com/carlosframework/rastrillo/migrate"
+
+ "amadan.net/oficina/sheets/internal/sheets/seal"
+)
+
+// newTestApp builds an App over a real, migrated, throwaway database
+// and a deterministic all-zero DEK.
+//
+// Nil auth: these tests exercise storage, services and rendering, not
+// sign-in — appWithRecorder (begin_test.go) is the helper for that. A
+// nil auth means Router mounts no auth handlers and identity() answers
+// "nobody", which is why routes needing an actor take one explicitly in
+// these tests rather than reading a session.
+func newTestApp(t *testing.T) *App {
+ t.Helper()
+ d, err := rdb.Open(filepath.Join(t.TempDir(), "test.db"), quietLogger())
+ if err != nil {
+ t.Fatalf("db.Open: %v", err)
+ }
+ t.Cleanup(func() { d.Close() })
+ if _, err := migrate.Apply(context.Background(), d, BootSchema); err != nil {
+ t.Fatalf("migrate: %v", err)
+ }
+ s, err := seal.New(make([]byte, dekBytes))
+ if err != nil {
+ t.Fatalf("seal.New: %v", err)
+ }
+ return New(d.G, nil, s, func(string) bool { return true })
+}
+
+// testMember is the actor most service tests write as.
+func testMember(t *testing.T, app *App) Member {
+ t.Helper()
+ m, err := app.memberFor("ada@example.com")
+ if err != nil {
+ t.Fatalf("memberFor: %v", err)
+ }
+ return m
+}
+
+// do runs one request through the app's router and hands back the
+// recorder, so a test reads as one line per request.
+func do(t *testing.T, app *App, req *http.Request) *httptest.ResponseRecorder {
+ t.Helper()
+ rec := httptest.NewRecorder()
+ app.Router().ServeHTTP(rec, req)
+ return rec
+}
diff --git a/internal/sheets/members.go b/internal/sheets/members.go
new file mode 100644
index 0000000..117dc5e
--- /dev/null
+++ b/internal/sheets/members.go
@@ -0,0 +1,76 @@
+package sheets
+
+import (
+ "errors"
+ "net/http"
+ "strings"
+
+ "gorm.io/gorm"
+ "gorm.io/gorm/clause"
+)
+
+// errNoDB is what every service method answers when it is called on an
+// App built without storage. Tests that render pages do that
+// deliberately; production never can, because main refuses to boot
+// without a database.
+var errNoDB = errors.New("sheets: no database")
+
+// memberFor returns the Member row for an address, creating it the
+// first time that address is seen.
+//
+// First sight is the right moment because admission has already
+// happened: auth.Config.Authorize refused every address not on the
+// allowlist before a session could exist, so anything reaching here
+// belongs here. Minting rows at boot from SHEETS_OWNERS instead would
+// create members for people who never signed in, and would go stale the
+// moment the allowlist changed.
+//
+// The write is an upsert-then-read rather than a read-then-create: two
+// tabs signing in at the same moment would otherwise race, and the loser
+// would get a unique-constraint error instead of a member.
+func (a *App) memberFor(address string) (Member, error) {
+ addr := strings.ToLower(strings.TrimSpace(address))
+ if addr == "" {
+ // An empty address would mint one shared member that every
+ // unauthenticated path then wrote edits as — the uid-0 trap
+ // wearing a different hat.
+ return Member{}, errors.New("sheets: refusing to mint a member for an empty address")
+ }
+ if a.db == nil {
+ return Member{}, errNoDB
+ }
+ m := Member{Address: addr}
+ if err := a.db.Clauses(clause.OnConflict{DoNothing: true}).Create(&m).Error; err != nil {
+ return Member{}, err
+ }
+ if m.ID != 0 {
+ return m, nil
+ }
+ // The insert conflicted, so the row is already there: read it.
+ var existing Member
+ if err := a.db.Where("address = ?", addr).First(&existing).Error; err != nil {
+ if errors.Is(err, gorm.ErrRecordNotFound) {
+ return Member{}, errors.New("sheets: member neither created nor found")
+ }
+ return Member{}, err
+ }
+ return existing, nil
+}
+
+// actor resolves the signed-in viewer to a Member.
+//
+// No session is not an error — it is "no actor" — so callers gate on ok
+// rather than on err. Every route that calls this has already been
+// through requireSession, so a false here in production means the
+// session went away between the middleware and the handler.
+func (a *App) actor(r *http.Request) (Member, bool) {
+ address, ok := a.identity(r)
+ if !ok {
+ return Member{}, false
+ }
+ m, err := a.memberFor(address)
+ if err != nil {
+ return Member{}, false
+ }
+ return m, true
+}
diff --git a/internal/sheets/members_test.go b/internal/sheets/members_test.go
new file mode 100644
index 0000000..fd787fc
--- /dev/null
+++ b/internal/sheets/members_test.go
@@ -0,0 +1,98 @@
+package sheets
+
+import "testing"
+
+func TestMemberForIsIdempotent(t *testing.T) {
+ app := newTestApp(t)
+ a, err := app.memberFor("Ada@example.com")
+ if err != nil {
+ t.Fatalf("memberFor: %v", err)
+ }
+ if a.ID == 0 {
+ t.Fatal("memberFor returned id 0 — the uid-0 trap AGENTS.md names")
+ }
+ b, err := app.memberFor("ada@example.com")
+ if err != nil {
+ t.Fatalf("memberFor again: %v", err)
+ }
+ if b.ID != a.ID {
+ t.Fatalf("the same person got two ids (%d then %d): the address "+
+ "comparison is case-sensitive, but the allowlist's is not", a.ID, b.ID)
+ }
+}
+
+func TestMemberForSurroundingSpaceIsTheSamePerson(t *testing.T) {
+ app := newTestApp(t)
+ a, err := app.memberFor("ada@example.com")
+ if err != nil {
+ t.Fatalf("memberFor: %v", err)
+ }
+ b, err := app.memberFor(" ada@example.com ")
+ if err != nil {
+ t.Fatalf("memberFor padded: %v", err)
+ }
+ if a.ID != b.ID {
+ t.Fatalf("a padded address minted a second member: %d then %d", a.ID, b.ID)
+ }
+}
+
+func TestMemberForRejectsAnEmptyAddress(t *testing.T) {
+ app := newTestApp(t)
+ if _, err := app.memberFor(""); err == nil {
+ t.Fatal(`memberFor("") = nil error; an empty address must never mint a member`)
+ }
+ if _, err := app.memberFor(" "); err == nil {
+ t.Fatal("memberFor with only whitespace = nil error, want a refusal")
+ }
+}
+
+func TestTwoPeopleGetTwoIDs(t *testing.T) {
+ app := newTestApp(t)
+ ada, err := app.memberFor("ada@example.com")
+ if err != nil {
+ t.Fatalf("memberFor ada: %v", err)
+ }
+ bob, err := app.memberFor("bob@example.com")
+ if err != nil {
+ t.Fatalf("memberFor bob: %v", err)
+ }
+ if ada.ID == bob.ID {
+ t.Fatalf("two addresses share one actor id (%d); every edit would be misattributed", ada.ID)
+ }
+}
+
+// A concurrent first sight is the realistic case — two tabs, one
+// person, both arriving on the same magic link — and read-then-create
+// loses it.
+func TestMemberForIsSafeUnderConcurrentFirstSight(t *testing.T) {
+ app := newTestApp(t)
+ const n = 8
+ ids := make(chan uint64, n)
+ errs := make(chan error, n)
+ start := make(chan struct{})
+ for i := 0; i < n; i++ {
+ go func() {
+ <-start
+ m, err := app.memberFor("ada@example.com")
+ if err != nil {
+ errs <- err
+ return
+ }
+ ids <- m.ID
+ }()
+ }
+ close(start)
+ var first uint64
+ for i := 0; i < n; i++ {
+ select {
+ case err := <-errs:
+ t.Fatalf("concurrent memberFor: %v", err)
+ case id := <-ids:
+ if first == 0 {
+ first = id
+ } else if id != first {
+ t.Fatalf("concurrent first sight minted two members: %d and %d", first, id)
+ }
+ }
+ }
+}
diff --git a/internal/sheets/migrations.go b/internal/sheets/migrations.go
index 23af022..c4fc717 100644
--- a/internal/sheets/migrations.go
+++ b/internal/sheets/migrations.go
@@ -37,8 +37,8 @@ var Schema = migrate.MustFromFS(migrationsFS, "sheets")
var BootSchema = migrate.Merge(sessions.Schema, auth.Schema, Schema)
// Models lists every GORM model `rastrillo migration generate`/`check`
-// diff Schema against — Sheet, Tab, Row, Col, Cell and Format from
-// models.go, plus Event.
+// diff Schema against — Sheet, Tab, Row, Col, Cell, Format and Member
+// from models.go, plus Event.
//
// Event is declared in models.go, not internal/sheets/store/events.go
// where the rest of its behaviour (AppendInput, AppendRecalc, the Kind
@@ -56,5 +56,5 @@ var BootSchema = migrate.Merge(sessions.Schema, auth.Schema, Schema)
// keeps every existing store.Event call site, production and test,
// compiling unchanged.
var Models = []any{
- Sheet{}, Tab{}, Row{}, Col{}, Cell{}, Format{}, Event{},
+ Sheet{}, Tab{}, Row{}, Col{}, Cell{}, Format{}, Event{}, Member{},
}
diff --git a/internal/sheets/migrations/0002_create_members.sql b/internal/sheets/migrations/0002_create_members.sql
new file mode 100644
index 0000000..971adbf
--- /dev/null
+++ b/internal/sheets/migrations/0002_create_members.sql
@@ -0,0 +1,4 @@
+CREATE TABLE `members` (`id` integer PRIMARY KEY AUTOINCREMENT,`address` text,`created_at` datetime);
+
+CREATE UNIQUE INDEX `idx_members_address` ON `members`(`address`);
+
diff --git a/internal/sheets/migrations/schema.sql b/internal/sheets/migrations/schema.sql
index 878400f..dd67e05 100644
--- a/internal/sheets/migrations/schema.sql
+++ b/internal/sheets/migrations/schema.sql
@@ -9,6 +9,8 @@ CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text,`tab_i
CREATE TABLE `formats` (`id` integer PRIMARY KEY AUTOINCREMENT,`digest` text,`number_fmt` text,`bold` numeric,`italic` numeric,`fill_intent_h` real,`fill_intent_c` real,`fill_hex` text,`align` text);
+CREATE TABLE `members` (`id` integer PRIMARY KEY AUTOINCREMENT,`address` text,`created_at` datetime);
+
CREATE TABLE `rows` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer,`position` integer,`height` integer);
CREATE TABLE `sheets` (`id` integer PRIMARY KEY AUTOINCREMENT,`title_sealed` blob,`created_by` integer,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime);
@@ -23,6 +25,8 @@ CREATE INDEX `idx_events_tab_id` ON `events`(`tab_id`);
CREATE UNIQUE INDEX `idx_formats_digest` ON `formats`(`digest`);
+CREATE UNIQUE INDEX `idx_members_address` ON `members`(`address`);
+
CREATE UNIQUE INDEX `idx_row_tab_pos` ON `rows`(`tab_id`,`position`);
CREATE INDEX `idx_sheets_deleted_at` ON `sheets`(`deleted_at`);
diff --git a/internal/sheets/models.go b/internal/sheets/models.go
index 9a97fb4..b967cc6 100644
--- a/internal/sheets/models.go
+++ b/internal/sheets/models.go
@@ -101,3 +101,25 @@ type Event struct {
DerivedCount int // recalc only: how many cells the pass touched
CreatedAt time.Time
}
+
+// Member is one person who may hold a session on this instance, and the
+// stable uint64 an edit records as its actor.
+//
+// Address is plaintext, deliberately. Spec §2.2 lists "membership,
+// grants, and roles — including readable email addresses" among the
+// things this design does NOT seal, and publishes that in the README
+// rather than leaving it to be discovered. Sealing it would also make
+// the uniqueIndex below impossible: seal uses a random nonce per call,
+// so the same address seals to different ciphertext every time and no
+// index could ever match one.
+//
+// This table exists because the session carries an address and every
+// authored column — Sheet.CreatedBy, Cell.UpdatedBy, Event.ActorID — is
+// a uint64. sessions.UserID cannot bridge the two: it returns
+// (0, false) for an email Subject, and dropping that ok scopes every
+// query to uid 0 (AGENTS.md / Mechanical rules).
+type Member struct {
+ ID uint64 `gorm:"primaryKey"`
+ Address string `gorm:"uniqueIndex;size:320"`
+ CreatedAt time.Time
+}