[stable35] refactor(jsignpdf): move the hash algorithm resolution to a dedicated class

lib/Handler/SignEngine/JSignPdf/HashAlgorithmResolver.php

@@ -0,0 +1,86 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 LibreCode coop and contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Libresign\Handler\SignEngine\JSignPdf;
+
+use OCA\Libresign\Service\Policy\PolicyService;
+use OCA\Libresign\Service\Policy\Provider\SignatureHashAlgorithm\SignatureHashAlgorithmPolicy;
+
+/**
+ * Resolves which hash algorithm JSignPdf has to use.
+ *
+ * The algorithm that signs the document depends on the PDF version, so it is
+ * not a valid answer for the other hashes JSignPdf takes: each one needs its
+ * own method here.
+ */
+class HashAlgorithmResolver {
+	private const float MIN_PDF_VERSION_SHA256 = 1.6;
+	private const float MIN_PDF_VERSION_SHA1_REJECT = 1.7;
+	private const string DEFAULT_ALGORITHM = 'SHA256';
+	/** @var string[] */
+	private const array SUPPORTED_ALGORITHMS = ['SHA1', 'SHA256', 'SHA384', 'SHA512', 'RIPEMD160'];
+
+	public function __construct(
+		private PolicyService $policyService,
+	) {
+	}
+
+	/**
+	 * Algorithm used to sign a document with the given PDF version.
+	 *
+	 * @param float|null $pdfVersion null when the content carries no readable PDF header
+	 */
+	public function forSignature(?float $pdfVersion): string {
+		$configuredAlgorithm = $this->getConfiguredAlgorithm();
+		/**
+		 * Need to respect the follow code:
+		 * https://github.com/intoolswetrust/jsignpdf/blob/JSignPdf_2_2_2/jsignpdf/src/main/java/net/sf/jsignpdf/types/HashAlgorithm.java#L46-L47
+		 */
+		if ($pdfVersion === null) {
+			return $this->validate($configuredAlgorithm);
+		}
+
+		return $this->forPdfVersion($pdfVersion, $configuredAlgorithm);
+	}
+
+	/**
+	 * PDFs older than 1.6 have to be upgraded before JSignPdf accepts SHA-256.
+	 */
+	public function requiresPdfVersionUpgradeForSha256(float $pdfVersion): bool {
+		if ($pdfVersion >= self::MIN_PDF_VERSION_SHA256) {
+			return false;
+		}
+
+		return $this->getConfiguredAlgorithm() === self::DEFAULT_ALGORITHM;
+	}
+
+	private function forPdfVersion(float $pdfVersion, string $configuredAlgorithm): string {
+		// Legacy compatibility: JSignPdf still requires SHA1 for very old PDFs (< 1.6).
+		// The policy still exposes SHA1 for supported legacy workflows, and the runtime
+		// must continue enforcing this fallback for ancient PDFs that JSignPdf cannot sign otherwise.
+		if ($pdfVersion < self::MIN_PDF_VERSION_SHA256) {
+			return 'SHA1';
+		}
+		if ($pdfVersion < self::MIN_PDF_VERSION_SHA1_REJECT) {
+			return self::DEFAULT_ALGORITHM;
+		}
+		if ($configuredAlgorithm === 'SHA1') {
+			return self::DEFAULT_ALGORITHM;
+		}
+
+		return $this->validate($configuredAlgorithm);
+	}
+
+	private function validate(string $algorithm): string {
+		return in_array($algorithm, self::SUPPORTED_ALGORITHMS, true) ? $algorithm : self::DEFAULT_ALGORITHM;
+	}
+
+	private function getConfiguredAlgorithm(): string {
+		return (string)$this->policyService->resolve(SignatureHashAlgorithmPolicy::KEY)->getEffectiveValue();
+	}
+}

lib/Handler/SignEngine/JSignPdfHandler.php → lib/Handler/SignEngine/JSignPdf/JSignPdfHandler.php

@@ -6,17 +6,17 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-namespace OCA\Libresign\Handler\SignEngine;
+namespace OCA\Libresign\Handler\SignEngine\JSignPdf;
 
 use Imagick;
 use ImagickPixel;
 use OCA\Libresign\AppInfo\Application;
 use OCA\Libresign\Exception\LibresignException;
 use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory;
+use OCA\Libresign\Handler\SignEngine\Pkcs12Handler;
 use OCA\Libresign\Helper\JavaHelper;
 use OCA\Libresign\Service\DocMdp\ConfigService as DocMdpConfigService;
 use OCA\Libresign\Service\Policy\PolicyService;
-use OCA\Libresign\Service\Policy\Provider\SignatureHashAlgorithm\SignatureHashAlgorithmPolicy;
 use OCA\Libresign\Service\Policy\Provider\SignatureText\SignatureTextPolicyValue;
 use OCA\Libresign\Service\Policy\Provider\Tsa\TsaPolicy;
 use OCA\Libresign\Service\Policy\Provider\Tsa\TsaPolicyValue;
@@ -33,9 +33,7 @@
 class JSignPdfHandler extends Pkcs12Handler {
 	private const float MIN_PDF_VERSION = 1.2;
 	private const string TARGET_OLD_PDF_VERSION = '1.3';
-	private const float MIN_PDF_VERSION_SHA256 = 1.6;
 	private const string TARGET_PDF_VERSION_SHA256 = '1.6';
-	private const float MIN_PDF_VERSION_SHA1_REJECT = 1.7;
 	private const int PAGE_FIRST = 1;
 	private const int SCALE_FACTOR_MIN = 5;
 
@@ -55,6 +53,7 @@ public function __construct(
 		protected CertificateEngineFactory $certificateEngineFactory,
 		protected JavaHelper $javaHelper,
 		private DocMdpConfigService $docMdpConfigService,
+		private HashAlgorithmResolver $hashAlgorithmResolver,
 	) {
 	}
 
@@ -151,49 +150,13 @@ private function createEmptyFile(string $path): void {
 		fclose($file);
 	}
 
-	private function getHashAlgorithm(string $pdfContent): string {
-		$configuredAlgorithm = (string)$this->policyService->resolve(SignatureHashAlgorithmPolicy::KEY)->getEffectiveValue();
-		/**
-		 * Need to respect the follow code:
-		 * https://github.com/intoolswetrust/jsignpdf/blob/JSignPdf_2_2_2/jsignpdf/src/main/java/net/sf/jsignpdf/types/HashAlgorithm.java#L46-L47
-		 */
-		$pdfVersion = $this->extractPdfVersion($pdfContent);
-
-		if ($pdfVersion === null) {
-			return $this->validateHashAlgorithm($configuredAlgorithm);
-		}
-
-		return $this->getHashAlgorithmForPdfVersion($pdfVersion, $configuredAlgorithm);
-	}
-
 	private function extractPdfVersion(string $content): ?float {
 		if (!preg_match('/^%PDF-(?<version>\d+(\.\d+)?)/', $content, $match)) {
 			return null;
 		}
 		return (float)$match['version'];
 	}
 
-	private function getHashAlgorithmForPdfVersion(float $pdfVersion, string $configuredAlgorithm): string {
-		// Legacy compatibility: JSignPdf still requires SHA1 for very old PDFs (< 1.6).
-		// The policy still exposes SHA1 for supported legacy workflows, and the runtime
-		// must continue enforcing this fallback for ancient PDFs that JSignPdf cannot sign otherwise.
-		if ($pdfVersion < 1.6) {
-			return 'SHA1';
-		}
-		if ($pdfVersion < self::MIN_PDF_VERSION_SHA1_REJECT) {
-			return 'SHA256';
-		}
-		if ($pdfVersion >= self::MIN_PDF_VERSION_SHA1_REJECT && $configuredAlgorithm === 'SHA1') {
-			return 'SHA256';
-		}
-		return $this->validateHashAlgorithm($configuredAlgorithm);
-	}
-
-	private function validateHashAlgorithm(string $algorithm): string {
-		$supportedAlgorithms = ['SHA1', 'SHA256', 'SHA384', 'SHA512', 'RIPEMD160'];
-		return in_array($algorithm, $supportedAlgorithms) ? $algorithm : 'SHA256';
-	}
-
 	/**
 	 * Normalizes very old PDFs (1.0/1.1) to 1.3.
 	 * Rationale: JSignPDF enum PdfVersion only defines 1.2+; for 1.0/1.1,
@@ -213,7 +176,7 @@ private function normalizePdfVersion(string $content): string {
 
 		// Convert PDFs < 1.6 to 1.6 if using SHA-256 (the default hash algorithm)
 		// This prevents "The chosen hash algorithm (SHA-256) requires a newer PDF version" error
-		if ($this->requiresPdfVersionUpgradeForSha256($version)) {
+		if ($this->hashAlgorithmResolver->requiresPdfVersionUpgradeForSha256($version)) {
 			return $this->replacePdfVersion($content, self::TARGET_PDF_VERSION_SHA256);
 		}
 
@@ -224,14 +187,6 @@ private function isVeryOldPdfVersion(float $version): bool {
 		return $version > 0 && $version < self::MIN_PDF_VERSION;
 	}
 
-	private function requiresPdfVersionUpgradeForSha256(float $version): bool {
-		if ($version >= self::MIN_PDF_VERSION_SHA256) {
-			return false;
-		}
-		$hashAlgorithm = (string)$this->policyService->resolve(SignatureHashAlgorithmPolicy::KEY)->getEffectiveValue();
-		return $hashAlgorithm === 'SHA256';
-	}
-
 	private function replacePdfVersion(string $content, string $newVersion): string {
 		return (string)preg_replace('/^%PDF-\d+(\.\d+)?/', '%PDF-' . $newVersion, $content, 1);
 	}
@@ -256,7 +211,7 @@ public function sign(): File {
 	#[\Override]
 	public function getSignedContent(): string {
 		$normalizedPdf = $this->normalizePdfVersion($this->getInputFile()->getContent());
-		$hashAlgorithm = $this->getHashAlgorithm($normalizedPdf);
+		$hashAlgorithm = $this->hashAlgorithmResolver->forSignature($this->extractPdfVersion($normalizedPdf));
 		$param = $this->getJSignParam();
 		$param->setCertificate($this->getCertificate())
 			->setPdf($normalizedPdf)

lib/Handler/SignEngine/Pkcs12Handler.php

@@ -14,6 +14,7 @@
 use OCA\Libresign\Handler\CertificateEngine\OrderCertificatesTrait;
 use OCA\Libresign\Handler\DocMdpHandler;
 use OCA\Libresign\Handler\FooterHandler;
+use OCA\Libresign\Handler\SignEngine\JSignPdf\JSignPdfHandler;
 use OCA\Libresign\Service\CaIdentifierService;
 use OCA\Libresign\Service\Crl\CrlService;
 use OCA\Libresign\Service\FolderService;
@@ -30,6 +31,11 @@
 class Pkcs12Handler extends SignEngineHandler {
 	use OrderCertificatesTrait;
 	protected string $certificate = '';
+	/** @var array<string, class-string<SignEngineHandler>> */
+	private const ENGINE_HANDLERS = [
+		'jSignPdfHandler' => JSignPdfHandler::class,
+		'phpNativeHandler' => PhpNativeHandler::class,
+	];
 	private ?JSignPdfHandler $jSignPdfHandler = null;
 	private ?PhpNativeHandler $phpNativeHandler = null;
 	private string $rootCertificatePem = '';
@@ -379,11 +385,13 @@ private function enrichLeafWithNativeData(
 	private function getHandler(): SignEngineHandler {
 		$sign_engine = $this->appConfig->getValueString(Application::APP_ID, 'signature_engine', 'JSignPdf');
 		$property = lcfirst($sign_engine) . 'Handler';
-		if (!property_exists($this, $property)) {
+		// Resolved through a class map instead of a name built at runtime, so
+		// moving a handler to another namespace cannot break this silently.
+		if (!isset(self::ENGINE_HANDLERS[$property])) {
 			// TRANSLATORS API/config error when LibreSign's signature_engine setting names a backend that is not available (for example a mistyped JSignPdf/native engine).
 			throw new LibresignException($this->l10n->t('Invalid Sign engine.'), 400);
 		}
-		$classHandler = 'OCA\\Libresign\\Handler\\SignEngine\\' . ucfirst($property);
+		$classHandler = self::ENGINE_HANDLERS[$property];
 		if (!$this->$property instanceof $classHandler) {
 			$this->$property = \OCP\Server::get($classHandler);
 		}

lib/SetupCheck/JSignPdfSetupCheck.php

@@ -9,7 +9,7 @@
 namespace OCA\Libresign\SetupCheck;
 
 use OCA\Libresign\AppInfo\Application;
-use OCA\Libresign\Handler\SignEngine\JSignPdfHandler;
+use OCA\Libresign\Handler\SignEngine\JSignPdf\JSignPdfHandler;
 use OCA\Libresign\Helper\JavaHelper;
 use OCA\Libresign\Service\Install\InstallService;
 use OCA\Libresign\Service\Install\SignSetupService;

tests/php/Unit/Handler/SignEngine/JSignPdf/HashAlgorithmResolverTest.php

@@ -0,0 +1,90 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2026 LibreCode coop and contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Libresign\Tests\Unit\Handler\SignEngine\JSignPdf;
+
+use OCA\Libresign\Handler\SignEngine\JSignPdf\HashAlgorithmResolver;
+use OCA\Libresign\Service\Policy\Model\ResolvedPolicy;
+use OCA\Libresign\Service\Policy\PolicyService;
+use OCA\Libresign\Service\Policy\Provider\SignatureHashAlgorithm\SignatureHashAlgorithmPolicy;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+
+class HashAlgorithmResolverTest extends TestCase {
+	private PolicyService&MockObject $policyService;
+
+	#[\Override]
+	protected function setUp(): void {
+		$this->policyService = $this->createMock(PolicyService::class);
+	}
+
+	private function getInstance(mixed $configuredAlgorithm): HashAlgorithmResolver {
+		$this->policyService
+			->method('resolve')
+			->with(SignatureHashAlgorithmPolicy::KEY)
+			->willReturn(
+				(new ResolvedPolicy())
+					->setPolicyKey(SignatureHashAlgorithmPolicy::KEY)
+					->setEffectiveValue($configuredAlgorithm)
+			);
+
+		return new HashAlgorithmResolver($this->policyService);
+	}
+
+	#[DataProvider('providerSignatureHashAlgorithm')]
+	public function testForSignature(mixed $configuredAlgorithm, ?float $pdfVersion, string $expected): void {
+		$resolver = $this->getInstance($configuredAlgorithm);
+
+		$this->assertSame($expected, $resolver->forSignature($pdfVersion));
+	}
+
+	public static function providerSignatureHashAlgorithm(): array {
+		return [
+			// Unknown PDF version: only the configured algorithm decides.
+			'unknown version keeps a supported algorithm' => ['SHA384', null, 'SHA384'],
+			'unknown version keeps RIPEMD160' => ['RIPEMD160', null, 'RIPEMD160'],
+			'unknown version falls back on an empty algorithm' => ['', null, 'SHA256'],
+			'unknown version falls back on an unsupported algorithm' => ['XYZ', null, 'SHA256'],
+			'unknown version falls back on an unset policy' => [null, null, 'SHA256'],
+			// JSignPdf only accepts SHA1 in PDFs older than 1.6.
+			'PDF 1.0 is signed with SHA1' => ['SHA256', 1.0, 'SHA1'],
+			'PDF 1.5 is signed with SHA1' => ['SHA512', 1.5, 'SHA1'],
+			// Between 1.6 and 1.7 JSignPdf only accepts SHA256.
+			'PDF 1.6 is signed with SHA256' => ['SHA384', 1.6, 'SHA256'],
+			'PDF 1.6 ignores an unsupported algorithm' => ['XYZ', 1.6, 'SHA256'],
+			// From 1.7 on the configured algorithm is used, except SHA1.
+			'PDF 1.7 keeps the configured SHA384' => ['SHA384', 1.7, 'SHA384'],
+			'PDF 1.7 keeps the configured SHA512' => ['SHA512', 1.7, 'SHA512'],
+			'PDF 1.7 keeps the configured RIPEMD160' => ['RIPEMD160', 1.7, 'RIPEMD160'],
+			'PDF 1.7 replaces SHA1 with SHA256' => ['SHA1', 1.7, 'SHA256'],
+			'PDF 2.0 replaces SHA1 with SHA256' => ['SHA1', 2.0, 'SHA256'],
+			'PDF 2.0 falls back on an unsupported algorithm' => ['XYZ', 2.0, 'SHA256'],
+			'PDF 2.0 keeps the configured SHA512' => ['SHA512', 2.0, 'SHA512'],
+		];
+	}
+
+	#[DataProvider('providerPdfVersionUpgrade')]
+	public function testRequiresPdfVersionUpgradeForSha256(mixed $configuredAlgorithm, float $pdfVersion, bool $expected): void {
+		$resolver = $this->getInstance($configuredAlgorithm);
+
+		$this->assertSame($expected, $resolver->requiresPdfVersionUpgradeForSha256($pdfVersion));
+	}
+
+	public static function providerPdfVersionUpgrade(): array {
+		return [
+			'SHA256 in a PDF 1.2 needs the upgrade' => ['SHA256', 1.2, true],
+			'SHA256 in a PDF 1.5 needs the upgrade' => ['SHA256', 1.5, true],
+			'SHA256 in a PDF 1.6 does not need the upgrade' => ['SHA256', 1.6, false],
+			'SHA256 in a PDF 1.7 does not need the upgrade' => ['SHA256', 1.7, false],
+			'SHA1 in a PDF 1.5 does not need the upgrade' => ['SHA1', 1.5, false],
+			'SHA512 in a PDF 1.5 does not need the upgrade' => ['SHA512', 1.5, false],
+			'an unset policy in a PDF 1.5 does not need the upgrade' => [null, 1.5, false],
+		];
+	}
+}