From da68d926352390fb1154831b0e335506dc4b7b87 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 20:58:50 -0300 Subject: [PATCH 01/36] feat: integrate pdf-signature-validator for native validation Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- composer.json | 9 + lib/Handler/SignEngine/Pkcs12Handler.php | 234 ++++++---------- .../PdfSignatureValidationService.php | 253 ++++++++++++++++++ .../Handler/SignEngine/Pkcs12HandlerTest.php | 13 +- .../PdfSignatureValidationServiceTest.php | 62 +++++ 5 files changed, 409 insertions(+), 162 deletions(-) create mode 100644 lib/Service/Signature/PdfSignatureValidationService.php create mode 100644 tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php diff --git a/composer.json b/composer.json index c40bf03912..70eb02933d 100644 --- a/composer.json +++ b/composer.json @@ -1,4 +1,13 @@ { + "repositories": [ + { + "type": "path", + "url": "../pdf-signature-validator", + "options": { + "symlink": true + } + } + ], "require-dev": { "bamarni/composer-bin-plugin": "^1.8", "nextcloud/ocp": "dev-stable32", diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 5bb90a1242..ea7a4aa85a 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -3,12 +3,12 @@ declare(strict_types=1); /** * SPDX-FileCopyrightText: 2020-2024 LibreCode coop and contributors - * SPDX-License-Identifier: AGPL-3.0-or-later + * SPDX-License-Identifier: AGPL-3.0-or-later */ namespace OCA\Libresign\Handler\SignEngine; -use DateTime; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; @@ -19,17 +19,16 @@ use OCA\Libresign\Service\CaIdentifierService; use OCA\Libresign\Service\Crl\CrlService; use OCA\Libresign\Service\FolderService; +use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Vendor\phpseclib3\File\ASN1; use OCP\Files\File; use OCP\IAppConfig; use OCP\IL10N; -use OCP\ITempManager; use Psr\Log\LoggerInterface; class Pkcs12Handler extends SignEngineHandler { use OrderCertificatesTrait; protected string $certificate = ''; - private array $signaturesFromPoppler = []; private ?JSignPdfHandler $jSignPdfHandler = null; private string $rootCertificatePem = ''; private bool $isLibreSignFile = false; @@ -40,11 +39,12 @@ public function __construct( protected CertificateEngineFactory $certificateEngineFactory, private IL10N $l10n, private FooterHandler $footerHandler, - private ITempManager $tempManager, private LoggerInterface $logger, private CaIdentifierService $caIdentifierService, private DocMdpHandler $docMdpHandler, private CrlService $crlService, + private PdfSignatureValidationService $pdfSignatureValidationService, + private PdfSignatureExtractor $pdfSignatureExtractor, ) { parent::__construct($l10n, $folderService, $logger); } @@ -109,11 +109,14 @@ public function getCertificateChain($resource): array { return $certificates; } - private function processSignature($resource, ?string $signature): array { + private function processSignature($resource, ?string $signature, array $metadata = [], array $validation = []): array { $result = []; if (!$signature) { - $result['chain'][0]['signature_validation'] = $this->getReadableSigState('Digest Mismatch.'); + $result['chain'][0]['signature_validation'] = [ + 'id' => 3, + 'label' => $this->l10n->t('Digest mismatch.'), + ]; return $result; } @@ -123,7 +126,7 @@ private function processSignature($resource, ?string $signature): array { $chain = $this->extractCertificateChain($signature); if (!empty($chain)) { $result['chain'] = $this->orderCertificates($chain); - $result = $this->enrichLeafWithPopplerData($resource, $result); + $result = $this->enrichLeafWithNativeData($result, $metadata, $validation); } $result = $this->extractDocMdpData($resource, $result); @@ -275,149 +278,57 @@ private function getRootCertificatePem(): string { return $this->rootCertificatePem; } - private function enrichLeafWithPopplerData($resource, array $result): array { + private function enrichLeafWithNativeData(array $result, array $metadata, array $validation): array { if (empty($result['chain'])) { return $result; } - $popplerOnlyFields = ['field', 'range', 'certificate_validation']; - if (!isset($result['chain'][0]['subject'])) { - return $result; - } - $needPoppler = false; - foreach ($popplerOnlyFields as $field) { - if (empty($result['chain'][0][$field])) { - $needPoppler = true; - break; - } - } - if (!isset($result['chain'][0]['signature_validation']) || $result['chain'][0]['signature_validation']['id'] !== 1) { - $needPoppler = true; - } - if (!$needPoppler) { - return $result; - } - $popplerChain = $this->chainFromPoppler($result['chain'][0]['subject'], $resource); - if (empty($popplerChain)) { - return $result; - } - foreach ($popplerOnlyFields as $field) { - if (isset($popplerChain[$field])) { - $result['chain'][0][$field] = $popplerChain[$field]; - } - } - if (!isset($result['chain'][0]['signature_validation']) || $result['chain'][0]['signature_validation']['id'] !== 1) { - if (isset($popplerChain['signature_validation'])) { - $result['chain'][0]['signature_validation'] = $popplerChain['signature_validation']; - } - } - return $result; - } + $leaf = &$result['chain'][0]; - private function chainFromPoppler(array $subject, $resource): array { - $fromFallback = $this->popplerUtilsPdfSignFallback($resource); - foreach ($fromFallback as $popplerSig) { - if (!isset($popplerSig['chain'][0]['subject'])) { - continue; - } - if ($popplerSig['chain'][0]['subject'] == $subject) { - return $popplerSig['chain'][0]; + foreach (['field', 'range', 'signature_type', 'signing_hash_algorithm', 'covers_entire_document'] as $key) { + if (array_key_exists($key, $metadata)) { + $leaf[$key] = $metadata[$key]; } } - return []; - } - private function popplerUtilsPdfSignFallback($resource): array { - if (!empty($this->signaturesFromPoppler)) { - return $this->signaturesFromPoppler; - } - if (shell_exec('which pdfsig') === null) { - return $this->signaturesFromPoppler; + if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { + $leaf['signature_validation'] = $this->getLocalizedSignatureValidation($validation['signatureValidation']); } - rewind($resource); - $content = stream_get_contents($resource); - $tempFile = $this->tempManager->getTemporaryFile('file.pdf'); - file_put_contents($tempFile, $content); - $content = shell_exec('env TZ=UTC pdfsig ' . $tempFile); - if (empty($content)) { - return $this->signaturesFromPoppler; + if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { + $leaf['certificate_validation'] = $this->getLocalizedCertificateValidation($validation['certificateValidation']); } - $lines = explode("\n", $content); - $lastSignature = 0; - foreach ($lines as $item) { - $isFirstLevel = preg_match('/^Signature\s#(\d)/', $item, $match); - if ($isFirstLevel) { - $lastSignature = (int)$match[1] - 1; - $this->signaturesFromPoppler[$lastSignature] = []; - continue; - } - - $match = []; - $isSecondLevel = preg_match('/^\s+-\s(?.+):\s(?.*)/', $item, $match); - if ($isSecondLevel) { - switch ((string)$match['key']) { - case 'Signing Time': - $this->signaturesFromPoppler[$lastSignature]['signingTime'] = DateTime::createFromFormat('M d Y H:i:s', $match['value'], new \DateTimeZone('UTC')); - break; - case 'Signer full Distinguished Name': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['subject'] = $this->parseDistinguishedNameWithMultipleValues($match['value']); - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['name'] = $match['value']; - break; - case 'Signing Hash Algorithm': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['signatureTypeSN'] = $match['value']; - break; - case 'Signature Validation': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['signature_validation'] = $this->getReadableSigState($match['value']); - break; - case 'Certificate Validation': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['certificate_validation'] = $this->getReadableCertState($match['value']); - break; - case 'Signed Ranges': - if (preg_match('/\[(\d+) - (\d+)\], \[(\d+) - (\d+)\]/', $match['value'], $ranges)) { - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['range'] = [ - 'offset1' => (int)$ranges[1], - 'length1' => (int)$ranges[2], - 'offset2' => (int)$ranges[3], - 'length2' => (int)$ranges[4], - ]; - } - break; - case 'Signature Field Name': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['field'] = $match['value']; - break; - case 'Signature Validation': - case 'Signature Type': - case 'Total document signed': - case 'Not total document signed': - default: - break; - } - } + if (!isset($leaf['certificate_validation'])) { + $leaf['certificate_validation'] = [ + 'id' => 3, + 'label' => $this->l10n->t('Certificate issuer is unknown.'), + ]; } - return $this->signaturesFromPoppler; + + return $result; } - private function getReadableSigState(string $status) { - return match ($status) { - 'Signature is Valid.' => [ + /** + * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. + */ + private function getLocalizedSignatureValidation(array $validation): array { + $id = (int)($validation['id'] ?? 6); + + return match ($id) { + 1 => [ 'id' => 1, 'label' => $this->l10n->t('Signature is valid.'), ], - 'Signature is Invalid.' => [ + 2 => [ 'id' => 2, 'label' => $this->l10n->t('Signature is invalid.'), ], - 'Digest Mismatch.' => [ + 3 => [ 'id' => 3, 'label' => $this->l10n->t('Digest mismatch.'), ], - "Document isn't signed or corrupted data." => [ - 'id' => 4, - 'label' => $this->l10n->t("Document isn't signed or corrupted data."), - ], - 'Signature has not yet been verified.' => [ + 5 => [ 'id' => 5, 'label' => $this->l10n->t('Signature has not yet been verified.'), ], @@ -428,63 +339,72 @@ private function getReadableSigState(string $status) { }; } - private function getReadableCertState(string $status) { - return match ($status) { - 'Certificate is Trusted.' => [ + /** + * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. + */ + private function getLocalizedCertificateValidation(array $validation): array { + $id = (int)($validation['id'] ?? 7); + + return match ($id) { + 1 => [ 'id' => 1, 'label' => $this->l10n->t('Certificate is trusted.'), ], - "Certificate issuer isn't Trusted." => [ + 2 => [ 'id' => 2, 'label' => $this->l10n->t("Certificate issuer isn't trusted."), ], - 'Certificate issuer is unknown.' => [ + 3 => [ 'id' => 3, 'label' => $this->l10n->t('Certificate issuer is unknown.'), ], - 'Certificate has been Revoked.' => [ + 4 => [ 'id' => 4, 'label' => $this->l10n->t('Certificate has been revoked.'), ], - 'Certificate has Expired' => [ + 5 => [ 'id' => 5, 'label' => $this->l10n->t('Certificate has expired'), ], - 'Certificate has not yet been verified.' => [ + 6 => [ 'id' => 6, 'label' => $this->l10n->t('Certificate has not yet been verified.'), ], default => [ 'id' => 7, - 'label' => $this->l10n->t('Unknown issue with Certificate or corrupted data.') + 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), ], }; } - private function parseDistinguishedNameWithMultipleValues(string $dn): array { - $result = []; - $pairs = preg_split('/,(?=(?:[^"]*"[^"]*")*[^"]*$)/', $dn); + /** + * @param resource $resource + * @return list + */ + private function extractNativeSignatureMetadata($resource): array { + rewind($resource); + $content = stream_get_contents($resource); + if (!is_string($content) || $content === '') { + return []; + } - foreach ($pairs as $pair) { - [$key, $value] = explode('=', $pair, 2); - if (empty($key) || empty($value)) { - return $result; - } - $key = trim($key); - $value = trim($value); - $value = trim($value, '"'); - - if (!isset($result[$key])) { - $result[$key] = $value; - } else { - if (!is_array($result[$key])) { - $result[$key] = [$result[$key]]; - } - $result[$key][] = $value; - } + try { + $signatures = $this->pdfSignatureExtractor->extractFromString($content); + } catch (\Throwable) { + return []; } + $metadata = []; - return $result; + foreach ($signatures as $index => $signature) { + $metadata[$index] = [ + 'field' => $signature->metadata->field, + 'range' => $signature->metadata->range, + 'signature_type' => $signature->metadata->signatureType, + 'covers_entire_document' => $signature->metadata->coversEntireDocument, + ]; + } + + return $metadata; } private function der2pem($derData) { diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php new file mode 100644 index 0000000000..c90d62741b --- /dev/null +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -0,0 +1,253 @@ +validator = new PdfSignatureValidator(); + $this->loadLibreSignCaCertificate(); + } + + /** + * Load LibreSign CA certificate and set it as trusted root. + */ + private function loadLibreSignCaCertificate(): void { + // Try to load from config path first + $configPath = $this->appConfig->getValueString(Application::APP_ID, 'config_path'); + if (!empty($configPath) && is_dir($configPath)) { + $caPemPath = $configPath . DIRECTORY_SEPARATOR . 'ca.pem'; + if (is_readable($caPemPath)) { + $cert = @file_get_contents($caPemPath); + if ($cert !== false) { + $this->libresignCaCertificate = $cert; + $this->validator->addTrustedRoot($cert); + return; + } + } + } + + // Try alternate location + $alternateConfig = $this->appConfig->getValueString( + Application::APP_ID, + 'libresign_ca_certificate' + ); + if (!empty($alternateConfig)) { + $this->libresignCaCertificate = $alternateConfig; + $this->validator->addTrustedRoot($alternateConfig); + } + } + + /** + * Add additional trusted root certificate. + */ + public function addTrustedRoot(string $certificatePem): void { + $this->validator->addTrustedRoot($certificatePem); + } + + /** + * Set multiple trusted root certificates. + * + * @param list $certificates + */ + public function setTrustedRoots(array $certificates): void { + $this->validator->setTrustedRoots($certificates); + } + + /** + * Validate PDF signatures from file resource. + * + * @param resource $resource PDF file resource + * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) + * @return list + */ + public function validateFromResource($resource, ?DateTime $signatureTime = null): array { + try { + $results = $this->validator->validateFromResource($resource); + return $this->mapValidationResults($results, $signatureTime); + } catch (\Throwable $e) { + $this->logger->warning('PDF signature validation failed', [ + 'error' => $e->getMessage(), + 'trace' => $e->getTraceAsString(), + ]); + return []; + } + } + + /** + * Validate PDF signatures from binary content. + * + * @param string $pdfContent Binary PDF content + * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) + * @return list + */ + public function validateFromString(string $pdfContent, ?DateTime $signatureTime = null): array { + try { + $results = $this->validator->validateFromString($pdfContent); + return $this->mapValidationResults($results, $signatureTime); + } catch (\Throwable $e) { + $this->logger->warning('PDF signature validation failed', [ + 'error' => $e->getMessage(), + 'trace' => $e->getTraceAsString(), + ]); + return []; + } + } + + /** + * Map validation results from PdfSignatureValidator to LibreSign format. + * + * @param list $results Results from PdfSignatureValidator + * @param ?\DateTime $signatureTime + * @return list + */ + private function mapValidationResults(array $results, ?DateTime $signatureTime = null): array { + $mapped = []; + + foreach ($results as $result) { + $sigValidation = $result['signatureValidation'] ?? null; + $certValidation = $result['certificateValidation'] ?? null; + + if (!$sigValidation instanceof ValidationResult || !$certValidation instanceof ValidationResult) { + continue; + } + + $mapped[] = [ + 'signatureValidation' => $this->mapSignatureValidation($sigValidation), + 'certificateValidation' => $this->mapCertificateValidation($certValidation), + 'raw' => [ + 'signature' => $sigValidation, + 'certificate' => $certValidation, + ], + ]; + } + + return $mapped; + } + + /** + * Map signature validation result to LibreSign format. + */ + private function mapSignatureValidation(ValidationResult $result): array { + return match ($result->state) { + ValidationState::SIGNATURE_VALID => [ + 'id' => 1, + 'label' => 'Signature is valid.', + 'isValid' => true, + ], + ValidationState::SIGNATURE_INVALID => [ + 'id' => 2, + 'label' => 'Signature is invalid.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::DIGEST_MISMATCH => [ + 'id' => 3, + 'label' => 'Digest mismatch.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::NOT_VERIFIED => [ + 'id' => 5, + 'label' => 'Signature has not yet been verified.', + 'reason' => $result->reason, + 'isValid' => false, + ], + default => [ + 'id' => 6, + 'label' => 'Unknown validation failure.', + 'reason' => $result->reason, + 'isValid' => false, + ], + }; + } + + /** + * Map certificate validation result to LibreSign format. + */ + private function mapCertificateValidation(ValidationResult $result): array { + return match ($result->state) { + ValidationState::CERT_TRUSTED => [ + 'id' => 1, + 'label' => 'Certificate is trusted.', + 'isValid' => true, + ], + ValidationState::CERT_ISSUER_NOT_TRUSTED => [ + 'id' => 2, + 'label' => "Certificate issuer isn't trusted.", + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_ISSUER_UNKNOWN => [ + 'id' => 3, + 'label' => 'Certificate issuer is unknown.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_REVOKED => [ + 'id' => 4, + 'label' => 'Certificate has been revoked.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_EXPIRED => [ + 'id' => 5, + 'label' => 'Certificate has expired.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_NOT_VERIFIED => [ + 'id' => 6, + 'label' => 'Certificate has not yet been verified.', + 'reason' => $result->reason, + 'isValid' => false, + ], + default => [ + 'id' => 7, + 'label' => 'Unknown issue with certificate or corrupted data.', + 'reason' => $result->reason, + 'isValid' => false, + ], + }; + } + + /** + * Check if LibreSign CA is loaded. + */ + public function isLibreSignCaLoaded(): bool { + return !empty($this->libresignCaCertificate); + } + + /** + * Get LibreSign CA certificate (PEM format). + */ + public function getLibreSignCaCertificate(): string { + return $this->libresignCaCertificate; + } +} diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index 888ec3f955..1864525ab6 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -9,6 +9,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; use OCA\Libresign\Handler\CertificateEngine\IEngineHandler; @@ -18,12 +19,12 @@ use OCA\Libresign\Service\CaIdentifierService; use OCA\Libresign\Service\Crl\CrlService; use OCA\Libresign\Service\FolderService; +use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Fixtures\PdfFixtureCatalog; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\IAppConfig; use OCP\IL10N; -use OCP\ITempManager; use OCP\L10N\IFactory as IL10NFactory; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; @@ -35,13 +36,14 @@ final class Pkcs12HandlerTest extends \OCA\Libresign\Tests\Unit\TestCase { private IAppConfig $appConfig; private IL10N $l10n; private FooterHandler&MockObject $footerHandler; - private ITempManager $tempManager; private LoggerInterface&MockObject $logger; private CertificateEngineFactory&MockObject $certificateEngineFactory; private IEngineHandler&MockObject $certificateEngine; private CaIdentifierService&MockObject $caIdentifierService; private DocMdpHandler&MockObject $docMdpHandler; private CrlService&MockObject $crlService; + private PdfSignatureValidationService&MockObject $pdfSignatureValidationService; + private PdfSignatureExtractor $pdfSignatureExtractor; public function setUp(): void { $this->folderService = $this->createMock(FolderService::class); @@ -50,7 +52,6 @@ public function setUp(): void { $this->certificateEngine = $this->createMock(IEngineHandler::class); $this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID); $this->footerHandler = $this->createMock(FooterHandler::class); - $this->tempManager = \OCP\Server::get(ITempManager::class); $this->logger = $this->createMock(LoggerInterface::class); $this->caIdentifierService = $this->createMock(CaIdentifierService::class); $this->docMdpHandler = $this->createMock(DocMdpHandler::class); @@ -71,11 +72,12 @@ private function getHandler(array $methods = []): Pkcs12Handler|MockObject { $this->certificateEngineFactory, $this->l10n, $this->footerHandler, - $this->tempManager, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, + $this->pdfSignatureValidationService, + $this->pdfSignatureExtractor, ]) ->onlyMethods($methods) ->getMock(); @@ -86,11 +88,12 @@ private function getHandler(array $methods = []): Pkcs12Handler|MockObject { $this->certificateEngineFactory, $this->l10n, $this->footerHandler, - $this->tempManager, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, + $this->pdfSignatureValidationService, + $this->pdfSignatureExtractor, ); } diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php new file mode 100644 index 0000000000..28aa701023 --- /dev/null +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -0,0 +1,62 @@ +newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapSignatureValidation', + new ValidationResult(ValidationState::DIGEST_MISMATCH, 'hash mismatch') + ); + + $this->assertSame(3, $result['id']); + $this->assertSame('Digest mismatch.', $result['label']); + $this->assertSame('hash mismatch', $result['reason']); + $this->assertFalse($result['isValid']); + } + + public function testMapCertificateValidationWithEnumState(): void { + $service = $this->newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapCertificateValidation', + new ValidationResult(ValidationState::CERT_TRUSTED) + ); + + $this->assertSame(1, $result['id']); + $this->assertSame('Certificate is trusted.', $result['label']); + $this->assertTrue($result['isValid']); + } + + private function newServiceWithoutConstructor(): PdfSignatureValidationService { + $reflection = new \ReflectionClass(PdfSignatureValidationService::class); + /** @var PdfSignatureValidationService $service */ + $service = $reflection->newInstanceWithoutConstructor(); + return $service; + } + + /** + * @return array + */ + private function invokePrivateMethod(PdfSignatureValidationService $service, string $method, ValidationResult $result): array { + $reflection = new \ReflectionClass($service); + $target = $reflection->getMethod($method); + $target->setAccessible(true); + /** @var array $mapped */ + $mapped = $target->invoke($service, $result); + return $mapped; + } +} From e065851801eb76be153b09574ee10b50c55ca454 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:00:59 -0300 Subject: [PATCH 02/36] refactor: translate signature validation messages via l10n Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 54 +++++++++++-------- .../PdfSignatureValidationServiceTest.php | 17 ++++++ 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index c90d62741b..a45498cb3a 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -14,6 +14,7 @@ use LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\AppInfo\Application; use OCP\IAppConfig; +use OCP\IL10N; use Psr\Log\LoggerInterface; /** @@ -29,6 +30,7 @@ class PdfSignatureValidationService { public function __construct( private IAppConfig $appConfig, + private IL10N $l10n, private LoggerInterface $logger, ) { $this->validator = new PdfSignatureValidator(); @@ -158,31 +160,31 @@ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ 'id' => 1, - 'label' => 'Signature is valid.', + 'label' => $this->l10n->t('Signature is valid.'), 'isValid' => true, ], ValidationState::SIGNATURE_INVALID => [ 'id' => 2, - 'label' => 'Signature is invalid.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Signature is invalid.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, - 'label' => 'Digest mismatch.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Digest mismatch.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, - 'label' => 'Signature has not yet been verified.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Signature has not yet been verified.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, - 'label' => 'Unknown validation failure.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Unknown validation failure.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], }; @@ -195,48 +197,56 @@ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ 'id' => 1, - 'label' => 'Certificate is trusted.', + 'label' => $this->l10n->t('Certificate is trusted.'), 'isValid' => true, ], ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, - 'label' => "Certificate issuer isn't trusted.", - 'reason' => $result->reason, + 'label' => $this->l10n->t("Certificate issuer isn't trusted."), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, - 'label' => 'Certificate issuer is unknown.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate issuer is unknown.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, - 'label' => 'Certificate has been revoked.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate has been revoked.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, - 'label' => 'Certificate has expired.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate has expired.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, - 'label' => 'Certificate has not yet been verified.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate has not yet been verified.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, - 'label' => 'Unknown issue with certificate or corrupted data.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], }; } + private function translateReason(?string $reason): ?string { + if ($reason === null || $reason === '') { + return $reason; + } + + return $this->l10n->t($reason); + } + /** * Check if LibreSign CA is loaded. */ diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index 28aa701023..1b5a8ee408 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -12,8 +12,20 @@ use LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Unit\TestCase; +use OCP\IL10N; +use PHPUnit\Framework\MockObject\MockObject; final class PdfSignatureValidationServiceTest extends TestCase { + private IL10N&MockObject $l10n; + + public function setUp(): void { + parent::setUp(); + $this->l10n = $this->createMock(IL10N::class); + $this->l10n + ->method('t') + ->willReturnCallback(static fn (string $text): string => $text); + } + public function testMapSignatureValidationWithEnumState(): void { $service = $this->newServiceWithoutConstructor(); $result = $this->invokePrivateMethod( @@ -45,6 +57,11 @@ private function newServiceWithoutConstructor(): PdfSignatureValidationService { $reflection = new \ReflectionClass(PdfSignatureValidationService::class); /** @var PdfSignatureValidationService $service */ $service = $reflection->newInstanceWithoutConstructor(); + + $l10nProperty = $reflection->getProperty('l10n'); + $l10nProperty->setAccessible(true); + $l10nProperty->setValue($service, $this->l10n); + return $service; } From 74b629f4894e02e7e8c87107e566e0698b63fc01 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:01:59 -0300 Subject: [PATCH 03/36] refactor: avoid translating dynamic reason strings Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index a45498cb3a..8a81bccd5b 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -166,25 +166,25 @@ private function mapSignatureValidation(ValidationResult $result): array { ValidationState::SIGNATURE_INVALID => [ 'id' => 2, 'label' => $this->l10n->t('Signature is invalid.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, 'label' => $this->l10n->t('Digest mismatch.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, 'label' => $this->l10n->t('Signature has not yet been verified.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], default => [ 'id' => 6, 'label' => $this->l10n->t('Unknown validation failure.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], }; @@ -203,50 +203,42 @@ private function mapCertificateValidation(ValidationResult $result): array { ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, 'label' => $this->l10n->t("Certificate issuer isn't trusted."), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, 'label' => $this->l10n->t('Certificate issuer is unknown.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, 'label' => $this->l10n->t('Certificate has been revoked.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, 'label' => $this->l10n->t('Certificate has expired.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, 'label' => $this->l10n->t('Certificate has not yet been verified.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], default => [ 'id' => 7, 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], }; } - private function translateReason(?string $reason): ?string { - if ($reason === null || $reason === '') { - return $reason; - } - - return $this->l10n->t($reason); - } - /** * Check if LibreSign CA is loaded. */ From aa0ab74bfbe9da9f1cb44e7c87e0bcaa64029244 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:03:58 -0300 Subject: [PATCH 04/36] feat: add translatable dictionary for validation reasons Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 64 ++++++++++++++++--- .../PdfSignatureValidationServiceTest.php | 22 +++++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 8a81bccd5b..ad3a4e0c43 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -166,25 +166,25 @@ private function mapSignatureValidation(ValidationResult $result): array { ValidationState::SIGNATURE_INVALID => [ 'id' => 2, 'label' => $this->l10n->t('Signature is invalid.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, 'label' => $this->l10n->t('Digest mismatch.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, 'label' => $this->l10n->t('Signature has not yet been verified.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, 'label' => $this->l10n->t('Unknown validation failure.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], }; @@ -203,42 +203,86 @@ private function mapCertificateValidation(ValidationResult $result): array { ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, 'label' => $this->l10n->t("Certificate issuer isn't trusted."), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, 'label' => $this->l10n->t('Certificate issuer is unknown.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, 'label' => $this->l10n->t('Certificate has been revoked.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, 'label' => $this->l10n->t('Certificate has expired.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, 'label' => $this->l10n->t('Certificate has not yet been verified.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], }; } + private function translateKnownReason(?string $reason): ?string { + if ($reason === null || $reason === '') { + return $reason; + } + + if (preg_match('/^Intermediate certificate at position (\d+) is not signed by issuer$/', $reason, $matches) === 1) { + return $this->l10n->t( + 'Intermediate certificate at position %s is not signed by issuer', + [$matches[1]] + ); + } + + $prefix = 'Certificate validation failed: '; + if (str_starts_with($reason, $prefix)) { + $detail = substr($reason, strlen($prefix)); + $translatedDetail = $this->translateKnownReason($detail) ?? $detail; + return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); + } + + return match ($reason) { + 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), + 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), + 'Signature does not match certificate' => $this->l10n->t('Signature does not match certificate'), + 'Failed to parse certificate' => $this->l10n->t('Failed to parse certificate'), + 'Certificate was not valid at time of signature' => $this->l10n->t('Certificate was not valid at time of signature'), + 'Certificate has expired' => $this->l10n->t('Certificate has expired'), + 'Empty certificate chain' => $this->l10n->t('Empty certificate chain'), + 'Certificate has no serial number' => $this->l10n->t('Certificate has no serial number'), + 'Certificate found in CRL' => $this->l10n->t('Certificate found in CRL'), + 'Invalid certificate' => $this->l10n->t('Invalid certificate'), + 'Leaf certificate is marked as CA' => $this->l10n->t('Leaf certificate is marked as CA'), + 'Certificate signature validation failed' => $this->l10n->t('Certificate signature validation failed'), + 'Self-signed certificate not in trusted roots' => $this->l10n->t('Self-signed certificate not in trusted roots'), + 'Root certificate is not self-signed' => $this->l10n->t('Root certificate is not self-signed'), + 'Root certificate is not in trusted list' => $this->l10n->t('Root certificate is not in trusted list'), + 'No binary signature' => $this->l10n->t('No binary signature'), + 'No certificates in signature' => $this->l10n->t('No certificates in signature'), + 'Signing certificate has expired' => $this->l10n->t('Signing certificate has expired'), + 'Signing certificate has been revoked' => $this->l10n->t('Signing certificate has been revoked'), + 'Signature verification incomplete' => $this->l10n->t('Signature verification incomplete'), + default => $reason, + }; + } + /** * Check if LibreSign CA is loaded. */ diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index 1b5a8ee408..cdc30d5a10 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -53,6 +53,28 @@ public function testMapCertificateValidationWithEnumState(): void { $this->assertTrue($result['isValid']); } + public function testMapReasonUsesDictionaryForKnownReason(): void { + $service = $this->newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapSignatureValidation', + new ValidationResult(ValidationState::DIGEST_MISMATCH, 'PDF content hash does not match signed digest') + ); + + $this->assertSame('PDF content hash does not match signed digest', $result['reason']); + } + + public function testMapReasonKeepsUnknownReasonUntouched(): void { + $service = $this->newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapSignatureValidation', + new ValidationResult(ValidationState::DIGEST_MISMATCH, 'custom runtime detail') + ); + + $this->assertSame('custom runtime detail', $result['reason']); + } + private function newServiceWithoutConstructor(): PdfSignatureValidationService { $reflection = new \ReflectionClass(PdfSignatureValidationService::class); /** @var PdfSignatureValidationService $service */ From 6250c8c97dfb9eeca793a0cb0f7a5523fede57c8 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:05:10 -0300 Subject: [PATCH 05/36] docs: add translator guidance for validation reasons Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index ad3a4e0c43..cb6963bd3a 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -245,6 +245,7 @@ private function translateKnownReason(?string $reason): ?string { } if (preg_match('/^Intermediate certificate at position (\d+) is not signed by issuer$/', $reason, $matches) === 1) { + // TRANSLATORS: %s is the numeric position of an intermediate certificate in the chain. return $this->l10n->t( 'Intermediate certificate at position %s is not signed by issuer', [$matches[1]] @@ -255,9 +256,12 @@ private function translateKnownReason(?string $reason): ?string { if (str_starts_with($reason, $prefix)) { $detail = substr($reason, strlen($prefix)); $translatedDetail = $this->translateKnownReason($detail) ?? $detail; + // TRANSLATORS: %s is a translated certificate validation detail message. return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); } + // TRANSLATORS: Technical validation reasons from pdf-signature-validator. + // Keep protocol/acronym terms like ByteRange, CRL and CA as-is. return match ($reason) { 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), From 1022811cf306240a8f41af1c5fbfb869fc43229c Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:06:48 -0300 Subject: [PATCH 06/36] docs: place translator comments above each l10n call Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index cb6963bd3a..b92b4f072c 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -160,29 +160,34 @@ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ 'id' => 1, + // TRANSLATORS: User-facing status when signature cryptographic validation succeeds. 'label' => $this->l10n->t('Signature is valid.'), 'isValid' => true, ], ValidationState::SIGNATURE_INVALID => [ 'id' => 2, + // TRANSLATORS: User-facing status when signature cryptographic validation fails. 'label' => $this->l10n->t('Signature is invalid.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, + // TRANSLATORS: User-facing status when signed digest does not match PDF content. 'label' => $this->l10n->t('Digest mismatch.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, + // TRANSLATORS: User-facing status when validation could not be fully completed. 'label' => $this->l10n->t('Signature has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, + // TRANSLATORS: Generic fallback status for unexpected signature validation failures. 'label' => $this->l10n->t('Unknown validation failure.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -197,41 +202,48 @@ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ 'id' => 1, + // TRANSLATORS: User-facing status when certificate is trusted. 'label' => $this->l10n->t('Certificate is trusted.'), 'isValid' => true, ], ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, + // TRANSLATORS: User-facing status when issuing CA is known but not trusted. 'label' => $this->l10n->t("Certificate issuer isn't trusted."), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, + // TRANSLATORS: User-facing status when certificate issuer cannot be identified/trusted. 'label' => $this->l10n->t('Certificate issuer is unknown.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, + // TRANSLATORS: User-facing status when certificate is revoked. 'label' => $this->l10n->t('Certificate has been revoked.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, + // TRANSLATORS: User-facing status when certificate is expired. 'label' => $this->l10n->t('Certificate has expired.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, + // TRANSLATORS: User-facing status when certificate validation could not be completed. 'label' => $this->l10n->t('Certificate has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, + // TRANSLATORS: Generic fallback status for unexpected certificate validation failures. 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -260,28 +272,46 @@ private function translateKnownReason(?string $reason): ?string { return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); } - // TRANSLATORS: Technical validation reasons from pdf-signature-validator. - // Keep protocol/acronym terms like ByteRange, CRL and CA as-is. return match ($reason) { + // TRANSLATORS: Technical term from PDF signatures. Keep "ByteRange" unchanged. 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), + // TRANSLATORS: Technical message for digest/hash mismatch in PDF signature verification. 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), + // TRANSLATORS: Certificate/public-key verification failed for signature bytes. 'Signature does not match certificate' => $this->l10n->t('Signature does not match certificate'), + // TRANSLATORS: X.509 certificate parsing failure. 'Failed to parse certificate' => $this->l10n->t('Failed to parse certificate'), + // TRANSLATORS: Signature timestamp is outside certificate validity window. 'Certificate was not valid at time of signature' => $this->l10n->t('Certificate was not valid at time of signature'), + // TRANSLATORS: Certificate validity date has ended. 'Certificate has expired' => $this->l10n->t('Certificate has expired'), + // TRANSLATORS: No certificates were found in provided certificate chain. 'Empty certificate chain' => $this->l10n->t('Empty certificate chain'), + // TRANSLATORS: Certificate does not provide a serial number field. 'Certificate has no serial number' => $this->l10n->t('Certificate has no serial number'), + // TRANSLATORS: CRL means Certificate Revocation List; keep acronym CRL unchanged. 'Certificate found in CRL' => $this->l10n->t('Certificate found in CRL'), + // TRANSLATORS: Certificate structure/content is invalid. 'Invalid certificate' => $this->l10n->t('Invalid certificate'), + // TRANSLATORS: CA means Certificate Authority; keep acronym CA unchanged. 'Leaf certificate is marked as CA' => $this->l10n->t('Leaf certificate is marked as CA'), + // TRANSLATORS: Certificate signature chain validation failed. 'Certificate signature validation failed' => $this->l10n->t('Certificate signature validation failed'), + // TRANSLATORS: Self-signed certificate is not present in trusted roots list. 'Self-signed certificate not in trusted roots' => $this->l10n->t('Self-signed certificate not in trusted roots'), + // TRANSLATORS: Root certificate must be self-signed to be considered a trust anchor. 'Root certificate is not self-signed' => $this->l10n->t('Root certificate is not self-signed'), + // TRANSLATORS: Root certificate is not present in configured trusted certificate list. 'Root certificate is not in trusted list' => $this->l10n->t('Root certificate is not in trusted list'), + // TRANSLATORS: Signature container has no binary signature payload. 'No binary signature' => $this->l10n->t('No binary signature'), + // TRANSLATORS: Signature payload has no embedded certificates. 'No certificates in signature' => $this->l10n->t('No certificates in signature'), + // TRANSLATORS: Certificate used for signing is expired. 'Signing certificate has expired' => $this->l10n->t('Signing certificate has expired'), + // TRANSLATORS: Certificate used for signing is revoked. 'Signing certificate has been revoked' => $this->l10n->t('Signing certificate has been revoked'), + // TRANSLATORS: Signature verification could not be fully completed. 'Signature verification incomplete' => $this->l10n->t('Signature verification incomplete'), default => $reason, }; From 74313b8cee58748c359c05a6b9fae1246b8c5dbf Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:07:42 -0300 Subject: [PATCH 07/36] docs: place translator comments above each l10n call Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index b92b4f072c..93bf9cff87 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -160,34 +160,34 @@ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ 'id' => 1, - // TRANSLATORS: User-facing status when signature cryptographic validation succeeds. + // TRANSLATORS User-facing status when signature cryptographic validation succeeds. 'label' => $this->l10n->t('Signature is valid.'), 'isValid' => true, ], ValidationState::SIGNATURE_INVALID => [ 'id' => 2, - // TRANSLATORS: User-facing status when signature cryptographic validation fails. + // TRANSLATORS User-facing status when signature cryptographic validation fails. 'label' => $this->l10n->t('Signature is invalid.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, - // TRANSLATORS: User-facing status when signed digest does not match PDF content. + // TRANSLATORS User-facing status when signed digest does not match PDF content. 'label' => $this->l10n->t('Digest mismatch.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, - // TRANSLATORS: User-facing status when validation could not be fully completed. + // TRANSLATORS User-facing status when validation could not be fully completed. 'label' => $this->l10n->t('Signature has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, - // TRANSLATORS: Generic fallback status for unexpected signature validation failures. + // TRANSLATORS Generic fallback status for unexpected signature validation failures. 'label' => $this->l10n->t('Unknown validation failure.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -202,48 +202,48 @@ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ 'id' => 1, - // TRANSLATORS: User-facing status when certificate is trusted. + // TRANSLATORS User-facing status when certificate is trusted. 'label' => $this->l10n->t('Certificate is trusted.'), 'isValid' => true, ], ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, - // TRANSLATORS: User-facing status when issuing CA is known but not trusted. + // TRANSLATORS User-facing status when issuing CA is known but not trusted. 'label' => $this->l10n->t("Certificate issuer isn't trusted."), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, - // TRANSLATORS: User-facing status when certificate issuer cannot be identified/trusted. + // TRANSLATORS User-facing status when certificate issuer cannot be identified/trusted. 'label' => $this->l10n->t('Certificate issuer is unknown.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, - // TRANSLATORS: User-facing status when certificate is revoked. + // TRANSLATORS User-facing status when certificate is revoked. 'label' => $this->l10n->t('Certificate has been revoked.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, - // TRANSLATORS: User-facing status when certificate is expired. + // TRANSLATORS User-facing status when certificate is expired. 'label' => $this->l10n->t('Certificate has expired.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, - // TRANSLATORS: User-facing status when certificate validation could not be completed. + // TRANSLATORS User-facing status when certificate validation could not be completed. 'label' => $this->l10n->t('Certificate has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, - // TRANSLATORS: Generic fallback status for unexpected certificate validation failures. + // TRANSLATORS Generic fallback status for unexpected certificate validation failures. 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -257,7 +257,7 @@ private function translateKnownReason(?string $reason): ?string { } if (preg_match('/^Intermediate certificate at position (\d+) is not signed by issuer$/', $reason, $matches) === 1) { - // TRANSLATORS: %s is the numeric position of an intermediate certificate in the chain. + // TRANSLATORS %s is the numeric position of an intermediate certificate in the chain. return $this->l10n->t( 'Intermediate certificate at position %s is not signed by issuer', [$matches[1]] @@ -268,50 +268,50 @@ private function translateKnownReason(?string $reason): ?string { if (str_starts_with($reason, $prefix)) { $detail = substr($reason, strlen($prefix)); $translatedDetail = $this->translateKnownReason($detail) ?? $detail; - // TRANSLATORS: %s is a translated certificate validation detail message. + // TRANSLATORS %s is a translated certificate validation detail message. return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); } return match ($reason) { - // TRANSLATORS: Technical term from PDF signatures. Keep "ByteRange" unchanged. + // TRANSLATORS Technical term from PDF signatures. Keep "ByteRange" unchanged. 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), - // TRANSLATORS: Technical message for digest/hash mismatch in PDF signature verification. + // TRANSLATORS Technical message for digest/hash mismatch in PDF signature verification. 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), - // TRANSLATORS: Certificate/public-key verification failed for signature bytes. + // TRANSLATORS Certificate/public-key verification failed for signature bytes. 'Signature does not match certificate' => $this->l10n->t('Signature does not match certificate'), - // TRANSLATORS: X.509 certificate parsing failure. + // TRANSLATORS X.509 certificate parsing failure. 'Failed to parse certificate' => $this->l10n->t('Failed to parse certificate'), - // TRANSLATORS: Signature timestamp is outside certificate validity window. + // TRANSLATORS Signature timestamp is outside certificate validity window. 'Certificate was not valid at time of signature' => $this->l10n->t('Certificate was not valid at time of signature'), - // TRANSLATORS: Certificate validity date has ended. + // TRANSLATORS Certificate validity date has ended. 'Certificate has expired' => $this->l10n->t('Certificate has expired'), - // TRANSLATORS: No certificates were found in provided certificate chain. + // TRANSLATORS No certificates were found in provided certificate chain. 'Empty certificate chain' => $this->l10n->t('Empty certificate chain'), - // TRANSLATORS: Certificate does not provide a serial number field. + // TRANSLATORS Certificate does not provide a serial number field. 'Certificate has no serial number' => $this->l10n->t('Certificate has no serial number'), - // TRANSLATORS: CRL means Certificate Revocation List; keep acronym CRL unchanged. + // TRANSLATORS CRL means Certificate Revocation List; keep acronym CRL unchanged. 'Certificate found in CRL' => $this->l10n->t('Certificate found in CRL'), - // TRANSLATORS: Certificate structure/content is invalid. + // TRANSLATORS Certificate structure/content is invalid. 'Invalid certificate' => $this->l10n->t('Invalid certificate'), - // TRANSLATORS: CA means Certificate Authority; keep acronym CA unchanged. + // TRANSLATORS CA means Certificate Authority; keep acronym CA unchanged. 'Leaf certificate is marked as CA' => $this->l10n->t('Leaf certificate is marked as CA'), - // TRANSLATORS: Certificate signature chain validation failed. + // TRANSLATORS Certificate signature chain validation failed. 'Certificate signature validation failed' => $this->l10n->t('Certificate signature validation failed'), - // TRANSLATORS: Self-signed certificate is not present in trusted roots list. + // TRANSLATORS Self-signed certificate is not present in trusted roots list. 'Self-signed certificate not in trusted roots' => $this->l10n->t('Self-signed certificate not in trusted roots'), - // TRANSLATORS: Root certificate must be self-signed to be considered a trust anchor. + // TRANSLATORS Root certificate must be self-signed to be considered a trust anchor. 'Root certificate is not self-signed' => $this->l10n->t('Root certificate is not self-signed'), - // TRANSLATORS: Root certificate is not present in configured trusted certificate list. + // TRANSLATORS Root certificate is not present in configured trusted certificate list. 'Root certificate is not in trusted list' => $this->l10n->t('Root certificate is not in trusted list'), - // TRANSLATORS: Signature container has no binary signature payload. + // TRANSLATORS Signature container has no binary signature payload. 'No binary signature' => $this->l10n->t('No binary signature'), - // TRANSLATORS: Signature payload has no embedded certificates. + // TRANSLATORS Signature payload has no embedded certificates. 'No certificates in signature' => $this->l10n->t('No certificates in signature'), - // TRANSLATORS: Certificate used for signing is expired. + // TRANSLATORS Certificate used for signing is expired. 'Signing certificate has expired' => $this->l10n->t('Signing certificate has expired'), - // TRANSLATORS: Certificate used for signing is revoked. + // TRANSLATORS Certificate used for signing is revoked. 'Signing certificate has been revoked' => $this->l10n->t('Signing certificate has been revoked'), - // TRANSLATORS: Signature verification could not be fully completed. + // TRANSLATORS Signature verification could not be fully completed. 'Signature verification incomplete' => $this->l10n->t('Signature verification incomplete'), default => $reason, }; From 0b3c68c040c9087957a7b51588f044528772be3e Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:08:59 -0300 Subject: [PATCH 08/36] refactor: remove redundant docs from CA getters Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 93bf9cff87..efe4bd8f44 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -317,16 +317,10 @@ private function translateKnownReason(?string $reason): ?string { }; } - /** - * Check if LibreSign CA is loaded. - */ public function isLibreSignCaLoaded(): bool { return !empty($this->libresignCaCertificate); } - /** - * Get LibreSign CA certificate (PEM format). - */ public function getLibreSignCaCertificate(): string { return $this->libresignCaCertificate; } From ffa0f69f188608b05d8d998b0140fe0ab83b9b5c Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:09:45 -0300 Subject: [PATCH 09/36] refactor: remove redundant comments in validation service Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index efe4bd8f44..1cbe587885 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -37,11 +37,7 @@ public function __construct( $this->loadLibreSignCaCertificate(); } - /** - * Load LibreSign CA certificate and set it as trusted root. - */ private function loadLibreSignCaCertificate(): void { - // Try to load from config path first $configPath = $this->appConfig->getValueString(Application::APP_ID, 'config_path'); if (!empty($configPath) && is_dir($configPath)) { $caPemPath = $configPath . DIRECTORY_SEPARATOR . 'ca.pem'; @@ -55,7 +51,6 @@ private function loadLibreSignCaCertificate(): void { } } - // Try alternate location $alternateConfig = $this->appConfig->getValueString( Application::APP_ID, 'libresign_ca_certificate' @@ -66,18 +61,10 @@ private function loadLibreSignCaCertificate(): void { } } - /** - * Add additional trusted root certificate. - */ public function addTrustedRoot(string $certificatePem): void { $this->validator->addTrustedRoot($certificatePem); } - /** - * Set multiple trusted root certificates. - * - * @param list $certificates - */ public function setTrustedRoots(array $certificates): void { $this->validator->setTrustedRoots($certificates); } @@ -153,9 +140,6 @@ private function mapValidationResults(array $results, ?DateTime $signatureTime = return $mapped; } - /** - * Map signature validation result to LibreSign format. - */ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ @@ -195,9 +179,6 @@ private function mapSignatureValidation(ValidationResult $result): array { }; } - /** - * Map certificate validation result to LibreSign format. - */ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ From 390e453f1174932ec8d5c71daa517d46df79b673 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:12:59 -0300 Subject: [PATCH 10/36] test: remove deprecated reflection accessibility calls Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Service/Signature/PdfSignatureValidationServiceTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index cdc30d5a10..6101860832 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -81,7 +81,6 @@ private function newServiceWithoutConstructor(): PdfSignatureValidationService { $service = $reflection->newInstanceWithoutConstructor(); $l10nProperty = $reflection->getProperty('l10n'); - $l10nProperty->setAccessible(true); $l10nProperty->setValue($service, $this->l10n); return $service; @@ -93,7 +92,6 @@ private function newServiceWithoutConstructor(): PdfSignatureValidationService { private function invokePrivateMethod(PdfSignatureValidationService $service, string $method, ValidationResult $result): array { $reflection = new \ReflectionClass($service); $target = $reflection->getMethod($method); - $target->setAccessible(true); /** @var array $mapped */ $mapped = $target->invoke($service, $result); return $mapped; From 37198471eabffb548b18ca5ae4839ef9fb71d6a8 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:15:12 -0300 Subject: [PATCH 11/36] build: use released pdf-signature-validator v0.2.0 Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- composer.json | 9 --------- 1 file changed, 9 deletions(-) diff --git a/composer.json b/composer.json index 70eb02933d..c40bf03912 100644 --- a/composer.json +++ b/composer.json @@ -1,13 +1,4 @@ { - "repositories": [ - { - "type": "path", - "url": "../pdf-signature-validator", - "options": { - "symlink": true - } - } - ], "require-dev": { "bamarni/composer-bin-plugin": "^1.8", "nextcloud/ocp": "dev-stable32", From 3a3b4fbd3d585e2de95e11a7e1553cb800e13fcd Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:19:28 -0300 Subject: [PATCH 12/36] refactor: centralize validation localization mapping Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 72 +------------------ .../PdfSignatureValidationService.php | 42 +++++++++++ 2 files changed, 44 insertions(+), 70 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index ea7a4aa85a..5071f5ac04 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -292,11 +292,11 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array } if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { - $leaf['signature_validation'] = $this->getLocalizedSignatureValidation($validation['signatureValidation']); + $leaf['signature_validation'] = $this->pdfSignatureValidationService->localizeSignatureValidation($validation['signatureValidation']); } if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { - $leaf['certificate_validation'] = $this->getLocalizedCertificateValidation($validation['certificateValidation']); + $leaf['certificate_validation'] = $this->pdfSignatureValidationService->localizeCertificateValidation($validation['certificateValidation']); } if (!isset($leaf['certificate_validation'])) { @@ -309,74 +309,6 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array return $result; } - /** - * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. - */ - private function getLocalizedSignatureValidation(array $validation): array { - $id = (int)($validation['id'] ?? 6); - - return match ($id) { - 1 => [ - 'id' => 1, - 'label' => $this->l10n->t('Signature is valid.'), - ], - 2 => [ - 'id' => 2, - 'label' => $this->l10n->t('Signature is invalid.'), - ], - 3 => [ - 'id' => 3, - 'label' => $this->l10n->t('Digest mismatch.'), - ], - 5 => [ - 'id' => 5, - 'label' => $this->l10n->t('Signature has not yet been verified.'), - ], - default => [ - 'id' => 6, - 'label' => $this->l10n->t('Unknown validation failure.'), - ], - }; - } - - /** - * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. - */ - private function getLocalizedCertificateValidation(array $validation): array { - $id = (int)($validation['id'] ?? 7); - - return match ($id) { - 1 => [ - 'id' => 1, - 'label' => $this->l10n->t('Certificate is trusted.'), - ], - 2 => [ - 'id' => 2, - 'label' => $this->l10n->t("Certificate issuer isn't trusted."), - ], - 3 => [ - 'id' => 3, - 'label' => $this->l10n->t('Certificate issuer is unknown.'), - ], - 4 => [ - 'id' => 4, - 'label' => $this->l10n->t('Certificate has been revoked.'), - ], - 5 => [ - 'id' => 5, - 'label' => $this->l10n->t('Certificate has expired'), - ], - 6 => [ - 'id' => 6, - 'label' => $this->l10n->t('Certificate has not yet been verified.'), - ], - default => [ - 'id' => 7, - 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), - ], - }; - } - /** * @param resource $resource * @return list diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 1cbe587885..74a4ab8c17 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -69,6 +69,48 @@ public function setTrustedRoots(array $certificates): void { $this->validator->setTrustedRoots($certificates); } + /** + * Normalize a signature validation payload by id/reason to the canonical LibreSign shape. + * + * @param array{id?: int|string, reason?: mixed} $validation + */ + public function localizeSignatureValidation(array $validation): array { + $id = (int)($validation['id'] ?? 6); + $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; + + $state = match ($id) { + 1 => ValidationState::SIGNATURE_VALID, + 2 => ValidationState::SIGNATURE_INVALID, + 3 => ValidationState::DIGEST_MISMATCH, + 5 => ValidationState::NOT_VERIFIED, + default => ValidationState::UNKNOWN_FAILURE, + }; + + return $this->mapSignatureValidation(new ValidationResult($state, $reason)); + } + + /** + * Normalize a certificate validation payload by id/reason to the canonical LibreSign shape. + * + * @param array{id?: int|string, reason?: mixed} $validation + */ + public function localizeCertificateValidation(array $validation): array { + $id = (int)($validation['id'] ?? 7); + $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; + + $state = match ($id) { + 1 => ValidationState::CERT_TRUSTED, + 2 => ValidationState::CERT_ISSUER_NOT_TRUSTED, + 3 => ValidationState::CERT_ISSUER_UNKNOWN, + 4 => ValidationState::CERT_REVOKED, + 5 => ValidationState::CERT_EXPIRED, + 6 => ValidationState::CERT_NOT_VERIFIED, + default => ValidationState::UNKNOWN_FAILURE, + }; + + return $this->mapCertificateValidation(new ValidationResult($state, $reason)); + } + /** * Validate PDF signatures from file resource. * From c5e1af86018fa4c6b51000ab4e8c89821e499387 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:21:15 -0300 Subject: [PATCH 13/36] fix: remove invalid UNKNOWN_FAILURE enum state Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 74a4ab8c17..a929c56f03 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -83,7 +83,7 @@ public function localizeSignatureValidation(array $validation): array { 2 => ValidationState::SIGNATURE_INVALID, 3 => ValidationState::DIGEST_MISMATCH, 5 => ValidationState::NOT_VERIFIED, - default => ValidationState::UNKNOWN_FAILURE, + default => ValidationState::NOT_VERIFIED, }; return $this->mapSignatureValidation(new ValidationResult($state, $reason)); @@ -105,7 +105,7 @@ public function localizeCertificateValidation(array $validation): array { 4 => ValidationState::CERT_REVOKED, 5 => ValidationState::CERT_EXPIRED, 6 => ValidationState::CERT_NOT_VERIFIED, - default => ValidationState::UNKNOWN_FAILURE, + default => ValidationState::CERT_NOT_VERIFIED, }; return $this->mapCertificateValidation(new ValidationResult($state, $reason)); From 96cb2538a008ae8e6ba4d23498fc266a045f452c Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:24:44 -0300 Subject: [PATCH 14/36] refactor: remove redundant localize* methods The service's validateFromResource() already returns properly formatted arrays with id, label, isValid, and reason. The localize* methods were unnecessarily converting numbers back to enums and then back to numbers. Now Pkcs12Handler uses the validation results directly without the circular conversion, eliminating code duplication and improving clarity. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 4 +- .../PdfSignatureValidationService.php | 42 ------------------- 2 files changed, 2 insertions(+), 44 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 5071f5ac04..3acb83402e 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -292,11 +292,11 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array } if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { - $leaf['signature_validation'] = $this->pdfSignatureValidationService->localizeSignatureValidation($validation['signatureValidation']); + $leaf['signature_validation'] = $validation['signatureValidation']; } if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { - $leaf['certificate_validation'] = $this->pdfSignatureValidationService->localizeCertificateValidation($validation['certificateValidation']); + $leaf['certificate_validation'] = $validation['certificateValidation']; } if (!isset($leaf['certificate_validation'])) { diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index a929c56f03..1cbe587885 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -69,48 +69,6 @@ public function setTrustedRoots(array $certificates): void { $this->validator->setTrustedRoots($certificates); } - /** - * Normalize a signature validation payload by id/reason to the canonical LibreSign shape. - * - * @param array{id?: int|string, reason?: mixed} $validation - */ - public function localizeSignatureValidation(array $validation): array { - $id = (int)($validation['id'] ?? 6); - $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; - - $state = match ($id) { - 1 => ValidationState::SIGNATURE_VALID, - 2 => ValidationState::SIGNATURE_INVALID, - 3 => ValidationState::DIGEST_MISMATCH, - 5 => ValidationState::NOT_VERIFIED, - default => ValidationState::NOT_VERIFIED, - }; - - return $this->mapSignatureValidation(new ValidationResult($state, $reason)); - } - - /** - * Normalize a certificate validation payload by id/reason to the canonical LibreSign shape. - * - * @param array{id?: int|string, reason?: mixed} $validation - */ - public function localizeCertificateValidation(array $validation): array { - $id = (int)($validation['id'] ?? 7); - $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; - - $state = match ($id) { - 1 => ValidationState::CERT_TRUSTED, - 2 => ValidationState::CERT_ISSUER_NOT_TRUSTED, - 3 => ValidationState::CERT_ISSUER_UNKNOWN, - 4 => ValidationState::CERT_REVOKED, - 5 => ValidationState::CERT_EXPIRED, - 6 => ValidationState::CERT_NOT_VERIFIED, - default => ValidationState::CERT_NOT_VERIFIED, - }; - - return $this->mapCertificateValidation(new ValidationResult($state, $reason)); - } - /** * Validate PDF signatures from file resource. * From 31547c1520ffe55e11a78243befa15d06f578ac7 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:26:34 -0300 Subject: [PATCH 15/36] fix: cs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 1cbe587885..4bc9550391 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -9,9 +9,9 @@ namespace OCA\Libresign\Service\Signature; use DateTime; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; use LibreSign\PdfSignatureValidator\Model\ValidationResult; use LibreSign\PdfSignatureValidator\Model\ValidationState; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; use OCA\Libresign\AppInfo\Application; use OCP\IAppConfig; use OCP\IL10N; From d58348c69315474a361710146ad6e594525f950f Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:30:24 -0300 Subject: [PATCH 16/36] fix: resolve Psalm type hint errors - FileController::fetchPreview: add inline type assertions for RedirectResponse to specify exact status code (303) when returning null-checked values - PdfSignatureValidationService: update docblocks for validateFromResource, validateFromString, and mapValidationResults to include the 'raw' field - Pkcs12Handler::extractNativeSignatureMetadata: change return type from list to array to match actual implementation All Psalm errors resolved, unit tests still passing. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Controller/FileController.php | 45 ++++++++++++++++--- lib/Handler/SignEngine/Pkcs12Handler.php | 2 +- .../PdfSignatureValidationService.php | 6 +-- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/lib/Controller/FileController.php b/lib/Controller/FileController.php index bac8b005a1..c30d0869df 100644 --- a/lib/Controller/FileController.php +++ b/lib/Controller/FileController.php @@ -457,7 +457,7 @@ private function fetchPreview( bool $forceIcon, string $mode, bool $mimeFallback = false, - ) : Http\Response { + ): Http\Response { if (!($node instanceof File) || (!$forceIcon && !$this->preview->isAvailable($node))) { return new DataResponse([], Http::STATUS_NOT_FOUND); } @@ -465,6 +465,15 @@ private function fetchPreview( return new DataResponse([], Http::STATUS_FORBIDDEN); } + // Avoid expensive external preview generators for PDFs when a mime fallback is explicitly requested. + if ($mimeFallback && $node->getMimeType() === 'application/pdf') { + $mimeFallbackResponse = $this->getMimeFallbackResponse($node->getMimeType()); + if ($mimeFallbackResponse !== null) { + /** @var Http\RedirectResponse $mimeFallbackResponse */ + return $mimeFallbackResponse; + } + } + $storage = $node->getStorage(); if ($storage->instanceOfStorage(SharedStorage::class)) { /** @var SharedStorage $storage */ @@ -483,19 +492,43 @@ private function fetchPreview( $response->cacheFor(3600 * 24, false, true); return $response; } catch (NotFoundException) { - // If we have no preview enabled, we can redirect to the mime icon if any - if ($mimeFallback) { - if ($url = $this->mimeIconProvider->getMimeIconUrl($node->getMimeType())) { - return new RedirectResponse($url); - } + $mimeFallbackResponse = $mimeFallback ? $this->getMimeFallbackResponse($node->getMimeType()) : null; + if ($mimeFallbackResponse !== null) { + /** @var Http\RedirectResponse $mimeFallbackResponse */ + return $mimeFallbackResponse; } return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\InvalidArgumentException) { return new DataResponse([], Http::STATUS_BAD_REQUEST); + } catch (\Throwable $e) { + $this->logger->warning('Failed to generate LibreSign thumbnail preview', [ + 'nodeId' => $node->getId(), + 'mimeType' => $node->getMimeType(), + 'exception' => $e, + ]); + + $mimeFallbackResponse = $mimeFallback ? $this->getMimeFallbackResponse($node->getMimeType()) : null; + if ($mimeFallbackResponse !== null) { + /** @var Http\RedirectResponse $mimeFallbackResponse */ + return $mimeFallbackResponse; + } + + return new DataResponse([], Http::STATUS_NOT_FOUND); } } + private function getMimeFallbackResponse(string $mimeType): ?\OCP\AppFramework\Http\RedirectResponse { + $url = $this->mimeIconProvider->getMimeIconUrl($mimeType); + if ($url) { + /** @var \OCP\AppFramework\Http\RedirectResponse $response */ + $response = new RedirectResponse($url, Http::STATUS_SEE_OTHER); + return $response; + } + + return null; + } + /** * Send a file * diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 3acb83402e..53b2e2e666 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -311,7 +311,7 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array /** * @param resource $resource - * @return list + * @return array */ private function extractNativeSignatureMetadata($resource): array { rewind($resource); diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 4bc9550391..8e13f3b07a 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -74,7 +74,7 @@ public function setTrustedRoots(array $certificates): void { * * @param resource $resource PDF file resource * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) - * @return list + * @return list */ public function validateFromResource($resource, ?DateTime $signatureTime = null): array { try { @@ -94,7 +94,7 @@ public function validateFromResource($resource, ?DateTime $signatureTime = null) * * @param string $pdfContent Binary PDF content * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) - * @return list + * @return list */ public function validateFromString(string $pdfContent, ?DateTime $signatureTime = null): array { try { @@ -114,7 +114,7 @@ public function validateFromString(string $pdfContent, ?DateTime $signatureTime * * @param list $results Results from PdfSignatureValidator * @param ?\DateTime $signatureTime - * @return list + * @return list */ private function mapValidationResults(array $results, ?DateTime $signatureTime = null): array { $mapped = []; From e6c5f4fdf2d1744de5119125f0e9186c901a3e5f Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:36:45 -0300 Subject: [PATCH 17/36] fix: update PasswordTest constructor arguments for Pkcs12Handler Update constructor arguments to match the new signature which includes PdfSignatureValidationService and PdfSignatureExtractor, and removes the no-longer-used TempManager. Also use real instance of PdfSignatureExtractor instead of mock since the class is final and cannot be mocked. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../php/Unit/Service/IdentifyMethod/PasswordTest.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php index e2a27beb66..7092c54e64 100644 --- a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php +++ b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php @@ -21,9 +21,10 @@ use OCA\Libresign\Service\FolderService; use OCA\Libresign\Service\IdentifyMethod\IdentifyService; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\Password; +use OCA\Libresign\Service\Signature\PdfSignatureValidationService; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCP\IAppConfig; use OCP\IL10N; -use OCP\ITempManager; use OCP\IUserSession; use OCP\L10N\IFactory as IL10NFactory; use PHPUnit\Framework\Attributes\DataProvider; @@ -39,11 +40,12 @@ final class PasswordTest extends \OCA\Libresign\Tests\Unit\TestCase { private CertificateEngineFactory&MockObject $certificateEngineFactory; private IL10N $l10n; private FooterHandler&MockObject $footerHandler; - private ITempManager $tempManager; private LoggerInterface&MockObject $logger; private CaIdentifierService&MockObject $caIdentifierService; private DocMdpHandler&MockObject $docMdpHandler; private CrlService&MockObject $crlService; + private PdfSignatureValidationService&MockObject $pdfSignatureValidationService; + private PdfSignatureExtractor $pdfSignatureExtractor; public function setUp(): void { $this->identifyService = $this->createMock(IdentifyService::class); @@ -52,12 +54,13 @@ public function setUp(): void { $this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); $this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID); $this->footerHandler = $this->createMock(FooterHandler::class); - $this->tempManager = \OCP\Server::get(ITempManager::class); $this->userSession = $this->createMock(IUserSession::class); $this->logger = $this->createMock(LoggerInterface::class); $this->caIdentifierService = $this->createMock(CaIdentifierService::class); $this->docMdpHandler = $this->createMock(DocMdpHandler::class); $this->crlService = $this->createMock(CrlService::class); + $this->pdfSignatureValidationService = $this->createMock(PdfSignatureValidationService::class); + $this->pdfSignatureExtractor = new PdfSignatureExtractor(); $this->pkcs12Handler = $this->getPkcs12Instance(); } @@ -80,11 +83,12 @@ private function getPkcs12Instance(array $methods = []) { $this->certificateEngineFactory, $this->l10n, $this->footerHandler, - $this->tempManager, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, + $this->pdfSignatureValidationService, + $this->pdfSignatureExtractor, ]) ->onlyMethods($methods) ->getMock(); From 33324273b10ea55dcbd3b2fc6c3e08fcc16e721a Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:38:12 -0300 Subject: [PATCH 18/36] fix: cs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- tests/php/Unit/Service/IdentifyMethod/PasswordTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php index 7092c54e64..752b20ecc6 100644 --- a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php +++ b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php @@ -8,6 +8,7 @@ namespace OCA\Libresign\Tests\Unit\Service\IdentifyMethod; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Enum\CrlValidationStatus; use OCA\Libresign\Exception\InvalidPasswordException; @@ -22,7 +23,6 @@ use OCA\Libresign\Service\IdentifyMethod\IdentifyService; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\Password; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCP\IAppConfig; use OCP\IL10N; use OCP\IUserSession; From a42f0eb9e4ad385e83237c5357c9c81eea45e410 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:08:43 -0300 Subject: [PATCH 19/36] fix: preserve legacy signature validation on native digest mismatch Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 16 ++++++++++++- .../Handler/SignEngine/Pkcs12HandlerTest.php | 24 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 53b2e2e666..e02ec16082 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -292,7 +292,12 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array } if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { - $leaf['signature_validation'] = $validation['signatureValidation']; + $signatureValidation = $validation['signatureValidation']; + + // Keep legacy OpenSSL result when native validator reports this known false-positive. + if (!$this->isDigestMismatchSignatureValidation($signatureValidation)) { + $leaf['signature_validation'] = $signatureValidation; + } } if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { @@ -309,6 +314,15 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array return $result; } + /** + * signer engines can produce signatures that the native validator currently flags as digest mismatch. + * In this case we preserve the legacy validation computed from the PKCS#7 signature. + */ + private function isDigestMismatchSignatureValidation(array $signatureValidation): bool { + return ($signatureValidation['id'] ?? null) === 3 + && ($signatureValidation['label'] ?? '') === 'Digest mismatch.'; + } + /** * @param resource $resource * @return array diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index 1864525ab6..a57d20abf0 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -443,4 +443,28 @@ private function normalizeDistinguishedName(mixed $distinguishedName): array { return $distinguishedName; } + public function testGetCertificateChainDoesNotOverrideLegacySignatureValidationOnDigestMismatch(): void { + $this->pdfSignatureValidationService->method('validateFromResource') + ->willReturn([ + [ + 'signatureValidation' => [ + 'id' => 3, + 'label' => 'Digest mismatch.', + 'reason' => 'PDF content hash does not match signed digest', + ], + ], + ]); + + $handler = $this->getHandler(); + $resource = fopen(__DIR__ . '/../../../fixtures/pdfs/small_valid-signed.pdf', 'r'); + $this->assertIsResource($resource); + + $result = $handler->getCertificateChain($resource); + fclose($resource); + + $this->assertNotEmpty($result); + $this->assertSame(1, $result[0]['chain'][0]['signature_validation']['id']); + $this->assertSame('Signature is valid.', $result[0]['chain'][0]['signature_validation']['label']); + } + } From eabc640cca65d22e366a5c05d7f4319116c39e4e Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Fri, 24 Apr 2026 11:36:13 -0300 Subject: [PATCH 20/36] fix: address native validator review feedback Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 17 +++++++++++++---- .../Signature/PdfSignatureValidationService.php | 14 +++++--------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index e02ec16082..f45807e40d 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -9,6 +9,9 @@ namespace OCA\Libresign\Handler\SignEngine; use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; +use LibreSign\PdfSignatureValidator\Model\ValidationReason; +use LibreSign\PdfSignatureValidator\Model\ValidationResult; +use LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; @@ -295,7 +298,7 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array $signatureValidation = $validation['signatureValidation']; // Keep legacy OpenSSL result when native validator reports this known false-positive. - if (!$this->isDigestMismatchSignatureValidation($signatureValidation)) { + if (!$this->isDigestMismatchSignatureValidation($validation)) { $leaf['signature_validation'] = $signatureValidation; } } @@ -318,9 +321,15 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array * signer engines can produce signatures that the native validator currently flags as digest mismatch. * In this case we preserve the legacy validation computed from the PKCS#7 signature. */ - private function isDigestMismatchSignatureValidation(array $signatureValidation): bool { - return ($signatureValidation['id'] ?? null) === 3 - && ($signatureValidation['label'] ?? '') === 'Digest mismatch.'; + private function isDigestMismatchSignatureValidation(array $validation): bool { + $rawSignatureValidation = $validation['raw']['signature'] ?? null; + if ($rawSignatureValidation instanceof ValidationResult) { + return $rawSignatureValidation->reasonCode === ValidationReason::DIGEST_MISMATCH + || $rawSignatureValidation->state === ValidationState::DIGEST_MISMATCH; + } + + $signatureValidation = $validation['signatureValidation'] ?? null; + return is_array($signatureValidation) && ($signatureValidation['id'] ?? null) === 3; } /** diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 8e13f3b07a..ddabf3c65c 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -8,7 +8,6 @@ namespace OCA\Libresign\Service\Signature; -use DateTime; use LibreSign\PdfSignatureValidator\Model\ValidationResult; use LibreSign\PdfSignatureValidator\Model\ValidationState; use LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; @@ -73,13 +72,12 @@ public function setTrustedRoots(array $certificates): void { * Validate PDF signatures from file resource. * * @param resource $resource PDF file resource - * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) * @return list */ - public function validateFromResource($resource, ?DateTime $signatureTime = null): array { + public function validateFromResource($resource): array { try { $results = $this->validator->validateFromResource($resource); - return $this->mapValidationResults($results, $signatureTime); + return $this->mapValidationResults($results); } catch (\Throwable $e) { $this->logger->warning('PDF signature validation failed', [ 'error' => $e->getMessage(), @@ -93,13 +91,12 @@ public function validateFromResource($resource, ?DateTime $signatureTime = null) * Validate PDF signatures from binary content. * * @param string $pdfContent Binary PDF content - * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) * @return list */ - public function validateFromString(string $pdfContent, ?DateTime $signatureTime = null): array { + public function validateFromString(string $pdfContent): array { try { $results = $this->validator->validateFromString($pdfContent); - return $this->mapValidationResults($results, $signatureTime); + return $this->mapValidationResults($results); } catch (\Throwable $e) { $this->logger->warning('PDF signature validation failed', [ 'error' => $e->getMessage(), @@ -113,10 +110,9 @@ public function validateFromString(string $pdfContent, ?DateTime $signatureTime * Map validation results from PdfSignatureValidator to LibreSign format. * * @param list $results Results from PdfSignatureValidator - * @param ?\DateTime $signatureTime * @return list */ - private function mapValidationResults(array $results, ?DateTime $signatureTime = null): array { + private function mapValidationResults(array $results): array { $mapped = []; foreach ($results as $result) { From 6afd97b8831cdb3718e0a3d9d35a73fe8cd25d18 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Fri, 24 Apr 2026 11:40:27 -0300 Subject: [PATCH 21/36] fix: cs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index f45807e40d..81d21364ff 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -8,10 +8,10 @@ namespace OCA\Libresign\Handler\SignEngine; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use LibreSign\PdfSignatureValidator\Model\ValidationReason; use LibreSign\PdfSignatureValidator\Model\ValidationResult; use LibreSign\PdfSignatureValidator\Model\ValidationState; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; From 61a0e723954fd4aae7ce16f2c12ac86bd4ecd0ce Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 18 Jun 2026 14:46:57 -0300 Subject: [PATCH 22/36] fix: remove phpseclib from direct deps, use provide via 3rdparty phpseclib is already vendored and scoped in the 3rdparty submodule under OCA\Libresign\Vendor\phpseclib3. Declaring it as a direct composer dependency caused roave/security-advisories to block the insecure 3.0.52 version locked in composer.lock. Remove phpseclib/phpseclib, paragonie/constant_time_encoding and paragonie/random_compat from composer.json require and composer.lock. Use "provide" to satisfy the transitive dependency declared by libresign/pdf-signature-validator without installing the package. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> From cba47bf9c6aa0cd4b9a811cd07d92f63fb97d524 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:13:55 -0300 Subject: [PATCH 23/36] refactor: move pdf-signature-validator to 3rdparty scoped vendor The libresign/pdf-signature-validator package is moved from libresign's own composer dependencies into the 3rdparty scoped vendor directory so that php-scoper can prefix both its classes and its phpseclib3 references with OCA\Libresign\Vendor\. - Remove libresign/pdf-signature-validator from composer.json + lock - Remove phpseclib/phpseclib 'provide' declaration (no longer needed) - Update all use statements: LibreSign\PdfSignatureValidator\... => OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\... - Update 3rdparty submodule ref to include the scoped package Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 8 ++++---- lib/Service/Signature/PdfSignatureValidationService.php | 6 +++--- tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php | 2 +- tests/php/Unit/Service/IdentifyMethod/PasswordTest.php | 2 +- .../Signature/PdfSignatureValidationServiceTest.php | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 81d21364ff..b52dad2987 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -8,10 +8,10 @@ namespace OCA\Libresign\Handler\SignEngine; -use LibreSign\PdfSignatureValidator\Model\ValidationReason; -use LibreSign\PdfSignatureValidator\Model\ValidationResult; -use LibreSign\PdfSignatureValidator\Model\ValidationState; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationReason; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index ddabf3c65c..60acdbaec3 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -8,9 +8,9 @@ namespace OCA\Libresign\Service\Signature; -use LibreSign\PdfSignatureValidator\Model\ValidationResult; -use LibreSign\PdfSignatureValidator\Model\ValidationState; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; use OCA\Libresign\AppInfo\Application; use OCP\IAppConfig; use OCP\IL10N; diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index a57d20abf0..f7edd1d48e 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -9,7 +9,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; use OCA\Libresign\Handler\CertificateEngine\IEngineHandler; diff --git a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php index 752b20ecc6..d507b4f802 100644 --- a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php +++ b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php @@ -8,7 +8,7 @@ namespace OCA\Libresign\Tests\Unit\Service\IdentifyMethod; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Enum\CrlValidationStatus; use OCA\Libresign\Exception\InvalidPasswordException; diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index 6101860832..ac4294dade 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -8,8 +8,8 @@ namespace OCA\Libresign\Tests\Unit\Service\Signature; -use LibreSign\PdfSignatureValidator\Model\ValidationResult; -use LibreSign\PdfSignatureValidator\Model\ValidationState; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Unit\TestCase; use OCP\IL10N; From 334f8418f9d0a75620282e78cc8dcf22739567a4 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:24:15 -0300 Subject: [PATCH 24/36] fix(cs): fix use statement ordering per php-cs-fixer Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 8 ++++---- lib/Service/Signature/PdfSignatureValidationService.php | 2 +- tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php | 2 +- tests/php/Unit/Service/IdentifyMethod/PasswordTest.php | 2 +- .../Signature/PdfSignatureValidationServiceTest.php | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index b52dad2987..52d692e955 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -8,10 +8,6 @@ namespace OCA\Libresign\Handler\SignEngine; -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationReason; -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; @@ -23,6 +19,10 @@ use OCA\Libresign\Service\Crl\CrlService; use OCA\Libresign\Service\FolderService; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationReason; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\Vendor\phpseclib3\File\ASN1; use OCP\Files\File; use OCP\IAppConfig; diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 60acdbaec3..817c75eeb8 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -8,10 +8,10 @@ namespace OCA\Libresign\Service\Signature; +use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; -use OCA\Libresign\AppInfo\Application; use OCP\IAppConfig; use OCP\IL10N; use Psr\Log\LoggerInterface; diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index f7edd1d48e..ce0e966c02 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -9,7 +9,6 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; use OCA\Libresign\Handler\CertificateEngine\IEngineHandler; @@ -21,6 +20,7 @@ use OCA\Libresign\Service\FolderService; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Fixtures\PdfFixtureCatalog; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\IAppConfig; diff --git a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php index d507b4f802..f8b1e837e3 100644 --- a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php +++ b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php @@ -8,7 +8,6 @@ namespace OCA\Libresign\Tests\Unit\Service\IdentifyMethod; -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Enum\CrlValidationStatus; use OCA\Libresign\Exception\InvalidPasswordException; @@ -23,6 +22,7 @@ use OCA\Libresign\Service\IdentifyMethod\IdentifyService; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\Password; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCP\IAppConfig; use OCP\IL10N; use OCP\IUserSession; diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index ac4294dade..1380eca9b2 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -8,10 +8,10 @@ namespace OCA\Libresign\Tests\Unit\Service\Signature; -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; -use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Unit\TestCase; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; use OCP\IL10N; use PHPUnit\Framework\MockObject\MockObject; From 435a7d479d2403d1ffb1918462c832d0f7bb0591 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:43:14 -0300 Subject: [PATCH 25/36] fix: propagate unexpected native validation exceptions Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 817c75eeb8..2a222deb4c 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -9,6 +9,7 @@ namespace OCA\Libresign\Service\Signature; use OCA\Libresign\AppInfo\Application; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Exception\UnsignedPdfException; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; @@ -76,15 +77,12 @@ public function setTrustedRoots(array $certificates): void { */ public function validateFromResource($resource): array { try { - $results = $this->validator->validateFromResource($resource); - return $this->mapValidationResults($results); - } catch (\Throwable $e) { - $this->logger->warning('PDF signature validation failed', [ - 'error' => $e->getMessage(), - 'trace' => $e->getTraceAsString(), - ]); + $results = $this->validateNativeFromResource($resource); + } catch (UnsignedPdfException) { return []; } + + return $this->mapValidationResults($results); } /** @@ -95,15 +93,27 @@ public function validateFromResource($resource): array { */ public function validateFromString(string $pdfContent): array { try { - $results = $this->validator->validateFromString($pdfContent); - return $this->mapValidationResults($results); - } catch (\Throwable $e) { - $this->logger->warning('PDF signature validation failed', [ - 'error' => $e->getMessage(), - 'trace' => $e->getTraceAsString(), - ]); + $results = $this->validateNativeFromString($pdfContent); + } catch (UnsignedPdfException) { return []; } + + return $this->mapValidationResults($results); + } + + /** + * @param resource $resource + * @return list, certificateValidation: ValidationResult}> + */ + protected function validateNativeFromResource($resource): array { + return $this->validator->validateFromResource($resource); + } + + /** + * @return list, certificateValidation: ValidationResult}> + */ + protected function validateNativeFromString(string $pdfContent): array { + return $this->validator->validateFromString($pdfContent); } /** From 00bd45b9f531f397b615b3af9d7a5f3036e74248 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:43:14 -0300 Subject: [PATCH 26/36] fix: always reset policy context on metadata failures Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 52d692e955..a7bddc8913 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -3,7 +3,7 @@ declare(strict_types=1); /** * SPDX-FileCopyrightText: 2020-2024 LibreCode coop and contributors - * SPDX-License-Identifier: AGPL-3.0-or-later + * SPDX-License-Identifier: AGPL-3.0-or-later */ namespace OCA\Libresign\Handler\SignEngine; @@ -19,6 +19,7 @@ use OCA\Libresign\Service\Crl\CrlService; use OCA\Libresign\Service\FolderService; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Exception\UnsignedPdfException; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationReason; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; @@ -344,8 +345,8 @@ private function extractNativeSignatureMetadata($resource): array { } try { - $signatures = $this->pdfSignatureExtractor->extractFromString($content); - } catch (\Throwable) { + $signatures = $this->extractNativeSignaturesFromContent($content); + } catch (UnsignedPdfException) { return []; } $metadata = []; @@ -362,6 +363,10 @@ private function extractNativeSignatureMetadata($resource): array { return $metadata; } + protected function extractNativeSignaturesFromContent(string $content): array { + return $this->pdfSignatureExtractor->extractFromString($content); + } + private function der2pem($derData) { $pem = chunk_split(base64_encode((string)$derData), 64, "\n"); $pem = "-----BEGIN CERTIFICATE-----\n" . $pem . "-----END CERTIFICATE-----\n"; From 6caca7b2feec575e0528573a36c009315971a1f5 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:43:14 -0300 Subject: [PATCH 27/36] test: cover native validation exception flow Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationServiceTest.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index 1380eca9b2..137e1e1e9e 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -10,20 +10,28 @@ use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Unit\TestCase; +use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Exception\UnsignedPdfException; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationResult; use OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\Model\ValidationState; +use OCP\IAppConfig; use OCP\IL10N; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\LoggerInterface; final class PdfSignatureValidationServiceTest extends TestCase { + private IAppConfig&MockObject $appConfig; private IL10N&MockObject $l10n; + private LoggerInterface&MockObject $logger; public function setUp(): void { parent::setUp(); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->appConfig->method('getValueString')->willReturn(''); $this->l10n = $this->createMock(IL10N::class); $this->l10n ->method('t') ->willReturnCallback(static fn (string $text): string => $text); + $this->logger = $this->createMock(LoggerInterface::class); } public function testMapSignatureValidationWithEnumState(): void { @@ -75,6 +83,32 @@ public function testMapReasonKeepsUnknownReasonUntouched(): void { $this->assertSame('custom runtime detail', $result['reason']); } + public function testValidateFromStringReturnsEmptyListForUnsignedPdfException(): void { + $this->logger->expects($this->never())->method('warning'); + + $service = new class($this->appConfig, $this->l10n, $this->logger) extends PdfSignatureValidationService { + protected function validateNativeFromString(string $pdfContent): array { + throw new UnsignedPdfException('Unsigned file.'); + } + }; + + $this->assertSame([], $service->validateFromString('%PDF-1.7')); + } + + public function testValidateFromStringPropagatesUnexpectedRuntimeException(): void { + $this->logger->expects($this->never())->method('warning'); + + $service = new class($this->appConfig, $this->l10n, $this->logger) extends PdfSignatureValidationService { + protected function validateNativeFromString(string $pdfContent): array { + throw new \RuntimeException('validator boom'); + } + }; + + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessage('validator boom'); + $service->validateFromString('%PDF-1.7'); + } + private function newServiceWithoutConstructor(): PdfSignatureValidationService { $reflection = new \ReflectionClass(PdfSignatureValidationService::class); /** @var PdfSignatureValidationService $service */ From c25b69cbb1284899dc4c01ff9bc1c608d0ea2fad Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:43:15 -0300 Subject: [PATCH 28/36] test: cover metadata extraction failure propagation Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Handler/SignEngine/Pkcs12HandlerTest.php | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index ce0e966c02..b81e92d478 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -467,4 +467,58 @@ public function testGetCertificateChainDoesNotOverrideLegacySignatureValidationO $this->assertSame('Signature is valid.', $result[0]['chain'][0]['signature_validation']['label']); } + public function testGetCertificateChainPropagatesUnexpectedNativeMetadataExtractionFailureAndResetsPolicyValidationContext(): void { + $this->logger->expects($this->never())->method('warning'); + + $policyCalls = []; + $certificateEngine = $this->createMock(IEngineHandler::class); + $certificateEngine->method('setPolicyUserIdForValidation') + ->willReturnCallback(function (?string $userId) use (&$policyCalls, $certificateEngine) { + $policyCalls[] = $userId; + return $certificateEngine; + }); + + $certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); + $certificateEngineFactory->method('getEngine')->willReturn($certificateEngine); + + $handler = new class( + $this->folderService, + $this->appConfig, + $certificateEngineFactory, + $this->l10n, + $this->footerHandler, + $this->logger, + $this->caIdentifierService, + $this->docMdpHandler, + $this->crlService, + $this->pdfSignatureValidationService, + $this->pdfSignatureExtractor, + ) extends Pkcs12Handler { + protected function extractNativeSignaturesFromContent(string $content): array { + throw new \RuntimeException('metadata boom'); + } + }; + + $handler->setPolicyUserIdForValidation('requester'); + $resource = fopen(__DIR__ . '/../../../fixtures/pdfs/small_valid-signed.pdf', 'r'); + $this->assertIsResource($resource); + + $thrown = null; + try { + $handler->getCertificateChain($resource); + $this->fail('Expected RuntimeException to be propagated.'); + } catch (\RuntimeException $exception) { + $thrown = $exception; + } finally { + fclose($resource); + } + + $this->assertInstanceOf(\RuntimeException::class, $thrown); + $this->assertSame('metadata boom', $thrown->getMessage()); + $this->assertSame(['requester', null], $policyCalls); + + $reflection = new \ReflectionProperty(Pkcs12Handler::class, 'policyUserIdForValidation'); + $this->assertNull($reflection->getValue($handler)); + } + } From 7d15296954d2c5a338dbdefb5e66f827c23f5a08 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:47:30 -0300 Subject: [PATCH 29/36] fix: cs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Unit/Handler/SignEngine/Pkcs12HandlerTest.php | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index b81e92d478..cbc48a2b96 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -481,19 +481,7 @@ public function testGetCertificateChainPropagatesUnexpectedNativeMetadataExtract $certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); $certificateEngineFactory->method('getEngine')->willReturn($certificateEngine); - $handler = new class( - $this->folderService, - $this->appConfig, - $certificateEngineFactory, - $this->l10n, - $this->footerHandler, - $this->logger, - $this->caIdentifierService, - $this->docMdpHandler, - $this->crlService, - $this->pdfSignatureValidationService, - $this->pdfSignatureExtractor, - ) extends Pkcs12Handler { + $handler = new class($this->folderService, $this->appConfig, $certificateEngineFactory, $this->l10n, $this->footerHandler, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, $this->pdfSignatureValidationService, $this->pdfSignatureExtractor, ) extends Pkcs12Handler { protected function extractNativeSignaturesFromContent(string $content): array { throw new \RuntimeException('metadata boom'); } From acb800e98a5acf4409b9fdff18c28bf470021d51 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:58:25 -0300 Subject: [PATCH 30/36] fix: unit tests Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> From fec6988e8525ec7ec44c585ac405ebb57945598a Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:13:46 -0300 Subject: [PATCH 31/36] fix(sign-engine): backport policy validation context Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../CertificateEngine/AEngineHandler.php | 20 ++++++++++++++----- .../CertificateEngine/IEngineHandler.php | 2 ++ lib/Handler/SignEngine/SignEngineHandler.php | 7 +++++-- lib/Service/Crl/CrlRevocationChecker.php | 6 +++--- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/lib/Handler/CertificateEngine/AEngineHandler.php b/lib/Handler/CertificateEngine/AEngineHandler.php index 093f5ed113..ee76985eed 100644 --- a/lib/Handler/CertificateEngine/AEngineHandler.php +++ b/lib/Handler/CertificateEngine/AEngineHandler.php @@ -75,6 +75,7 @@ abstract class AEngineHandler implements IEngineHandler { protected string $currentCaId = ''; protected IAppData $appData; private CrlDistributionPointsExtractor $crlDistributionPointsExtractor; + private ?string $policyUserIdForValidation = null; public function __construct( protected IConfig $config, @@ -161,6 +162,15 @@ public function parseCertificate(string $certificate): array { return $this->parseX509($certificate); } + #[\Override] + public function setPolicyUserIdForValidation(?string $userId): self { + $this->policyUserIdForValidation = is_string($userId) && trim($userId) !== '' + ? trim($userId) + : null; + + return $this; + } + private function parseX509(string $x509): array { $parsed = openssl_x509_parse(openssl_x509_read($x509)); @@ -195,7 +205,7 @@ private function addCrlValidationInfo(array &$certData, string $certPem): void { return; } - $crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem); + $crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem, $this->policyUserIdForValidation); $certData['crl_validation'] = $crlDetails['status']; if (!empty($crlDetails['revoked_at'])) { $certData['crl_revoked_at'] = $crlDetails['revoked_at']; @@ -204,10 +214,10 @@ private function addCrlValidationInfo(array &$certData, string $certPem): void { } } - $externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true); - $certData['crl_validation'] = $externalValidationEnabled - ? CrlValidationStatus::MISSING - : CrlValidationStatus::DISABLED; + $emptyCrlValidation = $this->crlRevocationChecker->validate([], $certPem, $this->policyUserIdForValidation); + $certData['crl_validation'] = ($emptyCrlValidation['status'] ?? CrlValidationStatus::NO_URLS) === CrlValidationStatus::DISABLED + ? CrlValidationStatus::DISABLED + : CrlValidationStatus::MISSING; $certData['crl_urls'] = []; } diff --git a/lib/Handler/CertificateEngine/IEngineHandler.php b/lib/Handler/CertificateEngine/IEngineHandler.php index 2f9073c31b..254ee5d270 100644 --- a/lib/Handler/CertificateEngine/IEngineHandler.php +++ b/lib/Handler/CertificateEngine/IEngineHandler.php @@ -78,6 +78,8 @@ public function toArray(): array; */ public function generateCrlDer(array $revokedCertificates, string $instanceId, int $generation, int $crlNumber): string; + public function setPolicyUserIdForValidation(?string $userId): self; + /** * Parse an X.509 certificate and return its details with CRL validation * @param string $certificate PEM-encoded certificate diff --git a/lib/Handler/SignEngine/SignEngineHandler.php b/lib/Handler/SignEngine/SignEngineHandler.php index c3b3d249a9..500625fba4 100644 --- a/lib/Handler/SignEngine/SignEngineHandler.php +++ b/lib/Handler/SignEngine/SignEngineHandler.php @@ -262,9 +262,12 @@ protected function getFileStream() { return $stream; } + protected function getCertificateEngineFactory(): CertificateEngineFactory { + return \OCP\Server::get(CertificateEngineFactory::class); + } + protected function getCertificateEngine(): IEngineHandler { - return \OCP\Server::get(CertificateEngineFactory::class) - ->getEngine(); + return $this->getCertificateEngineFactory()->getEngine(); } protected function beforeSign(): void { diff --git a/lib/Service/Crl/CrlRevocationChecker.php b/lib/Service/Crl/CrlRevocationChecker.php index 727ad962d7..6324c46075 100644 --- a/lib/Service/Crl/CrlRevocationChecker.php +++ b/lib/Service/Crl/CrlRevocationChecker.php @@ -56,8 +56,8 @@ public function __construct( * * @return array{status: CrlValidationStatus, revoked_at?: string} */ - public function validate(array $crlUrls, string $certPem): array { - return $this->validateFromUrlsWithDetails($crlUrls, $certPem); + public function validate(array $crlUrls, string $certPem, ?string $policyUserId = null): array { + return $this->validateFromUrlsWithDetails($crlUrls, $certPem, $policyUserId); } /** @@ -66,7 +66,7 @@ public function validate(array $crlUrls, string $certPem): array { * * @return array{status: CrlValidationStatus, revoked_at?: string} */ - private function validateFromUrlsWithDetails(array $crlUrls, string $certPem): array { + private function validateFromUrlsWithDetails(array $crlUrls, string $certPem, ?string $policyUserId = null): array { $externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true); if (empty($crlUrls)) { From 3682f8dfd632031dfe52cff6832f9324ca340416 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:13:54 -0300 Subject: [PATCH 32/36] fix(sign-engine): restore native validation flow Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 54 ++++++++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index a7bddc8913..51fa2043fc 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -36,6 +36,7 @@ class Pkcs12Handler extends SignEngineHandler { private ?JSignPdfHandler $jSignPdfHandler = null; private string $rootCertificatePem = ''; private bool $isLibreSignFile = false; + private ?string $policyUserIdForValidation = null; public function __construct( private FolderService $folderService, @@ -53,6 +54,11 @@ public function __construct( parent::__construct($l10n, $folderService, $logger); } + #[\Override] + protected function getCertificateEngineFactory(): CertificateEngineFactory { + return $this->certificateEngineFactory; + } + /** * @throws LibresignException When is not a signed file */ @@ -88,6 +94,14 @@ public function setIsLibreSignFile(): void { $this->isLibreSignFile = true; } + public function setPolicyUserIdForValidation(?string $userId): self { + $this->policyUserIdForValidation = is_string($userId) && trim($userId) !== '' + ? trim($userId) + : null; + + return $this; + } + /** * @param resource $resource * @throws LibresignException When is not a signed file @@ -96,20 +110,42 @@ public function setIsLibreSignFile(): void { #[\Override] public function getCertificateChain($resource): array { $certificates = []; + $certificateEngine = $this->getCertificateEngine(); + $certificateEngine->setPolicyUserIdForValidation($this->policyUserIdForValidation); - foreach ($this->getSignatures($resource) as $signature) { - if (!$signature) { - continue; - } + try { + $nativeMetadata = array_values($this->extractNativeSignatureMetadata($resource)); + rewind($resource); + $nativeValidation = array_values($this->pdfSignatureValidationService->validateFromResource($resource)); + $index = 0; + + foreach ($this->getSignatures($resource) as $signature) { + $metadata = $nativeMetadata[$index] ?? []; + $validation = $nativeValidation[$index] ?? []; + $index++; + + if (!$signature) { + continue; + } - $result = $this->processSignature($resource, $signature); + $result = $this->processSignature( + $resource, + $signature, + $metadata, + $validation + ); - if (empty($result['chain'])) { - continue; - } + if (empty($result['chain'])) { + continue; + } - $certificates[] = $result; + $certificates[] = $result; + } + } finally { + $certificateEngine->setPolicyUserIdForValidation(null); + $this->policyUserIdForValidation = null; } + return $certificates; } From 0a150d2c648f76e66f20bc7c1beddf277fef5a98 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:14:00 -0300 Subject: [PATCH 33/36] test(sign-engine): cover stable native validation flow Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index cbc48a2b96..e191bec963 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -45,17 +45,25 @@ final class Pkcs12HandlerTest extends \OCA\Libresign\Tests\Unit\TestCase { private PdfSignatureValidationService&MockObject $pdfSignatureValidationService; private PdfSignatureExtractor $pdfSignatureExtractor; + #[\Override] public function setUp(): void { $this->folderService = $this->createMock(FolderService::class); $this->appConfig = $this->getMockAppConfigWithReset(); $this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); $this->certificateEngine = $this->createMock(IEngineHandler::class); + $this->certificateEngine->method('setPolicyUserIdForValidation')->willReturnSelf(); + $this->certificateEngine->method('isSetupOk')->willReturn(true); + $this->certificateEngine->method('validateRootCertificate')->willReturnCallback(static function (): void { + }); $this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID); $this->footerHandler = $this->createMock(FooterHandler::class); $this->logger = $this->createMock(LoggerInterface::class); $this->caIdentifierService = $this->createMock(CaIdentifierService::class); $this->docMdpHandler = $this->createMock(DocMdpHandler::class); $this->crlService = $this->createMock(CrlService::class); + $this->pdfSignatureValidationService = $this->createMock(PdfSignatureValidationService::class); + $this->pdfSignatureValidationService->method('validateFromResource')->willReturn([]); + $this->pdfSignatureExtractor = new PdfSignatureExtractor(); $this->certificateEngineFactory->method('getEngine') ->willReturn($this->certificateEngine); From 7156bcba8f9eb88016e3e22c35430968d33350e6 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:17:43 -0300 Subject: [PATCH 34/36] chore(deps): point stable32 to pdf validator backport Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- 3rdparty | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/3rdparty b/3rdparty index 7146e7625b..d326a4844b 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit 7146e7625b7ea982fbbba82d1c07d21c3915f09f +Subproject commit d326a4844be1adc93b39e6afd6810657e5d114e5 From 1126c38d4dc87def6808e6116428c38603f8b76b Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:36:54 -0300 Subject: [PATCH 35/36] Revert "fix(sign-engine): backport policy validation context" This reverts commit fec6988e8525ec7ec44c585ac405ebb57945598a. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../CertificateEngine/AEngineHandler.php | 20 +++++-------------- .../CertificateEngine/IEngineHandler.php | 2 -- lib/Handler/SignEngine/SignEngineHandler.php | 7 ++----- lib/Service/Crl/CrlRevocationChecker.php | 6 +++--- 4 files changed, 10 insertions(+), 25 deletions(-) diff --git a/lib/Handler/CertificateEngine/AEngineHandler.php b/lib/Handler/CertificateEngine/AEngineHandler.php index ee76985eed..093f5ed113 100644 --- a/lib/Handler/CertificateEngine/AEngineHandler.php +++ b/lib/Handler/CertificateEngine/AEngineHandler.php @@ -75,7 +75,6 @@ abstract class AEngineHandler implements IEngineHandler { protected string $currentCaId = ''; protected IAppData $appData; private CrlDistributionPointsExtractor $crlDistributionPointsExtractor; - private ?string $policyUserIdForValidation = null; public function __construct( protected IConfig $config, @@ -162,15 +161,6 @@ public function parseCertificate(string $certificate): array { return $this->parseX509($certificate); } - #[\Override] - public function setPolicyUserIdForValidation(?string $userId): self { - $this->policyUserIdForValidation = is_string($userId) && trim($userId) !== '' - ? trim($userId) - : null; - - return $this; - } - private function parseX509(string $x509): array { $parsed = openssl_x509_parse(openssl_x509_read($x509)); @@ -205,7 +195,7 @@ private function addCrlValidationInfo(array &$certData, string $certPem): void { return; } - $crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem, $this->policyUserIdForValidation); + $crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem); $certData['crl_validation'] = $crlDetails['status']; if (!empty($crlDetails['revoked_at'])) { $certData['crl_revoked_at'] = $crlDetails['revoked_at']; @@ -214,10 +204,10 @@ private function addCrlValidationInfo(array &$certData, string $certPem): void { } } - $emptyCrlValidation = $this->crlRevocationChecker->validate([], $certPem, $this->policyUserIdForValidation); - $certData['crl_validation'] = ($emptyCrlValidation['status'] ?? CrlValidationStatus::NO_URLS) === CrlValidationStatus::DISABLED - ? CrlValidationStatus::DISABLED - : CrlValidationStatus::MISSING; + $externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true); + $certData['crl_validation'] = $externalValidationEnabled + ? CrlValidationStatus::MISSING + : CrlValidationStatus::DISABLED; $certData['crl_urls'] = []; } diff --git a/lib/Handler/CertificateEngine/IEngineHandler.php b/lib/Handler/CertificateEngine/IEngineHandler.php index 254ee5d270..2f9073c31b 100644 --- a/lib/Handler/CertificateEngine/IEngineHandler.php +++ b/lib/Handler/CertificateEngine/IEngineHandler.php @@ -78,8 +78,6 @@ public function toArray(): array; */ public function generateCrlDer(array $revokedCertificates, string $instanceId, int $generation, int $crlNumber): string; - public function setPolicyUserIdForValidation(?string $userId): self; - /** * Parse an X.509 certificate and return its details with CRL validation * @param string $certificate PEM-encoded certificate diff --git a/lib/Handler/SignEngine/SignEngineHandler.php b/lib/Handler/SignEngine/SignEngineHandler.php index 500625fba4..c3b3d249a9 100644 --- a/lib/Handler/SignEngine/SignEngineHandler.php +++ b/lib/Handler/SignEngine/SignEngineHandler.php @@ -262,12 +262,9 @@ protected function getFileStream() { return $stream; } - protected function getCertificateEngineFactory(): CertificateEngineFactory { - return \OCP\Server::get(CertificateEngineFactory::class); - } - protected function getCertificateEngine(): IEngineHandler { - return $this->getCertificateEngineFactory()->getEngine(); + return \OCP\Server::get(CertificateEngineFactory::class) + ->getEngine(); } protected function beforeSign(): void { diff --git a/lib/Service/Crl/CrlRevocationChecker.php b/lib/Service/Crl/CrlRevocationChecker.php index 6324c46075..727ad962d7 100644 --- a/lib/Service/Crl/CrlRevocationChecker.php +++ b/lib/Service/Crl/CrlRevocationChecker.php @@ -56,8 +56,8 @@ public function __construct( * * @return array{status: CrlValidationStatus, revoked_at?: string} */ - public function validate(array $crlUrls, string $certPem, ?string $policyUserId = null): array { - return $this->validateFromUrlsWithDetails($crlUrls, $certPem, $policyUserId); + public function validate(array $crlUrls, string $certPem): array { + return $this->validateFromUrlsWithDetails($crlUrls, $certPem); } /** @@ -66,7 +66,7 @@ public function validate(array $crlUrls, string $certPem, ?string $policyUserId * * @return array{status: CrlValidationStatus, revoked_at?: string} */ - private function validateFromUrlsWithDetails(array $crlUrls, string $certPem, ?string $policyUserId = null): array { + private function validateFromUrlsWithDetails(array $crlUrls, string $certPem): array { $externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true); if (empty($crlUrls)) { From 1d2d0f9fe209d56d78eb1d40f1f2569cf08e954b Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:38:16 -0300 Subject: [PATCH 36/36] fix(backport): drop policy context from stable32 Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 64 ++++++------------- .../Handler/SignEngine/Pkcs12HandlerTest.php | 43 ------------- 2 files changed, 21 insertions(+), 86 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 51fa2043fc..f7b2776f32 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -36,7 +36,6 @@ class Pkcs12Handler extends SignEngineHandler { private ?JSignPdfHandler $jSignPdfHandler = null; private string $rootCertificatePem = ''; private bool $isLibreSignFile = false; - private ?string $policyUserIdForValidation = null; public function __construct( private FolderService $folderService, @@ -54,11 +53,6 @@ public function __construct( parent::__construct($l10n, $folderService, $logger); } - #[\Override] - protected function getCertificateEngineFactory(): CertificateEngineFactory { - return $this->certificateEngineFactory; - } - /** * @throws LibresignException When is not a signed file */ @@ -94,14 +88,6 @@ public function setIsLibreSignFile(): void { $this->isLibreSignFile = true; } - public function setPolicyUserIdForValidation(?string $userId): self { - $this->policyUserIdForValidation = is_string($userId) && trim($userId) !== '' - ? trim($userId) - : null; - - return $this; - } - /** * @param resource $resource * @throws LibresignException When is not a signed file @@ -110,40 +96,32 @@ public function setPolicyUserIdForValidation(?string $userId): self { #[\Override] public function getCertificateChain($resource): array { $certificates = []; - $certificateEngine = $this->getCertificateEngine(); - $certificateEngine->setPolicyUserIdForValidation($this->policyUserIdForValidation); + $nativeMetadata = array_values($this->extractNativeSignatureMetadata($resource)); + rewind($resource); + $nativeValidation = array_values($this->pdfSignatureValidationService->validateFromResource($resource)); + $index = 0; - try { - $nativeMetadata = array_values($this->extractNativeSignatureMetadata($resource)); - rewind($resource); - $nativeValidation = array_values($this->pdfSignatureValidationService->validateFromResource($resource)); - $index = 0; - - foreach ($this->getSignatures($resource) as $signature) { - $metadata = $nativeMetadata[$index] ?? []; - $validation = $nativeValidation[$index] ?? []; - $index++; - - if (!$signature) { - continue; - } + foreach ($this->getSignatures($resource) as $signature) { + $metadata = $nativeMetadata[$index] ?? []; + $validation = $nativeValidation[$index] ?? []; + $index++; - $result = $this->processSignature( - $resource, - $signature, - $metadata, - $validation - ); + if (!$signature) { + continue; + } - if (empty($result['chain'])) { - continue; - } + $result = $this->processSignature( + $resource, + $signature, + $metadata, + $validation + ); - $certificates[] = $result; + if (empty($result['chain'])) { + continue; } - } finally { - $certificateEngine->setPolicyUserIdForValidation(null); - $this->policyUserIdForValidation = null; + + $certificates[] = $result; } return $certificates; diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index e191bec963..84e10a288d 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -51,7 +51,6 @@ public function setUp(): void { $this->appConfig = $this->getMockAppConfigWithReset(); $this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); $this->certificateEngine = $this->createMock(IEngineHandler::class); - $this->certificateEngine->method('setPolicyUserIdForValidation')->willReturnSelf(); $this->certificateEngine->method('isSetupOk')->willReturn(true); $this->certificateEngine->method('validateRootCertificate')->willReturnCallback(static function (): void { }); @@ -475,46 +474,4 @@ public function testGetCertificateChainDoesNotOverrideLegacySignatureValidationO $this->assertSame('Signature is valid.', $result[0]['chain'][0]['signature_validation']['label']); } - public function testGetCertificateChainPropagatesUnexpectedNativeMetadataExtractionFailureAndResetsPolicyValidationContext(): void { - $this->logger->expects($this->never())->method('warning'); - - $policyCalls = []; - $certificateEngine = $this->createMock(IEngineHandler::class); - $certificateEngine->method('setPolicyUserIdForValidation') - ->willReturnCallback(function (?string $userId) use (&$policyCalls, $certificateEngine) { - $policyCalls[] = $userId; - return $certificateEngine; - }); - - $certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); - $certificateEngineFactory->method('getEngine')->willReturn($certificateEngine); - - $handler = new class($this->folderService, $this->appConfig, $certificateEngineFactory, $this->l10n, $this->footerHandler, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, $this->pdfSignatureValidationService, $this->pdfSignatureExtractor, ) extends Pkcs12Handler { - protected function extractNativeSignaturesFromContent(string $content): array { - throw new \RuntimeException('metadata boom'); - } - }; - - $handler->setPolicyUserIdForValidation('requester'); - $resource = fopen(__DIR__ . '/../../../fixtures/pdfs/small_valid-signed.pdf', 'r'); - $this->assertIsResource($resource); - - $thrown = null; - try { - $handler->getCertificateChain($resource); - $this->fail('Expected RuntimeException to be propagated.'); - } catch (\RuntimeException $exception) { - $thrown = $exception; - } finally { - fclose($resource); - } - - $this->assertInstanceOf(\RuntimeException::class, $thrown); - $this->assertSame('metadata boom', $thrown->getMessage()); - $this->assertSame(['requester', null], $policyCalls); - - $reflection = new \ReflectionProperty(Pkcs12Handler::class, 'policyUserIdForValidation'); - $this->assertNull($reflection->getValue($handler)); - } - }