feat: integrate pdf-signature-validator for native validation - #7596
Conversation
There was a problem hiding this comment.
Pull request overview
Integrates the libresign/pdf-signature-validator Composer package into LibreSign to perform native (pure-PHP) PDF signature validation, and wires its metadata/validation outputs into LibreSign’s existing certificate-chain payload.
Changes:
- Add
PdfSignatureValidationServiceto run native validation and translate package results/reasons into LibreSign’s expected format. - Update
Pkcs12Handlerto enrich signature/certificate chain data using native extractor/validator outputs (replacing the previouspdfsig-based Poppler path). - Update install checks and unit tests to reflect the new validation flow and dependencies.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php | New unit tests for mapping native validator results into LibreSign payload shape. |
| tests/php/Unit/Service/IdentifyMethod/PasswordTest.php | Updates test construction for Pkcs12Handler after constructor dependency changes. |
| tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php | Adds tests for extractor metadata parsing and native validation integration in getCertificateChain(). |
| lib/Service/Signature/PdfSignatureValidationService.php | New service wrapping PdfSignatureValidator and translating results/reasons with l10n. |
| lib/Service/Install/ConfigureCheckService.php | Removes the pdfsig setup check, keeping pdfinfo as Poppler fallback check. |
| lib/Handler/SignEngine/Pkcs12Handler.php | Injects native extractor/validation service and uses them to enrich chain leaf data. |
| lib/Controller/FileController.php | Refactors preview mime-fallback redirects and adds broader error handling/logging. |
| composer.json | Adds libresign/pdf-signature-validator dependency. |
| composer.lock | Locks the new package and updates dependency hash. |
4db8ad1 to
4595c3e
Compare
4595c3e to
6015855
Compare
7341797 to
ac44e0c
Compare
|
/backport to stable35 |
|
/backport to stable34 |
|
/backport to stable33 |
ac44e0c to
ca66efc
Compare
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
The service's validateFromResource() already returns properly formatted arrays with id, label, isValid, and reason. The localize* methods were unnecessarily converting numbers back to enums and then back to numbers. Now Pkcs12Handler uses the validation results directly without the circular conversion, eliminating code duplication and improving clarity. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
- FileController::fetchPreview: add inline type assertions for RedirectResponse
to specify exact status code (303) when returning null-checked values
- PdfSignatureValidationService: update docblocks for validateFromResource,
validateFromString, and mapValidationResults to include the 'raw' field
- Pkcs12Handler::extractNativeSignatureMetadata: change return type from
list<array> to array<int, array{...}> to match actual implementation
All Psalm errors resolved, unit tests still passing.
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Update constructor arguments to match the new signature which includes PdfSignatureValidationService and PdfSignatureExtractor, and removes the no-longer-used TempManager. Also use real instance of PdfSignatureExtractor instead of mock since the class is final and cannot be mocked. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
phpseclib is already vendored and scoped in the 3rdparty submodule under OCA\Libresign\Vendor\phpseclib3. Declaring it as a direct composer dependency caused roave/security-advisories to block the insecure 3.0.52 version locked in composer.lock. Remove phpseclib/phpseclib, paragonie/constant_time_encoding and paragonie/random_compat from composer.json require and composer.lock. Use "provide" to satisfy the transitive dependency declared by libresign/pdf-signature-validator without installing the package. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
The libresign/pdf-signature-validator package is moved from libresign's own composer dependencies into the 3rdparty scoped vendor directory so that php-scoper can prefix both its classes and its phpseclib3 references with OCA\Libresign\Vendor\. - Remove libresign/pdf-signature-validator from composer.json + lock - Remove phpseclib/phpseclib 'provide' declaration (no longer needed) - Update all use statements: LibreSign\PdfSignatureValidator\... => OCA\Libresign\Vendor\LibreSign\PdfSignatureValidator\... - Update 3rdparty submodule ref to include the scoped package Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
ca66efc to
e9b887a
Compare
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
|
/backport to stable34 |
|
/backport to stable32 |
|
/backport to stable33 |
|
The backport to # Switch to the target branch and update it
git checkout stable35
git pull origin stable35
# Create the new backport branch
git checkout -b backport/7596/stable35
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 602c2215 58718e80 4e6236f2 a37d8e32 12139471 8723bcee 3dafc5ba 4751b240 6f3970f1 adaadf72 01295284 0dff4296 f45ff2b4 18a10a98 bebc11e0 8a078afb 8f8195d7 739ec4a2 7ea10ee5 6ba971e9 361675d6 aaa6617c 04ba7304 e9b887ab ad89ce51 f31f34fc 39192a05 adcb25dc 3574b2b6 8c9f9439
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/7596/stable35Error: Failed to clone repository: Failed to create working tree: Preparing worktree (new branch 'backport/7596/stable35') Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
Objective:
Remove necessity of Java when validate signatures of a document.