From 3d4bdf0294ccee53e367380d212f4edaa67e5b81 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Mon, 15 Dec 2025 17:39:24 -0300 Subject: [PATCH] fix: filter CA ID from OU only when certificate not generated The CA ID (libresign-ca-id:...) in OrganizationalUnit should only be filtered out when the certificate is not generated (isSetupOk() returns false). When the certificate is successfully generated, the CA ID must be preserved in the API response. This ensures: - Generated certificates: CA ID is visible (expected behavior) - Failed/not generated: CA ID is filtered to prevent stale data in form Integration tests validated: - features/account/signature.feature:2 (OpenSSL) - features/account/signature.feature:23 (CFSSL) - features/admin/certificate_openssl.feature:2 - features/admin/certificate_openssl.feature:35 Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../CertificateEngine/AEngineHandler.php | 9 +-- .../CertificateEngine/AEngineHandlerTest.php | 71 ++++++++++++------- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/lib/Handler/CertificateEngine/AEngineHandler.php b/lib/Handler/CertificateEngine/AEngineHandler.php index 90a6f0f44..99faa7ed3 100644 --- a/lib/Handler/CertificateEngine/AEngineHandler.php +++ b/lib/Handler/CertificateEngine/AEngineHandler.php @@ -738,7 +738,7 @@ abstract class AEngineHandler implements IEngineHandler { foreach ($names as $name => $value) { $return['rootCert']['names'][] = [ 'id' => $name, - 'value' => $this->filterNameValue($name, $value), + 'value' => $this->filterNameValue($name, $value, $generated), ]; } return $return; @@ -748,9 +748,10 @@ abstract class AEngineHandler implements IEngineHandler { return $generated ? $this->getCurrentConfigPath() : ''; } - private function filterNameValue(string $name, mixed $value): mixed { - if ($name === 'OU' && is_array($value)) { - return $this->removeCaIdFromOrganizationalUnit($value); + private function filterNameValue(string $name, mixed $value, bool $generated): mixed { + if ($name === 'OU' && is_array($value) && !$generated) { + $filtered = $this->removeCaIdFromOrganizationalUnit($value); + return empty($filtered) ? null : $filtered; } return $value; } diff --git a/tests/php/Unit/Handler/CertificateEngine/AEngineHandlerTest.php b/tests/php/Unit/Handler/CertificateEngine/AEngineHandlerTest.php index 9380f4de7..29f1b835f 100644 --- a/tests/php/Unit/Handler/CertificateEngine/AEngineHandlerTest.php +++ b/tests/php/Unit/Handler/CertificateEngine/AEngineHandlerTest.php @@ -291,35 +291,43 @@ final class AEngineHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase { } #[DataProvider('dataProviderToArrayCaIdFiltering')] - public function testToArrayFiltersCaIdFromOrganizationalUnit( + public function testToArrayFiltersCaIdFromOrganizationalUnitWhenNotGenerated( + bool $certificateGenerated, array $organizationalUnits, array $expectedOuValues, string $description, ): void { $instance = $this->getInstance(); + $tempPath = $this->tempManager->getTemporaryFolder('test-config'); + $instance->setConfigPath($tempPath); $instance->setOrganizationalUnit($organizationalUnits); $instance->setCountry('BR'); + if ($certificateGenerated) { + file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca.pem', 'fake-cert'); + file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca-key.pem', 'fake-key'); + } + $result = $instance->toArray(); - $ouFound = false; + $ouFound = null; foreach ($result['rootCert']['names'] as $name) { if ($name['id'] === 'OU') { - $ouFound = true; - $this->assertEquals( - $expectedOuValues, - $name['value'], - "OrganizationalUnit should filter CA IDs: $description" - ); + $ouFound = $name['value']; break; } } if (!empty($expectedOuValues)) { - $this->assertTrue($ouFound, "OU should be present in names array: $description"); + $this->assertNotNull($ouFound, "OU should be present in names array: $description"); + $this->assertEquals( + $expectedOuValues, + $ouFound, + "OrganizationalUnit filtering: $description" + ); } else { - $this->assertFalse($ouFound, "OU should not be present when filtered to empty: $description"); + $this->assertNull($ouFound, "OU should not be present when filtered to empty: $description"); } } @@ -364,30 +372,41 @@ final class AEngineHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase { public static function dataProviderToArrayCaIdFiltering(): array { return [ - 'OU without CA ID' => [ + 'OU without CA ID - not generated' => [ + false, ['Engineering', 'Security'], ['Engineering', 'Security'], - 'normal OU values should pass through', + 'normal OU values should pass through when not generated', ], - 'OU with CA ID at start' => [ + 'OU without CA ID - generated' => [ + true, + ['Engineering', 'Security'], + ['Engineering', 'Security'], + 'normal OU values should pass through when generated', + ], + 'OU with CA ID - not generated (filtered)' => [ + false, ['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'], ['Engineering', 'Security'], - 'CA ID at start should be filtered out', + 'CA ID should be filtered when certificate not generated', ], - 'OU with CA ID in middle' => [ - ['Engineering', 'libresign-ca-id:abc123_g:1_e:openssl', 'Security'], - ['Engineering', 'Security'], - 'CA ID in middle should be filtered out', + 'OU with CA ID - generated (kept)' => [ + true, + ['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'], + ['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'], + 'CA ID should be kept when certificate is generated', ], - 'OU with CA ID at end' => [ - ['Engineering', 'Security', 'libresign-ca-id:abc123_g:1_e:openssl'], - ['Engineering', 'Security'], - 'CA ID at end should be filtered out', + 'OU with only CA ID - not generated' => [ + false, + ['libresign-ca-id:abc123_g:1_e:openssl'], + [], + 'OU should be empty when only CA ID and not generated', ], - 'OU with multiple CA IDs' => [ - ['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'libresign-ca-id:xyz789_g:2_e:cfssl', 'Security'], - ['Engineering', 'Security'], - 'multiple CA IDs should be filtered out', + 'OU with only CA ID - generated' => [ + true, + ['libresign-ca-id:abc123_g:1_e:openssl'], + ['libresign-ca-id:abc123_g:1_e:openssl'], + 'OU with only CA ID should be kept when generated', ], ]; }