diff --git a/lib/Service/File/SigningWorkflowStatusResolver.php b/lib/Service/File/SigningWorkflowStatusResolver.php new file mode 100644 index 0000000000..2bd7196a63 --- /dev/null +++ b/lib/Service/File/SigningWorkflowStatusResolver.php @@ -0,0 +1,86 @@ +excludeIdDocUploaderPlaceholder($file, $signRequests); + $total = count($signers); + if ($total === 0) { + return null; + } + + $totalSigned = count(array_filter($signers, static fn (SignRequestEntity $signer): bool => $signer->getSigned() !== null)); + if ($totalSigned === $total) { + return FileStatus::SIGNED; + } + + if ($totalSigned > 0) { + $policy = $this->rejectionPolicyService->getPolicyValue($file); + if ($policy['enabled'] && !$policy['cancel_workflow']) { + $totalRejected = count(array_filter( + $signers, + static fn (SignRequestEntity $signer): bool => $signer->getStatusEnum() === SignRequestStatus::REJECTED, + )); + if ($totalSigned + $totalRejected === $total) { + return FileStatus::SIGNED; + } + } + return FileStatus::PARTIAL_SIGNED; + } + + return null; + } + + /** + * @param SignRequestEntity[] $signRequests + * @return SignRequestEntity[] + */ + private function excludeIdDocUploaderPlaceholder(FileEntity $file, array $signRequests): array { + $fileId = $file->getId(); + if ($fileId === null) { + return $signRequests; + } + try { + $idDocs = $this->idDocsMapper->getByFileId($fileId); + } catch (DoesNotExistException) { + return $signRequests; + } + if (!$idDocs instanceof IdDocs) { + return $signRequests; + } + $placeholderId = $idDocs->getSignRequestId(); + return array_values(array_filter( + $signRequests, + static fn (SignRequestEntity $signer): bool => $signer->getId() !== $placeholderId, + )); + } +} diff --git a/lib/Service/SignFileService.php b/lib/Service/SignFileService.php index c8a26de3b0..519e13a877 100644 --- a/lib/Service/SignFileService.php +++ b/lib/Service/SignFileService.php @@ -41,6 +41,7 @@ use OCA\Libresign\Helper\JSActions; use OCA\Libresign\Helper\ValidateHelper; use OCA\Libresign\Service\Envelope\EnvelopeStatusDeterminer; +use OCA\Libresign\Service\File\SigningWorkflowStatusResolver; use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\IToken; use OCA\Libresign\Service\Policy\PolicyService; @@ -129,6 +130,7 @@ public function __construct( private SignRequestService $signRequestService, private PolicyService $policyService, private TemporarySignatureImageService $temporarySignatureImageService, + private SigningWorkflowStatusResolver $signingWorkflowStatusResolver, ) { } @@ -1092,7 +1094,7 @@ protected function getSigners(): array { } protected function setNewStatusIfNecessary(FileEntity $libreSignFile): bool { - $newStatus = $this->evaluateStatusFromSigners(); + $newStatus = $this->evaluateStatusFromSigners($libreSignFile); if ($newStatus === null || $newStatus === $libreSignFile->getStatus()) { return false; @@ -1107,61 +1109,8 @@ private function updateEntityCacheAfterDbSave(FileEntity $file): void { $this->statusService->cacheFileStatus($file); } - private function evaluateStatusFromSigners(): ?int { - $signers = $this->excludeIdDocUploaderPlaceholder($this->getSigners()); - - $total = count($signers); - - if ($total === 0) { - return null; - } - - $totalSigned = count(array_filter($signers, fn ($s) => $s->getSigned() !== null)); - - if ($totalSigned === $total) { - return FileStatus::SIGNED->value; - } - - if ($totalSigned > 0) { - return FileStatus::PARTIAL_SIGNED->value; - } - - return null; - } - - /** - * @param SignRequestEntity[] $signers - * @return SignRequestEntity[] - */ - private function excludeIdDocUploaderPlaceholder(array $signers): array { - $placeholderId = $this->getIdDocUploaderSignRequestId(); - if ($placeholderId === null) { - return $signers; - } - - return array_values(array_filter( - $signers, - static fn (SignRequestEntity $signer): bool => $signer->getId() !== $placeholderId, - )); - } - - private function getIdDocUploaderSignRequestId(): ?int { - $fileId = $this->signRequest->getFileId(); - if ($fileId === null) { - return null; - } - - try { - $idDocs = $this->idDocsMapper->getByFileId($fileId); - } catch (DoesNotExistException) { - return null; - } - - if (!$idDocs instanceof IdDocs) { - return null; - } - - return $idDocs->getSignRequestId(); + private function evaluateStatusFromSigners(FileEntity $file): ?int { + return $this->signingWorkflowStatusResolver->resolve($file, $this->getSigners())?->value; } private function getOrGeneratePfxContent(SignEngineHandler $engine): string { diff --git a/lib/Service/SignatureRejection/SignatureRejectionService.php b/lib/Service/SignatureRejection/SignatureRejectionService.php index f39fc7baff..6b46fdb481 100644 --- a/lib/Service/SignatureRejection/SignatureRejectionService.php +++ b/lib/Service/SignatureRejection/SignatureRejectionService.php @@ -17,6 +17,7 @@ use OCA\Libresign\Enum\SignRequestStatus; use OCA\Libresign\Events\SignatureRejectedEvent; use OCA\Libresign\Exception\LibresignException; +use OCA\Libresign\Service\File\SigningWorkflowStatusResolver; use OCA\Libresign\Service\FileStatusService; use OCA\Libresign\Service\IdentifyMethodService; use OCP\AppFramework\Utility\ITimeFactory; @@ -46,6 +47,7 @@ public function __construct( private ITimeFactory $timeFactory, private IL10N $l10n, private LoggerInterface $logger, + private SigningWorkflowStatusResolver $statusResolver, ) { } @@ -107,6 +109,7 @@ private function persistRejection( ); $previousFileStatus = $libreSignFile->getStatus(); $rejectedAt = $this->timeFactory->getDateTime(); + $previousDocumentStates = []; $this->db->beginTransaction(); try { @@ -120,12 +123,20 @@ private function persistRejection( if ($workflowCanceled) { $this->cancelWorkflow($libreSignFile); + } else { + $this->resolveStatusOfRejectedDocuments($libreSignFile, $signRequests, $previousDocumentStates); } $this->db->commit(); } catch (\Throwable $e) { $this->db->rollBack(); $this->restoreState($libreSignFile, $signRequests, $previousSignerStatuses, $previousFileStatus); + foreach ($previousDocumentStates as $entry) { + $document = $entry['file']; + $document->setStatus($entry['status']); + $document->setMetadata($entry['metadata']); + $this->fileStatusService->invalidateCachedStatus($document); + } $this->logger->error('Failed to register the signature rejection: ' . $e->getMessage(), [ 'exception' => $e, 'signRequestId' => $signRequest->getId(), @@ -136,6 +147,59 @@ private function persistRejection( } } + /** + * Resolve the documents affected by this rejection in the same transaction. + * An envelope's final outcome is derived separately from its documents. + * + * @param list $signRequests + * @param array $previousDocumentStates + */ + private function resolveStatusOfRejectedDocuments( + FileEntity $libreSignFile, + array $signRequests, + array &$previousDocumentStates, + ): void { + foreach ($signRequests as $rejected) { + $fileId = $rejected->getFileId(); + if ($fileId === null || isset($previousDocumentStates[$fileId])) { + continue; + } + $document = $fileId === $libreSignFile->getId() + ? $libreSignFile + : $this->fileMapper->getById($fileId); + if ($document->isEnvelope()) { + continue; + } + $status = $this->statusResolver->resolve( + $document, + $this->signRequestMapper->getByFileId($fileId), + ); + if ($status === null || $status->value === $document->getStatus()) { + continue; + } + $previousDocumentStates[$fileId] = [ + 'file' => $document, + 'status' => $document->getStatus(), + 'metadata' => $document->getMetadata(), + ]; + if ($document->hasParent()) { + $parentId = $document->getParentFileId(); + if ($parentId !== null && !isset($previousDocumentStates[$parentId])) { + $parent = $libreSignFile->getId() === $parentId + ? $libreSignFile + : $this->fileMapper->getById($parentId); + $previousDocumentStates[$parentId] = [ + 'file' => $parent, + 'status' => $parent->getStatus(), + 'metadata' => $parent->getMetadata(), + ]; + } + } + $document->setStatusEnum($status); + $this->fileStatusService->update($document); + } + } + /** * A signer of an envelope has one signature request per document it contains, * plus one on the envelope itself. Signing closes all of them, so a rejection diff --git a/tests/integration/composer.json b/tests/integration/composer.json index ec16b88e49..8dfb3084ec 100644 --- a/tests/integration/composer.json +++ b/tests/integration/composer.json @@ -6,7 +6,7 @@ "jarnaiz/behat-junit-formatter": "^1.3", "php-http/guzzle7-adapter": "^1.1", "php-http/message": "^1.16", - "libresign/nextcloud-behat": "^1.6", + "libresign/nextcloud-behat": "^1.6.4", "libresign/mailpit-behat-extension": "^0.1.1", "libresign/behat-tsa-extension": "^0.1.0" }, diff --git a/tests/integration/composer.lock b/tests/integration/composer.lock index 5183a053f0..7c1d59da41 100644 --- a/tests/integration/composer.lock +++ b/tests/integration/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "9ffc13f6e2e3c1acd213729f3967fa79", + "content-hash": "3fe6d5cb1a652073484225331a048537", "packages": [ { "name": "behat/behat", @@ -478,57 +478,6 @@ ], "time": "2024-05-06T16:37:16+00:00" }, - { - "name": "estahn/json-query-wrapper", - "version": "v1.0.0", - "source": { - "type": "git", - "url": "https://github.com/estahn/json-query-wrapper.git", - "reference": "79516b22ba364db552fc6dca719fcedc57ca5862" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/estahn/json-query-wrapper/zipball/79516b22ba364db552fc6dca719fcedc57ca5862", - "reference": "79516b22ba364db552fc6dca719fcedc57ca5862", - "shasum": "" - }, - "require": { - "php": "^7.1|^8", - "symfony/process": "^4|^5" - }, - "require-dev": { - "phpunit/phpunit": "^7|^8|^9", - "vimeo/psalm": "4.x-dev" - }, - "type": "library", - "autoload": { - "psr-4": { - "JsonQueryWrapper\\": "src/JsonQueryWrapper" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Enrico Stahn", - "email": "enrico.stahn@gmail.com" - } - ], - "description": "Wrapper for jq, a lightweight and flexible command-line JSON processor", - "homepage": "https://github.com/estahn/json-query-wrapper", - "keywords": [ - "jq", - "json", - "query" - ], - "support": { - "issues": "https://github.com/estahn/json-query-wrapper/issues", - "source": "https://github.com/estahn/json-query-wrapper/tree/v1.0.0" - }, - "time": "2021-01-11T23:19:55+00:00" - }, { "name": "guzzlehttp/guzzle", "version": "7.15.3", @@ -1128,25 +1077,25 @@ }, { "name": "libresign/nextcloud-behat", - "version": "v1.6.2", + "version": "v1.6.4", "source": { "type": "git", "url": "https://github.com/LibreSign/nextcloud-behat.git", - "reference": "76feab4b9192f10ab2eb14ef63ac64a7912de864" + "reference": "3de5745c38a0a9c0458367cd92cb97310bdaa110" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/LibreSign/nextcloud-behat/zipball/76feab4b9192f10ab2eb14ef63ac64a7912de864", - "reference": "76feab4b9192f10ab2eb14ef63ac64a7912de864", + "url": "https://api.github.com/repos/LibreSign/nextcloud-behat/zipball/3de5745c38a0a9c0458367cd92cb97310bdaa110", + "reference": "3de5745c38a0a9c0458367cd92cb97310bdaa110", "shasum": "" }, "require": { - "behat/behat": "^3.29", - "estahn/json-query-wrapper": "*", - "guzzlehttp/guzzle": "^7.10", + "behat/behat": "^3.29 || ^4.0", + "guzzlehttp/guzzle": "^7.10 || ^8.0", "libresign/behat-builtin-extension": "^0.7.1", "php": ">=8.2", - "phpunit/phpunit": "*" + "phpunit/phpunit": "*", + "symfony/process": "^6.4 || ^7.0" }, "require-dev": { "bamarni/composer-bin-plugin": "^1.9", @@ -1182,9 +1131,9 @@ ], "support": { "issues": "https://github.com/LibreSign/nextcloud-behat/issues", - "source": "https://github.com/LibreSign/nextcloud-behat/tree/v1.6.2" + "source": "https://github.com/LibreSign/nextcloud-behat/tree/v1.6.4" }, - "time": "2026-09-24T14:06:44+00:00" + "time": "2026-10-19T16:45:47+00:00" }, { "name": "myclabs/deep-copy", @@ -4934,21 +4883,20 @@ }, { "name": "symfony/process", - "version": "v5.4.51", + "version": "v7.4.22", "source": { "type": "git", "url": "https://github.com/symfony/process.git", - "reference": "467bfc56f18f5ef6d5ccb09324d7e988c1c0a98f" + "reference": "a5a6912edbbfc4c98cd080bc4bbe37c8e7eeb20a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/process/zipball/467bfc56f18f5ef6d5ccb09324d7e988c1c0a98f", - "reference": "467bfc56f18f5ef6d5ccb09324d7e988c1c0a98f", + "url": "https://api.github.com/repos/symfony/process/zipball/a5a6912edbbfc4c98cd080bc4bbe37c8e7eeb20a", + "reference": "a5a6912edbbfc4c98cd080bc4bbe37c8e7eeb20a", "shasum": "" }, "require": { - "php": ">=7.2.5", - "symfony/polyfill-php80": "^1.16" + "php": ">=8.2" }, "type": "library", "autoload": { @@ -4976,7 +4924,7 @@ "description": "Executes commands in sub-processes", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/process/tree/v5.4.51" + "source": "https://github.com/symfony/process/tree/v7.4.22" }, "funding": [ { @@ -4996,7 +4944,7 @@ "type": "tidelift" } ], - "time": "2026-01-26T15:53:37+00:00" + "time": "2026-10-25T14:40:16+00:00" }, { "name": "symfony/service-contracts", diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index e58f005186..066728d5b5 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -121,6 +121,25 @@ public function resetNotifications($type, $user): void { self::runCommand('libresign:developer:reset --' . $type . '=' . $user); } + #[Given('activity preference :key for user :user is set to :value')] + public static function setActivityPreference(string $key, string $user, string $value): void { + $php = <<<'PHP' + require $argv[1] . '/lib/base.php'; + + \OCP\Server::get(\OCP\Config\IUserConfig::class) + ->setValueString($argv[2], 'activity', $argv[3], $argv[4]); + PHP; + + $command = sprintf( + 'php -r %s -- %s %s %s', + escapeshellarg($php), + escapeshellarg($user), + escapeshellarg($key), + escapeshellarg($value), + ); + self::runBashCommandWithResultCode($command, 0); + } + #[Given('user :user has the following notifications')] public function userNotifications(string $user, ?TableNode $body = null): void { $this->setCurrentUser($user); diff --git a/tests/integration/features/policies/groups_request_sign_policy.feature b/tests/integration/features/policies/groups_request_sign_policy.feature index 013107e330..7b7241841b 100644 --- a/tests/integration/features/policies/groups_request_sign_policy.feature +++ b/tests/integration/features/policies/groups_request_sign_policy.feature @@ -141,7 +141,9 @@ Feature: policies/groups_request_sign_policy And the response should be a JSON array with the following mandatory values | key | value | | (jq).ocs.data.policies.groups_request_sign.sourceScope | global | - | (jq).ocs.data.policies.groups_request_sign.effectiveValue | {\"allowGroups\":[\"policy-request-access-board-gadmin\"],\"denyGroups\":[]} | + | (jq)((.ocs.data.policies.groups_request_sign.effectiveValue \| fromjson).allowGroups \| length) | 1 | + | (jq)(.ocs.data.policies.groups_request_sign.effectiveValue \| fromjson).allowGroups[0] | policy-request-access-board-gadmin | + | (jq)((.ocs.data.policies.groups_request_sign.effectiveValue \| fromjson).denyGroups \| length) | 0 | When sending "get" to ocs "/apps/libresign/api/v1/policies/by-policy/group/groups_request_sign" Then the response should have a status code 200 @@ -178,7 +180,10 @@ Feature: policies/groups_request_sign_policy | key | value | | (jq).ocs.data.policies | (jq)length == 1 | | (jq).ocs.data.policies[0].targetId | policy-request-access-board-gadmin | - | (jq).ocs.data.policies[0].value | {\"allowGroups\":[\"policy-request-access-board-gadmin\"],\"denyGroups\":[\"policy-request-access-board-gadmin\"]} | + | (jq)((.ocs.data.policies[0].value \| fromjson).allowGroups \| length) | 1 | + | (jq)(.ocs.data.policies[0].value \| fromjson).allowGroups[0] | policy-request-access-board-gadmin | + | (jq)((.ocs.data.policies[0].value \| fromjson).denyGroups \| length) | 1 | + | (jq)(.ocs.data.policies[0].value \| fromjson).denyGroups[0] | policy-request-access-board-gadmin | # VALIDATION: After the deny override, the regular member loses the ability to request signatures Given as user "member-request-access-policy" @@ -404,7 +409,10 @@ Feature: policies/groups_request_sign_policy | (jq).ocs.data.policy.policyKey | groups_request_sign | | (jq).ocs.data.policy.targetId | policy-request-access-board-overlay | | (jq).ocs.data.policy.allowChildOverride | false | - | (jq).ocs.data.policy.value | {\"allowGroups\":[\"policy-request-access-board-overlay\"],\"denyGroups\":[\"policy-request-access-board-overlay\"]} | + | (jq)((.ocs.data.policy.value \| fromjson).allowGroups \| length) | 1 | + | (jq)(.ocs.data.policy.value \| fromjson).allowGroups[0] | policy-request-access-board-overlay | + | (jq)((.ocs.data.policy.value \| fromjson).denyGroups \| length) | 1 | + | (jq)(.ocs.data.policy.value \| fromjson).denyGroups[0] | policy-request-access-board-overlay | # VALIDATION: The delegated admin must keep the policy card editable and still be able to read the deny row they just created, # even after locking lower-level customization and while another managed allow rule remains editable. @@ -421,7 +429,10 @@ Feature: policies/groups_request_sign_policy | key | value | | (jq).ocs.data.policy.targetId | policy-request-access-board-overlay | | (jq).ocs.data.policy.allowChildOverride | false | - | (jq).ocs.data.policy.value | {\"allowGroups\":[\"policy-request-access-board-overlay\"],\"denyGroups\":[\"policy-request-access-board-overlay\"]} | + | (jq)((.ocs.data.policy.value \| fromjson).allowGroups \| length) | 1 | + | (jq)(.ocs.data.policy.value \| fromjson).allowGroups[0] | policy-request-access-board-overlay | + | (jq)((.ocs.data.policy.value \| fromjson).denyGroups \| length) | 1 | + | (jq)(.ocs.data.policy.value \| fromjson).denyGroups[0] | policy-request-access-board-overlay | # RULE: Removing the board row deletes only the delegated deny override. When sending "delete" to ocs "/apps/libresign/api/v1/policies/group/policy-request-access-board-overlay/groups_request_sign" @@ -436,13 +447,17 @@ Feature: policies/groups_request_sign_policy Then the response should have a status code 200 And the response should be a JSON array with the following mandatory values | key | value | - | (jq).ocs.data.policy.value | {\"allowGroups\":[\"policy-request-access-board-overlay\"],\"denyGroups\":[]} | + | (jq)((.ocs.data.policy.value \| fromjson).allowGroups \| length) | 1 | + | (jq)(.ocs.data.policy.value \| fromjson).allowGroups[0] | policy-request-access-board-overlay | + | (jq)((.ocs.data.policy.value \| fromjson).denyGroups \| length) | 0 | When sending "get" to ocs "/apps/libresign/api/v1/policies/group/policy-request-access-company-overlay/groups_request_sign" Then the response should have a status code 200 And the response should be a JSON array with the following mandatory values | key | value | - | (jq).ocs.data.policy.value | {\"allowGroups\":[\"policy-request-access-company-overlay\"],\"denyGroups\":[]} | + | (jq)((.ocs.data.policy.value \| fromjson).allowGroups \| length) | 1 | + | (jq)(.ocs.data.policy.value \| fromjson).allowGroups[0] | policy-request-access-company-overlay | + | (jq)((.ocs.data.policy.value \| fromjson).denyGroups \| length) | 0 | # CLEANUP: reset policy state and remove temporary groups. When sending "delete" to ocs "/apps/libresign/api/v1/policies/group/policy-request-access-board-overlay/groups_request_sign" diff --git a/tests/integration/features/sign/reject.feature b/tests/integration/features/sign/reject.feature index d3bad52ab5..89fc44b0b2 100644 --- a/tests/integration/features/sign/reject.feature +++ b/tests/integration/features/sign/reject.feature @@ -700,3 +700,45 @@ Feature: sign-signature-rejection | (jq).ocs.data.data[0].nodeType | envelope | | (jq).ocs.data.data[0].status | 6 | | (jq).ocs.data.data[0].signers[] \| select(.status == 3) \| .rejection.comment | I do not agree with this package | + + Scenario: A rejection after the other signer signed completes the continuing workflow + Given as user "admin" + And user "signer1" exists + And user "signer2" exists + And run the command "libresign:install --use-local-cert --java" with result code 0 + And run the command "libresign:install --use-local-cert --jsignpdf" with result code 0 + And run the command "libresign:install --use-local-cert --pdftk" with result code 0 + And run the command "libresign:configure:openssl --cn test" with result code 0 + And sending "post" to ocs "/apps/libresign/api/v1/policies/system/identify_methods" + | value | (string){"factors":[{"name":"account","enabled":true,"requirement":"required","signatureMethods":{"clickToSign":{"enabled":true}},"signatureMethodEnabled":"clickToSign"}]} | + And the response should have a status code 200 + And sending "post" to ocs "/apps/libresign/api/v1/policies/system/signature_rejection" + | value | {"enabled":true,"comment_mode":"optional","cancel_workflow":false} | + And the response should have a status code 200 + And sending "post" to ocs "/apps/libresign/api/v1/request-signature" + | file | {"base64":""} | + | signers | [{"identifyMethods":[{"method":"account","value":"signer1"}]},{"identifyMethods":[{"method":"account","value":"signer2"}]}] | + | name | document | + | policy | {"overrides":{"signature_rejection":{"enabled":true}}} | + And the response should have a status code 200 + And sending "get" to ocs "/apps/libresign/api/v1/file/list?details=1" + And fetch field "(FILE_ID)ocs.data.data.0.id" from previous JSON response + And as user "signer1" + And sending "get" to ocs "/apps/libresign/api/v1/file/list?details=1" + And fetch field "(SIGNER1_UUID)ocs.data.data.0.signers.0.sign_request_uuid" from previous JSON response + When sending "post" to ocs "/apps/libresign/api/v1/sign/uuid/" + | method | clickToSign | + Then the response should have a status code 200 + When as user "signer2" + And sending "post" to ocs "/apps/libresign/api/v1/sign/file_id//reject" + Then the response should have a status code 200 + And the response should be a JSON array with the following mandatory values + | key | value | + | (jq).ocs.data.workflowCanceled | false | + When as user "admin" + And sending "get" to ocs "/apps/libresign/api/v1/file/list?details=1" + Then the response should be a JSON array with the following mandatory values + | key | value | + | (jq).ocs.data.data[0].status | 3 | + | (jq).ocs.data.data[0].signers[0].status | 2 | + | (jq).ocs.data.data[0].signers[1].status | 3 | diff --git a/tests/php/Unit/Service/File/SigningWorkflowStatusResolverTest.php b/tests/php/Unit/Service/File/SigningWorkflowStatusResolverTest.php new file mode 100644 index 0000000000..8a5aa9851e --- /dev/null +++ b/tests/php/Unit/Service/File/SigningWorkflowStatusResolverTest.php @@ -0,0 +1,100 @@ +createMock(IdDocsMapper::class); + $idDocsMapper->method('getByFileId')->willReturnCallback(function (): IdDocs { + if ($this->idDocs === null) { + throw new DoesNotExistException('no identification document'); + } + return $this->idDocs; + }); + $policyService = $this->createMock(SignatureRejectionPolicyService::class); + $policyService->method('getPolicyValue')->willReturn($policy); + return new SigningWorkflowStatusResolver($idDocsMapper, $policyService); + } + + private static function file(): FileEntity { + $file = new FileEntity(); + $file->setId(42); + return $file; + } + + /** @return list */ + private static function signers(int $signed, int $rejected, int $pending): array { + $signers = []; + for ($index = 1; $index <= $signed + $rejected + $pending; $index++) { + $signer = new SignRequest(); + $signer->setId($index); + if ($index <= $signed) { + $signer->setSigned(new DateTime()); + $signer->setStatusEnum(SignRequestStatus::SIGNED); + } elseif ($index <= $signed + $rejected) { + $signer->setStatusEnum(SignRequestStatus::REJECTED); + } else { + $signer->setStatusEnum(SignRequestStatus::ABLE_TO_SIGN); + } + $signers[] = $signer; + } + return $signers; + } + + /** @param array{enabled: bool, cancel_workflow: bool} $policy */ + #[DataProvider('completionCases')] + public function testCompletion(array $policy, int $signed, int $rejected, int $pending, ?FileStatus $expected): void { + $this->assertSame($expected, $this->resolver($policy)->resolve(self::file(), self::signers($signed, $rejected, $pending))); + } + + public static function completionCases(): array { + $continue = ['enabled' => true, 'cancel_workflow' => false]; + $cancel = ['enabled' => true, 'cancel_workflow' => true]; + $disabled = ['enabled' => false, 'cancel_workflow' => false]; + return [ + 'no signers' => [$continue, 0, 0, 0, null], + 'all signed' => [$cancel, 2, 0, 0, FileStatus::SIGNED], + 'continue: signed then last rejected' => [$continue, 1, 1, 0, FileStatus::SIGNED], + 'continue: rejected then signed' => [$continue, 1, 1, 0, FileStatus::SIGNED], + 'continue: still pending' => [$continue, 1, 1, 1, FileStatus::PARTIAL_SIGNED], + 'continue: everyone rejected without signature' => [$continue, 0, 2, 0, null], + 'cancel policy: signed and rejected' => [$cancel, 1, 1, 0, FileStatus::PARTIAL_SIGNED], + 'disabled policy: signed and rejected' => [$disabled, 1, 1, 0, FileStatus::PARTIAL_SIGNED], + 'unsigned and pending' => [$continue, 0, 1, 1, null], + ]; + } + + public function testUnsignedIdentificationUploaderPlaceholderIsIgnored(): void { + $doc = new IdDocs(); + $doc->setSignRequestId(3); + $this->idDocs = $doc; + $signers = self::signers(1, 0, 0); + $placeholder = new SignRequest(); + $placeholder->setId(3); + $placeholder->setStatusEnum(SignRequestStatus::DRAFT); + $signers[] = $placeholder; + $this->assertSame(FileStatus::SIGNED, $this->resolver(['enabled' => false, 'cancel_workflow' => false])->resolve(self::file(), $signers)); + } +} diff --git a/tests/php/Unit/Service/SignFileServiceTest.php b/tests/php/Unit/Service/SignFileServiceTest.php index 798047dddf..db5b9bdc4f 100644 --- a/tests/php/Unit/Service/SignFileServiceTest.php +++ b/tests/php/Unit/Service/SignFileServiceTest.php @@ -42,6 +42,7 @@ use OCA\Libresign\Helper\ValidateHelper; use OCA\Libresign\Service\CertificateValidityPolicy; use OCA\Libresign\Service\Envelope\EnvelopeStatusDeterminer; +use OCA\Libresign\Service\File\SigningWorkflowStatusResolver; use OCA\Libresign\Service\FileStatusService; use OCA\Libresign\Service\FolderService; use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod; @@ -55,6 +56,8 @@ use OCA\Libresign\Service\Policy\Provider\CollectMetadata\CollectMetadataPolicy; use OCA\Libresign\Service\Policy\Provider\Footer\FooterPolicy; use OCA\Libresign\Service\Policy\Provider\Footer\FooterPolicyValue; +use OCA\Libresign\Service\Policy\Provider\SignatureRejection\SignatureRejectionPolicyValue; +use OCA\Libresign\Service\SignatureRejection\SignatureRejectionPolicyService; use OCA\Libresign\Service\SignerElementsService; use OCA\Libresign\Service\SignFileService; use OCA\Libresign\Service\SigningCoordinatorService; @@ -487,6 +490,12 @@ public function testRequestCodeDoesNotPassDestinationToToken(): void { ); } + private function getStatusResolver(): SigningWorkflowStatusResolver { + $policy = $this->createMock(SignatureRejectionPolicyService::class); + $policy->method('getPolicyValue')->willReturn(SignatureRejectionPolicyValue::defaults()); + return new SigningWorkflowStatusResolver($this->idDocsMapper, $policy); + } + private function getService(array $methods = []): SignFileService|MockObject { if ($methods) { return $this->getMockBuilder(SignFileService::class) @@ -532,6 +541,7 @@ private function getService(array $methods = []): SignFileService|MockObject { $this->signRequestService, $this->policyService, new TemporarySignatureImageService($this->tempManager, $this->l10n), + $this->getStatusResolver(), ]) ->onlyMethods($methods) ->getMock(); @@ -578,6 +588,7 @@ private function getService(array $methods = []): SignFileService|MockObject { $this->signRequestService, $this->policyService, new TemporarySignatureImageService($this->tempManager, $this->l10n), + $this->getStatusResolver(), ); } @@ -1056,7 +1067,9 @@ public function testIdDocApprovalReachesSignedWhenUploaderPlaceholderIsUnsigned( $signRequest->setFileId(1); $service->setSignRequest($signRequest); - $status = self::invokePrivate($service, 'evaluateStatusFromSigners'); + $file = new \OCA\Libresign\Db\File(); + $file->setId($signRequest->getFileId()); + $status = self::invokePrivate($service, 'evaluateStatusFromSigners', [$file]); $this->assertSame(FileStatus::SIGNED->value, $status); } @@ -1080,7 +1093,9 @@ public function testDraftSignersStillCountWhenFileIsNotAnIdentificationDocument( $signRequest->setFileId(99); $service->setSignRequest($signRequest); - $status = self::invokePrivate($service, 'evaluateStatusFromSigners'); + $file = new \OCA\Libresign\Db\File(); + $file->setId($signRequest->getFileId()); + $status = self::invokePrivate($service, 'evaluateStatusFromSigners', [$file]); $this->assertSame(FileStatus::PARTIAL_SIGNED->value, $status); } diff --git a/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php b/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php index 978d08dfdb..2614d4c703 100644 --- a/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php +++ b/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php @@ -10,12 +10,14 @@ use OCA\Libresign\Db\File; use OCA\Libresign\Db\FileMapper; +use OCA\Libresign\Db\IdDocsMapper; use OCA\Libresign\Db\SignRequest; use OCA\Libresign\Db\SignRequestMapper; use OCA\Libresign\Enum\FileStatus; use OCA\Libresign\Enum\SignRequestStatus; use OCA\Libresign\Events\SignatureRejectedEvent; use OCA\Libresign\Exception\LibresignException; +use OCA\Libresign\Service\File\SigningWorkflowStatusResolver; use OCA\Libresign\Service\FileStatusService; use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod; use OCA\Libresign\Service\IdentifyMethodService; @@ -93,6 +95,7 @@ private function getService(): SignatureRejectionService { $this->timeFactory, $this->l10n, $this->logger, + new SigningWorkflowStatusResolver($this->createMock(IdDocsMapper::class), $this->rejectionPolicyService), ); } @@ -285,6 +288,93 @@ public function testWorkflowKeepsRunningWhenThePolicyDoesNotCancelIt(): void { $this->assertSame(FileStatus::ABLE_TO_SIGN->value, $file->getStatus()); } + public function testLastRejectionAfterSignatureCompletesDocument(): void { + $this->withPolicy(['enabled' => true, 'cancel_workflow' => false]); + $file = $this->file(FileStatus::PARTIAL_SIGNED->value); + $rejected = $this->signRequest(); + $signed = $this->signRequest(SignRequestStatus::SIGNED->value, 2); + $signed->setSigned(new \DateTime('2026-09-05T10:00:00+00:00')); + $this->signRequestMapper->method('getByFileId')->with(10)->willReturn([$signed, $rejected]); + $this->fileStatusService->expects($this->once())->method('update')->with($file); + + $this->getService()->reject($file, $rejected); + + $this->assertSame(SignRequestStatus::REJECTED, $rejected->getStatusEnum()); + $this->assertSame(FileStatus::SIGNED->value, $file->getStatus()); + } + + public function testFailedStatusUpdateRollsBackRejectionAndRestoresFile(): void { + $this->withPolicy(['enabled' => true, 'cancel_workflow' => false]); + $file = $this->file(FileStatus::PARTIAL_SIGNED->value); + $rejected = $this->signRequest(); + $signed = $this->signRequest(SignRequestStatus::SIGNED->value, 2); + $signed->setSigned(new \DateTime('2026-09-05T10:00:00+00:00')); + $this->signRequestMapper->method('getByFileId')->with(10)->willReturn([$signed, $rejected]); + $this->fileStatusService->method('update')->willThrowException(new \RuntimeException('status update failed')); + $this->fileStatusService->expects($this->once())->method('invalidateCachedStatus')->with($file); + $this->db->expects($this->once())->method('rollBack'); + + try { + $this->getService()->reject($file, $rejected); + $this->fail('The failed status update must abort the rejection.'); + } catch (LibresignException $e) { + $this->assertSame('It was not possible to register the rejection. Nothing was changed.', $e->getMessage()); + } + $this->assertSame(FileStatus::PARTIAL_SIGNED->value, $file->getStatus()); + $this->assertSame(SignRequestStatus::ABLE_TO_SIGN, $rejected->getStatusEnum()); + } + + public function testAnEnvelopeRollbackRestoresBothDocumentsAndParentCache(): void { + $this->withPolicy(['enabled' => true, 'cancel_workflow' => false]); + $envelope = $this->envelope(FileStatus::PARTIAL_SIGNED->value); + $first = $this->file(FileStatus::PARTIAL_SIGNED->value, parentFileId: 1); + $second = $this->file(FileStatus::PARTIAL_SIGNED->value, parentFileId: 1); + $second->setId(20); + $pendingFirst = $this->signRequest(id: 11); + $pendingSecond = $this->signRequest(id: 12); + $pendingSecond->setFileId(20); + $envelopeSigner = $this->signRequest(id: 13); + $envelopeSigner->setFileId(1); + $signedFirst = $this->signRequest(SignRequestStatus::SIGNED->value, 21); + $signedFirst->setSigned(new \DateTime('2026-09-05T10:00:00+00:00')); + $signedSecond = $this->signRequest(SignRequestStatus::SIGNED->value, 22); + $signedSecond->setFileId(20); + $signedSecond->setSigned(new \DateTime('2026-09-05T10:00:00+00:00')); + + $this->envelopeChildSignRequests = [$pendingSecond]; + $this->envelopeSignRequest = $envelopeSigner; + $this->fileMapper->method('getById')->willReturnCallback( + static fn (int $id): File => $id === 20 ? $second : $envelope, + ); + $this->signRequestMapper->method('getByFileId')->willReturnCallback( + static fn (int $fileId): array => $fileId === 20 + ? [$signedSecond, $pendingSecond] + : [$signedFirst, $pendingFirst], + ); + $this->fileStatusService->expects($this->exactly(2))->method('update') + ->willReturnCallback(static function (File $file): File { + if ($file->getId() === 20) { + throw new \RuntimeException('second document update failed'); + } + return $file; + }); + $this->fileStatusService->expects($this->exactly(3))->method('invalidateCachedStatus'); + $this->db->expects($this->once())->method('rollBack'); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + + $this->expectException(LibresignException::class); + try { + $this->getService()->reject($first, $pendingFirst); + } finally { + $this->assertSame(FileStatus::PARTIAL_SIGNED->value, $first->getStatus()); + $this->assertSame(FileStatus::PARTIAL_SIGNED->value, $second->getStatus()); + $this->assertSame(FileStatus::PARTIAL_SIGNED->value, $envelope->getStatus()); + $this->assertSame(SignRequestStatus::ABLE_TO_SIGN, $pendingFirst->getStatusEnum()); + $this->assertSame(SignRequestStatus::ABLE_TO_SIGN, $pendingSecond->getStatusEnum()); + $this->assertSame(SignRequestStatus::ABLE_TO_SIGN, $envelopeSigner->getStatusEnum()); + } + } + public function testWorkflowIsClosedWhenThePolicyCancelsIt(): void { $this->withPolicy(['enabled' => true, 'cancel_workflow' => true]); $file = $this->file(FileStatus::PARTIAL_SIGNED->value);