golem15/sm-user-plugin
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: user groups delete keeps FormAfterDelete's declaration; members' subject events fire from FormBeforeDelete

Browse Source 
The member ids are read in the delete's transaction before the memberships
go, and the permissions events fire after it commits, as before; the
smoke now covers a group delete.
This commit is contained in:
Jakub Zych
2026-10-23 12:16:53 +02:00
parent 6f1cb1a774
commit dbb3d33b48
2 changed files with 27 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
controllers/usergroups_admin_controller.go
View File

@@ -121,23 +121,36 @@ func (c usergroupsAdminController) FormBeforeUpdate(ctx context.Context, model a
// FormBeforeDelete refuses the delete of a privileged group without
// golem15.users.manage_privileged_groups: it would take the privilege from
// every member. The record is the stored row, loaded in the delete's
// transaction.
// transaction. A delete that goes ahead reads the members there; each gets
// a pact.MCPSubjectChanged permissions event after the delete commits,
// since the group's grants are gone (FormAfterDelete removes the
// memberships).
func (c usergroupsAdminController) FormBeforeDelete(ctx context.Context, model any) error {
group, ok := model.(*models.UserGroup)
if !ok || group == nil {
return errors.New("user groups admin: form model is not a user group")
}
if !classes.IsPrivilegedCode(classes.PrivilegedGroupCodes(c.app.config()), group.Code) {
if classes.IsPrivilegedCode(classes.PrivilegedGroupCodes(c.app.config()), group.Code) {
if err := c.requirePrivilegedPermission(ctx); err != nil {
return err
}
}
tx, ok := cabana.TxFromContext(ctx)
if !ok {
return nil
}
return c.requirePrivilegedPermission(ctx)
members, err := groupMemberIDs(ctx, tx, group.ID)
if err != nil {
return err
}
c.permissionsChanged(ctx, tx, members)
return nil
}
// FormAfterDelete removes the memberships of the deleted group in the same
// transaction: users_groups has no foreign key that would do it. The member
// users stay; each gets a pact.MCPSubjectChanged permissions event after the
// delete commits, since the group's grants are gone.
func (c usergroupsAdminController) FormAfterDelete(ctx context.Context, model any) error {
// users stay.
func (usergroupsAdminController) FormAfterDelete(ctx context.Context, model any) error {
group, ok := model.(*models.UserGroup)
if !ok || group == nil {
return errors.New("user groups admin: form model is not a user group")
@@ -146,16 +159,8 @@ func (c usergroupsAdminController) FormAfterDelete(ctx context.Context, model an
if !ok {
return errors.New("user groups admin: a delete needs its transaction")
}
members, err := groupMemberIDs(ctx, tx, group.ID)
if err != nil {
return err
}
if err := tx.Session(&gorm.Session{NewDB: true}).WithContext(ctx).
Where("user_group_id = ?", group.ID).Delete(&models.UsersGroup{}).Error; err != nil {
return err
}
c.permissionsChanged(ctx, tx, members)
return nil
return tx.Session(&gorm.Session{NewDB: true}).WithContext(ctx).
Where("user_group_id = ?", group.ID).Delete(&models.UsersGroup{}).Error
}
// FormAfterUpdate tells the MCP endpoint that the members of the group may

6
mcp_guard_smoke_test.go
View File

@@ -175,6 +175,12 @@ func TestUserMCPGuardSmoke(t *testing.T) {
if !spy.has("user", user.ID, pact.MCPSubjectPermissions) {
t.Fatalf("group save fired %+v", spy.events)
}
// Deleting a group fires permissions for its members once it commits.
spy.reset()
env.expect(t, http.StatusOK, http.MethodDelete, groupPath(groupB.ID), manager, nil)
if !spy.has("user", user.ID, pact.MCPSubjectPermissions) {
t.Fatalf("group delete fired %+v", spy.events)
}
// Saving the user on the Users screen fires permissions too.
spy.reset()
users := env.admin(t, permAccessUsers)