From 110604fda94c366ed152643321e8dd677507c0ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D?= Date: Sun, 3 Mar 2024 12:24:59 +0100 Subject: [PATCH] chore(tests): UT for KeepassImport service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rubén D --- lib/SP/Domain/Account/Dtos/AccountDto.php | 2 +- lib/SP/Domain/Common/Dtos/Dto.php | 32 +++- .../Exceptions/NoSuchPropertyException.php | 6 +- lib/SP/Domain/Import/Services/ImportBase.php | 67 +++---- .../Domain/Import/Services/ImportHelper.php | 1 - .../Domain/Import/Services/KeepassImport.php | 80 ++++---- .../Domain/Import/Services/SyspassImport.php | 2 +- .../Domain/Import/Services/CsvImportTest.php | 1 - .../Import/Services/KeepassImportTest.php | 172 ++++++++++++++++++ 9 files changed, 285 insertions(+), 78 deletions(-) create mode 100644 tests/SPT/Domain/Import/Services/KeepassImportTest.php diff --git a/lib/SP/Domain/Account/Dtos/AccountDto.php b/lib/SP/Domain/Account/Dtos/AccountDto.php index d56594ac..8307c960 100644 --- a/lib/SP/Domain/Account/Dtos/AccountDto.php +++ b/lib/SP/Domain/Account/Dtos/AccountDto.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2022, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2024, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * diff --git a/lib/SP/Domain/Common/Dtos/Dto.php b/lib/SP/Domain/Common/Dtos/Dto.php index d4d64511..f2503bf1 100644 --- a/lib/SP/Domain/Common/Dtos/Dto.php +++ b/lib/SP/Domain/Common/Dtos/Dto.php @@ -57,7 +57,7 @@ abstract class Dto */ public function set(string $property, mixed $value): static|null { - if (property_exists($this, $property) && !in_array($property, $this->reservedProperties)) { + if ($this->checkProperty($property)) { $self = clone $this; $self->{$property} = $value; @@ -66,4 +66,34 @@ abstract class Dto return null; } + + private function checkProperty(string $property): bool + { + return property_exists($this, $property) && !in_array($property, $this->reservedProperties); + } + + /** + * Set any properties in bacth mode. This allows to set any property from dynamic calls. + * + * @param string[] $properties + * @param array $values + * + * @return Dto Returns a new instance with the poperties set. + */ + public function setBatch(array $properties, array $values): static + { + $self = clone $this; + + $filteredProperties = array_filter( + array_combine($properties, $values), + fn($key) => is_string($key) && $this->checkProperty($key), + ARRAY_FILTER_USE_KEY + ); + + foreach ($filteredProperties as $property => $value) { + $self->{$property} = $value; + } + + return $self; + } } diff --git a/lib/SP/Domain/Core/Exceptions/NoSuchPropertyException.php b/lib/SP/Domain/Core/Exceptions/NoSuchPropertyException.php index 3d266f5e..a5bec850 100644 --- a/lib/SP/Domain/Core/Exceptions/NoSuchPropertyException.php +++ b/lib/SP/Domain/Core/Exceptions/NoSuchPropertyException.php @@ -4,7 +4,7 @@ * * @author nuxsmin * @link https://syspass.org - * @copyright 2012-2023, Rubén Domínguez nuxsmin@$syspass.org + * @copyright 2012-2024, Rubén Domínguez nuxsmin@$syspass.org * * This file is part of sysPass. * @@ -24,14 +24,12 @@ namespace SP\Domain\Core\Exceptions; -use Exception; - /** * Class NoSuchPropertyException * * @package SP\Domain\Core\Exceptions */ -class NoSuchPropertyException extends Exception +class NoSuchPropertyException extends SPException { } diff --git a/lib/SP/Domain/Import/Services/ImportBase.php b/lib/SP/Domain/Import/Services/ImportBase.php index 4bc9acfb..b262f82e 100644 --- a/lib/SP/Domain/Import/Services/ImportBase.php +++ b/lib/SP/Domain/Import/Services/ImportBase.php @@ -66,7 +66,6 @@ abstract class ImportBase extends Service implements ImportService protected readonly ClientService $clientService; protected readonly TagServiceInterface $tagService; protected readonly ConfigService $configService; - private array $items; private array $cache; public function __construct( @@ -101,37 +100,43 @@ abstract class ImportBase extends Service implements ImportService * @throws SPException * @throws CryptException */ - final protected function addAccount(AccountCreateDto $accountCreateDto, ImportParamsDto $importParams): void - { + final protected function addAccount( + AccountCreateDto $accountCreateDto, + ImportParamsDto $importParams, + bool $useEncryption = false + ): void { if (empty($accountCreateDto->getCategoryId())) { - throw new ImportException(__u('Category Id not set. Unable to import account.')); + throw ImportException::error(__u('Category Id not set. Unable to import account.')); } if (empty($accountCreateDto->getClientId())) { - throw new ImportException(__u('Client Id not set. Unable to import account.')); + throw ImportException::error(__u('Client Id not set. Unable to import account.')); } - $hasValidHash = $this->getOrSetCache( - self::ITEM_MASTER_PASS_HASH, - '', - fn() => $this->validateHash($importParams) + $dto = $accountCreateDto->setBatch( + ['userId', 'userGroupId'], + [$importParams->getDefaultUser(), $importParams->getDefaultGroup()] ); - $dto = $accountCreateDto - ->set('userId', $importParams->getDefaultUser()) - ->set('userGroupId', $importParams->getDefaultGroup()); + if ($useEncryption) { + $hasValidHash = $this->getOrSetCache( + self::ITEM_MASTER_PASS_HASH, + 'current', + fn() => $this->validateHash($importParams) + ); - if ($hasValidHash === false && !empty($importParams->getMasterPassword())) { - if ($this->version >= 210) { - $pass = $this->crypt->decrypt( - $accountCreateDto->getPass(), - $accountCreateDto->getKey(), - $importParams->getMasterPassword() - ); + if ($hasValidHash === true && !empty($importParams->getMasterPassword())) { + if ($this->version >= 210) { + $pass = $this->crypt->decrypt( + $accountCreateDto->getPass(), + $accountCreateDto->getKey(), + $importParams->getMasterPassword() + ); - $dto = $accountCreateDto->set('pass', $pass)->set('key', ''); - } else { - throw ImportException::error(__u('The file was exported with an old sysPass version (<= 2.10).')); + $dto = $accountCreateDto->setBatch(['pass', 'key'], [$pass, '']); + } else { + throw ImportException::error(__u('The file was exported with an old sysPass version (<= 2.10).')); + } } } @@ -167,7 +172,7 @@ abstract class ImportBase extends Service implements ImportService $importParams->getMasterPassword(), $this->configService->getByParam('masterPwd') ); - } catch (NoSuchItemException $e) { + } catch (NoSuchItemException) { return false; } } @@ -184,8 +189,8 @@ abstract class ImportBase extends Service implements ImportService return $this->getOrSetCache( self::ITEM_CATEGORY, $category->getName(), - fn(): ?int => $this->categoryService->getByName($category->getName())?->getId() - ?: $this->categoryService->create($category) + fn(): int => $this->categoryService->getByName($category->getName())?->getId() + ?? $this->categoryService->create($category) ); } @@ -195,14 +200,12 @@ abstract class ImportBase extends Service implements ImportService */ protected function addClient(Client $client): int { - $clientId = $this->getOrSetCache( + return $this->getOrSetCache( self::ITEM_CLIENT, $client->getName(), - fn(): ?int => $this->clientService->getByName($client->getName())?->getId() - ?: $this->clientService->create($client) + fn(): int => $this->clientService->getByName($client->getName())?->getId() + ?? $this->clientService->create($client) ); - - return $clientId ?? $this->clientService->create($client); } /** @@ -213,8 +216,8 @@ abstract class ImportBase extends Service implements ImportService return $this->getOrSetCache( self::ITEM_TAG, $tag->getId(), - fn(): ?int => $this->tagService->getByName($tag->getName())?->getId() - ?: $this->tagService->create($tag) + fn(): int => $this->tagService->getByName($tag->getName())?->getId() + ?? $this->tagService->create($tag) ); } } diff --git a/lib/SP/Domain/Import/Services/ImportHelper.php b/lib/SP/Domain/Import/Services/ImportHelper.php index b6b92d68..217bf75c 100644 --- a/lib/SP/Domain/Import/Services/ImportHelper.php +++ b/lib/SP/Domain/Import/Services/ImportHelper.php @@ -35,7 +35,6 @@ use SP\Domain\Tag\Ports\TagServiceInterface; */ class ImportHelper { - public function __construct( private readonly AccountService $accountService, private readonly CategoryService $categoryService, diff --git a/lib/SP/Domain/Import/Services/KeepassImport.php b/lib/SP/Domain/Import/Services/KeepassImport.php index 49db9d4b..86b2d7af 100644 --- a/lib/SP/Domain/Import/Services/KeepassImport.php +++ b/lib/SP/Domain/Import/Services/KeepassImport.php @@ -27,7 +27,6 @@ namespace SP\Domain\Import\Services; use CallbackFilterIterator; use DOMElement; use DOMXPath; -use Exception; use SP\Core\Events\Event; use SP\Core\Events\EventMessage; use SP\Domain\Account\Dtos\AccountCreateDto; @@ -51,7 +50,7 @@ final class KeepassImport extends XmlImportBase implements ItemsImportService /** * @var SplObjectStorage[] $items */ - private array $items = []; + private array $entries = []; /** * Iniciar la importación desde KeePass @@ -89,40 +88,19 @@ final class KeepassImport extends XmlImportBase implements ItemsImportService $this->getGroups(); $this->getEntries(); - foreach ($this->items as $groupName => $accounts) { - try { - foreach ($accounts as $account) { - $this->addAccount( - $account->set('clientId', $clientId), - $importParamsDto - ); - - $this->eventDispatcher->notify( - 'run.import.keepass.process.account', - new Event( - $this, - EventMessage::factory() - ->addDetail(__u('Account imported'), $account->getName()) - ->addDetail(__u('Category'), $groupName) - ) - ); - } - } catch (Exception $e) { - processException($e); - - $this->eventDispatcher->notify('exception', new Event($e)); - } + foreach ($this->entries as $groupName => $accounts) { + $this->processAccounts($accounts, $clientId, $importParamsDto, $groupName); } } + /** * @throws DuplicatedItemException * @throws SPException */ private function getGroups(): void { - $DomXpath = new DOMXPath($this->document); - $tags = $DomXpath->query('/KeePassFile/Root/Group//Group/Name'); + $tags = (new DOMXPath($this->document))->query('/KeePassFile/Root//Group/Name'); $nodesList = new CallbackFilterIterator( $tags->getIterator(), @@ -143,7 +121,7 @@ final class KeepassImport extends XmlImportBase implements ItemsImportService { $groupName = Filter::getString($groupName); - if (!isset($this->items[$groupName])) { + if (!isset($this->entries[$groupName])) { $this->addCategory(new Category(['name' => $groupName, 'description' => 'KeePass'])); $this->eventDispatcher->notify( @@ -151,7 +129,7 @@ final class KeepassImport extends XmlImportBase implements ItemsImportService new Event($this, EventMessage::factory()->addDetail(__u('Category imported'), $groupName)) ); - $this->items[$groupName] = new SplObjectStorage(); + $this->entries[$groupName] = new SplObjectStorage(); } } @@ -160,8 +138,8 @@ final class KeepassImport extends XmlImportBase implements ItemsImportService */ private function getEntries(): void { - $DomXpath = new DOMXPath($this->document); - $entries = $DomXpath->query('/KeePassFile/Root/Group//Entry[not(parent::History)]'); + $DOMXPath = new DOMXPath($this->document); + $entries = $DOMXPath->query('/KeePassFile/Root/Group//Entry[not(parent::History)]'); $nodesList = new CallbackFilterIterator( $entries->getIterator(), @@ -174,23 +152,23 @@ final class KeepassImport extends XmlImportBase implements ItemsImportService $entryData = []; /** @var DOMElement $string */ - foreach ($DomXpath->query($path . '/String') as $string) { + foreach ($DOMXPath->query($path . '/String') as $string) { $key = $string->childNodes->item(0)->nodeValue; $value = $string->childNodes->item(1)->nodeValue; $entryData[$key] = $value; } - $groupName = $DomXpath->query($path . '/../Name')->item(0)->nodeValue; + $groupName = $DOMXPath->query($path . '/../Name')->item(0)->nodeValue; - $this->getItem($groupName)?->attach($this->mapEntryToAccount($entryData, $groupName)); + $this->getEntryFor($groupName)?->attach($this->mapEntryToAccount($entryData, $groupName)); } } - private function getItem(string $groupName): ?SplObjectStorage + private function getEntryFor(string $groupName): ?SplObjectStorage { - if (array_key_exists($groupName, $this->items)) { - return $this->items[$groupName]; + if (array_key_exists($groupName, $this->entries)) { + return $this->entries[$groupName]; } return null; @@ -207,4 +185,32 @@ final class KeepassImport extends XmlImportBase implements ItemsImportService notes: Filter::getString($entry['Notes'] ?? '') ); } + + private function processAccounts( + SplObjectStorage $accounts, + int $clientId, + ImportParamsDto $importParamsDto, + string $groupName + ): void { + foreach ($accounts as $account) { + try { + $this->addAccount($account->set('clientId', $clientId), $importParamsDto); + + $this->eventDispatcher->notify( + 'run.import.keepass.process.account', + new Event( + $this, + EventMessage::factory() + ->addDetail(__u('Account imported'), $account->getName()) + ->addDetail(__u('Category'), $groupName) + ) + ); + } catch (SPException $e) { + processException($e); + + $this->eventDispatcher->notify('exception', new Event($e)); + } + } + } + } diff --git a/lib/SP/Domain/Import/Services/SyspassImport.php b/lib/SP/Domain/Import/Services/SyspassImport.php index d47651a4..9daa8422 100644 --- a/lib/SP/Domain/Import/Services/SyspassImport.php +++ b/lib/SP/Domain/Import/Services/SyspassImport.php @@ -420,7 +420,7 @@ final class SyspassImport extends XmlImportBase implements ItemsImportService } try { - $this->addAccount(AccountCreateDto::fromAccount(new Account($data)), $importParams); + $this->addAccount(AccountCreateDto::fromAccount(new Account($data)), $importParams, true); $this->eventDispatcher->notify( 'run.import.syspass.process.account', diff --git a/tests/SPT/Domain/Import/Services/CsvImportTest.php b/tests/SPT/Domain/Import/Services/CsvImportTest.php index 44d47fb9..874af442 100644 --- a/tests/SPT/Domain/Import/Services/CsvImportTest.php +++ b/tests/SPT/Domain/Import/Services/CsvImportTest.php @@ -313,7 +313,6 @@ class CsvImportTest extends UnitaryTestCase $this->accountService = $this->createMock(AccountService::class); $this->categoryService = $this->createMock(CategoryService::class); $this->clientService = $this->createMock(ClientService::class); - $this->tagService = $this->createMock(TagServiceInterface::class); $importHelper = new ImportHelper( $this->accountService, diff --git a/tests/SPT/Domain/Import/Services/KeepassImportTest.php b/tests/SPT/Domain/Import/Services/KeepassImportTest.php new file mode 100644 index 00000000..187c124a --- /dev/null +++ b/tests/SPT/Domain/Import/Services/KeepassImportTest.php @@ -0,0 +1,172 @@ +. + */ + +namespace SPT\Domain\Import\Services; + +use DOMDocument; +use PHPUnit\Framework\MockObject\Exception; +use PHPUnit\Framework\MockObject\MockObject; +use SP\Domain\Account\Ports\AccountService; +use SP\Domain\Category\Models\Category; +use SP\Domain\Category\Ports\CategoryService; +use SP\Domain\Client\Models\Client; +use SP\Domain\Client\Ports\ClientService; +use SP\Domain\Config\Ports\ConfigService; +use SP\Domain\Core\Crypt\CryptInterface; +use SP\Domain\Core\Exceptions\SPException; +use SP\Domain\Import\Dtos\ImportParamsDto; +use SP\Domain\Import\Services\ImportHelper; +use SP\Domain\Import\Services\KeepassImport; +use SP\Domain\Tag\Ports\TagServiceInterface; +use SPT\UnitaryTestCase; + +/** + * Class KeepassImportTest + * + * @group unitary + */ +class KeepassImportTest extends UnitaryTestCase +{ + + private const KEEPASS_FILE = RESOURCE_PATH . DIRECTORY_SEPARATOR . 'import' . DIRECTORY_SEPARATOR . + 'data_keepass.xml'; + private CryptInterface|MockObject $crypt; + private KeepassImport $keepassImport; + private AccountService|MockObject $accountService; + private MockObject|CategoryService $categoryService; + private ClientService|MockObject $clientService; + private TagServiceInterface|MockObject $tagService; + + /** + * @throws Exception + * @throws SPException + */ + public function testDoImport() + { + $importParamsDto = $this->createStub(ImportParamsDto::class); + + $this->clientService + ->expects(self::once()) + ->method('getByName') + ->with('KeePass') + ->willReturn(null); + + $this->clientService + ->expects(self::once()) + ->method('create') + ->with(self::callback(static fn(Client $client) => $client->getName() === 'KeePass')) + ->willReturn(100); + + $this->categoryService + ->expects(self::exactly(9)) + ->method('getByName') + ->willReturn(null); + + $this->categoryService + ->expects(self::exactly(9)) + ->method('create') + ->with( + self::callback( + static fn(Category $category) => !empty($category->getName()) && + $category->getDescription() === 'KeePass' + ) + ) + ->willReturn(200); + + $this->accountService + ->expects(self::exactly(5)) + ->method('create'); + + $this->keepassImport->doImport($importParamsDto); + } + + /** + * @throws Exception + * @throws SPException + */ + public function testDoImportWithAccountException() + { + $importParamsDto = $this->createStub(ImportParamsDto::class); + + $this->clientService + ->expects(self::once()) + ->method('getByName') + ->with('KeePass') + ->willReturn(null); + + $this->clientService + ->expects(self::once()) + ->method('create') + ->with(self::callback(static fn(Client $client) => $client->getName() === 'KeePass')) + ->willReturn(100); + + $this->categoryService + ->expects(self::exactly(9)) + ->method('getByName') + ->willReturn(null); + + $this->categoryService + ->expects(self::exactly(9)) + ->method('create') + ->with( + self::callback( + static fn(Category $category) => !empty($category->getName()) && + $category->getDescription() === 'KeePass' + ) + ) + ->willReturn(200); + + $this->accountService + ->expects(self::exactly(5)) + ->method('create') + ->willThrowException(SPException::error('test')); + + $this->keepassImport->doImport($importParamsDto); + } + + protected function setUp(): void + { + parent::setUp(); + + $this->accountService = $this->createMock(AccountService::class); + $this->categoryService = $this->createMock(CategoryService::class); + $this->clientService = $this->createMock(ClientService::class); + $this->tagService = $this->createMock(TagServiceInterface::class); + + $importHelper = new ImportHelper( + $this->accountService, + $this->categoryService, + $this->clientService, + $this->tagService, + $this->createMock(ConfigService::class) + ); + + $this->crypt = $this->createMock(CryptInterface::class); + + $document = new DOMDocument(); + $document->load(self::KEEPASS_FILE, LIBXML_NOBLANKS); + + $this->keepassImport = new KeepassImport($this->application, $importHelper, $this->crypt, $document); + } +}