amadan

oficina / docs Public

Sharing, slice 1: grants, and a write path that authorizes the writer

§8.2 has described sharing since this spec was written and nothing
implemented it. documents.owner_id was the entire authorization model and
every signed-in person saw only their own rows — so "share this document"
had no answer, in an app whose whole subject is documents people share.

A grants table, a readable() seam beside owned(), and ApplyUpdate taking the
acting person.

Grants are keyed on a lowercased address, not a people.id. A grant has to be
makeable to somebody who has never signed in — that is most of what sharing
is for — and EnsurePerson's create branch is where a new person gets seeded
a sample library, so minting a Person row to hold a grant would hand an
invitee six sample documents before they had accepted. Keyed on the address
the grant simply waits, and there is a test for exactly that. It is also the
subject the suite doctrine uses in lone shape, so a home later adds a kind
rather than a different meaning.

readable() sits beside owned() rather than widening it. owned still means
ownership, so no call site that genuinely wants an owner silently starts
accepting grantees — which is what editing it in place would have done, and
TestOwnedScopesByOwner still passes unchanged, which is what it was written
for. The grant test is an EXISTS subquery, not a join: callers chain
.Where("sqid = ?"), .Order and .Find onto this seam and a join would hand
them duplicate rows and an ambiguous id. owner_id > 0 leads the clause so an
orphan stays invisible even when a grant points at it — 0002_owners.sql
wrote owner 0 onto every pre-existing row and three tests pin that 0 matches
nothing; a grants clause must not become the way around them.

ApplyUpdate is the dangerous half. It verified owned(db, doc.OwnerID) against
doc.ID — the document against its own owner, true of every document by
construction. Harmless while the only way to hold a *Document was an
owner-scoped fetch, so it was a belt over the read scope's braces; not
harmless the moment a reader can legitimately fetch one, because the check
never asked anything about the person writing. store.go's comment records
this class of bug being fixed twice already. This is the third, caught before
it shipped. It now authorizes the actor against the row, and refuses a reader
exactly as it refuses a stranger — a 404 at the handler, per §4.2.

Two consequences worth their own note. The store no longer trusts a caller's
struct, which re-pointed TestApplyUpdateRejectsAMismatchedOwner: it tampered
OwnerID on a copy and asserted refusal, which only worked because
authorization read the struct. A tampered field is now ignored, so the old
assertion would pass for the wrong reason. It asserts the real hazard
instead — a stranger holding an accurate document is refused — and keeps the
tampered struct deliberately unused to prove a caller cannot make themselves
an owner by editing their own copy. And seeding writes as the new person, so
a sample library goes through the same authorization as any other edit
rather than around it.

Full suite green, including the lint guards. No UI yet: that is slice 2.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 1ef53df7b3ff450dd178b64a07178114ac0303fe parent f6cf5c2
17 files changed, +637 −45
  • cmd/docs/main.go +1 −1
  • internal/docs/cursor_test.go +4 −4
  • internal/docs/editor_view_test.go +1 −1
  • internal/docs/export_test.go +2 −2
  • internal/docs/grants.go +142 −0
  • internal/docs/grants_test.go +187 −0
  • internal/docs/grants_write_test.go +128 −0
  • internal/docs/handlers.go +34 −1
  • internal/docs/list_test.go +3 −3
  • internal/docs/migrations/0005_grants.sql +40 −0
  • internal/docs/seed.go +6 −2
  • internal/docs/seed_test.go +6 −6
  • internal/docs/snapshots_test.go +12 −12
  • internal/docs/store.go +24 −7
  • internal/docs/store_test.go +21 −5
  • internal/docs/stream_test.go +1 −1
  • internal/docs/testing.go +25 −0

CI passed — run details

Intent

full ledger: sharing

