diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 4338e58552..c19246d26b 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -679,6 +679,7 @@ * nodeId: non-negative-int, * nodeType: 'file'|'envelope', * signatureFlow: 'none'|'parallel'|'ordered_numeric', + * policySnapshotFrozen?: bool, * docmdpLevel: int, * filesCount: int<0, max>, * files: list, @@ -755,6 +756,7 @@ * metadata: LibresignFileRuntimeMetadata, * docmdpLevel: int, * signatureFlow: 'none'|'parallel'|'ordered_numeric', + * policySnapshotFrozen: bool, * signersCount: int, * signers: list, * requested_by: LibresignRequestedBy, @@ -791,6 +793,7 @@ * size: non-negative-int, * docmdpLevel: int, * signatureFlow: 'none'|'parallel'|'ordered_numeric', + * policySnapshotFrozen: bool, * visibleElements: LibresignVisibleElement[], * signers: LibresignSignerDetail[], * signersCount: int, diff --git a/lib/Service/File/FileListService.php b/lib/Service/File/FileListService.php index e531854acd..b0e3e9827c 100644 --- a/lib/Service/File/FileListService.php +++ b/lib/Service/File/FileListService.php @@ -25,6 +25,7 @@ use OCA\Libresign\Enum\SignRequestStatus; use OCA\Libresign\ResponseDefinitions; use OCA\Libresign\Service\FileElementService; +use OCA\Libresign\Service\FileStatusService; use OCA\Libresign\Service\FolderService; use OCA\Libresign\Service\IdentifyMethodService; use OCA\Libresign\Service\SignatureRejection\RejectionViewer; @@ -94,11 +95,12 @@ public function listAssociatedFilesOfSignFlow( $signers = $this->signRequestMapper->getByMultipleFileId(array_map(fn (File $file) => $file->getId(), $return['data'])); $identifyMethods = $this->signRequestMapper->getIdentifyMethodsFromSigners($signers); + $envelopes = $this->getEnvelopesByFileId($return['data']); if ($details) { $visibleElements = $this->signRequestMapper->getVisibleElementsFromSigners($signers); - $return['data'] = $this->associateAllAndFormat($user, $return['data'], $signers, $identifyMethods, $visibleElements); + $return['data'] = $this->associateAllAndFormat($user, $return['data'], $signers, $identifyMethods, $visibleElements, $envelopes); } else { - $return['data'] = $this->associateAllAndFormatSummary($user, $return['data'], $signers, $identifyMethods); + $return['data'] = $this->associateAllAndFormatSummary($user, $return['data'], $signers, $identifyMethods, $envelopes); } $return['pagination']->setRouteName('ocs.libresign.File.list'); @@ -112,6 +114,7 @@ public function listAssociatedFilesOfSignFlow( * @param File[] $files * @param SignRequest[] $signers * @param array> $identifyMethods + * @param array $envelopes * @return list */ private function associateAllAndFormatSummary( @@ -119,11 +122,12 @@ private function associateAllAndFormatSummary( array $files, array $signers, array $identifyMethods, + array $envelopes, ): array { $formattedFiles = []; foreach ($files as $file) { $fileSigners = array_filter($signers, fn ($signer) => $signer->getFileId() === $file->getId()); - $formattedFiles[] = $this->formatSingleFileSummary($file, $fileSigners, $identifyMethods, $user); + $formattedFiles[] = $this->formatSingleFileSummary($file, $fileSigners, $identifyMethods, $user, $envelopes[$file->getId()] ?? null); } return $formattedFiles; } @@ -132,8 +136,9 @@ public function formatSingleFile(IUser $user, File $file): array { $signers = $this->signRequestMapper->getByMultipleFileId([$file->getId()]); $identifyMethods = $this->signRequestMapper->getIdentifyMethodsFromSigners($signers); $visibleElements = $this->signRequestMapper->getVisibleElementsFromSigners($signers); + $envelope = $this->getEnvelopesByFileId([$file])[$file->getId()] ?? null; - return $this->formatSingleFileData($file, $signers, $identifyMethods, $visibleElements, $user); + return $this->formatSingleFileData($file, $signers, $identifyMethods, $visibleElements, $user, null, $envelope); } public function formatSingleFileForSignRequest(File $file, ?SignRequest $currentSignRequest = null): array { @@ -212,6 +217,7 @@ public function formatEnvelopeChildFilesForSignRequest( * @param SignRequest[] $signers * @param array> $identifyMethods * @param array $visibleElements + * @param array $envelopes * @return list */ private function associateAllAndFormat( @@ -220,11 +226,12 @@ private function associateAllAndFormat( array $signers, array $identifyMethods, array $visibleElements, + array $envelopes, ): array { $formattedFiles = []; foreach ($files as $file) { $fileSigners = array_filter($signers, fn ($signer) => $signer->getFileId() === $file->getId()); - $formattedFiles[] = $this->formatSingleFileData($file, $fileSigners, $identifyMethods, $visibleElements, $user); + $formattedFiles[] = $this->formatSingleFileData($file, $fileSigners, $identifyMethods, $visibleElements, $user, null, $envelopes[$file->getId()] ?? null); } return $formattedFiles; } @@ -247,6 +254,7 @@ private function formatSingleFileData( array $visibleElements, ?IUser $user, ?int $meSignRequestId = null, + ?File $envelope = null, ): array { $file = [ 'id' => $fileEntity->getId(), @@ -261,6 +269,10 @@ private function formatSingleFileData( 'signatureFlow' => $fileEntity->getSignatureFlow(), 'nodeType' => $fileEntity->getNodeType(), ]; + // The sign page formats files without a user and may be public. + if ($user !== null) { + $file['policySnapshotFrozen'] = FileStatusService::isPolicySnapshotFrozen($fileEntity, $envelope); + } $file['signatureFlow'] = SignatureFlow::fromNumeric($file['signatureFlow'])->value; $file['statusText'] = $this->fileMapper->getTextOfStatus($file['status']); $file['requested_by'] = [ @@ -339,6 +351,7 @@ private function formatSingleFileSummary( array $signers, array $identifyMethods, IUser $user, + ?File $envelope = null, ): array { $metadata = $fileEntity->getMetadata() ?? []; $nodeType = $fileEntity->getNodeType(); @@ -384,6 +397,7 @@ private function formatSingleFileSummary( 'metadata' => $metadata, 'docmdpLevel' => $fileEntity->getDocmdpLevel(), 'signatureFlow' => SignatureFlow::fromNumeric($fileEntity->getSignatureFlow())->value, + 'policySnapshotFrozen' => FileStatusService::isPolicySnapshotFrozen($fileEntity, $envelope), 'signersCount' => count($signers), 'signers' => [], 'requested_by' => [ @@ -506,6 +520,40 @@ private function formatSignerData( return $data; } + /** + * The envelope of each file that has one, by file id. Envelopes already + * among the files are reused and every other one is loaded once. + * + * @param File[] $files + * @return array + */ + private function getEnvelopesByFileId(array $files): array { + $loaded = []; + foreach ($files as $file) { + $loaded[$file->getId()] = $file; + } + + $envelopes = []; + foreach ($files as $file) { + $parentId = $file->getParentFileId(); + if ($parentId === null) { + continue; + } + if (!array_key_exists($parentId, $loaded)) { + try { + $loaded[$parentId] = $this->fileMapper->getById($parentId); + } catch (DoesNotExistException) { + $loaded[$parentId] = null; + } + } + if ($loaded[$parentId] !== null) { + $envelopes[$file->getId()] = $loaded[$parentId]; + } + } + + return $envelopes; + } + private function isRequester(?File $fileEntity, ?IUser $user): bool { $requesterUserId = $fileEntity?->getUserId() ?? ''; return $requesterUserId !== '' && $user?->getUID() === $requesterUserId; @@ -830,6 +878,10 @@ public function formatFileWithChildren(File $mainEntity, array $childFiles, ?IUs 'metadata' => $metadata, 'docmdpLevel' => $mainEntity->getDocmdpLevel(), 'signatureFlow' => SignatureFlow::fromNumeric($mainEntity->getSignatureFlow())->value, + 'policySnapshotFrozen' => FileStatusService::isPolicySnapshotFrozen( + $mainEntity, + $this->getEnvelopesByFileId([$mainEntity])[$mainEntity->getId()] ?? null, + ), 'signers' => $signers, 'signersCount' => count($signers), 'requested_by' => [ diff --git a/lib/Service/FileService.php b/lib/Service/FileService.php index fd10696b78..bfa3959a82 100644 --- a/lib/Service/FileService.php +++ b/lib/Service/FileService.php @@ -455,6 +455,13 @@ private function loadLibreSignData(): void { $this->fileData->signatureFlow = SignatureFlow::fromNumeric($this->file->getSignatureFlow())->value; $this->fileData->docmdpLevel = $this->file->getDocmdpLevel(); $this->fileData->nodeType = $this->file->getNodeType(); + // Validation is public: only the requester learns about the freeze. + if ($this->options->getMe() !== null && $this->options->getMe()->getUID() === $this->file->getUserId()) { + $this->fileData->policySnapshotFrozen = FileStatusService::isPolicySnapshotFrozen( + $this->file, + $this->file->hasParent() ? $this->envelopeService->getEnvelopeByFileId($this->file->getId()) : null, + ); + } if ($this->fileData->nodeType !== 'envelope' && !$this->file->getParentFileId()) { $fileId = $this->file->getId(); diff --git a/lib/Service/FileStatusService.php b/lib/Service/FileStatusService.php index 112761662d..39471d1fcf 100644 --- a/lib/Service/FileStatusService.php +++ b/lib/Service/FileStatusService.php @@ -151,6 +151,24 @@ public function markPolicySnapshotFrozen(FileEntity $file, int $newStatus): void $file->setMetadata($metadata); } + /** + * A document freezes with its envelope, so pass the envelope of a document + * that has one. + */ + public static function isPolicySnapshotFrozen(FileEntity $file, ?FileEntity $envelope = null): bool { + return self::hasFrozenPolicySnapshot($file) + || ($envelope !== null && self::hasFrozenPolicySnapshot($envelope)); + } + + /** + * A request sent before the freeze was recorded has no marker, so it is + * frozen while its status says it was sent. + */ + private static function hasFrozenPolicySnapshot(FileEntity $file): bool { + return isset(($file->getMetadata() ?? [])[self::POLICY_SNAPSHOT_FROZEN_AT]) + || $file->getStatus() >= FileStatus::ABLE_TO_SIGN->value; + } + private function touchStatusChangedAt(FileEntity $file): void { $metadata = $file->getMetadata() ?? []; $metadata['status_changed_at'] = (new DateTime())->format(DateTimeInterface::ATOM); diff --git a/lib/Service/Policy/AbstractFilePolicyApplier.php b/lib/Service/Policy/AbstractFilePolicyApplier.php index 1840702a4a..9754a44caa 100644 --- a/lib/Service/Policy/AbstractFilePolicyApplier.php +++ b/lib/Service/Policy/AbstractFilePolicyApplier.php @@ -10,7 +10,6 @@ use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Db\FileMapper; -use OCA\Libresign\Enum\FileStatus; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Service\FileService; use OCA\Libresign\Service\FileStatusService; @@ -97,17 +96,7 @@ protected function isPolicySnapshotFrozen(FileEntity $file, string $policyKey, s } } - $envelope = $this->findEnvelope($file); - if ($this->hasFrozenMarker($file) || $this->hasFrozenMarker($envelope)) { - return true; - } - - return $file->getStatus() >= FileStatus::ABLE_TO_SIGN->value - || ($envelope !== null && $envelope->getStatus() >= FileStatus::ABLE_TO_SIGN->value); - } - - private function hasFrozenMarker(?FileEntity $file): bool { - return isset(($file?->getMetadata() ?? [])[FileStatusService::POLICY_SNAPSHOT_FROZEN_AT]); + return FileStatusService::isPolicySnapshotFrozen($file, $this->findEnvelope($file)); } private function findEnvelope(FileEntity $file): ?FileEntity { diff --git a/lib/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplier.php b/lib/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplier.php index 4841675292..924cc434c9 100644 --- a/lib/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplier.php +++ b/lib/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplier.php @@ -10,9 +10,11 @@ use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Enum\SignatureFlow; +use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Service\Policy\AbstractFilePolicyApplier; use OCA\Libresign\Service\Policy\Model\ResolvedPolicy; use OCA\Libresign\Service\Policy\Provider\Signature\SignatureFlowPolicy; +use OCP\AppFramework\Http; use OCP\IUser; class SignatureFlowFilePolicyApplier extends AbstractFilePolicyApplier { @@ -33,6 +35,11 @@ public function apply(FileEntity $file, array $data): void { #[\Override] public function sync(FileEntity $file, array $data): void { $requestOverrides = $this->getOverrides($data); + if ($this->isPolicySnapshotFrozen($file, SignatureFlowPolicy::KEY)) { + $this->assertFrozenFlowIsKept($file, $requestOverrides); + return; + } + $activeContext = $this->extractActiveContext($data); $resolvedPolicy = $activeContext === null ? $this->policyService->resolveForUserId(SignatureFlowPolicy::KEY, $file->getUserId(), $requestOverrides) @@ -63,4 +70,24 @@ private function getOverrides(array $data): array { private function assertOverrideAllowed(array $requestOverrides, ResolvedPolicy $resolvedPolicy): void { $this->assertRequestOverrideAllowed($requestOverrides, $resolvedPolicy, 'Signature flow override is blocked by %s.'); } + + /** + * A frozen request keeps the flow it was sent with. A client may still send + * that same flow along an unrelated update, which changes nothing. + * + * @param array $requestOverrides + */ + private function assertFrozenFlowIsKept(FileEntity $file, array $requestOverrides): void { + $submittedFlow = $requestOverrides[SignatureFlowPolicy::KEY] ?? null; + if ($submittedFlow === null || $submittedFlow === $file->getSignatureFlowEnum()->value) { + return; + } + + throw new LibresignException( + // TRANSLATORS Error shown when someone tries to change whether signers sign in order after the signing flow already started. + $this->l10n?->t('The signing order cannot be changed after the signing flow has started.') + ?? 'The signing order cannot be changed after the signing flow has started.', + Http::STATUS_UNPROCESSABLE_ENTITY, + ); + } } diff --git a/lib/Service/RequestSignatureService.php b/lib/Service/RequestSignatureService.php index 65d821dc46..30b4995908 100644 --- a/lib/Service/RequestSignatureService.php +++ b/lib/Service/RequestSignatureService.php @@ -31,6 +31,8 @@ use OCA\Libresign\Service\Validation\FileInputValidator; use OCA\Libresign\Service\Validation\SignerValidator; use OCA\Libresign\Service\Validation\SigningRequestValidator; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IMimeTypeDetector; use OCP\Files\Node; @@ -371,9 +373,15 @@ public function saveFile(array $data): FileEntity { if (!is_null($nodeId)) { try { $file = $this->fileMapper->getByNodeId($nodeId); + } catch (DoesNotExistException) { + $file = null; + } catch (MultipleObjectsReturnedException) { + // TRANSLATORS Error shown when a document cannot be updated because LibreSign has more than one record for it. + throw new LibresignException($this->l10n->t('This document is registered more than once, so it cannot be updated. Please contact your administrator.'), 422); + } + if ($file !== null) { $this->filePolicyApplier->syncAllPolicies($file, $data); return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0); - } catch (\Throwable) { } } diff --git a/openapi-full.json b/openapi-full.json index 310cb1cb7d..1838a98372 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -808,6 +808,7 @@ "size", "docmdpLevel", "signatureFlow", + "policySnapshotFrozen", "visibleElements", "signers", "signersCount", @@ -876,6 +877,9 @@ "ordered_numeric" ] }, + "policySnapshotFrozen": { + "type": "boolean" + }, "visibleElements": { "type": "array", "items": { @@ -1559,6 +1563,7 @@ "metadata", "docmdpLevel", "signatureFlow", + "policySnapshotFrozen", "signersCount", "signers", "requested_by", @@ -1613,6 +1618,9 @@ "ordered_numeric" ] }, + "policySnapshotFrozen": { + "type": "boolean" + }, "signersCount": { "type": "integer", "format": "int64" @@ -4020,6 +4028,9 @@ "ordered_numeric" ] }, + "policySnapshotFrozen": { + "type": "boolean" + }, "docmdpLevel": { "type": "integer", "format": "int64" diff --git a/openapi.json b/openapi.json index 0fe723b82b..e5e4a023a0 100644 --- a/openapi.json +++ b/openapi.json @@ -509,6 +509,7 @@ "size", "docmdpLevel", "signatureFlow", + "policySnapshotFrozen", "visibleElements", "signers", "signersCount", @@ -577,6 +578,9 @@ "ordered_numeric" ] }, + "policySnapshotFrozen": { + "type": "boolean" + }, "visibleElements": { "type": "array", "items": { @@ -1120,6 +1124,7 @@ "metadata", "docmdpLevel", "signatureFlow", + "policySnapshotFrozen", "signersCount", "signers", "requested_by", @@ -1174,6 +1179,9 @@ "ordered_numeric" ] }, + "policySnapshotFrozen": { + "type": "boolean" + }, "signersCount": { "type": "integer", "format": "int64" @@ -3281,6 +3289,9 @@ "ordered_numeric" ] }, + "policySnapshotFrozen": { + "type": "boolean" + }, "docmdpLevel": { "type": "integer", "format": "int64" diff --git a/src/components/RightSidebar/RequestSignatureTab.vue b/src/components/RightSidebar/RequestSignatureTab.vue index 5f186928ab..bef986f1bc 100644 --- a/src/components/RightSidebar/RequestSignatureTab.vue +++ b/src/components/RightSidebar/RequestSignatureTab.vue @@ -535,7 +535,7 @@ import { import { resolveSignatureFlowMode, resolveSignatureFlowPayloadForRequest, - type RequestSignatureFlowOverride, + type SignatureFlowMode, } from '../../views/Settings/PolicyWorkbench/settings/signature-flow/model' import type { components, operations } from '../../types/openapi/openapi' import type { @@ -632,6 +632,8 @@ const observerProfileEnabled = computed(() => observerProfilePolicy.value?.effec const canChooseSigningOrderAtRequestLevel = computed(() => policiesStore.canUseRequestOverride('signature_flow')) const canChooseFooterTemplateAtRequestLevel = computed(() => policiesStore.canUseRequestOverride('add_footer')) const isAdminFlowForced = computed(() => !canChooseSigningOrderAtRequestLevel.value) +// Once sent, a request keeps its signature flow even if it returns to draft, so the status alone does not tell. +const isSignatureFlowFrozen = computed(() => filesStore.getFile()?.policySnapshotFrozen === true) watch(() => policiesStore.getEffectiveValue('identify_methods'), (value) => { methods.value = normalizeIdentifyMethodsPolicy(value) @@ -643,6 +645,10 @@ const signatureFlow = computed(() => { const fileFlow = file?.signatureFlow const resolvedFileFlow = resolveSignatureFlowMode(fileFlow) + if (isSignatureFlowFrozen.value) { + return resolvedFileFlow ?? 'none' + } + if (!canChooseSigningOrderAtRequestLevel.value && resolvedPolicy && resolvedPolicy !== 'none') { return resolvedPolicy } @@ -694,7 +700,7 @@ const currentFile = computed(() => (filesStore.getFi const isCurrentFileDetailed = computed(() => currentFile.value?.detailsLoaded === true) const shouldLoadDetail = computed(() => totalSigners.value > 0) const showSigningOrderOptions = computed(() => !isOriginalFileDeleted.value && isCurrentFileDetailed.value && hasSigners.value && filesStore.canSave() && canChooseSigningOrderAtRequestLevel.value) -const showPreserveOrder = computed(() => !isOriginalFileDeleted.value && isCurrentFileDetailed.value && signingParticipantCount.value > 1 && filesStore.canSave() && canChooseSigningOrderAtRequestLevel.value) +const showPreserveOrder = computed(() => !isOriginalFileDeleted.value && isCurrentFileDetailed.value && signingParticipantCount.value > 1 && filesStore.canSave() && canChooseSigningOrderAtRequestLevel.value && !isSignatureFlowFrozen.value) const showRememberSignatureFlow = computed(() => showPreserveOrder.value && canSaveSignatureFlowPreference.value) const footerTemplateSourceOptions = computed(() => { return buildFooterTemplateSourceOptions(footerPolicy.value, { @@ -753,9 +759,9 @@ const signingOrderDiagramSigners = computed(() => { })) }) -function getSignatureFlowPayloadForSave(): RequestSignatureFlowOverride | null { +function getSignatureFlowPayloadForSave(): SignatureFlowMode | null { const resolvedFlow = resolveSignatureFlowMode(signatureFlow.value) - return resolveSignatureFlowPayloadForRequest(canChooseSigningOrderAtRequestLevel.value, resolvedFlow) + return resolveSignatureFlowPayloadForRequest(canChooseSigningOrderAtRequestLevel.value, resolvedFlow, isSignatureFlowFrozen.value) } function getFooterPolicyPayloadForSave(): string | null { @@ -1314,7 +1320,7 @@ function syncPreserveOrderWithFile() { function syncFileSignatureFlowWithPolicy() { const resolvedPolicy = resolveSignatureFlowMode(signatureFlowPolicy.value?.effectiveValue) - if (canChooseSigningOrderAtRequestLevel.value || !resolvedPolicy || resolvedPolicy === 'none') { + if (isSignatureFlowFrozen.value || canChooseSigningOrderAtRequestLevel.value || !resolvedPolicy || resolvedPolicy === 'none') { return } diff --git a/src/store/files.js b/src/store/files.js index 86785fb76c..a98cda09a0 100644 --- a/src/store/files.js +++ b/src/store/files.js @@ -120,6 +120,7 @@ import { isCurrentUserObserver, isSigningParticipant } from '../utils/participan * settings?: RuntimeFileSettingsRecord * requested_by?: Partial * signatureFlow?: SignatureFlowValue | null + * policySnapshotFrozen?: boolean * signers?: EditableSignerDraft[] | null * visibleElements?: VisibleElementRecord[] | null * url?: string @@ -154,6 +155,7 @@ import { isCurrentUserObserver, isSigningParticipant } from '../utils/participan * settings?: EditableFileSettingsDraft * requested_by?: Partial * signatureFlow?: SignatureFlowValue | null + * policySnapshotFrozen?: boolean * signers?: EditableSignerDraft[] | null * visibleElements?: (VisibleElementRecord | VisibleElementDraft)[] | null * signersCount?: number @@ -599,7 +601,9 @@ const _filesStore = defineStore('files', () => { clearRequestDraft(fileData.id) } - await store.addFile(fileData, { detailsLoaded: true }) + // Only the requester gets policySnapshotFrozen: when the detail omits it, + // a value merged from the list must not outlive this reload. + await store.addFile({ ...fileData, policySnapshotFrozen: fileData.policySnapshotFrozen }, { detailsLoaded: true }) return apiFiles.value[fileData.id] || files.value[fileData.id] || null } diff --git a/src/tests/components/RightSidebar/RequestSignatureTab.spec.ts b/src/tests/components/RightSidebar/RequestSignatureTab.spec.ts index 6997697443..5948ba3c60 100644 --- a/src/tests/components/RightSidebar/RequestSignatureTab.spec.ts +++ b/src/tests/components/RightSidebar/RequestSignatureTab.spec.ts @@ -1392,6 +1392,67 @@ describe('RequestSignatureTab - Critical Business Rules', () => { }) }) + describe('RULE: a frozen request keeps the signature flow it was sent with', () => { + const twoSigners = [ + { email: 'test1@example.com', signed: [] }, + { email: 'test2@example.com', signed: [] }, + ] + + it('sends the stored flow, none included, instead of the current policy', async () => { + await updatePolicies({ effectiveValue: 'ordered_numeric' }) + await updateFile({ + status: FILE_STATUS.DRAFT, + policySnapshotFrozen: true, + signatureFlow: 'none', + signers: twoSigners, + }) + + expect(wrapper.vm.signatureFlow).toBe('none') + expect(wrapper.vm.getSignatureFlowPayloadForSave()).toBe('none') + }) + + it('does not offer the signing order switch', async () => { + await updateFile({ + status: FILE_STATUS.ABLE_TO_SIGN, + policySnapshotFrozen: true, + signatureFlow: 'parallel', + signers: twoSigners, + }) + + expect(wrapper.vm.showPreserveOrder).toBe(false) + expect(wrapper.vm.showRememberSignatureFlow).toBe(false) + }) + + it('is not rewritten by a flow the policy now forces', async () => { + await updateFile({ + status: FILE_STATUS.ABLE_TO_SIGN, + policySnapshotFrozen: true, + signatureFlow: 'parallel', + signers: twoSigners, + }) + await updatePolicies({ + canUseAsRequestOverride: false, + effectiveValue: 'ordered_numeric', + }) + + expect(wrapper.vm.signatureFlow).toBe('parallel') + expect(filesStore.files[1].signatureFlow).toBe('parallel') + }) + + it('keeps the switch and the computed flow while the request is a draft', async () => { + await updatePolicies({ effectiveValue: 'ordered_numeric' }) + await updateFile({ + status: FILE_STATUS.DRAFT, + policySnapshotFrozen: false, + signatureFlow: 'none', + signers: twoSigners, + }) + + expect(wrapper.vm.showPreserveOrder).toBe(true) + expect(wrapper.vm.getSignatureFlowPayloadForSave()).toBe('ordered_numeric') + }) + }) + describe('RULE: isAdminFlowForced detection', () => { it('returns true when policy blocks request overrides', async () => { await updatePolicies({ canUseAsRequestOverride: false }) diff --git a/src/tests/store/files.spec.ts b/src/tests/store/files.spec.ts index 86da46c4fd..5560356d72 100644 --- a/src/tests/store/files.spec.ts +++ b/src/tests/store/files.spec.ts @@ -268,6 +268,32 @@ describe('files store - critical business rules', () => { }) }) + describe('RULE: the detail reload is the source of the policy snapshot freeze', () => { + it('replaces the flag loaded with the list', async () => { + const store = useFilesStore() + store.files[123] = { id: 123, status: 0, policySnapshotFrozen: false } + axiosMock.get.mockResolvedValue(generateOCSResponse({ + payload: { id: 123, status: 1, policySnapshotFrozen: true }, + })) + + const detailedFile = await store.fetchFileDetail({ fileId: 123, force: true }) + + expect(detailedFile?.policySnapshotFrozen).toBe(true) + }) + + it('does not keep the flag loaded with the list when the detail omits it', async () => { + const store = useFilesStore() + store.files[123] = { id: 123, status: 1, policySnapshotFrozen: true } + axiosMock.get.mockResolvedValue(generateOCSResponse({ + payload: { id: 123, status: 1 }, + })) + + const detailedFile = await store.fetchFileDetail({ fileId: 123, force: true }) + + expect(detailedFile?.policySnapshotFrozen).toBeUndefined() + }) + }) + describe('RULE: envelope filesCount reflects file operations', () => { it('adding files increments envelope filesCount', async () => { const store = useFilesStore() diff --git a/src/tests/views/Settings/PolicyWorkbench/settings/signature-flow/model.spec.ts b/src/tests/views/Settings/PolicyWorkbench/settings/signature-flow/model.spec.ts index 9af65f0a45..275cd4df1e 100644 --- a/src/tests/views/Settings/PolicyWorkbench/settings/signature-flow/model.spec.ts +++ b/src/tests/views/Settings/PolicyWorkbench/settings/signature-flow/model.spec.ts @@ -46,10 +46,16 @@ describe('signature-flow model', () => { expect(resolveSignatureFlowPayloadForRequest(false, 'parallel')).toBeNull() }) - it('returns request payload when request-level override is allowed', () => { + it('returns request payload when request-level override is allowed on a request not yet frozen', () => { expect(resolveSignatureFlowPayloadForRequest(true, 'ordered_numeric')).toBe('ordered_numeric') expect(resolveSignatureFlowPayloadForRequest(true, 'parallel')).toBe('parallel') expect(resolveSignatureFlowPayloadForRequest(true, 'none')).toBe('parallel') expect(resolveSignatureFlowPayloadForRequest(true, null)).toBe('parallel') }) + + it('returns the stored flow, none included, once the request is frozen', () => { + expect(resolveSignatureFlowPayloadForRequest(true, 'none', true)).toBe('none') + expect(resolveSignatureFlowPayloadForRequest(true, 'ordered_numeric', true)).toBe('ordered_numeric') + expect(resolveSignatureFlowPayloadForRequest(false, 'none', true)).toBeNull() + }) }) \ No newline at end of file diff --git a/src/types/openapi/openapi-full.ts b/src/types/openapi/openapi-full.ts index 8c2b8639b2..cffe81ca2c 100644 --- a/src/types/openapi/openapi-full.ts +++ b/src/types/openapi/openapi-full.ts @@ -1545,6 +1545,7 @@ export type components = { docmdpLevel: number; /** @enum {string} */ signatureFlow: "none" | "parallel" | "ordered_numeric"; + policySnapshotFrozen: boolean; visibleElements: components["schemas"]["VisibleElement"][]; signers: components["schemas"]["SignerDetail"][]; /** Format: int64 */ @@ -1737,6 +1738,7 @@ export type components = { docmdpLevel: number; /** @enum {string} */ signatureFlow: "none" | "parallel" | "ordered_numeric"; + policySnapshotFrozen: boolean; /** Format: int64 */ signersCount: number; signers: components["schemas"]["SignerSummary"][]; @@ -2409,6 +2411,7 @@ export type components = { nodeType: "file" | "envelope"; /** @enum {string} */ signatureFlow: "none" | "parallel" | "ordered_numeric"; + policySnapshotFrozen?: boolean; /** Format: int64 */ docmdpLevel: number; /** Format: int64 */ diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index bf45c3d352..6362399a56 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -1143,6 +1143,7 @@ export type components = { docmdpLevel: number; /** @enum {string} */ signatureFlow: "none" | "parallel" | "ordered_numeric"; + policySnapshotFrozen: boolean; visibleElements: components["schemas"]["VisibleElement"][]; signers: components["schemas"]["SignerDetail"][]; /** Format: int64 */ @@ -1300,6 +1301,7 @@ export type components = { docmdpLevel: number; /** @enum {string} */ signatureFlow: "none" | "parallel" | "ordered_numeric"; + policySnapshotFrozen: boolean; /** Format: int64 */ signersCount: number; signers: components["schemas"]["SignerSummary"][]; @@ -1892,6 +1894,7 @@ export type components = { nodeType: "file" | "envelope"; /** @enum {string} */ signatureFlow: "none" | "parallel" | "ordered_numeric"; + policySnapshotFrozen?: boolean; /** Format: int64 */ docmdpLevel: number; /** Format: int64 */ diff --git a/src/views/Settings/PolicyWorkbench/settings/signature-flow/model.ts b/src/views/Settings/PolicyWorkbench/settings/signature-flow/model.ts index 234c84cfba..f6950debd8 100644 --- a/src/views/Settings/PolicyWorkbench/settings/signature-flow/model.ts +++ b/src/views/Settings/PolicyWorkbench/settings/signature-flow/model.ts @@ -48,10 +48,16 @@ export function toRequestSignatureFlowOverride(flow: SignatureFlowMode | null): export function resolveSignatureFlowPayloadForRequest( canChooseSigningOrderAtRequestLevel: boolean, flow: SignatureFlowMode | null, -): RequestSignatureFlowOverride | null { + isFrozen = false, +): SignatureFlowMode | null { if (!canChooseSigningOrderAtRequestLevel) { return null } + // A frozen request only accepts the flow it already stores, `none` included. + if (isFrozen) { + return flow + } + return toRequestSignatureFlowOverride(flow) } diff --git a/tests/php/Api/Controller/FileControllerTest.php b/tests/php/Api/Controller/FileControllerTest.php index afe66fe67a..23e85c9039 100644 --- a/tests/php/Api/Controller/FileControllerTest.php +++ b/tests/php/Api/Controller/FileControllerTest.php @@ -13,6 +13,7 @@ use OCA\Libresign\Db\SignRequestMapper; use OCA\Libresign\Helper\JSActions; use OCA\Libresign\Service\Envelope\EnvelopeService; +use OCA\Libresign\Service\FileStatusService; use OCA\Libresign\Tests\Api\ApiTestCase; /** @@ -97,6 +98,38 @@ public function testValidateWithSuccessUsingUnloggedUser():void { // When there's no user and signer is not identified, settings is returned with default values $this->assertFalse($body['ocs']['data']['settings']['canRequestSign'], 'Can permission to request sign'); $this->assertFalse($body['ocs']['data']['settings']['canSign'], 'Can permission to sign'); + $this->assertArrayNotHasKey('policySnapshotFrozen', $body['ocs']['data']); + } + + /** + * @runInSeparateProcess + */ + public function testValidateTellsTheRequesterWhetherThePolicySnapshotIsFrozen():void { + $user = $this->createAccount('username', 'password'); + $file = $this->requestSignFile([ + 'file' => ['base64' => base64_encode(file_get_contents(__DIR__ . '/../../fixtures/pdfs/small_valid.pdf'))], + 'name' => 'test', + 'signers' => [ + [ + 'identifyMethods' => [[ + 'method' => 'email', + 'mandatory' => 0, + 'value' => 'person@test.coop', + ]], + ], + ], + 'userManager' => $user, + ]); + + $this->request + ->withRequestHeader([ + 'Authorization' => 'Basic ' . base64_encode('username:password') + ]) + ->withPath('/api/v1/file/validate/file_id/' . $file->getId()); + + $response = $this->assertRequest(); + $body = json_decode($response->getBody()->getContents(), true); + $this->assertSame(FileStatusService::isPolicySnapshotFrozen($file), $body['ocs']['data']['policySnapshotFrozen']); } /** diff --git a/tests/php/Api/Controller/RequestSignatureControllerTest.php b/tests/php/Api/Controller/RequestSignatureControllerTest.php index e79afb8b7d..9578176bfa 100644 --- a/tests/php/Api/Controller/RequestSignatureControllerTest.php +++ b/tests/php/Api/Controller/RequestSignatureControllerTest.php @@ -154,4 +154,50 @@ public function testPatchRegisterWithSuccess():void { $body = json_decode($response->getBody()->getContents(), true); $body['ocs']['data']['data']['signers'][] = ['email' => 'user@test.coop']; } + + /** + * @runInSeparateProcess + */ + public function testPatchRefusesAnotherSignatureFlowOnceTheRequestWasSent():void { + $user = $this->createAccount('allowrequestsign', 'password', 'testGroup'); + + $appConfig = $this->getMockAppConfig(); + $appConfig->setValueString(Application::APP_ID, 'groups_request_sign', '{"allowGroups":["admin","testGroup"],"denyGroups":[]}'); + $appConfig->setValueBool(Application::APP_ID, 'notifyUnsignedUser', false); + + $user->setEMailAddress('person@test.coop'); + $signers = [ + [ + 'identifyMethods' => [[ + 'method' => 'email', + 'requirement' => 'optional', + 'value' => 'person@test.coop', + ]], + ], + ]; + $file = $this->requestSignFile([ + 'file' => ['base64' => base64_encode(file_get_contents(__DIR__ . '/../../fixtures/pdfs/small_valid.pdf'))], + 'name' => 'test', + 'signers' => $signers, + 'userManager' => $user, + ]); + + $this->request + ->withMethod('PATCH') + ->withPath('/api/v1/request-signature') + ->withRequestHeader([ + 'Authorization' => 'Basic ' . base64_encode('allowrequestsign:password'), + 'Content-Type' => 'application/json' + ]) + ->withRequestBody([ + 'uuid' => $file->getUuid(), + 'signers' => $signers, + 'policy' => ['overrides' => ['signature_flow' => 'ordered_numeric']], + ]) + ->expectStatus(422); + + $response = $this->assertRequest(); + $body = json_decode($response->getBody()->getContents(), true); + $this->assertSame('The signing order cannot be changed after the signing flow has started.', $body['ocs']['data']['message']); + } } diff --git a/tests/php/Unit/Service/File/FileListServiceTest.php b/tests/php/Unit/Service/File/FileListServiceTest.php index 35317ad18e..2e6bbc934c 100644 --- a/tests/php/Unit/Service/File/FileListServiceTest.php +++ b/tests/php/Unit/Service/File/FileListServiceTest.php @@ -159,6 +159,118 @@ public static function provideSignatureFlowScenarios(): array { ]; } + /** @param array $metadata */ + #[DataProvider('providePolicySnapshotFreezeStates')] + public function testEveryFilePayloadTellsWhetherThePolicySnapshotIsFrozen(int $status, array $metadata, bool $expected): void { + $file = self::createFileEntity(1, 'file', 'doc.pdf', $metadata); + $file->setStatus($status); + $this->appConfig->method('getValueInt')->willReturn(100); + $this->signRequestMapper->method('getFilesAssociatedFilesWithMe')->willReturn([ + 'data' => [$file], + 'pagination' => new class { + public function setRouteName(string $routeName): void { + } + + public function getPagination(int $page, int $length, array $filter): array { + return []; + } + }, + ]); + $this->signRequestMapper->method('getByMultipleFileId')->willReturn([]); + $this->signRequestMapper->method('getByFileId')->willReturn([]); + $this->signRequestMapper->method('getIdentifyMethodsFromSigners')->willReturn([]); + $this->signRequestMapper->method('getVisibleElementsFromSigners')->willReturn([]); + $service = $this->getService(); + + $this->assertSame($expected, $service->formatSingleFile($this->user, $file)['policySnapshotFrozen']); + $this->assertSame($expected, $service->formatFileWithChildren($file, [], $this->user)['policySnapshotFrozen']); + $summaries = $service->listAssociatedFilesOfSignFlow($this->user, 1, 100, [], [], false); + $this->assertSame($expected, $summaries['data'][0]['policySnapshotFrozen']); + } + + public function testTheSignPagePayloadDoesNotTellWhetherThePolicySnapshotIsFrozen(): void { + $file = self::createFileEntity(1, 'file', 'doc.pdf'); + $this->signRequestMapper->method('getByMultipleFileId')->willReturn([]); + $this->signRequestMapper->method('getIdentifyMethodsFromSigners')->willReturn([]); + $this->signRequestMapper->method('getVisibleElementsFromSigners')->willReturn([]); + $service = $this->getService(); + + $this->assertArrayNotHasKey('policySnapshotFrozen', $service->formatSingleFileForSignRequest($file)); + $this->assertArrayNotHasKey('policySnapshotFrozen', $service->formatEnvelopeChildFilesForSignRequest([$file], [])[0]); + } + + public function testEveryFilePayloadOfADraftDocumentTellsItIsFrozenWhenItsEnvelopeWasSent(): void { + $envelope = self::createFileEntity(50, 'envelope', 'envelope', ['policy_snapshot_frozen_at' => '2026-01-01T00:00:00+00:00']); + $envelope->setStatus(0); + $documents = [self::createFileEntity(1, 'file', 'a.pdf'), self::createFileEntity(2, 'file', 'b.pdf')]; + foreach ($documents as $document) { + $document->setStatus(0); + $document->setParentFileId(50); + } + $this->fileMapper->method('getById')->with(50)->willReturn($envelope); + $this->appConfig->method('getValueInt')->willReturn(100); + $this->signRequestMapper->method('getFilesAssociatedFilesWithMe')->willReturn([ + 'data' => $documents, + 'pagination' => new class { + public function setRouteName(string $routeName): void { + } + + public function getPagination(int $page, int $length, array $filter): array { + return []; + } + }, + ]); + $this->signRequestMapper->method('getByMultipleFileId')->willReturn([]); + $this->signRequestMapper->method('getByFileId')->willReturn([]); + $this->signRequestMapper->method('getIdentifyMethodsFromSigners')->willReturn([]); + $this->signRequestMapper->method('getVisibleElementsFromSigners')->willReturn([]); + $service = $this->getService(); + + $this->assertTrue($service->formatSingleFile($this->user, $documents[0])['policySnapshotFrozen']); + $this->assertTrue($service->formatFileWithChildren($documents[0], [], $this->user)['policySnapshotFrozen']); + foreach ([false, true] as $details) { + $list = $service->listAssociatedFilesOfSignFlow($this->user, 1, 100, ['parentFileId' => 50], [], $details); + $this->assertSame([true, true], array_column($list['data'], 'policySnapshotFrozen')); + } + } + + public function testAListPageLoadsEachEnvelopeOnceAndReusesTheOnesItAlreadyHas(): void { + $listedEnvelope = self::createFileEntity(50, 'envelope', 'listed'); + $documents = []; + foreach ([[1, 50], [2, 60], [3, 60]] as [$id, $parentId]) { + $documents[] = $document = self::createFileEntity($id, 'file', $id . '.pdf'); + $document->setParentFileId($parentId); + } + $this->fileMapper->expects($this->once())->method('getById')->with(60) + ->willReturn(self::createFileEntity(60, 'envelope', 'other')); + $this->appConfig->method('getValueInt')->willReturn(100); + $this->signRequestMapper->method('getFilesAssociatedFilesWithMe')->willReturn([ + 'data' => [$listedEnvelope, ...$documents], + 'pagination' => new class { + public function setRouteName(string $routeName): void { + } + + public function getPagination(int $page, int $length, array $filter): array { + return []; + } + }, + ]); + $this->signRequestMapper->method('getByMultipleFileId')->willReturn([]); + $this->signRequestMapper->method('getIdentifyMethodsFromSigners')->willReturn([]); + + $list = $this->getService()->listAssociatedFilesOfSignFlow($this->user, 1, 100, [], [], false); + + $this->assertCount(4, $list['data']); + } + + public static function providePolicySnapshotFreezeStates(): array { + return [ + 'draft never sent' => [0, [], false], + 'sent request that returned to draft' => [0, ['policy_snapshot_frozen_at' => '2026-01-01T00:00:00+00:00'], true], + 'sent before the freeze was recorded' => [1, [], true], + ]; + } + public function testFileWithoutSignersUsesStatusMapping(): void { $file = self::createFileEntity(1, 'file', 'doc.pdf'); $file->setStatus(0); // DRAFT status diff --git a/tests/php/Unit/Service/FileServiceTest.php b/tests/php/Unit/Service/FileServiceTest.php index 9092322d11..0b500ee740 100644 --- a/tests/php/Unit/Service/FileServiceTest.php +++ b/tests/php/Unit/Service/FileServiceTest.php @@ -679,6 +679,68 @@ public function testToArrayUsesSignerScopedSignRequestUuidForCurrentSignerUrl(): $this->assertSame('libresign.page.getPdf:file-uuid', $result['files'][0]['file']); } + #[DataProvider('provideRequesterFiles')] + public function testToArrayTellsTheRequesterWhetherThePolicySnapshotIsFrozen(int $status, ?int $envelopeStatus, bool $expected): void { + $file = $this->createRequestedFile($status); + if ($envelopeStatus !== null) { + $envelope = $this->createRequestedFile($envelopeStatus); + $envelope->setId(10); + $envelope->setNodeType('envelope'); + $file->setParentFileId(10); + $this->envelopeService->method('getEnvelopeByFileId')->with(1)->willReturn($envelope); + } + + $result = $this->createFileService()->setFile($file)->setMe($this->createUser('creator-user'))->toArray(); + + $this->assertSame($expected, $result['policySnapshotFrozen']); + } + + /** @return iterable */ + public static function provideRequesterFiles(): iterable { + yield 'draft' => [FileStatus::DRAFT->value, null, false]; + yield 'sent' => [FileStatus::ABLE_TO_SIGN->value, null, true]; + yield 'draft document of a sent envelope' => [FileStatus::DRAFT->value, FileStatus::ABLE_TO_SIGN->value, true]; + } + + #[DataProvider('provideViewersWhoAreNotTheRequester')] + public function testToArrayTellsNobodyElseWhetherThePolicySnapshotIsFrozen(?string $viewerUid): void { + $result = $this->createFileService() + ->setFile($this->createRequestedFile(FileStatus::ABLE_TO_SIGN->value)) + ->setMe($viewerUid === null ? null : $this->createUser($viewerUid)) + ->toArray(); + + $this->assertArrayNotHasKey('policySnapshotFrozen', $result); + } + + /** @return iterable */ + public static function provideViewersWhoAreNotTheRequester(): iterable { + yield 'anonymous' => [null]; + yield 'another account' => ['signer-user']; + } + + private function createRequestedFile(int $status): \OCA\Libresign\Db\File { + $file = new \OCA\Libresign\Db\File(); + $file->setId(1); + $file->setUuid('file-uuid'); + $file->setName('contract.pdf'); + $file->setStatus($status); + $file->setCreatedAt(new \DateTime('2026-01-01T00:00:00Z')); + $file->setNodeId(100); + $file->setUserId('creator-user'); + + $creator = $this->createMock(\OCP\IUser::class); + $creator->method('getDisplayName')->willReturn('Creator User'); + $this->userManager->method('get')->willReturn($creator); + $this->fileMapper->method('getChildrenFiles')->willReturn([]); + return $file; + } + + private function createUser(string $uid): \OCP\IUser { + $user = $this->createMock(\OCP\IUser::class); + $user->method('getUID')->willReturn($uid); + return $user; + } + #[DataProvider('mapSignerDetailsToSummaryProvider')] public function testMapSignerDetailsToSummaryFiltersInvalidSigners(array $signers, array $expectedSummaries): void { $service = $this->createFileService(); diff --git a/tests/php/Unit/Service/FileStatusServiceTest.php b/tests/php/Unit/Service/FileStatusServiceTest.php index c77fa86bc6..0013b05220 100644 --- a/tests/php/Unit/Service/FileStatusServiceTest.php +++ b/tests/php/Unit/Service/FileStatusServiceTest.php @@ -578,6 +578,38 @@ public static function dataMarkPolicySnapshotFrozen(): array { ]; } + /** + * @param array $metadata + * @param array{0: int, 1: array}|null $envelopeState + */ + #[DataProvider('dataIsPolicySnapshotFrozen')] + public function testIsPolicySnapshotFrozen(int $status, array $metadata, ?array $envelopeState, bool $expected): void { + $file = new FileEntity(); + $file->setStatus($status); + $file->setMetadata($metadata); + $envelope = null; + if ($envelopeState !== null) { + $envelope = new FileEntity(); + $envelope->setStatus($envelopeState[0]); + $envelope->setMetadata($envelopeState[1]); + } + + $this->assertSame($expected, FileStatusService::isPolicySnapshotFrozen($file, $envelope)); + } + + public static function dataIsPolicySnapshotFrozen(): array { + $frozen = ['policy_snapshot_frozen_at' => '2026-01-01T00:00:00+00:00']; + return [ + 'draft never sent' => [FileStatus::DRAFT->value, [], null, false], + 'sent request' => [FileStatus::ABLE_TO_SIGN->value, $frozen, null, true], + 'sent request that returned to draft' => [FileStatus::DRAFT->value, $frozen, null, true], + 'sent before the freeze was recorded' => [FileStatus::ABLE_TO_SIGN->value, [], null, true], + 'document of a draft envelope' => [FileStatus::DRAFT->value, [], [FileStatus::DRAFT->value, []], false], + 'document of a sent envelope' => [FileStatus::DRAFT->value, [], [FileStatus::DRAFT->value, $frozen], true], + 'document of an envelope sent before the freeze was recorded' => [FileStatus::DRAFT->value, [], [FileStatus::ABLE_TO_SIGN->value, []], true], + ]; + } + public function testEveryDocumentFreezesWhenItsEnvelopeEntersTheSigningFlow(): void { $envelope = new FileEntity(); $envelope->setId(1); diff --git a/tests/php/Unit/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplierTest.php b/tests/php/Unit/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplierTest.php index ff3dd0c902..86eb6e314c 100644 --- a/tests/php/Unit/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplierTest.php +++ b/tests/php/Unit/Service/Policy/Provider/Signature/FilePolicy/SignatureFlowFilePolicyApplierTest.php @@ -8,17 +8,27 @@ namespace OCA\Libresign\Tests\Unit\Service\Policy\Provider\Signature\FilePolicy; +use OCA\Libresign\Db\File; +use OCA\Libresign\Db\SignRequestMapper; +use OCA\Libresign\Enum\FileStatus; use OCA\Libresign\Enum\SignatureFlow; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Service\FileService; +use OCA\Libresign\Service\IdentifyMethodService; +use OCA\Libresign\Service\Policy\Model\PolicySpec; use OCA\Libresign\Service\Policy\Model\ResolvedPolicy; use OCA\Libresign\Service\Policy\PolicyService; use OCA\Libresign\Service\Policy\Provider\Signature\FilePolicy\SignatureFlowFilePolicyApplier; use OCA\Libresign\Service\Policy\Provider\Signature\SignatureFlowPolicy; +use OCA\Libresign\Service\SequentialSigningService; use OCP\IL10N; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; final class SignatureFlowFilePolicyApplierTest extends \OCA\Libresign\Tests\Unit\TestCase { + private const FROZEN = ['policy_snapshot_frozen_at' => '2026-01-01T00:00:00+00:00']; + private PolicyService&MockObject $policyService; private FileService&MockObject $fileService; private IL10N&MockObject $l10n; @@ -26,6 +36,7 @@ final class SignatureFlowFilePolicyApplierTest extends \OCA\Libresign\Tests\Unit public function setUp(): void { parent::setUp(); $this->policyService = $this->createMock(PolicyService::class); + $this->policyService->method('getRequestLifecycle')->willReturn(PolicySpec::LIFECYCLE_REQUEST_SNAPSHOT); $this->fileService = $this->createMock(FileService::class); $this->l10n = $this->createMock(IL10N::class); $this->l10n->method('t')->willReturnArgument(0); @@ -116,6 +127,82 @@ public function testSyncUpdatesFileWhenSnapshotChanges(): void { $this->getApplier()->sync($file, []); } + /** + * @param array $metadata + * @param array $policyOverrides + */ + #[DataProvider('provideFrozenUpdates')] + public function testAFrozenRequestKeepsItsStoredFlow(int $status, array $metadata, array $policyOverrides): void { + $file = $this->createStoredRequest($status, $metadata, SignatureFlow::NONE); + $storedMetadata = $file->getMetadata(); + + $this->policyService->expects($this->never())->method('resolveForUserId'); + $this->fileService->expects($this->never())->method('update'); + + $this->getApplier()->sync($file, ['policyOverrides' => $policyOverrides]); + + $this->assertSame(SignatureFlow::NONE, $file->getSignatureFlowEnum()); + $this->assertSame($storedMetadata, $file->getMetadata()); + } + + /** @return iterable, 2: array}> */ + public static function provideFrozenUpdates(): iterable { + yield 'nothing submitted' => [FileStatus::ABLE_TO_SIGN->value, self::FROZEN, []]; + yield 'stored flow sent again' => [FileStatus::ABLE_TO_SIGN->value, self::FROZEN, [SignatureFlowPolicy::KEY => 'none']]; + yield 'null submitted' => [FileStatus::ABLE_TO_SIGN->value, self::FROZEN, [SignatureFlowPolicy::KEY => null]]; + yield 'returned to draft' => [FileStatus::DRAFT->value, self::FROZEN, []]; + yield 'sent before the freeze was recorded' => [FileStatus::ABLE_TO_SIGN->value, [], []]; + } + + #[DataProvider('provideFrozenChangeAttempts')] + public function testChangingTheFlowOfAFrozenRequestIsRefused(int $status, SignatureFlow $storedFlow, string $submittedFlow): void { + $file = $this->createStoredRequest($status, self::FROZEN, $storedFlow); + + $this->fileService->expects($this->never())->method('update'); + $this->expectException(LibresignException::class); + $this->expectExceptionCode(422); + $this->expectExceptionMessage('The signing order cannot be changed after the signing flow has started.'); + + $this->getApplier()->sync($file, ['policyOverrides' => [SignatureFlowPolicy::KEY => $submittedFlow]]); + } + + /** @return iterable */ + public static function provideFrozenChangeAttempts(): iterable { + yield 'another flow' => [FileStatus::ABLE_TO_SIGN->value, SignatureFlow::PARALLEL, 'ordered_numeric']; + yield 'parallel is not the same as no flow' => [FileStatus::ABLE_TO_SIGN->value, SignatureFlow::NONE, 'parallel']; + yield 'after returning to draft' => [FileStatus::DRAFT->value, SignatureFlow::PARALLEL, 'ordered_numeric']; + } + + public function testASignerAddedAfterTheFreezeFollowsTheStoredFlow(): void { + $file = $this->createStoredRequest(FileStatus::ABLE_TO_SIGN->value, self::FROZEN, SignatureFlow::ORDERED_NUMERIC); + $this->getApplier()->sync($file, []); + + $sequentialSigning = new SequentialSigningService( + $this->createMock(SignRequestMapper::class), + $this->createMock(IdentifyMethodService::class), + $this->createMock(LoggerInterface::class), + ); + $sequentialSigning->setFile($file); + $this->assertSame([1, 2], [ + $sequentialSigning->determineSigningOrder(null), + $sequentialSigning->determineSigningOrder(null), + ]); + } + + /** @param array $metadata */ + private function createStoredRequest(int $status, array $metadata, SignatureFlow $storedFlow): File { + $file = new File(); + $file->setUserId('john'); + $file->setStatus($status); + $file->setSignatureFlowEnum($storedFlow); + $file->setMetadata($metadata + [ + 'policy_snapshot' => [ + SignatureFlowPolicy::KEY => ['effectiveValue' => $storedFlow->value, 'sourceScope' => 'system'], + ], + ]); + return $file; + } + private function createResolvedPolicy( string $effectiveValue, string $sourceScope = 'system', diff --git a/tests/php/Unit/Service/RequestSignatureServiceTest.php b/tests/php/Unit/Service/RequestSignatureServiceTest.php index b6fda03b1b..19b28d11ee 100644 --- a/tests/php/Unit/Service/RequestSignatureServiceTest.php +++ b/tests/php/Unit/Service/RequestSignatureServiceTest.php @@ -229,6 +229,67 @@ public function testSaveFileReusesTheFileRegisteredForTheNodeId(): void { $this->assertSame($existing, $result); } + public function testSaveFileDoesNotCreateAnotherFileWhenTheRegisteredOneRefusesTheUpdate(): void { + $existing = new \OCA\Libresign\Db\File(); + $existing->setId(7); + $this->fileMapper->method('getByNodeId')->willReturn($existing); + $this->filePolicyApplier->method('syncAllPolicies') + ->willThrowException(new LibresignException('The signing order cannot be changed after the signing flow has started.', 422)); + $this->fileService->expects($this->never())->method('getNodeFromData'); + $this->filePolicyApplier->expects($this->never())->method('applyAll'); + $this->fileMapper->expects($this->never())->method('insert'); + + $this->expectException(LibresignException::class); + $this->expectExceptionCode(422); + + $this->getService()->saveFile([ + 'file' => ['nodeId' => 171], + 'name' => 'contract', + 'userManager' => $this->user, + ]); + } + + public function testSaveFileRefusesANodeRegisteredMoreThanOnceWithoutCreatingAnotherFile(): void { + $this->fileMapper->method('getByNodeId') + ->willThrowException(new \OCP\AppFramework\Db\MultipleObjectsReturnedException('Did expect one result but found more in query: SELECT *')); + $this->filePolicyApplier->expects($this->never())->method('syncAllPolicies'); + $this->fileService->expects($this->never())->method('getNodeFromData'); + $this->fileMapper->expects($this->never())->method('insert'); + + try { + $this->getService()->saveFile([ + 'file' => ['nodeId' => 171], + 'name' => 'contract', + 'userManager' => $this->user, + ]); + $this->fail('A node registered more than once must not be saved.'); + } catch (LibresignException $e) { + $this->assertSame(422, $e->getCode()); + $this->assertStringNotContainsString('SELECT', $e->getMessage()); + } + } + + public function testSaveFileCreatesTheFileWhenTheNodeIsNotRegisteredYet(): void { + $this->fileMapper->method('getByNodeId') + ->willThrowException(new \OCP\AppFramework\Db\DoesNotExistException('not found')); + $node = $this->createMock(\OCP\Files\File::class); + $node->method('getId')->willReturn(171); + $node->method('getExtension')->willReturn(''); + $node->method('getName')->willReturn('contract'); + $this->fileService->method('getNodeFromData')->willReturn($node); + $this->filePolicyApplier->expects($this->never())->method('syncAllPolicies'); + $this->filePolicyApplier->expects($this->once())->method('applyAll'); + $this->fileMapper->expects($this->once())->method('insert')->willReturnArgument(0); + + $file = $this->getService()->saveFile([ + 'file' => ['nodeId' => 171], + 'name' => 'contract', + 'userManager' => $this->user, + ]); + + $this->assertSame(171, $file->getNodeId()); + } + public function testANewRequestCreatedReadyToSignStartsWithItsPolicySnapshotFrozen(): void { $node = $this->createMock(\OCP\Files\File::class); $node->method('getId')->willReturn(171);