diff --git a/src/Infrastructure/Adapter/In/Web/Forms/ItemsPresetForm.php b/src/Infrastructure/Adapter/In/Web/Forms/ItemsPresetForm.php index a461b8b12..e887ae37e 100644 --- a/src/Infrastructure/Adapter/In/Web/Forms/ItemsPresetForm.php +++ b/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, []), diff --git a/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/ItemPreset/ItemPresetTest.php b/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/ItemPreset/ItemPresetTest.php index 8491292a3..72cc34c45 100644 --- a/tests/Integration/Infrastructure/Adapter/In/Web/Controllers/ItemPreset/ItemPresetTest.php +++ b/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. * diff --git a/tests/Integration/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php b/tests/Integration/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php index 71fa99daf..0be1ab596 100644 --- a/tests/Integration/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php +++ b/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. diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php index 4073c99ea..058ae1682 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/Forms/ItemsPresetFormTest.php +++ b/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);