c0134c0 · rastrillo/idear

diff --git a/.amadan/ci.d/35-test-peers b/.amadan/ci.d/35-test-peers

new file mode 100755

index 0000000..d9a435d

--- /dev/null

+++ b/.amadan/ci.d/35-test-peers

@@ -0,0 +1,2 @@

+#!/bin/sh

+exec make test-peers

diff --git a/Makefile b/Makefile

index 90f20ef..36bdbe9 100644

--- a/Makefile

+++ b/Makefile

@@ -1,4 +1,4 @@

-.PHONY: vet fmt-check test race ci

+.PHONY: vet fmt-check test test-peers 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

@@ -12,7 +12,7 @@

# change — through migrate.Generate, inside `test`. idear's own

# migrations are checked by schema_test.go, which is where a frozen

# checksum belongs.

-ci: vet fmt-check test race

+ci: vet fmt-check test test-peers race

vet:

go vet ./...

@@ -26,6 +26,16 @@ fmt-check:

test:

go test ./... -count=1

+# test-peers is the same suite built with the idear_peers tag, which

+# flips PeerGrants: reach extends to the actor's own rank. The policy

+# is a compile-time constant, so a build nobody compiles is a build

+# nobody tests — every mode-specific assertion states both answers by

+# hand, and this target is what makes the second one count. vet runs

+# too, because a file behind a build tag is invisible to plain vet.

+test-peers:

+ go vet -tags idear_peers ./...

+ go test -tags idear_peers ./... -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

diff --git a/SKILL.md b/SKILL.md

index 551caa0..a12dbe5 100644

--- a/SKILL.md

+++ b/SKILL.md

@@ -234,6 +234,14 @@ in `policy.go` — backs every mutation: actor active and at least Admin,

never the target, target strictly below actor's rank; nobody acts on

an Owner.

+**Peer grants** are a build-time switch: `go build -tags idear_peers`

+widens that last rule to "at or below", for granting and managing

+alike — an Admin may then invite, promote, demote, remove and restore

+other Admins. The Owner is untouched either way: never granted (only

+`Transfer`), never acted on. `idear.PeerGrants` reports which build

+you are in. Every binary and every `go test` must carry the same tag,

+or the app runs one policy and tests another.

+

Every mutation is one transaction with its invariant enforced inside it:

```go

@@ -301,9 +309,9 @@ one.

never go in `Models`.** Mixed in, `check` is permanently red and

`generate` collides a second `CREATE TABLE idear_members`. Guard:

`migrate.Generate(ctx, Schema.All(), Models)` zero changes (§7).

-- **An Admin may grant only Member** — `checkInviteRole` refuses

- anything not strictly below the poster's rank; build selectors from

- `MembersPage.Grantable`, never the three constants.

+- **An Admin may grant only Member** (Admin too under `idear_peers`) —

+ `checkInviteRole` refuses anything beyond the poster's reach; build

+ selectors from `MembersPage.Grantable`, never the three constants.

- **Set `Site` in production** — the default is the request's `Host`,

client-supplied, on a public unauthenticated page.

- **Set `ClientKey` behind a proxy**, or every request shares the

@@ -313,7 +321,7 @@ one.

General discipline: never bind a form onto a struct (idear's own carry

`Role`/`DeactivatedAt`) — read named fields from `PostForm`, not

`Form`. `role` never comes from a form you build; `checkInviteRole`'s

-strictly-below rule is what makes idear's own reads safe. 404, never

+reach rule is what makes idear's own reads safe. 404, never

403, for a non-member. Allow-lists, not escaping, for an `ORDER BY` or

a `style` attribute — derive test payloads from the list under test.

Hiding a control is never the enforcement — the store refuses again

diff --git a/example/app_test.go b/example/app_test.go

index 602522d..3af4dcd 100644

--- a/example/app_test.go

+++ b/example/app_test.go

@@ -298,7 +298,8 @@ func TestSignUpClaimInviteAcceptMembersRoleChange(t *testing.T) {

want(t, kim.get("/members"), http.StatusOK, "admin GET /members")

// The Admin invites a plain Member. checkInviteRole is

- // strictly-below, so this is the only role an Admin may grant.

+ // strictly-below, so this is the only role an Admin may grant —

+ // unless idear was built with PeerGrants.

res = kim.post("/members/invitations", url.Values{

"email": {"sam@example.test"}, "role": {"member"},

})

@@ -330,11 +331,21 @@ func TestSignUpClaimInviteAcceptMembersRoleChange(t *testing.T) {

// THE ROLE CHANGE. The Admin cannot make a peer: idear refuses a

// grant that is not strictly below the granter's rank, and it does

- // so at 403 rather than silently doing nothing.

+ // so at 403 rather than silently doing nothing. Built with

+ // PeerGrants it can, and the demotion back is allowed too.

res = kim.post("/members/"+strconv.FormatInt(samID, 10)+"/role", url.Values{"role": {"admin"}})

- want(t, res, http.StatusForbidden, "admin promotes to admin")

+ if idear.PeerGrants {

+ want(t, res, http.StatusSeeOther, "admin promotes to admin under PeerGrants")

+ if got := ta.member("sam@example.test"); got.Role != idear.RoleAdmin {

+ t.Fatalf("peer promotion did not land: role is %q", got.Role)

+ }

+ res = kim.post("/members/"+strconv.FormatInt(samID, 10)+"/role", url.Values{"role": {"member"}})

+ want(t, res, http.StatusSeeOther, "admin demotes a peer admin under PeerGrants")

+ } else {

+ want(t, res, http.StatusForbidden, "admin promotes to admin")

+ }

if got := ta.member("sam@example.test"); got.Role != idear.RoleMember {

- t.Fatalf("refused promotion still landed: role is %q", got.Role)

+ t.Fatalf("sam is %q, want member", got.Role)

}

// The Owner can.

diff --git a/example/pages/members.html b/example/pages/members.html

index 9ef19a4..23204bc 100644

--- a/example/pages/members.html

+++ b/example/pages/members.html

@@ -15,11 +15,11 @@

{{/* The role selector is built from Grantable, never from the three

role constants. An Admin may grant only Member — idear refuses a

- grant that is not strictly below the granter's own rank — so a

- selector offering Admin to an Admin would 403 on every submit,

- and one offering Owner would 403 for everybody including the

- Owner. Hiding a control is never the enforcement; the store

- refuses either way. */}}

+ grant that is not strictly below the granter's own rank, unless

+ built with PeerGrants — so a selector offering Admin to an Admin

+ would 403 on every submit, and one offering Owner would 403 for

+ everybody including the Owner. Hiding a control is never the

+ enforcement; the store refuses either way. */}}

diff --git a/example/render.go b/example/render.go

index 50c996e..66a2661 100644

--- a/example/render.go

+++ b/example/render.go

@@ -26,8 +26,9 @@ var pagesFS embed.FS

//

// Asking the real predicate rather than writing "is the viewer an

// admin" here is the point: the rule (nobody acts on an Owner, nobody

-// acts on an equal rank, nobody acts on themselves) lives in one

-// place, and a template that restated it would drift from it.

+// acts on an equal rank unless built with PeerGrants, nobody acts on

+// themselves) lives in one place, and a template that restated it

+// would drift from it.

var funcs = template.FuncMap{

"mayActOn": func(actor *idear.Member, target idear.Member) bool {

return idear.MayActOn(actor, &target) == nil

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

index f4e990b..ec1b55a 100644

--- a/handlers.go

+++ b/handlers.go

@@ -51,9 +51,10 @@ const flashError = "error"

// Grantable is the roles the VIEWER may hand out, and a role selector

// must be built from it rather than from the three constants. An Admin

// may grant only Member — checkInviteRole refuses a grant that is not

-// STRICTLY BELOW the actor's own rank — so a form that offered Admin

-// to an Admin would 403 on every submit, and one that offered Owner

-// would 403 for everybody including the Owner. It is computed by

+// STRICTLY BELOW the actor's own rank, unless built with PeerGrants —

+// so a form that offered Admin to an Admin would 403 on every submit,

+// and one that offered Owner would 403 for everybody including the

+// Owner. It is computed by

// Grantable, which asks the same predicate the store enforces, so the

// selector cannot drift from the rule.

type MembersPage struct {

@@ -323,9 +324,10 @@ func (h *Handlers) Members(w http.ResponseWriter, r *http.Request) {

//

// The role IS read from the form here, and that is safe for exactly

// one reason: checkInviteRole refuses RoleOwner outright, for every

-// actor including the Owner, and refuses anything not strictly below

-// the actor's own rank. A posted role can therefore only ever be worth

-// LESS than the poster already holds. Every other field is read by

+// actor including the Owner, and refuses anything beyond the actor's

+// reach. A posted role can therefore only ever be worth LESS than the

+// poster already holds — or, built with PeerGrants, as much and never

+// more. Every other field is read by

// name too — there is no struct binding anywhere in this file, so a

// field nobody named cannot arrive.

func (h *Handlers) Invite(w http.ResponseWriter, r *http.Request) {

diff --git a/handlers_test.go b/handlers_test.go

index 22fac2b..002a53b 100644

--- a/handlers_test.go

+++ b/handlers_test.go

@@ -343,7 +343,8 @@ func TestMemberIsRefusedEveryManagementAction(t *testing.T) {

}

// ---------------------------------------------------------------

-// §7.3 — an Admin cannot touch an Admin or the Owner.

+// §7.3 — an Admin cannot touch an Admin or the Owner. Under

+// PeerGrants the Admin half is lifted; the Owner half never is.

// ---------------------------------------------------------------

func TestAdminCannotActOnAdminOrOwner(t *testing.T) {

@@ -356,12 +357,15 @@ func TestAdminCannotActOnAdminOrOwner(t *testing.T) {

// Every target an Admin may not act on, including themselves —

// self-management is how an instance ends up with nobody able to

- // administer it.

+ // administer it. Peer admins are on the list only in the default

+ // build; TestAdminActsOnPeerAdmin covers them under PeerGrants.

targets := map[string]*idear.Member{

- "the owner": owner,

- "a peer admin": peer,

- "a deactivated admin": goneAdmin,

- "themselves": actor,

+ "the owner": owner,

+ "themselves": actor,

+ }

+ if !idear.PeerGrants {

+ targets["a peer admin"] = peer

+ targets["a deactivated admin"] = goneAdmin

}

// The three target-shaped mutations, derived from the route table

// rather than listed: every management route that carries an {id}.

@@ -426,6 +430,68 @@ func TestAdminCannotActOnAdminOrOwner(t *testing.T) {

}

+// TestAdminActsOnPeerAdmin walks one Admin through the whole peer

+// lifecycle over HTTP — invite an Admin, promote a Member to Admin,

+// demote them, remove them, restore them — and states the answer for

+// each build by hand: every step 403 in the default build, every step

+// landing under PeerGrants.

+func TestAdminActsOnPeerAdmin(t *testing.T) {

+ app, _ := newApp(t)

+ actor := app.H.Member(idear.RoleAdmin)

+ peer := app.H.Member(idear.RoleMember)

+ c := app.As(actor)

+

+ want := http.StatusForbidden

+ if idear.PeerGrants {

+ want = http.StatusSeeOther

+ }

+ steps := []struct {

+ path string

+ form url.Values

+ }{

+ {"/members/invitations", url.Values{"email": {"peer@example.test"}, "role": {string(idear.RoleAdmin)}}},

+ {fmt.Sprintf("/members/%d/role", peer.ID), url.Values{"role": {string(idear.RoleAdmin)}}},

+ {fmt.Sprintf("/members/%d/role", peer.ID), url.Values{"role": {string(idear.RoleMember)}}},

+ {fmt.Sprintf("/members/%d/role", peer.ID), url.Values{"role": {string(idear.RoleAdmin)}}},

+ {fmt.Sprintf("/members/%d/remove", peer.ID), url.Values{}},

+ {fmt.Sprintf("/members/%d/restore", peer.ID), url.Values{}},

+ }

+ for i, st := range steps {

+ // In the default build step 2 is refused, so the peer is still

+ // a Member at steps 3-6 and within reach after all. Owner-made

+ // promotion puts them where the refusals are meaningful.

+ if i == 2 && !idear.PeerGrants {

+ if err := app.H.Roster.SetRole(app.H.Ctx(), app.H.Owner(), peer, idear.RoleAdmin); err != nil {

+ t.Fatalf("the owner promoting the peer: %v", err)

+ }

+ res := c.Post(st.path, st.form)

+ if res.Status != want {

+ t.Errorf("step %d: admin POST %s %v → %d, want %d; body %q", i, st.path, st.form, res.Status, want, res.Body)

+ }

+

+ // Both builds end with the peer an active Admin: under PeerGrants

+ // by the admin's own last promote and restore, in the default build

+ // because the owner promoted them and every admin step bounced.

+ if got := app.H.Reload(peer.ID); got.Role != idear.RoleAdmin || !got.Active() {

+ t.Errorf("the peer is now %s/%v, want an active admin", got.Role, got.Active())

+ }

+ invs, err := app.H.Roster.PendingInvitations(app.H.Ctx())

+ if err != nil {

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

+ }

+ var adminInvites int

+ for _, inv := range invs {

+ if inv.Role == idear.RoleAdmin {

+ adminInvites++

+ }

+ if wantInvites := map[bool]int{false: 0, true: 1}[idear.PeerGrants]; adminInvites != wantInvites {

+ t.Errorf("%d admin invitations pending, want %d", adminInvites, wantInvites)

+ }

+}

+

// ---------------------------------------------------------------

// §7.4 — a posted role=owner never lands, on any path, for any actor.

// ---------------------------------------------------------------

@@ -1338,8 +1404,8 @@ func TestInvitationPageDoesNotDiscloseTheAddress(t *testing.T) {

// checkInviteRole, the test asks Grantable — so relaxing the rule

// moves the expectation with it: changing checkInviteRole's

// `rank(role) >= rank(actor.Role)` to `>`, which lets an Admin mint a

-// PEER ADMIN and puts the new admin beyond every other admin's reach,

-// left every test in this file green. That is the brief's anti-pattern

+// PEER ADMIN while MayActOn still puts the new admin beyond every

+// other admin's reach, left every test in this file green. That is the brief's anti-pattern

// in mirror form: total derivation buys drift-resistance and loses

// rule-change detection.

//

@@ -1349,6 +1415,10 @@ func TestInvitationPageDoesNotDiscloseTheAddress(t *testing.T) {

// actor's", "ownership moves only by Transfer" — and compares it in

// BOTH directions, exactly as TestRoutesMatchTheDesign does for the

// route table. It quotes the spec, never the implementation.

+//

+// PeerGrants is the one sanctioned departure from §5, and it changes

+// one row: an Admin may also grant Admin. That row is stated by hand

+// too, not derived from the constant's value.

func TestGrantableMatchesTheDesign(t *testing.T) {

design := map[idear.Role][]idear.Role{

// An Owner may grant Admin and Member. Not Owner: ownership

@@ -1362,6 +1432,12 @@ func TestGrantableMatchesTheDesign(t *testing.T) {

// A Member manages nothing.

idear.RoleMember: nil,

}

+ if idear.PeerGrants {

+ // Reach extends to the actor's own rank, for granting and

+ // managing alike, so the new Admin is within every Admin's

+ // reach. The Owner's row is unchanged: owner is never granted.

+ design[idear.RoleAdmin] = []idear.Role{idear.RoleAdmin, idear.RoleMember}

+ }

for actorRole, want := range design {

actor := &idear.Member{ID: 1, Role: actorRole}

diff --git a/peers.go b/peers.go

new file mode 100644

index 0000000..cbfa0c5

--- /dev/null

+++ b/peers.go

@@ -0,0 +1,28 @@

+//go:build !idear_peers

+

+package idear

+

+// PeerGrants reports how far an actor's authority reaches, and it is

+// fixed at COMPILE TIME by the idear_peers build tag rather than read

+// from Config.

+//

+// Without the tag (this file) it is false: an actor grants and manages

+// only ranks STRICTLY BELOW their own. An Admin invites, promotes,

+// demotes and removes Members, and never another Admin.

+//

+// Built with -tags idear_peers (peers_on.go) it is true: the reach

+// extends to the actor's OWN rank. An Admin may then invite or promote

+// someone to Admin, and may demote, remove and restore other Admins.

+//

+// The Owner is the same in both modes, because ownership is not a rank

+// that has peers — there is exactly one. Nobody is ever granted

+// RoleOwner (Transfer is the only route), and nobody at any rank acts

+// on the Owner. See reaches.

+//

+// It is a build tag and not a Config field because MayActOn and

+// Grantable are pure package functions an app calls with no Roster in

+// hand — to render a members page, say — and they must answer exactly

+// as the store will. A per-Roster setting would give them a second

+// source of truth to disagree with. It is exported so an app can

+// assert the mode it was built in, or word its own copy to match.

+const PeerGrants = false

diff --git a/peers_on.go b/peers_on.go

new file mode 100644

index 0000000..72e562f

--- /dev/null

+++ b/peers_on.go

@@ -0,0 +1,8 @@

+//go:build idear_peers

+

+package idear

+

+// PeerGrants is true: this binary was built with -tags idear_peers, so

+// an actor grants and manages ranks up to AND INCLUDING their own. The

+// Owner is still never granted and never acted on. See peers.go.

+const PeerGrants = true

diff --git a/policy.go b/policy.go

index 657ab16..61793b3 100644

--- a/policy.go

+++ b/policy.go

@@ -15,7 +15,7 @@ var ErrForbidden = errors.New("idear: forbidden")

// full role matrix can be exhausted in a table test with no database

// and no HTTP server.

//

-// Four rules, in order, each closing a specific hole:

+// Five rules, in order, each closing a specific hole:

//

// 1. actor must be non-nil and active — a deactivated admin keeps

// their row (removal is never a delete) but loses every privilege

@@ -25,10 +25,14 @@ var ErrForbidden = errors.New("idear: forbidden")

// 3. actor and target must be different people, compared by ID — an

// Owner deactivating or demoting themselves is exactly how an

// instance ends up with no one able to administer it.

-// 4. target's rank must be strictly below actor's — Admin manages

-// Member only, Owner manages Admin and Member, and nobody at any

-// rank manages an Owner. Equal rank is refused, not just higher

-// rank: two Owners or two Admins may never act on one another.

+// 4. target must not be the Owner — nobody at any rank manages an

+// Owner, in either PeerGrants mode. Ownership moves only by

+// Transfer.

+// 5. target must be within actor's reach (see reaches). By default

+// that is strictly below: Admin manages Member only, Owner manages

+// Admin and Member, and two Admins may never act on one another.

+// Built with PeerGrants, equal rank is within reach too, so an

+// Admin manages other Admins.

func MayActOn(actor, target *Member) error {

if err := mayManage(actor); err != nil {

return err

@@ -39,12 +43,33 @@ func MayActOn(actor, target *Member) error {

if actor.ID == target.ID {

return fmt.Errorf("%w: cannot act on self", ErrForbidden)

}

- if rank(target.Role) >= rank(actor.Role) {

- return fmt.Errorf("%w: target's rank is not below actor's", ErrForbidden)

+ if target.Role == RoleOwner {

+ return fmt.Errorf("%w: nobody acts on the owner", ErrForbidden)

+ }

+ if !reaches(actor.Role, target.Role) {

+ return fmt.Errorf("%w: target's rank is beyond actor's reach", ErrForbidden)

}

return nil

}

+// reaches is the ONE rank comparison behind both halves of authority:

+// who an actor may manage (MayActOn) and what role they may grant

+// (checkInviteRole). Keeping them on one predicate is what stops the

+// two drifting apart — an actor who could grant a rank they could not

+// then manage would mint someone beyond their own reach, and one who

+// could manage a rank they could not grant would be left demoting

+// people they could never promote back.

+//

+// Strictly below by default; at or below when built with PeerGrants.

+// It says nothing about RoleOwner — both callers refuse that

+// outright, before asking.

+func reaches(actor, other Role) bool {

+ if PeerGrants {

+ return rank(other) <= rank(actor)

+ }

+ return rank(other) < rank(actor)

+}

+

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

diff --git a/policy_test.go b/policy_test.go

index ae64c84..ebca144 100644

--- a/policy_test.go

+++ b/policy_test.go

@@ -24,6 +24,10 @@ func member(id int64, role Role, deactivated bool) *Member {

// from rank() or AtLeast() — a test that re-derives the implementation's

// own comparison proves only that the comparison equals itself, which

// is exactly the bug round 1's auditor found in the allow-list tests.

+//

+// Each cell carries TWO hand-written answers, one per build: the

+// default, and -tags idear_peers. `make ci` runs the suite under both,

+// so neither column is ever unexercised.

func TestMayActOn_Matrix(t *testing.T) {

// Distinct ids on both sides so "different person" cases are never

// accidentally also self-action cases.

@@ -39,42 +43,56 @@ func TestMayActOn_Matrix(t *testing.T) {

deactivatedAdmin := member(2, RoleAdmin, true)

cases := []struct {

- name string

- actor *Member

- target *Member

- wantErr bool

+ name string

+ actor *Member

+ target *Member

+ // Whether MayActOn refuses: in the default build, and in a

+ // build with PeerGrants.

+ refused, refusedPeers bool

}{

// The full 3x3 actor/target matrix, actor and target always

- // distinct people.

- {"owner acts on other owner", activeOwner, otherOwner, true},

- {"owner acts on admin", activeOwner, otherAdmin, false},

- {"owner acts on member", activeOwner, otherMember, false},

- {"admin acts on owner", activeAdmin, otherOwner, true},

- {"admin acts on other admin", activeAdmin, otherAdmin, true},

- {"admin acts on member", activeAdmin, otherMember, false},

- {"member acts on owner", activeMember, otherOwner, true},

- {"member acts on admin", activeMember, otherAdmin, true},

- {"member acts on other member", activeMember, otherMember, true},

+ // distinct people. Peer grants change exactly one cell: an

+ // Admin acting on another Admin. An Owner acting on another

+ // Owner stays refused — nobody acts on the Owner, whichever

+ // build, and there is only ever one to act on.

+ {"owner acts on other owner", activeOwner, otherOwner, true, true},

+ {"owner acts on admin", activeOwner, otherAdmin, false, false},

+ {"owner acts on member", activeOwner, otherMember, false, false},

+ {"admin acts on owner", activeAdmin, otherOwner, true, true},

+ {"admin acts on other admin", activeAdmin, otherAdmin, true, false},

+ {"admin acts on member", activeAdmin, otherMember, false, false},

+ {"member acts on owner", activeMember, otherOwner, true, true},

+ {"member acts on admin", activeMember, otherAdmin, true, true},

+ {"member acts on other member", activeMember, otherMember, true, true},

+

+ // A deactivated Admin is still an Admin-ranked target: within

+ // a peer's reach under PeerGrants, beyond it otherwise.

+ {"admin acts on deactivated admin", activeAdmin, member(21, RoleAdmin, true), true, false},

// Self-action: refused for every rank, including the Owner.

- {"owner acts on self", activeOwner, member(1, RoleOwner, false), true},

- {"admin acts on self", activeAdmin, member(2, RoleAdmin, false), true},

- {"member acts on self", activeMember, member(3, RoleMember, false), true},

+ {"owner acts on self", activeOwner, member(1, RoleOwner, false), true, true},

+ {"admin acts on self", activeAdmin, member(2, RoleAdmin, false), true, true},

+ {"member acts on self", activeMember, member(3, RoleMember, false), true, true},

// Inactive actor: refused regardless of rank, before rank is

// even considered.

- {"deactivated owner acts on member", deactivatedOwner, otherMember, true},

- {"deactivated admin acts on member", deactivatedAdmin, otherMember, true},

+ {"deactivated owner acts on member", deactivatedOwner, otherMember, true, true},

+ {"deactivated admin acts on member", deactivatedAdmin, otherMember, true, true},

+ {"deactivated admin acts on admin", deactivatedAdmin, otherAdmin, true, true},

// Defensive: a nil actor or nil target can never be permitted.

- {"nil actor", nil, otherMember, true},

- {"nil target", activeOwner, nil, true},

+ {"nil actor", nil, otherMember, true, true},

+ {"nil target", activeOwner, nil, true, true},

}

for _, c := range cases {

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

err := MayActOn(c.actor, c.target)

- if c.wantErr {

+ wantErr := c.refused

+ if PeerGrants {

+ wantErr = c.refusedPeers

+ }

+ if wantErr {

if err == nil {

t.Fatalf("MayActOn(%v, %v) = nil, want ErrForbidden", c.actor, c.target)

}

diff --git a/roster.go b/roster.go

index a8b1671..bbd2c58 100644

--- a/roster.go

+++ b/roster.go

@@ -253,13 +253,17 @@ func forbidden(format string, args ...any) error {

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

+// role must be within the actor's REACH — the same predicate MayActOn

+// uses to decide whom they may manage (see reaches). By default that

+// is STRICTLY BELOW: an Admin may invite or set only Member, because

+// an Admin who could mint a peer Admin would put the new Admin 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.

+//

+// Built with PeerGrants, reach includes the actor's own rank, for

+// granting and managing alike — so an Admin may mint an Admin

+// precisely because they, and every other Admin, may then manage one.

func checkInviteRole(actor *Member, role Role) error {

if !role.Valid() {

return fmt.Errorf("%w: %q", ErrInvalidRole, string(role))

@@ -267,7 +271,7 @@ func checkInviteRole(actor *Member, role Role) error {

if role == RoleOwner {

return forbidden("ownership moves only by Transfer, never by grant")

}

- if rank(role) >= rank(actor.Role) {

+ if !reaches(actor.Role, role) {

return forbidden("a %s may not grant %s", actor.Role, role)

}

return nil

@@ -370,9 +374,10 @@ func (rs *Roster) Claim(ctx context.Context, subject, email, name string) (*Memb

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

+// be within actor's reach — strictly below their rank, or up to and

+// including it under PeerGrants; 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.

//

// RE-INVITING SUPERSEDES. Any invitation for the same address that is

// still unaccepted and unrevoked is REVOKED in this same transaction,

@@ -746,10 +751,13 @@ func (rs *Roster) SetRole(ctx context.Context, actor, target *Member, role Role)

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

+ // run confirmed. RoleOwner is refused twice regardless of

+ // build (the early guard, and checkInviteRole's own owner

+ // branch) and in the default build a third time, by the

+ // strictly-below rank rule no actor can clear for owner.

+ // Under PeerGrants the Owner's rank DOES reach owner, so the

+ // first two are the whole defence there — which is why

+ // neither may go.

if err := checkInviteRole(cur, role); err != nil {

return err

}

diff --git a/roster_test.go b/roster_test.go

index db5274e..b716788 100644

--- a/roster_test.go

+++ b/roster_test.go

@@ -220,27 +220,37 @@ func TestInvite_RankRules(t *testing.T) {

name string

actor *idear.Member

role idear.Role

- ok bool

+ // Whether Invite succeeds: in the default build, and in a

+ // build with PeerGrants.

+ ok, okPeers bool

}{

- {"owner invites admin", owner, idear.RoleAdmin, true},

- {"owner invites member", owner, idear.RoleMember, true},

- {"admin invites member", admin, idear.RoleMember, true},

+ {"owner invites admin", owner, idear.RoleAdmin, true, true},

+ {"owner invites member", owner, idear.RoleMember, true, true},

+ {"admin invites member", admin, idear.RoleMember, true, 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},

+ // Under PeerGrants MayActOn allows equal rank, so the same

+ // grant leaves the new admin within every admin's reach.

+ {"admin invites admin", admin, idear.RoleAdmin, false, true},

+ // Ownership is never granted, peers or not.

+ {"owner invites owner", owner, idear.RoleOwner, false, false},

+ {"member invites member", member, idear.RoleMember, false, 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},

+ {"deactivated admin invites member", gone, idear.RoleMember, false, false},

}

for _, tc := range cases {

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

+ ok := tc.ok

+ if idear.PeerGrants {

+ ok = tc.okPeers

+ }

_, _, err := h.Roster.Invite(h.Ctx(), tc.actor, "invitee@example.test", tc.role)

- if tc.ok && err != nil {

+ if ok && err != nil {

t.Fatalf("Invite = %v, want success", err)

}

- if !tc.ok && !errors.Is(err, idear.ErrForbidden) {

+ if !ok && !errors.Is(err, idear.ErrForbidden) {

t.Fatalf("Invite = %v, want ErrForbidden", err)

}

})

@@ -689,13 +699,34 @@ func TestSetRole_Matrix(t *testing.T) {

}

// 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) {

+ // acting on an equal rank, so the new admin is beyond reach. Under

+ // PeerGrants it is allowed — and so is the demotion straight back,

+ // which is what keeps the new admin within reach.

+ err := h.Roster.SetRole(ctx, admin, other, idear.RoleAdmin)

+ if idear.PeerGrants {

+ if err != nil {

+ t.Errorf("admin promoting to admin under PeerGrants: %v", err)

+ }

+ if got := h.Reload(other.ID).Role; got != idear.RoleAdmin {

+ t.Errorf("role = %q, want admin", got)

+ }

+ if err := h.Roster.SetRole(ctx, admin, other, idear.RoleMember); err != nil {

+ t.Errorf("admin demoting a peer admin under PeerGrants: %v", err)

+ }

+ } else if !errors.Is(err, idear.ErrForbidden) {

t.Errorf("admin promoting to admin = %v, want ErrForbidden", err)

}

+ if got := h.Reload(other.ID).Role; got != idear.RoleMember {

+ t.Errorf("role = %q, want member", got)

+ }

if err := h.Roster.SetRole(ctx, admin, other, idear.RoleMember); err != nil {

t.Errorf("admin setting a member to member: %v", err)

}

+ // Ownership is never granted, not even by the Owner, whose own

+ // rank reaches owner under PeerGrants.

+ if err := h.Roster.SetRole(ctx, owner, other, idear.RoleOwner); !errors.Is(err, idear.ErrForbidden) {

+ t.Errorf("owner granting owner = %v, want ErrForbidden", 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)

}

@@ -751,7 +782,13 @@ func TestDeactivate_Matrix(t *testing.T) {

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) {

+ err := h.Roster.Deactivate(ctx, admin, otherAdmin)

+ switch {

+ case idear.PeerGrants && err != nil:

+ t.Errorf("admin deactivating a peer admin under PeerGrants: %v", err)

+ case idear.PeerGrants && h.Reload(otherAdmin.ID).Active():

+ t.Error("the peer admin is still active")

+ case !idear.PeerGrants && !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) {

@@ -797,7 +834,23 @@ func TestReactivate_Refusals(t *testing.T) {

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) {

+ if h.Reload(goneAdmin.ID).Active() {

+ t.Error("a refused Reactivate wrote anyway")

+ }

+

+ // A peer admin is refused in the default build, and reachable —

+ // so restorable — under PeerGrants.

+ err := h.Roster.Reactivate(ctx, admin, goneAdmin)

+ if idear.PeerGrants {

+ if err != nil {

+ t.Errorf("an admin reactivating a peer admin under PeerGrants: %v", err)

+ }

+ if !h.Reload(goneAdmin.ID).Active() {

+ t.Error("the peer admin is still deactivated")

+ }

+ return

+ }

+ if !errors.Is(err, idear.ErrForbidden) {

t.Errorf("an admin reactivating a peer admin = %v, want ErrForbidden", err)

}

if h.Reload(goneAdmin.ID).Active() {

rastrillo / idear Public

Merge branch 'peer-role-grants' into main