Skip to content

Commit f8d6c54

Browse files
vitormattosvitormattos
authored
Merge pull request #7153 from LibreSign/backport/7150/stable33
[stable33] fix: error handling and crl delete
2 parents dd423d9 + fc3024b commit f8d6c54

File tree

7 files changed

+123
-7
lines changed

7 files changed

+123
-7
lines changed

‎lib/Service/AccountService.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@
1717
use OCA\Libresign\Db\SignRequestMapper;
1818
use OCA\Libresign\Db\UserElement;
1919
use OCA\Libresign\Db\UserElementMapper;
20+
use OCA\Libresign\Enum\CRLReason;
2021
use OCA\Libresign\Enum\FileStatus;
2122
use OCA\Libresign\Exception\InvalidPasswordException;
2223
use OCA\Libresign\Exception\LibresignException;
2324
use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory;
2425
use OCA\Libresign\Handler\SignEngine\Pkcs12Handler;
2526
use OCA\Libresign\Helper\FileUploadHelper;
2627
use OCA\Libresign\Helper\ValidateHelper;
28+
use OCA\Libresign\Service\Crl\CrlService;
2729
use OCA\Settings\Mailer\NewUserMailHelper;
2830
use OCP\Accounts\IAccountManager;
2931
use OCP\AppFramework\Db\DoesNotExistException;
@@ -78,6 +80,7 @@ public function __construct(
7880
private IClientService $clientService,
7981
private ITimeFactory $timeFactory,
8082
private FileUploadHelper $uploadHelper,
83+
private CrlService $crlService,
8184
) {
8285
}
8386

@@ -565,7 +568,16 @@ public function uploadPfx(array $file, IUser $user): void {
565568
}
566569

567570
public function deletePfx(IUser $user): void {
568-
$this->pkcs12Handler->deletePfx($user->getUID());
571+
$uid = $user->getUID();
572+
573+
$this->crlService->revokeUserCertificates(
574+
$uid,
575+
CRLReason::CESSATION_OF_OPERATION,
576+
'Certificate deleted by account owner.',
577+
$uid,
578+
);
579+
580+
$this->pkcs12Handler->deletePfx($uid);
569581
}
570582

