amadan

oficina / docs Public

A grant admits its holder, or the invitation can never be redeemed

$DOCS_ALLOWED_ADDRESSES gates who may receive a magic link at all, and
gateBegin answers identically for a refused address on purpose, so the
endpoint is not a membership oracle. Sharing a document to somebody outside
that list therefore produced a grant nobody could redeem: 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, because it
looks like it worked.

So a grant widens admission. One clause in allows(), deliberately: sharing
changing who may sign in to an instance is a real decision about its front
door, and reversing it should be deleting two lines rather than unpicking a
design. It only ever widens — an addrs == nil allowlist has already returned
true before the clause is reached — and it admits the grant's holder and
nobody else.

HasGrant answers on the address alone, never on which document, because
admission is not a per-document question. A database error answers false: an
error refuses admission rather than granting it, and the address still gets
in if it is on the configured list.

Wired in New rather than inside resolveAllowlist. The allowlist is resolved
before the store exists, deliberately, so a misconfigured allowlist refuses
to boot before anything else is built — that ordering is worth more than the
convenience of putting this where the allowlist is made.

Full suite green, gofmt and vet clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 76d9b493f4e27c9b24850994f3cde4c3874064b1 parent 1ef53df
4 files changed, +142 −2
  • internal/docs/app.go +9 −0
  • internal/docs/auth.go +25 −2
  • internal/docs/grants.go +23 −0
  • internal/docs/grants_admission_test.go +85 −0

CI passed — run details

Intent

full ledger: sharing

diff --git a/internal/docs/app.go b/internal/docs/app.go
index 581c7aa..b576fc3 100644
--- a/internal/docs/app.go
+++ b/internal/docs/app.go
@@ -380,6 +380,15 @@ 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.
+ list.granted = func(address string) bool {
+ return store.HasGrant(context.Background(), address)
+ }
+
render, err := newRenderer()
if err != nil {
rt.Close(context.Background())
diff --git a/internal/docs/auth.go b/internal/docs/auth.go
index 7fff498..0c5c744 100644
--- a/internal/docs/auth.go
+++ b/internal/docs/auth.go
@@ -148,6 +148,24 @@ 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
}
// allows reports whether address may sign in.
@@ -155,8 +173,13 @@ func (a allowlist) allows(address string) bool {
if a.addrs == nil {
return true
}
- _, ok := a.addrs[normalizeAddress(address)]
- return ok
+ normalized := normalizeAddress(address)
+ if _, ok := a.addrs[normalized]; ok {
+ return true
+ }
+ // A grant admits its holder, and nobody else. This only ever widens: an
+ // addrs == nil allowlist has already returned true above.
+ return a.granted != nil && a.granted(normalized)
}
// resolveAllowlist applies Config.AllowedAddresses' own fail-closed
diff --git a/internal/docs/grants.go b/internal/docs/grants.go
index f7e254c..ee5cd82 100644
--- a/internal/docs/grants.go
+++ b/internal/docs/grants.go
@@ -140,3 +140,26 @@ func (s *Store) Grant(ctx context.Context, owner int64, sqid, address string, ro
DoUpdates: clause.AssignmentColumns([]string{"role"}),
}).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 answers false: refusing admission on an error is the safe
+// direction, and the address still gets in if it is on the configured list.
+func (s *Store) HasGrant(ctx context.Context, address string) bool {
+ subject := subjectOf(address)
+ if subject == "" {
+ return false
+ }
+ var count int64
+ if err := s.db.WithContext(ctx).Model(&Grant{}).
+ Where("subject = ?", subject).Count(&count).Error; err != nil {
+ s.logger.Error("docs: checking grants for admission", "err", err)
+ return false
+ }
+ return count > 0
+}
diff --git a/internal/docs/grants_admission_test.go b/internal/docs/grants_admission_test.go
new file mode 100644
index 0000000..5f12126
--- /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.allows("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 { return granted[address] }
+
+ if !list.allows("bob@example.com") {
+ t.Error("bob holds a grant and is still refused admission — his invitation can never be redeemed")
+ }
+ if list.allows("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 { return address == "bob@example.com" }
+
+ if !list.allows(" 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 { return false }
+
+ if !list.allows("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 store.HasGrant(ctx, "bob@example.com") {
+ t.Fatal("bob holds a grant before one was made")
+ }
+ if err := store.Grant(ctx, alice, doc.Sqid, "bob@example.com", RoleReader); err != nil {
+ t.Fatalf("Grant: %v", err)
+ }
+ if !store.HasGrant(ctx, "bob@example.com") {
+ t.Error("bob holds a grant and HasGrant says otherwise")
+ }
+}