[stable32] feat: setup check implementation#7839
Draft
backportbot-libresign[bot] wants to merge 68 commits into
Draft
[stable32] feat: setup check implementation#7839backportbot-libresign[bot] wants to merge 68 commits into
backportbot-libresign[bot] wants to merge 68 commits into
Conversation
Adds trait with verifyResourceIntegrity and getErrorAndTipFromVerify methods used by multiple setup checks. Related issue: #6590 Type: Feature Checklist: - [x] Add verifyResourceIntegrity method - [x] Add getErrorAndTipFromVerify method with translated messages - [x] Declare required properties (signSetupService, urlGenerator, appManager, logger) - [x] Use IL10N for all user-facing strings Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds ExecMock, FileSystemMock, SetupCheckFunctions to mock system calls, and updates bootstrap.php to load them. Related issue: #6590 Type: Test Checklist: - [x] Create ExecMock class - [x] Create FileSystemMock class - [x] Create SetupCheckFunctions with file_exists and exec overrides - [x] Update bootstrap.php to include new files Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds JavaSetupCheck to verify Java installation, path, version and encoding. Related issue: #6590 Type: Feature Checklist: - [x] Implement ISetupCheck interface - [x] Use SetupCheckUtils trait - [x] Verify Java path exists - [x] Check Java version matches required version - [x] Check native.encoding for UTF-8 - [x] Return SetupResult with appropriate severity and translated messages - [x] Add unit tests covering all scenarios Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds JSignPdfSetupCheck to verify JSignPdf binary, integrity, Java dependency and version. Related issue: #6590 Type: Feature Checklist: - [x] Implement ISetupCheck interface - [x] Use SetupCheckUtils trait - [x] Verify JSignPdf path is configured and exists - [x] Verify Java is available - [x] Check JSignPdf version against required version - [x] Return SetupResult with appropriate severity and translated messages - [x] Add unit tests covering all scenarios Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds PDFtkSetupCheck to verify PDFtk binary, integrity, Java dependency and version. Related issue: #6590 Type: Feature Checklist: - [x] Implement ISetupCheck interface - [x] Use SetupCheckUtils trait - [x] Verify PDFtk path is configured and exists - [x] Verify Java is available - [x] Check PDFtk version matches required version - [x] Return SetupResult with appropriate severity and translated messages - [x] Add unit tests covering all scenarios Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds PopplerSetupCheck to verify pdfsig and pdfinfo utilities (optional). Related issue: #6590 Type: Feature Checklist: - [x] Implement ISetupCheck interface - [x] Check pdfsig installation and version - [x] Check pdfinfo installation and version - [x] Return info severity when missing (optional check) - [x] Return success when both tools are working - [x] Add unit tests covering all scenarios Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds ImagickSetupCheck to verify imagick PHP extension (optional). Related issue: #6590 Type: Feature Checklist: - [x] Implement ISetupCheck interface - [x] Check if imagick extension is loaded - [x] Return info severity when not loaded (optional check) - [x] Return success when loaded - [x] Add unit tests covering both states Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds CertificateEngineSetupCheck to process certificate engine configuration results. Related issue: #6590 Type: Feature Checklist: - [x] Implement ISetupCheck interface - [x] Use CertificateEngineFactory to get current engine - [x] Process ConfigureCheckHelper results from engine - [x] Convert to SetupResult with aggregated messages and tips - [x] Handle engine not defined (error) - [x] Add unit tests covering success, warning, error, and mixed results Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Registers all six setup check classes in Application.php. Related issue: #6590 Type: Feature Checklist: - [x] Add registerSetupCheck for JavaSetupCheck - [x] Add registerSetupCheck for JSignPdfSetupCheck - [x] Add registerSetupCheck for PDFtkSetupCheck - [x] Add registerSetupCheck for PopplerSetupCheck - [x] Add registerSetupCheck for ImagickSetupCheck - [x] Add registerSetupCheck for CertificateEngineSetupCheck Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Adds @deprecated annotation to ConfigureCheckService pointing to the new individual setup checks. Related issue: #6590 Type: Maintenance Checklist: - [x] Add @deprecated tag with version 13.0.4 - [x] Mention replacement classes (JavaSetupCheck, JSignPdfSetupCheck, etc.) Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
This refactoring consolidates and organizes test classes related to SetupChecks, while also applying standardization improvements and best practices in the main codebase. - Move `ExecMock` and `FileSystemMock` classes to the new `Mock` namespace under `tests/php/Unit/SetupCheck/` and create a `functions.php` file to hold the overridden global functions (`file_exists`, `exec`). - Update `bootstrap.php` to load mocks from the new location and remove the obsolete `SetupCheckFunctions.php` file. - Apply code style fixes (indentation, spacing, trailing commas) to all files touched by the refactoring. Related issue: #6590 Type: Maintenance Checklist: - [x] Move mocks to subdirectory and update bootstrap - [x] Apply code style fixes Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
This commit applies automated code style fixes using `composer cs:fix` and addresses issues identified by Psalm static analysis across the SetupCheck classes and their related tests. The changes include: - Normalizing indentation (tabs vs spaces) and adding missing trailing commas in argument lists across all SetupCheck classes and `SetupCheckUtils.php`. - Fixing the formatting of the `@deprecated` comment in `ConfigureCheckService.php`. - Adding `#[\Override]` attributes to all methods implementing `ISetupCheck`. Related issue: #6590 Type: Maintenance Checklist: - [x] Run `composer cs:fix` to apply coding standards - [x] Address Psalm static analysis findings - [x] Add `#[\Override]` attributes to interface methods - [x] Fix deprecation comment formatting Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
- Add `@var list<string>` type annotation to `$output` variable in `JavaSetupCheck.php` to satisfy Psalm's type checking. - Improve empty output check in `PDFtkSetupCheck.php` by verifying `empty($versionOutput)` in addition to the return code, preventing false positives when the command returns an empty string but a zero exit code. These changes enhance code quality and the reliability of the setup checks. Related issue: #6590 Type: Bug fix Checklist: - [x] Add type annotation for Psalm in JavaSetupCheck - [x] Fix empty output check in PDFtkSetupCheck Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
The previous fix for empty output in PDFtkSetupCheck changed the error message from "PDFtk binary is invalid" to "Failure to check PDFtk version". This commit updates the corresponding test assertion to align with the new behavior. Related issue: #6590 Type: Test Checklist: - [x] Update assertion in PDFtkSetupCheckTest Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Move ExecMock and FileSystemMock from Unit/SetupCheck to the Mock/ directory to centralize test doubles and follow standard testing practices. Consolidate function overrides (file_exists, exec) from multiple test files into a single functions.php file to improve maintainability and reduce duplication. This refactoring addresses the reviewer's request by: - Placing mocks in the proper namespace (OCA\Libresign\Tests\Mock) before the function overrides, aligning with PHP namespace fallback mechanics. - Centralizing mock logic in dedicated, reusable classes. - Making test setup cleaner and more maintainable. Related issue: #6590 Type: Test Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Refactor the libresign:configure:check command to consume the new setup check classes via ISetupCheckManager, instead of the deprecated ConfigureCheckService. The command now: - Filters only checks from the LibreSign namespace - Displays friendly resource names (e.g. "Java" instead of FQCN) - Respects the --sign (system category) and --certificate (security) filtering options Related issue: #6590 Type: Feature Checklist: - [x] Replace ConfigureCheckService with ISetupCheckManager - [x] Add namespace filtering for LibreSign checks - [x] Improve resource name display - [x] Preserve option-based filtering by category Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
…ultService Centralize the logic for filtering and formatting LibreSign setup check results using the new SetupCheckResultService. This service internally uses the ISetupCheckManager and the individual SetupCheck classes, resolving the deprecation of the old ConfigureCheckService. Related issue: #6590 Type: Refactor Checklist: - [x] Update `Check` command to use `SetupCheckResultService` - [x] Update `AdminController` to use `SetupCheckResultService` - [x] Remove direct dependency on `ConfigureCheckService` from controllers - [x] Create new `SetupCheckResultService` with methods for formatted results - [x] Update `@psalm-type LibresignConfigureCheck` to include 'info' status - [x] Ensure legacy API output (`getLegacyFormattedChecks`) remains unchanged Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
… data The API endpoint GET /api/v1/admin/certificate was returning the Organizational Unit (OU) field as an array when the certificate was already generated. This violated the OpenAPI schema which expects a string, causing test failures in AdminControllerTest. This change ensures that for the OU field, any array value is always converted to a comma-separated string. The CA ID is preserved when the certificate is generated, and removed only when it is not (matching the original behavior of the filter). Although discovered during refactoring of ConfigureCheckService, this issue was pre-existing and is fixed here following the "leave it better than you found it" principle. Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Add comprehensive test coverage for the new SetupCheckResultService, which formats and filters ISetupCheck results for LibreSign. The tests verify: - Filtering of checks by LibreSign namespace - Correct formatting of results with category - Legacy format without category - Severity mapping (warning → info, etc.) Related issue: #6590 Type: Test Checklist: - [x] Add tests for getFormattedChecks filtering - [x] Add tests for getLegacyFormattedChecks format - [x] Add tests for severity mapping - [x] Use data providers for multiple scenarios - [x] Mock ISetupCheckManager and SetupResult Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
The toArray() method in AEngineHandler now returns the Organizational Unit as a string (or null) instead of an array, to match the API contract. This commit updates the test data provider to expect the correct new return types. Related issue: #6590 Type: Tests Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Signed-off-by: Guilherme Carvalho <dev.gcarvalho@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
The certificate API now returns the Organizational Unit value as a single comma-separated string (matching the OpenAPI schema) instead of an array. Update the affected scenarios to assert the value as a string. Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Map each binary check to its stable lowercase resource, and expand the certificate engine into the granular resources produced by the engine handler instead of the single aggregated CertificateEngineSetupCheck result (kept for the Nextcloud setup panel). Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
…te message Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
…grade message Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
…grade message Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
…uct name Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
…essage Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
…essage Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
Co-authored-by: Vitor Mattos <vitor@php.rio> Signed-off-by: Yves César Amorim de Azevedo <48072419+YvesCesar@users.noreply.github.com>
…ct names Signed-off-by: YvesCesar <yvesamorim73@gmail.com>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio>
Signed-off-by: Vitor Mattos <vitor@php.rio> [skip ci]
67380e7 to
1043359
Compare
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
1043359 to
5ee3496
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #7070
Warning, This backport's changes differ from the original and might be incomplete⚠️
Todo
Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.