571583
/**

‎lib/Service/IdentifyMethod/SignatureMethod/Password.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ private function validateCertificateRevocation(array $certificateData): void {
5353
return;
5454
}
5555
if ($status === CrlValidationStatus::REVOKED) {
56-
throw new LibresignException($this->identifyService->getL10n()->t('Certificate has been revoked'), 400);
56+
throw new LibresignException($this->identifyService->getL10n()->t('Certificate has been revoked'), 422);
5757
}
5858
// Admin explicitly disabled external CRL validation – allow signing.
5959
if ($status === CrlValidationStatus::DISABLED) {
@@ -63,19 +63,19 @@ private function validateCertificateRevocation(array $certificateData): void {
6363
// fail-closed – we cannot confirm the certificate is not revoked.
6464
throw new LibresignException(
6565
$this->identifyService->getL10n()->t('Certificate revocation status could not be verified'),
66-
400
66+
422
6767
);
6868
}
6969

7070
private function validateCertificateExpiration(array $certificateData): void {
7171
if (array_key_exists('validTo_time_t', $certificateData)) {
7272
$validTo = $certificateData['validTo_time_t'];
7373
if (!is_int($validTo)) {
74-
throw new LibresignException($this->identifyService->getL10n()->t('Invalid certificate'), 400);
74+
throw new LibresignException($this->identifyService->getL10n()->t('Invalid certificate'), 422);
7575
}
7676
$now = (new \DateTime())->getTimestamp();
7777
if ($validTo <= $now) {
78-
throw new LibresignException($this->identifyService->getL10n()->t('Certificate has expired'), 400);
78+
throw new LibresignException($this->identifyService->getL10n()->t('Certificate has expired'), 422);
7979
}
8080
}
8181
}

‎src/tests/store/sign.spec.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,20 @@ describe('useSignStore', () => {
357357
expect(result.type).toBe('signError')
358358
expect(result.errors).toEqual(['err'])
359359
})
360+
361+
it('returns signError for certificate validation errors and preserves API message', () => {
362+
const store = useSignStore()
363+
const apiErrors = [{ message: 'Certificate has been revoked', code: 422 }]
364+
const error = {
365+
response: { data: { ocs: { data: { errors: apiErrors } } } },
366+
}
367+
368+
const result = store.parseSignError(error)
369+
370+
expect(result.type).toBe('signError')
371+
expect(result.action).toBeUndefined()
372+
expect(result.errors).toEqual(apiErrors)
373+
})
360374
})
361375

362376
describe('setFileToSign', () => {

‎src/tests/views/SignPDF/Sign.spec.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,53 @@ describe('Sign.vue - signWithTokenCode', () => {
400400
})
401401
})
402402

403+
describe('Sign.vue - API error handling', () => {
404+
it('keeps certificate validation errors in signStore and does not open certificate modal', async () => {
405+
const SignComponent = await import('../../../views/SignPDF/_partials/Sign.vue')
406+
const submitSignature = (SignComponent.default as any).methods.submitSignature
407+
408+
const apiErrors = [{ message: 'Certificate has been revoked', code: 422 }]
409+
const context = {
410+
loading: false,
411+
elements: [],
412+
canCreateSignature: false,
413+
signRequestUuid: 'test-sign-request-uuid',
414+
signMethodsStore: {
415+
certificateEngine: 'openssl',
416+
},
417+
signatureElementsStore: {
418+
signs: {},
419+
},
420+
actionHandler: {
421+
showModal: vi.fn(),
422+
closeModal: vi.fn(),
423+
},
424+
signStore: {
425+
document: { id: 10 },
426+
clearSigningErrors: vi.fn(),
427+
setSigningErrors: vi.fn(),
428+
submitSignature: vi.fn().mockRejectedValue({
429+
type: 'signError',
430+
errors: apiErrors,
431+
}),
432+
},
433+
$emit: vi.fn(),
434+
sidebarStore: {
435+
hideSidebar: vi.fn(),
436+
},
437+
}
438+
439+
await submitSignature.call(context, {
440+
method: 'password',
441+
token: '123456',
442+
})
443+
444+
expect(context.actionHandler.showModal).not.toHaveBeenCalled()
445+
expect(context.signStore.setSigningErrors).toHaveBeenCalledWith(apiErrors)
446+
expect(context.loading).toBe(false)
447+
})
448+
})
449+
403450
describe('proceedWithSigning - Full flow with WhatsApp token', () => {
404451
let proceedWithSigningLogic: ProceedWithSigningLogic
405452

‎tests/php/Unit/Handler/SigningErrorHandlerTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ public static function libresignExceptionProvider(): array {
4646
'message' => 'Password required',
4747
'expectedAction' => JSActions::ACTION_CREATE_SIGNATURE_PASSWORD,
4848
],
49+
'code 422 revoked certificate triggers do nothing action' => [
50+
'code' => 422,
51+
'message' => 'Certificate has been revoked',
52+
'expectedAction' => JSActions::ACTION_DO_NOTHING,
53+
],
4954
'code 401 triggers do nothing action' => [
5055
'code' => 401,
5156
'message' => 'Unauthorized',

‎tests/php/Unit/Service/AccountServiceTest.php‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717
use OCA\Libresign\Db\SignRequestMapper;
1818
use OCA\Libresign\Db\UserElement;
1919
use OCA\Libresign\Db\UserElementMapper;
20+
use OCA\Libresign\Enum\CRLReason;
2021
use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory;
2122
use OCA\Libresign\Handler\SignEngine\Pkcs12Handler;
2223
use OCA\Libresign\Helper\FileUploadHelper;
2324
use OCA\Libresign\Helper\ValidateHelper;
2425
use OCA\Libresign\Service\AccountService;
26+
use OCA\Libresign\Service\Crl\CrlService;
2527
use OCA\Libresign\Service\FolderService;
2628
use OCA\Libresign\Service\IdDocsService;
2729
use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod;
@@ -81,6 +83,7 @@ final class AccountServiceTest extends \OCA\Libresign\Tests\Unit\TestCase {
8183
private RequestSignatureService&MockObject $requestSignatureService;
8284
private Pkcs12Handler&MockObject $pkcs12Handler;
8385
private FileUploadHelper&MockObject $uploadHelper;
86+
private CrlService&MockObject $crlService;
8487

8588
public function setUp(): void {
8689
parent::setUp();
@@ -115,6 +118,7 @@ public function setUp(): void {
115118
$this->clientService = $this->createMock(ClientService::class);
116119
$this->timeFactory = $this->createMock(TimeFactory::class);
117120
$this->uploadHelper = $this->createMock(FileUploadHelper::class);
121+
$this->crlService = $this->createMock(CrlService::class);
118122
}
119123

120124
private function getService(): AccountService {
@@ -146,10 +150,32 @@ private function getService(): AccountService {
146150
$this->folderService,
147151
$this->clientService,
148152
$this->timeFactory,
149-
$this->uploadHelper
153+
$this->uploadHelper,
154+
$this->crlService
150155
);
151156
}
152157

158+
public function testDeletePfxRevokesCertificatesWithReasonAndDeletesPfx(): void {
159+
$user = $this->createMock(IUser::class);
160+
$user->method('getUID')->willReturn('admin');
161+
162+
$this->crlService->expects($this->once())
163+
->method('revokeUserCertificates')
164+
->with(
165+
'admin',
166+
CRLReason::CESSATION_OF_OPERATION,
167+
'Certificate deleted by account owner.',
168+
'admin'
169+
)
170+
->willReturn(1);
171+
172+
$this->pkcs12Handler->expects($this->once())
173+
->method('deletePfx')
174+
->with('admin');
175+
176+
$this->getService()->deletePfx($user);
177+
}
178+
153179
#[DataProvider('provideValidateCertificateDataCases')]
154180
public function testValidateCertificateDataUsingDataProvider($arguments, $expectedErrorMessage):void {
155181
if (is_callable($arguments)) {

‎tests/php/Unit/Service/IdentifyMethod/PasswordTest.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,12 @@ public static function providerValidateToSignWithError(): array {
136136
}
137137

138138
#[DataProvider('providerValidateToSignWithCertificateData')]
139-
public function testValidateToSignWithCertificateData(array $certificateData, bool $shouldThrow, string $expectedMessage = ''): void {
139+
public function testValidateToSignWithCertificateData(
140+
array $certificateData,
141+
bool $shouldThrow,
142+
string $expectedMessage = '',
143+
?int $expectedCode = null,
144+
): void {
140145
$this->pkcs12Handler = $this->getPkcs12Instance(['getPfxOfCurrentSigner', 'setCertificate', 'setPassword', 'readCertificate']);
141146
$this->pkcs12Handler->method('getPfxOfCurrentSigner')->willReturn('mock-pfx');
142147
$this->pkcs12Handler->method('setCertificate')->willReturnSelf();
@@ -153,6 +158,9 @@ public function testValidateToSignWithCertificateData(array $certificateData, bo
153158
if ($expectedMessage) {
154159
$this->expectExceptionMessage($expectedMessage);
155160
}
161+
if ($expectedCode !== null) {
162+
$this->expectExceptionCode($expectedCode);
163+
}
156164
}
157165

158166
$password->validateToSign();
@@ -183,13 +191,15 @@ public static function providerValidateToSignWithCertificateData(): array {
183191
],
184192
'shouldThrow' => true,
185193
'expectedMessage' => 'Certificate has expired',
194+
'expectedCode' => 422,
186195
],
187196
'invalid certificate - validTo_time_t is string' => [
188197
'certificateData' => [
189198
'validTo_time_t' => '1234567890',
190199
],
191200
'shouldThrow' => true,
192201
'expectedMessage' => 'Invalid certificate',
202+
'expectedCode' => 422,
193203
],
194204
'invalid certificate - validTo_time_t is null' => [
195205
'certificateData' => [
@@ -233,6 +243,7 @@ public static function providerValidateToSignWithCertificateData(): array {
233243
],
234244
'shouldThrow' => true,
235245
'expectedMessage' => 'Certificate has been revoked',
246+
'expectedCode' => 422,
236247
],
237248
'valid certificate with crl validation' => [
238249
'certificateData' => [
@@ -255,6 +266,7 @@ public static function providerValidateToSignWithCertificateData(): array {
255266
],
256267
'shouldThrow' => true,
257268
'expectedMessage' => 'Certificate revocation status could not be verified',
269+
'expectedCode' => 422,
258270
],
259271
'invalid certificate - crl validation empty string' => [
260272
'certificateData' => [

0 commit comments

Comments
 (0)