Skip to content

fix(signing): preserve nullable code request contracts - #9454

Merged
vitormattos merged 2 commits into
LibreSign:mainfrom
lfals:fix/sign-code-parameter-contracts
Nov 15, 2026
Merged

vitormattos merged 2 commits into
LibreSign:mainfrom
lfals:fix/sign-code-parameter-contracts

Conversation

@lfals

@lfals lfals commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The signing-code endpoints accept null for identifyMethod and idDocApproval, but generated Hey API types reject it. The extractor emits nullable: true with an enum that excludes null. In OpenAPI 3.0, nullable does not override the enum constraint.

The PHP methods also require positional identifyMethod and signMethod arguments despite the generated request bodies describing them as optional.

Solution

  • Keep the nullable PHP annotations and document omission explicitly. Preserve identifyMethod: 'account' | 'email' | null and idDocApproval: 'true' | null.
  • Give identifyMethod and signMethod explicit null defaults in both code-request methods.
  • Apply a Composer-managed patch to the pinned OpenAPI Extractor. Nullable enums include null; non-nullable enums remain unchanged, and existing null entries are not duplicated.
  • Regenerate the OpenAPI specifications and TypeScript types.
  • Add controller regressions for omitted/null methods and non-approval flags, plus schema and extractor regressions.

Only the literal string "true" selects the ID-document approval flow. Missing methods still reach the existing validation/error path; this does not choose a signing method automatically or bypass signing validation.

Tests and evidence

  • Before: the new omitted-argument controller cases failed with two ArgumentCountError errors. The SDK compilation fixture rejected null in both identifyMethod fields and in idDocApproval.
  • After: the full SignFileControllerTest passed in an isolated PHPUnit bootstrap: 19 tests, 67 assertions. This used the current checkout's classes and installed OCP dependencies, without a bootstrapped Nextcloud server.
  • vendor-bin/phpunit/vendor/bin/phpunit -c tests/php/openapi/phpunit.xml: 16 tests, 168 assertions.
  • Composer patch reinstall/application and OpenAPI/TypeScript generation: passed.
  • npm run ts:check: passed.
  • Hey API 0.99.0 generation and strict TypeScript compilation: passed for omission, explicit null, and supported strings. Negative cases reject boolean approval flags, "false", and unsupported identification methods.
  • PHP syntax checks, focused PHP CS Fixer, and git diff --check: passed.
  • Composer validation passed with the existing missing-license metadata warning for the extractor's bin package.

Known limits

  • No full Nextcloud HTTP/middleware integration run, full backend suite, or Psalm run was performed.
  • The Hey API fixture is a local experiment, not a committed SDK or CI test.
  • The extractor patch needs maintenance until an upstream release provides the same behavior. Its checksum and dependency versions are locked.

Compatibility and related work

Based directly on main at c0e2e302605ed51ff87931ec47ff69c80db2d331, independently of #9453 and the policy changes.

The patch also adds null to the currently nullable IdDocsUploadErrorResponse.type enum when regenerating from this base. It does not change that endpoint's PHP/runtime behavior. #9449 removes that field's nullability; after combining the PRs and regenerating, the patch leaves its non-nullable enum unchanged. #9451's error-handling changes are not included.

Reference: OpenAPI 3.0.3 Schema Object.

@lfals
lfals requested a review from a team as a code owner November 13, 2026 02:39

@vitormattos vitormattos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lfals, please address the integration issue below and request another review once the updated branch and CI are ready.

This PR needs to be updated against the current main before merging.

PR #9449 has already made IdDocsUploadErrorResponse.type non-nullable, but this branch's generated openapi-full.json still includes null in that enum.

The current main also contains #9450's required policy value contract and #9457's strict OpenAPI/TypeScript generation tests.

Please resolve the conflicts and regenerate the OpenAPI specifications and TypeScript types from the combined PHP source, ensuring that:

  • The ID-document error type remains non-nullable.
  • The signing-code fields retain the intended nullable enums.
  • The policy contracts introduced by #9450 remain intact.
  • Both OpenApiGenerationTest.php and OpenApiSigningCodeContractTest.php remain registered in the OpenAPI PHPUnit suite.

Then rerun composer run openapi, the OpenAPI PHPUnit suite, npm run ts:check, and the PR CI checks.

This is an integration requirement, not a defect in the nullable-enum fix itself.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
@lfals
lfals force-pushed the fix/sign-code-parameter-contracts branch from 4b79c6a to e651e9e Compare November 14, 2026 22:47
Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
@vitormattos
vitormattos merged commit 9f99fd7 into LibreSign:main Nov 15, 2026
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants