Skip to content

[stable35] fix(rejection): notify the next signing order only after the commit - #9037

Closed
backportbot-libresign[bot] wants to merge 4 commits into
stable35from
backport/8912/stable35
Closed

backportbot-libresign[bot] wants to merge 4 commits into
stable35from
backport/8912/stable35

Conversation

@backportbot-libresign

@backportbot-libresign backportbot-libresign Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Backport of #8912

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

Todo

  • Review and resolve any conflicts
  • Review and verify the backported changes
  • 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.

tnalbant and others added 4 commits October 6, 2026 00:58
A rejection that lets the workflow continue releases the next signing
order inside its transaction. releaseNextOrder() persisted ABLE_TO_SIGN
and dispatched the notification in the same method, so on an envelope
with several documents the signers released for an earlier document
were already notified when a later document failed and rolled the
transaction back: the message was delivered for an activation that no
longer existed.

activateSignersForOrder() now only persists the state and returns the
signers it activated. activateNextOrder() exposes that for callers that
run inside a transaction and notifyActivatedSigners() dispatches the
notifications, which SignatureRejectionService calls after commit().
releaseNextOrder() keeps activating and notifying in one step, so the
non-transactional callers are unchanged, and a failing listener can no
longer undo a recorded rejection.

Signed-off-by: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com>
Every signer/identify-method notification is isolated: a failed delivery is logged with the sign request id and the identify method, and the remaining activated signers are still notified instead of being skipped.

The post-commit catch in SignatureRejectionService now catches recoverable exceptions (\Exception) instead of \Throwable, so programming errors are not converted into a logged delivery failure; the log includes the sign request id.

Adds regression tests for both behaviours.

Signed-off-by: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com>
Keep recoverable notification failures in SequentialSigningService,
extract per-signer delivery, and preserve rollback assertions.
Adapt identification lookup isolation from PR #8948 while keeping
rejection event ordering for that follow-up PR.

Co-authored-by: André Maia <andrefnkmm@gmail.com>

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
fix(rejection): dispatch rejection event before signer notifications

Return activated signers after committing the rejection and dispatch

SignatureRejectedEvent before attempting their notifications. Preserve

event delivery when a post-commit notification error propagates.

Adapt the event-ordering correction from PR #8948 and strengthen the

successful-order and notification-error regression assertions.

Co-authored-by: André Maia <andrefnkmm@gmail.com>

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

[skip ci]

Copy link
Copy Markdown
Member

Closing this backport because #8912 is not independently applicable to stable35.

The fix in #8912 assumes the rejection "continue" behavior introduced on main by #8423/#8706, including SignatureRejectionService::releaseNextSigningOrders() and the call into SequentialSigningService. stable35 still uses the previous rejection policy model and does not contain that flow.

The automatic cherry-pick therefore produced an invalid partial backport: it references $activatedSigners and $this->sequentialSigningService without the prerequisite behavior being present, and omitted the corresponding rejection-service regression tests.

Backporting #8912 alone would effectively require backporting the unreleased rejection-policy/continue feature chain, which is outside the scope of this bugfix. The same prerequisite is absent from stable34 and stable33, which is why the bot correctly found no applicable changes there.

No backport of #8912 is needed for stable35/stable34/stable33.

@vitormattos vitormattos closed this Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants