499d200 · oficina/docs

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/admission_test.go b/internal/docs/admission_test.go

new file mode 100644

index 0000000..2c11100

--- /dev/null

+++ b/internal/docs/admission_test.go

@@ -0,0 +1,217 @@

+package docs

+

+import (

+ "context"

+ "errors"

+ "net/http"

+ "net/http/httptest"

+ "net/url"

+ "strings"

+ "testing"

+)

+

+// requireAdmission has to tell three answers apart: admitted, refused, and

+// "could not tell". The third is the one that costs somebody their work if

+// it is folded into the second.

+//

+// A guest's editor posts through fetch. save-queue.mjs classifies a 4xx as

+// "the bytes are the problem" and drops the batch permanently — and

+// because Yjs updates depend on their predecessors, everything after it is

+// poisoned too. A 5xx it retries. So a database stall answered 403 costs a

+// guest their paragraph, while the same stall answered 503 costs them

+// nothing. (Re-review 3, finding 1.)

+//

+// Tested at the middleware rather than over HTTP: the app's own session

+// layer refuses first when the database is genuinely broken, so an

+// end-to-end version would assert on rastrillo's 403 and never reach this

+// code at all. That is exactly the "passes for a reason other than the one

+// it names" failure this branch has produced four times, and the honest

+// fix is to test the unit whose contract this is.

+func TestRequireAdmissionTellsRefusalFromFailure(t *testing.T) {

+ reached := false

+ next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

+ reached = true

+ w.WriteHeader(http.StatusNoContent)

+ })

+

+ cases := []struct {

+ name string

+ granted func(string) (bool, error)

+ method string

+ want int

+ passes bool

+ }{

+ {"a check that fails is retryable, not a refusal", func(string) (bool, error) {

+ return false, errors.New("database is stalled")

+ }, http.MethodPost, http.StatusServiceUnavailable, false},

+

+ {"a failed check on a GET is also retryable", func(string) (bool, error) {

+ return false, errors.New("database is stalled")

+ }, http.MethodGet, http.StatusServiceUnavailable, false},

+

+ {"a definite refusal of a write is 403", func(string) (bool, error) {

+ return false, nil

+ }, http.MethodPost, http.StatusForbidden, false},

+

+ {"a definite refusal of a read is a redirect to sign in", func(string) (bool, error) {

+ return false, nil

+ }, http.MethodGet, http.StatusSeeOther, false},

+

+ {"a guest who still holds a grant passes through", func(string) (bool, error) {

+ return true, nil

+ }, http.MethodPost, http.StatusNoContent, true},

+ }

+

+ for _, c := range cases {

+ t.Run(c.name, func(t *testing.T) {

+ reached = false

+ list := allowlist{

+ addrs: map[string]struct{}{"alice@example.com": {}},

+ granted: c.granted,

+ }

+ h := requireAdmissionOf(list, "/signin", signedInAs("bob@example.com"))(next)

+

+ req := httptest.NewRequest(c.method, "/d/abc/updates", nil)

+ rec := httptest.NewRecorder()

+ h.ServeHTTP(rec, req)

+

+ if rec.Code != c.want {

+ t.Errorf("status %d, want %d", rec.Code, c.want)

+ }

+ if reached != c.passes {

+ t.Errorf("reached the handler = %v, want %v", reached, c.passes)

+ }

+ })

+ }

+}

+

+// A configured member never touches the grant check, so a broken database

+// cannot eject them. Without this the 503 above could be the answer

+// everybody gets.

+func TestAMemberIsUnaffectedByAFailedGrantCheck(t *testing.T) {

+ called := false

+ list := allowlist{

+ addrs: map[string]struct{}{"alice@example.com": {}},

+ granted: func(string) (bool, error) {

+ called = true

+ return false, errors.New("database is stalled")

+ },

+ }

+

+ reached := false

+ h := requireAdmissionOf(list, "/signin", signedInAs("alice@example.com"))(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

+ reached = true

+ }))

+

+ h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/documents", nil))

+

+ if !reached {

+ t.Error("a member was stopped by a grant check that is not about them")

+ }

+ if called {

+ t.Error("a member's request consulted the grant store; allows() must answer from the configured set first")

+ }

+}

+

+// signedInAs is the identity lookup requireAdmissionOf takes, standing in

+// for rauth.From, whose context key is unexported.

+func signedInAs(address string) func(*http.Request) (string, bool) {

+ return func(*http.Request) (string, bool) { return address, true }

+}

+

+// admits — the plain-predicate form rauth's Authorize hook takes — fails

+// CLOSED. It cannot distinguish "no" from "could not tell", so it has to

+// choose, and at sign-in the safe choice is refusal: there is no unsaved

+// work to lose and a genuinely admitted person can try again.

+//

+// This is the opposite choice from requireAdmission's 503 above, made for

+// the opposite reason, and neither is right in the other's place. It has

+// its own test because a mutation that made admits() return true on an

+// error survived the whole suite: nothing anywhere pinned the direction,

+// so a database failure would have opened the front door to any address.

+func TestAdmitsFailsClosed(t *testing.T) {

+ list := allowlist{

+ addrs: map[string]struct{}{"alice@example.com": {}},

+ granted: func(string) (bool, error) {

+ return false, errors.New("database is stalled")

+ },

+ }

+

+ if list.admits("mallory@example.com") {

+ t.Error("a failed grant check admitted a stranger; sign-in must fail closed")

+ }

+ // A member is still admitted: the failure must not lock everybody out

+ // either, and allows() answers them without asking the database.

+ if !list.admits("alice@example.com") {

+ t.Error("a configured member was refused because of a grant check that is not about them")

+ }

+}

+

+// gateBegin fails closed too, and nothing pinned it.

+//

+// A mutation flipping its condition to `err != nil || admitted` passed the

+// whole suite. Under it, a database stall turns POST /signin into an open

+// relay: every address a stranger types reaches Begin and gets a magic

+// link mailed. Config.AllowedAddresses' own comment calls gateBegin "the

+// wiring that actually matters" for exactly that, and admits() would still

+// refuse the session at callback — but the mail is already out, sent by us,

+// to an address we chose to accept. (Re-review 4, finding 1.)

+func TestGateBeginFailsClosed(t *testing.T) {

+ list := allowlist{

+ addrs: map[string]struct{}{"alice@example.com": {}},

+ granted: func(string) (bool, error) {

+ return false, errors.New("database is stalled")

+ },

+ }

+

+ for _, c := range []struct {

+ name string

+ address string

+ want bool

+ }{

+ {"a stranger during a failed check is refused", "mallory@example.com", false},

+ {"a member is unaffected, the check never runs for them", "alice@example.com", true},

+ } {

+ t.Run(c.name, func(t *testing.T) {

+ reached := false

+ h := gateBegin(func(w http.ResponseWriter, r *http.Request) {

+ reached = true

+ }, list, "/signin")

+

+ req := httptest.NewRequest(http.MethodPost, "/signin",

+ strings.NewReader(url.Values{"address": {c.address}}.Encode()))

+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

+ h(httptest.NewRecorder(), req)

+

+ if reached != c.want {

+ t.Errorf("reached Begin = %v, want %v — a mail %s sent", reached, c.want,

+ map[bool]string{true: "was not", false: "was"}[reached])

+ }

+ })

+ }

+}

+

+// HasGrant reports its error rather than folding it into false, which is

+// the whole reason the 503 above can exist. A mutation returning

+// (false, nil) on a query failure passed the suite: the chain was tested

+// from the stub downward and never from the store up. (Re-review 4,

+// finding 2.)

+func TestHasGrantReportsItsError(t *testing.T) {

+ app, err := NewForTest(t)

+ if err != nil {

+ t.Fatalf("NewForTest: %v", err)

+ }

+

+ // A cancelled context is the cheapest real failure: the query cannot

+ // run, and the honest answer is "could not tell", never "no".

+ ctx, cancel := context.WithCancel(context.Background())

+ cancel()

+

+ held, err := app.Store().HasGrant(ctx, "bob@example.com")

+ if err == nil {

+ t.Error("a failed grant check reported no error; the caller cannot tell a refusal from an outage")

+ }

+ if held {

+ t.Error("a failed grant check answered true")

+ }

+}

diff --git a/internal/docs/app.go b/internal/docs/app.go

index 581c7aa..5308573 100644

--- a/internal/docs/app.go

+++ b/internal/docs/app.go

@@ -18,6 +18,7 @@ import (

"net/http"

"net/url"

"strings"

+ "time"

"github.com/carlosframework/rastrillo/csrf"

"github.com/carlosframework/rastrillo/db"

@@ -380,6 +381,29 @@ func New(cfg Config) (*App, error) {

}

store := NewStore(d.G, rt, parseSampleDocuments(cfg.SampleDocuments), logger)

+ // A grant admits its holder. Wired here rather than inside

+ // resolveAllowlist because the allowlist is resolved before the store

+ // exists — deliberately, so a misconfigured allowlist refuses to boot

+ // before anything else is built. See allowlist.granted for why an

+ // unredeemable invitation is the failure worth avoiding.

+ // The sample library is for members. A guest admitted by a grant is

+ // visiting, not moving in.

+ store.seedOnlyMembers = list.configured

+

+ list.granted = func(address string) (bool, error) {

+ // A short deadline rather than context.Background(): this runs on

+ // the sign-in path and now on every request (requireAdmission), so

+ // a stalled query must fail rather than hold a request open.

+ //

+ // The error is passed up rather than folded into false. Each

+ // caller wants a different answer to "the check failed": sign-in

+ // refuses, and a live request answers 503 so the editor's save

+ // queue retries instead of dropping the batch.

+ ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)

+ defer cancel()

+ return store.HasGrant(ctx, address)

+ }

+

render, err := newRenderer()

if err != nil {

rt.Close(context.Background())

@@ -401,7 +425,7 @@ func New(cfg Config) (*App, error) {

return nil, fmt.Errorf("docs: build auth: %w", err)

}

- h := &handlers{render: render, logger: logger, store: store, hub: newHub(), previews: newPreviews()}

+ h := &handlers{render: render, logger: logger, store: store, hub: newHub(), previews: newPreviews(), origin: cfg.Origin, isMember: list.configured, oneShots: newOneShots()}

r := chi.NewRouter()

// Mounted above every route, per rastrillo SKILL.md §5: refuses a

@@ -427,6 +451,12 @@ func New(cfg Config) (*App, error) {

r.Post("/signin", gateBegin(authPlugin.Begin, list, "/signin"))

r.Get("/auth/callback", authPlugin.Callback)

r.Get("/auth/verify", authPlugin.Verify)

+ // The published document, and the only route here no session guards.

+ // It is authorized by the token in its own path (links.go) rather

+ // than by who is asking — which is what a capability is, and why it

+ // sits above the RequireSession group rather than inside it with an

+ // exception carved into the middleware.

+ r.Get("/p/{token}", h.publicShow)

r.Post("/signout", authPlugin.Signout)

// staticHandler (assets.go) composes the committed editor bundle

// with render.assets.Handler()'s vendored design-system tree —

@@ -441,10 +471,14 @@ func New(cfg Config) (*App, error) {

// GET/HEAD redirects to /signin; anything else 403s.

r.Group(func(r chi.Router) {

r.Use(authPlugin.RequireSession)

+ // Admission is re-asked per request, not only at sign-in, so

+ // revoking a guest's last grant ends their access now rather

+ // than whenever their 30-day session happens to expire.

+ r.Use(requireAdmission(list, "/signin"))

r.Get("/", h.list)

r.Post("/documents", h.create)

// Title search for the jump-to palette (search.go). Inside

- // this group like every other document route, and owner-scoped

+ // this group like every other document route, and reader-scoped

// through Store.Search → Store.List rather than through a

// check of its own — a person's document titles are theirs.

r.Get("/documents/search", h.searchJSON)

@@ -455,11 +489,19 @@ func New(cfg Config) (*App, error) {

// is exempted from csrf.Protect above: rastrillo SKILL.md §5's

// origin check is what stops a cross-site page from writing

// to a document through this route.

+ // Sharing. All three owner-only, and all three answering 404 to

+ // anybody else — including a writer, who may edit the document and

+ // still may not decide who else reaches it.

+ r.Get("/d/{sqid}/share", h.shareGet)

+ r.Post("/d/{sqid}/share", h.sharePost)

+ r.Post("/d/{sqid}/share/revoke", h.shareRevoke)

+ r.Post("/d/{sqid}/share/link", h.shareLink)

+

r.Get("/d/{sqid}/updates", h.updatesGet)

r.Post("/d/{sqid}/updates", h.updatesPost)

// Export (export.go). Inside this group, so a signed-out

// request gets the same redirect every other document route

- // gives it, and owner-scoped through the same Store.Get, so a

+ // gives it, and reader-scoped through the same Store.GetFor, so a

// document someone else owns 404s identically to one that does

// not exist. Extensions in the path rather than a ?format=

// query: the URL is what a person sees when they save the

@@ -468,7 +510,7 @@ func New(cfg Config) (*App, error) {

r.Get("/d/{sqid}/export.md", h.exportMarkdown)

r.Get("/d/{sqid}/export.pdf", h.exportPDF)

// Plan 5: the live-update stream (stream.go). A GET, so

- // csrf.Protect passes it through untouched, and owner-scoped

+ // csrf.Protect passes it through untouched, and reader-scoped

// through the same getDocumentForUpdates helper as the two

// routes above — a stranger gets 404, never 403.

r.Get("/d/{sqid}/stream", h.stream)

diff --git a/internal/docs/auth.go b/internal/docs/auth.go

index 7fff498..38dd9d2 100644

--- a/internal/docs/auth.go

+++ b/internal/docs/auth.go

@@ -107,14 +107,29 @@ func buildMailer(cfg Config, logger *slog.Logger) (mail.Sender, *recordingMailer

// dev-mode behavior (the link lands in the log, clearly marked) is

// unchanged; this only adds a second, in-memory place to find it.

type recordingMailer struct {

- mu sync.Mutex

- next mail.Sender

+ mu sync.Mutex

+ next mail.Sender

+ // lastTo is recorded alongside lastBody, and it is not decoration.

+ // gateBegin answers a refused address with exactly the 303 ?sent=1 a

+ // real one gets — deliberately, so the endpoint is not a membership

+ // oracle — so a caller that reads only lastBody cannot tell "a link

+ // was mailed to this address" from "somebody else's link is still

+ // sitting here". TestSignIn used to make that mistake and reported

+ // success for an address the allowlist had refused.

+ lastTo string

lastBody string

+ // sends counts mails, and it is the field that makes the recipient

+ // check above actually work. Comparing only lastTo answers "was the

+ // most recent mail addressed here", which stays true for every later

+ // REFUSAL of the same address — the recorder simply keeps the earlier

+ // success. A caller has to know a NEW mail was sent, not that an old

+ // one is still lying there.

+ sends int

}

func (m *recordingMailer) Send(ctx context.Context, to, subject, body string) error {

m.mu.Lock()

- m.lastBody = body

+ m.lastTo, m.lastBody, m.sends = to, body, m.sends+1

m.mu.Unlock()

return m.next.Send(ctx, to, subject, body)

}

@@ -148,10 +163,45 @@ func normalizeAddress(address string) string {

// there is a startup refusal, not a value this type carries.

type allowlist struct {

addrs map[string]struct{}

+

+ // granted widens admission to anybody holding a grant on a document

+ // here, and is nil when nothing has wired the store in (every test of

+ // the allowlist alone, and any construction before the store exists).

+ //

+ // It exists because $DOCS_ALLOWED_ADDRESSES gates who may receive a

+ // magic link at all, and gateBegin deliberately answers identically for

+ // a refused address so this endpoint is not a membership oracle.

+ // Without this clause, sharing a document to somebody outside the list

+ // produces a grant that can never be redeemed: the sharer sees success,

+ // the recipient gets a link that silently does nothing, and neither is

+ // told why. That is the worst available failure shape, because it looks

+ // like it worked.

+ //

+ // Deliberately one clause in allows(). Sharing widening who may sign in

+ // is a real decision about this instance's front door, and reversing it

+ // should be deleting two lines rather than unpicking a design.

+ granted func(address string) (bool, error)

}

-// allows reports whether address may sign in.

-func (a allowlist) allows(address string) bool {

+// configured reports whether address is a MEMBER of this instance — on

+// $DOCS_ALLOWED_ADDRESSES, rather than admitted by a grant.

+//

+// The distinction the branch review found missing (finding 1). Suite §1

+// ruling 1 and Docs §8.2 both say an outside guest is a *guest* member;

+// the branch had no guest kind, so a grant was a full membership and

+// admission became transitive — Alice grants Bob, Bob creates and grants

+// Mallory, and the operator's allowlist has quietly stopped being the

+// membership list.

+//

+// A guest reads and writes what they were granted. Creating documents and

+// changing who reaches one are member powers, and this is the predicate

+// that says so.

+//

+// addrs == nil (development's admit-everyone) makes everybody a member,

+// which is the same latitude allows() already gives and the same reason:

+// a dev instance with no configuration should not need a membership list

+// to be usable.

+func (a allowlist) configured(address string) bool {

if a.addrs == nil {

return true

}

@@ -159,6 +209,42 @@ func (a allowlist) allows(address string) bool {

return ok

}

+// allows reports whether address may sign in, and whether the question

+// could be answered at all.

+//

+// The error is separate from the answer because the two demand different

+// responses on a live request: a definite "no" ejects, an unanswerable

+// question must not. See requireAdmission.

+func (a allowlist) allows(address string) (bool, error) {

+ if a.addrs == nil {

+ return true, nil

+ }

+ normalized := normalizeAddress(address)

+ if _, ok := a.addrs[normalized]; ok {

+ return true, nil

+ }

+ // A grant admits its holder, and nobody else — as a guest, not as a

+ // member: see configured() above for the powers that stay behind the

+ // configured list, and requireAdmission for why this is now re-asked on

+ // every request rather than only at sign-in.

+ if a.granted == nil {

+ return false, nil

+ }

+ return a.granted(normalized)

+}

+

+// admits is allows for callers that take a plain predicate — rauth's own

+// Authorize hook, which has no way to say "could not tell".

+//

+// A failed check refuses, which is the right direction at sign-in: there

+// is no unsaved work to lose, and a person who is genuinely admitted can

+// try again. requireAdmission is the caller that must NOT do this, and it

+// uses allows directly for exactly that reason.

+func (a allowlist) admits(address string) bool {

+ ok, err := a.allows(address)

+ return err == nil && ok

+}

+

// resolveAllowlist applies Config.AllowedAddresses' own fail-closed

// rule (see its doc comment on Config): a non-empty, comma-separated

// list always wins, parsed and normalized; an empty one is fine only

@@ -268,7 +354,13 @@ func gateBegin(next http.HandlerFunc, list allowlist, signinPath string) http.Ha

// malformed request, just reached here instead, before any

// mail could be sent over it.

_ = r.ParseForm()

- if list.allows(r.FormValue("address")) {

+ // A failed check refuses here, unlike requireAdmission's 503. At

+ // sign-in there is nothing to lose by refusing — no unsaved work,

+ // and the person can try again — and gateBegin's whole contract is

+ // that a refusal is indistinguishable from a success, which a 503

+ // would break by telling a stranger the check happened.

+ admitted, err := list.allows(r.FormValue("address"))

+ if err == nil && admitted {

if r.Form == nil {

r.Form = url.Values{}

}

@@ -280,12 +372,95 @@ func gateBegin(next http.HandlerFunc, list allowlist, signinPath string) http.Ha

}

+// requireAdmission re-asks the front-door question on every request, and

+// answers a person who no longer passes it by ending them at /signin.

+//

+// It exists because allows() used to run only at sign-in. A session lasts

+// 30 days (rastrillo/auth.DefaultSessionTTL), so revoking somebody's last

+// grant took their document away and left them a working session for a

+// month — during which, before the guest rule above, they went on creating

+// documents and granting them to other people. "Stop sharing" was complete

+// for the document and silent about the instance. (Branch review, finding

+// 3.)

+//

+// Cheap for members: allows() checks the configured set first and returns

+// before touching the database. Only a guest pays a query, and a guest is

+// exactly who the answer can have changed for.

+//

+// It sits after RequireSession so it can rely on there being an identity;

+// with no identity it does nothing and lets RequireSession's own redirect

+// stand.

+func requireAdmission(list allowlist, signinPath string) func(http.Handler) http.Handler {

+ return requireAdmissionOf(list, signinPath, func(r *http.Request) (string, bool) {

+ id, ok := rauth.From(r)

+ return id.Address, ok

+ })

+}

+

+// requireAdmissionOf is requireAdmission with the identity lookup passed

+// in, which is the only way this middleware can be tested at all: rauth

+// stores the identity under an unexported context key, so a test cannot

+// build a request that carries one.

+//

+// The seam is here rather than the test reaching through HTTP because an

+// end-to-end version of the "database is down" case never reaches this

+// code — rastrillo's own session layer refuses first, and the test would

+// have asserted on its 403 while proving nothing about this middleware.

+// That is the failure mode this branch has now produced four times.

+func requireAdmissionOf(list allowlist, signinPath string, identity func(*http.Request) (string, bool)) func(http.Handler) http.Handler {

+ return func(next http.Handler) http.Handler {

+ return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

+ address, ok := identity(r)

+ if !ok {

+ next.ServeHTTP(w, r)

+ return

+ }

+ admitted, err := list.allows(address)

+ if err != nil {

+ // The question could not be answered, which is not the

+ // same as "no". 503 rather than 403: save-queue.mjs

+ // retries a 5xx and permanently drops a 4xx, so folding a

+ // database stall into a refusal would cost a guest their

+ // paragraph. Retryable, and honest.

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusServiceUnavailable)

+ return

+ }

+ if admitted {

+ next.ServeHTTP(w, r)

+ return

+ }

+

+ // A redirect answers "show me a page". It is never an answer

+ // to "store this".

+ //

+ // The first version of this middleware 303'd every method,

+ // which was the worst defect on this branch and was introduced

+ // by the fix for the previous one. fetch() follows redirects by

+ // default, so the editor's POST became a GET of the sign-in

+ // page, came back 200 with HTML, and save-queue.mjs read res.ok

+ // and reported the batch saved. A revoked writer typed into a

+ // page that told them everything was fine — strictly worse than

+ // the reader-editor bug, which at least lit "save failed".

+ //

+ // rastrillo's own refuseSession draws the line in exactly this

+ // place and for exactly this reason. (Re-review, finding 1.)

+ if r.Method == http.MethodGet || r.Method == http.MethodHead {

+ http.Redirect(w, r, signinPath, http.StatusSeeOther)

+ return

+ }

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusForbidden)

+ })

+ }

+}

+

// newAuthPlugin wires rastrillo/auth per its SKILL.md contract: one

// *Auth built at boot (New — SKILL.md's own warning against building

// a Flow per request, since the rate limiter's budget lives on the

// value), Origin as both the emailed link's base and the cookie

// decision, InstanceKey and Mailer already resolved and fail-closed

-// by resolveInstanceKey/buildMailer above. Authorize is list.allows —

+// by resolveInstanceKey/buildMailer above. Authorize is list.admits —

+// the fail-closed predicate form, because rauth's hook has no way to

+// say "could not tell" and refusing is the safe answer here —

// belt-and-suspenders alongside gateBegin's own gate on POST /signin:

// this defends admission itself (a link somehow minted for a

// non-permitted address — a future code path, or a link mailed before

@@ -301,7 +476,7 @@ func newAuthPlugin(cfg Config, writer *sql.DB, instanceKey string, mailer mail.S

Origin: cfg.Origin,

InstanceKey: instanceKey,

Mailer: mailer,

- Authorize: list.allows,

+ Authorize: list.admits,

Logger: logger,

})

}

diff --git a/internal/docs/auth_allowlist_test.go b/internal/docs/auth_allowlist_test.go

index 215e535..ddb20ef 100644

--- a/internal/docs/auth_allowlist_test.go

+++ b/internal/docs/auth_allowlist_test.go

@@ -216,12 +216,12 @@ func TestResolveAllowlistParsesCommaSeparatedList(t *testing.T) {

t.Fatalf("resolveAllowlist: %v", err)

}

for _, addr := range []string{"alice@example.com", "ALICE@EXAMPLE.COM", "bob@example.com", "carol@example.com"} {

- if !list.allows(addr) {

- t.Errorf("list.allows(%q) = false, want true", addr)

+ if !list.admits(addr) {

+ t.Errorf("list.admits(%q) = false, want true", addr)

}

- if list.allows("mallory@example.com") {

- t.Error("list.allows(\"mallory@example.com\") = true, want false — not on the configured list")

+ if list.admits("mallory@example.com") {

+ t.Error("list.admits(\"mallory@example.com\") = true, want false — not on the configured list")

}

@@ -253,7 +253,7 @@ func TestResolveAllowlistEmptyInDevelopmentAdmitsEveryone(t *testing.T) {

if err != nil {

t.Fatalf("resolveAllowlist: %v", err)

}

- if !list.allows("anyone@example.com") {

+ if !list.admits("anyone@example.com") {

t.Error("an empty AllowedAddresses in development must admit everyone")

}

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..5574a77

--- /dev/null

+++ b/internal/docs/grants.go

@@ -0,0 +1,341 @@

+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"`

+ // CreatedBy is the person who made this grant — the spec's §4 data

+ // model has it and this table shipped without it. It is what lets an

+ // operator answer "who let this address in", which is a question with

+ // no other source once a grant exists.

+ CreatedBy int64 `gorm:"column:created_by"`

+ 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")

+

+// errEmptyAddress is a share form submitted with nothing in the address

+// field. Its own error rather than a generic one because the share handler

+// re-renders the page with a message beside the field, and a person mistyping

+// should not get a 500.

+var errEmptyAddress = errors.New("docs: cannot grant to an empty address")

+

+// 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.

+//

+// It defers to normalizeAddress rather than repeating it. The two were the

+// same two lines in two files — the two halves of one key, free to drift

+// into comparing differently-normalised forms of the same address, which

+// would have shown up as a grant that silently admits nobody.

+func subjectOf(address string) string {

+ return normalizeAddress(address)

+}

+

+// addressOf maps a person id back to the address they signed in with, so

+// Grant can refuse a grant somebody makes to themselves.

+func (s *Store) addressOf(ctx context.Context, person int64) (string, error) {

+ var p Person

+ if err := s.db.WithContext(ctx).First(&p, person).Error; err != nil {

+ return "", err

+ }

+ return subjectOf(p.Email), nil

+}

+

+// looksLikeAnAddress is the cheapest check that keeps nonsense out of the

+// front door.

+//

+// Not a validator: RFC 5322 is not worth implementing here, and rauth owns

+// the real parsing downstream. This exists because Grant accepted any

+// non-empty string, so "not-an-email" became a row, appeared on the share

+// page, and made HasGrant answer true — widening admission to a string

+// that can never sign in, and leaving the owner a share list with a typo

+// on it they will read as a person. (Branch review, finding 10.)

+func looksLikeAnAddress(subject string) bool {

+ at := strings.Index(subject, "@")

+ return at > 0 && at < len(subject)-1 && !strings.ContainsAny(subject, " \t")

+}

+

+// 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 == "" || !looksLikeAnAddress(subject) {

+ return errEmptyAddress

+ }

+

+ 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

+ }

+

+ // Granting to yourself is refused: you already reach it by owning it,

+ // and the row would put the owner on their own share list as though

+ // somebody had been invited.

+ ownerAddress, err := s.addressOf(ctx, owner)

+ if err != nil {

+ return fmt.Errorf("docs: resolve owner address: %w", err)

+ }

+ if ownerAddress == subject {

+ return errEmptyAddress

+ }

+

+ grant := Grant{

+ DocID: doc.ID,

+ Subject: subject,

+ Role: role,

+ CreatedBy: owner,

+ 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", "created_by"}),

+ }).Create(&grant).Error

+}

+

+// HasGrant reports whether an address holds a grant on any document here.

+//

+// The allowlist asks this so a granted address can sign in — see allowlist's

+// `granted` field for why an unredeemable invitation is the worst available

+// failure shape. It answers on the address alone, never on which document,

+// because admission is not a per-document question.

+//

+// A database error is REPORTED rather than folded into false, and that

+// distinction costs a guest their work if it is lost.

+//

+// requireAdmission runs on every request. Answering false on a failed

+// check ejects a guest mid-session, and their editor's next POST gets a

+// 403 — which save-queue.mjs reads as "the bytes are the problem",

+// dropping the batch permanently and, because Yjs updates depend on their

+// predecessors, poisoning everything after it. One two-second database

+// stall would cost a guest their paragraph. So the caller gets the error

+// and can answer 503, which the queue retries. (Re-review 3, finding 1.)

+func (s *Store) HasGrant(ctx context.Context, address string) (bool, error) {

+ subject := subjectOf(address)

+ if subject == "" {

+ return false, nil

+ }

+ // Scoped to grants on documents that still exist. A grant on a

+ // soft-deleted or orphaned (owner_id = 0) document opens nothing —

+ // readable hides those rows — so counting it here would hold the front

+ // door open for somebody who can reach nothing at all, and would keep

+ // holding it after every document they were given had been deleted.

+ // (Branch review, finding 12.)

+ var count int64

+ if err := s.db.WithContext(ctx).Model(&Grant{}).

+ Joins("JOIN documents ON documents.id = grants.doc_id").

+ Where("grants.subject = ? AND documents.deleted_at IS NULL AND documents.owner_id > 0",

+ subject).

+ Count(&count).Error; err != nil {

+ s.logger.Error("docs: checking grants for admission", "err", err)

+ return false, err

+ }

+ return count > 0, nil

+}

+

+// ListGrants returns who currently reaches a document, oldest first, for its

+// owner. Owner-scoped through Get, so a grantee asking gets the same answer a

+// stranger does — the share page is the owner's alone (§4.2).

+func (s *Store) ListGrants(ctx context.Context, owner int64, sqid string) ([]*Grant, error) {

+ doc, err := s.Get(ctx, owner, sqid)

+ if err != nil {

+ return nil, err

+ }

+ var grants []*Grant

+ err = s.db.WithContext(ctx).

+ Where("doc_id = ?", doc.ID).

+ Order("created_at ASC, id ASC").

+ Find(&grants).Error

+ return grants, err

+}

+

+// Revoke takes an address's access away.

+//

+// Revoking something that is not there succeeds. The owner asked for an

+// outcome — that this address does not reach this document — and that outcome

+// holds either way; reporting an error would only tell them about a race with

+// their own second click.

+func (s *Store) Revoke(ctx context.Context, owner int64, sqid, address string) error {

+ doc, err := s.Get(ctx, owner, sqid)

+ if err != nil {

+ return err

+ }

+ return s.db.WithContext(ctx).

+ Where("doc_id = ? AND subject = ?", doc.ID, subjectOf(address)).

+ Delete(&Grant{}).Error

+}

+

+// GetFor fetches a document the acting person may read: theirs, or one they

+// hold a grant on.

+//

+// Deliberately a second method rather than a widening of Get. Get is

+// owner-scoped and several paths want exactly that — sharing, revoking and

+// listing grants are the owner's alone — so widening Get in place would have

+// handed every one of them grantees as well, silently. The two names say

+// which question is being asked.

+//

+// A document the person may not read answers gorm.ErrRecordNotFound, the same

+// as one that does not exist, and the handler turns both into 404 (§4.2).

+func (s *Store) GetFor(ctx context.Context, a Actor, sqid string) (*Document, error) {

+ var doc Document

+ err := readable(s.db.WithContext(ctx), a.PersonID, a.Subject).

+ Where("documents.sqid = ? AND documents.deleted_at IS NULL", sqid).

+ First(&doc).Error

+ if err != nil {

+ return nil, err

+ }

+ return &doc, nil

+}

+

+// CanWrite reports whether the acting person may change a document.

+//

+// The same EXISTS clause writable builds, asked as a question rather than

+// used as a scope, because the document page needs the answer before it

+// renders: a reader must not be handed an editor (branch review, #2). The

+// one predicate, asked twice, so the page and the update route cannot

+// disagree about who may write.

+//

+// A database error answers false. Rendering a read-only page to somebody

+// who may write costs them a reload; rendering an editor to somebody who

+// may not costs them their work.

+func (s *Store) CanWrite(ctx context.Context, a Actor, docID int64) bool {

+ var count int64

+ if err := writable(s.db.WithContext(ctx).Model(&Document{}), a).

+ Where("documents.id = ?", docID).

+ Count(&count).Error; err != nil {

+ s.logger.Error("docs: checking write access", "doc", docID, "err", err)

+ return false

+ }

+ return count > 0

+}

+

+// ListFor returns every non-deleted document the acting person may read —

+// the ones they own, plus the ones they hold a grant on — newest first, on

+// the same UpdatedAt clock List uses so this list and handlers.go's recency

+// buckets cannot disagree.

+//

+// A second method beside List rather than a widening of it, for the reason

+// GetFor gives: List is owner-scoped and callers exist that want exactly

+// that, and widening in place hands every one of them grantees silently.

+//

+// The order deliberately does not separate "mine" from "shared with me".

+// Recency is the sort the list page already teaches, and a shared document

+// somebody is working in today belongs above one of yours you have not

+// opened since spring. Which are yours the row answers itself, through

+// docRow.SharedNote, without reordering the list.

+func (s *Store) ListFor(ctx context.Context, a Actor) ([]*Document, error) {

+ var docs []*Document

+ err := readable(s.db.WithContext(ctx), a.PersonID, a.Subject).

+ Where("documents.deleted_at IS NULL").

+ Order("documents.updated_at DESC, documents.id DESC").

+ Find(&docs).Error

+ if err != nil {

+ return nil, fmt.Errorf("docs: list readable documents: %w", err)

+ }

+ return docs, nil

+}

diff --git a/internal/docs/grants_admission_test.go b/internal/docs/grants_admission_test.go

new file mode 100644

index 0000000..c39bac2

--- /dev/null

+++ b/internal/docs/grants_admission_test.go

@@ -0,0 +1,85 @@

+package docs

+

+import (

+ "context"

+ "testing"

+)

+

+// A grant implies admission.

+//

+// $DOCS_ALLOWED_ADDRESSES gates who may receive a magic link at all, and

+// gateBegin deliberately answers identically for a refused address so the

+// endpoint is not a membership oracle. Without this, sharing a document to

+// somebody outside the allowlist produces a grant that can never be redeemed:

+// the sharer sees success, the recipient gets a link that silently does

+// nothing, and neither of them is told why. That is the worst available

+// failure shape — it looks like it worked.

+//

+// So a grant widens admission. It is one clause in allows(), on purpose:

+// reversing this decision is deleting it, not unpicking a design.

+func TestAGrantAdmitsAnAddressTheAllowlistDoesNot(t *testing.T) {

+ list := allowlist{addrs: map[string]struct{}{

+ "alice@example.com": {},

+ }}

+

+ if list.admits("bob@example.com") {

+ t.Fatal("bob is admitted before any grant exists; this test proves nothing")

+ }

+

+ granted := map[string]bool{"bob@example.com": true}

+ list.granted = func(address string) (bool, error) { return granted[address], nil }

+

+ if !list.admits("bob@example.com") {

+ t.Error("bob holds a grant and is still refused admission — his invitation can never be redeemed")

+ }

+ if list.admits("carol@example.com") {

+ t.Error("carol holds no grant and was admitted; a grant admits its holder, not everybody")

+ }

+}

+

+// The address is normalised on the way in, the same way the grant stored it.

+func TestGrantedAdmissionNormalisesTheAddress(t *testing.T) {

+ list := allowlist{addrs: map[string]struct{}{}}

+ list.granted = func(address string) (bool, error) { return address == "bob@example.com", nil }

+

+ if !list.admits(" Bob@Example.COM ") {

+ t.Error("a grant made to bob did not admit the same address typed differently")

+ }

+}

+

+// An allowlist that admits everybody keeps doing so — the grant clause is a

+// widening, never a narrowing.

+func TestGrantsDoNotNarrowAnOpenAllowlist(t *testing.T) {

+ list := allowlist{} // addrs nil: development's admit-everyone

+ list.granted = func(string) (bool, error) { return false, nil }

+

+ if !list.admits("anybody@example.com") {

+ t.Error("an open allowlist stopped admitting everybody once grants were wired in")

+ }

+}

+

+// And the store answers the question the allowlist asks.

+func TestHasGrantAnswersForARealGrant(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)

+ }

+

+ if held, err := store.HasGrant(ctx, "bob@example.com"); err != nil || held {

+ t.Fatalf("bob holds a grant before one was made (held=%v err=%v)", held, err)

+ }

+ if err := store.Grant(ctx, alice, doc.Sqid, "bob@example.com", RoleReader); err != nil {

+ t.Fatalf("Grant: %v", err)

+ }

+ if held, err := store.HasGrant(ctx, "bob@example.com"); err != nil || !held {

+ t.Errorf("bob holds a grant and HasGrant says otherwise (held=%v err=%v)", held, err)

+ }

+}

diff --git a/internal/docs/grants_edges_test.go b/internal/docs/grants_edges_test.go

new file mode 100644

index 0000000..2417ed2

--- /dev/null

+++ b/internal/docs/grants_edges_test.go

@@ -0,0 +1,64 @@

+package docs_test

+

+import (

+ "net/http"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// Grant refuses what is not an address.

+//

+// Before this, Grant accepted any non-empty string: "not-an-email" became

+// a row, appeared on the share page as though it were a person, and made

+// HasGrant answer true — widening the instance's front door to a string

+// that can never sign in. (Branch review, finding 10.)

+func TestGrantRefusesNonsenseAddresses(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ for _, bad := range []string{"not-an-email", "@example.com", "bob@", "bob example@x.com"} {

+ rec := post(t, app, alice, "/d/"+sqid+"/share", "address="+strings.ReplaceAll(bad, " ", "+"))

+ if rec.Code == http.StatusSeeOther {

+ t.Errorf("granted to %q", bad)

+ }

+ if body := get(t, app, alice, "/d/"+sqid+"/share").Body.String(); strings.Contains(body, bad) {

+ t.Errorf("the share page lists %q", bad)

+ }

+}

+

+// An owner cannot grant to themselves. They already reach it by owning it,

+// and the row would put them on their own share list looking like an

+// invitation somebody made.

+func TestGrantRefusesTheOwner(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share", "address=alice@example.com&role=writer"); rec.Code == http.StatusSeeOther {

+ t.Error("the owner granted a document to herself")

+ }

+}

+

+// A grant on a document that no longer exists does not hold the front door

+// open. Admission is meant to follow from reaching something; a grant on a

+// deleted document reaches nothing, because readable hides it.

+func TestAGrantOnADeletedDocumentAdmitsNobody(t *testing.T) {

+ app := members(t, "alice@example.com")

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+

+ if _, err := app.TestSignIn(t, "bob@example.com"); err != nil {

+ t.Fatalf("precondition: the grantee cannot sign in: %v", err)

+ }

+

+ app.TestSoftDelete(t, sqid)

+

+ if _, err := app.TestSignIn(t, "bob@example.com"); err == nil {

+ t.Error("a grant on a deleted document still admits its holder")

+ }

+}

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/guest_test.go b/internal/docs/guest_test.go

new file mode 100644

index 0000000..7524cb3

--- /dev/null

+++ b/internal/docs/guest_test.go

@@ -0,0 +1,225 @@

+package docs_test

+

+import (

+ "net/http"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// members builds an app whose front door is a real allowlist, so the

+// difference between a configured member and somebody admitted only by a

+// grant actually exists. NewForTest's empty allowlist means "admit

+// everyone" in development, which is why none of this was reachable before.

+func members(t *testing.T, addresses string) *docs.App {

+ t.Helper()

+ app, err := docs.NewForTestWith(t, docs.Config{AllowedAddresses: addresses})

+ if err != nil {

+ t.Fatalf("NewForTestWith: %v", err)

+ }

+ return app

+}

+

+// A grant admits its holder as a GUEST, not as a member.

+//

+// Suite §1 ruling 1: "Sharing is membership; an outside guest is a guest

+// member." Docs §8.2: "Outside guests are granted a specific document and

+// become guest members." The branch had no guest kind at all, so a single

+// reader grant was an unbounded, transitive key to the instance's front

+// door: Bob is admitted by Alice's grant, creates his own documents, shares

+// them to Mallory, who is now admitted and can repeat it. Alice cannot

+// undo any of it, and $DOCS_ALLOWED_ADDRESSES silently stopped being the

+// membership list the operator configured. (Branch review, finding 1.)

+//

+// A guest reads and writes what they were granted, and nothing else.

+func TestAGuestCannotCreateOrShare(t *testing.T) {

+ app := members(t, "alice@example.com")

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+

+ // Bob gets in — the invitation is redeemable, which is the whole

+ // reason grant-admission exists.

+ bob := signInCookies(t, app, "bob@example.com")

+ if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusOK {

+ t.Fatalf("the guest cannot read the document he was granted: %d", rec.Code)

+ }

+

+ // But he is a guest. He cannot mint documents of his own — 403

+ // exactly, so a 500 cannot pass for a refusal.

+ if rec := post(t, app, bob, "/documents", "title=Bob%27s+own"); rec.Code != http.StatusForbidden {

+ t.Errorf("guest create: status %d, want 403", rec.Code)

+ }

+ // ...and he cannot admit anybody else, which is the transitive half

+ // and the one that makes the first unbounded.

+ //

+ // Asserting 403 specifically, not merely "not 303". The first version

+ // of this test accepted any non-303, which the OWNER check already

+ // produces for Bob on Alice's document — so deleting the member guard

+ // from sharePost left it green. The re-review caught that by mutation:

+ // three of the four guarded routes had never been exercised at all.

+ for _, c := range []struct{ what, path, body string }{

+ {"share", "/d/" + sqid + "/share", "address=mallory@example.com&role=writer"},

+ {"revoke", "/d/" + sqid + "/share/revoke", "address=alice@example.com"},

+ {"publish", "/d/" + sqid + "/share/link", "on=1"},

+ } {

+ if rec := post(t, app, bob, c.path, c.body); rec.Code != http.StatusForbidden {

+ t.Errorf("guest %s: status %d, want 403 from the member guard", c.what, rec.Code)

+ }

+}

+

+// A guest is not shown a control they cannot use. Two of them: the create

+// form in the list header, and File → New document in the editor's menu

+// bar. Both post /documents, which answers a guest 403 with a bare text

+// page. (Re-review, finding 3.)

+func TestAGuestIsNotShownCreateControls(t *testing.T) {

+ app := members(t, "alice@example.com")

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+ bob := signInCookies(t, app, "bob@example.com")

+

+ for _, page := range []string{"/", "/d/" + sqid} {

+ rec := get(t, app, bob, page)

+ // The status first: an absent control and an unrendered page look

+ // identical to a substring check, so without this the test passes

+ // on a 404 — which is the opposite of what it claims to show.

+ if rec.Code != http.StatusOK {

+ t.Fatalf("the guest's %s answered %d, want 200", page, rec.Code)

+ }

+ if strings.Contains(rec.Body.String(), `action="/documents"`) {

+ t.Errorf("the guest's %s offers a create control they are refused", page)

+ }

+

+ // Alice keeps both, or the check above passes on an app that offers

+ // nobody a way to create a document.

+ for _, page := range []string{"/", "/d/" + sqid} {

+ if body := get(t, app, alice, page).Body.String(); !strings.Contains(body, `action="/documents"`) {

+ t.Errorf("the member's %s lost its create control", page)

+ }

+}

+

+// A configured member keeps everything. Without this the rule above passes

+// on an app where nobody can create anything.

+func TestAMemberStillCreatesAndShares(t *testing.T) {

+ app := members(t, "alice@example.com,carol@example.com")

+ carol := signInCookies(t, app, "carol@example.com")

+

+ sqid := createDoc(t, app, carol, "Carol's")

+ if rec := post(t, app, carol, "/d/"+sqid+"/share", "address=dave@example.com&role=reader"); rec.Code != http.StatusSeeOther {

+ t.Errorf("a configured member could not share: status %d", rec.Code)

+ }

+}

+

+// Revoking the last grant ends admission, not just the document.

+//

+// Before this, `allows()` ran only at sign-in, so a revoked guest kept a

+// valid session for up to 30 days — and, per the finding above, kept

+// creating and sharing with it. "Stop sharing" was complete for the

+// document and said nothing about the instance. (Branch review, finding 3.)

+func TestRevokingTheLastGrantEndsAdmission(t *testing.T) {

+ app := members(t, "alice@example.com")

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+

+ bob := signInCookies(t, app, "bob@example.com")

+ if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusOK {

+ t.Fatalf("precondition: the guest cannot read (%d)", rec.Code)

+ }

+

+ post(t, app, alice, "/d/"+sqid+"/share/revoke", "address=bob@example.com")

+

+ // The same cookies, still cryptographically valid, now reach nothing.

+ // 303 exactly: "not 200" would let a 500 pass for a refusal.

+ for _, path := range []string{"/", "/d/" + sqid} {

+ if rec := get(t, app, bob, path); rec.Code != http.StatusSeeOther {

+ t.Errorf("a revoked guest's GET %s answered %d, want 303", path, rec.Code)

+ }

+

+ // A member is not touched by any of this: the recheck must key on

+ // admission, not on holding a grant.

+ if rec := get(t, app, alice, "/"); rec.Code != http.StatusOK {

+ t.Errorf("the owner lost access to her own list: %d", rec.Code)

+ }

+}

+

+// A revoked guest's writes are REFUSED, not redirected.

+//

+// The worst defect on this branch, and I introduced it while fixing the

+// last one. requireAdmission answered every method with 303 → /signin.

+// fetch() follows redirects by default, so the editor's POST became a GET

+// of the sign-in page, came back 200 with HTML, and save-queue.mjs read

+// res.ok and reported the batch saved. Alice revokes Bob mid-sentence and

+// Bob types for an hour into a page that says everything is fine.

+//

+// That is strictly worse than the reader-editor bug this branch already

+// fixed: that one at least lit the "save failed" indicator. rastrillo's

+// own refuseSession redirects GET/HEAD and 403s everything else, for

+// exactly this reason, and requireAdmission now does the same.

+//

+// A redirect is a fine answer to "show me a page". It is never an answer

+// to "store this". (Re-review, finding 1.)

+func TestARevokedGuestsWritesAreRefusedNotRedirected(t *testing.T) {

+ app := members(t, "alice@example.com")

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+ bob := signInCookies(t, app, "bob@example.com")

+

+ // He really can write, first — or the assertion below passes on an

+ // app where nobody can.

+ if rec := postBytes(t, app, bob, "/d/"+sqid+"/updates", docs.ReadFragmentFixture(t, "hello")); rec.Code != http.StatusNoContent {

+ t.Fatalf("precondition: the guest writer cannot write (%d)", rec.Code)

+ }

+

+ post(t, app, alice, "/d/"+sqid+"/share/revoke", "address=bob@example.com")

+

+ // Now every write must be refused with a status the browser reports

+ // as a failure. A 3xx here is the bug: fetch follows it and the save

+ // queue reads the sign-in page's 200 as success.

+ rec := postBytes(t, app, bob, "/d/"+sqid+"/updates", docs.ReadFragmentFixture(t, "hello"))

+ if rec.Code >= 300 && rec.Code < 400 {

+ t.Errorf("a revoked guest's write was redirected (%d → %q); fetch follows it and the save reports ok",

+ rec.Code, rec.Header().Get("Location"))

+ }

+ if rec.Code != http.StatusForbidden {

+ t.Errorf("a revoked guest's write answered %d; want 403", rec.Code)

+ }

+

+ // GETs still redirect — that is what a redirect is for, and a person

+ // following a link deserves the sign-in page rather than a bare 403.

+ if rec := get(t, app, bob, "/"); rec.Code != http.StatusSeeOther {

+ t.Errorf("a revoked guest's GET / answered %d, want 303 to sign-in", rec.Code)

+ }

+}

+

+// The wiring, end to end through the real sign-in, rather than allows()

+// and HasGrant each tested against a stand-in for the other.

+func TestAGrantAdmitsThroughTheRealSignIn(t *testing.T) {

+ app := members(t, "alice@example.com")

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ // Before any grant, Bob is refused — and refused invisibly, since

+ // gateBegin answers a refused address exactly as it answers a real

+ // one so the endpoint is not a membership oracle.

+ if _, err := app.TestSignIn(t, "bob@example.com"); err == nil {

+ t.Fatal("an address on no list and holding no grant was mailed a link")

+ }

+

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+

+ link, err := app.TestSignIn(t, "bob@example.com")

+ if err != nil {

+ t.Fatalf("a granted address was not mailed a link: %v", err)

+ }

+ if !strings.Contains(link, "/auth/verify") {

+ t.Fatalf("not a verify link: %q", link)

+ }

+}

diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go

index 85d655b..6c14a80 100644

--- a/internal/docs/handlers.go

+++ b/internal/docs/handlers.go

@@ -57,6 +57,36 @@ type handlers struct {

// exists at all.

streamHeartbeat time.Duration

streamTTL time.Duration

+

+ // isMember answers "is this address on the configured allowlist",

+ // as opposed to admitted by a grant. It gates the member-only powers

+ // — creating a document, and changing who reaches one — so that a

+ // guest reads and writes what they were granted and nothing else

+ // (suite §1 ruling 1, Docs §8.2, branch review finding 1).

+ //

+ // A func rather than the allowlist value because handlers has no

+ // business with the rest of that type, and because app.go must be the

+ // one place the two halves of admission are wired together.

+ isMember func(address string) bool

+

+ // oneShots parks a freshly minted public URL for exactly one render

+ // (oneshot.go). Here rather than on Store for the reason hub and

+ // previews are: Store is persistence, and this value is deliberately

+ // never persisted.

+ oneShots *oneShots

+

+ // origin is cfg.Origin, the app's own validated base URL (app.go's

+ // New refuses to boot without one outside development). The share

+ // page needs it to print a public link somebody can copy into a

+ // message: a relative path is not a thing you can paste to a person

+ // who is not already here.

+ //

+ // The configured origin rather than r.Host, deliberately. r.Host is

+ // attacker-controlled, and a link built from it is a link an

+ // attacker can aim somewhere else by sending the owner a request

+ // with a Host header of their choosing. csrf.Protect already trusts

+ // cfg.Origin for exactly this reason; this trusts the same value.

+ origin string

}

// listPage is GET /'s view data (spec §6.4). Title and the group

@@ -79,6 +109,12 @@ type handlers struct {

// (rastrillo/auth.DefaultSessionTTL); a page with no way to end one

// on purpose is a real gap on something public, not a cosmetic one.

type listPage struct {

+ // CanCreate gates the create form in the list header. A guest is

+ // answered 403 by POST /documents, and a form whose only outcome is a

+ // bare error page is a control that exists to fail — the same defect

+ // as the editor a reader could type into. (Re-review, finding 3.)

+ CanCreate bool

+

Title string

Groups []docGroup

EmptyTitle string

@@ -134,6 +170,16 @@ type docRow struct {

// could not be read, or one past a cold render's budget — and

// renders as the empty box the row has always shown.

Thumb thumbView

+

+ // SharedNote is set on a row the reader does not own — "Shared with

+ // you" — and empty on their own.

+ //

+ // The list used to be able to say nothing about whose a document was,

+ // because every row was the reader's; the comment on `list` said so

+ // and gave that as the reason. Sharing made it false, and a row that

+ // looks like yours and is not is the setup for opening somebody's

+ // document expecting to edit it. (Branch review, finding 9.)

+ SharedNote string

}

// owner resolves the signed-in person's Store.Person id for r —

@@ -153,6 +199,42 @@ 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

+}

+

+// member reports whether the person on this request is a full member of

+// the instance rather than a guest admitted by a grant.

+//

+// A guest reaching a member-only route is answered 403 rather than 404.

+// The app answers 404 for documents, and for a good reason (§4.2: "not

+// yours" must not be distinguishable from "does not exist"), but there is

+// no document here to hide and nothing secret about the instance having

+// members — the guest was invited by one. Pretending POST /documents does

+// not exist would be a lie told for no gain.

+func (h *handlers) member(r *http.Request) bool {

+ if h.isMember == nil {

+ return true

+ }

+ id, ok := rauth.From(r)

+ return ok && h.isMember(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

@@ -175,7 +257,9 @@ func (h *handlers) owner(r *http.Request) (int64, error) {

// the page — rendering it would be inventing display data, not

// showing real ownership.

func (h *handlers) list(w http.ResponseWriter, r *http.Request) {

- owner, err := h.owner(r)

+ // actor, not owner: this page answers "what can I reach", which

+ // since sharing landed is a wider question than "what is mine".

+ who, err := h.actor(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)

@@ -190,6 +274,7 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request) {

query := strings.TrimSpace(r.URL.Query().Get("q"))

data := listPage{

+ CanCreate: h.member(r),

Title: rastrillo.T(r, "docs.list.title"),

EmptyTitle: rastrillo.T(r, "docs.list.empty.title"),

EmptyBody: rastrillo.T(r, "docs.list.empty.body"),

@@ -207,7 +292,7 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request) {

}

if query != "" {

- hits, err := h.store.Search(r.Context(), owner, query)

+ hits, err := h.store.SearchFor(r.Context(), who, query)

if err != nil {

h.logger.Error("docs: search documents", "err", err)

http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

@@ -218,10 +303,11 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request) {

thumb := h.thumbnailer(r.Context())

for _, d := range hits {

data.Results = append(data.Results, docRow{

- Href: "/d/" + d.Sqid,

- Title: d.Title,

- When: formatWhen(now, d.UpdatedAt),

- Thumb: thumb(d),

+ SharedNote: sharedNote(r, d, who.PersonID),

+ Href: "/d/" + d.Sqid,

+ Title: d.Title,

+ When: formatWhen(now, d.UpdatedAt),

+ Thumb: thumb(d),

})

}

if err := h.render.page(w, r, "list", data); err != nil {

@@ -231,13 +317,13 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request) {

return

}

- docs, err := h.store.List(r.Context(), owner)

+ docs, err := h.store.ListFor(r.Context(), who)

if err != nil {

h.logger.Error("docs: list documents", "err", err)

http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

return

}

- data.Groups = groupByRecency(r, time.Now().UTC(), docs, h.thumbnailer(r.Context()))

+ data.Groups = groupByRecency(r, time.Now().UTC(), docs, h.thumbnailer(r.Context()), who.PersonID)

if err := h.render.page(w, r, "list", data); err != nil {

h.logger.Error("docs: render list", "err", err)

http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

@@ -254,6 +340,14 @@ 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) {

+ // Member-only: a guest reads and writes what they were granted, and

+ // minting documents of their own is not that. Without this, one grant

+ // is a transitive key to the front door — the guest creates, shares,

+ // and admits the next person (branch review, finding 1).

+ if !h.member(r) {

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusForbidden)

+ return

+ }

owner, err := h.owner(r)

if err != nil {

h.logger.Error("docs: resolve signed-in person", "err", err)

@@ -307,8 +401,54 @@ type showPage struct {

// to happen in eleven locales nobody in this repo reads. One key,

// one string, and the link can never disagree with the page it

// leads to.

- BackLabel string

- Sqid string // views/show.html's [docs-editor] mount point's data-sqid, the editor bundle's own base for /d/{sqid}/updates

+ BackLabel string

+ Sqid string // views/show.html's [docs-editor] mount point's data-sqid, the editor bundle's own base for /d/{sqid}/updates

+

+ // CanShare and ShareLabel drive the share control views/show.html

+ // puts in the shell bar's "nav" block — which topbar.html renders

+ // inside [rst-shell-tail], immediately before the account

+ // disclosure. That is the position Paul asked for, and it is a slot

+ // the shell already owns; drawing a bar of our own to hold it is the

+ // chrome drift spec §10 says to watch for.

+ //

+ // CanShare is ownership, not readability. A grantee — a writer

+ // included, since no grant carries re-sharing — is not offered it,

+ // because sharePage is owner-only and answers them 404. Rendering a

+ // control that visibly fails is worse than rendering none: it reads

+ // as a broken app rather than as a boundary.

+ //

+ // ShareLabel reuses docs.share.title rather than adding a key of its

+ // own, for BackLabel's reason directly above: one string, so a

+ // control can never disagree with the page it leads to across

+ // eleven locales.

+ //

+ // It is a label rather than a glyph because rastrillo's icon set has

+ // no "share" in it. That set is a closed vocabulary of twelve, whose

+ // own comments argue each addition, and Icon renders an unknown slug

+ // as nothing at all — so {{icon "share"}} would have shipped an

+ // invisible control that no test of ours would have caught. A

+ // thirteenth icon is a rastrillo change, not a docs one.

+ CanShare bool

+ ShareLabel string

+

+ // CanWrite gates the editor: the ProseMirror mount point, its

+ // data-sqid, and the script that mounts it (views/show.html).

+ //

+ // A reader grantee reaches this page — GetFor admits them, and that is

+ // correct, they may read it. What was not correct was rendering them

+ // the owner's page: the bundle mounted, the toolbar enabled, and the

+ // document became contenteditable for somebody every write of whose

+ // the server refuses. The refusal worked; that is not the point. By

+ // the time the 404 arrives the person has typed a paragraph, and

+ // save-queue.mjs classifies a 4xx as permanent, drops the batch and

+ // keeps taking keystrokes. They lose the work, having been shown an

+ // editor. "It fails safely on the server" is not a defence when the

+ // cost is a person's work. (Branch review, finding 2.)

+ //

+ // So the editor is absent rather than disabled. A disabled editor is

+ // one CSS rule from being an editor again; an absent one has nothing

+ // to re-enable. views/public.html already demonstrates the shape.

+ CanWrite bool

EditorScript string // views/show.html's <script src>, from editorScriptPath() — never a literal, so it can never drift from what staticHandler actually serves

// SaveRetrying/SaveFailed are the editor's own save-status copy

@@ -418,21 +558,23 @@ func newDialogView(r *http.Request) dialogView {

// not plain text — inside a page-shaped canvas (views/show.html). An

// 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

+// from one that does not exist — because Store.GetFor's reader-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")

- owner, err := h.owner(r)

+ actor, err := h.actor(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)

+ // GetFor, not Get: a grantee reads this page too. Get stays owner-only

+ // and is what the share and revoke paths use.

+ doc, err := h.store.GetFor(r.Context(), actor, sqid)

if err != nil {

if errors.Is(err, gorm.ErrRecordNotFound) {

http.Error(w, rastrillo.T(r, "docs.error.notfound"), http.StatusNotFound)

@@ -456,6 +598,9 @@ func (h *handlers) show(w http.ResponseWriter, r *http.Request) {

SignOutLabel: rastrillo.T(r, "docs.signout.label"),

BackLabel: rastrillo.T(r, "docs.list.title"),

Sqid: doc.Sqid,

+ CanShare: doc.OwnerID == actor.PersonID,

+ CanWrite: h.store.CanWrite(r.Context(), actor, doc.ID),

+ ShareLabel: rastrillo.T(r, "docs.share.title"),

EditorScript: editorScriptPath(),

SaveRetrying: rastrillo.T(r, "docs.editor.status.retrying"),

SaveFailed: rastrillo.T(r, "docs.editor.status.failed"),

@@ -463,7 +608,7 @@ func (h *handlers) show(w http.ResponseWriter, r *http.Request) {

JumpToScript: jumpToScriptPath(),

Toolbar: newToolbarView(r),

- Menu: newMenuView(r, doc.Sqid),

+ Menu: newMenuView(r, doc.Sqid, h.member(r)),

FontFaces: fontFaces(),

ViewMode: viewMode(r),

PageLabel: rastrillo.T(r, "docs.editor.page.number"),

@@ -483,10 +628,10 @@ func (h *handlers) show(w http.ResponseWriter, r *http.Request) {

// corrupt document that looks intact.

const maxLogBytes = 8 << 20

-// getDocumentForUpdates is the owner-scoped lookup both

+// getDocumentForUpdates is the reader-scoped lookup both

// updatesGet and updatesPost share: it resolves the signed-in

-// person, then loads the document through Store.Get — already

-// owner-scoped and already 404-not-403 (its own doc comment) — so a

+// person, then loads the document through Store.GetFor — already

+// reader-scoped and already 404-not-403 (its own doc comment) — so a

// stranger's sqid returns exactly the same outcome as one that does

// not exist. ok is false once a response has already been written

// (404 or 500) and the caller must return immediately without doing

@@ -494,14 +639,17 @@ const maxLogBytes = 8 << 20

func (h *handlers) getDocumentForUpdates(w http.ResponseWriter, r *http.Request) (doc *Document, ok bool) {

sqid := r.PathValue("sqid")

- owner, err := h.owner(r)

+ actor, err := h.actor(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 nil, false

}

- doc, err = h.store.Get(r.Context(), owner, sqid)

+ // Readable, not owned. This backs the stream, the updates GET and both

+ // exports, all of which a grantee may use. The updates POST authorizes

+ // separately in ApplyUpdate, where a reader is refused.

+ doc, err = h.store.GetFor(r.Context(), actor, sqid)

if err != nil {

if errors.Is(err, gorm.ErrRecordNotFound) {

http.Error(w, rastrillo.T(r, "docs.error.notfound"), http.StatusNotFound)

@@ -522,7 +670,7 @@ func (h *handlers) getDocumentForUpdates(w http.ResponseWriter, r *http.Request)

// will not decode; the server cannot detect that corruption itself

// (it never calls into the core here), so the browser is the only

// reader that can, and raw=1 is what lets it fall back to the one

-// source it can always trust. Owner-scoped exactly like show

+// source it can always trust. Reader-scoped exactly like show

// (getDocumentForUpdates), and capped at maxLogBytes above.

//

// Why snapshot-then-tail loses nothing: a snapshot IS an ordinary Yjs

@@ -670,8 +818,27 @@ 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 —

+ // the same BODY too, which it did not used to be. This branch

+ // answered http.NotFound ("404 page not found") while a stranger

+ // got http.Error with the catalog's text, so the sentence above

+ // and store.go's "refused identically" were both false. Harmless

+ // as an oracle (a reader already knows the document exists) and

+ // still worth fixing, because the comment was the thing being

+ // relied on. (Branch review, finding 7f.)

+ if errors.Is(err, ErrNotWritable) {

+ http.Error(w, rastrillo.T(r, "docs.error.notfound"), http.StatusNotFound)

+ return

+ }

if updateIsClientFault(err) {

// Info, not Error: a client sending bytes the core

// cleanly rejects is an ordinary, expected outcome of

@@ -751,6 +918,16 @@ func updateIsClientFault(err error) bool {

return core.Recovered(err) || core.Rejected(err) || errors.Is(err, core.ErrUpdateLogFull)

}

+// sharedNote labels a row the reader does not own. Empty for their own

+// documents: a list where every row carries a badge is a list where none

+// of them reads as information.

+func sharedNote(r *http.Request, d *Document, mine int64) string {

+ if d.OwnerID == mine {

+ return ""

+ }

+ return rastrillo.T(r, "docs.list.shared")

+}

+

// groupByRecency buckets docs — already newest-first, per Store.List

// — into recency groups without re-sorting: consecutive documents

// that share a bucket join the same group, and a new bucket starts a

@@ -767,7 +944,7 @@ func updateIsClientFault(err error) bool {

// group labels ("Today … Yesterday … Today") the instant an older

// document got a new update. See store_test.go's

// TestListOrdersByUpdatedAt.

-func groupByRecency(r *http.Request, now time.Time, docs []*Document, thumb func(*Document) thumbView) []docGroup {

+func groupByRecency(r *http.Request, now time.Time, docs []*Document, thumb func(*Document) thumbView, mine int64) []docGroup {

var groups []docGroup

var key string

for _, d := range docs {

@@ -778,10 +955,11 @@ func groupByRecency(r *http.Request, now time.Time, docs []*Document, thumb func

}

g := &groups[len(groups)-1]

g.Rows = append(g.Rows, docRow{

- Href: "/d/" + d.Sqid,

- Title: d.Title,

- When: formatWhen(now, d.UpdatedAt),

- Thumb: thumb(d),

+ SharedNote: sharedNote(r, d, mine),

+ Href: "/d/" + d.Sqid,

+ Title: d.Title,

+ When: formatWhen(now, d.UpdatedAt),

+ Thumb: thumb(d),

})

}

return groups

diff --git a/internal/docs/links.go b/internal/docs/links.go

new file mode 100644

index 0000000..d6ef71f

--- /dev/null

+++ b/internal/docs/links.go

@@ -0,0 +1,176 @@

+package docs

+

+import (

+ "context"

+ "crypto/rand"

+ "crypto/sha256"

+ "encoding/base64"

+ "encoding/hex"

+ "time"

+

+ "gorm.io/gorm"

+)

+

+// Link is one document's read-only public capability: whoever holds the

+// token may read the document, signed in or not.

+//

+// The third rung of the suite doctrine's ruling 15, and the reason it has

+// no Role field is the ruling's other half — every write has a named

+// actor, and a link holds no name. See 0006_links.sql.

+type Link struct {

+ ID int64 `gorm:"column:id;primaryKey"`

+ DocID int64 `gorm:"column:doc_id;uniqueIndex"`

+ // TokenHash, never the token (suite doctrine ruling 16). There is no

+ // field on this struct that can yield a working URL, which is the

+ // point: nothing that reads a Link out of the database — a query, a

+ // log line, a debugger — is holding a capability.

+ TokenHash string `gorm:"column:token_hash;uniqueIndex"`

+ CreatedAt time.Time `gorm:"column:created_at"`

+}

+

+// TableName pins the table rather than leaving it to GORM's pluraliser,

+// the same reason Grant does.

+func (Link) TableName() string { return "links" }

+

+// tokenBytes is 128 bits of entropy, which base64url renders as 22

+// characters. The token is the entire authorization for a published

+// document, so this is the one number in this file worth arguing about:

+// 128 bits is the floor at which guessing stops being a strategy, and a

+// URL a person pastes into a message is not improved by being longer.

+const tokenBytes = 16

+

+// newToken mints a capability. crypto/rand, never math/rand, and never

+// anything derived from the document — a token computed from doc_id, sqid,

+// title or a clock is guessable by whoever reads this file.

+//

+// rand.Read is documented never to return a short read without an error,

+// so an error here means the system has no entropy and the honest response

+// is to fail the request rather than publish under a weak token.

+func newToken() (string, error) {

+ buf := make([]byte, tokenBytes)

+ if _, err := rand.Read(buf); err != nil {

+ return "", err

+ }

+ return base64.RawURLEncoding.EncodeToString(buf), nil

+}

+

+// hashToken is the one-way form stored in the database.

+//

+// SHA-256, unsalted, and both halves of that are deliberate. A salt

+// defends against precomputation over a small input space; there is no

+// small input space here, because the token is 128 bits from crypto/rand

+// and is its own salt. A password hash (bcrypt, argon2) would buy the same

+// nothing while making every load of a published page do key-stretching

+// work. The property that matters — the stored form does not yield the

+// token — a fast hash gives just as completely.

+//

+// Hex rather than base64 for the stored form: it is fixed-width, it is

+// what a person comparing rows by eye expects of a SHA-256, and the column

+// is never parsed by anything but this function.

+func hashToken(token string) string {

+ sum := sha256.Sum256([]byte(token))

+ return hex.EncodeToString(sum[:])

+}

+

+// Publish turns a document's public link on and returns it, minting a

+// fresh token every time.

+//

+// Fresh every time, deliberately: with Unpublish deleting the row, that

+// makes "off" a revocation rather than a pause. Somebody who kept a URL

+// from an earlier audience does not get back in when the owner publishes

+// again for a different one.

+//

+// Owner-only through Get, so a writer grantee is answered exactly as a

+// stranger is — no grant carries re-sharing, and publishing to everyone is

+// the largest re-share there is.

+func (s *Store) Publish(ctx context.Context, owner int64, sqid string) (string, error) {

+ doc, err := s.Get(ctx, owner, sqid)

+ if err != nil {

+ return "", err

+ }

+ token, err := newToken()

+ if err != nil {

+ return "", err

+ }

+

+ link := Link{DocID: doc.ID, TokenHash: hashToken(token), CreatedAt: time.Now().UTC()}

+ // Delete-then-insert rather than an upsert on doc_id: the old token

+ // must stop working, and a row whose token changed and a row that was

+ // replaced are the same outcome only if nothing else ever holds a

+ // reference to the row. Nothing does, and this way the intent reads

+ // off the code.

+ err = s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {

+ if err := tx.Where("doc_id = ?", doc.ID).Delete(&Link{}).Error; err != nil {

+ return err

+ }

+ return tx.Create(&link).Error

+ })

+ if err != nil {

+ return "", err

+ }

+ // The plaintext token is returned here and never again: this is the

+ // only moment it exists outside the caller's hand (ruling 16).

+ return token, nil

+}

+

+// Unpublish turns it off. Like Revoke, unpublishing something that is not

+// published succeeds: the owner asked for an outcome — that this document

+// is not readable by strangers — and that outcome holds either way.

+func (s *Store) Unpublish(ctx context.Context, owner int64, sqid string) error {

+ doc, err := s.Get(ctx, owner, sqid)

+ if err != nil {

+ return err

+ }

+ return s.db.WithContext(ctx).Where("doc_id = ?", doc.ID).Delete(&Link{}).Error

+}

+

+// IsPublished reports whether a document currently has a public link.

+//

+// It answers a bool and not a link, and that is ruling 16 showing up in a

+// signature: there is no way to ask this store for a published document's

+// URL, because the store does not have it. The share page can say "this is

+// published" and offer to replace or stop it; it cannot show the URL

+// again, and no future caller can be tempted to make it.

+//

+// Owner-scoped through Get: whether a document is published is part of the

+// share page, which is the owner's alone.

+func (s *Store) IsPublished(ctx context.Context, owner int64, sqid string) (bool, error) {

+ doc, err := s.Get(ctx, owner, sqid)

+ if err != nil {

+ return false, err

+ }

+ var count int64

+ if err := s.db.WithContext(ctx).Model(&Link{}).

+ Where("doc_id = ?", doc.ID).Count(&count).Error; err != nil {

+ return false, err

+ }

+ return count > 0, nil

+}

+

+// ByToken resolves a public link to its document, for the one route in

+// this app that no session guards.

+//

+// An unknown token and a token whose document has been deleted answer

+// identically — gorm.ErrRecordNotFound, which the handler turns into the

+// same 404 a nonexistent URL gets. That is §4.2's rule reaching the public

+// route: without it, the difference between the two answers is an oracle

+// for which documents are published.

+//

+// The empty token is rejected before the query, so a request to a route

+// that somehow lost its path value cannot match a row with an empty

+// column.

+func (s *Store) ByToken(ctx context.Context, token string) (*Document, error) {

+ if token == "" {

+ return nil, gorm.ErrRecordNotFound

+ }

+ var doc Document

+ err := s.db.WithContext(ctx).

+ Joins("JOIN links ON links.doc_id = documents.id").

+ Where("links.token_hash = ? AND documents.deleted_at IS NULL AND documents.owner_id > 0",

+ hashToken(token)).

+ First(&doc).Error

+ if err != nil {

+ return nil, err

+ }

+ return &doc, nil

+}

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/menu.go b/internal/docs/menu.go

index ea91fe5..3779653 100644

--- a/internal/docs/menu.go

+++ b/internal/docs/menu.go

@@ -160,7 +160,34 @@ func viewMode(r *http.Request) string {

return ViewPaged

}

-func newMenuView(r *http.Request, sqid string) menuView {

+// fileItems is the File menu, whose first entry is the one thing in this

+// bar a guest may not do.

+//

+// "New document" is a plain form posting /documents, which a guest is

+// answered 403 for. A control that exists only to fail is the same defect

+// as the editor a reader could type into, in a smaller place — so it is

+// absent for them rather than present and refused.

+func fileItems(r *http.Request, sqid string, canCreate bool) []menuItem {

+ items := make([]menuItem, 0, 8)

+ if canCreate {

+ items = append(items, menuItem{Kind: menuForm, Action: "/documents", Label: rastrillo.T(r, "docs.editor.menu.file.new")})

+ }

+ return append(items,

+ menuItem{Kind: menuLink, Href: "/", Label: rastrillo.T(r, "docs.editor.menu.file.open")},

+ menuItem{Kind: menuDivider},

+ // The two exports, moved here from [docs-doc-actions]. Still plain

+ // links with the download attribute, so they behave identically to

+ // before for anyone who had learned where they were — the route

+ // did not change, only where it is named.

+ menuItem{Kind: menuLink, Href: "/d/" + sqid + "/export.pdf", Download: true, Label: rastrillo.T(r, "docs.export.pdf.label")},

+ menuItem{Kind: menuLink, Href: "/d/" + sqid + "/export.md", Download: true, Label: rastrillo.T(r, "docs.export.markdown.label")},

+ menuItem{Kind: menuCommand, Command: "print", Label: rastrillo.T(r, "docs.editor.menu.file.print"), Shortcut: "Ctrl+P"},

+ menuItem{Kind: menuDivider},

+ menuItem{Kind: menuForm, Action: "/signout", Label: rastrillo.T(r, "docs.signout.label")},

+ )

+}

+

+func newMenuView(r *http.Request, sqid string, canCreate bool) menuView {

mode := viewMode(r)

return menuView{

Label: rastrillo.T(r, "docs.editor.menu.label"),

@@ -168,22 +195,7 @@ func newMenuView(r *http.Request, sqid string) menuView {

{

ID: "file",

Title: rastrillo.T(r, "docs.editor.menu.file"),

- Items: []menuItem{

- {Kind: menuForm, Action: "/documents", Label: rastrillo.T(r, "docs.editor.menu.file.new")},

- {Kind: menuLink, Href: "/", Label: rastrillo.T(r, "docs.editor.menu.file.open")},

- {Kind: menuDivider},

- // The two exports, moved here from

- // [docs-doc-actions]. Still plain links with the

- // download attribute, so they behave identically to

- // before for anyone who had learned where they were

- // — the route did not change, only where it is

- // named.

- {Kind: menuLink, Href: "/d/" + sqid + "/export.pdf", Download: true, Label: rastrillo.T(r, "docs.export.pdf.label")},

- {Kind: menuLink, Href: "/d/" + sqid + "/export.md", Download: true, Label: rastrillo.T(r, "docs.export.markdown.label")},

- {Kind: menuCommand, Command: "print", Label: rastrillo.T(r, "docs.editor.menu.file.print"), Shortcut: "Ctrl+P"},

- {Kind: menuDivider},

- {Kind: menuForm, Action: "/signout", Label: rastrillo.T(r, "docs.signout.label")},

- },

+ Items: fileItems(r, sqid, canCreate),

},

{

ID: "edit",

diff --git a/internal/docs/menu_test.go b/internal/docs/menu_test.go

index f341463..5b726fa 100644

--- a/internal/docs/menu_test.go

+++ b/internal/docs/menu_test.go

@@ -14,7 +14,15 @@ import (

// clicked.

func newMenuViewForTest() menuView {

- return newMenuView(httptest.NewRequest("GET", "/d/abc123", nil), "abc123")

+ return newMenuViewFor(true)

+}

+

+// newMenuViewFor builds either variant. The guest's File menu is one item

+// shorter, and the structure rules below (no leading divider, every form

+// carries an action) have to hold for it too — a shorter list is exactly

+// where a leading divider would appear.

+func newMenuViewFor(canCreate bool) menuView {

+ return newMenuView(httptest.NewRequest("GET", "/d/abc123", nil), "abc123", canCreate)

}

// Every command the menu names is implemented by one of the two modules

@@ -89,7 +97,22 @@ func TestMenuCommandsExistInTheBundle(t *testing.T) {

// malformed in a manner the Go compiler is happy with and a person is

// not.

func TestMenuStructureIsWellFormed(t *testing.T) {

- v := newMenuViewForTest()

+ // Both variants. The guest's File menu is one item shorter, and a

+ // shorter list is exactly where a leading divider appears — so the

+ // rules below have to hold for it, not only for a member's.

+ //

+ // The previous commit added newMenuViewFor and its comment saying so,

+ // and then never called it with false. The claim sat in a commit

+ // message for one round. (Re-review 4, finding 3.)

+ for _, canCreate := range []bool{true, false} {

+ t.Run(map[bool]string{true: "member", false: "guest"}[canCreate], func(t *testing.T) {

+ assertMenuWellFormed(t, newMenuViewFor(canCreate))

+ })

+ }

+}

+

+func assertMenuWellFormed(t *testing.T, v menuView) {

+ t.Helper()

if len(v.Menus) == 0 {

t.Fatal("the menu bar has no menus")

diff --git a/internal/docs/migrations/0005_grants.sql b/internal/docs/migrations/0005_grants.sql

new file mode 100644

index 0000000..200481b

--- /dev/null

+++ b/internal/docs/migrations/0005_grants.sql

@@ -0,0 +1,51 @@

+-- 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.

+-- `created_by` is the person who made the grant, and it is in the spec's own

+-- data model (§4) — this table shipped without it, and the omission was never

+-- argued, only overlooked while the nullable `doc_id` above was argued at

+-- length. It is the one column that answers "who let this address in".

+--

+-- That question has teeth because admission is transitive in shape even where

+-- it is closed in policy: a guest is admitted by somebody's grant, and if a

+-- guest ever gains the power to grant, the chain of who admitted whom is only

+-- reconstructable if each link recorded its author. Adding it now, while the

+-- table is new, costs a line; adding it after a leak costs the history.

+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_by INTEGER NOT NULL DEFAULT 0,

+ 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/migrations/0006_links.sql b/internal/docs/migrations/0006_links.sql

new file mode 100644

index 0000000..518bd1e

--- /dev/null

+++ b/internal/docs/migrations/0006_links.sql

@@ -0,0 +1,55 @@

+-- Publishing: a link is one document's read-only capability, held by whoever

+-- has the URL rather than by anybody the app can name.

+--

+-- This is the third rung of the suite doctrine's ruling 15 — private, granted,

+-- and a read-only link — and the ruling's other half is why there is no role

+-- column here. Every write has a named actor; a link holds no name, so a link

+-- can only ever read. A `role` column would be a place for somebody to later

+-- write "writer" into, and the answer to "should an anonymous link be able to

+-- edit" is already no (Paul's own Sheets ruling 12). The column that cannot

+-- exist is the cheapest way to keep that decision.

+--

+-- `token_hash` is a SHA-256 of the capability, never the capability itself

+-- (suite doctrine ruling 16, following Sheets spec §8.3). The token is 128

+-- bits from crypto/rand, base64url, never derived from doc_id, sqid, title or

+-- clock — a guessable token is the whole feature failing silently, and a

+-- derived one is guessable by anybody who reads this file.

+--

+-- Stored as a hash because a token is a STANDING KEY where a document is a

+-- snapshot. Leaked content is what it was on the day it leaked; a leaked token

+-- keeps opening the live document, every later revision, until somebody

+-- notices. Hashing turns a backup or a replica from an ongoing capability into

+-- a one-time disclosure. That argument holds here even though this app stores

+-- its content in plaintext, which is the tempting reason to skip it.

+--

+-- The consequence is that the app cannot show the URL twice. It is shown once,

+-- in the response to the publish that minted it, and after that the share page

+-- can say only whether a link exists. An owner who loses it replaces it.

+--

+-- Unsalted, deliberately: a salt defends a small input space and there is not

+-- one here — the token's own 128 bits are the salt. SHA-256 rather than a

+-- password hash for the same reason, and because a public page load should not

+-- do key-stretching work.

+--

+-- UNIQUE(doc_id): one live link per document, so "the link" is never an

+-- ambiguous phrase in the UI or in an incident. Turning publishing off DELETEs

+-- the row, and turning it back on mints a fresh token — which makes "off" a

+-- revocation rather than a pause. Somebody who kept a URL from last March does

+-- not get back in when the owner republishes for a different audience.

+--

+-- No expiry column, deliberately: an unexpiring link the owner can see and

+-- switch off is honest, while an expiry nobody is reminded of is a promise the

+-- app makes on the owner's behalf and then keeps only by accident.

+CREATE TABLE IF NOT EXISTS links (

+ id INTEGER PRIMARY KEY AUTOINCREMENT,

+ doc_id INTEGER NOT NULL,

+ token_hash TEXT NOT NULL,

+ created_at DATETIME

+);

+

+CREATE UNIQUE INDEX IF NOT EXISTS idx_links_doc ON links (doc_id);

+

+-- The public read path has only a token to go on, so the lookup that has to be

+-- fast — and single-row — leads on its hash. Byte-equality on an indexed

+-- column is not a timing concern at 128 bits of entropy.

+CREATE UNIQUE INDEX IF NOT EXISTS idx_links_token_hash ON links (token_hash);

diff --git a/internal/docs/oneshot.go b/internal/docs/oneshot.go

new file mode 100644

index 0000000..04e4331

--- /dev/null

+++ b/internal/docs/oneshot.go

@@ -0,0 +1,84 @@

+package docs

+

+import (

+ "fmt"

+ "sync"

+ "time"

+)

+

+// oneShotTTL is how long a minted public URL waits to be collected. Long

+// enough to survive a 303 and a slow render, short enough that a URL left

+// behind by a browser that never followed the redirect is gone before

+// anybody could come back to the machine and find it.

+const oneShotTTL = 30 * time.Second

+

+// oneShots holds each freshly minted public URL for exactly one read.

+//

+// It exists because of ruling 16 and the redirect, together. The token is

+// shown once and the database cannot produce it again, so the share page

+// has to be handed it — but sharePost answers 303 (a POST that renders its

+// own page is a POST a reload re-submits, and here a re-submit would

+// rotate the link the person is in the middle of copying). So the URL has

+// to survive one redirect and no longer.

+//

+// In memory, and that is consistent rather than lazy: this app is one

+// process on one SQLite file, which is the same assumption hub and

+// previews already make. A restart between the POST and the GET loses the

+// URL; the owner republishes, which is the same recovery as losing it any

+// other way, and no worse than the alternative of writing the plaintext

+// token to the database — which is the whole thing being avoided.

+type oneShots struct {

+ mu sync.Mutex

+ items map[string]oneShot

+}

+

+type oneShot struct {

+ value string

+ expires time.Time

+}

+

+func newOneShots() *oneShots { return &oneShots{items: map[string]oneShot{}} }

+

+// key scopes a pending URL to one person and one document, so a second

+// person on the same document — or the same person on another — can never

+// collect it.

+func oneShotKey(person int64, sqid string) string {

+ return fmt.Sprintf("%d:%s", person, sqid)

+}

+

+// put stores value under key, replacing anything already there. Publishing

+// twice in a row leaves the newer URL, which is the one that works.

+func (o *oneShots) put(key, value string) {

+ o.mu.Lock()

+ defer o.mu.Unlock()

+ o.sweepLocked()

+ o.items[key] = oneShot{value: value, expires: time.Now().Add(oneShotTTL)}

+}

+

+// take returns the value and removes it. A second read returns "", which

+// is what makes this show-once rather than show-until-expiry: a reload of

+// the share page after collecting the URL shows the published state and no

+// URL, exactly as a visit tomorrow would.

+func (o *oneShots) take(key string) string {

+ o.mu.Lock()

+ defer o.mu.Unlock()

+ item, ok := o.items[key]

+ delete(o.items, key)

+ if !ok || time.Now().After(item.expires) {

+ return ""

+ }

+ return item.value

+}

+

+// sweepLocked drops expired entries. Called on put rather than on a timer:

+// the map only grows when somebody publishes, so that is the only moment

+// it can need clearing, and a background goroutine for a map this size

+// would be more machinery than the problem.

+func (o *oneShots) sweepLocked() {

+ now := time.Now()

+ for k, v := range o.items {

+ if now.After(v.expires) {

+ delete(o.items, k)

+ }

+}

diff --git a/internal/docs/publiclink_test.go b/internal/docs/publiclink_test.go

new file mode 100644

index 0000000..07bdb76

--- /dev/null

+++ b/internal/docs/publiclink_test.go

@@ -0,0 +1,330 @@

+package docs_test

+

+import (

+ "net/http"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// anon is a request with no cookies at all — nobody, arriving from

+// somewhere else. The whole point of a public link is that this works, so

+// the helper exists to make every test below say "signed out" in one word.

+func anon(t *testing.T, app *docs.App, path string) *http.Response {

+ t.Helper()

+ rec := get(t, app, nil, path)

+ return rec.Result()

+}

+

+// linkFor publishes a document and returns its public path, collected the

+// one way the app offers it: the share page rendered immediately after the

+// publish. Under ruling 16 the database holds a SHA-256 and cannot produce

+// a URL, so there is no second chance and no store-level shortcut — the

+// test has to take the same one shot a person does.

+func linkFor(t *testing.T, app *docs.App, cookies []*http.Cookie, sqid string) string {

+ t.Helper()

+ if rec := post(t, app, cookies, "/d/"+sqid+"/share/link", "on=1"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST link: status %d, want 303", rec.Code)

+ }

+ return readLink(t, get(t, app, cookies, "/d/"+sqid+"/share").Body.String())

+}

+

+// readLink pulls a /p/... path out of a rendered share page.

+func readLink(t *testing.T, body string) string {

+ t.Helper()

+ i := strings.Index(body, "/p/")

+ if i < 0 {

+ t.Fatal("the share page does not show the public link it just created")

+ }

+ rest := body[i:]

+ end := strings.IndexAny(rest, `"'< `)

+ if end < 0 {

+ t.Fatal("could not read the public link off the share page")

+ }

+ return rest[:end]

+}

+

+// The feature, end to end: Paul's "being able to share read-only publicly".

+// A signed-out stranger with the link reads the document.

+func TestAPublicLinkLetsASignedOutStrangerRead(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ // Before the link exists, the app has no public door at all.

+ if res := anon(t, app, "/d/"+sqid); res.StatusCode == http.StatusOK {

+ t.Fatal("a signed-out request read a document through the private route")

+ }

+

+ path := linkFor(t, app, alice, sqid)

+ res := anon(t, app, path)

+ if res.StatusCode != http.StatusOK {

+ t.Fatalf("GET %s signed out: status %d, want 200", path, res.StatusCode)

+ }

+}

+

+// Read-only means read-only, and the interesting half is that it holds

+// against the routes rather than only against the rendering. Ruling 15:

+// every write has a named actor, and a link holds no name.

+func TestAPublicLinkCarriesNoWrite(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ linkFor(t, app, alice, sqid)

+

+ // Holding the link does not admit its holder to any authenticated

+ // route: it is a capability on one document, not a session.

+ for _, path := range []string{

+ "/",

+ "/d/" + sqid,

+ "/d/" + sqid + "/share",

+ "/d/" + sqid + "/updates",

+ } {

+ if res := anon(t, app, path); res.StatusCode == http.StatusOK {

+ t.Errorf("signed out, %s answered 200; a link is not a session", path)

+ }

+

+ // And the public page offers no editor to drive one with.

+ //

+ // "editor-entry" used to be in this list and appears on no page in

+ // this app — a ban that could never fail. The mount element and the

+ // real served script path are what actually matter.

+ body := get(t, app, nil, linkFor(t, app, alice, sqid)).Body.String()

+ for _, banned := range []string{"docs-text docs-editor", "data-sqid", docs.TestEditorScriptPath(), "/updates"} {

+ if strings.Contains(body, banned) {

+ t.Errorf("the public page ships %q; it is a reading surface", banned)

+ }

+}

+

+// A wrong token is indistinguishable from a document that does not exist —

+// spec §4.2's rule, which the public route has to keep too or it becomes

+// an oracle for which documents are published.

+func TestAWrongTokenIsANotFound(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ path := linkFor(t, app, alice, sqid)

+

+ for _, bad := range []string{"/p/", "/p/nope", "/p/" + strings.Repeat("a", 43), path + "x"} {

+ if res := anon(t, app, bad); res.StatusCode != http.StatusNotFound {

+ t.Errorf("GET %s answered %d, want exactly 404", bad, res.StatusCode)

+ }

+}

+

+// The refusals are byte-identical, which is the invariant the branch

+// claims and nothing previously checked. The old test asserted only

+// "!= 200", so a 500 from a database error would have passed it while

+// telling a stranger that this token means something.

+//

+// Unknown, revoked, and pointing-at-a-deleted-document must be one answer.

+// The differences between them are exactly the oracle §4.2 exists to deny:

+// which documents were ever published, and which were withdrawn.

+func TestPublicRefusalsAreIndistinguishable(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+

+ type answer struct{ status, body, ctype string }

+ capture := func(path string) answer {

+ rec := get(t, app, nil, path)

+ return answer{

+ status: rec.Result().Status,

+ body: rec.Body.String(),

+ ctype: rec.Header().Get("Content-Type"),

+ }

+

+ // (i) a syntactically valid token that was never minted.

+ unknown := capture("/p/" + strings.Repeat("A", 22))

+

+ // (ii) a real token, revoked.

+ revokedSqid := createDoc(t, app, alice, "Revoked")

+ revokedPath := linkFor(t, app, alice, revokedSqid)

+ post(t, app, alice, "/d/"+revokedSqid+"/share/link", "on=0")

+ revoked := capture(revokedPath)

+

+ // (iii) a live token whose document has been deleted underneath it.

+ deletedSqid := createDoc(t, app, alice, "Deleted")

+ deletedPath := linkFor(t, app, alice, deletedSqid)

+ app.TestSoftDelete(t, deletedSqid)

+ deleted := capture(deletedPath)

+

+ if unknown.status != "404 Not Found" {

+ t.Fatalf("an unknown token answered %q, want 404", unknown.status)

+ }

+ for name, got := range map[string]answer{"revoked": revoked, "deleted-document": deleted} {

+ if got != unknown {

+ t.Errorf("%s answers differently from an unknown token:\n got: %+v\nwant: %+v", name, got, unknown)

+ }

+}

+

+// The published page shows the document's actual content.

+//

+// Every other test here publishes an EMPTY document, so if ByToken's join

+// ever scanned links.id into Document.ID the wrong document would be

+// replayed and every one of them would still pass with a 200. GORM emits

+// SELECT documents.* here today; nothing held it.

+func TestAPublicLinkShowsTheDocumentsContent(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ app.TestWrite(t, alice, sqid, "hello")

+

+ body := get(t, app, nil, linkFor(t, app, alice, sqid)).Body.String()

+ if !strings.Contains(body, "hello") {

+ t.Error("the published page does not contain the document's text")

+ }

+ if strings.Contains(body, sqid) {

+ t.Error("the published page names the private sqid")

+ }

+}

+

+// A published URL is shown once and never again — ruling 16 as a

+// behaviour rather than as a column type. Reloading the share page after

+// collecting it must show the document as published WITHOUT the URL.

+func TestThePublicURLIsShownExactlyOnce(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ path := linkFor(t, app, alice, sqid) // consumes the one showing

+

+ again := get(t, app, alice, "/d/"+sqid+"/share").Body.String()

+ if strings.Contains(again, path) {

+ t.Error("the share page showed the public URL a second time")

+ }

+ if strings.Contains(again, "/p/") {

+ t.Error("the share page still prints a /p/ URL after the one showing")

+ }

+ // But it still says the document is published, or the owner cannot

+ // tell a published document from an unpublished one.

+ //

+ // The catalog key, not the word: NewForTest wires no BaseCatalog, so

+ // strings render as their keys — and "published" matched the key

+ // "docs.share.public.published" whatever the page actually said,

+ // which made this assertion unfalsifiable.

+ if !strings.Contains(again, "docs.share.public.published") {

+ t.Error("the share page does not say the document is published")

+ }

+ // And the link itself still works — shown once, not valid once.

+ if res := anon(t, app, path); res.StatusCode != http.StatusOK {

+ t.Errorf("the link stopped working after being shown: %d", res.StatusCode)

+ }

+}

+

+// Nothing anywhere stores the plaintext token. The point of ruling 16,

+// asserted against the database rather than against the code that writes

+// to it.

+func TestTheTokenIsNotStoredInPlaintext(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ path := linkFor(t, app, alice, sqid)

+ token := strings.TrimPrefix(path, "/p/")

+

+ if app.TestRowContains(t, "links", token) {

+ t.Error("the plaintext token is in the links table")

+ }

+}

+

+// Turning it off is the revoke, and it has to be immediate: a link that

+// keeps working after the owner switched it off is the failure that makes

+// people distrust the whole control.

+func TestTurningThePublicLinkOffClosesIt(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ path := linkFor(t, app, alice, sqid)

+

+ if res := anon(t, app, path); res.StatusCode != http.StatusOK {

+ t.Fatalf("precondition: the link does not work (%d)", res.StatusCode)

+ }

+ if rec := post(t, app, alice, "/d/"+sqid+"/share/link", "on=0"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST link off: status %d, want 303", rec.Code)

+ }

+ if res := anon(t, app, path); res.StatusCode == http.StatusOK {

+ t.Error("the link still reads the document after being turned off")

+ }

+

+ // Turning it back on mints a NEW token: the old one stays dead, so

+ // "off" is a revocation rather than a pause somebody can undo for a

+ // stranger who kept the URL.

+ again := linkFor(t, app, alice, sqid)

+ if again == path {

+ t.Error("re-enabling reissued the same token; the revoked link came back")

+ }

+ if res := anon(t, app, path); res.StatusCode == http.StatusOK {

+ t.Error("the old token works again after the link was re-enabled")

+ }

+}

+

+// Publishing over a live link rotates it. This is the property the store

+// promises ("a fresh token every time") and the one the off/on test above

+// cannot see: with the row deleted in between, any implementation mints a

+// new token, so that test passes on a Publish that reuses an existing one.

+// A mutation that made Publish return the existing row survived it, which

+// is why this test exists.

+//

+// It is the rotation an owner needs when a link has gone somewhere they

+// did not intend: republish, and the copy that escaped stops working.

+func TestPublishingAgainRotatesALiveLink(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ first := linkFor(t, app, alice, sqid)

+ second := linkFor(t, app, alice, sqid) // no Unpublish in between

+ if second == first {

+ t.Fatal("publishing over a live link reissued the same token")

+ }

+ if res := anon(t, app, first); res.StatusCode == http.StatusOK {

+ t.Error("the rotated-away token still reads the document")

+ }

+ if res := anon(t, app, second); res.StatusCode != http.StatusOK {

+ t.Errorf("the new token does not work: status %d", res.StatusCode)

+ }

+}

+

+// Only the owner controls it. A writer grantee cannot publish somebody

+// else's document to the whole internet — the same rule that stops them

+// re-sharing it to one person, and it matters far more here.

+func TestOnlyTheOwnerMayPublish(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ bob := signInCookies(t, app, "bob@example.com")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+

+ for _, c := range []struct{ path, body string }{

+ {"/d/" + sqid + "/share/link", "on=1"},

+ {"/d/" + sqid + "/share/link", "on=0"},

+ {"/d/" + sqid + "/share/revoke", "address=alice@example.com"},

+ {"/d/" + sqid + "/share", "address=eve@example.com&role=reader"},

+ } {

+ if rec := post(t, app, bob, c.path, c.body); rec.Code != http.StatusNotFound {

+ t.Errorf("a writer grantee reached POST %s (%s): status %d, want 404", c.path, c.body, rec.Code)

+ }

+}

+

+// The public page must not carry who else can reach the document. The

+// share list is the owner's alone, and a published page is the widest

+// audience this app has.

+func TestThePublicPageLeaksNoAudience(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+ path := linkFor(t, app, alice, sqid)

+

+ body := get(t, app, nil, path).Body.String()

+ for _, secret := range []string{"alice@example.com", "bob@example.com", sqid} {

+ if strings.Contains(body, secret) {

+ t.Errorf("the public page names %q", secret)

+ }

+}

diff --git a/internal/docs/publish.go b/internal/docs/publish.go

new file mode 100644

index 0000000..48562dd

--- /dev/null

+++ b/internal/docs/publish.go

@@ -0,0 +1,135 @@

+package docs

+

+import (

+ "errors"

+ "html/template"

+ "net/http"

+

+ "github.com/carlosframework/rastrillo"

+ "gorm.io/gorm"

+)

+

+// publicPage is GET /p/{token}'s view data: a document, and nothing about

+// who can reach it.

+//

+// Compare showPage, which carries Sqid, an editor script, save-status copy

+// and a share control. None of that is here, and the absence is the

+// feature — this struct is the list of things a stranger is allowed to

+// know, so anything added to it is a decision rather than an oversight.

+//

+// Sqid in particular is absent on purpose. It is the private route's key,

+// and printing it on a page anyone can read would hand a stranger the URL

+// the signed-in app is organised around.

+type publicPage struct {

+ Title string

+ HTML template.HTML

+

+ // ReadOnly is the standing notice that this is a published copy. It

+ // is authored copy rather than a rendering decision, so it goes

+ // through the catalog like every other string here.

+ ReadOnly string

+}

+

+// publicShow serves GET /p/{token}, the one route in this app that no

+// session guards.

+//

+// It resolves the token to a document and renders it. There is no branch

+// on who is asking, because a capability does not have a who: a signed-in

+// owner following their own link gets the same page a stranger does, which

+// is what makes the control testable by the person who turned it on.

+//

+// Every failure is 404 (§4.2 reaching the public route): an unknown token,

+// a revoked one, and a token whose document was deleted are one answer, or

+// the differences between them tell a stranger which documents exist and

+// which were published and then withdrawn.

+func (h *handlers) publicShow(w http.ResponseWriter, r *http.Request) {

+ doc, err := h.store.ByToken(r.Context(), r.PathValue("token"))

+ if err != nil {

+ if errors.Is(err, gorm.ErrRecordNotFound) {

+ http.NotFound(w, r)

+ return

+ }

+ h.logger.Error("docs: resolve public link", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+

+ html, err := h.store.HTML(r.Context(), doc)

+ if err != nil {

+ h.logger.Error("docs: public document html", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+

+ // no-store, said rather than implied. The first version of this

+ // handler set no header at all and its comment claimed that kept a

+ // shared cache from retaining a revoked copy — which is not what an

+ // absent Cache-Control means. Absent leaves it to heuristics; a

+ // revoked link's page could sit in an intermediary. stream.go and

+ // search.go already say no-store for exactly this reason.

+ //

+ // X-Robots-Tag because "anyone with the link" is not "anyone": a

+ // capability that leaks into a crawlable place should not also become

+ // a search result. It is not a security control — the link is the

+ // authorization and a crawler that has it can read the page — it is a

+ // refusal to help the leak spread.

+ w.Header().Set("Cache-Control", "no-store")

+ w.Header().Set("X-Robots-Tag", "noindex, nofollow")

+ if err := h.render.page(w, r, "public", publicPage{

+ Title: doc.Title,

+ HTML: html,

+ ReadOnly: rastrillo.T(r, "docs.public.readonly"),

+ }); err != nil {

+ h.logger.Error("docs: render public document", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ }

+}

+

+// shareLink serves POST /d/{sqid}/share/link: the owner's switch.

+//

+// One route for both directions rather than two, because the form is a

+// switch and its current state decides which way it points — two routes

+// would let a stale page turn something on that the owner had just turned

+// off, from a button that still said "off".

+//

+// Owner-only through Publish/Unpublish's own Get. A writer grantee is

+// answered 404: publishing to everyone is the largest re-share there is,

+// and no grant carries re-sharing.

+func (h *handlers) shareLink(w http.ResponseWriter, r *http.Request) {

+ // Member-only, as the other two share routes are. Publishing to the

+ // whole internet is the largest re-share there is.

+ if !h.member(r) {

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusForbidden)

+ return

+ }

+ owner, err := h.owner(r)

+ if err != nil {

+ h.logger.Error("docs: resolve owner for publish", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+ sqid := r.PathValue("sqid")

+

+ on := r.PostFormValue("on") == "1"

+ if on {

+ var token string

+ token, err = h.store.Publish(r.Context(), owner, sqid)

+ if err == nil {

+ // The only moment this URL exists outside the token itself.

+ // Parked for exactly one render (oneshot.go) and never

+ // written down, because the database holds only a hash.

+ h.oneShots.put(oneShotKey(owner, sqid), h.origin+"/p/"+token)

+ }

+ } else {

+ err = h.store.Unpublish(r.Context(), owner, sqid)

+ }

+ switch {

+ case err == nil:

+ http.Redirect(w, r, "/d/"+sqid+"/share", http.StatusSeeOther)

+ case errors.Is(err, gorm.ErrRecordNotFound):

+ http.NotFound(w, r)

+ default:

+ h.logger.Error("docs: set public link", "sqid", sqid, "on", on, "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ }

+}

diff --git a/internal/docs/reader_noeditor_test.go b/internal/docs/reader_noeditor_test.go

new file mode 100644

index 0000000..f39abe7

--- /dev/null

+++ b/internal/docs/reader_noeditor_test.go

@@ -0,0 +1,81 @@

+package docs_test

+

+import (

+ "net/http"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// A reader grantee must not be handed an editor. Found by the branch

+// review, and the worst defect on this branch: `show` used GetFor

+// (correct — a reader may read) and then rendered the owner's page

+// verbatim, so ProseMirror mounted, the toolbar enabled, and the document

+// became contenteditable for somebody the server will refuse.

+//

+// The server refusal was never in doubt: ApplyUpdate returns ErrNotWritable

+// and updatesPost answers 404. The damage is upstream of that. The browser

+// has already taken the keystrokes; the save queue classifies the 404 as

+// permanent, drops the batch, and keeps accepting typing. The person is

+// told "save failed" after writing a paragraph into a page that told them

+// they could.

+//

+// "It fails safely on the server" is not a defence when the cost is a

+// person's work.

+func TestAReaderIsNotGivenAnEditor(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ bob := signInCookies(t, app, "bob@example.com")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+

+ rec := get(t, app, bob, "/d/"+sqid)

+ if rec.Code != http.StatusOK {

+ t.Fatalf("GET /d/%s as a reader: status %d, want 200", sqid, rec.Code)

+ }

+ body := rec.Body.String()

+

+ // The mount ELEMENT, not the string: `[docs-editor]` also appears a

+ // dozen times in this page's own <style> block, so banning the bare

+ // word would fail on a page that is perfectly correct — and would

+ // have hidden whether the element itself was gone. `docs-text

+ // docs-editor` is the attribute pair the mount point carries and the

+ // stylesheet never does.

+ //

+ // data-sqid is the bundle's only way to learn where to post, and the

+ // editor script is checked by its real served path rather than a

+ // guessed substring: the review found this test's earlier sibling

+ // banning "editor-entry", a string that appears on no page in this

+ // app, which is a ban that could never fail.

+ for _, banned := range []string{"docs-text docs-editor", "data-sqid", docs.TestEditorScriptPath()} {

+ if strings.Contains(body, banned) {

+ t.Errorf("a reader's document page ships %q", banned)

+ }

+

+ // The content is still there. A read-only page that renders nothing is

+ // not the fix.

+ if !strings.Contains(body, "Minutes") {

+ t.Error("the reader's page does not show the document")

+ }

+}

+

+// The writer keeps everything. Without this the fix above passes on a

+// page that gives nobody an editor.

+func TestAWriterKeepsTheEditor(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ bob := signInCookies(t, app, "bob@example.com")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+

+ for who, cookies := range map[string][]*http.Cookie{"writer": bob, "owner": alice} {

+ body := get(t, app, cookies, "/d/"+sqid).Body.String()

+ for _, needed := range []string{"docs-text docs-editor", "data-sqid", docs.TestEditorScriptPath()} {

+ if !strings.Contains(body, needed) {

+ t.Errorf("the %s's page is missing %q", who, needed)

+ }

+}

diff --git a/internal/docs/render.go b/internal/docs/render.go

index e18cbe5..a581b5e 100644

--- a/internal/docs/render.go

+++ b/internal/docs/render.go

@@ -62,6 +62,60 @@ var Catalog = rastrillo.Catalog{

// labels are recency buckets (handlers.go's recencyBucket); a

// document's own title and its row's timestamp are data, not

// authored copy, so neither is a catalog key.

+ "docs.share.title": "Share",

+ "docs.share.heading": "Who can open this",

+ "docs.share.address": "Email address",

+ "docs.share.role": "They can",

+ "docs.share.role.reader": "Read it",

+ "docs.share.role.writer": "Read and edit it",

+ "docs.share.submit": "Share",

+ "docs.share.revoke": "Stop sharing",

+ "docs.share.back": "Back to the document",

+ "docs.share.empty": "Only you can open this document.",

+ "docs.share.error.address": "Type the email address of the person you want to share with.",

+

+ // docs.share.public.* is the third rung — a read-only link anyone

+ // can follow, signed in or not (suite doctrine ruling 15).

+ //

+ // The body says "anyone with the link", not "the public": the

+ // distinction is the whole security model of a capability, and an

+ // owner who reads it as "listed somewhere" or as "safe because it is

+ // unguessable" is being told the wrong thing either way. It also

+ // says read, twice over with the heading, because the one question

+ // an owner has about this switch is whether a stranger can change

+ // their document.

+ //

+ // The off label is "Stop publishing" rather than "Turn off": what it

+ // does is kill the URL, and a person who has handed that URL to

+ // somebody deserves a verb that says so. It pairs with

+ // docs.share.revoke's "Stop sharing" by design — one vocabulary for

+ // taking access away, whichever rung it was granted on.

+ "docs.share.public.heading": "Anyone with a link",

+ "docs.share.public.body": "Publish a link that anyone can open to read this document. They will not be able to change it, and they will not need to sign in.",

+ "docs.share.public.on": "Publish a link",

+ "docs.share.public.off": "Stop publishing",

+ "docs.share.public.hint": "Publishing again makes a new link, and the old one stops working.",

+

+ // docs.share.public.once is the one-time notice, and the copy has to

+ // carry a real instruction because the app genuinely cannot repeat

+ // itself: it stores a hash of the token, not the token (suite ruling

+ // 16). "You will not see it again" is a fact here, not a scare.

+ //

+ // docs.share.public.published is what the section says on every later

+ // visit — published, without the URL, which is the shape of the whole

+ // ruling in one line of copy.

+ "docs.share.public.once": "Copy this link now. This is the only time it will be shown.",

+ "docs.share.public.published": "This document is published. The link was shown once when it was created and cannot be shown again — replace it if you no longer have it.",

+ "docs.share.public.replace": "Replace the link",

+

+ // docs.public.readonly is the standing notice on the published page

+ // itself (views/public.html). It is there because a person arriving

+ // from a pasted link has no other way to know they are looking at a

+ // copy they cannot edit — the page has no editor to try, so without

+ // this the absence reads as a page that failed to load its own

+ // controls.

+ "docs.public.readonly": "You are reading a shared copy of this document.",

+

"docs.list.title": "Documents",

"docs.list.group.today": "Today",

"docs.list.group.yesterday": "Yesterday",

@@ -180,6 +234,12 @@ var Catalog = rastrillo.Catalog{

// bar's account menu, which list.html and show.html each fill with

// an "account" block; it was in their two page headers until the

// chrome moved to the topbar shell (see shellLayout above).

+ // docs.list.shared marks a row on the list that belongs to somebody

+ // else. It exists because the list stopped being "your documents" the

+ // day sharing landed, and a row that looks like yours and is not is

+ // how a person opens a document expecting to be able to edit it.

+ "docs.list.shared": "Shared with you",

+

"docs.signout.label": "Sign out",

// docs.editor.status.* is the save-status indicator fix round 1's

@@ -393,6 +453,8 @@ const headTemplate = "views/head.html"

// pageTemplates maps a page name to its file under views/.

var pageTemplates = map[string]string{

"list": "views/list.html",

+ "public": "views/public.html",

+ "share": "views/share.html",

"show": "views/show.html",

"signin": "views/signin.html",

}

diff --git a/internal/docs/search.go b/internal/docs/search.go

index 32fe3f5..f1a0f83 100644

--- a/internal/docs/search.go

+++ b/internal/docs/search.go

@@ -126,12 +126,44 @@ func (s *Store) Search(ctx context.Context, owner int64, query string) ([]*Docum

if q == "" {

return nil, nil

}

-

docs, err := s.List(ctx, owner)

if err != nil {

return nil, err

}

+ return rankAndTrim(docs, q), nil

+}

+

+// SearchFor is Search over everything the acting person may read: theirs,

+// plus what has been shared with them. It sits beside Search for the same

+// reason ListFor sits beside List.

+//

+// It exists at all because Search runs over a list rather than over SQL,

+// so widening only the list page would have shipped an app where a shared

+// document is on the list and cannot be searched for. That reads as a

+// broken search, not as a scope decision, and it is the sort of split a

+// person hits on their second day rather than their first.

+//

+// Both spellings run the same ranking, so the list page, the palette and

+// this cannot drift — the plan's D3, kept while the scope widened.

+func (s *Store) SearchFor(ctx context.Context, a Actor, query string) ([]*Document, error) {

+ q := normaliseForSearch(query)

+ if q == "" {

+ return nil, nil

+ }

+ docs, err := s.ListFor(ctx, a)

+ if err != nil {

+ return nil, err

+ }

+ return rankAndTrim(docs, q), nil

+}

+// rankAndTrim is the ranking both spellings share: keep the titles that

+// match, best rank first, capped at maxSearchResults.

+//

+// It takes documents rather than a scope so that the scope decision lives

+// in exactly one place per caller — the List or ListFor above it — and

+// cannot be half-applied here.

+func rankAndTrim(docs []*Document, q string) []*Document {

type scored struct {

doc *Document

rank searchRank

@@ -143,9 +175,9 @@ func (s *Store) Search(ctx context.Context, owner int64, query string) ([]*Docum

}

- // SliceStable, and List already returns newest-first: that makes

- // "most recently updated" the tie-break without re-reading a clock

- // or comparing timestamps here.

+ // SliceStable, and the list arrives newest-first: that makes "most

+ // recently updated" the tie-break without re-reading a clock or

+ // comparing timestamps here.

sort.SliceStable(hits, func(i, j int) bool { return hits[i].rank < hits[j].rank })

if len(hits) > maxSearchResults {

@@ -155,7 +187,7 @@ func (s *Store) Search(ctx context.Context, owner int64, query string) ([]*Docum

for _, h := range hits {

out = append(out, h.doc)

}

- return out, nil

+ return out

}

// searchResult is one row of the JSON the palette reads.

@@ -172,19 +204,19 @@ type searchResult struct {

// cannot drift from the one the list page uses (the plan's D3). It also

// means no bulk export of a person's titles to the browser.

//

-// Inside app.go's RequireSession group and owner-scoped through

+// Inside app.go's RequireSession group and reader-scoped through

// Search → List, so a document belonging to someone else cannot appear

// here — the same property every other document route has, by the same

// mechanism rather than by a check of its own.

func (h *handlers) searchJSON(w http.ResponseWriter, r *http.Request) {

- owner, err := h.owner(r)

+ who, err := h.actor(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.Search(r.Context(), owner, r.URL.Query().Get("q"))

+ docs, err := h.store.SearchFor(r.Context(), who, r.URL.Query().Get("q"))

if err != nil {

h.logger.Error("docs: search documents", "err", err)

http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

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/share.go b/internal/docs/share.go

new file mode 100644

index 0000000..1ee4c20

--- /dev/null

+++ b/internal/docs/share.go

@@ -0,0 +1,246 @@

+package docs

+

+import (

+ "errors"

+ "net/http"

+

+ "github.com/carlosframework/rastrillo"

+)

+

+// sharePage is the share screen's view data, resolved in the handler rather

+// than in the template, as every other page here does it.

+type sharePage struct {

+ Title string

+ DocTitle string

+ Sqid string

+ Heading string

+ AddressLabel string

+ RoleLabel string

+ RoleReader string

+ RoleWriter string

+ ShareButton string

+ RevokeButton string

+ BackLabel string

+ EmptyBody string

+ Error string

+ Grants []shareRow

+ SignOutLabel string

+

+ // The public link, the third rung of ruling 15. PublicURL is empty

+ // when the document is not published, and that emptiness is the

+ // switch's state — one field rather than a bool beside a string that

+ // could disagree with it.

+ PublicHeading string

+ PublicBody string

+

+ // Published is whether a link exists; PublicURL is the link itself and

+ // is set only in the render immediately after publishing.

+ //

+ // Two fields rather than one because they are two different facts now:

+ // under ruling 16 the app knows a document is published without being

+ // able to say what its URL is. Collapsing them would make "published"

+ // unrepresentable whenever the URL is unavailable — which is almost

+ // always.

+ Published bool

+ PublicURL string

+ PublishButton string

+ UnpublishBtn string

+ ReplaceButton string

+ CopyHint string

+ OnceWarning string

+ PublishedBody string

+}

+

+// shareRow is one person who reaches this document.

+type shareRow struct {

+ Address string

+ Role string

+}

+

+// shareGet serves GET /d/{sqid}/share: who reaches this document, and the

+// form that adds somebody.

+//

+// Owner-only, through Store.ListGrants, which goes through the owner-scoped

+// Get. A grantee — even a writer — gets the 404 a stranger gets: re-sharing

+// is not something a grant carries, and a document you may not share does not

+// announce itself as one you may not share (spec §4.2).

+func (h *handlers) shareGet(w http.ResponseWriter, r *http.Request) {

+ owner, err := h.owner(r)

+ if err != nil {

+ h.logger.Error("docs: resolve owner for share", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+ sqid := r.PathValue("sqid")

+

+ doc, err := h.store.Get(r.Context(), owner, sqid)

+ if err != nil {

+ http.NotFound(w, r)

+ return

+ }

+ grants, err := h.store.ListGrants(r.Context(), owner, sqid)

+ if err != nil {

+ h.logger.Error("docs: list grants", "sqid", sqid, "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+

+ h.renderShare(w, r, doc, grants, "")

+}

+

+func (h *handlers) renderShare(w http.ResponseWriter, r *http.Request, doc *Document, grants []*Grant, errKey string) {

+ // A failure to read the published state is logged and then rendered as

+ // "not published" rather than failing the page: the grant list above it

+ // is the part somebody came here for, and a 500 over the switch would

+ // take the working half of the screen down with it.

+ published, err := h.store.IsPublished(r.Context(), doc.OwnerID, doc.Sqid)

+ if err != nil {

+ h.logger.Error("docs: read published state", "sqid", doc.Sqid, "err", err)

+ }

+

+ // The URL appears only in the render that follows the publish that

+ // minted it (ruling 16: the database holds a hash and cannot produce a

+ // URL). Collected here, once; a reload shows the published state

+ // without it, exactly as a visit tomorrow would.

+ publicURL := h.oneShots.take(oneShotKey(doc.OwnerID, doc.Sqid))

+

+ rows := make([]shareRow, 0, len(grants))

+ for _, g := range grants {

+ label := "docs.share.role.reader"

+ if g.Role == RoleWriter {

+ label = "docs.share.role.writer"

+ }

+ rows = append(rows, shareRow{Address: g.Subject, Role: rastrillo.T(r, label)})

+ }

+

+ page := sharePage{

+ Title: rastrillo.T(r, "docs.share.title"),

+ DocTitle: doc.Title,

+ Sqid: doc.Sqid,

+ Heading: rastrillo.T(r, "docs.share.heading"),

+ AddressLabel: rastrillo.T(r, "docs.share.address"),

+ RoleLabel: rastrillo.T(r, "docs.share.role"),

+ RoleReader: rastrillo.T(r, "docs.share.role.reader"),

+ RoleWriter: rastrillo.T(r, "docs.share.role.writer"),

+ ShareButton: rastrillo.T(r, "docs.share.submit"),

+ RevokeButton: rastrillo.T(r, "docs.share.revoke"),

+ BackLabel: rastrillo.T(r, "docs.share.back"),

+ EmptyBody: rastrillo.T(r, "docs.share.empty"),

+ Grants: rows,

+ SignOutLabel: rastrillo.T(r, "docs.signout.label"),

+

+ PublicHeading: rastrillo.T(r, "docs.share.public.heading"),

+ PublicBody: rastrillo.T(r, "docs.share.public.body"),

+ Published: published,

+ PublicURL: publicURL,

+ PublishButton: rastrillo.T(r, "docs.share.public.on"),

+ UnpublishBtn: rastrillo.T(r, "docs.share.public.off"),

+ ReplaceButton: rastrillo.T(r, "docs.share.public.replace"),

+ CopyHint: rastrillo.T(r, "docs.share.public.hint"),

+ OnceWarning: rastrillo.T(r, "docs.share.public.once"),

+ PublishedBody: rastrillo.T(r, "docs.share.public.published"),

+ }

+ if errKey != "" {

+ page.Error = rastrillo.T(r, errKey)

+ }

+

+ // no-store whenever this render carries the plaintext capability.

+ // "Shown once" was a server-side truth only: the browser's disk cache

+ // and bfcache would keep the URL, and any intermediary might, so on a

+ // shared machine the next person could page back to it.

+ // (Re-review, finding 5.)

+ if page.PublicURL != "" {

+ w.Header().Set("Cache-Control", "no-store")

+ }

+ if err := h.render.page(w, r, "share", page); err != nil {

+ h.logger.Error("docs: render share", "sqid", doc.Sqid, "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ }

+}

+

+// sharePost serves POST /d/{sqid}/share: grant an address access.

+//

+// Scriptless: a plain form with a named address field and a role radio, and a

+// 303 back to this page afterwards so a reload does not re-post.

+func (h *handlers) sharePost(w http.ResponseWriter, r *http.Request) {

+ // Member-only, on top of the owner check below: changing who reaches

+ // a document is a member power. A guest owns no documents (create is

+ // closed to them), so this is belt and braces — and it is the brace

+ // that holds if a guest ever acquires one.

+ if !h.member(r) {

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusForbidden)

+ return

+ }

+ owner, err := h.owner(r)

+ if err != nil {

+ h.logger.Error("docs: resolve owner for share", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+ sqid := r.PathValue("sqid")

+

+ doc, err := h.store.Get(r.Context(), owner, sqid)

+ if err != nil {

+ http.NotFound(w, r)

+ return

+ }

+

+ role := RoleReader

+ if r.PostFormValue("role") == string(RoleWriter) {

+ role = RoleWriter

+ }

+

+ err = h.store.Grant(r.Context(), owner, sqid, r.PostFormValue("address"), role)

+ switch {

+ case err == nil:

+ http.Redirect(w, r, "/d/"+sqid+"/share", http.StatusSeeOther)

+ case errors.Is(err, errEmptyAddress):

+ // Re-render with the message rather than redirecting, so the person

+ // sees what was wrong beside the field they typed it into.

+ grants, listErr := h.store.ListGrants(r.Context(), owner, sqid)

+ if listErr != nil {

+ h.logger.Error("docs: list grants after refusal", "sqid", sqid, "err", listErr)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+ // Content-Type before WriteHeader, or the header render.page

+ // sets afterwards is dropped and the response works only by

+ // sniffing.

+ w.Header().Set("Content-Type", "text/html; charset=utf-8")

+ w.WriteHeader(http.StatusBadRequest)

+ h.renderShare(w, r, doc, grants, "docs.share.error.address")

+ default:

+ h.logger.Error("docs: grant", "sqid", sqid, "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ }

+}

+

+// shareRevoke serves POST /d/{sqid}/share/revoke.

+func (h *handlers) shareRevoke(w http.ResponseWriter, r *http.Request) {

+ // Member-only, on top of the owner check below: changing who reaches

+ // a document is a member power. A guest owns no documents (create is

+ // closed to them), so this is belt and braces — and it is the brace

+ // that holds if a guest ever acquires one.

+ if !h.member(r) {

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusForbidden)

+ return

+ }

+ owner, err := h.owner(r)

+ if err != nil {

+ h.logger.Error("docs: resolve owner for revoke", "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+ sqid := r.PathValue("sqid")

+

+ if _, err := h.store.Get(r.Context(), owner, sqid); err != nil {

+ http.NotFound(w, r)

+ return

+ }

+ if err := h.store.Revoke(r.Context(), owner, sqid, r.PostFormValue("address")); err != nil {

+ h.logger.Error("docs: revoke", "sqid", sqid, "err", err)

+ http.Error(w, rastrillo.T(r, "docs.error.generic"), http.StatusInternalServerError)

+ return

+ }

+ http.Redirect(w, r, "/d/"+sqid+"/share", http.StatusSeeOther)

+}

diff --git a/internal/docs/share_nav_test.go b/internal/docs/share_nav_test.go

new file mode 100644

index 0000000..93ff9ce

--- /dev/null

+++ b/internal/docs/share_nav_test.go

@@ -0,0 +1,72 @@

+package docs_test

+

+import (

+ "net/http"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// Paul's note: "there should be a 'share' icon probably beside the account

+// dropdown top right". Before this, the share page existed and nothing led

+// to it — sharing worked only for somebody who knew to type /share onto a

+// URL, which is not a feature, it is a rumour.

+//

+// The control goes in the shell's own "nav" block, which topbar.html puts

+// inside [rst-shell-tail] immediately before the account disclosure. That

+// is the requested position, and it is a slot the design system already

+// owns rather than a bar this app draws for itself (spec §10).

+func TestTheOwnerGetsAShareControlInTheBar(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ rec := get(t, app, alice, "/d/"+sqid)

+ if rec.Code != http.StatusOK {

+ t.Fatalf("GET /d/%s: status %d, want 200", sqid, rec.Code)

+ }

+ body := rec.Body.String()

+ if !strings.Contains(body, "/d/"+sqid+"/share") {

+ t.Error("the document page offers no route to its own share page")

+ }

+

+ // Position, not just presence: the link has to be in the shell's tail,

+ // beside the account dropdown, rather than anywhere on the page. A

+ // test that only asserted presence would pass on a link buried in the

+ // footer.

+ tail := body[strings.Index(body, "rst-shell-tail"):]

+ acct := strings.Index(tail, "rst-shell-account")

+ link := strings.Index(tail, "/d/"+sqid+"/share")

+ if link < 0 || acct < 0 || link > acct {

+ t.Error("the share control is not in the shell tail before the account dropdown")

+ }

+}

+

+// A grantee must not be offered it. sharePage is owner-only and answers a

+// grantee with 404, so a share link on their page would be a control that

+// visibly fails — worse than no control, because it reads as a broken app

+// rather than as a boundary.

+func TestAGranteeIsNotOfferedTheShareControl(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ bob := signInCookies(t, app, "bob@example.com")

+

+ // A writer, deliberately: the strongest grant there is still does not

+ // carry re-sharing (Store.Grant's own rule).

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+

+ rec := get(t, app, bob, "/d/"+sqid)

+ if rec.Code != http.StatusOK {

+ t.Fatalf("GET /d/%s as Bob: status %d, want 200", sqid, rec.Code)

+ }

+ if strings.Contains(rec.Body.String(), "/d/"+sqid+"/share") {

+ t.Error("a writer grantee was offered a share control they cannot use")

+ }

+

+ // And the boundary it stands for is real, not just hidden in markup.

+ if rec := get(t, app, bob, "/d/"+sqid+"/share"); rec.Code != http.StatusNotFound {

+ t.Errorf("GET share as a grantee: status %d, want 404", rec.Code)

+ }

+}

diff --git a/internal/docs/share_test.go b/internal/docs/share_test.go

new file mode 100644

index 0000000..b861990

--- /dev/null

+++ b/internal/docs/share_test.go

@@ -0,0 +1,175 @@

+package docs_test

+

+import (

+ "net/http"

+ "net/http/httptest"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// post sends a scriptless form the way a browser would: urlencoded body, a

+// real Origin, and the signed-in person's cookies. There is no CSRF token

+// field anywhere in this app — csrf.Protect reads Sec-Fetch-Site first and

+// falls back to Origin — so a test that forgets the header is testing the

+// CSRF layer rather than the handler.

+func post(t *testing.T, app *docs.App, cookies []*http.Cookie, path, body string) *httptest.ResponseRecorder {

+ t.Helper()

+ req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(body))

+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

+ req.Header.Set("Origin", docs.DefaultOrigin)

+ for _, c := range cookies {

+ req.AddCookie(c)

+ }

+ rec := httptest.NewRecorder()

+ app.Handler().ServeHTTP(rec, req)

+ return rec

+}

+

+func get(t *testing.T, app *docs.App, cookies []*http.Cookie, path string) *httptest.ResponseRecorder {

+ t.Helper()

+ req := httptest.NewRequest(http.MethodGet, path, nil)

+ for _, c := range cookies {

+ req.AddCookie(c)

+ }

+ rec := httptest.NewRecorder()

+ app.Handler().ServeHTTP(rec, req)

+ return rec

+}

+

+// createDoc makes a document as the signed-in person and returns its sqid.

+func createDoc(t *testing.T, app *docs.App, cookies []*http.Cookie, title string) string {

+ t.Helper()

+ rec := post(t, app, cookies, "/documents", "title="+title)

+ if rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST /documents: status %d, want 303", rec.Code)

+ }

+ return strings.TrimPrefix(rec.Header().Get("Location"), "/d/")

+}

+

+// The whole point of the feature, end to end and scriptless: Alice shares a

+// document with Bob by typing his address into a form, and Bob can read it.

+func TestShareThenTheGranteeCanRead(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ // Bob cannot see it yet.

+ bob := signInCookies(t, app, "bob@example.com")

+ if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusNotFound {

+ t.Fatalf("bob reading before the share: status %d, want 404", rec.Code)

+ }

+

+ rec := post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+ if rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST share: status %d, want 303", rec.Code)

+ }

+

+ if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusOK {

+ t.Errorf("bob reading after the share: status %d, want 200", rec.Code)

+ }

+}

+

+// The share page lists who currently holds a grant, or the owner cannot tell

+// what they have shared and with whom — which is the question the page exists

+// to answer.

+func TestSharePageListsGrantees(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST share: status %d", rec.Code)

+ }

+

+ rec := get(t, app, alice, "/d/"+sqid+"/share")

+ if rec.Code != http.StatusOK {

+ t.Fatalf("GET share: status %d, want 200", rec.Code)

+ }

+ if !strings.Contains(rec.Body.String(), "bob@example.com") {

+ t.Error("the share page does not name the person the document is shared with")

+ }

+}

+

+// Only the owner shares. A grantee — even one who can write — reaches the

+// share page exactly as a stranger does, with a 404 rather than a 403: a

+// document you may not share does not announce itself as one you may not

+// share (spec §4.2).

+func TestOnlyTheOwnerReachesTheSharePage(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST share: status %d", rec.Code)

+ }

+

+ bob := signInCookies(t, app, "bob@example.com")

+ if rec := get(t, app, bob, "/d/"+sqid+"/share"); rec.Code != http.StatusNotFound {

+ t.Errorf("a writer reaching the share page: status %d, want 404", rec.Code)

+ }

+ if rec := post(t, app, bob, "/d/"+sqid+"/share", "address=carol@example.com&role=reader"); rec.Code != http.StatusNotFound {

+ t.Errorf("a writer re-sharing: status %d, want 404", rec.Code)

+ }

+

+ carol := signInCookies(t, app, "carol@example.com")

+ if rec := get(t, app, carol, "/d/"+sqid); rec.Code != http.StatusNotFound {

+ t.Error("bob managed to re-share the document with carol")

+ }

+}

+

+// Revoking takes the access away again. Without this the share page is a

+// one-way door and a mistyped address is permanent.

+func TestRevokeRemovesAccess(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ bob := signInCookies(t, app, "bob@example.com")

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST share: status %d", rec.Code)

+ }

+ if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusOK {

+ t.Fatalf("bob cannot read after the share; the revoke below would prove nothing")

+ }

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share/revoke", "address=bob@example.com"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST revoke: status %d, want 303", rec.Code)

+ }

+

+ if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusNotFound {

+ t.Errorf("bob reading after revoke: status %d, want 404", rec.Code)

+ }

+}

+

+// A reader granted read access cannot write, through the real HTTP path

+// rather than only at the store. The 404 is the same one a stranger gets.

+func TestAReaderCannotPostUpdates(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST share: status %d", rec.Code)

+ }

+

+ bob := signInCookies(t, app, "bob@example.com")

+ rec := post(t, app, bob, "/d/"+sqid+"/updates", "")

+ if rec.Code != http.StatusNotFound {

+ t.Errorf("a reader posting an update: status %d, want 404", rec.Code)

+ }

+}

+

+// An empty address is refused rather than stored, so the page cannot mint a

+// grant that matches nobody and lists as a blank row forever.

+func TestShareRefusesAnEmptyAddress(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ rec := post(t, app, alice, "/d/"+sqid+"/share", "address=&role=reader")

+ if rec.Code == http.StatusSeeOther {

+ t.Error("an empty address was accepted as a share")

+ }

+}

diff --git a/internal/docs/shared_list_test.go b/internal/docs/shared_list_test.go

new file mode 100644

index 0000000..7ad59c4

--- /dev/null

+++ b/internal/docs/shared_list_test.go

@@ -0,0 +1,139 @@

+package docs_test

+

+import (

+ "net/http"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// The difference between "sharing works" and "sharing is usable". Slice 2

+// left a grantee able to open a shared document at its direct URL and with

+// no way to find it again: List was owner-only, so the document Alice

+// shared was reachable only by somebody who had kept the link.

+//

+// A person who has to keep a URL in a chat scrollback to reach a document

+// does not have the document.

+func TestASharedDocumentAppearsInTheGranteesList(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+

+ // Before the grant, Bob's list does not mention it. Without this the

+ // test would pass on a list that shows everybody everything.

+ bob := signInCookies(t, app, "bob@example.com")

+ if rec := get(t, app, bob, "/"); strings.Contains(rec.Body.String(), "Minutes") {

+ t.Fatal("Bob's list named a document nobody had shared with him")

+ }

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST share: status %d, want 303", rec.Code)

+ }

+

+ rec := get(t, app, bob, "/")

+ if rec.Code != http.StatusOK {

+ t.Fatalf("GET / as Bob: status %d, want 200", rec.Code)

+ }

+ if !strings.Contains(rec.Body.String(), "Minutes") {

+ t.Error("Bob's list does not name the document shared with him")

+ }

+ if !strings.Contains(rec.Body.String(), "/d/"+sqid) {

+ t.Error("Bob's list names the document but does not link to it")

+ }

+

+ // Carol was granted nothing and sees nothing: the list widened by

+ // exactly one grant, not into a list of everything.

+ carol := signInCookies(t, app, "carol@example.com")

+ if rec := get(t, app, carol, "/"); strings.Contains(rec.Body.String(), "Minutes") {

+ t.Error("Carol's list named a document she holds no grant on")

+ }

+}

+

+// A shared row says it is shared, and the reader's own rows do not.

+//

+// The list stopped being "your documents" the day sharing landed. A row

+// that looks like yours and is not is how somebody opens a document

+// expecting to edit it — and with a reader grant they now get a page with

+// no editor, which is correct and needs explaining before the click, not

+// after it. (Branch review, finding 9.)

+func TestASharedRowSaysSo(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Alices")

+ bob := signInCookies(t, app, "bob@example.com")

+ createDoc(t, app, bob, "Bobs")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+

+ // The catalog KEY, not the English. NewForTest's app never wires

+ // rastrillo.Options.BaseCatalog (only cmd/docs/main.go does), so every

+ // catalog string renders as its own key here. Asserting the prose

+ // would be asserting a string that never appears — and, worse, a

+ // prose fragment can accidentally match a key: a nearby test checked

+ // for "published" and passed on the key "docs.share.public.published"

+ // rather than on anything being published.

+ const marker = "docs.list.shared"

+

+ body := get(t, app, bob, "/").Body.String()

+ if !strings.Contains(body, marker) {

+ t.Error("the shared row carries no marker")

+ }

+ // Exactly one row is marked: Bob's own must not be. A list where

+ // every row is badged says nothing.

+ if n := strings.Count(body, marker); n != 1 {

+ t.Errorf("%d rows marked shared, want exactly 1", n)

+ }

+

+ // And Alice, who owns hers, sees no marker at all.

+ if strings.Contains(get(t, app, alice, "/").Body.String(), marker) {

+ t.Error("the owner's own document is marked as shared with her")

+ }

+}

+

+// Revoking takes it back out of the list, not just out of the document. A

+// revoke that left the row on the grantee's list would leave them clicking

+// into a 404 and reading it as a bug.

+func TestRevokingRemovesItFromTheGranteesList(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ bob := signInCookies(t, app, "bob@example.com")

+

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+ if rec := get(t, app, bob, "/"); !strings.Contains(rec.Body.String(), "Minutes") {

+ t.Fatal("precondition: the shared document is not on Bob's list")

+ }

+

+ if rec := post(t, app, alice, "/d/"+sqid+"/share/revoke", "address=bob@example.com"); rec.Code != http.StatusSeeOther {

+ t.Fatalf("POST revoke: status %d, want 303", rec.Code)

+ }

+ if rec := get(t, app, bob, "/"); strings.Contains(rec.Body.String(), "Minutes") {

+ t.Error("a revoked document is still on Bob's list")

+ }

+}

+

+// Search is the list's other door. Slice 2's Search ran over List, so

+// widening only the list handler would have produced an app where a shared

+// document is on the list but cannot be searched for — the kind of split

+// that reads as a broken search rather than as a scope decision.

+func TestAGranteeCanSearchForASharedDocument(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Quarterly plan")

+ bob := signInCookies(t, app, "bob@example.com")

+

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader")

+

+ rec := get(t, app, bob, "/?q=quarterly")

+ if rec.Code != http.StatusOK {

+ t.Fatalf("GET /?q=quarterly as Bob: status %d, want 200", rec.Code)

+ }

+ if !strings.Contains(rec.Body.String(), "Quarterly plan") {

+ t.Error("Bob cannot search for the document shared with him")

+ }

+

+ carol := signInCookies(t, app, "carol@example.com")

+ if rec := get(t, app, carol, "/?q=quarterly"); strings.Contains(rec.Body.String(), "Quarterly plan") {

+ t.Error("Carol's search found a document she holds no grant on")

+ }

+}

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..5ef83f9 100644

--- a/internal/docs/store.go

+++ b/internal/docs/store.go

@@ -29,6 +29,20 @@ type Store struct {

// gate lives here rather than at the HTTP layer.

sampleDocuments bool

+ // seedOnlyMembers, when set, keeps the sample library away from a

+ // person admitted only by a grant.

+ //

+ // The sample documents are "welcome to your instance" content. Somebody

+ // invited to read one document is not arriving at their instance; they

+ // are arriving at somebody else's, to read one thing. Seeding them six

+ // documents of ours is the outcome 0005_grants.sql's comment says the

+ // design avoids — it just avoided it at grant time and then did it at

+ // sign-in time instead. (Branch review, finding 9.)

+ //

+ // nil means "seed everybody", which is what every construction without

+ // an allowlist gets, and matches sampleDocuments' own default shape.

+ seedOnlyMembers func(address string) bool

+

// logger is where a sample-seeding failure goes — see

// seedIfEnabled's own doc comment for why that failure is logged

// and swallowed here rather than ever returned to EnsurePerson's

@@ -242,7 +256,10 @@ func (s *Store) seedIfEnabled(ctx context.Context, newOwner int64, email string)

if !s.sampleDocuments {

return

}

- if err := Seed(ctx, s, newOwner); err != nil {

+ if s.seedOnlyMembers != nil && !s.seedOnlyMembers(email) {

+ return

+ }

+ 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)

}

@@ -693,7 +710,7 @@ func (s *Store) currentSeq(ctx context.Context, docID int64) (int64, error) {

// is to refuse the entire operation before any of them run. doc is a

// value a caller can construct or mutate freely (as the regression

// test below does); it is not a database guarantee. Store.Get is

-// owner-scoped, so doc.OwnerID is trustworthy *when doc came from

+// reader-scoped, so doc.OwnerID is trustworthy *when doc came from

// Get* — but nothing at this call site enforces that it did, which is

// the entire defect. So rather than trust the struct, this re-derives

// ownership from the database itself: look up document doc.ID through

@@ -718,12 +735,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 a 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.go b/internal/docs/stream.go

index 35ce48e..bd73b35 100644

--- a/internal/docs/stream.go

+++ b/internal/docs/stream.go

@@ -55,7 +55,7 @@ const (

// opaque binary Yjs updates on it would have meant base64 and about a

// third more bytes on every keystroke batch.

//

-// Owner-scoped through getDocumentForUpdates, so a stranger gets the

+// Reader-scoped through getDocumentForUpdates, so a stranger gets the

// same 404 the update routes give — never a 403, which would confirm

// the document exists.

func (h *handlers) stream(w http.ResponseWriter, r *http.Request) {

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..29562f2 100644

--- a/internal/docs/testing.go

+++ b/internal/docs/testing.go

@@ -2,6 +2,7 @@ package docs

import (

"context"

+ "database/sql"

"fmt"

"io"

"net/http"

@@ -11,6 +12,7 @@ import (

"path/filepath"

"strings"

"testing"

+ "time"

)

// NewForTest builds an App against a temporary SQLite database. It is

@@ -26,12 +28,34 @@ import (

// sign-in landed and was deleted in fix round 1 on Task 1 — every

// test built on NewForTest signs in for real via TestSignIn now.

func NewForTest(t *testing.T) (*App, error) {

+ return NewForTestWith(t, Config{})

+}

+

+// NewForTestWith is NewForTest with the caller's own Config, for the tests

+// that need a real allowlist.

+//

+// It exists because NewForTest fixes an empty AllowedAddresses, which in

+// development means "admit everyone" (resolveAllowlist) — so the whole

+// grant-admission path, and the guest/member distinction built on it, were

+// unreachable from any test in the suite. The branch review found exactly

+// that hole: allows() and HasGrant were each tested in isolation, and the

+// wiring that makes them one thing was not, so moving one line in app.go

+// would have silently recreated the unredeemable invitation its comment

+// warns about, green. (Branch review, finding 7e.)

+//

+// DBPath and Env are filled in when the caller leaves them empty, so a test

+// that only wants an allowlist writes only an allowlist.

+func NewForTestWith(t *testing.T, cfg Config) (*App, error) {

t.Helper()

- app, err := New(Config{

- DBPath: filepath.Join(t.TempDir(), "docs.db"),

- Env: "development",

- })

+ if cfg.DBPath == "" {

+ cfg.DBPath = filepath.Join(t.TempDir(), "docs.db")

+ }

+ if cfg.Env == "" {

+ cfg.Env = "development"

+ }

+

+ app, err := New(cfg)

if err != nil {

return nil, err

}

@@ -66,6 +90,12 @@ func (a *App) TestSignIn(t *testing.T, address string) (string, error) {

return "", fmt.Errorf("docs: TestSignIn requires Env %q; app was built with Env %q", "development", a.cfg.Env)

}

+ // Snapshot the send count before the POST: the check below has to see

+ // a NEW mail, not merely a recorder whose last occupant matches.

+ a.mailRecorder.mu.Lock()

+ before := a.mailRecorder.sends

+ a.mailRecorder.mu.Unlock()

+

form := url.Values{"address": {address}, "force": {"1"}}

req := httptest.NewRequest(http.MethodPost, "/signin", strings.NewReader(form.Encode()))

req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

@@ -88,9 +118,22 @@ func (a *App) TestSignIn(t *testing.T, address string) (string, error) {

}

a.mailRecorder.mu.Lock()

- body := a.mailRecorder.lastBody

+ to, body, after := a.mailRecorder.lastTo, a.mailRecorder.lastBody, a.mailRecorder.sends

a.mailRecorder.mu.Unlock()

+ // A refused address gets the same 303 ?sent=1 a real one gets

+ // (gateBegin, deliberately, so the endpoint is not a membership

+ // oracle). So the only evidence that this address was admitted is that

+ // a new mail went out, to it. Both halves are needed: the count alone

+ // would accept somebody else's mail, and the recipient alone stays

+ // true for every later refusal of an address that once succeeded.

+ if after == before {

+ return "", fmt.Errorf("docs: TestSignIn: no mail was sent for %q — the address was refused", address)

+ }

+ if normalizeAddress(to) != normalizeAddress(address) {

+ return "", fmt.Errorf("docs: TestSignIn: the mail went to %q, not %q", to, address)

+ }

+

link := magicLinkPattern.FindString(body)

if link == "" {

return "", fmt.Errorf("docs: TestSignIn: no magic link found in the mailed body for %q", address)

@@ -213,3 +256,103 @@ 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}

+}

+

+// TestEditorScriptPath is the served path of the editor bundle, exported so

+// an external test can assert a page does or does not ship it.

+//

+// It exists because the branch review found a test banning the string

+// "editor-entry" from the public page — a string that appears on no page in

+// this app, so the ban could never fail. A test that names the real path

+// cannot drift from what staticHandler actually serves, because it is the

+// same function that builds the <script src>.

+func TestEditorScriptPath() string { return editorScriptPath() }

+

+// TestSoftDelete marks a document deleted the way a delete verb would, for

+// tests that need a live link pointing at a document that is no longer

+// there. There is no delete route yet; this is the state the schema

+// already models (documents.deleted_at) and that ByToken filters on.

+func (a *App) TestSoftDelete(t *testing.T, sqid string) {

+ t.Helper()

+ err := a.store.db.Model(&Document{}).

+ Where("sqid = ?", sqid).

+ Update("deleted_at", time.Now().UTC()).Error

+ if err != nil {

+ t.Fatalf("docs: TestSoftDelete(%q): %v", sqid, err)

+ }

+}

+

+// TestWrite applies one update to a document as the signed-in owner,

+// through the same store path a real edit takes, so a test can publish a

+// document that actually has text in it.

+func (a *App) TestWrite(t *testing.T, cookies []*http.Cookie, sqid, fixture string) {

+ t.Helper()

+ var doc Document

+ if err := a.store.db.Where("sqid = ?", sqid).First(&doc).Error; err != nil {

+ t.Fatalf("docs: TestWrite: find %q: %v", sqid, err)

+ }

+ actor := a.TestActorFor(t, &doc)

+ if _, err := a.store.ApplyUpdate(context.Background(), &doc, actor, ReadFragmentFixture(t, fixture)); err != nil {

+ t.Fatalf("docs: TestWrite(%q): %v", fixture, err)

+ }

+}

+

+// TestRowContains reports whether any column of any row in table holds

+// needle as a substring.

+//

+// It exists for the one assertion that cannot be made against behaviour:

+// ruling 16 says the plaintext token is not in the database, and the only

+// honest way to check that is to look in the database. A test that asked

+// the store instead would be asking the code that is supposed to be wrong.

+func (a *App) TestRowContains(t *testing.T, table, needle string) bool {

+ t.Helper()

+ rows, err := a.store.db.Raw("SELECT * FROM " + table).Rows()

+ if err != nil {

+ t.Fatalf("docs: TestRowContains: %v", err)

+ }

+ defer rows.Close()

+

+ cols, err := rows.Columns()

+ if err != nil {

+ t.Fatalf("docs: TestRowContains: columns: %v", err)

+ }

+ for rows.Next() {

+ cells := make([]any, len(cols))

+ for i := range cells {

+ cells[i] = new(sql.RawBytes)

+ }

+ if err := rows.Scan(cells...); err != nil {

+ t.Fatalf("docs: TestRowContains: scan: %v", err)

+ }

+ for _, c := range cells {

+ if strings.Contains(string(*(c.(*sql.RawBytes))), needle) {

+ return true

+ }

+ return false

+}

diff --git a/internal/docs/views/list.html b/internal/docs/views/list.html

index 27806b2..7caaae0 100644

--- a/internal/docs/views/list.html

+++ b/internal/docs/views/list.html

@@ -311,10 +311,10 @@

placeholder="{{.SearchPlace}}" aria-label="{{.SearchLabel}}" autocomplete="off">

<button rst-btn="ghost" type="submit">{{.SearchLabel}}</button>

</form>

- <form docs-create-form method="post" action="/documents">

+{{if .CanCreate}} <form docs-create-form method="post" action="/documents">

<button rst-btn="primary" type="submit">{{.CreateLabel}}</button>

- </form>

+ </form>{{end}}

</div>

</header>

{{/* Searching renders a ranked, UNGROUPED result list. Recency

@@ -328,7 +328,7 @@

<span rst-row-main><a href="{{.Href}}">{{.Title}}</a></span>

- <span docs-row-when>{{.When}}</span>

+ <span docs-row-when>{{.When}}</span>{{if .SharedNote}}<span rst-muted docs-row-shared>{{.SharedNote}}</span>{{end}}

</div>

@@ -347,7 +347,7 @@

<span rst-row-main><a href="{{.Href}}">{{.Title}}</a></span>

- <span docs-row-when>{{.When}}</span>

+ <span docs-row-when>{{.When}}</span>{{if .SharedNote}}<span rst-muted docs-row-shared>{{.SharedNote}}</span>{{end}}

</div>

diff --git a/internal/docs/views/public.html b/internal/docs/views/public.html

new file mode 100644

index 0000000..c85172b

--- /dev/null

+++ b/internal/docs/views/public.html

@@ -0,0 +1,26 @@

+{{define "title"}}{{.Title}}{{end}}

+{{/* The published document: the one screen in this app a stranger sees.

+

+ No "account" block is defined, and that is deliberate rather than an

+ omission. tokens.css hides a disclosure whose menu is empty, so

+ leaving it undefined removes the account dropdown — and a signed-out

+ reader must not be offered a sign-out control for a session they do

+ not have. render.go's own comment on shellLayout records that this is

+ how a screen opts out of it.

+

+ No "nav" block either: the share control lives there on the private

+ page, and this reader owns nothing to share.

+

+ No editor. Not disabled, not hidden — absent. A read-only surface

+ built by hiding an editor is one CSS rule away from being an editor

+ again, and the updates route it would post to is behind the session

+ guard anyway, so the mount point would only produce a page that

+ fails at the network. Ruling 15's other half: every write has a

+ named actor, and whoever is reading this has no name. */}}

+{{define "brand"}}<a rst-shell-brand href="/">{{.Title}}</a>{{end}}

+{{define "content"}}

+<article rst-stack docs-public>

+ <p rst-notice docs-public-notice>{{.ReadOnly}}</p>

+ <div docs-public-body>{{.HTML}}</div>

+</article>

+{{end}}

diff --git a/internal/docs/views/share.html b/internal/docs/views/share.html

new file mode 100644

index 0000000..86f0545

--- /dev/null

+++ b/internal/docs/views/share.html

@@ -0,0 +1,117 @@

+{{define "title"}}{{.Title}} — {{.DocTitle}}{{end}}

+{{define "account"}}<form method="post" action="/signout"><button type="submit">{{.SignOutLabel}}</button></form>{{end}}

+

+{{/* The share screen. Scriptless throughout, per spec §6.2, which lists

+ sharing among the working form posts: two plain <form>s, no token

+ field (csrf.Protect reads Sec-Fetch-Site and falls back to Origin),

+ and a 303 back here afterwards so a reload never re-posts.

+

+ Bare attributes rather than the class vocabulary, per

+ TestOurTemplatesUseTheAttributeGrammar — which greps raw bytes, so

+ even naming that spelling in a comment fails it. This comment used

+ to, and did.

+

+ Each row's revoke is its own <form>, not a link: revoking is a

+ mutation and a GET that changes state is one a prefetcher can fire.

+ That is also why the address rides a hidden field rather than the

+ URL — the row already names it in text, and a URL is copied,

+ logged and pasted in a way a form body is not. */}}

+{{define "content"}}

+<article rst-stack>

+ <header rst-page-header>

+ <div rst-titles>

+ <h1>{{.Heading}}</h1>

+ <p rst-muted>{{.DocTitle}}</p>

+ </div>

+ <a href="/d/{{.Sqid}}">{{.BackLabel}}</a>

+ </header>

+

+ {{if .Error}}<p rst-notice role="alert">{{.Error}}</p>{{end}}

+

+ {{/* Who reaches it today. An empty list says so in words rather than

+ rendering an empty table, because a table with no rows reads as a

+ thing that failed to load. */}}

+ {{if .Grants}}

+ <ul rst-list docs-share-list>

+ {{range .Grants}}

+ <li rst-list-item>

+ <span docs-share-who>{{.Address}}</span>

+ <span rst-muted docs-share-role>{{.Role}}</span>

+ <form method="post" action="/d/{{$.Sqid}}/share/revoke">

+ <input type="hidden" name="address" value="{{.Address}}">

+ <button type="submit">{{$.RevokeButton}}</button>

+ </form>

+ </li>

+ {{end}}

+ </ul>

+ {{else}}

+ <p rst-muted>{{.EmptyBody}}</p>

+ {{end}}

+

+ {{/* Adding somebody. The role is a radio group rather than a select so

+ both options are visible without opening anything, and so the

+ keyboard contract is the one radios already have. Reader is

+ checked by default: the safer of the two is the one a person gets

+ by not choosing. */}}

+ <form method="post" action="/d/{{.Sqid}}/share" docs-share-form>

+ <label for="share-address">{{.AddressLabel}}</label>

+ <input id="share-address" name="address" type="email" autocomplete="email" required>

+

+ <fieldset>

+ <legend>{{.RoleLabel}}</legend>

+ <label><input type="radio" name="role" value="reader" checked> {{.RoleReader}}</label>

+ <label><input type="radio" name="role" value="writer"> {{.RoleWriter}}</label>

+ </fieldset>

+

+ <button type="submit">{{.ShareButton}}</button>

+ </form>

+

+ {{/* The third rung: a read-only link anyone can follow, signed in or

+ not. One <form> whichever way the switch currently points, with

+ the direction in a hidden field — so a page left open in a tab

+ cannot turn publishing back on from a button that still says

+ "off", which two separate routes would have allowed.

+

+ The URL is printed as text, not as a link. It is meant to be

+ copied into a message rather than followed from here, and a

+ page whose only affordance is a link invites the owner to click

+ it, land on their own published copy, and wonder where the app

+ went. Reading it as text also makes it obvious that this is a

+ thing you hand to someone.

+

+ No copy button: that needs JavaScript, and this screen has a

+ zero-JS floor (spec §6.2). Select-and-copy works everywhere,

+ today. */}}

+ <section docs-share-public>

+ <h2>{{.PublicHeading}}</h2>

+ <p rst-muted>{{.PublicBody}}</p>

+

+ {{if .Published}}

+ {{/* The URL appears here only in the render that follows the publish

+ that minted it — the database holds a SHA-256 and cannot produce

+ a URL (suite ruling 16). Afterwards this section says the

+ document is published and offers to replace or stop it. */}}

+ {{if .PublicURL}}

+ <p rst-notice docs-share-once role="status">{{.OnceWarning}}</p>

+ <p docs-share-url><code>{{.PublicURL}}</code></p>

+ {{else}}

+ <p rst-muted docs-share-state>{{.PublishedBody}}</p>

+ {{end}}

+ <p rst-muted>{{.CopyHint}}</p>

+ <form method="post" action="/d/{{.Sqid}}/share/link">

+ <input type="hidden" name="on" value="0">

+ <button type="submit">{{.UnpublishBtn}}</button>

+ </form>

+ <form method="post" action="/d/{{.Sqid}}/share/link">

+ <input type="hidden" name="on" value="1">

+ <button type="submit">{{.ReplaceButton}}</button>

+ </form>

+ {{else}}

+ <form method="post" action="/d/{{.Sqid}}/share/link">

+ <input type="hidden" name="on" value="1">

+ <button type="submit">{{.PublishButton}}</button>

+ </form>

+ {{end}}

+ </section>

+</article>

+{{end}}

diff --git a/internal/docs/views/show.html b/internal/docs/views/show.html

index 3f4b6c0..b0735bd 100644

--- a/internal/docs/views/show.html

+++ b/internal/docs/views/show.html

@@ -66,6 +66,19 @@

full-width menu row, so it needs no rst-btn of its own — which

is the point of putting it here instead of leaving it as a ghost

button in a page header. */}}

+{{/* nav is the shell's slot inside [rst-shell-tail], rendered

+ immediately before the account disclosure — so this link lands

+ beside the account dropdown at the top right, which is where a

+ share control belongs and where Paul asked for one.

+

+ A link, not a form: opening the share page is a read, and GET is

+ what a prefetcher, a middle-click and a bookmark all expect. The

+ mutations live on that page, each in its own POST.

+

+ Its text rather than an icon because the design system has no

+ share glyph — see showPage.CanShare's comment for why inventing

+ one here would have shipped an invisible control. */}}

+{{define "nav"}}{{if .CanShare}}<a href="/d/{{.Sqid}}/share">{{.ShareLabel}}</a>{{end}}{{end}}

{{define "account"}}<form method="post" action="/signout"><button type="submit">{{.SignOutLabel}}</button></form>{{end}}

{{define "content"}}<style>

/* This screen is the one that is not a column of content. It is a

@@ -1311,7 +1324,7 @@

controls, so it was the one place a person would not look. They are

still ordinary links to ordinary URLs, so both still work with no

JavaScript at all — spec §6.2 — and both are GETs of a document the

- signed-in person already owns, so neither needs a form or a CSRF

+ signed-in person may already read, so neither needs a form or a CSRF

token. download tells the browser to save rather than navigate; the

filename it uses comes from the server's Content-Disposition, not

from this attribute, because a title is arbitrary text a person

@@ -1525,7 +1538,13 @@

TestShowRendersContentWithoutScript pins it. data-sqid is the

bundle's only way to learn which document's update log

(/d/{{.Sqid}}/updates) to load and post to. */}}

- <div docs-text docs-editor data-sqid="{{.Sqid}}">{{.HTML}}</div>

+ {{/* A reader gets the same container without the mount point, the

+ data-sqid or (below) the script — absent, not disabled, because a

+ disabled editor is one CSS rule from being an editor again. The

+ server-rendered HTML is identical either way, so this is the

+ scriptless read view the whole page already falls back to. */}}

+ {{if .CanWrite}}<div docs-text docs-editor data-sqid="{{.Sqid}}">{{.HTML}}</div>

+ {{else}}<div docs-text>{{.HTML}}</div>{{end}}

</div>

{{/* [docs-save-status] (fix round 1, F2): a sibling of [docs-editor],

@@ -1590,6 +1609,6 @@

RequireSession group) and, if it throws, editor-entry.js's own catch

marks [docs-editor] data-editor-failed rather than clearing it — the

read view above stays exactly as rendered. */}}

-<script type="module" defer src="{{.EditorScript}}"></script>

+{{if .CanWrite}}<script type="module" defer src="{{.EditorScript}}"></script>{{end}}

diff --git a/internal/docs/writer_http_test.go b/internal/docs/writer_http_test.go

new file mode 100644

index 0000000..6658ec4

--- /dev/null

+++ b/internal/docs/writer_http_test.go

@@ -0,0 +1,61 @@

+package docs_test

+

+import (

+ "bytes"

+ "net/http"

+ "net/http/httptest"

+ "strings"

+ "testing"

+

+ "github.com/carlosframework/docs/internal/docs"

+)

+

+// A writer grantee can actually write, over HTTP, and the owner sees it.

+//

+// The branch tested the writer's authorization at the store

+// (grants_write_test.go asserts "not ErrNotWritable" for a one-byte

+// update) and the reader's refusal at the route, and never the writer's

+// success at the route. So if updatesPost regressed to owner-only Get,

+// every existing test would still pass while collaborative editing — the

+// point of a writer grant — was silently dead. (Branch review, finding

+// 7d.)

+func TestAWriterCanPostUpdatesOverHTTP(t *testing.T) {

+ app, _ := docs.NewForTest(t)

+ alice := signInCookies(t, app, "alice@example.com")

+ sqid := createDoc(t, app, alice, "Minutes")

+ bob := signInCookies(t, app, "bob@example.com")

+ post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer")

+

+ rec := postBytes(t, app, bob, "/d/"+sqid+"/updates", docs.ReadFragmentFixture(t, "hello"))

+ if rec.Code != http.StatusNoContent {

+ t.Fatalf("writer POST /updates: status %d, want 204", rec.Code)

+ }

+

+ // The owner sees what the writer wrote. Without this the test passes

+ // on a 204 that dropped the update on the floor.

+ if body := get(t, app, alice, "/d/"+sqid).Body.String(); !strings.Contains(body, "hello") {

+ t.Error("the owner's page does not show the writer's text")

+ }

+

+ // And a grantee can read the update log, which is how the editor

+ // catches up — also untested before.

+ logRec := get(t, app, bob, "/d/"+sqid+"/updates")

+ if logRec.Code != http.StatusOK || logRec.Body.Len() == 0 {

+ t.Errorf("writer GET /updates: status %d, %d bytes", logRec.Code, logRec.Body.Len())

+ }

+}

+

+// postBytes is post for a binary body: the update routes take raw CRDT

+// bytes, not a form.

+func postBytes(t *testing.T, app *docs.App, cookies []*http.Cookie, path string, body []byte) *httptest.ResponseRecorder {

+ t.Helper()

+ req := httptest.NewRequest(http.MethodPost, path, bytes.NewReader(body))

+ req.Header.Set("Content-Type", "application/octet-stream")

+ req.Header.Set("Origin", docs.DefaultOrigin)

+ for _, c := range cookies {

+ req.AddCookie(c)

+ }

+ rec := httptest.NewRecorder()

+ app.Handler().ServeHTTP(rec, req)

+ return rec

+}

oficina / docs Public

Merge branch 'sharing' into main