Move backend user authorization guards to the actual write events

[LukeTowers]

Fixes #1464

Problem

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.

Fix

An earlier revision of this PR taught the pivot handlers to skip models with nothing to write. Review showed that approach is inherently brittle — the save pipeline has several independent write channels (attribute updates, deferred binding commits, relation syncs queued by setSimpleValue(), purgeable attributes that look dirty but aren't), and any skip heuristic must re-derive all of them. Two gaps were confirmed: relation-widget fields on the related model were silently dropped, and _-prefixed helper fields defeated the isDirty() check.

This revision moves the guard instead, onto the events that only fire for actual writes:

• beforeCreate() / beforeUpdate() guard attribute writes. Eloquent's update event only fires when attributes have genuinely changed — after purgeable attributes are stripped — so a save with nothing to write requires no permission.
• model.relation.beforeAttach / beforeDetach / beforeAdd / beforeRemove guard relation writes. Direct attach()/detach(), deferred binding commits (commitDeferredAfter → add() → attach()) and queued setSimpleValue() syncs (afterSave → sync() → attach()/detach()) all funnel through these events, so the guard fires at the moment the relation write happens rather than trying to predict it.

The pivot handlers save every prepared model unconditionally again — restoring pre-1.2.10 behavior for plugin afterSave hooks and relation-widget fields — and onRelationManagePivotUpdate() is wrapped in Db::transaction() to match onRelationManagePivotCreate().

Guard semantics

• Attribute changes: unchanged rules (self non-escalating edits allowed; others' records need backend.manage_users; superuser targets need a superuser actor; CLI/queue contexts exempt).
• Relation changes are guarded only for the relations the user model owns — groups, avatar, throttle, listed in the public $permissionGuardedRelations property. Changing them on another user's record requires backend.manage_users, now even via direct attach()/detach() — those paths previously bypassed the save-based guard entirely.
• Plugin-added relations on the user model are deliberately not guarded: a plugin writing its own user-side relation (e.g. assigning a user to a support ticket via $user->tickets()->add()) keeps working for unprivileged operators, since that relation is the plugin's domain. Plugins with genuinely sensitive relations can append to $permissionGuardedRelations or bind their own guard to the same model.relation.* events. (Relations that merely reference users from the other side — $ticket->users()->attach() — never touched the guard, since the events fire on the parent model.)
• Relation changes to your own record are allowed: backend groups carry no permissions out of the box (getMergedPermissions() merges only role + user permissions), so group membership is not an escalation vector.

Tests

RelationControllerPivotTest (10 cases) exercises the production pivot save sequence end-to-end: pivot-only saves with and without the permission, related-field edits (applied and refused), relation-widget fields via the setSimpleValue() path (applied and refused), and deferred binding commits (applied and refused). UserAuthorizationTest grows from 31 to 39 cases: no-op saves need no permission, creates are guarded, group attach/detach/sync are guarded for other users' records, self group changes stay allowed, and a plugin-style relation added to the user model is not guarded.

Negative control: with the model.relation.* bindings commented out, all six unauthorized-relation-change tests fail — the coverage bites.

Full core suite passes (backend: 169 tests / 371 assertions; the only failures elsewhere are 3 pre-existing environmental failures in cms/ControllerTest caused by a local asset-URL config, present on the unmodified branch too). phpcs clean on all changed files.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved many-to-many pivot persistence by ensuring related model prep and saves run atomically.
  • Pivot updates now reliably handle authorization: pivot-only changes succeed without requiring permission to edit the related record, while related record field edits remain protected.
  • Deferred relation bindings are committed during pivot updates when authorized.

• New Features

  • Stricter user update authorization, including guarded relation changes (e.g., group membership), with clearer denial behavior.

• Tests

  • Added pivot regression coverage and expanded user/group authorization scenarios.

[LukeTowers]

@quendistudio this should fix #1464

[coderabbitai]

🧹 Nitpick comments (1)

modules/backend/behaviors/RelationController.php (1)

1572-1576: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate save loop between the two pivot handlers.

Both onRelationManagePivotCreate() and onRelationManagePivotUpdate() now contain the identical prepareModelsToSave() + per-model save(null, $sessionKey) loop. Extracting a small helper would remove the duplication this PR introduced.

♻️ Proposed helper extraction

+    /**
+     * Prepares and saves each model for the pivot save data, under the pivot session key.
+     */
+    protected function relationSavePivotPreparedModels($hydratedModel, array $saveData): void
+    {
+        $modelsToSave = $this->prepareModelsToSave($hydratedModel, $saveData);
+        foreach ($modelsToSave as $modelToSave) {
+            $modelToSave->save(null, $this->pivotWidget->getSessionKey());
+        }
+    }

Then in onRelationManagePivotCreate():

             foreach ($hydratedModels as $hydratedModel) {
-                $modelsToSave = $this->prepareModelsToSave($hydratedModel, $saveData);
-                foreach ($modelsToSave as $modelToSave) {
-                    $modelToSave->save(null, $this->pivotWidget->getSessionKey());
-                }
+                $this->relationSavePivotPreparedModels($hydratedModel, $saveData);
             }

And in onRelationManagePivotUpdate():

         Db::transaction(function () use ($hydratedModel, $saveData) {
-            $modelsToSave = $this->prepareModelsToSave($hydratedModel, $saveData);
-            foreach ($modelsToSave as $modelToSave) {
-                $modelToSave->save(null, $this->pivotWidget->getSessionKey());
-            }
+            $this->relationSavePivotPreparedModels($hydratedModel, $saveData);
         });

Also applies to: 1592-1597

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/backend/behaviors/RelationController.php` around lines 1572 - 1576,
Extract the duplicated prepare-and-save sequence from
onRelationManagePivotCreate() and onRelationManagePivotUpdate() into a shared
private helper, preserving prepareModelsToSave(), the foreach iteration, and
each model’s save(null, $this->pivotWidget->getSessionKey()) call. Replace both
inline loops with calls to the helper.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@modules/backend/behaviors/RelationController.php`:
- Around line 1572-1576: Extract the duplicated prepare-and-save sequence from
onRelationManagePivotCreate() and onRelationManagePivotUpdate() into a shared
private helper, preserving prepareModelsToSave(), the foreach iteration, and
each model’s save(null, $this->pivotWidget->getSessionKey()) call. Replace both
inline loops with calls to the helper.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 51b57b4c-1aa8-4c2d-9e5b-090719a5a60e

📥 Commits

Reviewing files that changed from the base of the PR and between 3ee9e6b and f3e0158.

📒 Files selected for processing (4)

• modules/backend/behaviors/RelationController.php
• modules/backend/models/User.php
• modules/backend/tests/behaviors/RelationControllerPivotTest.php
• modules/backend/tests/models/UserAuthorizationTest.php

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.