amadan

oficina / docs Public

app: documents belong to people

Every read and write scoped by owner, through one function rather than
scattered WHERE clauses. Not scope.Owned: verified in plan 1 that it is
Where("user_id = ?") and returns a short list rather than an error on a
shared table, which looks like an empty library rather than a bug.

A document you cannot see 404s and never 403s, so a stranger cannot
learn a sqid exists by probing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011F5CivVm4CmL9ByfSSAaXA
Paul Campbell pushed by paul@keymail.dev a7e1e9d22509f22adebba368e68f26426982efa6 parent 32156a4
8 files changed, +434 −43
  • internal/docs/handlers.go +58 −16
  • internal/docs/list_test.go +10 −2
  • internal/docs/migrations/0002_owners.sql +9 −0
  • internal/docs/models.go +27 −0
  • internal/docs/scope_test.go +93 −0
  • internal/docs/store.go +100 −17
  • internal/docs/store_test.go +67 −8
  • internal/docs/testing.go +70 −0

Intent

full ledger: signin-and-samples

diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go
index 69fdfd0..c0b46c7 100644
--- a/internal/docs/handlers.go
+++ b/internal/docs/handlers.go
@@ -10,6 +10,7 @@ import (
"gorm.io/gorm"
"github.com/carlosframework/rastrillo"
+ rauth "github.com/carlosframework/rastrillo/auth"
)
// handlers holds what the app's HTTP handlers need — rastrillo
@@ -67,6 +68,23 @@ type docRow struct {
When string
}
+// owner resolves the signed-in person's Store.Person id for r —
+// deliberately auth.From(r).Address, never sessions.UserID: under the
+// magic-link plugin the session Subject is the verified email itself
+// (rastrillo SKILL.md §5), so UserID's ParseInt would fail and, worse,
+// a version of this code that dropped its ok return would silently
+// scope every query to person 0. Every handler below is mounted only
+// inside app.go's RequireSession group, so From always finds an
+// identity here in practice; the error return exists so a handler
+// answers 500 rather than panicking if that ever stops being true.
+func (h *handlers) owner(r *http.Request) (int64, error) {
+ id, ok := rauth.From(r)
+ if !ok {
+ return 0, errors.New("docs: no signed-in identity on request")
+ }
+ return h.store.EnsurePerson(r.Context(), id.Address)
+}
+
// 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
@@ -77,11 +95,20 @@ type docRow struct {
// engine renders there — reserved, not faked: today it is an empty,
// bordered box.
//
-// A row shows title and when it was last touched, not who: Document
-// carries no author column yet (Task 5 adds identity), and rendering
-// a "who" today would mean inventing one.
+// A row shows title and when it was last touched, not who: List is
+// already scoped to the signed-in viewer's own documents (Store.List's
+// owner parameter), so every row's "who" is the same person reading
+// the page — rendering it would be inventing display data, not
+// showing real ownership.
func (h *handlers) list(w http.ResponseWriter, r *http.Request) {
- docs, err := h.store.List(r.Context())
+ owner, err := h.owner(r)
+ if err != nil {
+ h.logger.Error("docs: resolve signed-in person", "err", err)
+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)
+ return
+ }
+
+ docs, err := h.store.List(r.Context(), owner)
if err != nil {
h.logger.Error("docs: list documents", "err", err)
http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)
@@ -113,12 +140,19 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request) {
// navigation, replays the GET the redirect points at, never the POST
// that created the document.
func (h *handlers) create(w http.ResponseWriter, r *http.Request) {
+ owner, err := h.owner(r)
+ if err != nil {
+ h.logger.Error("docs: resolve signed-in person", "err", err)
+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)
+ return
+ }
+
title := strings.TrimSpace(r.PostFormValue("title"))
if title == "" {
title = rastrillo.T(r, "docs.document.untitled")
}
- doc, err := h.store.Create(r.Context(), title)
+ doc, err := h.store.Create(r.Context(), owner, title)
if err != nil {
h.logger.Error("docs: create document", "err", err)
http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)
@@ -142,16 +176,23 @@ type showPage struct {
// show serves GET /d/{sqid}: the document read-only, its title and
// its CRDT text inside a page-shaped canvas (views/show.html). An
-// unknown or malformed sqid 404s — spec §4.2's contract that a
-// document you cannot see is indistinguishable from one that does not
-// exist. There is no signed-in viewer yet (identity is Task 5), so
-// today "cannot see" only ever means "no such row"; the 404 is the
-// same either way, and nothing here will need to change once scoping
-// lands — it only gains another way to reach this same branch.
+// unknown sqid and a sqid someone else owns 404 identically — spec
+// §4.2's contract that a document you cannot see is indistinguishable
+// from one that does not exist — because Store.Get's owner-scoped
+// query never distinguishes "no such row" from "not yours" in the
+// first place; there is no separate check here that could 403 by
+// mistake.
func (h *handlers) show(w http.ResponseWriter, r *http.Request) {
sqid := r.PathValue("sqid")
- doc, err := h.store.Get(r.Context(), sqid)
+ owner, err := h.owner(r)
+ if err != nil {
+ h.logger.Error("docs: resolve signed-in person", "err", err)
+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)
+ return
+ }
+
+ doc, err := h.store.Get(r.Context(), owner, sqid)
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
http.Error(w, rastrillo.T(r, "docs.error.notfound"), http.StatusNotFound)
@@ -214,10 +255,11 @@ func groupByRecency(r *http.Request, now time.Time, docs []*Document) []docGroup
// recencyBucket returns a stable bucket key and the catalog key for
// its label, comparing calendar days in UTC — Document.CreatedAt and
// UpdatedAt are both stored in UTC (Store.Create), and there is no
-// viewer time zone to convert to yet, since there is no signed-in
-// viewer (Task 5). time.Truncate(24*time.Hour) lines up with the
-// calendar day specifically because both arguments are UTC: the day
-// never shifts under DST the way it can in a zone that observes it.
+// per-viewer time zone to convert to yet (a signed-in viewer exists
+// now, but nothing stores their zone). time.Truncate(24*time.Hour)
+// lines up with the calendar day specifically because both arguments
+// are UTC: the day never shifts under DST the way it can in a zone
+// that observes it.
func recencyBucket(now, t time.Time) (key, labelKey string) {
days := int(now.Truncate(24*time.Hour).Sub(t.Truncate(24*time.Hour)).Hours() / 24)
switch {
diff --git a/internal/docs/list_test.go b/internal/docs/list_test.go
index d3671ab..a047183 100644
--- a/internal/docs/list_test.go
+++ b/internal/docs/list_test.go
@@ -56,7 +56,8 @@ func signInCookies(t *testing.T, app *docs.App, address string) []*http.Cookie {
func TestListShowsDocuments(t *testing.T) {
app, _ := docs.NewForTest(t)
ctx := context.Background()
- if _, err := app.Store().Create(ctx, "Board minutes"); err != nil {
+ owner := app.TestPerson(t, "reader@example.com")
+ if _, err := app.Store().Create(ctx, owner, "Board minutes"); err != nil {
t.Fatal(err)
}
cookies := signInCookies(t, app, "reader@example.com")
@@ -167,7 +168,14 @@ func TestCreateWrongOriginRejected(t *testing.T) {
// And confirm nothing was actually created — the refusal is real,
// not just a wrong status code on an otherwise-successful write.
- rows, err := app.Store().List(context.Background())
+ // csrf.Protect refuses this request before RequireSession or
+ // h.owner ever run (app.go mounts it first), so no document could
+ // have been created under any owner; TestPerson here only mints an
+ // id to list against, it does not stand in for a session that was
+ // ever actually established.
+ ctx := context.Background()
+ owner := app.TestPerson(t, "reader@example.com")
+ rows, err := app.Store().List(ctx, owner)
if err != nil {
t.Fatal(err)
}
diff --git a/internal/docs/migrations/0002_owners.sql b/internal/docs/migrations/0002_owners.sql
new file mode 100644
index 0000000..4f47586
--- /dev/null
+++ b/internal/docs/migrations/0002_owners.sql
@@ -0,0 +1,9 @@
+CREATE TABLE IF NOT EXISTS people (
+ id INTEGER PRIMARY KEY,
+ email TEXT NOT NULL UNIQUE,
+ created_at DATETIME NOT NULL
+);
+
+ALTER TABLE documents ADD COLUMN owner_id INTEGER NOT NULL DEFAULT 0;
+
+CREATE INDEX IF NOT EXISTS idx_documents_owner_id ON documents (owner_id);
diff --git a/internal/docs/models.go b/internal/docs/models.go
index d1447c9..ada43ba 100644
--- a/internal/docs/models.go
+++ b/internal/docs/models.go
@@ -36,11 +36,21 @@ var Schema = migrate.MustFromFS(migrationFS, "docs")
// corrupts the document. This is the Go side of the rule clippy.toml
// enforces in Rust (disallowed-methods on yrs::Doc::new /
// Doc::with_client_id).
+//
+// OwnerID is the document's owner — Person.ID, never taken from a
+// request body (Store.Create's owner parameter comes from the
+// signed-in person; see handlers.go's owner helper and rastrillo
+// SKILL.md §3/§4's mass-assignment rule). Every read and write is
+// scoped by it through store.go's own owned function, deliberately
+// not rastrillo's scope.Owned — see that function's doc comment and
+// spec §4.2.
type Document struct {
ID int64 `gorm:"primaryKey"`
Sqid string `gorm:"column:sqid;uniqueIndex;not null"`
Title string `gorm:"column:title;not null"`
+ OwnerID int64 `gorm:"column:owner_id;not null;index"`
+
GUID string `gorm:"column:guid;not null"`
ClientID uint64 `gorm:"column:client_id;not null"`
@@ -49,6 +59,23 @@ type Document struct {
DeletedAt *time.Time `gorm:"column:deleted_at"`
}
+// Person is this app's own record of someone who has signed in —
+// deliberately separate from rastrillo/auth's session Subject, which
+// under the magic-link plugin is the verified email address itself,
+// not a numeric id (rastrillo SKILL.md §5: "Under auth, never
+// sessions.UserID: the Subject is the verified email... map the
+// address to your user row's id before scoping"). Store.EnsurePerson
+// is the one place that mapping happens, get-or-create, keyed on
+// Email. A document's OwnerID is a Person.ID, never an email — so an
+// email can change (a future feature) without rewriting every
+// document row it ever owned.
+type Person struct {
+ ID int64 `gorm:"primaryKey"`
+ Email string `gorm:"column:email;uniqueIndex;not null"`
+
+ CreatedAt time.Time `gorm:"column:created_at;not null"`
+}
+
// DocUpdate is one Yjs update in a document's log, applied in Seq
// order to rebuild the document's CRDT state (Store.replay).
//
diff --git a/internal/docs/scope_test.go b/internal/docs/scope_test.go
new file mode 100644
index 0000000..9387b7c
--- /dev/null
+++ b/internal/docs/scope_test.go
@@ -0,0 +1,93 @@
+package docs
+
+import (
+ "context"
+ "testing"
+)
+
+// TestOwnedScopesByOwner tests store.go's owned function directly —
+// the plan's own requirement (Task 2: "Write the scoping as one
+// function that every query goes through, and test that function
+// directly"), not only indirectly through Store.List/Store.Get, which
+// would prove the seam is wired in but not pin what the seam itself
+// actually does. Deliberately not a test of rastrillo's scope.Owned
+// (which this app does not use — see owned's own doc comment and
+// spec §4.2): this exercises Docs' own function, in this package, so
+// the day it grows a grants join the test above is what has to keep
+// passing.
+func TestOwnedScopesByOwner(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)
+ }
+
+ doc, err := store.Create(ctx, alice, "Alice's document")
+ if err != nil {
+ t.Fatalf("Create: %v", err)
+ }
+
+ var aliceRows []*Document
+ if err := owned(store.db.WithContext(ctx), alice).Find(&aliceRows).Error; err != nil {
+ t.Fatalf("owned(alice): %v", err)
+ }
+ if len(aliceRows) != 1 || aliceRows[0].ID != doc.ID {
+ t.Errorf("owned(alice) = %+v, want exactly [%d]", aliceRows, doc.ID)
+ }
+
+ var bobRows []*Document
+ if err := owned(store.db.WithContext(ctx), bob).Find(&bobRows).Error; err != nil {
+ t.Fatalf("owned(bob): %v", err)
+ }
+ if len(bobRows) != 0 {
+ t.Errorf("owned(bob) returned %d row(s), want 0 — bob does not own alice's document", len(bobRows))
+ }
+
+ // An owner nobody has ever used (no Person row, no document) is
+ // scoped to nothing, not an error — the same shape a stranger
+ // probing gets, never a distinct failure mode of its own.
+ var noRows []*Document
+ if err := owned(store.db.WithContext(ctx), 999999).Find(&noRows).Error; err != nil {
+ t.Fatalf("owned(unknown owner): %v", err)
+ }
+ if len(noRows) != 0 {
+ t.Errorf("owned(unknown owner) returned %d row(s), want 0", len(noRows))
+ }
+}
+
+// TestEnsurePersonIsStableAndCaseInsensitive pins EnsurePerson's own
+// contract, since it is what turns a session's verified address into
+// the owner id every scoped query above keys on: the same address
+// (however it is cased) must always resolve to the same Person id, or
+// two requests from the same person could silently own different
+// libraries.
+func TestEnsurePersonIsStableAndCaseInsensitive(t *testing.T) {
+ app, err := NewForTest(t)
+ if err != nil {
+ t.Fatalf("NewForTest: %v", err)
+ }
+ ctx := context.Background()
+ store := app.Store()
+
+ first, err := store.EnsurePerson(ctx, "Alice@Example.com")
+ if err != nil {
+ t.Fatalf("EnsurePerson: %v", err)
+ }
+ second, err := store.EnsurePerson(ctx, " alice@example.com ")
+ if err != nil {
+ t.Fatalf("EnsurePerson (again): %v", err)
+ }
+ if first != second {
+ t.Errorf("EnsurePerson(%q) = %d, EnsurePerson(%q) = %d, want the same id", "Alice@Example.com", first, " alice@example.com ", second)
+ }
+}
diff --git a/internal/docs/store.go b/internal/docs/store.go
index 8ba25e3..66cc8ef 100644
--- a/internal/docs/store.go
+++ b/internal/docs/store.go
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
mrand "math/rand/v2"
+ "strings"
"time"
"github.com/google/uuid"
@@ -69,13 +70,85 @@ func newSqid() (string, error) {
// expected.
const maxSqidAttempts = 5
-// Create makes a new document: a GUID and a client id assigned here
-// by the app (never drawn from yrs — see Document's doc comment) and
-// stored, an app-generated sqid, and the given title. It does not
-// touch the CRDT core at all: an empty document needs no update log
-// entry, and Text/ApplyUpdate are what open a core handle when there
-// is work for one to do.
-func (s *Store) Create(ctx context.Context, title string) (*Document, error) {
+// owned scopes g to documents owner may see — the one seam every
+// query in this file goes through, per the plan's own ruling.
+// Deliberately not rastrillo's scope.Owned (SKILL.md §3): that helper
+// is `g.Where("user_id = ?", owner)`, which is exactly what this is
+// today, but authored, versioned and tested outside this package.
+// Applied to a table that later grows sharing (spec §4.2's `grants`
+// join, out of scope for this plan), scope.Owned would keep silently
+// filtering to creator-only forever — a document library that looks
+// empty rather than broken, the worst available failure shape, on the
+// helper an author would naturally reach for. Writing `owned` here
+// instead means the day sharing lands, this is the one function that
+// changes (to add the join), not a search-and-replace across
+// scattered WHERE clauses — and TestOwnedScopesByOwner (scope_test.go)
+// pins its current, ownership-only contract so a future edit that
+// widens it does so on purpose.
+//
+// It does not filter deleted_at: that is not an ownership concern, and
+// callers that care (List, Get) add it themselves, deliberately
+// visible at the call site rather than folded into a function named
+// for something else.
+func owned(g *gorm.DB, owner int64) *gorm.DB {
+ return g.Where("owner_id = ?", owner)
+}
+
+// EnsurePerson maps an authenticated address to this app's own,
+// numeric Person id — get-or-create, keyed on a case-insensitive,
+// trimmed Email. This is the translation rastrillo SKILL.md §5
+// requires under the magic-link plugin: the session Subject is the
+// verified email itself, not a numeric id, so sessions.UserID returns
+// (0, false) and must never be used (see Document.OwnerID's doc
+// comment). handlers.go's owner helper calls this on every
+// authenticated request; TestPerson (testing.go) calls it directly so
+// a test can mint an owner id without going through HTTP at all — the
+// two must and do agree, since TestPerson is this method and nothing
+// else.
+func (s *Store) EnsurePerson(ctx context.Context, email string) (int64, error) {
+ email = strings.TrimSpace(strings.ToLower(email))
+ if email == "" {
+ return 0, errors.New("docs: EnsurePerson: address is empty")
+ }
+
+ var p Person
+ err := s.db.WithContext(ctx).Where("email = ?", email).First(&p).Error
+ if err == nil {
+ return p.ID, nil
+ }
+ if !errors.Is(err, gorm.ErrRecordNotFound) {
+ return 0, fmt.Errorf("docs: look up person %q: %w", email, err)
+ }
+
+ p = Person{Email: email, CreatedAt: time.Now().UTC()}
+ if err := s.db.WithContext(ctx).Create(&p).Error; err != nil {
+ // A concurrent first sign-in from the same brand-new address
+ // can race two inserts against the unique index on email; the
+ // loser looks up what the winner just created rather than
+ // failing the request over a benign collision.
+ if errors.Is(err, gorm.ErrDuplicatedKey) {
+ if err2 := s.db.WithContext(ctx).Where("email = ?", email).First(&p).Error; err2 == nil {
+ return p.ID, nil
+ }
+ }
+ return 0, fmt.Errorf("docs: create person %q: %w", email, err)
+ }
+ return p.ID, nil
+}
+
+// Create makes a new document owned by owner: a GUID and a client id
+// assigned here by the app (never drawn from yrs — see Document's doc
+// comment) and stored, an app-generated sqid, and the given title. It
+// does not touch the CRDT core at all: an empty document needs no
+// update log entry, and Text/ApplyUpdate are what open a core handle
+// when there is work for one to do.
+//
+// owner is always a Person id the caller already resolved from the
+// signed-in session (handlers.go's owner helper, or TestPerson in
+// tests) — never a value read off the request body. rastrillo
+// SKILL.md §3: "Creating takes the owner from the session, never the
+// form."
+func (s *Store) Create(ctx context.Context, owner int64, title string) (*Document, error) {
guid := uuid.NewString()
// math/rand/v2's Int64 returns a non-negative value in [0, 1<<63),
// so converting to uint64 never wraps — the brief's "a counter or
@@ -92,6 +165,7 @@ func (s *Store) Create(ctx context.Context, title string) (*Document, error) {
doc := &Document{
Sqid: sqid,
Title: title,
+ OwnerID: owner,
GUID: guid,
ClientID: clientID,
CreatedAt: now,
@@ -109,18 +183,22 @@ func (s *Store) Create(ctx context.Context, title string) (*Document, error) {
return nil, fmt.Errorf("docs: create document: %w", lastErr)
}
-// List returns every non-deleted document, newest first — "newest"
-// meaning most recently touched (UpdatedAt), the same clock
-// handlers.go's groupByRecency/formatWhen bucket and display, and the
-// one ApplyUpdate now bumps on every applied update. Ordering by
-// CreatedAt instead (fix round 2's own bug: they agreed only by
+// List returns every one of owner's non-deleted documents, newest
+// first — "newest" meaning most recently touched (UpdatedAt), the same
+// clock handlers.go's groupByRecency/formatWhen bucket and display,
+// and the one ApplyUpdate now bumps on every applied update. Ordering
+// by CreatedAt instead (fix round 2's own bug: they agreed only by
// accident, because nothing bumped UpdatedAt away from CreatedAt) let
// this list and its own recency buckets disagree the moment a
// document's UpdatedAt ever moved without a matching reorder here —
// see store_test.go's TestListOrdersByUpdatedAt.
-func (s *Store) List(ctx context.Context) ([]*Document, error) {
+//
+// A document owner does not own is not in this list — not filtered
+// out afterward, never selected in the first place, through owned
+// above.
+func (s *Store) List(ctx context.Context, owner int64) ([]*Document, error) {
var docs []*Document
- err := s.db.WithContext(ctx).
+ err := owned(s.db.WithContext(ctx), owner).
Where("deleted_at IS NULL").
Order("updated_at DESC, id DESC").
Find(&docs).Error
@@ -130,10 +208,15 @@ func (s *Store) List(ctx context.Context) ([]*Document, error) {
return docs, nil
}
-// Get looks up a document by its sqid.
-func (s *Store) Get(ctx context.Context, sqid string) (*Document, error) {
+// Get looks up one of owner's documents by its sqid. A sqid that
+// exists but belongs to someone else is indistinguishable from a sqid
+// that does not exist at all — both return gorm.ErrRecordNotFound
+// through owned above, never a distinct "yes, but not yours" outcome —
+// which is what lets handlers.go answer 404 and never 403 for a
+// document (spec §4.2).
+func (s *Store) Get(ctx context.Context, owner int64, sqid string) (*Document, error) {
var doc Document
- err := s.db.WithContext(ctx).
+ err := owned(s.db.WithContext(ctx), owner).
Where("sqid = ? AND deleted_at IS NULL", sqid).
First(&doc).Error
if err != nil {
diff --git a/internal/docs/store_test.go b/internal/docs/store_test.go
index 88071cb..d349346 100644
--- a/internal/docs/store_test.go
+++ b/internal/docs/store_test.go
@@ -2,6 +2,8 @@ package docs_test
import (
"context"
+ "net/http"
+ "net/http/httptest"
"testing"
"time"
@@ -14,8 +16,9 @@ func TestCreateListGetRoundTrip(t *testing.T) {
t.Fatalf("NewForTest: %v", err)
}
ctx := context.Background()
+ owner := app.TestPerson(t, "owner@example.com")
- made, err := app.Store().Create(ctx, "First document")
+ made, err := app.Store().Create(ctx, owner, "First document")
if err != nil {
t.Fatalf("Create: %v", err)
}
@@ -26,7 +29,7 @@ func TestCreateListGetRoundTrip(t *testing.T) {
t.Fatal("Create must assign a GUID and client id, never draw them from yrs")
}
- list, err := app.Store().List(ctx)
+ list, err := app.Store().List(ctx, owner)
if err != nil {
t.Fatalf("List: %v", err)
}
@@ -34,7 +37,7 @@ func TestCreateListGetRoundTrip(t *testing.T) {
t.Fatalf("List = %+v, want one document titled First document", list)
}
- got, err := app.Store().Get(ctx, made.Sqid)
+ got, err := app.Store().Get(ctx, owner, made.Sqid)
if err != nil {
t.Fatalf("Get: %v", err)
}
@@ -49,8 +52,9 @@ func TestTextComesFromTheCore(t *testing.T) {
t.Fatalf("NewForTest: %v", err)
}
ctx := context.Background()
+ owner := app.TestPerson(t, "owner@example.com")
- doc, err := app.Store().Create(ctx, "With content")
+ doc, err := app.Store().Create(ctx, owner, "With content")
if err != nil {
t.Fatalf("Create: %v", err)
}
@@ -90,8 +94,9 @@ func TestListOrdersByUpdatedAt(t *testing.T) {
t.Fatalf("NewForTest: %v", err)
}
ctx := context.Background()
+ owner := app.TestPerson(t, "owner@example.com")
- older, err := app.Store().Create(ctx, "Older document")
+ older, err := app.Store().Create(ctx, owner, "Older document")
if err != nil {
t.Fatalf("Create(older): %v", err)
}
@@ -99,12 +104,12 @@ func TestListOrdersByUpdatedAt(t *testing.T) {
// calls back to back can land in the same tick on a coarse clock,
// and this test's whole point is ordering, not simultaneity.
time.Sleep(5 * time.Millisecond)
- newer, err := app.Store().Create(ctx, "Newer document")
+ newer, err := app.Store().Create(ctx, owner, "Newer document")
if err != nil {
t.Fatalf("Create(newer): %v", err)
}
- list, err := app.Store().List(ctx)
+ list, err := app.Store().List(ctx, owner)
if err != nil {
t.Fatalf("List: %v", err)
}
@@ -118,7 +123,7 @@ func TestListOrdersByUpdatedAt(t *testing.T) {
t.Fatalf("ApplyUpdate(older): %v", err)
}
- list, err = app.Store().List(ctx)
+ list, err = app.Store().List(ctx, owner)
if err != nil {
t.Fatalf("List after update: %v", err)
}
@@ -126,3 +131,57 @@ func TestListOrdersByUpdatedAt(t *testing.T) {
t.Fatalf("List after applying an update to the older document = %+v, want [older, newer] — applying an update must move a document to the top of the list", list)
}
}
+
+// TestADocumentYouDoNotOwnIsNotThere is the brief's own literal test
+// (Task 2, plan 3): Bob must not be able to fetch Alice's document by
+// sqid, and it must not appear in his own list either — both routed
+// through Store.Get/List's owner parameter and store.go's owned seam,
+// never a separate "is this yours" check that could leak a distinct
+// error shape.
+func TestADocumentYouDoNotOwnIsNotThere(t *testing.T) {
+ app, _ := docs.NewForTest(t)
+ ctx := context.Background()
+
+ alice := app.TestPerson(t, "alice@example.com")
+ bob := app.TestPerson(t, "bob@example.com")
+
+ doc, err := app.Store().Create(ctx, alice, "Alice's private notes")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ // Bob must not be able to fetch it, and must not be told it exists.
+ if _, err := app.Store().Get(ctx, bob, doc.Sqid); err == nil {
+ t.Fatal("Bob fetched Alice's document")
+ }
+
+ list, err := app.Store().List(ctx, bob)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(list) != 0 {
+ t.Errorf("Bob's list has %d documents, want 0", len(list))
+ }
+}
+
+// TestUnownedDocumentOverHTTPIs404Not403 is the brief's own literal
+// test: the same isolation TestADocumentYouDoNotOwnIsNotThere proves
+// at the store, proven again over the real router, because a store
+// that scopes correctly and a handler that still leaks a 403 (or a
+// 200 on someone else's document) would both pass the store-level
+// test alone. Spec §4.2: a document you cannot see returns 404, so a
+// stranger cannot learn a sqid exists by probing it.
+func TestUnownedDocumentOverHTTPIs404Not403(t *testing.T) {
+ app, _ := docs.NewForTest(t)
+ ctx := context.Background()
+ alice := app.TestPerson(t, "alice@example.com")
+ doc, _ := app.Store().Create(ctx, alice, "Alice's private notes")
+
+ req := app.TestRequest(t, "bob@example.com", http.MethodGet, "/d/"+doc.Sqid, nil)
+ rec := httptest.NewRecorder()
+ app.Handler().ServeHTTP(rec, req)
+
+ if rec.Code != http.StatusNotFound {
+ t.Errorf("status %d, want 404 — a document you cannot see does not exist (spec §4.2)", rec.Code)
+ }
+}
diff --git a/internal/docs/testing.go b/internal/docs/testing.go
index 6fa4c4e..1a1136f 100644
--- a/internal/docs/testing.go
+++ b/internal/docs/testing.go
@@ -1,7 +1,9 @@
package docs
import (
+ "context"
"fmt"
+ "io"
"net/http"
"net/http/httptest"
"net/url"
@@ -96,6 +98,74 @@ func (a *App) TestSignIn(t *testing.T, address string) (string, error) {
return linkPath(link)
}
+// TestPerson creates (or finds) a person by address and returns this
+// app's own numeric id for them — Store.EnsurePerson, the same
+// address-to-id mapping a real request goes through via handlers.go's
+// owner helper (auth.From(r).Address, never sessions.UserID — see
+// Document.OwnerID's doc comment). It is what Store.Create/List/Get's
+// owner parameter expects, and what Task 2's own tests use directly
+// against the store without any HTTP round trip at all:
+//
+// alice := app.TestPerson(t, "alice@example.com")
+// doc, err := app.Store().Create(ctx, alice, "Alice's private notes")
+//
+// TestPerson does not sign anyone in — no cookie, no session. For a
+// request that carries one, use TestRequest, which calls TestPerson
+// itself so the two always agree on who an address maps to.
+func (a *App) TestPerson(t *testing.T, address string) int64 {
+ t.Helper()
+ id, err := a.store.EnsurePerson(context.Background(), address)
+ if err != nil {
+ t.Fatalf("docs: TestPerson(%q): %v", address, err)
+ }
+ return id
+}
+
+// TestRequest builds an httptest request for method and path, signed
+// in as address: a real magic-link round trip (TestSignIn) mints the
+// session cookies, and TestPerson ensures address's Person row exists
+// first — the same row Store.Create/List/Get's owner id needs to
+// already be there for handlers.go's owner helper to find on the
+// request this returns.
+//
+// For a state-changing method (anything but GET/HEAD) it also sets
+// Sec-Fetch-Site: same-origin, exactly what a real same-origin
+// browser sends and csrf.Protect (app.go) checks first (TestSignIn's
+// own doc comment) — so a later task's signed-in POST does not have
+// to re-derive this to get past CSRF.
+//
+// req := app.TestRequest(t, "bob@example.com", http.MethodGet, "/d/"+doc.Sqid, nil)
+func (a *App) TestRequest(t *testing.T, address, method, path string, body io.Reader) *http.Request {
+ t.Helper()
+
+ // Ensure address's Person row exists before any handler on this
+ // request reads it — EnsurePerson is get-or-create, so this is a
+ // no-op if TestSignIn's own callback already created it via the
+ // exact same call.
+ a.TestPerson(t, address)
+
+ link, err := a.TestSignIn(t, address)
+ if err != nil {
+ t.Fatalf("docs: TestRequest: TestSignIn(%q): %v", address, err)
+ }
+ verifyReq := httptest.NewRequest(http.MethodGet, link, nil)
+ verifyRec := httptest.NewRecorder()
+ a.Handler().ServeHTTP(verifyRec, verifyReq)
+ cookies := verifyRec.Result().Cookies()
+ if len(cookies) == 0 {
+ t.Fatalf("docs: TestRequest: following the magic link for %q set no session cookie", address)
+ }
+
+ req := httptest.NewRequest(method, path, body)
+ for _, c := range cookies {
+ req.AddCookie(c)
+ }
+ if method != http.MethodGet && method != http.MethodHead {
+ req.Header.Set("Sec-Fetch-Site", "same-origin")
+ }
+ return req
+}
+
// ReadFixture loads fixtures/data/<name>.update.bin — a real Yjs
// update produced by browser yjs (fixtures/generate.mjs), never
// hand-edited. Not in a _test.go file for the same reason as