Skip to content

Commit 2440ec3

Browse files
vitormattosvitormattos
authored
Merge pull request #7403 from LibreSign/backport/7402/stable32
[stable32] fix: CRL disabled flow and signing error UX
2 parents 133906b + 3e0659a commit 2440ec3

11 files changed

Lines changed: 520 additions & 62 deletions

File tree

‎lib/Handler/CertificateEngine/AEngineHandler.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,10 @@ private function addCrlValidationInfo(array &$certData, string $certPem): void {
193193
$certData['crl_revoked_at'] = $crlDetails['revoked_at'];
194194
}
195195
} else {
196-
$certData['crl_validation'] = CrlValidationStatus::MISSING;
196+
$externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true);
197+
$certData['crl_validation'] = $externalValidationEnabled
198+
? CrlValidationStatus::MISSING
199+
: CrlValidationStatus::DISABLED;
197200
$certData['crl_urls'] = [];
198201
}
199202
}

‎lib/Service/Crl/CrlRevocationChecker.php‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,33 @@ public function __construct(
4646

4747
/**
4848
* Validate a certificate against the CRL Distribution Points found in its
49-
* data. Returns an array with at least a 'status' key ({@see CrlValidationStatus})
49+
* data. Returns an array with a 'status' key (always {@see CrlValidationStatus})
5050
* and optionally 'revoked_at' (ISO 8601) when the certificate is revoked.
51+
*
52+
* @return array{status: CrlValidationStatus, revoked_at?: string}
5153
*/
5254
public function validate(array $crlUrls, string $certPem): array {
5355
return $this->validateFromUrlsWithDetails($crlUrls, $certPem);
5456
}
5557

58+
/**
59+
* Internal validation worker that iterates through CRL distribution points
60+
* and returns the validation status from the first accessible/conclusive point.
61+
*
62+
* @return array{status: CrlValidationStatus, revoked_at?: string}
63+
*/
5664
private function validateFromUrlsWithDetails(array $crlUrls, string $certPem): array {
65+
$externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true);
66+
5767
if (empty($crlUrls)) {
68+
// When external validation is disabled, treat an empty distribution-point
69+
// list the same as if all points were intentionally skipped.
70+
if (!$externalValidationEnabled) {
71+
return ['status' => CrlValidationStatus::DISABLED];
72+
}
5873
return ['status' => CrlValidationStatus::NO_URLS];
5974
}
6075

61-
$externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true);
62-
6376
$accessibleUrls = 0;
6477
$disabledUrls = 0;
6578
foreach ($crlUrls as $crlUrl) {
@@ -101,6 +114,11 @@ private function validateFromUrlsWithDetails(array $crlUrls, string $certPem): a
101114
return ['status' => CrlValidationStatus::VALIDATION_FAILED];
102115
}
103116

