chore(tests): UT for KeepassImport service

Signed-off-by: Rubén D <nuxsmin@syspass.org>
2026-03-03 07:04:07 +01:00 · 2024-03-03 12:24:59 +01:00
parent 7f84c30de8
commit 110604fda9
9 changed files with 285 additions and 78 deletions
--- 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.
 *
--- 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;
+    }
 }
--- 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
 {

 }
--- 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)
        );
    }
 }
--- 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,
--- 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<AccountCreateDto>[] $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));
+            }
+        }
+    }
+
 }
--- 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',
--- 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,
--- a/tests/SPT/Domain/Import/Services/KeepassImportTest.php
+++ b/tests/SPT/Domain/Import/Services/KeepassImportTest.php
@@ -0,0 +1,172 @@
+<?php
+/*
+ * sysPass
+ *
+ * @author nuxsmin
+ * @link https://syspass.org
+ * @copyright 2012-2024, Rubén Domínguez nuxsmin@$syspass.org
+ *
+ * This file is part of sysPass.
+ *
+ * sysPass is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * sysPass is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with sysPass.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+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);
+    }
+}