Skip to content

Commit 3d4bdf0

Browse files
committed
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>
1 parent 65ac52b commit 3d4bdf0

2 files changed

Lines changed: 50 additions & 30 deletions

File tree

lib/Handler/CertificateEngine/AEngineHandler.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ public function toArray(): array {
738738
foreach ($names as $name => $value) {
739739
$return['rootCert']['names'][] = [
740740
'id' => $name,
741-
'value' => $this->filterNameValue($name, $value),
741+
'value' => $this->filterNameValue($name, $value, $generated),
742742
];
743743
}
744744
return $return;
@@ -748,9 +748,10 @@ private function getConfigPathForApi(bool $generated): string {
748748
return $generated ? $this->getCurrentConfigPath() : '';
749749
}
750750

751-
private function filterNameValue(string $name, mixed $value): mixed {
752-
if ($name === 'OU' && is_array($value)) {
753-
return $this->removeCaIdFromOrganizationalUnit($value);
751+
private function filterNameValue(string $name, mixed $value, bool $generated): mixed {
752+
if ($name === 'OU' && is_array($value) && !$generated) {
753+
$filtered = $this->removeCaIdFromOrganizationalUnit($value);
754+
return empty($filtered) ? null : $filtered;
754755
}
755756
return $value;
756757
}

tests/php/Unit/Handler/CertificateEngine/AEngineHandlerTest.php

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -291,35 +291,43 @@ public function testToArrayFiltersConfigPathWhenNotGenerated(
291291
}
292292

293293
#[DataProvider('dataProviderToArrayCaIdFiltering')]
294-
public function testToArrayFiltersCaIdFromOrganizationalUnit(
294+
public function testToArrayFiltersCaIdFromOrganizationalUnitWhenNotGenerated(
295+
bool $certificateGenerated,
295296
array $organizationalUnits,
296297
array $expectedOuValues,
297298
string $description,
298299
): void {
299300
$instance = $this->getInstance();
300301

302+
$tempPath = $this->tempManager->getTemporaryFolder('test-config');
303+
$instance->setConfigPath($tempPath);
301304
$instance->setOrganizationalUnit($organizationalUnits);
302305
$instance->setCountry('BR');
303306

307+
if ($certificateGenerated) {
308+
file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca.pem', 'fake-cert');
309+
file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca-key.pem', 'fake-key');
310+
}
311+
304312
$result = $instance->toArray();
305313

306-
$ouFound = false;
314+
$ouFound = null;
307315
foreach ($result['rootCert']['names'] as $name) {
308316
if ($name['id'] === 'OU') {
309-
$ouFound = true;
310-
$this->assertEquals(
311-
$expectedOuValues,
312-
$name['value'],
313-
"OrganizationalUnit should filter CA IDs: $description"
314-
);
317+
$ouFound = $name['value'];
315318
break;
316319
}
317320
}
318321

319322
if (!empty($expectedOuValues)) {
320-
$this->assertTrue($ouFound, "OU should be present in names array: $description");
323+
$this->assertNotNull($ouFound, "OU should be present in names array: $description");
324+
$this->assertEquals(
325+
$expectedOuValues,
326+
$ouFound,
327+
"OrganizationalUnit filtering: $description"
328+
);
321329
} else {
322-
$this->assertFalse($ouFound, "OU should not be present when filtered to empty: $description");
330+
$this->assertNull($ouFound, "OU should not be present when filtered to empty: $description");
323331
}
324332
}
325333

@@ -364,30 +372,41 @@ public static function dataProviderToArrayConfigPath(): array {
364372

365373
public static function dataProviderToArrayCaIdFiltering(): array {
366374
return [
367-
'OU without CA ID' => [
375+
'OU without CA ID - not generated' => [
376+
false,
368377
['Engineering', 'Security'],
369378
['Engineering', 'Security'],
370-
'normal OU values should pass through',
379+
'normal OU values should pass through when not generated',
371380
],
372-
'OU with CA ID at start' => [
373-
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
381+
'OU without CA ID - generated' => [
382+
true,
374383
['Engineering', 'Security'],
375-
'CA ID at start should be filtered out',
376-
],
377-
'OU with CA ID in middle' => [
378-
['Engineering', 'libresign-ca-id:abc123_g:1_e:openssl', 'Security'],
379384
['Engineering', 'Security'],
380-
'CA ID in middle should be filtered out',
385+
'normal OU values should pass through when generated',
381386
],
382-
'OU with CA ID at end' => [
383-
['Engineering', 'Security', 'libresign-ca-id:abc123_g:1_e:openssl'],
387+
'OU with CA ID - not generated (filtered)' => [
388+
false,
389+
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
384390
['Engineering', 'Security'],
385-
'CA ID at end should be filtered out',
391+
'CA ID should be filtered when certificate not generated',
386392
],
387-
'OU with multiple CA IDs' => [
388-
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'libresign-ca-id:xyz789_g:2_e:cfssl', 'Security'],
389-
['Engineering', 'Security'],
390-
'multiple CA IDs should be filtered out',
393+
'OU with CA ID - generated (kept)' => [
394+
true,
395+
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
396+
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
397+
'CA ID should be kept when certificate is generated',
398+
],
399+
'OU with only CA ID - not generated' => [
400+
false,
401+
['libresign-ca-id:abc123_g:1_e:openssl'],
402+
[],
403+
'OU should be empty when only CA ID and not generated',
404+
],
405+
'OU with only CA ID - generated' => [
406+
true,
407+
['libresign-ca-id:abc123_g:1_e:openssl'],
408+
['libresign-ca-id:abc123_g:1_e:openssl'],
409+
'OU with only CA ID should be kept when generated',
391410
],
392411
];
393412
}

0 commit comments

Comments
 (0)