fix: a permission preset needs the permission it grants

src/Infrastructure/Adapter/In/Web/Forms/ItemsPresetForm.php

@@ -24,6 +24,7 @@
 
 namespace SP\Infrastructure\Adapter\In\Web\Forms;
 
+use SP\Application\Account\Services\AccountAcl;
 use SP\Domain\Core\Acl\AclActionsInterface;
 use SP\Domain\Core\Exceptions\InvalidArgumentException;
 use SP\Domain\Core\Exceptions\ValidationException;
@@ -129,6 +130,23 @@ protected function analyzeRequestData(): void
      */
     private function makePermissionPreset(): AccountPermission
     {
+        // A permission preset shares accounts, so writing one needs the authority that sharing an
+        // account by hand needs.
+        //
+        // Every preset action is gated on `isMgmItemsPreset()` alone — the profile switch labelled
+        // "Default Values Management" — while choosing who an account is shared with is gated on
+        // `AccountAcl::getShowPermission()`: an application or account administrator, or
+        // `isAccPermission()`. A fixed permission preset is applied by `addPresetPermissions()` to
+        // every account its target creates, outside the `$userCanChangePermissions` gate that
+        // guards the hand-picked sharing on the very same request. So a holder of the innocuous
+        // sounding switch, and nothing else, could make every account a colleague creates from
+        // then on shared with themselves for editing — sharing they could never have granted by
+        // hand. The other three preset types (password policy, privacy, session timeout) grant no
+        // access and stay with `isMgmItemsPreset()`.
+        if (!AccountAcl::getShowPermission($this->context->getUserData(), $this->context->getUserProfile())) {
+            throw new ValidationException(__u('You don\'t have permission to assign account permissions'));
+        }
+
         $accountPermission = new AccountPermission(
             $this->request->analyzeArray('users_view', null, []),
             $this->request->analyzeArray('users_edit', null, []),

tests/Integration/Infrastructure/Adapter/In/Web/Controllers/ItemPreset/ItemPresetTest.php

@@ -37,6 +37,7 @@
 use SP\Domain\ItemPreset\Models\ItemPreset;
 use SP\Domain\ItemPreset\Models\SessionTimeout;
 use SP\Domain\ItemPreset\Ports\ItemPresetInterface;
+use SP\Domain\User\Models\ProfileData;
 use SP\Domain\User\Models\User as UserModel;
 use SP\Infrastructure\Database\QueryData;
 use SP\Tests\Support\BodyChecker;
@@ -49,6 +50,15 @@
 #[Group('integration')]
 class ItemPresetTest extends IntegrationTestCase
 {
+    /**
+     * Saving a permission preset needs the authority to share accounts by hand, which the
+     * harness's randomly generated profile holds only about half the time.
+     */
+    protected function getUserProfile(): ProfileData
+    {
+        return parent::getUserProfile()->mutate(['accPermission' => true]);
+    }
+
     /**
      * The create form is built for a preset type passed on the route.
      *

tests/Integration/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php

@@ -32,6 +32,7 @@
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\NotFoundExceptionInterface;
 use SP\Domain\ItemPreset\Ports\ItemPresetInterface;
+use SP\Domain\User\Models\ProfileData;
 use SP\Tests\Support\IntegrationTestCase;
 
 /**
@@ -44,6 +45,15 @@
 #[Group('integration')]
 class ItemsPresetFormTest extends IntegrationTestCase
 {
+    /**
+     * Saving a permission preset needs the authority to share accounts by hand, which the
+     * harness's randomly generated profile holds only about half the time.
+     */
+    protected function getUserProfile(): ProfileData
+    {
+        return parent::getUserProfile()->mutate(['accPermission' => true]);
+    }
+
     /**
      * A permission preset has to name somebody, otherwise it grants nothing and would sit in
      * the list doing nothing.

tests/Unit/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php

@@ -36,6 +36,7 @@
 use SP\Domain\ItemPreset\Models\Password;
 use SP\Domain\ItemPreset\Models\SessionTimeout;
 use SP\Domain\ItemPreset\Ports\ItemPresetInterface;
+use SP\Domain\User\Models\ProfileData;
 use SP\Infrastructure\Adapter\In\Web\Forms\ItemsPresetForm;
 use SP\Tests\Support\UnitaryTestCase;
 
@@ -88,6 +89,8 @@ public function aPresetOfAnUnknownKindIsRefused(): void
     #[Test]
     public function aPermissionPresetGrantingNobodyAnythingIsRefused(): void
     {
+        $this->givenTheSignedInUserMayShareAccounts();
+
         $this->givenARequest(
             ['type' => ItemPresetInterface::ITEM_TYPE_ACCOUNT_PERMISSION],
             ['user_id' => 1]
@@ -111,6 +114,8 @@ public function aPermissionPresetGrantingNobodyAnythingIsRefused(): void
     #[Test]
     public function aPermissionPresetCarriesWhoWasNamed(): void
     {
+        $this->givenTheSignedInUserMayShareAccounts();
+
         $this->givenARequest(
             ['type' => ItemPresetInterface::ITEM_TYPE_ACCOUNT_PERMISSION],
             ['user_id' => 1],
@@ -129,6 +134,31 @@ public function aPermissionPresetCarriesWhoWasNamed(): void
         self::assertSame([], $preset->getUserGroupsEdit());
     }
 
+    /**
+     * A permission preset shares every account its target creates from then on, so writing one
+     * needs the authority sharing an account by hand needs. "Default Values Management" alone is
+     * the switch that reaches this form, and it must not be a way to grant access its holder could
+     * never have granted directly.
+     *
+     * @throws ValidationException
+     */
+    #[Test]
+    public function aPermissionPresetFromSomebodyWhoCannotShareAccountsIsRefused(): void
+    {
+        $this->context->setUserProfile(new ProfileData(['mgmItemsPreset' => true]));
+
+        $this->givenARequest(
+            ['type' => ItemPresetInterface::ITEM_TYPE_ACCOUNT_PERMISSION],
+            ['user_id' => 1],
+            ['users_view' => [], 'users_edit' => [12], 'user_groups_view' => [], 'user_groups_edit' => []]
+        );
+
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage('You don\'t have permission to assign account permissions');
+
+        $this->buildForm()->validateFor(AclActionsInterface::ITEMPRESET_CREATE);
+    }
+
     /**
      * A session timeout is bound to an address, so an address that is not one cannot be stored:
      * the rule would either never match or match everybody.
@@ -314,6 +344,14 @@ private function givenARequest(
         $this->request->method('analyzeUnsafeString')->willReturn($unsafeString);
     }
 
+    /**
+     * Signs the test in as somebody whose profile lets them choose who an account is shared with.
+     */
+    private function givenTheSignedInUserMayShareAccounts(): void
+    {
+        $this->context->setUserProfile(new ProfileData(['mgmItemsPreset' => true, 'accPermission' => true]));
+    }
+
     private function buildForm(): ItemsPresetForm
     {
         return new ItemsPresetForm($this->application, $this->request);