Skip to content

[stable33] feat: setup check implementation#7840

Draft
backportbot-libresign[bot] wants to merge 68 commits into
stable33from
backport/7070/stable33
Draft

[stable33] feat: setup check implementation#7840
backportbot-libresign[bot] wants to merge 68 commits into
stable33from
backport/7070/stable33

Conversation

@backportbot-libresign

@backportbot-libresign backportbot-libresign Bot commented Jun 28, 2026

Copy link
Copy Markdown

Backport of #7070

Warning, This backport's changes differ from the original and might be incomplete ⚠️

Todo

  • Review and resolve any conflicts
  • Amend HEAD commit to remove the line stating to skip CI

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

guilhermercarvalho and others added 30 commits June 28, 2026 20:20
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>
YvesCesar and others added 23 commits June 28, 2026 20:20
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]
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
@vitormattos vitormattos force-pushed the backport/7070/stable33 branch from 39a3fba to eade2ea Compare June 29, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 0. Needs triage

Development

Successfully merging this pull request may close these issues.

3 participants