Skip to content

Commit 888001c

Browse files
committed
groups: prevent moderators from attacking admins.
1 parent 5aff343 commit 888001c

2 files changed

Lines changed: 159 additions & 0 deletions

File tree

groups/reject-event.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,22 @@ func RejectEvent(ctx context.Context, event nostr.Event) (reject bool, msg strin
169169
return true, "a private group must also be closed"
170170
}
171171
case nip29.PutUser:
172+
if !isPrimaryRole {
173+
// moderators can't grant roles, and can't change the roles of users who already have them
174+
group.mu.RLock()
175+
for _, t := range a.Targets {
176+
if len(t.RoleNames) > 0 {
177+
group.mu.RUnlock()
178+
return true, "restricted: only admins can add or change user roles"
179+
}
180+
if currentRoles, isMember := group.Members[t.PubKey]; isMember && len(currentRoles) > 0 {
181+
group.mu.RUnlock()
182+
return true, "restricted: only admins can modify users with roles"
183+
}
184+
}
185+
group.mu.RUnlock()
186+
}
187+
172188
ineffective := true
173189
group.mu.RLock()
174190
for _, t := range a.Targets {
@@ -183,6 +199,18 @@ func RejectEvent(ctx context.Context, event nostr.Event) (reject bool, msg strin
183199
}
184200
group.mu.RUnlock()
185201
case nip29.RemoveUser:
202+
if !isPrimaryRole {
203+
// moderators can't remove admins or other moderators
204+
group.mu.RLock()
205+
for _, t := range a.Targets {
206+
if roles, isMember := group.Members[t]; isMember && len(roles) > 0 {
207+
group.mu.RUnlock()
208+
return true, "restricted: only admins can remove admins or moderators"
209+
}
210+
}
211+
group.mu.RUnlock()
212+
}
213+
186214
ineffective := true
187215
group.mu.RLock()
188216
for _, t := range a.Targets {

groups/reject_event_test.go

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package groups
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"fiatjaf.com/nostr"
8+
"fiatjaf.com/nostr/nip29"
9+
"github.com/puzpuzpuz/xsync/v3"
10+
"github.com/stretchr/testify/require"
11+
12+
"github.com/fiatjaf/pyramid/pyramid"
13+
)
14+
15+
func TestRejectModerationRoleEscalation(t *testing.T) {
16+
prevState := State
17+
prevMembers := pyramid.Members
18+
prevAbsoluteKey := pyramid.AbsoluteKey
19+
defer func() {
20+
State = prevState
21+
pyramid.Members = prevMembers
22+
pyramid.AbsoluteKey = prevAbsoluteKey
23+
}()
24+
25+
pyramid.Members = xsync.NewMapOf[nostr.PubKey, pyramid.Member]()
26+
pyramid.AbsoluteKey = nostr.PubKey{255}
27+
28+
adminSk := nostr.Generate()
29+
moderatorSk := nostr.Generate()
30+
otherModeratorSk := nostr.Generate()
31+
newMemberSk := nostr.Generate()
32+
admin := adminSk.Public()
33+
moderator := moderatorSk.Public()
34+
otherModerator := otherModeratorSk.Public()
35+
newMember := newMemberSk.Public()
36+
37+
adminRole := &nip29.Role{Name: PRIMARY_ROLE_NAME}
38+
moderatorRole := &nip29.Role{Name: SECONDARY_ROLE_NAME}
39+
40+
State = &GroupsState{
41+
Groups: xsync.NewMapOf[string, *Group](),
42+
publicKey: nostr.Generate().Public(),
43+
}
44+
45+
group := &Group{Group: nip29.Group{
46+
Address: nip29.GroupAddress{ID: "g"},
47+
Members: map[nostr.PubKey][]*nip29.Role{
48+
admin: {adminRole},
49+
moderator: {moderatorRole},
50+
otherModerator: {moderatorRole},
51+
},
52+
}}
53+
group.last50 = make([]nostr.ID, 50)
54+
State.Groups.Store("g", group)
55+
56+
ctx := context.Background()
57+
58+
sign := func(sk nostr.SecretKey, kind nostr.Kind, tags nostr.Tags) nostr.Event {
59+
evt := nostr.Event{
60+
PubKey: sk.Public(),
61+
CreatedAt: nostr.Now(),
62+
Kind: kind,
63+
Tags: tags,
64+
}
65+
require.NoError(t, evt.Sign(sk))
66+
return evt
67+
}
68+
69+
tests := []struct {
70+
name string
71+
event nostr.Event
72+
wantRej bool
73+
wantMsg string
74+
}{
75+
{
76+
name: "moderator cannot grant admin to self",
77+
event: sign(moderatorSk, nostr.KindSimpleGroupPutUser, nostr.Tags{{"h", "g"}, {"p", moderator.Hex(), PRIMARY_ROLE_NAME}}),
78+
wantRej: true,
79+
wantMsg: "restricted: only admins can add or change user roles",
80+
},
81+
{
82+
name: "moderator cannot grant moderator role to someone",
83+
event: sign(moderatorSk, nostr.KindSimpleGroupPutUser, nostr.Tags{{"h", "g"}, {"p", newMember.Hex(), SECONDARY_ROLE_NAME}}),
84+
wantRej: true,
85+
wantMsg: "restricted: only admins can add or change user roles",
86+
},
87+
{
88+
name: "moderator cannot demote an admin",
89+
event: sign(moderatorSk, nostr.KindSimpleGroupPutUser, nostr.Tags{{"h", "g"}, {"p", admin.Hex()}}),
90+
wantRej: true,
91+
wantMsg: "restricted: only admins can modify users with roles",
92+
},
93+
{
94+
name: "moderator can add a plain member",
95+
event: sign(moderatorSk, nostr.KindSimpleGroupPutUser, nostr.Tags{{"h", "g"}, {"p", newMember.Hex()}}),
96+
wantRej: false,
97+
},
98+
{
99+
name: "moderator cannot remove an admin",
100+
event: sign(moderatorSk, nostr.KindSimpleGroupRemoveUser, nostr.Tags{{"h", "g"}, {"p", admin.Hex()}}),
101+
wantRej: true,
102+
wantMsg: "restricted: only admins can remove admins or moderators",
103+
},
104+
{
105+
name: "moderator cannot remove another moderator",
106+
event: sign(moderatorSk, nostr.KindSimpleGroupRemoveUser, nostr.Tags{{"h", "g"}, {"p", otherModerator.Hex()}}),
107+
wantRej: true,
108+
wantMsg: "restricted: only admins can remove admins or moderators",
109+
},
110+
{
111+
name: "admin can grant admin role",
112+
event: sign(adminSk, nostr.KindSimpleGroupPutUser, nostr.Tags{{"h", "g"}, {"p", moderator.Hex(), PRIMARY_ROLE_NAME}}),
113+
wantRej: false,
114+
},
115+
{
116+
name: "admin can remove a moderator",
117+
event: sign(adminSk, nostr.KindSimpleGroupRemoveUser, nostr.Tags{{"h", "g"}, {"p", moderator.Hex()}}),
118+
wantRej: false,
119+
},
120+
}
121+
122+
for _, tt := range tests {
123+
t.Run(tt.name, func(t *testing.T) {
124+
reject, msg := RejectEvent(ctx, tt.event)
125+
require.Equal(t, tt.wantRej, reject)
126+
if tt.wantRej {
127+
require.Equal(t, tt.wantMsg, msg)
128+
}
129+
})
130+
}
131+
}

0 commit comments

Comments
 (0)