diff --git a/cmd/docs/main.go b/cmd/docs/main.go
index 7656c6a..7f68336 100644
--- a/cmd/docs/main.go
+++ b/cmd/docs/main.go
@@ -303,7 +303,7 @@ func main() {
logger.Error("docs: seed: resolve person", "address", seedAddress, "err", err)
os.Exit(1)
}
- if err := docs.Seed(context.Background(), app.Store(), owner); err != nil {
+ if err := docs.Seed(context.Background(), app.Store(), docs.Actor{PersonID: owner, Subject: seedAddress}); err != nil {
logger.Error("docs: seed", "address", seedAddress, "err", err)
os.Exit(1)
}
diff --git a/internal/docs/cursor_test.go b/internal/docs/cursor_test.go
index 0dbcce3..888a3c3 100644
--- a/internal/docs/cursor_test.go
+++ b/internal/docs/cursor_test.go
@@ -218,7 +218,7 @@ func pastCadence(t *testing.T, app *App, title string) *Document {
t.Fatalf("Create: %v", err)
}
for i := 0; i < snapshotCadence+3; i++ {
- if _, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t)); err != nil {
t.Fatalf("ApplyUpdate %d: %v", i, err)
}
}
@@ -407,7 +407,7 @@ func TestRejectedUpdateConsumesNoSeq(t *testing.T) {
t.Fatalf("Create: %v", err)
}
- seq, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t))
+ seq, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t))
if err != nil {
t.Fatalf("ApplyUpdate: %v", err)
}
@@ -415,11 +415,11 @@ func TestRejectedUpdateConsumesNoSeq(t *testing.T) {
t.Fatalf("first ApplyUpdate returned seq %d, want 1", seq)
}
- if _, err := app.Store().ApplyUpdate(ctx, doc, []byte{0xff, 0x00, 0x13, 0x37}); err == nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), []byte{0xff, 0x00, 0x13, 0x37}); err == nil {
t.Fatal("ApplyUpdate accepted garbage")
}
- seq, err = app.Store().ApplyUpdate(ctx, doc, validUpdate(t))
+ seq, err = app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t))
if err != nil {
t.Fatalf("ApplyUpdate after a rejection: %v", err)
}
diff --git a/internal/docs/editor_view_test.go b/internal/docs/editor_view_test.go
index c2c1966..986119c 100644
--- a/internal/docs/editor_view_test.go
+++ b/internal/docs/editor_view_test.go
@@ -20,7 +20,7 @@ func TestShowRendersContentWithoutScript(t *testing.T) {
owner := app.TestPerson(t, "reader@example.com")
doc, _ := app.Store().Create(ctx, owner, "Readable")
update := readFixture(t, "../../fixtures/vectors/fragment/simple.bin")
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err != nil {
t.Fatalf("ApplyUpdate: %v", err)
}
diff --git a/internal/docs/export_test.go b/internal/docs/export_test.go
index ccf9355..495dd68 100644
--- a/internal/docs/export_test.go
+++ b/internal/docs/export_test.go
@@ -28,7 +28,7 @@ func exportable(t *testing.T, app *App, address, title string) *Document {
// ApplyUpdate returns the accepted row's seq alongside its error
// since the live-update stream landed (store.go); this test does not
// care which seq, only that the update went in.
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err != nil {
t.Fatalf("ApplyUpdate: %v", err)
}
return doc
@@ -429,7 +429,7 @@ func TestEveryClassTheRendererEmitsIsStyled(t *testing.T) {
if err != nil {
t.Fatalf("Create: %v", err)
}
- if _, err := app.Store().ApplyUpdate(ctx, doc, readFixture(t, "../../fixtures/vectors/fragment/"+name)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), readFixture(t, "../../fixtures/vectors/fragment/"+name)); err != nil {
t.Fatalf("ApplyUpdate %s: %v", name, err)
}
html, err := app.Store().HTML(ctx, doc)
diff --git a/internal/docs/grants.go b/internal/docs/grants.go
new file mode 100644
index 0000000..f7e254c
--- /dev/null
+++ b/internal/docs/grants.go
@@ -0,0 +1,142 @@
+package docs
+
+import (
+ "context"
+ "errors"
+ "fmt"
+ "strings"
+ "time"
+
+ "gorm.io/gorm"
+ "gorm.io/gorm/clause"
+)
+
+// Role is what a grant lets its holder do. Two values, spelled out, because a
+// role read out of a database during an incident should say what it is.
+type Role string
+
+const (
+ // RoleReader may read the document and nothing else.
+ RoleReader Role = "reader"
+ // RoleWriter may read it and apply updates to it.
+ RoleWriter Role = "writer"
+)
+
+func (r Role) valid() bool { return r == RoleReader || r == RoleWriter }
+
+// Grant is one person's access to one document.
+//
+// Subject is a lowercased, trimmed address rather than a people.id — see
+// 0005_grants.sql for why, which is that EnsurePerson's create branch seeds a
+// sample library, so minting a Person row to hold a grant would hand an
+// invitee six documents they never asked for before they had accepted.
+type Grant struct {
+ ID int64 `gorm:"column:id;primaryKey"`
+ DocID int64 `gorm:"column:doc_id;index"`
+ Subject string `gorm:"column:subject;index"`
+ Role Role `gorm:"column:role"`
+ CreatedAt time.Time `gorm:"column:created_at"`
+}
+
+// TableName pins the table, since GORM would otherwise pluralise Grant to
+// "grants" by a rule rather than by a decision.
+func (Grant) TableName() string { return "grants" }
+
+// ErrNotOwner is returned when somebody who is not a document's owner tries to
+// change who may reach it. A grantee — even a writer — cannot re-share.
+var ErrNotOwner = errors.New("docs: only a document's owner may share it")
+
+// ErrNotWritable is returned when the acting person may not write a document.
+// It does not distinguish a reader from a stranger: the write path answers
+// both the same way, for the reason §4.2 gives about reads.
+var ErrNotWritable = errors.New("docs: not writable by this person")
+
+// Actor is who is acting on a request: this app's numeric person id, and the
+// address their grants are keyed on. Both are needed because ownership is
+// keyed on the id and grants are keyed on the address — see 0005_grants.sql
+// for why grants are not keyed on the id.
+type Actor struct {
+ PersonID int64
+ Subject string
+}
+
+// writable scopes a query to the documents one person may change: the ones
+// they own, plus the ones they hold a writer grant on. A reader grant is
+// absent from this query on purpose, and that absence is the role's whole
+// meaning.
+func writable(g *gorm.DB, a Actor) *gorm.DB {
+ if a.PersonID <= 0 {
+ return g.Where("1 = 0")
+ }
+ return g.Where(
+ "documents.owner_id > 0 AND (documents.owner_id = ? OR EXISTS ("+
+ "SELECT 1 FROM grants WHERE grants.doc_id = documents.id "+
+ "AND grants.subject = ? AND grants.role = ?))",
+ a.PersonID, subjectOf(a.Subject), RoleWriter,
+ )
+}
+
+// subjectOf normalises an address the way EnsurePerson stores one, so a grant
+// made to "Bob@Example.com " is the grant Bob holds when he signs in.
+func subjectOf(address string) string {
+ return strings.ToLower(strings.TrimSpace(address))
+}
+
+// readable scopes a query to the documents one person may read: the ones they
+// own, plus the ones they hold a grant on.
+//
+// This is the seam sharing adds beside owned. owned still answers "whose
+// document is this" and still means exactly what it meant — no call site that
+// genuinely wants ownership silently starts accepting grantees, which is what
+// widening owned in place would have done.
+//
+// The grant test is an EXISTS subquery rather than a join, and deliberately:
+// callers chain .Where("sqid = ?"), .Order and .Find onto this, and a join
+// would hand them duplicate rows and an ambiguous `id` in clauses written
+// before sharing existed.
+//
+// owner_id > 0 leads, so an orphan stays invisible even if a grant points at
+// it. 0002_owners.sql wrote owner_id 0 onto every pre-existing row, and three
+// tests pin that 0 matches nothing; a grants clause must not become the way
+// around them.
+func readable(g *gorm.DB, owner int64, subject string) *gorm.DB {
+ if owner <= 0 {
+ return g.Where("1 = 0")
+ }
+ return g.Where(
+ "documents.owner_id > 0 AND (documents.owner_id = ? OR EXISTS ("+
+ "SELECT 1 FROM grants WHERE grants.doc_id = documents.id AND grants.subject = ?))",
+ owner, subjectOf(subject),
+ )
+}
+
+// Grant gives an address access to a document. Only the owner may call it:
+// re-sharing is not something a grant carries, so a writer cannot widen who
+// else reaches the document.
+//
+// Granting twice to the same address changes the role rather than stacking a
+// second row, because two rows whose precedence nobody decided is a question
+// that gets answered differently by whichever query runs first.
+func (s *Store) Grant(ctx context.Context, owner int64, sqid, address string, role Role) error {
+ if !role.valid() {
+ return fmt.Errorf("docs: unknown role %q", role)
+ }
+ subject := subjectOf(address)
+ if subject == "" {
+ return errors.New("docs: cannot grant to an empty address")
+ }
+
+ doc, err := s.Get(ctx, owner, sqid)
+ if err != nil {
+ // Get is owner-scoped, so this is "not yours" and "no such document"
+ // answering identically — spec §4.2, kept here rather than leaked into
+ // a distinguishable share error.
+ return err
+ }
+
+ grant := Grant{DocID: doc.ID, Subject: subject, Role: role, CreatedAt: time.Now().UTC()}
+ return s.db.WithContext(ctx).Clauses(clause.OnConflict{
+ Columns: []clause.Column{{Name: "doc_id"}, {Name: "subject"}},
+ DoUpdates: clause.AssignmentColumns([]string{"role"}),
+ }).Create(&grant).Error
+}
diff --git a/internal/docs/grants_test.go b/internal/docs/grants_test.go
new file mode 100644
index 0000000..09e817b
--- /dev/null
+++ b/internal/docs/grants_test.go
@@ -0,0 +1,187 @@
+package docs
+
+import (
+ "context"
+ "testing"
+)
+
+// readable is the seam sharing adds beside owned. owned answers "whose
+// document is this"; readable answers "who may read it", which is the owner
+// plus anybody holding a grant.
+//
+// owned's own doc comment says the day sharing lands is the day this changes,
+// and TestOwnedScopesByOwner was written to be the test that has to keep
+// passing when it does. It still tests owned, unchanged, and owned still means
+// exactly what it meant. What is new is a second function beside it, so no
+// call site that genuinely wants ownership silently starts accepting grantees.
+func TestReadableIncludesGrantees(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, err := store.EnsurePerson(ctx, "alice@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson(alice): %v", err)
+ }
+ bob, err := store.EnsurePerson(ctx, "bob@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson(bob): %v", err)
+ }
+ carol, err := store.EnsurePerson(ctx, "carol@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson(carol): %v", err)
+ }
+
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+ if err := store.Grant(ctx, alice, doc.Sqid, "bob@example.com", RoleReader); err != nil {
+ t.Fatalf("Grant: %v", err)
+ }
+
+ // Bob reads it because he was granted it.
+ var bobRows []*Document
+ if err := readable(store.db.WithContext(ctx), bob, "bob@example.com").Find(&bobRows).Error; err != nil {
+ t.Fatalf("readable(bob): %v", err)
+ }
+ if len(bobRows) != 1 || bobRows[0].ID != doc.ID {
+ t.Errorf("readable(bob) = %d row(s), want exactly Alice's document", len(bobRows))
+ }
+
+ // Alice still reads her own.
+ var aliceRows []*Document
+ if err := readable(store.db.WithContext(ctx), alice, "alice@example.com").Find(&aliceRows).Error; err != nil {
+ t.Fatalf("readable(alice): %v", err)
+ }
+ if len(aliceRows) != 1 {
+ t.Errorf("readable(alice) = %d row(s), want 1", len(aliceRows))
+ }
+
+ // Carol was granted nothing and reads nothing. Without this the whole
+ // thing could be a query that returns every row.
+ var carolRows []*Document
+ if err := readable(store.db.WithContext(ctx), carol, "carol@example.com").Find(&carolRows).Error; err != nil {
+ t.Fatalf("readable(carol): %v", err)
+ }
+ if len(carolRows) != 0 {
+ t.Errorf("readable(carol) = %d row(s), want 0 — carol holds no grant", len(carolRows))
+ }
+}
+
+// A grant is keyed on the address, not on a people row, so it can be made to
+// somebody who has never signed in. EnsurePerson seeds a sample library on the
+// create branch, so minting a Person for an invitee would hand them six sample
+// documents they never asked for.
+func TestGrantToSomebodyWhoHasNeverSignedIn(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, err := store.EnsurePerson(ctx, "alice@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson(alice): %v", err)
+ }
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+
+ if err := store.Grant(ctx, alice, doc.Sqid, "dave@example.com", RoleReader); err != nil {
+ t.Fatalf("Grant to a stranger: %v", err)
+ }
+
+ var people []*Person
+ if err := store.db.WithContext(ctx).Where("email = ?", "dave@example.com").Find(&people).Error; err != nil {
+ t.Fatalf("counting people: %v", err)
+ }
+ if len(people) != 0 {
+ t.Errorf("granting minted a Person row for dave, which would seed him a sample library")
+ }
+
+ // And the grant works the moment he does sign in.
+ dave, err := store.EnsurePerson(ctx, "dave@example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson(dave): %v", err)
+ }
+ var daveRows []*Document
+ if err := readable(store.db.WithContext(ctx), dave, "dave@example.com").Find(&daveRows).Error; err != nil {
+ t.Fatalf("readable(dave): %v", err)
+ }
+ if len(daveRows) != 1 {
+ t.Errorf("readable(dave) = %d row(s), want the document granted before he existed", len(daveRows))
+ }
+}
+
+// Addresses are matched the way EnsurePerson stores them: lowercased and
+// trimmed. A grant made to "Bob@Example.com " must be the grant Bob holds.
+func TestGrantSubjectIsNormalised(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, _ := store.EnsurePerson(ctx, "alice@example.com")
+ bob, _ := store.EnsurePerson(ctx, "bob@example.com")
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+
+ if err := store.Grant(ctx, alice, doc.Sqid, " Bob@Example.COM ", RoleReader); err != nil {
+ t.Fatalf("Grant: %v", err)
+ }
+
+ var rows []*Document
+ if err := readable(store.db.WithContext(ctx), bob, "bob@example.com").Find(&rows).Error; err != nil {
+ t.Fatalf("readable(bob): %v", err)
+ }
+ if len(rows) != 1 {
+ t.Errorf("readable(bob) = %d row(s); the grant's address was not normalised", len(rows))
+ }
+}
+
+// An orphaned document — owner_id 0, which 0002_owners.sql writes onto every
+// pre-existing row — stays invisible even if a grant points at it. Three tests
+// already pin that owner 0 matches nothing; a grants clause must not become
+// the way around them.
+func TestReadableKeepsOrphansInvisible(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, _ := store.EnsurePerson(ctx, "alice@example.com")
+ bob, _ := store.EnsurePerson(ctx, "bob@example.com")
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+ if err := store.Grant(ctx, alice, doc.Sqid, "bob@example.com", RoleReader); err != nil {
+ t.Fatalf("Grant: %v", err)
+ }
+
+ // Orphan it after the grant exists.
+ if err := store.db.WithContext(ctx).Model(&Document{}).Where("id = ?", doc.ID).
+ Update("owner_id", 0).Error; err != nil {
+ t.Fatalf("orphaning: %v", err)
+ }
+
+ var rows []*Document
+ if err := readable(store.db.WithContext(ctx), bob, "bob@example.com").Find(&rows).Error; err != nil {
+ t.Fatalf("readable(bob): %v", err)
+ }
+ if len(rows) != 0 {
+ t.Errorf("an orphaned document was readable through a grant; owner 0 must match nothing")
+ }
+}
diff --git a/internal/docs/grants_write_test.go b/internal/docs/grants_write_test.go
new file mode 100644
index 0000000..579636e
--- /dev/null
+++ b/internal/docs/grants_write_test.go
@@ -0,0 +1,128 @@
+package docs
+
+import (
+ "context"
+ "errors"
+ "testing"
+)
+
+// The write path has to authorize the *actor*, not the document.
+//
+// Before sharing, ApplyUpdate verified `owned(db, doc.OwnerID).Where("id = ?",
+// doc.ID)` — the document against its own owner, which is true of every
+// document by construction. That was harmless while the only way to hold a
+// *Document was to have fetched it through an owner-scoped query, so the
+// check was a belt on top of the read scope's braces. The moment reads accept
+// grantees it stops being harmless: a reader who can fetch the document can
+// satisfy a check that only ever asked whether the document is its own
+// owner's.
+//
+// store.go's own comment records this class of bug being fixed twice already.
+// This is the third, caught before it shipped rather than after.
+func TestApplyUpdateRefusesAReader(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, _ := store.EnsurePerson(ctx, "alice@example.com")
+ bob, _ := store.EnsurePerson(ctx, "bob@example.com")
+
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+ if err := store.Grant(ctx, alice, doc.Sqid, "bob@example.com", RoleReader); err != nil {
+ t.Fatalf("Grant: %v", err)
+ }
+
+ // Bob can read it — that is what the grant is for.
+ var rows []*Document
+ if err := readable(store.db.WithContext(ctx), bob, "bob@example.com").Find(&rows).Error; err != nil {
+ t.Fatalf("readable(bob): %v", err)
+ }
+ if len(rows) != 1 {
+ t.Fatalf("bob cannot read the document he was granted; the rest of this test proves nothing")
+ }
+
+ // And he cannot write it.
+ _, err = store.ApplyUpdate(ctx, doc, Actor{PersonID: bob, Subject: "bob@example.com"}, []byte{0})
+ if !errors.Is(err, ErrNotWritable) {
+ t.Errorf("ApplyUpdate as a reader: got %v, want ErrNotWritable", err)
+ }
+}
+
+// A writer grant does let them write, or the role is decoration.
+func TestApplyUpdateAllowsAWriter(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, _ := store.EnsurePerson(ctx, "alice@example.com")
+ bob, _ := store.EnsurePerson(ctx, "bob@example.com")
+
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+ if err := store.Grant(ctx, alice, doc.Sqid, "bob@example.com", RoleWriter); err != nil {
+ t.Fatalf("Grant: %v", err)
+ }
+
+ // A malformed CRDT update is fine here: it must get *past* authorization
+ // and fail on its own merits, which is what distinguishes "refused" from
+ // "not a valid update".
+ _, err = store.ApplyUpdate(ctx, doc, Actor{PersonID: bob, Subject: "bob@example.com"}, []byte{0})
+ if errors.Is(err, ErrNotWritable) {
+ t.Errorf("ApplyUpdate as a writer was refused authorization; the grant's role is doing nothing")
+ }
+}
+
+// The owner keeps writing without any grant to themselves.
+func TestApplyUpdateAllowsTheOwner(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, _ := store.EnsurePerson(ctx, "alice@example.com")
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+
+ _, err = store.ApplyUpdate(ctx, doc, Actor{PersonID: alice, Subject: "alice@example.com"}, []byte{0})
+ if errors.Is(err, ErrNotWritable) {
+ t.Errorf("the owner was refused a write to her own document: %v", err)
+ }
+}
+
+// A stranger holding neither ownership nor a grant is refused, and refused the
+// same way a reader is — the write path does not tell them which.
+func TestApplyUpdateRefusesAStranger(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ alice, _ := store.EnsurePerson(ctx, "alice@example.com")
+ carol, _ := store.EnsurePerson(ctx, "carol@example.com")
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+
+ _, err = store.ApplyUpdate(ctx, doc, Actor{PersonID: carol, Subject: "carol@example.com"}, []byte{0})
+ if !errors.Is(err, ErrNotWritable) {
+ t.Errorf("ApplyUpdate as a stranger: got %v, want ErrNotWritable", err)
+ }
+}
diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go
index 85d655b..e9cfcc1 100644
--- a/internal/docs/handlers.go
+++ b/internal/docs/handlers.go
@@ -153,6 +153,25 @@ func (h *handlers) owner(r *http.Request) (int64, error) {
return h.store.EnsurePerson(r.Context(), id.Address)
}
+// actor is owner plus the address, which is what a write needs.
+//
+// Ownership is keyed on the numeric person id; grants are keyed on the
+// address (0005_grants.sql says why). A write path has to check both — the
+// person might own the document, or hold a writer grant on it — so it needs
+// both halves of the same identity, and taking them from one place keeps them
+// from drifting apart at a call site.
+func (h *handlers) actor(r *http.Request) (Actor, error) {
+ id, ok := rauth.From(r)
+ if !ok {
+ return Actor{}, errors.New("docs: no signed-in identity on request")
+ }
+ person, err := h.store.EnsurePerson(r.Context(), id.Address)
+ if err != nil {
+ return Actor{}, err
+ }
+ return Actor{PersonID: person, Subject: id.Address}, nil
+}
+
// list serves GET /: the document list, spec §6.4 and Paul's four
// rulings against it — newest first, grouped by recency, one row per
// document. Cut, deliberately, not by oversight: no template
@@ -670,8 +689,22 @@ func (h *handlers) updatesPost(w http.ResponseWriter, r *http.Request) {
return
}
- seq, err := h.store.ApplyUpdate(r.Context(), doc, update)
+ actor, err := h.actor(r)
if err != nil {
+ h.logger.Error("docs: resolve actor for update", "sqid", doc.Sqid, "err", err)
+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)
+ return
+ }
+
+ seq, err := h.store.ApplyUpdate(r.Context(), doc, actor, update)
+ if err != nil {
+ // A reader posting an update gets the same 404 a stranger gets.
+ // Spec §4.2: a document you may not write does not announce itself
+ // as one you may not write.
+ if errors.Is(err, ErrNotWritable) {
+ http.NotFound(w, r)
+ return
+ }
if updateIsClientFault(err) {
// Info, not Error: a client sending bytes the core
// cleanly rejects is an ordinary, expected outcome of
diff --git a/internal/docs/list_test.go b/internal/docs/list_test.go
index 41fe248..77bd794 100644
--- a/internal/docs/list_test.go
+++ b/internal/docs/list_test.go
@@ -97,7 +97,7 @@ func TestListDrawsAThumbnailOfPageOne(t *testing.T) {
app, _ := docs.NewForTest(t)
ctx := context.Background()
owner := app.TestPerson(t, "reader@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
cookies := signInCookies(t, app, "reader@example.com")
@@ -204,7 +204,7 @@ func TestEditingADocumentChangesItsThumbnail(t *testing.T) {
// A real update through the real path, which is what bumps
// updated_at.
- if _, err := app.Store().ApplyUpdate(ctx, doc, docs.ReadFragmentFixture(t, "hello")); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), docs.ReadFragmentFixture(t, "hello")); err != nil {
t.Fatalf("ApplyUpdate: %v", err)
}
@@ -236,7 +236,7 @@ func TestAThumbnailCarriesNoneOfTheDocumentsWords(t *testing.T) {
app, _ := docs.NewForTest(t)
ctx := context.Background()
owner := app.TestPerson(t, "reader@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
cookies := signInCookies(t, app, "reader@example.com")
diff --git a/internal/docs/migrations/0005_grants.sql b/internal/docs/migrations/0005_grants.sql
new file mode 100644
index 0000000..c6a684f
--- /dev/null
+++ b/internal/docs/migrations/0005_grants.sql
@@ -0,0 +1,40 @@
+-- Sharing: a grant is one person's access to one document.
+--
+-- Spec §8.2 has described this since the design was written and nothing
+-- implemented it: until now `documents.owner_id` was the whole authorization
+-- model and every signed-in person saw only their own rows.
+--
+-- `subject` is a lowercased, trimmed address, NOT a `people.id`, and that is
+-- deliberate. A grant has to be makeable to somebody who has never signed in —
+-- that is most of what sharing is for — and `EnsurePerson`'s create branch is
+-- where a brand-new person gets seeded a sample library. Minting a Person row
+-- to hold a grant would hand an invitee six sample documents they never asked
+-- for, before they had even accepted. Keyed on the address, the grant simply
+-- waits, and `EnsurePerson` maps the address to an id whenever they arrive.
+--
+-- It is also the same subject the suite doctrine's §3 uses in lone shape: a
+-- lowercased address. Docs is lone-shaped today, and when a home arrives the
+-- subject gains a kind rather than a different meaning.
+--
+-- `role` is `reader` or `writer`. Not an integer, because a role read out of a
+-- database during an incident should say what it is. Document-scoped only:
+-- there is no team in this app yet, so there is nothing a NULL doc_id could
+-- mean, and a nullable column whose one legal value is "not that" is an
+-- invitation to guess. Spec §8.2's team-wide grant arrives with a team.
+--
+-- The UNIQUE index is the whole conflict story: granting twice to the same
+-- address on the same document updates the role rather than stacking two rows
+-- whose precedence nobody has decided.
+CREATE TABLE IF NOT EXISTS grants (
+ id INTEGER PRIMARY KEY AUTOINCREMENT,
+ doc_id INTEGER NOT NULL,
+ subject TEXT NOT NULL,
+ role TEXT NOT NULL,
+ created_at DATETIME
+);
+
+CREATE UNIQUE INDEX IF NOT EXISTS idx_grants_doc_subject ON grants (doc_id, subject);
+
+-- The read path asks "which documents may this subject reach", so the index
+-- that matters leads on subject.
+CREATE INDEX IF NOT EXISTS idx_grants_subject ON grants (subject);
diff --git a/internal/docs/seed.go b/internal/docs/seed.go
index dce1f72..fb59e89 100644
--- a/internal/docs/seed.go
+++ b/internal/docs/seed.go
@@ -78,7 +78,11 @@ var seedDocs = []seedDoc{
// the mailer, the auth plugin. Store.EnsurePerson calling this
// directly, in-package, is what makes per-person seeding possible at
// all without a Store depending back on the *App that holds it.
-func Seed(ctx context.Context, store *Store, owner int64) error {
+// The actor is the new person themselves: seeding writes their sample
+// library as them, so it goes through exactly the same write
+// authorization as any other edit rather than around it.
+func Seed(ctx context.Context, store *Store, actor Actor) error {
+ owner := actor.PersonID
base := time.Now().UTC()
for _, sd := range seedDocs {
@@ -96,7 +100,7 @@ func Seed(ctx context.Context, store *Store, owner int64) error {
// theirs can be open, so there is nobody subscribed to notify
// (design D5's accepted consequence — the broadcast lives in
// updatesPost, not in Store).
- if _, err := store.ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := store.ApplyUpdate(ctx, doc, actor, update); err != nil {
return fmt.Errorf("docs: seed: apply update for %q: %w", sd.Title, err)
}
diff --git a/internal/docs/seed_test.go b/internal/docs/seed_test.go
index c100db2..600c458 100644
--- a/internal/docs/seed_test.go
+++ b/internal/docs/seed_test.go
@@ -29,7 +29,7 @@ func TestSeedProducesReadableDocuments(t *testing.T) {
ctx := context.Background()
owner := app.TestPerson(t, "seed@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
@@ -66,7 +66,7 @@ func TestSeedVariesTimestamps(t *testing.T) {
ctx := context.Background()
owner := app.TestPerson(t, "seed@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
@@ -106,7 +106,7 @@ func TestSeedContentMatchesItsSource(t *testing.T) {
ctx := context.Background()
owner := app.TestPerson(t, "source-check@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
@@ -170,7 +170,7 @@ func TestSeededDocumentsRenderAsHTML(t *testing.T) {
}
ctx := context.Background()
owner := app.TestPerson(t, "render@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
docList, err := app.Store().List(ctx, owner)
@@ -202,7 +202,7 @@ func TestWaterworksHistoryHasHeadings(t *testing.T) {
}
ctx := context.Background()
owner := app.TestPerson(t, "headings@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
docList, _ := app.Store().List(ctx, owner)
@@ -244,7 +244,7 @@ func TestSeedTextReturnsRealProse(t *testing.T) {
}
ctx := context.Background()
owner := app.TestPerson(t, "text-prose@example.com")
- if err := docs.Seed(ctx, app.Store(), owner); err != nil {
+ if err := docs.Seed(ctx, app.Store(), app.TestActorForPerson(t, owner)); err != nil {
t.Fatalf("Seed: %v", err)
}
docList, err := app.Store().List(ctx, owner)
diff --git a/internal/docs/snapshots_test.go b/internal/docs/snapshots_test.go
index f7b4284..4aac115 100644
--- a/internal/docs/snapshots_test.go
+++ b/internal/docs/snapshots_test.go
@@ -179,7 +179,7 @@ func TestSnapshotThenTailRendersIdenticallyToAFullReplay(t *testing.T) {
if i == tc.distinctAt {
update = distinct
}
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err != nil {
t.Fatalf("ApplyUpdate %d: %v", i, err)
}
}
@@ -256,7 +256,7 @@ func TestConcurrentPostsAtTheCadenceBoundaryDoNotStrandContent(t *testing.T) {
}
for i := 0; i < snapshotCadence; i++ {
- if _, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t)); err != nil {
t.Fatalf("warmup %d: %v", i, err)
}
}
@@ -312,7 +312,7 @@ func TestASnapshotContainsEveryRowItsSeqClaims(t *testing.T) {
}
for i := 0; i < snapshotCadence; i++ {
- if _, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t)); err != nil {
t.Fatalf("warmup %d: %v", i, err)
}
}
@@ -382,7 +382,7 @@ func TestWritingASnapshotDeletesOnlyWhatItSupersedes(t *testing.T) {
seed(other, 10)
seed(other, 20)
- if _, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t)); err != nil {
t.Fatalf("ApplyUpdate: %v", err)
}
h, _, _, err := app.Store().replay(ctx, doc)
@@ -473,10 +473,10 @@ func raceEightPosts(t *testing.T, app *App, doc *Document) error {
if i == carriesTheDistinctContent {
// Written by this one goroutine and read only after
// wg.Wait below, which is the happens-before edge.
- _, distinctErr = app.Store().ApplyUpdate(ctx, doc, distinct)
+ _, distinctErr = app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), distinct)
return
}
- _, _ = app.Store().ApplyUpdate(ctx, doc, ordinary)
+ _, _ = app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), ordinary)
}(i)
}
wg.Wait()
@@ -560,7 +560,7 @@ func TestADocumentAlreadyAtTheCapRecovers(t *testing.T) {
// The first write still fails — the handle is full before the
// snapshot can help — but it must leave a snapshot behind.
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err == nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err == nil {
t.Log("first write succeeded; acceptable if the cap was not reached")
}
snap, err := app.Store().newestSnapshot(ctx, doc.ID)
@@ -572,7 +572,7 @@ func TestADocumentAlreadyAtTheCapRecovers(t *testing.T) {
}
// The retry replays snapshot + empty tail and succeeds.
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err != nil {
t.Errorf("retry after a snapshot existed still failed: %v", err)
}
}
@@ -592,7 +592,7 @@ func TestACorruptSnapshotFallsBackToTheLog(t *testing.T) {
t.Fatalf("Create: %v", err)
}
for i := 0; i < snapshotCadence+2; i++ {
- if _, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t)); err != nil {
t.Fatalf("ApplyUpdate %d: %v", i, err)
}
}
@@ -643,7 +643,7 @@ func TestADocumentPastTheRetainedCapKeepsAcceptingWrites(t *testing.T) {
update := validUpdate(t)
for i := 0; i < core.MaxRetainedUpdates+50; i++ {
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err != nil {
t.Fatalf("ApplyUpdate %d of %d: %v", i, core.MaxRetainedUpdates+50, err)
}
}
@@ -804,7 +804,7 @@ func TestALongLogWithNoUsableSnapshotStillAcceptsWrites(t *testing.T) {
total, distinct := longLogDistinctRows(t)
seedLongLog(t, app, doc, total, distinct)
- seq, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t))
+ seq, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t))
if err != nil {
t.Fatalf("ApplyUpdate on a %d-row log with no snapshot: %v", total, err)
}
@@ -881,7 +881,7 @@ func TestALongLogWhoseOnlySnapshotIsFromAnotherABIStillReadsAndWrites(t *testing
}
assertHoldsTheWholeLog(t, app, doc, string(html))
- if _, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t)); err != nil {
t.Fatalf("ApplyUpdate after an ABI bump invalidated the snapshot: %v", err)
}
}
diff --git a/internal/docs/store.go b/internal/docs/store.go
index 846eeea..52cba00 100644
--- a/internal/docs/store.go
+++ b/internal/docs/store.go
@@ -242,7 +242,7 @@ func (s *Store) seedIfEnabled(ctx context.Context, newOwner int64, email string)
if !s.sampleDocuments {
return
}
- if err := Seed(ctx, s, newOwner); err != nil {
+ if err := Seed(ctx, s, Actor{PersonID: newOwner, Subject: email}); err != nil {
s.logger.Error("docs: seed sample documents for new person", "person", newOwner, "address", email, "err", err)
}
}
@@ -718,12 +718,29 @@ func (s *Store) currentSeq(ctx context.Context, docID int64) (int64, error) {
// Note also that this bumps the COLUMN and not the *Document it was
// handed — a caller holding that struct has a stale UpdatedAt, which
// is why the cache takes its key from the row Store.List returned.
-func (s *Store) ApplyUpdate(ctx context.Context, doc *Document, update []byte) (int64, error) {
- var ownerCheck Document
- if err := owned(s.db.WithContext(ctx), doc.OwnerID).
- Where("id = ?", doc.ID).
- First(&ownerCheck).Error; err != nil {
- return 0, fmt.Errorf("docs: apply update to document %d: verify owner %d: %w", doc.ID, doc.OwnerID, err)
+func (s *Store) ApplyUpdate(ctx context.Context, doc *Document, actor Actor, update []byte) (int64, error) {
+ // Authorize the ACTOR, not the document.
+ //
+ // This check used to be owned(db, doc.OwnerID).Where("id = ?", doc.ID) —
+ // the document against its own owner, which is true of every document by
+ // construction. That was harmless only because the sole way to hold a
+ // *Document was to have fetched it through an owner-scoped query, so this
+ // was a belt on top of the read scope's braces. Sharing removes that
+ // guarantee: a reader can now legitimately fetch a document, and the old
+ // check would have waved their write through, because it never asked
+ // anything about the person doing the writing.
+ //
+ // Reader and stranger are refused identically. The write path does not
+ // tell somebody which of the two they are, for the same reason §4.2 gives
+ // about reads.
+ var writeCheck Document
+ if err := writable(s.db.WithContext(ctx), actor).
+ Where("documents.id = ?", doc.ID).
+ First(&writeCheck).Error; err != nil {
+ if errors.Is(err, gorm.ErrRecordNotFound) {
+ return 0, fmt.Errorf("docs: apply update to document %d: %w", doc.ID, ErrNotWritable)
+ }
+ return 0, fmt.Errorf("docs: apply update to document %d: verify writer %d: %w", doc.ID, actor.PersonID, err)
}
h, replayedSeq, lastSnapshotSeq, err := s.replay(ctx, doc)
diff --git a/internal/docs/store_test.go b/internal/docs/store_test.go
index 886cb0e..679c789 100644
--- a/internal/docs/store_test.go
+++ b/internal/docs/store_test.go
@@ -67,7 +67,7 @@ func TestTextComesFromTheCore(t *testing.T) {
// but targets root "content", which Store.Text no longer reads —
// it would read back "", proving nothing about the CRDT.
update := docs.ReadFragmentFixture(t, "hello")
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err != nil {
t.Fatalf("ApplyUpdate: %v", err)
}
@@ -125,7 +125,7 @@ func TestListOrdersByUpdatedAt(t *testing.T) {
time.Sleep(5 * time.Millisecond)
update := docs.ReadFixture(t, "hello")
- if _, err := app.Store().ApplyUpdate(ctx, older, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, older, app.TestActorFor(t, older), update); err != nil {
t.Fatalf("ApplyUpdate(older): %v", err)
}
@@ -246,15 +246,31 @@ func TestApplyUpdateRefusesAnotherOwnersDocument(t *testing.T) {
// clock, not hidden by two timestamps landing in the same tick.
time.Sleep(5 * time.Millisecond)
+ // Sharing re-pointed this test, and the re-pointing is the finding.
+ //
+ // It used to hand ApplyUpdate a *Document whose OwnerID had been
+ // tampered to a person who does not exist, and assert the write was
+ // refused. That worked because ApplyUpdate authorized the struct it was
+ // given. It no longer does: it authorizes the ACTING person against the
+ // row in the database, so a tampered OwnerID on a caller's copy is now
+ // simply ignored — which is the safer behaviour, and means the old
+ // assertion would pass for the wrong reason or fail for one.
+ //
+ // So the hazard is expressed the way it actually exists now: a stranger,
+ // holding a perfectly accurate *Document, is refused, and nothing is
+ // written. The tampered struct stays, unused by the authorization, to
+ // prove exactly that — a caller cannot make themselves the owner by
+ // editing a field on their own copy.
tampered := *doc
tampered.OwnerID = owner + 1_000_000 // guaranteed to belong to nobody
+ stranger := app.TestActorForPerson(t, app.TestPerson(t, "stranger@example.com"))
// Fragment fixture, not ReadFixture's plain-Y.Text one: this test's
// content assertions below go through Store.Text, which reads root
// "prosemirror" now — see TestTextComesFromTheCore's own comment.
update := docs.ReadFragmentFixture(t, "hello")
- if _, err := app.Store().ApplyUpdate(ctx, &tampered, update); err == nil {
- t.Fatal("ApplyUpdate returned nil for a mismatched owner — want a real error, before anything is written")
+ if _, err := app.Store().ApplyUpdate(ctx, &tampered, stranger, update); err == nil {
+ t.Fatal("ApplyUpdate returned nil for a stranger — want a refusal, before anything is written")
}
after, err := app.Store().Get(ctx, owner, doc.Sqid)
@@ -275,7 +291,7 @@ func TestApplyUpdateRefusesAnotherOwnersDocument(t *testing.T) {
// Prove the fix does not break the working path: the legitimate
// owner's own ApplyUpdate must still apply end to end.
- if _, err := app.Store().ApplyUpdate(ctx, doc, update); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), update); err != nil {
t.Fatalf("ApplyUpdate (legitimate owner): %v", err)
}
legit, err := app.Store().Text(ctx, doc)
diff --git a/internal/docs/stream_test.go b/internal/docs/stream_test.go
index 81dd058..8372077 100644
--- a/internal/docs/stream_test.go
+++ b/internal/docs/stream_test.go
@@ -139,7 +139,7 @@ func TestStreamAnnouncesTheCurrentSeqOnConnect(t *testing.T) {
if err != nil {
t.Fatalf("Create: %v", err)
}
- if _, err := app.Store().ApplyUpdate(ctx, doc, validUpdate(t)); err != nil {
+ if _, err := app.Store().ApplyUpdate(ctx, doc, app.TestActorFor(t, doc), validUpdate(t)); err != nil {
t.Fatalf("ApplyUpdate: %v", err)
}
diff --git a/internal/docs/testing.go b/internal/docs/testing.go
index bb6d1fa..73b4395 100644
--- a/internal/docs/testing.go
+++ b/internal/docs/testing.go
@@ -213,3 +213,28 @@ func ReadFragmentFixture(t *testing.T, name string) []byte {
}
return b
}
+
+// TestActorFor returns the Actor for a document's own owner.
+//
+// It exists because ApplyUpdate now authorizes the acting person rather than
+// the document, so a test that writes to a document has to say who is
+// writing. Almost every existing one is the owner editing their own document,
+// and this resolves that owner for real — through the people row the document
+// points at — rather than fabricating an Actor that would satisfy the check
+// without exercising it. A test wanting a grantee builds the Actor itself, as
+// grants_write_test.go does.
+func (a *App) TestActorFor(t *testing.T, doc *Document) Actor {
+ t.Helper()
+ return a.TestActorForPerson(t, doc.OwnerID)
+}
+
+// TestActorForPerson is the same lookup keyed on the person directly, for a
+// test that has an id in hand rather than a document.
+func (a *App) TestActorForPerson(t *testing.T, person int64) Actor {
+ t.Helper()
+ var p Person
+ if err := a.Store().db.Where("id = ?", person).First(&p).Error; err != nil {
+ t.Fatalf("TestActorForPerson: resolving person %d: %v", person, err)
+ }
+ return Actor{PersonID: p.ID, Subject: p.Email}
+}