Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions lib/Controller/AdminController.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use OCA\Libresign\Handler\CertificateEngine\IEngineHandler;
use OCA\Libresign\Helper\ConfigureCheckHelper;
use OCA\Libresign\ResponseDefinitions;
use OCA\Libresign\Service\Certificate\ValidateService;
use OCA\Libresign\Service\CertificatePolicyService;
use OCA\Libresign\Service\Install\ConfigureCheckService;
use OCA\Libresign\Service\Install\InstallService;
Expand Down Expand Up @@ -54,6 +55,7 @@ public function __construct(
protected ISession $session,
private SignatureBackgroundService $signatureBackgroundService,
private CertificatePolicyService $certificatePolicyService,
private ValidateService $validateService,
) {
parent::__construct(Application::APP_ID, $request);
$this->eventSource = $this->eventSourceFactory->create();
Expand Down Expand Up @@ -136,15 +138,14 @@ private function generateCertificate(
): IEngineHandler {
$names = [];
if (isset($rootCert['names'])) {
$this->validateService->validateNames($rootCert['names']);
foreach ($rootCert['names'] as $item) {
if (empty($item['id'])) {
throw new LibresignException('Parameter id is required!', 400);
}
$names[$item['id']]['value'] = $this->trimAndThrowIfEmpty($item['id'], $item['value']);
$names[$item['id']]['value'] = trim((string)$item['value']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be good to rewrite this line in multiple lines, to make it easier to read. Something like:

$itemId = $item['id'];
$itemValue = strval($item['value']);

$names[$itemId]['value'] = trim($itemValue);

It's something silly, but may help people new to the application to understand it's code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this too much vars?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe would be best create a fallback issue to implement a NormalizeService class to add the trim rule into all values and also add an uppercase rule into Country and maintain the scope of this PR more amalgamated with the issue that motivated this PR.

}
}
$this->validateService->validate('CN', $rootCert['commonName']);
$this->installService->generate(
$this->trimAndThrowIfEmpty('commonName', $rootCert['commonName']),
trim((string)$rootCert['commonName']),
$names,
$properties,
);
Expand Down Expand Up @@ -177,13 +178,6 @@ public function loadCertificate(): DataResponse {
return new DataResponse($certificate);
}

private function trimAndThrowIfEmpty(string $key, $value): string {
if (empty($value)) {
throw new LibresignException("parameter '{$key}' is required!", 400);
}
return trim((string)$value);
}

/**
* Check the configuration of LibreSign
*
Expand Down
70 changes: 70 additions & 0 deletions lib/Service/Certificate/RulesService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Libresign\Service\Certificate;

use OCP\IL10N;

class RulesService {

private array $rules = [
'CN' => [
'required' => true,
'min' => 1,
'max' => 64,
],
'C' => [
'min' => 2,
'max' => 2,
],
'ST' => [
'min' => 1,
'max' => 128,
],
'L' => [
'min' => 1,
'max' => 128,
],
'O' => [
'min' => 1,
'max' => 64,
],
'OU' => [
'min' => 1,
'max' => 64,
],
];

public function __construct(
protected IL10N $l10n,
) {

}

public function getRule(string $fieldName): array {
if (!isset($this->rules[$fieldName]['helperText'])) {
$this->rules[$fieldName]['helperText'] = $this->getHelperText($fieldName);
if (empty($this->rules[$fieldName]['helperText'])) {
unset($this->rules[$fieldName]['helperText']);
}
}
return $this->rules[$fieldName];
}

public function getHelperText(string $fieldName): ?string {
return match ($fieldName) {
'CN' => $this->l10n->t('Common Name (CN)'),
'C' => $this->l10n->t('Two-letter ISO 3166 country code'),
'ST' => $this->l10n->t('Full name of states or provinces'),
'L' => $this->l10n->t('Name of a locality or place, such as a city, county, or other geographic region'),
'O' => $this->l10n->t('Name of an organization'),
'OU' => $this->l10n->t('Name of an organizational unit'),
default => null,
};
}
}
48 changes: 48 additions & 0 deletions lib/Service/Certificate/ValidateService.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Libresign\Service\Certificate;

use InvalidArgumentException;
use OCP\IL10N;

class ValidateService {

public function __construct(
protected RulesService $rulesService,
protected IL10N $l10n,
) {

}

public function validate(string $fieldName, string $value):void {
$rule = $this->rulesService->getRule($fieldName);
$value = trim($value);
$length = strlen($value);
if (!$length && isset($rule['required']) && $rule['required']) {
throw new InvalidArgumentException(
$this->l10n->t("Parameter '%s' is required!", [$fieldName])
);
}
if ($length > $rule['max'] || $length < $rule['min']) {
throw new InvalidArgumentException(
$this->l10n->t("Parameter '%s' should be betweeen %s and %s.", [$fieldName, $rule['min'], $rule['max']])
);
}
}

public function validateNames(array $names) {
foreach ($names as $item) {
if (empty($item['id'])) {
throw new InvalidArgumentException('Parameter id is required!');
}
$this->validate($item['id'], $item['value']);
}
}

}
4 changes: 2 additions & 2 deletions src/helpers/certification.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ export function selectCustonOption(id) {
export const options = [
{
id: 'CN',
label: t('libresign', 'Name (CN)'),
label: t('libresign', 'Common Name (CN)'),
max: 64,
value: '',
helperText: t('libresign', 'Name (CN)'),
helperText: t('libresign', 'Common Name (CN)'),
},
{
id: 'C',
Expand Down
4 changes: 2 additions & 2 deletions src/views/Settings/RootCertificateCfssl.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<table class="grid">
<tbody>
<tr>
<td>{{ t('libresign', 'Name (CN)') }}</td>
<td>{{ t('libresign', 'Common Name (CN)') }}</td>
<td>{{ certificate.rootCert.commonName }}</td>
</tr>
<tr v-for="(customName) in certificate.rootCert.names" :key="customName.id" class="customNames">
Expand Down Expand Up @@ -57,7 +57,7 @@
</div>
<div v-else id="formRootCertificate" class="form-libresign">
<div class="form-group">
<label for="commonName" class="form-heading--required">{{ t('libresign', 'Name (CN)') }}</label>
<label for="commonName" class="form-heading--required">{{ t('libresign', 'Common Name (CN)') }}</label>
<NcTextField id="commonName"
ref="commonName"
v-model="certificate.rootCert.commonName"
Expand Down
4 changes: 2 additions & 2 deletions src/views/Settings/RootCertificateOpenSsl.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<table class="grid">
<tbody>
<tr>
<td>{{ t('libresign', 'Name (CN)') }}</td>
<td>{{ t('libresign', 'Common Name (CN)') }}</td>
<td>{{ certificate.rootCert.commonName }}</td>
</tr>
<tr v-for="(customName) in certificate.rootCert.names" :key="customName.id" class="customNames">
Expand Down Expand Up @@ -53,7 +53,7 @@
</div>
<div v-else id="formRootCertificateOpenSsl" class="form-libresign">
<div class="form-group">
<label for="commonName" class="form-heading--required">{{ t('libresign', 'Name (CN)') }}</label>
<label for="commonName" class="form-heading--required">{{ t('libresign', 'Common Name (CN)') }}</label>
<NcTextField id="commonName"
ref="commonName"
v-model="certificate.rootCert.commonName"
Expand Down
36 changes: 36 additions & 0 deletions tests/integration/features/admin/certificate_openssl.feature
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,24 @@ Feature: admin/certificate_openssl
| (jq).ocs.data.rootCert.names\|length | 0 |
| (jq).ocs.data.generated | true |

Scenario: Generate root cert with fail without CN
Given as user "admin"
When sending "post" to ocs "/apps/libresign/api/v1/admin/certificate/openssl"
| rootCert | {"commonName":""} |
Then the response should have a status code 401
And the response should be a JSON array with the following mandatory values
| key | value |
| (jq).ocs.data.message | Parameter 'CN' is required! |

Scenario: Generate root cert with a big CN
Given as user "admin"
When sending "post" to ocs "/apps/libresign/api/v1/admin/certificate/openssl"
| rootCert | {"commonName":"0123456789012345678901234567890123456789012345678901234567890123456789"} |
Then the response should have a status code 401
And the response should be a JSON array with the following mandatory values
| key | value |
| (jq).ocs.data.message | Parameter 'CN' should be betweeen 1 and 64. |

Scenario: Generate root cert with success using optional names values
Given as user "admin"
When sending "post" to ocs "/apps/libresign/api/v1/admin/certificate/openssl"
Expand All @@ -26,3 +44,21 @@ Feature: admin/certificate_openssl
| (jq).ocs.data.rootCert.names[0].id | C |
| (jq).ocs.data.rootCert.names[0].value | BR |
| (jq).ocs.data.generated | true |

Scenario: Generate root cert with fail when country have less then 2 characters
Given as user "admin"
When sending "post" to ocs "/apps/libresign/api/v1/admin/certificate/openssl"
| rootCert | {"commonName":"Common Name","names":[{"id": "C","value":"B"}]} |
Then the response should have a status code 401
And the response should be a JSON array with the following mandatory values
| key | value |
| (jq).ocs.data.message | Parameter 'C' should be betweeen 2 and 2. |

Scenario: Generate root cert with fail when country have more then 2 characters
Given as user "admin"
When sending "post" to ocs "/apps/libresign/api/v1/admin/certificate/openssl"
| rootCert | {"commonName":"Common Name","names":[{"id": "C","value":"BRA"}]} |
Then the response should have a status code 401
And the response should be a JSON array with the following mandatory values
| key | value |
| (jq).ocs.data.message | Parameter 'C' should be betweeen 2 and 2. |
Loading
Loading