117+
/**
118+
* Download and validate CRL content from a single source URL.
119+
*
120+
* @return array{status: CrlValidationStatus, revoked_at?: string}
121+
*/
104122
private function downloadAndValidateWithDetails(string $crlUrl, string $certPem, bool $isLocal): array {
105123
try {
106124
if ($isLocal) {
@@ -221,6 +239,11 @@ protected function isSerialNumberInCrl(string $crlText, string $serialNumber): b
221239
return preg_match('/Serial Number: 0*' . preg_quote($normalizedSerial, '/') . '/', $crlText) === 1;
222240
}
223241

242+
/**
243+
* Check if certificate serial is revoked in the provided CRL content.
244+
*
245+
* @return array{status: CrlValidationStatus, revoked_at?: string}
246+
*/
224247
private function checkCertificateInCrlWithDetails(string $certPem, string $crlContent): array {
225248
try {
226249
$certResource = openssl_x509_read($certPem);

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

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,31 @@ private function validateCertificateRevocation(array $certificateData): void {
5959
if ($status === CrlValidationStatus::DISABLED) {
6060
return;
6161
}
62-
// Any other status (urls_inaccessible, validation_failed, validation_error, etc.):
63-
// fail-closed – we cannot confirm the certificate is not revoked.
64-
throw new LibresignException(
65-
$this->identifyService->getL10n()->t('Certificate revocation status could not be verified'),
66-
422
67-
);
62+
$this->logRevocationBlockedSigning($status);
63+
throw new LibresignException($this->getRevocationErrorMessage($status), 422);
64+
}
65+
66+
private function logRevocationBlockedSigning(CrlValidationStatus $status): void {
67+
$this->identifyService->getLogger()->warning('Signing blocked due to CRL validation status', [
68+
'status' => $status->value,
69+
'signer_uid' => $this->userSession->getUser()?->getUID(),
70+
]);
71+
}
72+
73+
private function getRevocationErrorMessage(CrlValidationStatus $status): string {
74+
return match ($status) {
75+
// TRANSLATORS Error when the CRL distribution points (URLs) cannot be reached to check if certificate is revoked
76+
CrlValidationStatus::URLS_INACCESSIBLE => $this->identifyService->getL10n()->t('Cannot reach the certificate revocation service. Signing is not allowed.'),
77+
// TRANSLATORS Error when an error occurs during certificate revocation status validation
78+
CrlValidationStatus::VALIDATION_ERROR => $this->identifyService->getL10n()->t('An error occurred during certificate validation. Signing is not allowed.'),
79+
// TRANSLATORS Error when certificate validation completed but could not determine if certificate is revoked
80+
CrlValidationStatus::VALIDATION_FAILED => $this->identifyService->getL10n()->t('Certificate validation failed. Signing is not allowed. Contact your administrator.'),
81+
// TRANSLATORS Error when certificate has no CRL distribution points (URLs to check revocation)
82+
CrlValidationStatus::NO_URLS => $this->identifyService->getL10n()->t('This certificate has no revocation URLs. Signing is not allowed. Contact your administrator.'),
83+
// TRANSLATORS Error when certificate has no revocation information configured
84+
CrlValidationStatus::MISSING => $this->identifyService->getL10n()->t('This certificate has no revocation information. Signing is not allowed. Contact your administrator.'),
85+
default => $this->identifyService->getL10n()->t('Certificate validation could not be completed. Signing is not allowed.'),
86+
};
6887
}
6988

7089
private function validateCertificateExpiration(array $certificateData): void {

‎playwright/e2e/sign-password-non-retriable-error.spec.ts‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2026 LibreCode coop and contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import { expect, test } from '@playwright/test'
7+
import { login } from '../support/nc-login'
8+
import { configureOpenSsl, deleteUserPfx, setAppConfig } from '../support/nc-provisioning'
9+
10+
async function prepareSignFlow(page: Parameters<typeof test>[1] extends (args: infer T) => any ? T['page'] : never, adminUser: string) {
11+
await page.goto('./apps/libresign')
12+
13+
await page.getByRole('button', { name: 'Upload from URL' }).click()
14+
await page.getByRole('textbox', { name: 'URL of a PDF file' }).fill('https://raw.githubusercontent.com/LibreSign/libresign/main/tests/php/fixtures/pdfs/small_valid.pdf')
15+
await page.getByRole('button', { name: 'Send' }).click()
16+
await page.getByRole('button', { name: 'Add signer' }).click()
17+
await page.getByPlaceholder('Account').fill(adminUser)
18+
await page.getByText('admin@email.tld').click()
19+
await page.getByRole('button', { name: 'Save' }).click()
20+
await page.getByRole('button', { name: 'Request signatures' }).click()
21+
await page.getByRole('button', { name: 'Send' }).click()
22+
await page.getByRole('button', { name: 'Sign document' }).click()
23+
await page.getByRole('button', { name: 'Define a password and sign the document.' }).click()
24+
await page.getByLabel('Enter a password').fill('Password1234')
25+
await page.getByRole('button', { name: 'Confirm' }).click()
26+
await page.getByRole('button', { name: 'Sign the document.' }).click()
27+
await page.getByLabel('Signature password').fill('Password1234')
28+
}
29+
30+
async function bootstrapAdminCertificate(page: Parameters<typeof test>[1] extends (args: infer T) => any ? T['page'] : never) {
31+
const adminUser = process.env.NEXTCLOUD_ADMIN_USER ?? 'admin'
32+
const adminPassword = process.env.NEXTCLOUD_ADMIN_PASSWORD ?? 'admin'
33+
34+
await login(page.request, adminUser, adminPassword)
35+
36+
await configureOpenSsl(page.request, 'LibreSign Test', {
37+
C: 'BR',
38+
OU: ['Organization Unit'],
39+
ST: 'Rio de Janeiro',
40+
O: 'LibreSign',
41+
L: 'Rio de Janeiro',
42+
})
43+
44+
await setAppConfig(
45+
page.request,
46+
'libresign',
47+
'identify_methods',
48+
JSON.stringify([
49+
{ name: 'account', enabled: true, mandatory: true, signatureMethods: { password: { enabled: true } } },
50+
{ name: 'email', enabled: false, mandatory: false },
51+
]),
52+
)
53+
54+
await setAppConfig(
55+
page.request,
56+
'libresign',
57+
'crl_external_validation_enabled',
58+
'1',
59+
)
60+
61+
await deleteUserPfx(page.request, adminUser, adminPassword)
62+
63+
return { adminUser }
64+
}
65+
66+
test('switches from blocked (enabled) to normal (disabled) without extra scenarios', async ({ page }) => {
67+
const { adminUser } = await bootstrapAdminCertificate(page)
68+
await prepareSignFlow(page, adminUser)
69+
70+
const signRoute = '**/ocs/v2.php/apps/libresign/api/v1/sign/**'
71+
72+
const blockedHandler = async (route) => {
73+
await route.fulfill({
74+
status: 422,
75+
contentType: 'application/json',
76+
body: JSON.stringify({
77+
ocs: {
78+
meta: {
79+
status: 'failure',
80+
statuscode: 422,
81+
message: 'Certificate revocation status could not be verified',
82+
},
83+
data: {
84+
action: 0,
85+
errors: [{
86+
code: 422,
87+
message: 'Certificate revocation status could not be verified',
88+
}],
89+
},
90+
},
91+
}),
92+
})
93+
}
94+
95+
await page.route(signRoute, blockedHandler)
96+
97+
await page.getByRole('button', { name: 'Sign document' }).click()
98+
99+
await expect(page.getByLabel('Signature password')).toBeHidden()
100+
await expect(page.getByRole('button', { name: 'Sign the document.' })).toBeHidden()
101+
await expect(page.getByRole('button', { name: 'Try signing again' })).toBeVisible()
102+
await expect(page.locator('.button-wrapper').getByText('Certificate revocation status could not be verified').first()).toBeVisible()
103+
await page.screenshot({ path: '/tmp/playwright-results/non-retriable-blocked-ui.png', fullPage: true })
104+
105+
await setAppConfig(
106+
page.request,
107+
'libresign',
108+
'crl_external_validation_enabled',
109+
'0',
110+
)
111+
112+
await page.unroute(signRoute, blockedHandler)
113+
114+
await page.route(signRoute, async (route) => {
115+
await route.fulfill({
116+
status: 200,
117+
contentType: 'application/json',
118+
body: JSON.stringify({
119+
ocs: {
120+
meta: {
121+
status: 'ok',
122+
statuscode: 200,
123+
message: null,
124+
},
125+
data: {
126+
action: 0,
127+
status: 'signed',
128+
},
129+
},
130+
}),
131+
})
132+
})
133+
134+
await page.getByRole('button', { name: 'Try signing again' }).click()
135+
await page.getByRole('button', { name: 'Sign the document.' }).click()
136+
await page.getByLabel('Signature password').fill('Password1234')
137+
await page.getByRole('button', { name: 'Sign document' }).click()
138+
139+
await expect(page.getByText('Signing is blocked until the certificate validation issue is resolved.')).toBeHidden()
140+
await expect(page.getByRole('button', { name: 'Try signing again' })).toBeHidden()
141+
await page.screenshot({ path: '/tmp/playwright-results/non-retriable-normal-ui.png', fullPage: true })
142+
})

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

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@
66
import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
77
import type { MockedFunction } from 'vitest'
88
import { setActivePinia, createPinia } from 'pinia'
9-
import { mount } from '@vue/test-utils'
9+
import { flushPromises, mount } from '@vue/test-utils'
1010
import { useSignMethodsStore } from '../../../store/signMethods.js'
1111
import type { useSignStore } from '../../../store/sign.js'
12+
import { FILE_STATUS, SIGN_REQUEST_STATUS } from '../../../constants.js'
1213

1314
type TokenMethodKey = 'smsToken' | 'whatsappToken' | 'signalToken' | 'telegramToken' | 'xmppToken'
1415

@@ -509,6 +510,106 @@ describe('Sign.vue - signWithTokenCode', () => {
509510
expect(context.signStore.setSigningErrors).toHaveBeenCalledWith(apiErrors)
510511
expect(context.loading).toBe(false)
511512
})
513+
514+
it('closes password modal when signing fails with non-retriable certificate error', async () => {
515+
const apiErrors = [{ message: 'Certificate revocation status could not be verified', code: 422 }]
516+
const context = {
517+
loading: false,
518+
elements: [],
519+
canCreateSignature: false,
520+
signRequestUuid: 'test-sign-request-uuid',
521+
signMethodsStore: {
522+
certificateEngine: 'openssl',
523+
},
524+
signatureElementsStore: {
525+
signs: {},
526+
},
527+
actionHandler: {
528+
showModal: vi.fn(),
529+
closeModal: vi.fn(),
530+
},
531+
signStore: {
532+
document: { id: 10 },
533+
clearSigningErrors: vi.fn(),
534+
setSigningErrors: vi.fn(),
535+
submitSignature: vi.fn().mockRejectedValue({
536+
type: 'signError',
537+
errors: apiErrors,
538+
}),
539+
},
540+
$emit: vi.fn(),
541+
sidebarStore: {
542+
hideSidebar: vi.fn(),
543+
},
544+
}
545+
546+
await submitSignatureCompatMethod.call(context, {
547+
method: 'password',
548+
token: '123456',
549+
})
550+
551+
expect(context.actionHandler.closeModal).toHaveBeenCalledWith('password')
552+
expect(context.signStore.setSigningErrors).toHaveBeenCalledWith(apiErrors)
553+
expect(context.loading).toBe(false)
554+
})
555+
556+
it('blocks sign CTA and shows explicit retry action when non-retriable error exists', async () => {
557+
setActivePinia(createPinia())
558+
559+
const SignComponent = await import('../../../views/SignPDF/_partials/Sign.vue')
560+
const realSign = SignComponent.default
561+
const { useSignStore } = await import('../../../store/sign.js')
562+
563+
const mountedSignStore = useSignStore()
564+
mountedSignStore.document = createSignDocument({
565+
status: FILE_STATUS.ABLE_TO_SIGN,
566+
signers: [{ me: true, status: SIGN_REQUEST_STATUS.ABLE_TO_SIGN, signRequestId: 501 }],
567+
visibleElements: [],
568+
})
569+
mountedSignStore.setSigningErrors([
570+
{ message: 'Certificate validation failed', code: 422 },
571+
])
572+
573+
const wrapper = mount(realSign, {
574+
global: {
575+
stubs: {
576+
NcButton: {
577+
template: '<button><slot /></button>',
578+
},
579+
NcDialog: true,
580+
NcLoadingIcon: true,
581+
TokenManager: true,
582+
EmailManager: true,
583+
UploadCertificate: true,
584+
Documents: true,
585+
Signatures: true,
586+
Draw: true,
587+
ManagePassword: true,
588+
CreatePassword: true,
589+
NcNoteCard: {
590+
template: '<div class="nc-note-card-stub"><slot /></div>',
591+
},
592+
NcPasswordField: true,
593+
NcRichText: {
594+
props: ['text'],
595+
template: '<span>{{ text }}</span>',
596+
},
597+
},
598+
mocks: {
599+
$watch: vi.fn(),
600+
$nextTick: vi.fn(),
601+
},
602+
},
603+
})
604+
605+
await wrapper.vm.$nextTick()
606+
await wrapper.vm.$nextTick()
607+
await flushPromises()
608+
609+
expect(wrapper.text()).toContain('Try signing again')
610+
expect(wrapper.text()).not.toContain('Sign the document.')
611+
expect(wrapper.findAll('.nc-note-card-stub')).toHaveLength(1)
612+
})
512613
})
513614

514615
describe('proceedWithSigning - Full flow with WhatsApp token', () => {

0 commit comments

Comments
 (0)