idear: the roster store, every invariant in its own transaction (Task 3)
Config/New plus the twelve store operations, a shared test harness, and
the four required race tests. Every mutation is one transaction, and
every guard runs against rows RE-READ inside that transaction: the
*Member a caller passes in is a claim about the past, and a concurrent
writer is under no obligation to honour it.
Claim zero ROWS, not zero ACTIVE rows — a fully deactivated
roster must not reopen the claim and hand a stranger Owner
of an instance full of dormant data
Invite refuses RoleOwner outright on every path for every actor,
and refuses granting a role that does not rank strictly
below the actor's (an Admin minting a peer Admin has
escalated past MayActOn's equal-rank refusal)
Accept consumes by CAS with rows-affected checked, in the same
transaction as the Member insert; the row is read back
only after the CAS has claimed it
SetRole refuses RoleOwner, and refuses an Owner target
Deactivate never removes the Owner
Transfer re-reads both rows, confirms the actor is still Owner and
the target is still ACTIVE, then demotes and promotes with
two conditional rows-affected-checked updates
ErrLastOwner is checked after an authority floor but before the full
MayActOn matrix. After MayActOn it would be unreachable (nobody
outranks an Owner); before any authority test a plain Member probing
Deactivate would learn which row is the Owner.
The four races use real goroutines off a start barrier. The first
version of the two paired ones was green for the wrong reason: closing
a channel readies waiters FIFO and the last readied lands in runnext,
so Revoke won 58-60 of every 60 rounds. pair() flips the spawn order on
alternate iterations, and both tests now fail if either outcome never
occurs.
Two things need review, both written up in the task report:
migrations/0001_init.sql changes TEXT timestamps to DATETIME, and the
frozen checksum is re-recorded once. modernc.org/sqlite decodes a text
timestamp into a time.Time only when the column's DECLARED type is
DATE/DATETIME/TIMESTAMP, so against TEXT every GORM read of a Member or
Invitation failed with "unsupported Scan ... into type *time.Time" —
the tables could be written and never read. DATETIME is what
`rastrillo migration generate` itself emits for a GORM time.Time. The
never-update rule protects ledgers that hold the old checksum; today no
ledger anywhere holds this migration. The constant's comment records
the re-recording and restates that it is the only one.
ErrInvalidRole is added to the sentinels: a handler renders a malformed
role 400 and an authority refusal 403, and folding them together loses
that. Refusing RoleOwner is deliberately not this error.
Verified: gofmt clean, go vet clean, go test -count=1 green x5, and
CGO_ENABLED=1 -race green x3. A mutation run confirms each invariant
test goes red for its own regression; make ci now runs the race target
so -race is part of the gate rather than something someone remembers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
11 files changed,
+2258
−22
.amadan/ci.d/40-race+2 −0Makefile+20 −7errors.go+49 −0go.mod+4 −2internal/ideartest/harness.go+202 −0migrations/0001_init.sql+7 −7policy.go+26 −5race_test.go+374 −0roster.go+716 −0roster_test.go+838 −0schema_test.go+20 −1
diff --git a/.amadan/ci.d/40-race b/.amadan/ci.d/40-race| new file mode 100755 |
| index 0000000..99f4109 |
| --- /dev/null |
| +++ b/.amadan/ci.d/40-race |
| @@ -0,0 +1,2 @@ |
| +#!/bin/sh |
| +exec make race |
diff --git a/Makefile b/Makefile| index 88a7009..4a6fedd 100644 |
| --- a/Makefile |
| +++ b/Makefile |
| @@ -1,12 +1,11 @@ |
| -.PHONY: vet fmt-check test ci |
| +.PHONY: vet fmt-check test race ci |
| # ci is the one gate: what a runner executes and what you run before |
| # pushing are the same definition (amadan's own rule — CI steps delegate |
| -# to make targets, never keep their own copies of the commands). Task 1 |
| -# is pure Go with no database and no HTTP, so vet + fmt-check + test is |
| -# the whole gate; later tasks add migration-check the way the scaffold's |
| -# own Makefile does, once there is a schema to check. |
| -ci: vet fmt-check test |
| +# to make targets, never keep their own copies of the commands). Later |
| +# tasks add migration-check the way the scaffold's own Makefile does, |
| +# once there is an app schema to check. |
| +ci: vet fmt-check test race |
| vet: |
| go vet ./... |
| @@ -14,5 +13,19 @@ vet: |
| fmt-check: |
| @out=$$(gofmt -l .); if [ -n "$$out" ]; then echo "gofmt needed:"; echo "$$out"; exit 1; fi |
| +# -count=1 on purpose: these tests build a real SQLite database and |
| +# race real goroutines against it, and a cached PASS re-reports one |
| +# scheduling of a race as though it were every scheduling. |
| test: |
| - go test ./... |
| + go test ./... -count=1 |
| + |
| +# race is a separate target because it needs a different toolchain |
| +# setting, not because it is optional. The store's invariants are |
| +# transaction invariants and race_test.go drives them with real |
| +# goroutines; -race is what makes those tests able to report a data |
| +# race in the store itself rather than only a broken outcome. -race |
| +# requires cgo, and everything else in this Makefile runs with |
| +# CGO_ENABLED=0, so this target sets it for its own command and no |
| +# other. |
| +race: |
| + CGO_ENABLED=1 go test ./... -race -count=1 |
diff --git a/errors.go b/errors.go| new file mode 100644 |
| index 0000000..c4feca6 |
| --- /dev/null |
| +++ b/errors.go |
| @@ -0,0 +1,49 @@ |
| +package idear |
| + |
| +import "errors" |
| + |
| +// The store's sentinels. Every Roster method that refuses returns one |
| +// of these (or an ErrForbidden from MayActOn — see policy.go), wrapped |
| +// with context where a reason helps a log line. Callers test with |
| +// errors.Is, never by comparing strings. |
| +var ( |
| + // ErrOwnerExists is Claim's refusal: this instance already has a |
| + // roster, so the claim is closed. It is what the losers of a |
| + // concurrent first-signup race receive — see Claim, and the |
| + // reconciliation path in the design spec §5, which is how such a |
| + // loser is healed rather than stranded. |
| + ErrOwnerExists = errors.New("idear: this instance already has an owner") |
| + |
| + // ErrNoInvitation is the single answer to every unusable |
| + // invitation: no such token, already accepted, revoked, or |
| + // expired. It is deliberately one error and not four — the caller |
| + // holding a token is not entitled to learn WHICH of those is true, |
| + // and a handler that rendered the distinction would turn the |
| + // public GET /invitations/{token} route into an oracle. |
| + ErrNoInvitation = errors.New("idear: no valid invitation for that address") |
| + |
| + // ErrNotFound is a lookup miss: no member with that id or subject. |
| + // Handlers answer it with the app's own 404 renderer, never with a |
| + // message that distinguishes it from a permission refusal. |
| + ErrNotFound = errors.New("idear: no such member") |
| + |
| + // ErrLastOwner refuses any change that would leave the instance |
| + // without exactly one active Owner: deactivating the Owner, or |
| + // changing the Owner's role by any route other than Transfer. |
| + // |
| + // It is checked BEFORE the full MayActOn matrix, and deliberately: |
| + // "the owner cannot be removed" is true for every actor at every |
| + // rank, so returning ErrForbidden instead would tell an Admin that |
| + // some higher-ranked actor could do this — which is false. It is |
| + // still a refusal: handlers render it at 403 alongside |
| + // ErrForbidden, not as a bad request. |
| + ErrLastOwner = errors.New("idear: the owner cannot be removed") |
| + |
| + // ErrInvalidRole is a role argument that is not one of the three |
| + // known roles. It is a malformed argument, not a refusal of |
| + // authority — a handler renders it 400 where it renders |
| + // ErrForbidden 403. Refusing RoleOwner is NOT this error: owner is |
| + // a perfectly valid role, and Invite and SetRole refuse it with |
| + // ErrForbidden because ownership moves only by Transfer. |
| + ErrInvalidRole = errors.New("idear: not a role") |
| +) |
diff --git a/go.mod b/go.mod| index c360f41..8f9b00f 100644 |
| --- a/go.mod |
| +++ b/go.mod |
| @@ -2,7 +2,10 @@ module amadan.net/rastrillo/idear |
| go 1.25.0 |
| -require github.com/carlosframework/rastrillo v0.18.1-0.20260823225238-7439afc0d687 |
| +require ( |
| + github.com/carlosframework/rastrillo v0.18.1-0.20260823225238-7439afc0d687 |
| + gorm.io/gorm v1.31.2 |
| +) |
| require ( |
| github.com/dustin/go-humanize v1.0.1 // indirect |
| @@ -15,7 +18,6 @@ require ( |
| github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect |
| golang.org/x/sys v0.46.0 // indirect |
| golang.org/x/text v0.20.0 // indirect |
| - gorm.io/gorm v1.31.2 // indirect |
| gorm.io/plugin/dbresolver v1.6.2 // indirect |
| modernc.org/libc v1.74.1 // indirect |
| modernc.org/mathutil v1.7.1 // indirect |
diff --git a/internal/ideartest/harness.go b/internal/ideartest/harness.go| new file mode 100644 |
| index 0000000..0021a00 |
| --- /dev/null |
| +++ b/internal/ideartest/harness.go |
| @@ -0,0 +1,202 @@ |
| +// Package ideartest builds a real idear roster over a real SQLite |
| +// file, for idear's own tests and its example app's. |
| +// |
| +// Real, not a stand-in: the invariants this module exists to hold are |
| +// transaction invariants, and a fake store cannot be raced. The |
| +// database is rastrillo/db's split pool — one writer connection, |
| +// several readers — because that pool's routing is the environment the |
| +// store is written against, and the traps in it (a statement issued |
| +// outside a transaction that thinks it is inside one hangs rather than |
| +// erroring) only reproduce on the real thing. |
| +package ideartest |
| + |
| +import ( |
| + "context" |
| + "fmt" |
| + "path/filepath" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/rastrillo/db" |
| + "github.com/carlosframework/rastrillo/migrate" |
| + "github.com/carlosframework/rastrillo/sessions" |
| + |
| + "amadan.net/rastrillo/idear" |
| +) |
| + |
| +// Harness is one instance's world: a database, its migrations, and the |
| +// Roster over them. Close is registered with t.Cleanup, so a test just |
| +// calls New and forgets about it. |
| +// |
| +// Every helper on it reports failure with t.Fatalf, so every helper |
| +// must be called from the TEST goroutine. The race tests below drive |
| +// h.Roster directly from their worker goroutines and collect errors to |
| +// assert on afterwards; that is not a stylistic choice, it is what |
| +// testing.T's contract requires. |
| +type Harness struct { |
| + T *testing.T |
| + DB *db.DB |
| + Roster *idear.Roster |
| + |
| + seq int // makes seeded subjects and addresses unique per harness |
| +} |
| + |
| +// New returns a Harness over a fresh temp database with idear's |
| +// default Config. |
| +func New(t *testing.T) *Harness { |
| + t.Helper() |
| + return NewWith(t, idear.Config{}) |
| +} |
| + |
| +// NewWith is New with the caller's Config — OpenSignUp, a short |
| +// InviteTTL, a custom NotFound. cfg.DB is filled in by the harness and |
| +// anything the caller put there is ignored: the point of the harness |
| +// is that the database is the harness's. |
| +func NewWith(t *testing.T, cfg idear.Config) *Harness { |
| + t.Helper() |
| + |
| + d, err := db.Open(filepath.Join(t.TempDir(), "idear.db"), nil) |
| + if err != nil { |
| + t.Fatalf("db.Open: %v", err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + |
| + // The documented BootSchema order: sessions first, then idear. |
| + // Merged, never folded into an app's own Schema — see idear.Schema. |
| + if _, err := migrate.Apply(context.Background(), d, migrate.Merge(sessions.Schema, idear.Schema)); err != nil { |
| + t.Fatalf("migrate.Apply: %v", err) |
| + } |
| + |
| + cfg.DB = d.G |
| + rs, err := idear.New(cfg) |
| + if err != nil { |
| + t.Fatalf("idear.New: %v", err) |
| + } |
| + return &Harness{T: t, DB: d, Roster: rs} |
| +} |
| + |
| +// Ctx is the context every harness helper and most tests use. |
| +func (h *Harness) Ctx() context.Context { return context.Background() } |
| + |
| +// Member seeds one member at role, straight into the table. |
| +// |
| +// It goes around the store on purpose. The store will not mint an |
| +// Owner except by Claim, nor an Admin except by Invite-then-Accept, |
| +// and a test of Deactivate should not have to perform an invitation |
| +// flow to reach its starting position. Seeding is the arrangement; |
| +// the store is what is under test. |
| +func (h *Harness) Member(role idear.Role) *idear.Member { |
| + h.T.Helper() |
| + h.seq++ |
| + n := h.seq |
| + return h.MemberAs( |
| + fmt.Sprintf("subject-%d", n), |
| + fmt.Sprintf("member-%d@example.test", n), |
| + fmt.Sprintf("Member %d", n), |
| + role, |
| + ) |
| +} |
| + |
| +// MemberAs seeds a member with a caller-chosen subject, address and |
| +// name — for tests that care what those are. |
| +func (h *Harness) MemberAs(subject, email, name string, role idear.Role) *idear.Member { |
| + h.T.Helper() |
| + m := &idear.Member{Subject: subject, Email: email, Name: name, Role: role} |
| + if err := h.DB.G.Create(m).Error; err != nil { |
| + h.T.Fatalf("seeding %s %q: %v", role, subject, err) |
| + } |
| + return m |
| +} |
| + |
| +// Owner seeds the instance's Owner. Most tests want one of these and |
| +// then some victims. |
| +func (h *Harness) Owner() *idear.Member { return h.Member(idear.RoleOwner) } |
| + |
| +// Deactivated seeds a member at role who is already deactivated. |
| +func (h *Harness) Deactivated(role idear.Role) *idear.Member { |
| + h.T.Helper() |
| + m := h.Member(role) |
| + now := time.Now().UTC() |
| + if err := h.DB.G.Model(&idear.Member{}).Where("id = ?", m.ID). |
| + Update("deactivated_at", now).Error; err != nil { |
| + h.T.Fatalf("deactivating seeded member %d: %v", m.ID, err) |
| + } |
| + m.DeactivatedAt = &now |
| + return m |
| +} |
| + |
| +// Reload re-reads a member by id — the only honest way to assert what |
| +// a mutation did, since the *Member a test is holding was read before |
| +// the call. |
| +func (h *Harness) Reload(id int64) *idear.Member { |
| + h.T.Helper() |
| + var m idear.Member |
| + if err := h.DB.G.Where("id = ?", id).Take(&m).Error; err != nil { |
| + h.T.Fatalf("reloading member %d: %v", id, err) |
| + } |
| + return &m |
| +} |
| + |
| +// Invitation re-reads an invitation by id, spent ones included — |
| +// PendingInvitations deliberately hides those, and a test of Revoke or |
| +// Accept needs to see them. |
| +func (h *Harness) Invitation(id int64) *idear.Invitation { |
| + h.T.Helper() |
| + var inv idear.Invitation |
| + if err := h.DB.G.Where("id = ?", id).Take(&inv).Error; err != nil { |
| + h.T.Fatalf("reloading invitation %d: %v", id, err) |
| + } |
| + return &inv |
| +} |
| + |
| +// Expire backdates an invitation's ExpiresAt so it is already dead. |
| +// |
| +// It writes a time.Time through the same driver the store writes |
| +// through, deliberately: the expiry comparison in Accept's CAS is a |
| +// text comparison over the driver's own timestamp format, and a test |
| +// that seeded the column with a hand-written string would be testing a |
| +// format the store never produces. |
| +func (h *Harness) Expire(invitationID int64) { |
| + h.T.Helper() |
| + past := time.Now().UTC().Add(-time.Hour) |
| + if err := h.DB.G.Model(&idear.Invitation{}).Where("id = ?", invitationID). |
| + Update("expires_at", past).Error; err != nil { |
| + h.T.Fatalf("expiring invitation %d: %v", invitationID, err) |
| + } |
| +} |
| + |
| +// CountMembers is every row in the roster, deactivated included — |
| +// which is the count Claim's "zero rows, not zero active rows" rule is |
| +// about. |
| +func (h *Harness) CountMembers() int64 { |
| + h.T.Helper() |
| + var n int64 |
| + if err := h.DB.G.Model(&idear.Member{}).Count(&n).Error; err != nil { |
| + h.T.Fatalf("counting members: %v", err) |
| + } |
| + return n |
| +} |
| + |
| +// Owners is every row at RoleOwner. The single-owner invariant is |
| +// asserted against this: exactly one, and active. |
| +func (h *Harness) Owners() []idear.Member { |
| + h.T.Helper() |
| + var out []idear.Member |
| + if err := h.DB.G.Where("role = ?", idear.RoleOwner).Order("id").Find(&out).Error; err != nil { |
| + h.T.Fatalf("listing owners: %v", err) |
| + } |
| + return out |
| +} |
| + |
| +// TheOwner asserts that exactly one Owner row exists and returns it. |
| +// Every race test ends with this call: "exactly one owner" is the |
| +// invariant, and a test that only checked the count of successes would |
| +// not notice a transaction that left two. |
| +func (h *Harness) TheOwner() *idear.Member { |
| + h.T.Helper() |
| + owners := h.Owners() |
| + if len(owners) != 1 { |
| + h.T.Fatalf("roster has %d owners, want exactly 1: %+v", len(owners), owners) |
| + } |
| + return &owners[0] |
| +} |
diff --git a/migrations/0001_init.sql b/migrations/0001_init.sql| index 87c25cb..a6338e6 100644 |
| --- a/migrations/0001_init.sql |
| +++ b/migrations/0001_init.sql |
| @@ -4,9 +4,9 @@ CREATE TABLE IF NOT EXISTS idear_members ( |
| email TEXT, |
| name TEXT, |
| role TEXT NOT NULL, |
| - deactivated_at TEXT, |
| - created_at TEXT, |
| - updated_at TEXT |
| + deactivated_at DATETIME, |
| + created_at DATETIME, |
| + updated_at DATETIME |
| ); |
| CREATE UNIQUE INDEX IF NOT EXISTS idx_idear_members_subject ON idear_members (subject); |
| @@ -19,10 +19,10 @@ CREATE TABLE IF NOT EXISTS idear_invitations ( |
| role TEXT NOT NULL, |
| token_hash TEXT, |
| invited_by INTEGER, |
| - created_at TEXT, |
| - expires_at TEXT, |
| - accepted_at TEXT, |
| - revoked_at TEXT |
| + created_at DATETIME, |
| + expires_at DATETIME, |
| + accepted_at DATETIME, |
| + revoked_at DATETIME |
| ); |
| CREATE UNIQUE INDEX IF NOT EXISTS idx_idear_invitations_token_hash ON idear_invitations (token_hash); |
diff --git a/policy.go b/policy.go| index d77ad60..657ab16 100644 |
| --- a/policy.go |
| +++ b/policy.go |
| @@ -30,11 +30,8 @@ var ErrForbidden = errors.New("idear: forbidden") |
| // rank manages an Owner. Equal rank is refused, not just higher |
| // rank: two Owners or two Admins may never act on one another. |
| func MayActOn(actor, target *Member) error { |
| - if actor == nil || !actor.Active() { |
| - return fmt.Errorf("%w: actor is not an active member", ErrForbidden) |
| - } |
| - if !actor.Role.AtLeast(RoleAdmin) { |
| - return fmt.Errorf("%w: actor must be at least admin", ErrForbidden) |
| + if err := mayManage(actor); err != nil { |
| + return err |
| } |
| if target == nil { |
| return fmt.Errorf("%w: no target", ErrForbidden) |
| @@ -47,3 +44,27 @@ func MayActOn(actor, target *Member) error { |
| } |
| return nil |
| } |
| + |
| +// mayManage is rules 1 and 2 of MayActOn on their own: the authority |
| +// FLOOR, with no target in it. It answers "is this actor allowed to |
| +// manage anything at all" — which is the question Invite and Revoke |
| +// ask (they have no target Member), and the question the store's other |
| +// mutations must ask FIRST, before any invariant check that names the |
| +// target. |
| +// |
| +// That ordering is what keeps ErrLastOwner from becoming a membership |
| +// oracle. Deactivate and SetRole refuse an Owner target with |
| +// ErrLastOwner rather than ErrForbidden, because "the owner cannot be |
| +// removed" is true for every actor and ErrForbidden would imply some |
| +// higher rank could do it. But a plain Member must still be refused |
| +// for lacking authority, not told which row is the Owner's — so the |
| +// floor is checked first and the target-shaped invariant second. |
| +func mayManage(actor *Member) error { |
| + if actor == nil || !actor.Active() { |
| + return fmt.Errorf("%w: actor is not an active member", ErrForbidden) |
| + } |
| + if !actor.Role.AtLeast(RoleAdmin) { |
| + return fmt.Errorf("%w: actor must be at least admin", ErrForbidden) |
| + } |
| + return nil |
| +} |
diff --git a/race_test.go b/race_test.go| new file mode 100644 |
| index 0000000..2e55a96 |
| --- /dev/null |
| +++ b/race_test.go |
| @@ -0,0 +1,374 @@ |
| +package idear_test |
| + |
| +import ( |
| + "errors" |
| + "fmt" |
| + "sync" |
| + "testing" |
| + |
| + "amadan.net/rastrillo/idear" |
| + "amadan.net/rastrillo/idear/internal/ideartest" |
| +) |
| + |
| +// These four are the point of the store, and they are written as |
| +// ACTUAL races: goroutines released together off a start barrier, not |
| +// sequential calls arranged to look concurrent. Round 1 of the CARLOS |
| +// bake-off found a real ownership-transfer defect in a hand-rolled |
| +// membership layer, and found it only because someone ran it as a |
| +// race; a sequential rehearsal of the same calls passes over the bug, |
| +// because the bug lives in the window between a read and a write and a |
| +// sequential test never opens that window. |
| +// |
| +// Every worker below drives h.Roster directly and returns its error |
| +// through a slice. None of them touch testing.T, and none call a |
| +// harness helper: those report with t.Fatalf, which is only legal on |
| +// the test goroutine. |
| +// |
| +// A note on what these can and cannot prove. rastrillo/db's writer |
| +// pool holds exactly ONE connection, so write transactions queue in |
| +// database/sql rather than collide in SQLite — which is precisely why |
| +// none of these need a retry loop or a sleep, and why seeing |
| +// "database is locked" here would be a bug in the store (a |
| +// transaction held open across something that is not a database |
| +// operation) and never something to paper over. What the single writer |
| +// does NOT do is serialise a read-then-write pair that spans two |
| +// transactions. That gap is the whole attack surface, and it is what |
| +// these tests aim at. |
| + |
| +// release returns a start barrier: the workers block on wait() until |
| +// the test closes the gate, so they enter the store together instead |
| +// of trickling in as they are spawned. |
| +func release() (gate chan struct{}, wait func()) { |
| + gate = make(chan struct{}) |
| + return gate, func() { <-gate } |
| +} |
| + |
| +// pair runs two operations concurrently off one start barrier, with |
| +// the spawn order flipped when swap is true. |
| +// |
| +// The flip is not decoration. Closing a channel wakes its waiters in |
| +// FIFO order and the LAST one readied lands in the P's runnext slot, |
| +// so it is the last-spawned goroutine that actually runs first — a |
| +// systematic bias, and a measured one: without the flip, Revoke won |
| +// 58-60 of every 60 rounds below and the Accept-wins branch of the |
| +// invariant was barely exercised at all. A race test that only ever |
| +// resolves one way is green for a reason unrelated to the property it |
| +// claims to check. Alternating the spawn order splits the outcomes |
| +// without weakening the race: both goroutines are still released |
| +// together and still contend for the same single writer connection. |
| +func pair(swap bool, first, second func()) { |
| + if swap { |
| + first, second = second, first |
| + } |
| + gate, wait := release() |
| + var wg sync.WaitGroup |
| + wg.Add(2) |
| + go func() { |
| + defer wg.Done() |
| + wait() |
| + first() |
| + }() |
| + go func() { |
| + defer wg.Done() |
| + wait() |
| + second() |
| + }() |
| + close(gate) |
| + wg.Wait() |
| +} |
| + |
| +// TestConcurrentClaimYieldsOneOwner races six first-signups at an |
| +// unclaimed instance. |
| +// |
| +// Regression it catches: Claim counting rows OUTSIDE its transaction, |
| +// or counting only ACTIVE rows. Either way more than one goroutine |
| +// sees an empty roster, and the instance ends up with two Owners — |
| +// two people who can each demote the other, on a roster that is |
| +// supposed to have exactly one root of authority. |
| +func TestConcurrentClaimYieldsOneOwner(t *testing.T) { |
| + const n = 6 |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + |
| + gate, wait := release() |
| + errs := make([]error, n) |
| + var wg sync.WaitGroup |
| + for i := range n { |
| + wg.Add(1) |
| + go func() { |
| + defer wg.Done() |
| + wait() |
| + _, errs[i] = h.Roster.Claim(ctx, |
| + fmt.Sprintf("claimant-%d", i), |
| + fmt.Sprintf("claimant-%d@example.test", i), |
| + fmt.Sprintf("Claimant %d", i)) |
| + }() |
| + } |
| + close(gate) |
| + wg.Wait() |
| + |
| + won, refused := 0, 0 |
| + for i, err := range errs { |
| + switch { |
| + case err == nil: |
| + won++ |
| + case errors.Is(err, idear.ErrOwnerExists): |
| + refused++ |
| + default: |
| + t.Errorf("claimant %d failed with an unexpected error: %v", i, err) |
| + } |
| + } |
| + if won != 1 { |
| + t.Errorf("%d of %d concurrent claims succeeded, want exactly 1", won, n) |
| + } |
| + if refused != n-1 { |
| + t.Errorf("%d claims were refused with ErrOwnerExists, want %d", refused, n-1) |
| + } |
| + if got := h.CountMembers(); got != 1 { |
| + t.Errorf("the roster holds %d rows, want exactly 1", got) |
| + } |
| + if owner := h.TheOwner(); !owner.Active() { |
| + t.Error("the owner is not active") |
| + } |
| +} |
| + |
| +// TestConcurrentTransfersKeepOneOwner races six transfers out of the |
| +// same Owner, each to a different target. |
| +// |
| +// Regression it catches: Transfer trusting the *Member it was handed |
| +// instead of re-reading the actor's row inside its transaction. Every |
| +// goroutine holds a struct that says "I am the Owner", and every one |
| +// of them is telling the truth about the moment it was read. Without |
| +// the re-read, all six promote their target and the instance ends up |
| +// with six Owners and no way back — this is the exact defect round 1 |
| +// of the bake-off found in a hand-rolled version. |
| +func TestConcurrentTransfersKeepOneOwner(t *testing.T) { |
| + const n = 6 |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + |
| + owner := h.Owner() |
| + targets := make([]*idear.Member, n) |
| + for i := range targets { |
| + targets[i] = h.Member(idear.RoleAdmin) |
| + } |
| + |
| + gate, wait := release() |
| + errs := make([]error, n) |
| + var wg sync.WaitGroup |
| + for i := range n { |
| + wg.Add(1) |
| + go func() { |
| + defer wg.Done() |
| + wait() |
| + errs[i] = h.Roster.Transfer(ctx, owner, targets[i]) |
| + }() |
| + } |
| + close(gate) |
| + wg.Wait() |
| + |
| + won := 0 |
| + for i, err := range errs { |
| + switch { |
| + case err == nil: |
| + won++ |
| + case errors.Is(err, idear.ErrForbidden): |
| + // The transaction re-read an actor who is no longer Owner. |
| + default: |
| + t.Errorf("transfer %d failed with an unexpected error: %v", i, err) |
| + } |
| + } |
| + if won != 1 { |
| + t.Errorf("%d of %d concurrent transfers succeeded, want exactly 1", won, n) |
| + } |
| + |
| + // The count is the invariant, not the number of successes: a |
| + // transaction that half-committed would leave two Owners while |
| + // reporting one success. |
| + newOwner := h.TheOwner() |
| + if !newOwner.Active() { |
| + t.Error("the new owner is not active") |
| + } |
| + if newOwner.ID == owner.ID { |
| + t.Error("ownership did not move at all; one transfer should have won") |
| + } |
| + if got := h.Reload(owner.ID).Role; got != idear.RoleAdmin { |
| + t.Errorf("the outgoing owner's role = %q, want admin — the demote and the promote are one transaction", got) |
| + } |
| +} |
| + |
| +// TestRevokeRacingAcceptNeverAdmits races a Revoke against an Accept of |
| +// the same invitation, many times over, each iteration on a fresh |
| +// invitation. |
| +// |
| +// Regression it catches: Accept consuming the invitation by lookup and |
| +// then writing, instead of by compare-and-swap with rows-affected |
| +// checked. Between the lookup and the write, Revoke commits — and the |
| +// invitation is admitted after it was withdrawn, which is the whole |
| +// point of being able to withdraw one. Softening the CAS's WHERE |
| +// clause (dropping "revoked_at IS NULL", say) fails here too, as does |
| +// moving the Member insert out of the invitation's transaction. |
| +// |
| +// Note that "never both" is the assertion, not "Accept always loses". |
| +// Either outcome is correct; what is not correct is both succeeding. |
| +func TestRevokeRacingAcceptNeverAdmits(t *testing.T) { |
| + const iterations = 60 |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + admitted, revoked, both, neither := 0, 0, 0, 0 |
| + for i := range iterations { |
| + email := fmt.Sprintf("racer-%d@example.test", i) |
| + subject := fmt.Sprintf("racer-%d", i) |
| + inv, token, err := h.Roster.Invite(ctx, owner, email, idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("iteration %d: Invite: %v", i, err) |
| + } |
| + |
| + var ( |
| + acceptErr, revokeErr error |
| + acceptedM *idear.Member |
| + ) |
| + pair(i%2 == 1, |
| + func() { acceptedM, acceptErr = h.Roster.Accept(ctx, token, subject, "Racer") }, |
| + func() { revokeErr = h.Roster.Revoke(ctx, owner, inv.ID) }, |
| + ) |
| + acceptDone, revokeDone := acceptErr == nil, revokeErr == nil |
| + |
| + if acceptErr != nil && !errors.Is(acceptErr, idear.ErrNoInvitation) { |
| + t.Fatalf("iteration %d: Accept failed unexpectedly: %v", i, acceptErr) |
| + } |
| + if revokeErr != nil && !errors.Is(revokeErr, idear.ErrNoInvitation) { |
| + t.Fatalf("iteration %d: Revoke failed unexpectedly: %v", i, revokeErr) |
| + } |
| + |
| + switch { |
| + case acceptDone && revokeDone: |
| + both++ |
| + t.Errorf("iteration %d: the invitation was BOTH accepted and revoked", i) |
| + case acceptDone: |
| + admitted++ |
| + case revokeDone: |
| + revoked++ |
| + default: |
| + neither++ |
| + t.Errorf("iteration %d: neither Accept nor Revoke succeeded (accept=%v revoke=%v)", i, acceptErr, revokeErr) |
| + } |
| + |
| + // The row's own state must agree with who won, and a member |
| + // must exist if and only if Accept won. A CAS that updated the |
| + // invitation but lost the member insert would show up here. |
| + stored := h.Invitation(inv.ID) |
| + _, lookupErr := h.Roster.BySubject(ctx, subject) |
| + if acceptDone { |
| + if stored.AcceptedAt == nil { |
| + t.Errorf("iteration %d: Accept succeeded but AcceptedAt is NULL", i) |
| + } |
| + if stored.RevokedAt != nil { |
| + t.Errorf("iteration %d: the accepted invitation is also marked revoked", i) |
| + } |
| + if lookupErr != nil { |
| + t.Errorf("iteration %d: Accept succeeded but the member is not in the roster: %v", i, lookupErr) |
| + } |
| + if acceptedM != nil && acceptedM.Role != idear.RoleMember { |
| + t.Errorf("iteration %d: admitted at %q, want the invited role", i, acceptedM.Role) |
| + } |
| + } else { |
| + if stored.AcceptedAt != nil { |
| + t.Errorf("iteration %d: Accept failed but the invitation is marked accepted", i) |
| + } |
| + if !errors.Is(lookupErr, idear.ErrNotFound) { |
| + t.Errorf("iteration %d: a refused Accept admitted the subject anyway: %v", i, lookupErr) |
| + } |
| + } |
| + } |
| + |
| + t.Logf("%d iterations: %d admitted, %d revoked, %d both, %d neither", |
| + iterations, admitted, revoked, both, neither) |
| + // Both outcomes should actually occur across this many rounds. If |
| + // one never does, the race is not being exercised — the test would |
| + // still be green while proving nothing. |
| + if admitted == 0 || revoked == 0 { |
| + t.Errorf("only one outcome ever occurred (%d admitted, %d revoked); "+ |
| + "the two calls are not actually racing and this test proves nothing", |
| + admitted, revoked) |
| + } |
| +} |
| + |
| +// TestTransferRacingDeactivateNeverStrandsOwner races a Transfer of |
| +// ownership TO a member against a Deactivate OF that same member. |
| +// |
| +// Regression it catches: Transfer not confirming, inside its own |
| +// transaction, that the target is still active. The losing order is |
| +// Deactivate-then-Transfer: the transfer promotes a row that was |
| +// deactivated a microsecond ago, and the instance now has a |
| +// DEACTIVATED Owner. That is terminal through idear's own API — |
| +// MayActOn refuses acting on an Owner at every rank, so nobody can |
| +// reactivate them, nobody can demote them, and nobody can be promoted |
| +// past them. It also catches Deactivate checking "is this the Owner" |
| +// against the caller's stale struct rather than the row: in the other |
| +// order, the target IS the Owner by the time Deactivate's transaction |
| +// runs, and must be refused. |
| +func TestTransferRacingDeactivateNeverStrandsOwner(t *testing.T) { |
| + const iterations = 40 |
| + transferWins, deactivateWins := 0, 0 |
| + |
| + for i := range iterations { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + target := h.Member(idear.RoleAdmin) |
| + |
| + var transferErr, removeErr error |
| + pair(i%2 == 1, |
| + func() { transferErr = h.Roster.Transfer(ctx, owner, target) }, |
| + func() { removeErr = h.Roster.Deactivate(ctx, owner, target) }, |
| + ) |
| + if transferErr == nil { |
| + transferWins++ |
| + } else { |
| + deactivateWins++ |
| + } |
| + |
| + if transferErr != nil && !errors.Is(transferErr, idear.ErrForbidden) { |
| + t.Fatalf("iteration %d: Transfer failed unexpectedly: %v", i, transferErr) |
| + } |
| + if removeErr != nil && !errors.Is(removeErr, idear.ErrForbidden) && !errors.Is(removeErr, idear.ErrLastOwner) { |
| + t.Fatalf("iteration %d: Deactivate failed unexpectedly: %v", i, removeErr) |
| + } |
| + |
| + // The invariant, whichever order won: exactly one Owner, and |
| + // that Owner is ACTIVE. |
| + current := h.TheOwner() |
| + if !current.Active() { |
| + t.Fatalf("iteration %d: the instance has a DEACTIVATED owner (member %d) — "+ |
| + "nobody can administer it and nobody can be promoted (transfer=%v deactivate=%v)", |
| + i, current.ID, transferErr, removeErr) |
| + } |
| + |
| + switch { |
| + case transferErr == nil: |
| + // Transfer won the race, so Deactivate must have been |
| + // refused: by the time it ran, its target was the Owner. |
| + if current.ID != target.ID { |
| + t.Errorf("iteration %d: Transfer succeeded but the owner is %d, not %d", i, current.ID, target.ID) |
| + } |
| + if removeErr == nil { |
| + t.Errorf("iteration %d: the new owner was deactivated by the racing Deactivate", i) |
| + } |
| + default: |
| + // Deactivate won, so ownership must not have moved. |
| + if current.ID != owner.ID { |
| + t.Errorf("iteration %d: Transfer was refused but ownership moved to %d", i, current.ID) |
| + } |
| + } |
| + } |
| + |
| + t.Logf("%d iterations: transfer won %d, deactivate won %d", iterations, transferWins, deactivateWins) |
| + if transferWins == 0 || deactivateWins == 0 { |
| + t.Errorf("only one order ever occurred (transfer %d, deactivate %d); "+ |
| + "the two calls are not actually racing and this test proves nothing", |
| + transferWins, deactivateWins) |
| + } |
| +} |
diff --git a/roster.go b/roster.go| new file mode 100644 |
| index 0000000..97cb710 |
| --- /dev/null |
| +++ b/roster.go |
| @@ -0,0 +1,716 @@ |
| +package idear |
| + |
| +import ( |
| + "context" |
| + "errors" |
| + "fmt" |
| + "log/slog" |
| + "net/http" |
| + "strings" |
| + "time" |
| + |
| + "github.com/carlosframework/rastrillo/sessions" |
| + "gorm.io/gorm" |
| +) |
| + |
| +// defaultInviteTTL is how long an invitation stays redeemable when the |
| +// app does not say. Seven days: long enough to survive a weekend and a |
| +// forwarded mail, short enough that a link found in an old inbox is |
| +// already dead. Round 1 of the bake-off penalised immortal invitation |
| +// tokens; this is that finding applied before it is earned twice. |
| +const defaultInviteTTL = 7 * 24 * time.Hour |
| + |
| +// Config configures New. DB is required; everything else has a |
| +// serviceable default. |
| +type Config struct { |
| + // DB is the app's database, as *gorm.DB — d.G from rastrillo/db. |
| + // idear's migrations (Schema, merged into the app's BootSchema) |
| + // must already be applied before any method runs. |
| + // |
| + // It is expected to be rastrillo/db's split pool: one writer |
| + // connection, several readers, routed per statement. That routing |
| + // is why every mutation below funnels through tx and why nothing |
| + // inside a transaction may touch rs.cfg.DB — see tx. |
| + DB *gorm.DB |
| + |
| + // OpenSignUp admits any verified address at RoleMember with no |
| + // invitation. It is read by the admission adapters, not by the |
| + // store: the roster itself never decides policy it was not asked |
| + // about. |
| + OpenSignUp bool |
| + |
| + // InviteTTL is how long an invitation Invite mints stays |
| + // redeemable. Default defaultInviteTTL. |
| + InviteTTL time.Duration |
| + |
| + // Subject resolves the viewer's session Subject from a request. |
| + // Default: sessions.Current(r).Subject. It exists as an override |
| + // for an app whose viewer arrives some other way; it reads a |
| + // session the caller must ALREADY have resolved, so idear's |
| + // middleware must be mounted inside the app's session guard. |
| + Subject func(*http.Request) (string, bool) |
| + |
| + // NotFound answers a request from someone who is not an active |
| + // member. It MUST be the same renderer the app gives chi's own |
| + // NotFound: an app with a custom 404 page and idear's default |
| + // http.NotFound produces two distinguishable 404s, and that delta |
| + // is a membership oracle. Default http.NotFound. |
| + NotFound func(http.ResponseWriter, *http.Request) |
| + |
| + // Forbidden answers a member who may see a page but may not act on |
| + // it. Default: 403 with plain text. |
| + Forbidden func(http.ResponseWriter, *http.Request) |
| + |
| + Logger *slog.Logger |
| +} |
| + |
| +// Roster is the store: who is in this instance, at what role, and who |
| +// may change that. Build exactly one per process (New) and share it — |
| +// it holds no per-request state. |
| +// |
| +// Every mutation below is ONE transaction, and every invariant is |
| +// enforced INSIDE the transaction that maintains it. An invariant |
| +// checked outside its transaction is not an invariant: it is a |
| +// prediction, and a concurrent writer is under no obligation to honour |
| +// it. That is the whole reason this type exists rather than a handful |
| +// of queries at the call sites. |
| +type Roster struct { |
| + cfg Config |
| +} |
| + |
| +// New validates cfg and returns a ready *Roster. |
| +func New(cfg Config) (*Roster, error) { |
| + if cfg.DB == nil { |
| + return nil, errors.New("idear: Config.DB is required") |
| + } |
| + if cfg.InviteTTL == 0 { |
| + cfg.InviteTTL = defaultInviteTTL |
| + } |
| + if cfg.Subject == nil { |
| + cfg.Subject = func(r *http.Request) (string, bool) { |
| + sess, ok := sessions.Current(r) |
| + if !ok || sess.Subject == "" { |
| + return "", false |
| + } |
| + return sess.Subject, true |
| + } |
| + } |
| + if cfg.NotFound == nil { |
| + cfg.NotFound = http.NotFound |
| + } |
| + if cfg.Forbidden == nil { |
| + cfg.Forbidden = func(w http.ResponseWriter, r *http.Request) { |
| + http.Error(w, "Forbidden", http.StatusForbidden) |
| + } |
| + } |
| + if cfg.Logger == nil { |
| + cfg.Logger = slog.Default() |
| + } |
| + return &Roster{cfg: cfg}, nil |
| +} |
| + |
| +// OpenSignUp reports whether this instance admits any verified address |
| +// at RoleMember with no invitation. The admission adapters read it; |
| +// the store never does. |
| +func (rs *Roster) OpenSignUp() bool { return rs.cfg.OpenSignUp } |
| + |
| +// InviteTTL is how long a freshly minted invitation stays redeemable. |
| +func (rs *Roster) InviteTTL() time.Duration { return rs.cfg.InviteTTL } |
| + |
| +// now is the one clock the store reads, and it is UTC on purpose — |
| +// twice over. |
| +// |
| +// The obvious reason is that every row is UTC. The second is a trap: |
| +// timestamps reach SQLite through the driver as time.Time.String(), |
| +// and String() appends " m=+0.000000001" to any time that still |
| +// carries a monotonic reading. A stored value with that suffix breaks |
| +// the text comparison the CAS in Accept depends on — "expires_at > ?" |
| +// would compare a monotonic-tagged string against a plain one and |
| +// answer nonsense. time.Now().UTC() strips the monotonic reading; |
| +// time.Now() alone does not, and neither does Add on top of it. |
| +func (rs *Roster) now() time.Time { return time.Now().UTC() } |
| + |
| +// tx runs fn in one transaction. |
| +// |
| +// Inside fn, use ONLY tx. A statement issued against rs.cfg.DB from |
| +// inside fn does not join the transaction: it goes to the pool whose |
| +// single writer connection this transaction is already holding, waits |
| +// for a connection that cannot be released until fn returns, and |
| +// HANGS — it does not error. If a test of this package ever hangs, |
| +// that is the first thing to look for. |
| +func (rs *Roster) tx(ctx context.Context, fn func(tx *gorm.DB) error) error { |
| + return rs.cfg.DB.WithContext(ctx).Transaction(fn) |
| +} |
| + |
| +// normalizeEmail is the one spelling of an address idear stores or |
| +// compares. Addresses arrive from forms and identity plugins with |
| +// stray whitespace and arbitrary case, and an invitation whose Email |
| +// matches only when the invitee retypes the capitalisation they were |
| +// sent is not a working invitation. Every write and every comparison |
| +// goes through here so both sides are normalised the same way. |
| +func normalizeEmail(email string) string { |
| + return strings.ToLower(strings.TrimSpace(email)) |
| +} |
| + |
| +// forbidden builds an ErrForbidden carrying reason, so a log line can |
| +// say what was refused while callers still test with errors.Is. |
| +func forbidden(format string, args ...any) error { |
| + return fmt.Errorf("%w: %s", ErrForbidden, fmt.Sprintf(format, args...)) |
| +} |
| + |
| +// checkInviteRole is the role gate Invite and SetRole share. |
| +// |
| +// RoleOwner is refused OUTRIGHT here, on every path, for every actor — |
| +// including an actor who IS the Owner. Ownership moves only by |
| +// Transfer, which is the only operation that demotes the outgoing |
| +// Owner in the same transaction as it promotes the incoming one, and |
| +// therefore the only one that keeps "exactly one Owner" true at every |
| +// commit boundary. Any other route to RoleOwner is a second Owner. |
| +// |
| +// The second rule is subtler and closes an escalation: the granted |
| +// role must rank STRICTLY BELOW the actor's. An Admin may invite or |
| +// set only Member, because an Admin who could mint a peer Admin has |
| +// escalated — MayActOn refuses acting on an equal rank, so the new |
| +// Admin would be beyond the granter's reach, and beyond the reach of |
| +// every other Admin too. "Admins manage Members only" (design spec §5) |
| +// has to hold for creation as well as for management, or it holds for |
| +// neither. |
| +func checkInviteRole(actor *Member, role Role) error { |
| + if !role.Valid() { |
| + return fmt.Errorf("%w: %q", ErrInvalidRole, string(role)) |
| + } |
| + if role == RoleOwner { |
| + return forbidden("ownership moves only by Transfer, never by grant") |
| + } |
| + if rank(role) >= rank(actor.Role) { |
| + return forbidden("a %s may not grant %s", actor.Role, role) |
| + } |
| + return nil |
| +} |
| + |
| +// loadMember re-reads one member by id inside tx. Every mutation that |
| +// takes an *Member argument re-reads it through here rather than |
| +// trusting the struct it was handed: that struct was read before the |
| +// transaction opened, so its Role and DeactivatedAt are a claim about |
| +// the past. The row inside the transaction is the fact. |
| +func loadMember(tx *gorm.DB, id int64) (*Member, error) { |
| + var m Member |
| + if err := tx.Where("id = ?", id).Take(&m).Error; err != nil { |
| + if errors.Is(err, gorm.ErrRecordNotFound) { |
| + return nil, ErrNotFound |
| + } |
| + return nil, err |
| + } |
| + return &m, nil |
| +} |
| + |
| +// IsEmpty reports whether the roster has ZERO ROWS — not zero ACTIVE |
| +// rows. A roster whose members have every one been deactivated is not |
| +// empty, and must not reopen the claim: doing so would hand a stranger |
| +// Owner of an instance full of dormant data. |
| +// |
| +// It is advisory, and answered from the read pool, so it may lag a |
| +// concurrent Claim by one WAL snapshot. Nothing depends on it being |
| +// current: Claim re-asks the same question inside its own transaction, |
| +// which is where the answer is binding. |
| +func (rs *Roster) IsEmpty(ctx context.Context) (bool, error) { |
| + var n int64 |
| + if err := rs.cfg.DB.WithContext(ctx).Model(&Member{}).Count(&n).Error; err != nil { |
| + return false, err |
| + } |
| + return n == 0, nil |
| +} |
| + |
| +// Claim makes the first arrival the Owner of an unclaimed instance. |
| +// |
| +// It succeeds only when idear_members holds zero rows, counted inside |
| +// the same transaction as the insert. Two concurrent first signups |
| +// therefore produce exactly one Owner; the loser gets ErrOwnerExists |
| +// and is an orphan — a user row in the app with no membership — which |
| +// the signed-in reconciliation route exists to heal (design spec §5). |
| +// That is a designed path, not an accident. |
| +func (rs *Roster) Claim(ctx context.Context, subject, email, name string) (*Member, error) { |
| + subject = strings.TrimSpace(subject) |
| + if subject == "" { |
| + return nil, errors.New("idear: Claim needs a non-empty subject") |
| + } |
| + m := &Member{ |
| + Subject: subject, |
| + Email: normalizeEmail(email), |
| + Name: strings.TrimSpace(name), |
| + Role: RoleOwner, |
| + } |
| + err := rs.tx(ctx, func(tx *gorm.DB) error { |
| + var n int64 |
| + if err := tx.Model(&Member{}).Count(&n).Error; err != nil { |
| + return err |
| + } |
| + if n != 0 { |
| + return ErrOwnerExists |
| + } |
| + return tx.Create(m).Error |
| + }) |
| + if err != nil { |
| + return nil, err |
| + } |
| + return m, nil |
| +} |
| + |
| +// Invite mints an invitation to join at role and returns the plaintext |
| +// token EXACTLY ONCE — only its SHA-256 digest is stored, so a leaked |
| +// database yields no usable links and idear cannot re-send the old one |
| +// (invite again instead). |
| +// |
| +// actor must be an active member of at least Admin rank, and role must |
| +// rank strictly below actor's; RoleOwner is refused for everyone. See |
| +// checkInviteRole. The actor is re-read inside the transaction, so an |
| +// admin deactivated a moment ago cannot still hand out invitations. |
| +func (rs *Roster) Invite(ctx context.Context, actor *Member, email string, role Role) (*Invitation, string, error) { |
| + if actor == nil { |
| + return nil, "", forbidden("no actor") |
| + } |
| + email = normalizeEmail(email) |
| + if email == "" { |
| + return nil, "", errors.New("idear: Invite needs a non-empty email") |
| + } |
| + token, err := newToken() |
| + if err != nil { |
| + return nil, "", fmt.Errorf("idear: minting invitation token: %w", err) |
| + } |
| + |
| + now := rs.now() |
| + inv := &Invitation{ |
| + Email: email, |
| + Role: role, |
| + TokenHash: hashToken(token), |
| + ExpiresAt: now.Add(rs.cfg.InviteTTL), |
| + } |
| + err = rs.tx(ctx, func(tx *gorm.DB) error { |
| + cur, err := loadMember(tx, actor.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if err := mayManage(cur); err != nil { |
| + return err |
| + } |
| + if err := checkInviteRole(cur, role); err != nil { |
| + return err |
| + } |
| + inv.InvitedBy = cur.ID |
| + return tx.Create(inv).Error |
| + }) |
| + if err != nil { |
| + return nil, "", err |
| + } |
| + return inv, token, nil |
| +} |
| + |
| +// Revoke kills an outstanding invitation. It refuses with |
| +// ErrNoInvitation when there is nothing to kill — no such id, or one |
| +// already accepted or already revoked — which is the same answer |
| +// Accept gives, so neither call distinguishes the cases for a caller |
| +// who should not be told them apart. |
| +// |
| +// The update is conditional and rows-affected-checked, so Revoke |
| +// racing Accept resolves one way or the other and never both: whichever |
| +// transaction commits first leaves the other's WHERE clause matching |
| +// nothing. |
| +func (rs *Roster) Revoke(ctx context.Context, actor *Member, id int64) error { |
| + if actor == nil { |
| + return forbidden("no actor") |
| + } |
| + now := rs.now() |
| + return rs.tx(ctx, func(tx *gorm.DB) error { |
| + cur, err := loadMember(tx, actor.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if err := mayManage(cur); err != nil { |
| + return err |
| + } |
| + res := tx.Model(&Invitation{}). |
| + Where("id = ? AND accepted_at IS NULL AND revoked_at IS NULL", id). |
| + Update("revoked_at", now) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected != 1 { |
| + return ErrNoInvitation |
| + } |
| + return nil |
| + }) |
| +} |
| + |
| +// Accept redeems an invitation token and writes the Member it buys. |
| +// |
| +// The invitation is consumed by COMPARE-AND-SWAP, not by lookup: one |
| +// conditional UPDATE that requires the row to be unaccepted, unrevoked |
| +// and unexpired, with rows-affected checked, in the same transaction |
| +// as the Member insert. A lookup followed by a write is not the same |
| +// thing — between the two, Revoke can commit, and the invitation is |
| +// admitted after it was withdrawn. |
| +// |
| +// Single use cannot be delegated to the app's own unique-email index |
| +// either: idear can neither see that index nor enforce it, and the |
| +// app's user row is written by code idear does not control. The CAS is |
| +// the invariant, and it is the only one. |
| +// |
| +// The role is taken from the stored invitation, never from a caller, |
| +// and RoleOwner is refused even here — an owner-role invitation should |
| +// be impossible to mint, and a row that carries one is corruption, not |
| +// permission. |
| +// |
| +// A subject that already has a Member row — including a DEACTIVATED |
| +// one, because removal is never a delete — collides with the unique |
| +// index and rolls the whole transaction back, invitation included. A |
| +// returning member is readmitted by Reactivate, not by a fresh |
| +// invitation; that is exactly why Reactivate exists (design spec §4). |
| +func (rs *Roster) Accept(ctx context.Context, token, subject, name string) (*Member, error) { |
| + subject = strings.TrimSpace(subject) |
| + if subject == "" { |
| + return nil, errors.New("idear: Accept needs a non-empty subject") |
| + } |
| + if token == "" { |
| + return nil, ErrNoInvitation |
| + } |
| + hash := hashToken(token) |
| + now := rs.now() |
| + |
| + var m *Member |
| + err := rs.tx(ctx, func(tx *gorm.DB) error { |
| + res := tx.Model(&Invitation{}). |
| + Where("token_hash = ? AND accepted_at IS NULL AND revoked_at IS NULL AND expires_at > ?", hash, now). |
| + Update("accepted_at", now) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected != 1 { |
| + return ErrNoInvitation |
| + } |
| + |
| + // Safe only because the CAS above just claimed this row inside |
| + // this transaction: the read cannot see a competing writer. |
| + var inv Invitation |
| + if err := tx.Where("token_hash = ?", hash).Take(&inv).Error; err != nil { |
| + return err |
| + } |
| + if !inv.Role.Valid() || inv.Role == RoleOwner { |
| + return forbidden("invitation carries role %q, which cannot be granted", string(inv.Role)) |
| + } |
| + m = &Member{ |
| + Subject: subject, |
| + Email: inv.Email, |
| + Name: strings.TrimSpace(name), |
| + Role: inv.Role, |
| + } |
| + return tx.Create(m).Error |
| + }) |
| + if err != nil { |
| + return nil, err |
| + } |
| + return m, nil |
| +} |
| + |
| +// SetRole changes target's role. |
| +// |
| +// Refusals, in the order they are checked and for the reason each is |
| +// checked where it is: |
| +// |
| +// - the new role must be one of the three, and must not be |
| +// RoleOwner: ownership moves only by Transfer (checkInviteRole). |
| +// - actor must clear the authority floor — active, at least Admin — |
| +// so a plain Member is refused for lacking authority and learns |
| +// nothing about the target. |
| +// - the target must not BE the Owner: ErrLastOwner. Demoting the |
| +// Owner by this route would leave the instance with no Owner at |
| +// all, so this is an invariant and not a permission, and it is |
| +// checked against the row inside the transaction. |
| +// - MayActOn(actor, target) for the rest of the matrix. |
| +// |
| +// A deactivated target may have their role changed; it takes effect if |
| +// and when they are reactivated. |
| +func (rs *Roster) SetRole(ctx context.Context, actor, target *Member, role Role) error { |
| + if actor == nil || target == nil { |
| + return forbidden("no actor or no target") |
| + } |
| + if !role.Valid() { |
| + return fmt.Errorf("%w: %q", ErrInvalidRole, string(role)) |
| + } |
| + if role == RoleOwner { |
| + return forbidden("ownership moves only by Transfer, never by SetRole") |
| + } |
| + return rs.tx(ctx, func(tx *gorm.DB) error { |
| + cur, err := loadMember(tx, actor.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if err := mayManage(cur); err != nil { |
| + return err |
| + } |
| + tgt, err := loadMember(tx, target.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if tgt.Role == RoleOwner { |
| + return ErrLastOwner |
| + } |
| + if err := MayActOn(cur, tgt); err != nil { |
| + return err |
| + } |
| + // checkInviteRole repeats the RoleOwner refusal above, and |
| + // deliberately: the early one answers before a transaction is |
| + // opened, this one answers against the actor's CURRENT rank. |
| + // Deleting either leaves the other holding, which a mutation |
| + // run confirmed — RoleOwner is refused three ways here (the |
| + // early guard, this branch, and the strictly-below-actor rank |
| + // rule, which no actor can clear for owner) and the suite only |
| + // goes red when all three are gone. |
| + if err := checkInviteRole(cur, role); err != nil { |
| + return err |
| + } |
| + res := tx.Model(&Member{}).Where("id = ? AND role <> ?", tgt.ID, RoleOwner). |
| + Update("role", role) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected != 1 { |
| + return ErrLastOwner |
| + } |
| + return nil |
| + }) |
| +} |
| + |
| +// Deactivate removes target from the roster — by setting |
| +// DeactivatedAt, never by deleting the row, because a deleted row |
| +// dangles every AuthorID in the app's own tables. |
| +// |
| +// The Owner can never be deactivated (ErrLastOwner), checked against |
| +// the row inside the transaction. That check is what makes Transfer |
| +// racing a Deactivate of the same target safe from this side: if the |
| +// transfer commits first, this transaction re-reads a target who is |
| +// now the Owner and refuses, instead of deactivating the Owner it just |
| +// became. |
| +// |
| +// Deactivating an already-deactivated member is a no-op, not an error. |
| +func (rs *Roster) Deactivate(ctx context.Context, actor, target *Member) error { |
| + if actor == nil || target == nil { |
| + return forbidden("no actor or no target") |
| + } |
| + now := rs.now() |
| + return rs.tx(ctx, func(tx *gorm.DB) error { |
| + cur, err := loadMember(tx, actor.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if err := mayManage(cur); err != nil { |
| + return err |
| + } |
| + tgt, err := loadMember(tx, target.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if tgt.Role == RoleOwner { |
| + return ErrLastOwner |
| + } |
| + if err := MayActOn(cur, tgt); err != nil { |
| + return err |
| + } |
| + if !tgt.Active() { |
| + return nil |
| + } |
| + res := tx.Model(&Member{}). |
| + Where("id = ? AND deactivated_at IS NULL AND role <> ?", tgt.ID, RoleOwner). |
| + Update("deactivated_at", now) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected != 1 { |
| + return ErrLastOwner |
| + } |
| + return nil |
| + }) |
| +} |
| + |
| +// Reactivate readmits a deactivated member at the role they still |
| +// carry. |
| +// |
| +// It is a first-class operation and not a convenience: Subject is |
| +// unique and removal is deactivation, so without Reactivate a removed |
| +// person can never be readmitted by ANY path — keymail's Authorize |
| +// sees the inactive row and refuses, password re-signup hits the app's |
| +// duplicate-email check, and a fresh invitation's Member insert |
| +// collides with the dead row (design spec §4). |
| +// |
| +// Reactivating an already-active member is a no-op, not an error. |
| +func (rs *Roster) Reactivate(ctx context.Context, actor, target *Member) error { |
| + if actor == nil || target == nil { |
| + return forbidden("no actor or no target") |
| + } |
| + return rs.tx(ctx, func(tx *gorm.DB) error { |
| + cur, err := loadMember(tx, actor.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if err := mayManage(cur); err != nil { |
| + return err |
| + } |
| + tgt, err := loadMember(tx, target.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if err := MayActOn(cur, tgt); err != nil { |
| + return err |
| + } |
| + if tgt.Active() { |
| + return nil |
| + } |
| + res := tx.Model(&Member{}). |
| + Where("id = ? AND deactivated_at IS NOT NULL", tgt.ID). |
| + Update("deactivated_at", nil) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected != 1 { |
| + return ErrNotFound |
| + } |
| + return nil |
| + }) |
| +} |
| + |
| +// Transfer hands ownership of the instance from owner to to, in one |
| +// transaction: demote the outgoing Owner to Admin, promote the |
| +// incoming one to Owner. Exactly one Owner exists at every commit |
| +// boundary, including this one — there is no instant, even inside the |
| +// transaction, at which the instance has two Owners or none. |
| +// |
| +// Both rows are RE-READ inside the transaction, and two things |
| +// confirmed about them: |
| +// |
| +// - the actor is still the Owner. Six concurrent transfers therefore |
| +// resolve to one: the first commits, and every other transaction |
| +// re-reads an actor who is now an Admin. Round 1 of the bake-off |
| +// found exactly this bug in a hand-rolled version, and found it |
| +// only because someone ran it as an actual race. |
| +// - the target is still ACTIVE. Without this, a transfer racing a |
| +// Deactivate of the same target produces a DEACTIVATED Owner: an |
| +// instance with nobody able to administer it and nobody able to be |
| +// promoted, because MayActOn lets no rank act on an Owner. It is |
| +// unrecoverable through idear's own API, which is what makes it |
| +// worth a re-read rather than a comment. |
| +// |
| +// Both updates are conditional and rows-affected-checked as well, so |
| +// the invariant is stated at the statement and not only in the |
| +// preceding reads. |
| +func (rs *Roster) Transfer(ctx context.Context, owner, to *Member) error { |
| + if owner == nil || to == nil { |
| + return forbidden("no owner or no target") |
| + } |
| + if owner.ID == to.ID { |
| + return forbidden("ownership cannot be transferred to its current holder") |
| + } |
| + return rs.tx(ctx, func(tx *gorm.DB) error { |
| + cur, err := loadMember(tx, owner.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if !cur.Active() || cur.Role != RoleOwner { |
| + return forbidden("only the current owner may transfer ownership") |
| + } |
| + tgt, err := loadMember(tx, to.ID) |
| + if err != nil { |
| + return err |
| + } |
| + if !tgt.Active() { |
| + return forbidden("ownership cannot be transferred to a deactivated member") |
| + } |
| + |
| + res := tx.Model(&Member{}).Where("id = ? AND role = ?", cur.ID, RoleOwner). |
| + Update("role", RoleAdmin) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected != 1 { |
| + return forbidden("ownership moved before this transfer could complete") |
| + } |
| + res = tx.Model(&Member{}).Where("id = ? AND deactivated_at IS NULL", tgt.ID). |
| + Update("role", RoleOwner) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected != 1 { |
| + return forbidden("the target stopped being an active member before this transfer could complete") |
| + } |
| + return nil |
| + }) |
| +} |
| + |
| +// BySubject resolves a session Subject to its member row, active or |
| +// not. It deliberately does NOT filter on Active: the middleware has |
| +// to be able to tell a deactivated member from a stranger in order to |
| +// log the difference, even though it answers both with the app's 404. |
| +// Callers decide with Member.Active. |
| +func (rs *Roster) BySubject(ctx context.Context, subject string) (*Member, error) { |
| + subject = strings.TrimSpace(subject) |
| + if subject == "" { |
| + // An empty subject is a request with no session, not a |
| + // wildcard. Answering it from the database would match |
| + // whichever row happens to have an empty subject. |
| + return nil, ErrNotFound |
| + } |
| + var m Member |
| + err := rs.cfg.DB.WithContext(ctx).Where("subject = ?", subject).Take(&m).Error |
| + if errors.Is(err, gorm.ErrRecordNotFound) { |
| + return nil, ErrNotFound |
| + } |
| + if err != nil { |
| + return nil, err |
| + } |
| + return &m, nil |
| +} |
| + |
| +// ByID resolves a member id, active or not. See BySubject. |
| +func (rs *Roster) ByID(ctx context.Context, id int64) (*Member, error) { |
| + var m Member |
| + err := rs.cfg.DB.WithContext(ctx).Where("id = ?", id).Take(&m).Error |
| + if errors.Is(err, gorm.ErrRecordNotFound) { |
| + return nil, ErrNotFound |
| + } |
| + if err != nil { |
| + return nil, err |
| + } |
| + return &m, nil |
| +} |
| + |
| +// Members lists the whole roster, deactivated members included — the |
| +// members page shows them so they can be restored, and a list that |
| +// hid them would make Reactivate unreachable from the UI. |
| +// |
| +// The order is Owner, then Admins, then Members, then by id. It is |
| +// spelled out as a CASE rather than ORDER BY role because the column |
| +// is text: alphabetical order would put Admin above Owner. |
| +func (rs *Roster) Members(ctx context.Context) ([]Member, error) { |
| + var out []Member |
| + err := rs.cfg.DB.WithContext(ctx). |
| + Order("CASE role WHEN 'owner' THEN 0 WHEN 'admin' THEN 1 ELSE 2 END"). |
| + Order("id"). |
| + Find(&out).Error |
| + if err != nil { |
| + return nil, err |
| + } |
| + return out, nil |
| +} |
| + |
| +// PendingInvitations lists the invitations that can still be redeemed |
| +// right now: unaccepted, unrevoked and unexpired. Expired and spent |
| +// rows stay in the table as a record; they are simply not offered. |
| +func (rs *Roster) PendingInvitations(ctx context.Context) ([]Invitation, error) { |
| + var out []Invitation |
| + err := rs.cfg.DB.WithContext(ctx). |
| + Where("accepted_at IS NULL AND revoked_at IS NULL AND expires_at > ?", rs.now()). |
| + Order("id"). |
| + Find(&out).Error |
| + if err != nil { |
| + return nil, err |
| + } |
| + return out, nil |
| +} |
diff --git a/roster_test.go b/roster_test.go| new file mode 100644 |
| index 0000000..9b7ca24 |
| --- /dev/null |
| +++ b/roster_test.go |
| @@ -0,0 +1,838 @@ |
| +package idear_test |
| + |
| +import ( |
| + "errors" |
| + "strings" |
| + "testing" |
| + "time" |
| + |
| + "amadan.net/rastrillo/idear" |
| + "amadan.net/rastrillo/idear/internal/ideartest" |
| +) |
| + |
| +// ---------------------------------------------------------------- New |
| + |
| +func TestNew_RequiresDB(t *testing.T) { |
| + if _, err := idear.New(idear.Config{}); err == nil { |
| + t.Fatal("idear.New with no DB returned no error; a Roster over a nil handle panics on first use") |
| + } |
| +} |
| + |
| +func TestNew_Defaults(t *testing.T) { |
| + h := ideartest.New(t) |
| + if got := h.Roster.InviteTTL(); got != 7*24*time.Hour { |
| + t.Errorf("default InviteTTL = %v, want 7 days", got) |
| + } |
| + if h.Roster.OpenSignUp() { |
| + t.Error("OpenSignUp defaults to true; an instance must be invite-only unless it says otherwise") |
| + } |
| +} |
| + |
| +// -------------------------------------------------------- IsEmpty/Claim |
| + |
| +func TestClaim_FirstArrivalBecomesOwner(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + |
| + empty, err := h.Roster.IsEmpty(ctx) |
| + if err != nil { |
| + t.Fatalf("IsEmpty: %v", err) |
| + } |
| + if !empty { |
| + t.Fatal("a fresh roster is not empty") |
| + } |
| + |
| + m, err := h.Roster.Claim(ctx, "subject-1", " Founder@Example.TEST ", " Founder ") |
| + if err != nil { |
| + t.Fatalf("Claim: %v", err) |
| + } |
| + if m.Role != idear.RoleOwner { |
| + t.Errorf("claimant's role = %q, want owner", m.Role) |
| + } |
| + if m.Email != "founder@example.test" { |
| + t.Errorf("claimant's email = %q, want it trimmed and lowercased", m.Email) |
| + } |
| + if m.Name != "Founder" { |
| + t.Errorf("claimant's name = %q, want it trimmed", m.Name) |
| + } |
| + if got := h.TheOwner(); got.ID != m.ID { |
| + t.Errorf("stored owner id = %d, want %d", got.ID, m.ID) |
| + } |
| +} |
| + |
| +func TestClaim_SecondClaimRefused(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + |
| + if _, err := h.Roster.Claim(ctx, "first", "first@example.test", "First"); err != nil { |
| + t.Fatalf("first Claim: %v", err) |
| + } |
| + _, err := h.Roster.Claim(ctx, "second", "second@example.test", "Second") |
| + if !errors.Is(err, idear.ErrOwnerExists) { |
| + t.Fatalf("second Claim error = %v, want ErrOwnerExists", err) |
| + } |
| + if n := h.CountMembers(); n != 1 { |
| + t.Errorf("roster holds %d members after a refused claim, want 1", n) |
| + } |
| +} |
| + |
| +// TestClaim_DeactivatedRosterDoesNotReopenTheClaim is the "zero rows, |
| +// not zero ACTIVE rows" rule. A roster whose every member has been |
| +// deactivated still holds their rows — and their history, and every |
| +// AuthorID in the app's own tables pointing at them. Reopening the |
| +// claim would hand a stranger Owner of all of it. |
| +func TestClaim_DeactivatedRosterDoesNotReopenTheClaim(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + |
| + h.Deactivated(idear.RoleOwner) |
| + h.Deactivated(idear.RoleMember) |
| + |
| + empty, err := h.Roster.IsEmpty(ctx) |
| + if err != nil { |
| + t.Fatalf("IsEmpty: %v", err) |
| + } |
| + if empty { |
| + t.Error("IsEmpty says a roster of deactivated members is empty; it counts active rows, not rows") |
| + } |
| + |
| + _, err = h.Roster.Claim(ctx, "stranger", "stranger@example.test", "Stranger") |
| + if !errors.Is(err, idear.ErrOwnerExists) { |
| + t.Fatalf("Claim on an all-deactivated roster = %v, want ErrOwnerExists", err) |
| + } |
| + if n := h.CountMembers(); n != 2 { |
| + t.Errorf("roster holds %d members, want the 2 dormant ones and no stranger", n) |
| + } |
| +} |
| + |
| +func TestClaim_RequiresSubject(t *testing.T) { |
| + h := ideartest.New(t) |
| + if _, err := h.Roster.Claim(h.Ctx(), " ", "nobody@example.test", ""); err == nil { |
| + t.Fatal("Claim with a blank subject succeeded; the subject is the join to the app's own identity") |
| + } |
| + if n := h.CountMembers(); n != 0 { |
| + t.Errorf("roster holds %d members after a refused claim, want 0", n) |
| + } |
| +} |
| + |
| +// --------------------------------------------------------------- Invite |
| + |
| +// TestInvite_RefusesOwnerRoleForEveryActor is the flat rule: no actor, |
| +// at any rank, on any path, mints an owner-role invitation. Ownership |
| +// moves only by Transfer. |
| +func TestInvite_RefusesOwnerRoleForEveryActor(t *testing.T) { |
| + h := ideartest.New(t) |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + member := h.Member(idear.RoleMember) |
| + |
| + for _, actor := range []*idear.Member{owner, admin, member} { |
| + _, _, err := h.Roster.Invite(h.Ctx(), actor, "new@example.test", idear.RoleOwner) |
| + if !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("a %s inviting at owner: err = %v, want ErrForbidden", actor.Role, err) |
| + } |
| + } |
| + if invs, err := h.Roster.PendingInvitations(h.Ctx()); err != nil { |
| + t.Fatalf("PendingInvitations: %v", err) |
| + } else if len(invs) != 0 { |
| + t.Errorf("%d invitations were minted despite every call being refused", len(invs)) |
| + } |
| +} |
| + |
| +func TestInvite_RankRules(t *testing.T) { |
| + h := ideartest.New(t) |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + member := h.Member(idear.RoleMember) |
| + gone := h.Deactivated(idear.RoleAdmin) |
| + |
| + cases := []struct { |
| + name string |
| + actor *idear.Member |
| + role idear.Role |
| + ok bool |
| + }{ |
| + {"owner invites admin", owner, idear.RoleAdmin, true}, |
| + {"owner invites member", owner, idear.RoleMember, true}, |
| + {"admin invites member", admin, idear.RoleMember, true}, |
| + // An admin who could mint a peer admin has escalated: MayActOn |
| + // refuses acting on an equal rank, so the new admin would be |
| + // beyond the granter's reach and every other admin's too. |
| + {"admin invites admin", admin, idear.RoleAdmin, false}, |
| + {"member invites member", member, idear.RoleMember, false}, |
| + // The actor is re-read inside the transaction, so a privilege |
| + // revoked a moment ago is revoked for this call too. |
| + {"deactivated admin invites member", gone, idear.RoleMember, false}, |
| + } |
| + for _, tc := range cases { |
| + t.Run(tc.name, func(t *testing.T) { |
| + _, _, err := h.Roster.Invite(h.Ctx(), tc.actor, "invitee@example.test", tc.role) |
| + if tc.ok && err != nil { |
| + t.Fatalf("Invite = %v, want success", err) |
| + } |
| + if !tc.ok && !errors.Is(err, idear.ErrForbidden) { |
| + t.Fatalf("Invite = %v, want ErrForbidden", err) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +func TestInvite_RejectsUnknownRole(t *testing.T) { |
| + h := ideartest.New(t) |
| + owner := h.Owner() |
| + _, _, err := h.Roster.Invite(h.Ctx(), owner, "invitee@example.test", idear.Role("superuser")) |
| + if !errors.Is(err, idear.ErrInvalidRole) { |
| + t.Fatalf("Invite at role \"superuser\" = %v, want ErrInvalidRole", err) |
| + } |
| +} |
| + |
| +// TestInvite_StoresOnlyTheHash pins the "hashed at rest" rule from the |
| +// design's §4: the plaintext exists in the return value and the emitted |
| +// link, and nowhere in the table. |
| +func TestInvite_StoresOnlyTheHash(t *testing.T) { |
| + h := ideartest.New(t) |
| + owner := h.Owner() |
| + |
| + inv, token, err := h.Roster.Invite(h.Ctx(), owner, "Invitee@Example.TEST", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if token == "" { |
| + t.Fatal("Invite returned an empty token") |
| + } |
| + stored := h.Invitation(inv.ID) |
| + if stored.TokenHash == token { |
| + t.Error("the plaintext token is what is stored; only its digest may be") |
| + } |
| + if strings.Contains(stored.TokenHash, token) || strings.Contains(stored.Email, token) { |
| + t.Error("the plaintext token appears in a stored column") |
| + } |
| + if stored.Email != "invitee@example.test" { |
| + t.Errorf("stored email = %q, want it normalised", stored.Email) |
| + } |
| + if stored.InvitedBy != owner.ID { |
| + t.Errorf("InvitedBy = %d, want the inviting owner %d", stored.InvitedBy, owner.ID) |
| + } |
| + // The digest is only demonstrably the RIGHT digest if the token |
| + // still redeems — asserting sha256(token) here would just restate |
| + // the implementation back to itself. |
| + if _, err := h.Roster.Accept(h.Ctx(), token, "invitee-subject", "Invitee"); err != nil { |
| + t.Fatalf("Accept with the returned token: %v", err) |
| + } |
| +} |
| + |
| +func TestInvite_ExpiresAtHonoursTTL(t *testing.T) { |
| + h := ideartest.NewWith(t, idear.Config{InviteTTL: time.Minute}) |
| + owner := h.Owner() |
| + |
| + before := time.Now().UTC() |
| + inv, _, err := h.Roster.Invite(h.Ctx(), owner, "invitee@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + after := time.Now().UTC() |
| + if inv.ExpiresAt.Before(before.Add(time.Minute)) || inv.ExpiresAt.After(after.Add(time.Minute)) { |
| + t.Errorf("ExpiresAt = %v, want about %v", inv.ExpiresAt, before.Add(time.Minute)) |
| + } |
| +} |
| + |
| +// --------------------------------------------------------------- Revoke |
| + |
| +func TestRevoke_KillsAPendingInvitation(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + inv, token, err := h.Roster.Invite(ctx, owner, "invitee@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if err := h.Roster.Revoke(ctx, owner, inv.ID); err != nil { |
| + t.Fatalf("Revoke: %v", err) |
| + } |
| + if h.Invitation(inv.ID).RevokedAt == nil { |
| + t.Error("RevokedAt is still NULL after Revoke") |
| + } |
| + if _, err := h.Roster.Accept(ctx, token, "invitee-subject", "Invitee"); !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Errorf("Accept of a revoked invitation = %v, want ErrNoInvitation", err) |
| + } |
| +} |
| + |
| +func TestRevoke_Refusals(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + member := h.Member(idear.RoleMember) |
| + |
| + inv, _, err := h.Roster.Invite(ctx, owner, "invitee@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + |
| + if err := h.Roster.Revoke(ctx, member, inv.ID); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("a member revoking = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.Revoke(ctx, owner, inv.ID+9999); !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Errorf("revoking an unknown id = %v, want ErrNoInvitation", err) |
| + } |
| + if err := h.Roster.Revoke(ctx, owner, inv.ID); err != nil { |
| + t.Fatalf("Revoke: %v", err) |
| + } |
| + if err := h.Roster.Revoke(ctx, owner, inv.ID); !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Errorf("revoking twice = %v, want ErrNoInvitation", err) |
| + } |
| +} |
| + |
| +// --------------------------------------------------------------- Accept |
| + |
| +func TestAccept_AdmitsAtTheInvitedRole(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + inv, token, err := h.Roster.Invite(ctx, owner, "invitee@example.test", idear.RoleAdmin) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + m, err := h.Roster.Accept(ctx, token, "invitee-subject", "Invitee") |
| + if err != nil { |
| + t.Fatalf("Accept: %v", err) |
| + } |
| + if m.Role != idear.RoleAdmin { |
| + t.Errorf("admitted at %q, want the invited role admin", m.Role) |
| + } |
| + if m.Email != "invitee@example.test" { |
| + t.Errorf("member email = %q, want the invitation's address", m.Email) |
| + } |
| + if h.Invitation(inv.ID).AcceptedAt == nil { |
| + t.Error("AcceptedAt is still NULL after a successful Accept") |
| + } |
| +} |
| + |
| +func TestAccept_Refusals(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + t.Run("unknown token", func(t *testing.T) { |
| + _, err := h.Roster.Accept(ctx, "not-a-token", "s1", "One") |
| + if !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Fatalf("err = %v, want ErrNoInvitation", err) |
| + } |
| + }) |
| + |
| + t.Run("empty token", func(t *testing.T) { |
| + _, err := h.Roster.Accept(ctx, "", "s2", "Two") |
| + if !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Fatalf("err = %v, want ErrNoInvitation", err) |
| + } |
| + }) |
| + |
| + t.Run("replay", func(t *testing.T) { |
| + _, token, err := h.Roster.Invite(ctx, owner, "replay@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if _, err := h.Roster.Accept(ctx, token, "replay-1", "One"); err != nil { |
| + t.Fatalf("first Accept: %v", err) |
| + } |
| + _, err = h.Roster.Accept(ctx, token, "replay-2", "Two") |
| + if !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Fatalf("replayed Accept = %v, want ErrNoInvitation", err) |
| + } |
| + if _, err := h.Roster.BySubject(ctx, "replay-2"); !errors.Is(err, idear.ErrNotFound) { |
| + t.Fatalf("the replayer was admitted anyway: %v", err) |
| + } |
| + }) |
| + |
| + t.Run("expired", func(t *testing.T) { |
| + inv, token, err := h.Roster.Invite(ctx, owner, "expired@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + h.Expire(inv.ID) |
| + _, err = h.Roster.Accept(ctx, token, "expired-subject", "Expired") |
| + if !errors.Is(err, idear.ErrNoInvitation) { |
| + t.Fatalf("Accept of an expired invitation = %v, want ErrNoInvitation", err) |
| + } |
| + if h.Invitation(inv.ID).AcceptedAt != nil { |
| + t.Error("the expired invitation was consumed by the refused Accept") |
| + } |
| + }) |
| + |
| + t.Run("blank subject", func(t *testing.T) { |
| + inv, token, err := h.Roster.Invite(ctx, owner, "blank@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if _, err := h.Roster.Accept(ctx, token, " ", "Blank"); err == nil { |
| + t.Fatal("Accept with a blank subject succeeded") |
| + } |
| + if h.Invitation(inv.ID).AcceptedAt != nil { |
| + t.Error("the invitation was consumed by the refused Accept") |
| + } |
| + }) |
| +} |
| + |
| +// TestAccept_RefusesAForgedOwnerInvitation is defence in depth. Invite |
| +// cannot mint one, so a row carrying role='owner' is corruption — and |
| +// corruption must not become a second Owner. |
| +func TestAccept_RefusesAForgedOwnerInvitation(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + inv, token, err := h.Roster.Invite(ctx, owner, "forged@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + // Forge it behind the store's back, the way a stray migration or a |
| + // hand-edited database would. |
| + if err := h.DB.G.Model(&idear.Invitation{}).Where("id = ?", inv.ID). |
| + Update("role", idear.RoleOwner).Error; err != nil { |
| + t.Fatalf("forging the row: %v", err) |
| + } |
| + |
| + if _, err := h.Roster.Accept(ctx, token, "forged-subject", "Forged"); !errors.Is(err, idear.ErrForbidden) { |
| + t.Fatalf("Accept of an owner-role invitation = %v, want ErrForbidden", err) |
| + } |
| + h.TheOwner() // still exactly one, and it is not the forger |
| + if _, err := h.Roster.BySubject(ctx, "forged-subject"); !errors.Is(err, idear.ErrNotFound) { |
| + t.Errorf("the forger was admitted: %v", err) |
| + } |
| + if h.Invitation(inv.ID).AcceptedAt != nil { |
| + t.Error("the forged invitation was consumed; the whole transaction should have rolled back") |
| + } |
| +} |
| + |
| +// TestAccept_ReturningMemberCollidesAndRollsBack pins the design's §4 |
| +// corollary: a deactivated member cannot be readmitted by a fresh |
| +// invitation, because Subject is unique and their row is still there. |
| +// Reactivate is the path — and the invitation must survive the failure |
| +// intact rather than being burnt by it. |
| +func TestAccept_ReturningMemberCollidesAndRollsBack(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + gone := h.Deactivated(idear.RoleMember) |
| + |
| + inv, token, err := h.Roster.Invite(ctx, owner, gone.Email, idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if _, err := h.Roster.Accept(ctx, token, gone.Subject, gone.Name); err == nil { |
| + t.Fatal("Accept re-admitted a deactivated subject; the unique index should have refused it") |
| + } |
| + if h.Invitation(inv.ID).AcceptedAt != nil { |
| + t.Error("the invitation was consumed by the failed Accept; the transaction did not roll back") |
| + } |
| + if h.Reload(gone.ID).Active() { |
| + t.Error("the failed Accept reactivated the dormant row") |
| + } |
| +} |
| + |
| +// -------------------------------------------------------------- SetRole |
| + |
| +func TestSetRole_RefusesOwner(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + |
| + // As the new role: ownership moves only by Transfer. |
| + if err := h.Roster.SetRole(ctx, owner, admin, idear.RoleOwner); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("SetRole(..., owner) = %v, want ErrForbidden", err) |
| + } |
| + if got := h.Reload(admin.ID).Role; got != idear.RoleAdmin { |
| + t.Errorf("target's role is now %q; a refused SetRole wrote anyway", got) |
| + } |
| + h.TheOwner() |
| + |
| + // As the target: the Owner's role changes only by Transfer either, |
| + // and this is an invariant rather than a permission — no rank can |
| + // do it, so ErrLastOwner is the honest answer, not ErrForbidden. |
| + if err := h.Roster.SetRole(ctx, admin, owner, idear.RoleMember); !errors.Is(err, idear.ErrLastOwner) { |
| + t.Errorf("SetRole demoting the owner = %v, want ErrLastOwner", err) |
| + } |
| + if got := h.Reload(owner.ID).Role; got != idear.RoleOwner { |
| + t.Errorf("owner's role is now %q", got) |
| + } |
| +} |
| + |
| +// TestSetRole_AuthorityFloorComesFirst pins the ORDER of the checks. A |
| +// plain Member aiming at the Owner must be refused for lacking |
| +// authority — ErrForbidden — and not handed ErrLastOwner, which would |
| +// answer "which row is the owner" to someone who had not established |
| +// they may ask anything at all. |
| +func TestSetRole_AuthorityFloorComesFirst(t *testing.T) { |
| + h := ideartest.New(t) |
| + owner := h.Owner() |
| + member := h.Member(idear.RoleMember) |
| + |
| + err := h.Roster.SetRole(h.Ctx(), member, owner, idear.RoleMember) |
| + if !errors.Is(err, idear.ErrForbidden) { |
| + t.Fatalf("a member demoting the owner = %v, want ErrForbidden", err) |
| + } |
| + if errors.Is(err, idear.ErrLastOwner) { |
| + t.Error("a member was told which row is the owner") |
| + } |
| +} |
| + |
| +func TestSetRole_Matrix(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + member := h.Member(idear.RoleMember) |
| + other := h.Member(idear.RoleMember) |
| + |
| + if err := h.Roster.SetRole(ctx, owner, member, idear.RoleAdmin); err != nil { |
| + t.Fatalf("owner promoting a member to admin: %v", err) |
| + } |
| + if got := h.Reload(member.ID).Role; got != idear.RoleAdmin { |
| + t.Errorf("role = %q, want admin", got) |
| + } |
| + |
| + // An admin minting a peer admin is an escalation: MayActOn refuses |
| + // acting on an equal rank, so the new admin is beyond reach. |
| + if err := h.Roster.SetRole(ctx, admin, other, idear.RoleAdmin); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("admin promoting to admin = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.SetRole(ctx, admin, other, idear.RoleMember); err != nil { |
| + t.Errorf("admin setting a member to member: %v", err) |
| + } |
| + if err := h.Roster.SetRole(ctx, admin, admin, idear.RoleMember); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("admin demoting themselves = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.SetRole(ctx, owner, other, idear.Role("root")); !errors.Is(err, idear.ErrInvalidRole) { |
| + t.Errorf("SetRole to \"root\" = %v, want ErrInvalidRole", err) |
| + } |
| + if err := h.Roster.SetRole(ctx, owner, &idear.Member{ID: 9999}, idear.RoleMember); !errors.Is(err, idear.ErrNotFound) { |
| + t.Errorf("SetRole on an unknown id = %v, want ErrNotFound", err) |
| + } |
| +} |
| + |
| +// ----------------------------------------------------------- Deactivate |
| + |
| +// TestDeactivate_NeverRemovesTheOwner is the invariant that keeps an |
| +// instance administrable. It is checked against the row inside the |
| +// transaction, which is what also makes Transfer racing Deactivate safe |
| +// from this side. |
| +func TestDeactivate_NeverRemovesTheOwner(t *testing.T) { |
| + h := ideartest.New(t) |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + |
| + if err := h.Roster.Deactivate(h.Ctx(), admin, owner); !errors.Is(err, idear.ErrLastOwner) { |
| + t.Fatalf("an admin deactivating the owner = %v, want ErrLastOwner", err) |
| + } |
| + if !h.Reload(owner.ID).Active() { |
| + t.Fatal("the owner was deactivated") |
| + } |
| + // The owner cannot do it to themselves either. |
| + if err := h.Roster.Deactivate(h.Ctx(), owner, owner); err == nil { |
| + t.Error("the owner deactivated themselves") |
| + } |
| + if !h.Reload(owner.ID).Active() { |
| + t.Error("the owner was deactivated") |
| + } |
| +} |
| + |
| +func TestDeactivate_Matrix(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + otherAdmin := h.Member(idear.RoleAdmin) |
| + member := h.Member(idear.RoleMember) |
| + |
| + if err := h.Roster.Deactivate(ctx, admin, member); err != nil { |
| + t.Fatalf("admin deactivating a member: %v", err) |
| + } |
| + if h.Reload(member.ID).Active() { |
| + t.Error("the member is still active") |
| + } |
| + // Idempotent: removing someone already removed is not an error. |
| + if err := h.Roster.Deactivate(ctx, admin, member); err != nil { |
| + t.Errorf("deactivating twice = %v, want nil", err) |
| + } |
| + if err := h.Roster.Deactivate(ctx, admin, otherAdmin); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("admin deactivating a peer admin = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.Deactivate(ctx, member, otherAdmin); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("a deactivated member deactivating anyone = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.Deactivate(ctx, owner, admin); err != nil { |
| + t.Errorf("owner deactivating an admin: %v", err) |
| + } |
| +} |
| + |
| +// ----------------------------------------------------------- Reactivate |
| + |
| +func TestReactivate_RestoresPriorAccessAndNoMore(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + gone := h.Deactivated(idear.RoleAdmin) |
| + |
| + if err := h.Roster.Reactivate(ctx, owner, gone); err != nil { |
| + t.Fatalf("Reactivate: %v", err) |
| + } |
| + back := h.Reload(gone.ID) |
| + if !back.Active() { |
| + t.Fatal("the member is still deactivated") |
| + } |
| + if back.Role != idear.RoleAdmin { |
| + t.Errorf("role after reactivation = %q, want the admin they were", back.Role) |
| + } |
| + // Idempotent. |
| + if err := h.Roster.Reactivate(ctx, owner, back); err != nil { |
| + t.Errorf("reactivating an active member = %v, want nil", err) |
| + } |
| + h.TheOwner() |
| +} |
| + |
| +func TestReactivate_Refusals(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + member := h.Member(idear.RoleMember) |
| + admin := h.Member(idear.RoleAdmin) |
| + goneAdmin := h.Deactivated(idear.RoleAdmin) |
| + |
| + if err := h.Roster.Reactivate(ctx, member, goneAdmin); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("a member reactivating = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.Reactivate(ctx, admin, goneAdmin); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("an admin reactivating a peer admin = %v, want ErrForbidden", err) |
| + } |
| + if h.Reload(goneAdmin.ID).Active() { |
| + t.Error("a refused Reactivate wrote anyway") |
| + } |
| +} |
| + |
| +// ------------------------------------------------------------- Transfer |
| + |
| +func TestTransfer_MovesOwnershipExactlyOnce(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + |
| + if err := h.Roster.Transfer(ctx, owner, admin); err != nil { |
| + t.Fatalf("Transfer: %v", err) |
| + } |
| + if got := h.TheOwner(); got.ID != admin.ID { |
| + t.Errorf("owner is member %d, want %d", got.ID, admin.ID) |
| + } |
| + if got := h.Reload(owner.ID).Role; got != idear.RoleAdmin { |
| + t.Errorf("outgoing owner's role = %q, want admin", got) |
| + } |
| + // The outgoing owner is no longer the owner, so they cannot |
| + // transfer again. |
| + if err := h.Roster.Transfer(ctx, owner, h.Member(idear.RoleMember)); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("a former owner transferring = %v, want ErrForbidden", err) |
| + } |
| + h.TheOwner() |
| +} |
| + |
| +func TestTransfer_Refusals(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + admin := h.Member(idear.RoleAdmin) |
| + gone := h.Deactivated(idear.RoleAdmin) |
| + |
| + if err := h.Roster.Transfer(ctx, admin, owner); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("a non-owner transferring = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.Transfer(ctx, owner, owner); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("transferring to oneself = %v, want ErrForbidden", err) |
| + } |
| + // The one that matters: a deactivated Owner is an instance nobody |
| + // can administer and nobody can be promoted into. |
| + if err := h.Roster.Transfer(ctx, owner, gone); !errors.Is(err, idear.ErrForbidden) { |
| + t.Errorf("transferring to a deactivated member = %v, want ErrForbidden", err) |
| + } |
| + if err := h.Roster.Transfer(ctx, owner, &idear.Member{ID: 9999}); !errors.Is(err, idear.ErrNotFound) { |
| + t.Errorf("transferring to an unknown id = %v, want ErrNotFound", err) |
| + } |
| + if got := h.TheOwner(); got.ID != owner.ID { |
| + t.Errorf("ownership moved despite every transfer being refused (now %d)", got.ID) |
| + } |
| +} |
| + |
| +// -------------------------------------------------------------- Lookups |
| + |
| +func TestBySubjectAndByID(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + gone := h.Deactivated(idear.RoleMember) |
| + |
| + got, err := h.Roster.BySubject(ctx, owner.Subject) |
| + if err != nil || got.ID != owner.ID { |
| + t.Fatalf("BySubject = (%v, %v), want the owner", got, err) |
| + } |
| + // Deactivated members resolve: the middleware has to tell a |
| + // dormant member from a stranger in order to log the difference, |
| + // even though it answers both with the app's 404. |
| + got, err = h.Roster.BySubject(ctx, gone.Subject) |
| + if err != nil { |
| + t.Fatalf("BySubject on a deactivated member = %v, want the row", err) |
| + } |
| + if got.Active() { |
| + t.Error("the deactivated member reads as active") |
| + } |
| + if _, err := h.Roster.BySubject(ctx, "nobody"); !errors.Is(err, idear.ErrNotFound) { |
| + t.Errorf("BySubject on a stranger = %v, want ErrNotFound", err) |
| + } |
| + // An empty subject is a request with no session, never a wildcard. |
| + if _, err := h.Roster.BySubject(ctx, " "); !errors.Is(err, idear.ErrNotFound) { |
| + t.Errorf("BySubject(\"\") = %v, want ErrNotFound", err) |
| + } |
| + if _, err := h.Roster.ByID(ctx, 9999); !errors.Is(err, idear.ErrNotFound) { |
| + t.Errorf("ByID on an unknown id = %v, want ErrNotFound", err) |
| + } |
| + if m, err := h.Roster.ByID(ctx, owner.ID); err != nil || m.Role != idear.RoleOwner { |
| + t.Errorf("ByID = (%v, %v), want the owner", m, err) |
| + } |
| +} |
| + |
| +func TestMembers_RankedAndIncludesDeactivated(t *testing.T) { |
| + h := ideartest.New(t) |
| + member := h.Member(idear.RoleMember) |
| + admin := h.Member(idear.RoleAdmin) |
| + owner := h.Owner() |
| + gone := h.Deactivated(idear.RoleMember) |
| + |
| + got, err := h.Roster.Members(h.Ctx()) |
| + if err != nil { |
| + t.Fatalf("Members: %v", err) |
| + } |
| + want := []int64{owner.ID, admin.ID, member.ID, gone.ID} |
| + if len(got) != len(want) { |
| + t.Fatalf("Members returned %d rows, want %d — deactivated members must be listed so they can be restored", len(got), len(want)) |
| + } |
| + for i, id := range want { |
| + if got[i].ID != id { |
| + t.Errorf("Members[%d].ID = %d, want %d (owner, then admins, then members, then by id)", i, got[i].ID, id) |
| + } |
| + } |
| +} |
| + |
| +func TestPendingInvitations_HidesSpentAndExpired(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + live, _, err := h.Roster.Invite(ctx, owner, "live@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + revoked, _, err := h.Roster.Invite(ctx, owner, "revoked@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if err := h.Roster.Revoke(ctx, owner, revoked.ID); err != nil { |
| + t.Fatalf("Revoke: %v", err) |
| + } |
| + accepted, token, err := h.Roster.Invite(ctx, owner, "accepted@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if _, err := h.Roster.Accept(ctx, token, "accepted-subject", "Accepted"); err != nil { |
| + t.Fatalf("Accept: %v", err) |
| + } |
| + expired, _, err := h.Roster.Invite(ctx, owner, "expired@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + h.Expire(expired.ID) |
| + |
| + got, err := h.Roster.PendingInvitations(ctx) |
| + if err != nil { |
| + t.Fatalf("PendingInvitations: %v", err) |
| + } |
| + if len(got) != 1 || got[0].ID != live.ID { |
| + ids := make([]int64, len(got)) |
| + for i, inv := range got { |
| + ids[i] = inv.ID |
| + } |
| + t.Fatalf("PendingInvitations returned %v, want only the live one (%d); revoked=%d accepted=%d expired=%d", |
| + ids, live.ID, revoked.ID, accepted.ID, expired.ID) |
| + } |
| +} |
| + |
| +// TestExpiryComparisonMatchesGo pins the assumption underneath Accept's |
| +// CAS, which is load-bearing and invisible. |
| +// |
| +// Timestamps reach SQLite as text, in the driver's own rendering of |
| +// time.Time.String(), in columns whose declared type is DATETIME. The |
| +// CAS asks the database "expires_at > ?", and that is a TEXT |
| +// comparison. It answers correctly only because the rendering is |
| +// fixed-width through the seconds and sorts lexicographically the way |
| +// the instants sort — including across the boundary where the |
| +// fractional part is trimmed away entirely. |
| +// |
| +// It would stop answering correctly the moment a stored value carried |
| +// a monotonic clock reading, because String() appends " m=+0.0001" to |
| +// those. That is why the store's clock is time.Now().UTC() and never |
| +// time.Now(): .UTC() strips the monotonic reading, and Add does not. |
| +func TestExpiryComparisonMatchesGo(t *testing.T) { |
| + h := ideartest.New(t) |
| + ctx := h.Ctx() |
| + owner := h.Owner() |
| + |
| + inv, _, err := h.Roster.Invite(ctx, owner, "clock@example.test", idear.RoleMember) |
| + if err != nil { |
| + t.Fatalf("Invite: %v", err) |
| + } |
| + if strings.Contains(inv.ExpiresAt.String(), "m=") { |
| + t.Fatalf("ExpiresAt carries a monotonic reading (%s); stored, it would break every expiry comparison", |
| + inv.ExpiresAt.String()) |
| + } |
| + |
| + base := h.Invitation(inv.ID).ExpiresAt |
| + for _, delta := range []time.Duration{ |
| + -time.Hour, -time.Second, -time.Millisecond, -time.Nanosecond, |
| + 0, time.Nanosecond, time.Millisecond, time.Second, time.Hour, |
| + } { |
| + probe := base.Add(delta) |
| + var n int64 |
| + if err := h.DB.G.Model(&idear.Invitation{}). |
| + Where("id = ? AND expires_at > ?", inv.ID, probe).Count(&n).Error; err != nil { |
| + t.Fatalf("counting at delta %v: %v", delta, err) |
| + } |
| + want := int64(0) |
| + if base.After(probe) { |
| + want = 1 |
| + } |
| + if n != want { |
| + t.Errorf("delta %v: SQL says %d row(s) with expires_at > probe, Go says %v", delta, n, want == 1) |
| + } |
| + } |
| + |
| + // And the trimmed-fraction boundary explicitly: a whole-second |
| + // timestamp renders without a fractional part at all, so the |
| + // character that follows the seconds changes from '.' to ' '. |
| + whole := base.Truncate(time.Second) |
| + if err := h.DB.G.Model(&idear.Invitation{}).Where("id = ?", inv.ID). |
| + Update("expires_at", whole).Error; err != nil { |
| + t.Fatalf("storing a whole-second expiry: %v", err) |
| + } |
| + for _, delta := range []time.Duration{-time.Nanosecond, time.Nanosecond} { |
| + probe := whole.Add(delta) |
| + var n int64 |
| + if err := h.DB.G.Model(&idear.Invitation{}). |
| + Where("id = ? AND expires_at > ?", inv.ID, probe).Count(&n).Error; err != nil { |
| + t.Fatalf("counting at delta %v: %v", delta, err) |
| + } |
| + want := int64(0) |
| + if whole.After(probe) { |
| + want = 1 |
| + } |
| + if n != want { |
| + t.Errorf("whole-second boundary, delta %v: SQL says %d, Go says %v", delta, n, want == 1) |
| + } |
| + } |
| +} |
diff --git a/schema_test.go b/schema_test.go| index 6772ca6..8317b0d 100644 |
| --- a/schema_test.go |
| +++ b/schema_test.go |
| @@ -89,6 +89,25 @@ func TestSchema_Apply_Twice(t *testing.T) { |
| // |
| // ==> THIS CONSTANT MAY NEVER BE UPDATED. <== |
| // |
| +// It was re-recorded ONCE, on 2026-08-24, during task 3 and before |
| +// idear had a v0.1.0 or a single consumer, because the migration as |
| +// task 2 wrote it did not work: the timestamp columns were declared |
| +// TEXT, and modernc.org/sqlite only decodes a text timestamp back into |
| +// a time.Time when the column's DECLARED type is DATE, DATETIME or |
| +// TIMESTAMP (rows.go, ColumnTypeDatabaseTypeName). Against TEXT |
| +// columns every GORM read of a Member or an Invitation failed with |
| +// "unsupported Scan, storing driver.Value type string into type |
| +// *time.Time" — the tables could be written and never read. DATETIME |
| +// is also what `rastrillo migration generate` emits for a GORM model's |
| +// time.Time (cmd/rastrillo/new.go), so this is the framework's own |
| +// spelling and not a local invention. |
| +// |
| +// That re-recording is the only one there will be. The rule this |
| +// comment states is about migrations some app has in its ledger; on |
| +// 2026-08-24 no ledger anywhere held this one. From the first tag |
| +// onwards the sentence above is literal, and a failure here means an |
| +// edit to revert — not a constant to refresh. |
| +// |
| // A failure here does not mean the constant is stale — it means an |
| // edit to a shipped migration file changed its checksum, and that |
| // edit must be reverted. Every app that has applied this migration has |
| @@ -98,7 +117,7 @@ func TestSchema_Apply_Twice(t *testing.T) { |
| // migration file beside it, with a new ID and a new entry here. See |
| // rastrillo's own migrate/frozen_checksums_test.go, which this test is |
| // deliberately shaped after. |
| -const frozenIdearChecksum = "61aeb6d8542eae10ca8d131f121f16252a395cb0af28f32972806d02c483dfd1" |
| +const frozenIdearChecksum = "78c47fc344a9c69a564ff91205713baef3f298f6e008d827ca763668ab51941c" |
| func TestSchema_FrozenChecksum(t *testing.T) { |
| for _, m := range idear.Schema.All() { |