diff --git a/src/Application/Import/Ports/ImportHelperInterface.php b/src/Application/Import/Ports/ImportHelperInterface.php index 58834353e..a854232a0 100644 --- a/src/Application/Import/Ports/ImportHelperInterface.php +++ b/src/Application/Import/Ports/ImportHelperInterface.php @@ -25,6 +25,7 @@ namespace SP\Application\Import\Ports; +use SP\Application\Account\Ports\AccountPresetService; use SP\Application\Account\Ports\AccountService; use SP\Application\Category\Ports\CategoryService; use SP\Application\Client\Ports\ClientService; @@ -39,6 +40,8 @@ interface ImportHelperInterface { public function getAccountService(): AccountService; + public function getAccountPresetService(): AccountPresetService; + /** * @return CategoryService */ diff --git a/src/Application/Import/Services/ImportBase.php b/src/Application/Import/Services/ImportBase.php index 88db3c7af..a8a29c4fb 100644 --- a/src/Application/Import/Services/ImportBase.php +++ b/src/Application/Import/Services/ImportBase.php @@ -31,6 +31,7 @@ use SP\Application\Application; use SP\Domain\Crypt\Hash; use SP\Domain\Account\Dtos\AccountCreateDto; +use SP\Application\Account\Ports\AccountPresetService; use SP\Application\Account\Ports\AccountService; use SP\Domain\Category\Models\Category; use SP\Domain\Category\Models\Category as CategoryModel; @@ -68,6 +69,7 @@ abstract class ImportBase extends Service implements ImportService protected int $version = 0; protected int $counter = 0; protected readonly AccountService $accountService; + protected readonly AccountPresetService $accountPresetService; /** @var CategoryService */ protected readonly CategoryService $categoryService; protected readonly ClientService $clientService; @@ -84,6 +86,7 @@ public function __construct( parent::__construct($application); $this->accountService = $importHelper->getAccountService(); + $this->accountPresetService = $importHelper->getAccountPresetService(); $this->categoryService = $importHelper->getCategoryService(); $this->clientService = $importHelper->getClientService(); $this->tagService = $importHelper->getTagService(); @@ -157,7 +160,19 @@ final protected function addAccount( $dto = $dto->mutate(['pass' => $pass, 'key' => '']); } - $this->accountService->create($dto); + // The password lifetime applies to an imported account too. Every other door that creates + // one — the web form, the API's create and edit-pass — asks the preset service for this; + // the importers went straight to AccountService::create(), which applies the permission + // and privacy presets but has never applied the password ones. So a fixed policy saying + // passwords expire after ninety days said nothing about the several thousand accounts + // that arrived through a CSV. + // + // The lifetime clamp only. checkPasswordPreset(), which validates length and character + // classes, is deliberately not applied here: an import is a migration of credentials that + // already exist elsewhere, and refusing the ones that predate the policy would mean not + // being able to migrate at all. The policy still bites when the account is next edited, + // and the clamp is what makes that happen rather than waiting for someone to notice. + $this->accountService->create($this->accountPresetService->checkPasswordExpiry($dto)); $this->counter++; } diff --git a/src/Application/Import/Services/ImportHelper.php b/src/Application/Import/Services/ImportHelper.php index b8777adf3..26485ddf0 100644 --- a/src/Application/Import/Services/ImportHelper.php +++ b/src/Application/Import/Services/ImportHelper.php @@ -25,6 +25,7 @@ namespace SP\Application\Import\Services; +use SP\Application\Account\Ports\AccountPresetService; use SP\Application\Account\Ports\AccountService; use SP\Application\Category\Ports\CategoryService; use SP\Application\Client\Ports\ClientService; @@ -46,7 +47,8 @@ public function __construct( private CategoryService $categoryService, private ClientService $clientService, private TagService $tagService, - private ConfigService $configService + private ConfigService $configService, + private AccountPresetService $accountPresetService ) { } @@ -55,6 +57,11 @@ public function getAccountService(): AccountService return $this->accountService; } + public function getAccountPresetService(): AccountPresetService + { + return $this->accountPresetService; + } + /** * @return CategoryService */ diff --git a/tests/Unit/Application/Import/Services/CsvImportTest.php b/tests/Unit/Application/Import/Services/CsvImportTest.php index 37f0ddc33..b41e4b97b 100644 --- a/tests/Unit/Application/Import/Services/CsvImportTest.php +++ b/tests/Unit/Application/Import/Services/CsvImportTest.php @@ -31,6 +31,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\Rule\InvokedCount; use SP\Domain\Account\Dtos\AccountCreateDto; +use SP\Application\Account\Ports\AccountPresetService; use SP\Application\Account\Ports\AccountService; use SP\Domain\Category\Models\Category; use SP\Application\Category\Ports\CategoryService; @@ -59,6 +60,9 @@ class CsvImportTest extends UnitaryTestCase private AccountService|MockObject $accountService; private MockObject|CategoryService $categoryService; private ClientService|MockObject $clientService; + private AccountPresetService|MockObject $accountPresetService; + /** @var int|null What the lifetime clamp should answer with, when a test cares. */ + private ?int $clampTo = null; private FileHandlerInterface|MockObject $fileHandler; private CsvImport $csvImport; @@ -139,6 +143,52 @@ public function testDoImport() $this->csvImport->doImport($params); } + /** + * An imported account is subject to the password lifetime. + * + * Every other door that creates an account asks the preset service to clamp passDateChange to + * the policy's lifetime — the web form, and the API's create and edit-pass. The importers went + * straight to AccountService::create(), which applies the permission and privacy presets but + * has never applied the password ones, so a fixed policy saying passwords expire after ninety + * days said nothing at all about the several thousand accounts that arrived through a CSV. + * + * The stored account is whatever the clamp returned, not the DTO the importer built. + * + * @throws ImportException + * @throws FileException + */ + public function testAnImportedAccountIsSubjectToThePasswordLifetime() + { + $params = new CsvImportParamsDto($this->fileHandler, 1, 1); + + $this->fileHandler + ->expects(self::once()) + ->method('readFromCsv') + ->willReturnCallback( + static function () { + yield ['Account_name', 'Client_name', 'Category_name', 'a_url', 'a_login', 'a_password', 'a_note']; + } + ); + + $this->clientService->expects(self::once())->method('getByName')->willThrowException(NoSuchItemException::error('test')); + $this->clientService->expects(self::once())->method('create')->willReturn(100); + $this->categoryService->expects(self::once())->method('getByName')->willThrowException(NoSuchItemException::error('test')); + $this->categoryService->expects(self::once())->method('create')->willReturn(200); + + $clamped = 1_234_567_890; + + $this->clampTo = $clamped; + + $this->accountService + ->expects(self::once()) + ->method('create') + ->with( + new Callback(static fn(AccountCreateDto $dto): bool => $dto->passDateChange === $clamped) + ); + + $this->csvImport->doImport($params); + } + /** * @throws ImportException * @throws FileException @@ -396,12 +446,26 @@ protected function setUp(): void $this->categoryService = $this->createMock(CategoryService::class); $this->clientService = $this->createMock(ClientService::class); + $this->accountPresetService = $this->createStub(AccountPresetService::class); + + // The lifetime clamp runs on import. By default it changes nothing; a test that cares sets + // $clampTo. One stub rather than a per-test override, because the first stub registered is + // the one that answers — a later re-stub of the same method silently does nothing. + $this->accountPresetService + ->method('checkPasswordExpiry') + ->willReturnCallback( + fn(AccountCreateDto $dto): AccountCreateDto => $this->clampTo === null + ? $dto + : $dto->mutate(['passDateChange' => $this->clampTo]) + ); + $importHelper = new ImportHelper( $this->accountService, $this->categoryService, $this->clientService, $this->createStub(TagService::class), - $this->createStub(ConfigService::class) + $this->createStub(ConfigService::class), + $this->accountPresetService ); $crypt = $this->createStub(CryptInterface::class); diff --git a/tests/Unit/Application/Import/Services/KeepassImportTest.php b/tests/Unit/Application/Import/Services/KeepassImportTest.php index bb7e07057..a71322130 100644 --- a/tests/Unit/Application/Import/Services/KeepassImportTest.php +++ b/tests/Unit/Application/Import/Services/KeepassImportTest.php @@ -29,6 +29,7 @@ use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\MockObject\MockObject; +use SP\Application\Account\Ports\AccountPresetService; use SP\Application\Account\Ports\AccountService; use SP\Domain\Category\Models\Category; use SP\Application\Category\Ports\CategoryService; @@ -60,6 +61,7 @@ class KeepassImportTest extends UnitaryTestCase private AccountService|MockObject $accountService; private MockObject|CategoryService $categoryService; private ClientService|MockObject $clientService; + private AccountPresetService|MockObject $accountPresetService; /** * @throws Exception @@ -195,7 +197,8 @@ public function testDoImportWithEmptyGroupName() $this->categoryService, $this->clientService, $this->createStub(TagService::class), - $this->createStub(ConfigService::class) + $this->createStub(ConfigService::class), + $this->accountPresetService ); $keepassImport = new KeepassImport( @@ -216,12 +219,17 @@ protected function setUp(): void $this->categoryService = $this->createMock(CategoryService::class); $this->clientService = $this->createMock(ClientService::class); + $this->accountPresetService = $this->createStub(AccountPresetService::class); + // The lifetime clamp runs on import; by default it changes nothing. + $this->accountPresetService->method('checkPasswordExpiry')->willReturnArgument(0); + $importHelper = new ImportHelper( $this->accountService, $this->categoryService, $this->clientService, $this->createStub(TagService::class), - $this->createStub(ConfigService::class) + $this->createStub(ConfigService::class), + $this->accountPresetService ); $crypt = $this->createStub(CryptInterface::class); diff --git a/tests/Unit/Application/Import/Services/SyspassImportTest.php b/tests/Unit/Application/Import/Services/SyspassImportTest.php index cd8d61693..48b5d13e7 100644 --- a/tests/Unit/Application/Import/Services/SyspassImportTest.php +++ b/tests/Unit/Application/Import/Services/SyspassImportTest.php @@ -35,6 +35,7 @@ use RuntimeException; use SP\Infrastructure\Crypt\Crypt; use SP\Domain\Account\Dtos\AccountCreateDto; +use SP\Application\Account\Ports\AccountPresetService; use SP\Application\Account\Ports\AccountService; use SP\Domain\Category\Models\Category; use SP\Application\Category\Ports\CategoryService; @@ -74,6 +75,7 @@ class SyspassImportTest extends UnitaryTestCase private CryptInterface|MockObject $crypt; private SyspassImport $sysPassImport; private ConfigService|MockObject $configService; + private AccountPresetService|MockObject $accountPresetService; /** * @throws ImportException @@ -594,7 +596,8 @@ public function testDoImportWithEncryptedFile() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -679,7 +682,8 @@ public function testDoImportWithEncryptedFileAndCryptoException() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -747,7 +751,8 @@ public function testDoImportRefusesEncryptedFileWithNoPasswordSupplied() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -789,7 +794,8 @@ public function testDoImportDecryptsRawDataForModernFormatWithoutBase64Decoding( $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -841,7 +847,8 @@ public function testDoImportRejectsFilesEncryptedByOldSysPassVersions() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -891,7 +898,8 @@ public function testDoImportTreatsUndecodableDecryptedDataAsWrongPassword() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -951,7 +959,8 @@ public function testDoImportAbortsWhenAccountReferencesUnknownCategory() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -1001,7 +1010,8 @@ public function testDoImportAbortsWhenAccountReferencesUnknownClient() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -1067,7 +1077,8 @@ public function testDoImportRejectsEncryptedAccountsFromOldSysPassVersions() $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $document = new DOMDocument(); @@ -1229,13 +1240,18 @@ protected function setUp(): void $this->clientService = $this->createMock(ClientService::class); $this->tagService = $this->createMock(TagService::class); $this->configService = $this->createMock(ConfigService::class); + $this->accountPresetService = $this->createStub(AccountPresetService::class); + // The lifetime clamp is applied on import; by default it changes nothing. + $this->accountPresetService->method('checkPasswordExpiry') + ->willReturnArgument(0); $importHelper = new ImportHelper( $this->accountService, $this->categoryService, $this->clientService, $this->tagService, - $this->configService + $this->configService, + $this->accountPresetService ); $this->crypt = $this->createMock(CryptInterface::class);