diff --git a/src/Domain/Account/Adapters/Account.php b/src/Domain/Account/Adapters/Account.php index 16875cced..e77927d2a 100644 --- a/src/Domain/Account/Adapters/Account.php +++ b/src/Domain/Account/Adapters/Account.php @@ -33,6 +33,7 @@ use SP\Domain\Common\Providers\Link; use SP\Domain\Common\Services\ServiceException; use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Domain\Core\Acl\AclInterface; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Acl\ActionNotFoundException; use SP\Domain\Core\Acl\ActionsInterface; @@ -61,10 +62,11 @@ final class Account extends Adapter implements AccountAdapter public function __construct( ConfigDataInterface $configData, string $baseUrl, + AclInterface $acl, private readonly CustomFieldDataService $customFieldService, private readonly ActionsInterface $actions ) { - parent::__construct($configData, $baseUrl); + parent::__construct($configData, $baseUrl, $acl); } /** @@ -81,7 +83,7 @@ public function includeCustomFields(AccountEnrichedDto $accountEnrichedDto): Col $accountEnrichedDto->getId(), $this->customFieldService ), - new CustomField($this->configData, $this->baseUrl) + new CustomField($this->configData, $this->baseUrl, $this->acl) ); } diff --git a/src/Domain/Category/Adapters/Category.php b/src/Domain/Category/Adapters/Category.php index 8cb51ca10..d1846ac6f 100644 --- a/src/Domain/Category/Adapters/Category.php +++ b/src/Domain/Category/Adapters/Category.php @@ -33,6 +33,7 @@ use SP\Domain\Common\Providers\Link; use SP\Domain\Common\Services\ServiceException; use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Domain\Core\Acl\AclInterface; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Acl\ActionNotFoundException; use SP\Domain\Core\Acl\ActionsInterface; @@ -60,10 +61,11 @@ final class Category extends Adapter implements CategoryAdapter public function __construct( ConfigDataInterface $configData, string $baseUrl, + AclInterface $acl, private readonly CustomFieldDataService $customFieldDataService, private readonly ActionsInterface $actions ) { - parent::__construct($configData, $baseUrl); + parent::__construct($configData, $baseUrl, $acl); } /** @@ -76,7 +78,7 @@ public function includeCustomFields(CategoryModel $data): Collection { return $this->collection( $this->getCustomFieldsForItem(AclActionsInterface::CATEGORY, $data->getId(), $this->customFieldDataService), - new CustomField($this->configData, $this->baseUrl) + new CustomField($this->configData, $this->baseUrl, $this->acl) ); } diff --git a/src/Domain/Client/Adapters/Client.php b/src/Domain/Client/Adapters/Client.php index 6945dc3ce..fba80125b 100644 --- a/src/Domain/Client/Adapters/Client.php +++ b/src/Domain/Client/Adapters/Client.php @@ -33,6 +33,7 @@ use SP\Domain\Common\Providers\Link; use SP\Domain\Common\Services\ServiceException; use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Domain\Core\Acl\AclInterface; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Acl\ActionNotFoundException; use SP\Domain\Core\Acl\ActionsInterface; @@ -60,10 +61,11 @@ final class Client extends Adapter implements ClientAdapter public function __construct( ConfigDataInterface $configData, string $baseUrl, + AclInterface $acl, private readonly CustomFieldDataService $customFieldDataService, private readonly ActionsInterface $actions ) { - parent::__construct($configData, $baseUrl); + parent::__construct($configData, $baseUrl, $acl); } /** @@ -76,7 +78,7 @@ public function includeCustomFields(ClientModel $client): Collection { return $this->collection( $this->getCustomFieldsForItem(AclActionsInterface::CLIENT, $client->getId(), $this->customFieldDataService), - new CustomField($this->configData, $this->baseUrl) + new CustomField($this->configData, $this->baseUrl, $this->acl) ); } diff --git a/src/Domain/Common/Adapters/Adapter.php b/src/Domain/Common/Adapters/Adapter.php index 6d16f2c1e..12c3dfd3c 100644 --- a/src/Domain/Common/Adapters/Adapter.php +++ b/src/Domain/Common/Adapters/Adapter.php @@ -27,6 +27,7 @@ use League\Fractal\TransformerAbstract; use SP\Domain\Config\Ports\ConfigDataInterface; +use SP\Domain\Core\Acl\AclInterface; /** * Class Adapter @@ -35,7 +36,8 @@ abstract class Adapter extends TransformerAbstract { public function __construct( protected readonly ConfigDataInterface $configData, - protected readonly string $baseUrl + protected readonly string $baseUrl, + protected readonly AclInterface $acl ) { } diff --git a/src/Domain/CustomField/Adapters/CustomField.php b/src/Domain/CustomField/Adapters/CustomField.php index 43f642029..15afdb51f 100644 --- a/src/Domain/CustomField/Adapters/CustomField.php +++ b/src/Domain/CustomField/Adapters/CustomField.php @@ -26,6 +26,7 @@ namespace SP\Domain\CustomField\Adapters; use SP\Domain\Common\Adapters\Adapter; +use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Common\Dtos\Dto; use SP\Domain\CustomField\Ports\CustomFieldAdapter; use SP\Domain\CustomField\Services\CustomFieldItem; @@ -35,6 +36,9 @@ */ final class CustomField extends Adapter implements CustomFieldAdapter { + /** What the theme prints in place of a value the viewer may not see. */ + public const MASKED = '***'; + /** * @param CustomFieldItem $data * @return array @@ -47,9 +51,33 @@ public function transform($data): array 'definitionId' => $data->definitionId, 'definitionName' => $data->definitionName, 'help' => $data->help, - 'value' => $data->value, + 'value' => $this->valueFor($data), 'encrypted' => $data->isEncrypted, 'required' => $data->required, ]; } + + /** + * The value, or what the interface would show in its place. + * + * `ItemTrait::getCustomFieldsForItem()` decrypts a stored value whenever the row carries a + * key, without asking who is looking — the deciding is left to whoever renders it. The theme + * does decide: `aux-customfields.inc` prints `***` unless `showViewCustomPass`, which + * `AccountHelper` sets from the account's own view-password permission. Nothing decided here, + * so `account/view?customFields=1` answered with the decrypted value — on a token for + * `account/view`, an action the API otherwise keeps apart from `account/viewPass` precisely + * because one of them hands out secrets. + * + * A field that was never encrypted is not a secret and is returned as it is. + */ + private function valueFor(CustomFieldItem $data): ?string + { + if (!$data->isValueEncrypted) { + return $data->value; + } + + return $this->acl->checkUserAccess(AclActionsInterface::CUSTOMFIELD_VIEW_PASS) + ? $data->value + : self::MASKED; + } } diff --git a/tests/Unit/Domain/Account/Adapters/AccountTest.php b/tests/Unit/Domain/Account/Adapters/AccountTest.php index 805306b5a..241cc8dd5 100644 --- a/tests/Unit/Domain/Account/Adapters/AccountTest.php +++ b/tests/Unit/Domain/Account/Adapters/AccountTest.php @@ -30,6 +30,7 @@ use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\Exception; use SP\Domain\Account\Adapters\Account; +use SP\Domain\Core\Acl\AclInterface; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Acl\ActionNotFoundException; use SP\Domain\Core\Acl\ActionsInterface; @@ -70,6 +71,7 @@ public function testAdapt(): void $adapter = new Account( $this->config->getConfigData(), 'testUrl', + $this->createStub(AclInterface::class), $this->createStub(CustomFieldDataService::class), $actions ); @@ -152,7 +154,7 @@ public function testIncludeCustomFields(): void ); - $adapter = new Account($this->config->getConfigData(), 'testUrl', $customFieldsService, $actions); + $adapter = new Account($this->config->getConfigData(), 'testUrl', $this->createStub(AclInterface::class), $customFieldsService, $actions); $fractal = new Manager(); $fractal->parseIncludes('customFields'); diff --git a/tests/Unit/Domain/Category/Adapters/CategoryTest.php b/tests/Unit/Domain/Category/Adapters/CategoryTest.php index 1b06e34d3..3c6b8ae8b 100644 --- a/tests/Unit/Domain/Category/Adapters/CategoryTest.php +++ b/tests/Unit/Domain/Category/Adapters/CategoryTest.php @@ -32,6 +32,7 @@ use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\MockObject\MockObject; use SP\Domain\Category\Adapters\Category; +use SP\Domain\Core\Acl\AclInterface; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Acl\ActionNotFoundException; use SP\Domain\Core\Acl\ActionsInterface; @@ -61,6 +62,7 @@ public function testAdapt(): void $adapter = new Category( $this->config->getConfigData(), 'testUrl', + $this->createStub(AclInterface::class), $this->customFieldDataService, $this->actions ); @@ -93,6 +95,7 @@ public function testIncludeCustomFields(): void $adapter = new Category( $this->config->getConfigData(), 'testUrl', + $this->createStub(AclInterface::class), $this->customFieldDataService, $this->actions ); diff --git a/tests/Unit/Domain/Client/Adapters/ClientTest.php b/tests/Unit/Domain/Client/Adapters/ClientTest.php index 68e4c86cb..a6a94d6d4 100644 --- a/tests/Unit/Domain/Client/Adapters/ClientTest.php +++ b/tests/Unit/Domain/Client/Adapters/ClientTest.php @@ -32,6 +32,7 @@ use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\MockObject\MockObject; use SP\Domain\Client\Adapters\Client; +use SP\Domain\Core\Acl\AclInterface; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Acl\ActionNotFoundException; use SP\Domain\Core\Acl\ActionsInterface; @@ -61,6 +62,7 @@ public function testAdapt(): void $adapter = new Client( $this->config->getConfigData(), 'testUrl', + $this->createStub(AclInterface::class), $this->customFieldDataService, $this->actions ); @@ -94,6 +96,7 @@ public function testIncludeCustomFields(): void $adapter = new Client( $this->config->getConfigData(), 'testUrl', + $this->createStub(AclInterface::class), $this->customFieldDataService, $this->actions ); diff --git a/tests/Unit/Domain/CustomField/Adapters/CustomFieldAdapterTest.php b/tests/Unit/Domain/CustomField/Adapters/CustomFieldAdapterTest.php index 44dfb7334..7bef6c4f7 100644 --- a/tests/Unit/Domain/CustomField/Adapters/CustomFieldAdapterTest.php +++ b/tests/Unit/Domain/CustomField/Adapters/CustomFieldAdapterTest.php @@ -8,16 +8,85 @@ use PHPUnit\Framework\TestCase; use SP\Domain\Config\Ports\ConfigDataInterface; use SP\Domain\CustomField\Adapters\CustomField; +use SP\Domain\Core\Acl\AclActionsInterface; +use SP\Domain\Core\Acl\AclInterface; use SP\Domain\CustomField\Services\CustomFieldItem; #[Group('unitary')] class CustomFieldAdapterTest extends TestCase { - private function makeAdapter(): CustomField + private function makeAdapter(bool $mayViewPass = true): CustomField { $configData = $this->createStub(ConfigDataInterface::class); - return new CustomField($configData, 'https://example.com'); + $acl = $this->createStub(AclInterface::class); + $acl->method('checkUserAccess')->willReturnCallback( + static fn(int $action) => $action === AclActionsInterface::CUSTOMFIELD_VIEW_PASS ? $mayViewPass : true + ); + + return new CustomField($configData, 'https://example.com', $acl); + } + + /** + * @param array $overrides + */ + private function makeItem(array $overrides = []): CustomFieldItem + { + return new CustomFieldItem( + required: $overrides['required'] ?? false, + showInList: false, + help: '', + definitionId: 5, + definitionName: 'API Key', + typeId: 2, + typeName: 'password', + typeText: 'Password', + moduleId: 10, + formId: 'cf_5', + value: $overrides['value'] ?? 'secret', + isEncrypted: $overrides['isEncrypted'] ?? true, + isValueEncrypted: $overrides['isValueEncrypted'] ?? true, + ); + } + + /** + * A stored secret is not handed to a caller who may not see it. + * + * `ItemTrait::getCustomFieldsForItem()` decrypts whenever the row carries a key, without asking + * who is looking — the deciding is left to whoever renders it. The theme does decide: + * `aux-customfields.inc` prints `***` unless `showViewCustomPass`, which `AccountHelper` sets + * from the account's own view-password permission. Nothing decided here, so + * `account/view?customFields=1` answered with the decrypted value — on a token for + * `account/view`, which the API otherwise keeps apart from `account/viewPass` precisely because + * one of them hands out secrets. The account's own password is not in this response for the + * same reason. + */ + public function testAnEncryptedValueIsMaskedForACallerWhoMayNotViewPasswords(): void + { + $result = $this->makeAdapter(mayViewPass: false)->transform($this->makeItem()); + + self::assertSame(CustomField::MASKED, $result['value']); + self::assertTrue($result['encrypted'], 'the caller is still told the field holds a secret'); + } + + public function testAnEncryptedValueIsReturnedToACallerWhoMayViewPasswords(): void + { + $result = $this->makeAdapter(mayViewPass: true)->transform($this->makeItem()); + + self::assertSame('secret', $result['value']); + } + + /** + * A field that was never encrypted is not a secret, and the permission does not apply to it — + * masking every custom field would have been a different change, and a wrong one. + */ + public function testAValueThatWasNeverEncryptedIsReturnedRegardless(): void + { + $result = $this->makeAdapter(mayViewPass: false)->transform( + $this->makeItem(['isValueEncrypted' => false, 'isEncrypted' => false, 'value' => 'plain']) + ); + + self::assertSame('plain', $result['value']); } public function testTransformReturnsExpectedKeys(): void