rastrillo / idear Public

Admins may grant and manage their own rank when built with idear_peers

Until now every actor's reach stopped strictly below their own rank:
an Admin could invite, promote, demote and remove Members, and never
another Admin. Some instances want Admins to be able to make Admins.

The switch is a build tag, not a Config field. MayActOn and Grantable
are pure package functions an app calls with no Roster in hand (the
example's members page does exactly that), and they must answer the
way the store will; a per-Roster setting would give them a second
source of truth to drift from. Built with -tags idear_peers, the
exported const PeerGrants is true and reach extends to the actor's own
rank.

Granting and managing move together. Both now ask one predicate,
reaches, so an Admin who may mint an Admin may also demote, remove and
restore one. Moving only the grant half would mint Admins beyond every
Admin's reach, which is the escalation the old comment warned about.

The Owner is the same in both builds. Ownership is a singleton, not a
rank with peers: RoleOwner is still never granted (Transfer only), and
MayActOn now refuses an Owner target as an explicit rule rather than
leaning on "nothing ranks above owner", which stops being true for the
Owner's own rank once equal rank is within reach.

Human

Prompt

I want to be able to toggle the behaviour that controls
whether or not a team member can promote/demote other users. Right
now, they can only elevate users to a role that's _lower_ than theirs.
I'd like to be able to toggle between that behaviour, and being able
to elevate users to roles up to _and including_ their role. So
"admins" can promote other people to be "admins". I don't know if
"owner" needs any special treatment here -- happy to take your advice.
I'm happy for this behaviour to get baked in at compile time, if that
makes sense.

From this commit's own message. The prompt ledger has no entries for it — which is what a squashed, rebased or amended commit looks like, and what every commit looks like in a repo without a ledger.

Summary

adds PeerGrants (build tag idear_peers) and a shared reaches
predicate behind MayActOn and checkInviteRole; every mode-specific
test states both builds' answers by hand; make ci gains test-peers,
which vets and tests the whole module under the tag, and amadan CI
gains the matching 35-test-peers step.

Agent notes

two mutations were checked against the peers build and
both went red: dropping MayActOn's explicit Owner-target rule, and
making reaches strict under PeerGrants. An app that uses the tag must
pass it to every go build AND go test, or it tests one policy and
ships another; SKILL.md §5 says so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude pushed by james@keymail.dev 1 month ago e757d2e7aeb6161d80d9a24c8e264f2b2899b5b2 parent 78f5829

14 files changed, +338 −88

.amadan/ci.d/35-test-peers +2 −0
Makefile +12 −2
SKILL.md +12 −4
example/app_test.go +15 −4
example/pages/members.html +5 −5
example/render.go +3 −2
handlers.go +8 −6
handlers_test.go +84 −8
peers.go +28 −0
peers_on.go +8 −0
policy.go +32 −7
policy_test.go +40 −22
roster.go +23 −15
roster_test.go +66 −13

CI passed — run details

CI log

=== vet ===
go vet ./...
=== fmt ===
=== test ===
go test ./... -count=1
ok  	amadan.net/rastrillo/idear	1.094s
ok  	amadan.net/rastrillo/idear/example	2.550s
?   	amadan.net/rastrillo/idear/internal/ideartest	[no test files]
=== test-peers ===
go vet -tags idear_peers ./...
go test -tags idear_peers ./... -count=1
ok  	amadan.net/rastrillo/idear	1.089s
ok  	amadan.net/rastrillo/idear/example	2.555s
?   	amadan.net/rastrillo/idear/internal/ideartest	[no test files]
=== race ===
CGO_ENABLED=1 go test ./... -race -count=1
ok  	amadan.net/rastrillo/idear	10.744s
ok  	amadan.net/rastrillo/idear/example	31.102s
?   	amadan.net/rastrillo/idear/internal/ideartest	[no test files]

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