Authorize deferred bindings on otherwise-clean models

Follow-up to review feedback on the previous commit. Skipping the save for a
clean model and calling commitDeferred() directly bypassed the model's save
pipeline, so an actor without backend.manage_users could commit a deferred
`groups` binding on another user - group membership carries permissions, so
that was an escalation path that did not exist before.

A model with pending deferred bindings is now never skipped: committing those
bindings changes the model's relations, so it goes through the normal save and
is authorized like any other write. Only models that are genuinely inert -
existing, clean, and with nothing deferred - are passed over, which is still
the #1464 case.

Adds the negative-authorization test that was missing (it fails against the
previous commit), and uses firstOrFail() in the hydration helper for a clearer
failure mode.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: LukeTowers

modules/backend/behaviors/RelationController.php

@@ -1682,26 +1682,47 @@ public function relationExtendRefreshResults($field)
     /**
      * Saves the models prepared from a pivot form submission.
      *
-     * Models that already exist and have no changed attributes are skipped. `prepareModelsToSave()`
+     * Models that already exist and have nothing pending are skipped. `prepareModelsToSave()`
      * always queues the related model, so a submission containing nothing but pivot data would
      * otherwise trigger that model's save events - including authorization guards such as
-     * `Backend\Models\User::beforeSave()`. Their deferred bindings are still committed so that
-     * relation widgets on the pivot form continue to work.
+     * `Backend\Models\User::beforeSave()`.
+     *
+     * A model with deferred bindings is never skipped, even when its own attributes are clean:
+     * committing those bindings is a change to the model's relations, so it must go through the
+     * normal save pipeline and be authorized like any other.
      */
     protected function relationSavePivotModels(array $modelsToSave): void
     {
         $sessionKey = $this->pivotWidget->getSessionKey();
 
         foreach ($modelsToSave as $modelToSave) {
-            if ($modelToSave->exists && !$modelToSave->isDirty()) {
-                $modelToSave->commitDeferred($sessionKey);
+            if (
+                $modelToSave->exists
+                && !$modelToSave->isDirty()
+                && !$this->relationHasDeferredBindings($modelToSave, $sessionKey)
+            ) {
                 continue;
             }
 
             $modelToSave->save(null, $sessionKey);
         }
     }
 
+    /**
+     * Returns whether the model has any deferred bindings pending for the given session key.
+     */
+    protected function relationHasDeferredBindings($model, string $sessionKey): bool
+    {
+        $binding = new DeferredBinding;
+
+        $binding->setConnection($model->getConnectionName());
+
+        return $binding
+            ->where('master_type', get_class($model))
+            ->where('session_key', $sessionKey)
+            ->exists();
+    }
+
     /**
      * Returns the existing record IDs for the relation.
      */

modules/backend/tests/behaviors/RelationControllerPivotTest.php

@@ -84,7 +84,7 @@ protected function getHydratedRelatedUser(): User
     {
         return $this->fixture->users()
             ->where('backend_users.id', $this->targetUser->id)
-            ->first();
+            ->firstOrFail();
     }
 
     /**
@@ -202,7 +202,7 @@ public function testRelatedModelSaveIsStillAuthorizedWhenItsOwnFieldsAreEdited()
      * Skipping the save must not drop pending relation work. A deferred binding leaves the
      * owning model clean, so it would be lost if the model were simply passed over.
      */
-    public function testDeferredBindingsAreCommittedForSkippedModels(): void
+    public function testDeferredBindingsAreCommittedForOtherwiseCleanModels(): void
     {
         $this->actingAs((new UserFixture)->withPermission('backend.manage_users', true));
 
@@ -223,4 +223,28 @@ public function testDeferredBindingsAreCommittedForSkippedModels(): void
         $this->assertEquals(1, $hydrated->groups()->count(), 'The deferred binding was committed');
         $this->assertEquals(1, $this->getPivotValue());
     }
+
+    /**
+     * Skipping the save must not become an authorization bypass. A deferred binding leaves the
+     * owning model clean, so a naive skip would commit relation changes - such as adding a user
+     * to a group, which carries permissions - without ever consulting `User::beforeSave()`.
+     */
+    public function testDeferredBindingsAreStillAuthorizedForOtherwiseCleanModels(): void
+    {
+        $this->actingAs((new UserFixture)->withPermission('backend.manage_users', false));
+
+        $hydrated = $this->getHydratedRelatedUser();
+
+        $group = UserGroup::create([
+            'name' => 'Test Group',
+            'code' => 'test-group',
+        ]);
+        $hydrated->groups()->add($group, 'pivottestsessionkey');
+
+        $this->assertFalse($hydrated->isDirty(), 'The deferred binding leaves the user clean');
+
+        $this->expectException(AuthorizationException::class);
+
+        $this->savePivotForm($hydrated, ['pivot' => ['is_default' => true]]);
+    }
 }