Move backend user authorization guards to the actual write events (#1503)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

RelationController's pivot handlers save every model returned by prepareModelsToSave(). That list always includes the related model — FormModelSaver::setModelAttributes() pushes it before it even looks at $saveData — so a submission containing nothing but pivot data still saves the related record.

For a belongsToMany relation to Backend\Models\User, that no-op save fired User::beforeSave(), which throws for any operator lacking backend.manage_users. The relation became unusable for them. This is a regression against 1.2.9, surfaced by the user authorization guards added since.

Fixes #1464

Author: LukeTowers

modules/backend/behaviors/RelationController.php

@@ -1583,14 +1583,18 @@ public function onRelationManagePivotUpdate()
     {
         $this->beforeAjax();
 
-        $foreignKeyName = $this->relationModel->getQualifiedKeyName();
         $hydratedModel = $this->pivotWidget->model;
         $saveData = $this->pivotWidget->getSaveData();
 
-        $modelsToSave = $this->prepareModelsToSave($hydratedModel, $saveData);
-        foreach ($modelsToSave as $modelToSave) {
-            $modelToSave->save(null, $this->pivotWidget->getSessionKey());
-        }
+        /*
+         * If any of the models fail to save, abort the whole update
+         */
+        Db::transaction(function () use ($hydratedModel, $saveData) {
+            $modelsToSave = $this->prepareModelsToSave($hydratedModel, $saveData);
+            foreach ($modelsToSave as $modelToSave) {
+                $modelToSave->save(null, $this->pivotWidget->getSessionKey());
+            }
+        });
 
         return ['#'.$this->relationGetId('view') => $this->relationRenderView()];
     }

modules/backend/models/User.php

@@ -78,6 +78,32 @@ class User extends UserBase
      */
     public static $loginAttribute = 'login';
 
+    /**
+     * @var array<string> Relations on this model that require `backend.manage_users`
+     * to change on another user's record. Deliberately limited to the relations this
+     * model owns: authorization semantics for plugin-added relations belong to the
+     * plugin, which can append to this list or bind its own guard to the
+     * `model.relation.*` events.
+     */
+    public array $permissionGuardedRelations = ['groups', 'avatar', 'throttle'];
+
+    public function __construct(array $attributes = [])
+    {
+        parent::__construct($attributes);
+
+        // Guard relation writes at the point they actually happen: direct
+        // attach()/detach(), deferred binding commits and queued relation
+        // syncs all funnel through these relation events.
+        $guard = function (string $relationName) {
+            $this->authorizeRelationChange($relationName);
+        };
+
+        $this->bindEvent('model.relation.beforeAttach', $guard);
+        $this->bindEvent('model.relation.beforeDetach', $guard);
+        $this->bindEvent('model.relation.beforeAdd', $guard);
+        $this->bindEvent('model.relation.beforeRemove', $guard);
+    }
+
     /**
      * @return string Returns the user's full name.
      */
@@ -156,9 +182,33 @@ public function canBeManagedByUser(?User $user = null): bool
     }
 
     /**
-     * Before save event — enforce authorization rules to prevent privilege escalation.
+     * Before create event — enforce authorization rules to prevent privilege escalation.
      */
-    public function beforeSave()
+    public function beforeCreate()
+    {
+        $this->authorizeChange();
+    }
+
+    /**
+     * Before update event — enforce authorization rules to prevent privilege escalation.
+     *
+     * Bound to update rather than save so that a save with nothing to write (e.g. a
+     * pivot form submission re-saving an untouched related record) requires no
+     * permission: the update event only fires when attributes have actually changed,
+     * after purgeable attributes have been stripped. Relation writes (group
+     * membership, avatar) are guarded separately by authorizeRelationChange().
+     */
+    public function beforeUpdate()
+    {
+        $this->authorizeChange();
+    }
+
+    /**
+     * Enforce authorization rules for a change to this record's attributes.
+     *
+     * @throws AuthorizationException if the current user lacks permission
+     */
+    protected function authorizeChange(): void
     {
         $actor = BackendAuth::getUser();
         if (!$actor) {
@@ -176,6 +226,35 @@ public function beforeSave()
         }
     }
 
+    /**
+     * Enforce authorization rules for a change to one of this record's relations.
+     *
+     * Only the relations listed in $permissionGuardedRelations are guarded, so
+     * plugin-added relations (e.g. records that reference a user) behave normally.
+     *
+     * Changes to your own record's guarded relations are allowed: groups do not
+     * carry permissions out of the box, so they are not an escalation vector.
+     *
+     * @throws AuthorizationException if the current user lacks permission
+     */
+    protected function authorizeRelationChange(string $relationName): void
+    {
+        if (!in_array($relationName, $this->permissionGuardedRelations)) {
+            return;
+        }
+
+        $actor = BackendAuth::getUser();
+        if (!$actor) {
+            return;
+        }
+
+        $isCurrentUser = $this->exists && $actor->getKey() === $this->getKey();
+
+        if (!$isCurrentUser && !$this->canBeManagedByUser($actor)) {
+            throw new AuthorizationException(Lang::get('backend::lang.user.cannot_manage_user'));
+        }
+    }
+
     /**
      * Before delete event — enforce authorization rules.
      */