diff --git a/.github/workflows/behat-mariadb.yml b/.github/workflows/behat-mariadb.yml index 697a379da4..bec4b96e9e 100644 --- a/.github/workflows/behat-mariadb.yml +++ b/.github/workflows/behat-mariadb.yml @@ -129,9 +129,11 @@ jobs: ./occ app:enable --force notifications git clone --depth 1 -b ${{ matrix.server-versions }} https://github.com/nextcloud/activity apps/activity ./occ app:enable --force activity + ./occ app:enable --force guests ./occ config:system:set mail_smtpport --value 1025 --type integer ./occ config:system:set mail_smtphost --value mailhog ./occ config:system:set allow_local_remote_servers --value true --type boolean + ./occ config:system:set auth.bruteforce.protection.enabled --value false --type boolean - name: Run behat working-directory: apps/${{ env.APP_NAME }}/tests/integration diff --git a/.github/workflows/behat-mysql.yml b/.github/workflows/behat-mysql.yml index 87d0b8ec5c..9a2588fb93 100644 --- a/.github/workflows/behat-mysql.yml +++ b/.github/workflows/behat-mysql.yml @@ -137,6 +137,7 @@ jobs: ./occ app:enable --force notifications git clone --depth 1 -b ${{ matrix.server-versions }} https://github.com/nextcloud/activity apps/activity ./occ app:enable --force activity + ./occ app:enable --force guests ./occ config:system:set mail_smtpport --value 1025 --type integer ./occ config:system:set mail_smtphost --value mailhog ./occ config:system:set allow_local_remote_servers --value true --type boolean diff --git a/.github/workflows/behat-pgsql.yml b/.github/workflows/behat-pgsql.yml index 440cd0f571..7cd1878280 100644 --- a/.github/workflows/behat-pgsql.yml +++ b/.github/workflows/behat-pgsql.yml @@ -133,6 +133,7 @@ jobs: ./occ app:enable --force notifications git clone --depth 1 -b ${{ matrix.server-versions }} https://github.com/nextcloud/activity apps/activity ./occ app:enable --force activity + ./occ app:enable --force guests ./occ config:system:set mail_smtpport --value 1025 --type integer ./occ config:system:set mail_smtphost --value mailhog ./occ config:system:set allow_local_remote_servers --value true --type boolean diff --git a/.github/workflows/behat-sqlite.yml b/.github/workflows/behat-sqlite.yml index e3d6f9734f..a89714f635 100644 --- a/.github/workflows/behat-sqlite.yml +++ b/.github/workflows/behat-sqlite.yml @@ -124,6 +124,7 @@ jobs: ./occ app:enable --force notifications git clone --depth 1 -b ${{ matrix.server-versions }} https://github.com/nextcloud/activity apps/activity ./occ app:enable --force activity + ./occ app:enable --force guests ./occ config:system:set mail_smtpport --value 1025 --type integer ./occ config:system:set mail_smtphost --value mailhog ./occ config:system:set allow_local_remote_servers --value true --type boolean diff --git a/lib/Controller/AccountController.php b/lib/Controller/AccountController.php index b69a4b5aa2..aa6e7c5c32 100644 --- a/lib/Controller/AccountController.php +++ b/lib/Controller/AccountController.php @@ -297,6 +297,7 @@ public function createSignatureElement(array $elements): JSONResponse { #[NoAdminRequired] #[NoCSRFRequired] + #[PublicPage] public function getSignatureElements(): JSONResponse { $userId = $this->userSession->getUser()?->getUID(); try { diff --git a/lib/Service/AccountService.php b/lib/Service/AccountService.php index 9221398202..fa908dcc25 100644 --- a/lib/Service/AccountService.php +++ b/lib/Service/AccountService.php @@ -50,6 +50,7 @@ use OCP\Files\Folder; use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; +use OCP\Files\NotFoundException; use OCP\Http\Client\IClientService; use OCP\IConfig; use OCP\IGroupManager; @@ -308,14 +309,17 @@ public function getPdfByUuid(string $uuid): File { public function getFileByNodeIdAndSessionId(int $nodeId, string $sessionId): File { $rootSignatureFolder = $this->folderService->getFolder(); if (!$rootSignatureFolder->nodeExists($sessionId)) { - throw new DoesNotExistException('Not found'); + try { + return $this->folderService->getFileById($nodeId); + } catch (NotFoundException $th) { + throw new DoesNotExistException('Not found'); + } } - $nodes = $rootSignatureFolder->getById($nodeId); - if (empty($nodes)) { + try { + return $this->folderService->getFileById($nodeId); + } catch (NotFoundException $th) { throw new DoesNotExistException('Not found'); } - $file = current($nodes); - return $file; } public function canRequestSign(?IUser $user = null): bool { @@ -380,9 +384,7 @@ private function updateFileOfVisibleElement(array $data): void { return; } $userElement = $this->userElementMapper->findOne(['id' => $data['elementId']]); - $userFolder = $this->folderService->getFolder($userElement->getFileId()); - /** @var \OCP\Files\File */ - $file = $userFolder->getById($userElement->getFileId())[0]; + $file = $this->folderService->getFileById($userElement->getFileId()); $file->putContent($this->getFileRaw($data)); } @@ -475,9 +477,10 @@ public function deleteSignatureElement(?IUser $user, string $sessionId, int $nod 'user_id' => $user->getUID(), ]); $this->userElementMapper->delete($element); - $file = $this->root->getById($element->getFileId()); - if ($file) { - current($file)->delete(); + try { + $file = $this->folderService->getFileById($element->getFileId()); + $file->delete(); + } catch (NotFoundException $e) { } } else { $rootSignatureFolder = $this->folderService->getFolder(); diff --git a/lib/Service/FolderService.php b/lib/Service/FolderService.php index 0e056221f4..1255ccd0b4 100644 --- a/lib/Service/FolderService.php +++ b/lib/Service/FolderService.php @@ -28,6 +28,7 @@ use OCP\AppFramework\Services\IAppConfig; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\Config\IUserMountCache; +use OCP\Files\File; use OCP\Files\Folder; use OCP\Files\IAppData; use OCP\Files\IRootFolder; @@ -62,37 +63,37 @@ public function getUserId(): ?string { } /** - * Get folder for user + * Get folder for user and creates it if non-existent * * @psalm-suppress MixedReturnStatement - * @psalm-suppress InvalidReturnStatement - * @psalm-suppress MixedMethodCall + * @throws NotFoundException + * @throws NotPermittedException */ - public function getFolder(int $nodeId = null): Folder { - if ($nodeId) { - $mountsContainingFile = $this->userMountCache->getMountsForFileId($nodeId); - foreach ($mountsContainingFile as $fileInfo) { - $this->root->getByIdInPath($nodeId, $fileInfo->getMountPoint()); - } - $node = $this->root->getById($nodeId); - if (!$node) { - throw new \Exception('Invalid node'); - } - return $node[0]->getParent(); + public function getFolder(): Folder { + $path = $this->getLibreSignDefaultPath(); + $containerFolder = $this->getContainerFolder(); + if (!$containerFolder->nodeExists($path)) { + return $containerFolder->newFolder($path); } - - return $this->getOrCreateFolder(); + return $containerFolder->get($path); } /** - * Finds a folder and creates it if non-existent - * - * @psalm-suppress MixedReturnStatement * @throws NotFoundException - * @throws NotPermittedException */ - private function getOrCreateFolder(): Folder { + public function getFileById(int $nodeId = null): File { $path = $this->getLibreSignDefaultPath(); + $containerFolder = $this->getContainerFolder(); + if (!$containerFolder->nodeExists($path)) { + throw new NotFoundException('Invalid node'); + } + /** @var Folder $folder */ + $folder = $containerFolder->get($path); + $file = $folder->getById($nodeId); + return current($file); + } + + private function getContainerFolder(): Folder { $withoutPermission = false; if ($this->getUserId()) { $containerFolder = $this->root->getUserFolder($this->getUserId()); @@ -110,22 +111,12 @@ private function getOrCreateFolder(): Folder { $reflection = new \ReflectionClass($containerFolder); $reflectionProperty = $reflection->getProperty('folder'); $reflectionProperty->setAccessible(true); - $containerFolder = $reflectionProperty->getValue($containerFolder); - } else { - $containerFolder = $this->root->getUserFolder($this->getUserId()); + return $reflectionProperty->getValue($containerFolder); } - if ($containerFolder->nodeExists($path)) { - $folder = $containerFolder->get($path); - } else { - $folder = $containerFolder->newFolder($path); - } - return $folder; + return $this->root->getUserFolder($this->getUserId()); } - /** - * @psalm-suppress MixedReturnStatement - */ - public function getLibreSignDefaultPath(): string { + private function getLibreSignDefaultPath(): string { if (!$this->userId) { return 'unauthenticated'; } diff --git a/lib/Service/SignFileService.php b/lib/Service/SignFileService.php index 37408d54b6..afff9c206e 100644 --- a/lib/Service/SignFileService.php +++ b/lib/Service/SignFileService.php @@ -246,20 +246,15 @@ public function setVisibleElements(array $list): self { } try { if ($this->user instanceof IUser) { - $mountsContainingFile = $this->userMountCache->getMountsForFileId($nodeId); - foreach ($mountsContainingFile as $fileInfo) { - $this->root->getByIdInPath($nodeId, $fileInfo->getMountPoint()); - } - /** @var \OCP\Files\File[] */ - $node = $this->root->getById($nodeId); + $node = $this->folderService->getFileById($nodeId); } else { $filesOfElementes = $this->signerElementsService->getElementsFromSession(); $node = array_filter($filesOfElementes, fn ($file) => $file->getId() === $nodeId); + $node = current($node); } if (!$node) { throw new \Exception('empty'); } - $node = current($node); } catch (\Throwable $th) { throw new LibresignException($this->l10n->t('You need to define a visible signature or initials to sign this document.')); } diff --git a/lib/Service/SignerElementsService.php b/lib/Service/SignerElementsService.php index cb3acf70ff..f656025a66 100644 --- a/lib/Service/SignerElementsService.php +++ b/lib/Service/SignerElementsService.php @@ -52,7 +52,10 @@ public function getUserElementByNodeId(string $userId, $nodeId): array { 'id' => $element->getId(), 'type' => $element->getType(), 'file' => [ - 'url' => $this->urlGenerator->linkToRoute('core.Preview.getPreviewByFileId', ['fileId' => $element->getFileId(), 'x' => self::ELEMENT_SIGN_WIDTH, 'y' => self::ELEMENT_SIGN_HEIGHT]), + 'url' => $this->urlGenerator->linkToRoute('ocs.libresign.account.getSignatureElementPreview', [ + 'apiVersion' => 'v1', + 'nodeId' => $element->getFileId(), + ]), 'nodeId' => $element->getFileId() ], 'uid' => $element->getUserId(), @@ -73,7 +76,10 @@ public function getUserElements(string $userId): array { 'id' => $element->getId(), 'type' => $element->getType(), 'file' => [ - 'url' => $this->urlGenerator->linkToRoute('core.Preview.getPreviewByFileId', ['fileId' => $element->getFileId(), 'x' => self::ELEMENT_SIGN_WIDTH, 'y' => self::ELEMENT_SIGN_HEIGHT]), + 'url' => $this->urlGenerator->linkToRoute('ocs.libresign.account.getSignatureElementPreview', [ + 'apiVersion' => 'v1', + 'nodeId' => $element->getFileId(), + ]), 'nodeId' => $element->getFileId() ], 'starred' => $element->getStarred() ? 1 : 0, @@ -85,7 +91,7 @@ public function getUserElements(string $userId): array { private function signatureFileExists(UserElement $userElement): bool { try { - $this->folderService->getFolder($userElement->getFileId()); + $this->folderService->getFileById($userElement->getFileId()); } catch (\Exception $e) { $this->userElementMapper->delete($userElement); return false; diff --git a/lib/Service/TFile.php b/lib/Service/TFile.php index c1165f5695..7231e66bbb 100644 --- a/lib/Service/TFile.php +++ b/lib/Service/TFile.php @@ -42,8 +42,7 @@ public function getNodeFromData(array $data): Node { return $data['file']['fileNode']; } if (isset($data['file']['fileId'])) { - $userFolder = $this->folderService->getFolder($data['file']['fileId']); - return $userFolder->getById($data['file']['fileId'])[0]; + return $this->folderService->getFileById($data['file']['fileId']); } if (isset($data['file']['path'])) { return $this->folderService->getFileByPath($data['file']['path']); diff --git a/tests/Unit/Service/FolderServiceTest.php b/tests/Unit/Service/FolderServiceTest.php index 4f3d7b429c..646dd481db 100644 --- a/tests/Unit/Service/FolderServiceTest.php +++ b/tests/Unit/Service/FolderServiceTest.php @@ -2,69 +2,135 @@ namespace OCA\Libresign\Tests\Unit\Service; +use Exception; use OCA\Libresign\Service\FolderService; use OCP\AppFramework\Services\IAppConfig; use OCP\Files\AppData\IAppDataFactory; use OCP\Files\Config\IUserMountCache; +use OCP\Files\Folder; +use OCP\Files\IAppData; use OCP\Files\IRootFolder; +use OCP\Files\SimpleFS\ISimpleFile; +use OCP\Files\SimpleFS\ISimpleFolder; use OCP\IGroupManager; use OCP\IL10N; +use PHPUnit\Framework\MockObject\MockObject; + +final class FakeFolder implements ISimpleFolder { + public Folder $folder; + + public function getName(): string { + return 'fake'; + } + + public function getDirectoryListing(): array { + return []; + } + + public function delete(): void { + } + + public function fileExists(string $name): bool { + return false; + } + + public function getFile(string $name): ISimpleFile { + throw new Exception('fake class'); + } + + public function newFile(string $name, $content = null): ISimpleFile { + throw new Exception('fake class'); + } + + public function getFolder(string $name): ISimpleFolder { + throw new Exception('fake class'); + } + + public function newFolder(string $path): ISimpleFolder { + throw new Exception('fake class'); + } +} final class FolderServiceTest extends \OCA\Libresign\Tests\Unit\TestCase { - public function testGetFolderWithInvalidNodeId() { - $folder = $this->createMock(\OCP\Files\Folder::class); - $root = $this->createMock(IRootFolder::class); - $root - ->method('getUserFolder') - ->willReturn($folder); - $userMountCache = $this->createMock(IUserMountCache::class); - $userMountCache - ->method('getMountsForFileId') - ->willreturn([]); - $appDataFactory = $this->createMock(IAppDataFactory::class); - $groupManager = $this->createMock(IGroupManager::class); - $appConfig = $this->createMock(IAppConfig::class); - $l10n = $this->createMock(IL10N::class); + private IRootFolder|MockObject $root; + private IUserMountCache|MockObject $userMountCache; + private IAppDataFactory|MockObject $appDataFactory; + private IGroupManager|MockObject $groupManager; + private IAppConfig|MockObject $appConfig; + private IL10N|MockObject $l10n; + + public function setUp(): void { + parent::setUp(); + $this->root = $this->createMock(IRootFolder::class); + $this->userMountCache = $this->createMock(IUserMountCache::class); + $this->appDataFactory = $this->createMock(IAppDataFactory::class); + $this->groupManager = $this->createMock(IGroupManager::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->l10n = $this->createMock(IL10N::class); + } + private function getFolderService(?int $userId = 171): FolderSErvice { $service = new FolderService( - $root, - $userMountCache, - $appDataFactory, - $groupManager, - $appConfig, - $l10n, - 171 + $this->root, + $this->userMountCache, + $this->appDataFactory, + $this->groupManager, + $this->appConfig, + $this->l10n, + $userId ); - $this->expectExceptionMessage('Invalid node'); + return $service; + } + + public function testGetFolderAsUnauthenticatedWhenUserIdIsInvalid() { + $folder = $this->createMock(\OCP\Files\Folder::class); + $folder->method('nodeExists') + ->with($this->equalTo('unauthenticated')) + ->willReturn(true); + $folder->method('get')->willReturn($folder); + $fakeFolder = new FakeFolder(); + $fakeFolder->folder = $folder; + $appData = $this->createMock(IAppData::class); + $appData->method('getFolder')->willReturn($fakeFolder); + $this->appDataFactory->method('get')->willReturn($appData); + + $service = $this->getFolderService(null); $service->getFolder(171); + $this->assertTrue(true); + } + + public function testGetFileWithInvalidNodeId() { + $folder = $this->createMock(\OCP\Files\Folder::class); + $folder->method('isUpdateable')->willReturn(true); + $this->root->method('getUserFolder')->willReturn($folder); + + $service = $this->getFolderService(); + $this->expectExceptionMessage('Invalid node'); + $service->getFileById(171); } public function testGetFolderWithValidNodeId() { - $userMountCache = $this->createMock(IUserMountCache::class); - $userMountCache + $this->userMountCache ->method('getMountsForFileId') ->willreturn([]); $node = $this->createMock(\OCP\Files\File::class); $folder = $this->createMock(\OCP\Files\Folder::class); $node->method('getParent') ->willReturn($folder); - $root = $this->createMock(IRootFolder::class); - $root->method('getById') - ->willReturn([$node]); - $appDataFactory = $this->createMock(IAppDataFactory::class); - $groupManager = $this->createMock(IGroupManager::class); - $appConfig = $this->createMock(IAppConfig::class); - $l10n = $this->createMock(IL10N::class); + $this->root->method('getUserFolder') + ->willReturn($node); - $service = new FolderService( - $root, - $userMountCache, - $appDataFactory, - $groupManager, - $appConfig, - $l10n, - 1 - ); + $folder->method('nodeExists')->willReturn(true); + $folder->method('get')->willReturn($folder); + $fakeFolder = new FakeFolder(); + $fakeFolder->folder = $folder; + $appData = $this->createMock(IAppData::class); + $appData->method('getFolder') + ->willReturn($fakeFolder); + $this->appDataFactory->method('get') + ->willReturn($appData); + + $service = $this->getFolderService(1); $actual = $service->getFolder(171); $this->assertInstanceOf(\OCP\Files\Folder::class, $actual); } diff --git a/tests/integration/features/account/signature.feature b/tests/integration/features/account/signature.feature index ea644d8a64..7115ff518b 100644 --- a/tests/integration/features/account/signature.feature +++ b/tests/integration/features/account/signature.feature @@ -119,3 +119,49 @@ Feature: account/signature And the response should be a JSON array with the following mandatory values | key | value | | message | Certificate file deleted with success. | + + Scenario: Create password to guest + Given guest "guest@test.coop" exists + And run the command "config:app:set guests whitelist --value libresign" with result code 0 + And as user "guest@test.coop" + And sending "post" to ocs "/apps/libresign/api/v1/account/signature" + | signPassword | password | + Then the response should have a status code 200 + + Scenario: CRUD of signature element to guest + Given guest "guest@test.coop" exists + And run the command "config:app:set guests whitelist --value libresign" with result code 0 + And as user "guest@test.coop" + When sending "post" to ocs "/apps/libresign/api/v1/account/signature/elements" + | elements | [{"type":"signature","file":{"base64":"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="}}] | + Then the response should have a status code 200 + When sending "get" to ocs "/apps/libresign/api/v1/account/signature/elements" + Then the response should be a JSON array with the following mandatory values + | key | value | + | elements | (jq).[]\|.type == "signature" | + And fetch field "(NODE_ID)elements.0.file.nodeId" from prevous JSON response + When sending "delete" to ocs "/apps/libresign/api/v1/account/signature/elements/" + Then the response should have a status code 200 + + Scenario: CRUD of signature element to signer by email without account + Given guest "guest@test.coop" exists + And run the command "config:app:set guests whitelist --value libresign" with result code 0 + And run the command "libresign:configure:openssl --cn test" with result code 0 + And as user "admin" + And sending "post" to ocs "/apps/provisioning_api/api/v1/config/apps/libresign/identify_methods" + | value | (string)[{"name":"email","enabled":true,"mandatory":true,"can_create_account":false}] | + And sending "post" to ocs "/apps/libresign/api/v1/request-signature" + | file | {"url":"/apps/libresign/develop/pdf"} | + | users | [{"identify":{"email":"guest@test.coop"}}] | + | name | document | + And as user "" + When sending "post" to ocs "/apps/libresign/api/v1/account/signature/elements" + | elements | [{"type":"signature","file":{"base64":"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="}}] | + Then the response should have a status code 200 + When sending "get" to ocs "/apps/libresign/api/v1/account/signature/elements" + Then the response should be a JSON array with the following mandatory values + | key | value | + | elements | (jq).[]\|.type == "signature" | + And fetch field "(NODE_ID)elements.0.file.nodeId" from prevous JSON response + When sending "delete" to ocs "/apps/libresign/api/v1/account/signature/elements/" + Then the response should have a status code 200 diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index f0a79259fd..6e82696022 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -14,6 +14,7 @@ class FeatureContext extends NextcloudApiContext implements OpenedEmailStorageAwareContext { private array $signer = []; private array $file = []; + private static array $environments = []; private OpenedEmailStorage $openedEmailStorage; /** @@ -31,6 +32,7 @@ public static function beforeSuite(BeforeSuiteScope $scope) { * @BeforeScenario */ public static function BeforeScenario(): void { + self::$environments = []; self::runCommand('libresign:developer:reset --all'); } @@ -44,6 +46,9 @@ public static function runCommand(string $command): array { if (posix_getuid() !== $owner['uid']) { $fullCommand = 'runuser -u ' . $owner['name'] . ' -- ' . $fullCommand; } + if (!empty(self::$environments)) { + $fullCommand = http_build_query(self::$environments, '', ' ') . ' ' . $fullCommand; + } $fullCommand .= ' 2>&1'; exec($fullCommand, $output, $resultCode); return [ @@ -57,7 +62,30 @@ public static function runCommand(string $command): array { */ public static function runCommandWithResultCode(string $command, int $resultCode = 0): void { $return = self::runCommand($command); - Assert::assertEquals($resultCode, $return['resultCode']); + Assert::assertEquals($resultCode, $return['resultCode'], print_r($return, true)); + } + + /** + * @Given create an environment :name with value :value to be used by occ command + */ + public static function createAnEnvironmentWithValueToBeUsedByOccCommand($name, $value) { + self::$environments[$name] = $value; + } + + /** + * @When guest :guest exists + * @param string $guest + */ + public function assureGuestExists(string $guest): void { + $response = $this->userExists($guest); + if ($response->getStatusCode() !== 200) { + $this->createAnEnvironmentWithValueToBeUsedByOccCommand('OC_PASS', '123456'); + $this->runCommandWithResultCode('guests:add admin ' . $guest . ' --password-from-env', 0); + // Set a display name different than the user ID to be able to + // ensure in the tests that the right value was returned. + $this->setUserDisplayName($guest); + $this->createdUsers[] = $guest; + } } public function setOpenedEmailStorage(OpenedEmailStorage $storage): void {