From b0d079c9253419ac9d74fbc833ca8217638e8330 Mon Sep 17 00:00:00 2001 From: "r.carneiro" Date: Sat, 22 Aug 2026 21:27:57 +0300 Subject: [PATCH] fix: promote certificate validity dates to signer root The certificate signers merge service stopped promoting the end-entity certificate validity dates (valid_from/valid_to) to the top-level signer object. The validation panel and the SignerDetail API contract read these fields from the signer root, so the panel always showed "No expiration date" even though the leaf certificate carries a valid notAfter. Restore the promotion of the ISO UTC validity dates from chain[0] to the signer root, as the pre-refactor logic did. Update the SignersLoader test that had codified the buggy behavior and add a regression test covering the merged contract. Signed-off-by: r.carneiro --- .../File/CertificateSignersMergeService.php | 10 ++++++ .../features/file/validate.feature | 2 ++ .../CertificateSignersMergeServiceTest.php | 31 +++++++++++++++++++ .../Unit/Service/File/SignersLoaderTest.php | 10 +++--- 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/lib/Service/File/CertificateSignersMergeService.php b/lib/Service/File/CertificateSignersMergeService.php index cdb1756d48..2cba68abb4 100644 --- a/lib/Service/File/CertificateSignersMergeService.php +++ b/lib/Service/File/CertificateSignersMergeService.php @@ -352,6 +352,16 @@ private function processChainData(\stdClass $signer, array $chain): void { if (isset($chain[0])) { $this->enrichSignerWithCertificateValidation($signer, $chain[0]); } + + // Promote the end-entity (leaf) certificate validity dates to the signer root so the + // validation UI and the SignerDetail API contract can read them from a stable location. + // The chain still carries the full per-certificate dates for the certificate-chain view. + if (isset($signer->chain[0]['valid_from']) && !isset($signer->valid_from)) { + $signer->valid_from = $signer->chain[0]['valid_from']; + } + if (isset($signer->chain[0]['valid_to']) && !isset($signer->valid_to)) { + $signer->valid_to = $signer->chain[0]['valid_to']; + } } private function enrichSignerWithCertificateValidation(\stdClass $signer, array $endEntityCert): void { diff --git a/tests/integration/features/file/validate.feature b/tests/integration/features/file/validate.feature index a184af54a1..8e0f8256c5 100644 --- a/tests/integration/features/file/validate.feature +++ b/tests/integration/features/file/validate.feature @@ -52,6 +52,8 @@ Feature: validate | (jq).ocs.data.signers[0].subject.O | Organization | | (jq).ocs.data.signers[0].signature_validation | {"id":1,"label":"Signature is valid."} | | (jq).ocs.data.signers[0].signatureTypeSN | RSA-SHA256 | + | (jq)(.ocs.data.signers[0].valid_from \| test("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}[+-][0-9]{2}:[0-9]{2}$")) | true | + | (jq)(.ocs.data.signers[0].valid_to \| test("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}[+-][0-9]{2}:[0-9]{2}$")) | true | Scenario Outline: Unauthenticated user can fetch the validation ednpoint Given as user "admin" diff --git a/tests/php/Unit/Service/File/CertificateSignersMergeServiceTest.php b/tests/php/Unit/Service/File/CertificateSignersMergeServiceTest.php index 306652f354..f578de4ecb 100644 --- a/tests/php/Unit/Service/File/CertificateSignersMergeServiceTest.php +++ b/tests/php/Unit/Service/File/CertificateSignersMergeServiceTest.php @@ -154,6 +154,37 @@ public static function providerCertificateInfoMatching(): array { ]; } + public function testMergePromotesLeafCertValidityDatesToSignerRoot(): void { + $fileData = new \stdClass(); + $fileData->signers = []; + + $certData = [[ + 'uid' => 'email:signer@example.com', + 'chain' => [[ + 'subject' => ['CN' => 'Signer User'], + 'validFrom_time_t' => 1769644731, + 'validTo_time_t' => 1769731131, + ]], + ]]; + + $this->getService()->merge( + $fileData, + $certData, + 'example.com', + 'Signed', + fn (array $cert, string $host): ?string => $cert['subject']['UID'] ?? null, + fn (string $method, string $value): string => $method . ':' . $value, + fn (string $accountId): ?string => null, + ); + + $this->assertCount(1, $fileData->signers); + $signer = $fileData->signers[0]; + $this->assertSame('2026-01-28T23:58:51+00:00', $signer->valid_from); + $this->assertSame('2026-01-29T23:58:51+00:00', $signer->valid_to); + $this->assertSame('2026-01-28T23:58:51+00:00', $signer->chain[0]['valid_from']); + $this->assertSame('2026-01-29T23:58:51+00:00', $signer->chain[0]['valid_to']); + } + public function testMergeDoesNotExportTopLevelTsaWithTimestampData(): void { $fileData = new \stdClass(); $fileData->signers = []; diff --git a/tests/php/Unit/Service/File/SignersLoaderTest.php b/tests/php/Unit/Service/File/SignersLoaderTest.php index 2abf5e2e90..9e5d7675fa 100644 --- a/tests/php/Unit/Service/File/SignersLoaderTest.php +++ b/tests/php/Unit/Service/File/SignersLoaderTest.php @@ -528,10 +528,12 @@ public function testLoadSignersFromCertDataPreventsDuplicateFormattedDates(): vo $this->assertSame('2026-01-28T23:58:51+00:00', $signer->chain[0]['valid_from']); $this->assertSame('2026-01-29T23:58:51+00:00', $signer->chain[0]['valid_to']); - // Root level should NOT have the formatted dates from backend - // These fields should only exist in the chain, not duplicated at root level - $this->assertObjectNotHasProperty('valid_from', $signer, 'valid_from should not be copied to root level'); - $this->assertObjectNotHasProperty('valid_to', $signer, 'valid_to should not be copied to root level'); + // Root level should expose the ISO UTC validity dates (from validFrom/validTo_time_t) so the + // validation UI and the SignerDetail API contract can read them from a stable location. The + // locale-formatted backend strings (e.g. 'January 28, 2026, 11:58:51 PM') must NOT leak here, + // but the ISO form IS promoted. + $this->assertSame('2026-01-28T23:58:51+00:00', $signer->valid_from); + $this->assertSame('2026-01-29T23:58:51+00:00', $signer->valid_to); // Also verify other technical fields are not duplicated $this->assertObjectNotHasProperty('validFrom_time_t', $signer);