Skip to content

fix(validation): base the validation summary on applied signatures only - #9009

Open
maia-andre wants to merge 2 commits into
LibreSign:mainfrom
maia-andre:fix/validation-summary-unsigned-8967
Open

maia-andre wants to merge 2 commits into
LibreSign:mainfrom
maia-andre:fix/validation-summary-unsigned-8967

Conversation

@maia-andre

Copy link
Copy Markdown
Contributor

Resolves: #8967

📝 Summary

The validation page showed "This document is valid" for a document nobody has signed yet, and for an envelope where nobody signed.

getDocumentValidationSummary() in src/views/Validation.vue only returned "No digital signatures were found in this document" when the signer list was empty. A pending signer carries no signature_validation, modification_validation or document_modification_state, so none of the error or warning checks matched and the summary fell through to "valid".

The summary now considers only the signers who have signed, meaning a signer with a signing date or with signature validation data:

  • nobody has signed → the existing "No digital signatures were found in this document" (info) note, whatever the number of signers;
  • at least one signature → the current checks, run on the signed signers only.

The same function feeds the header note for single files and envelopes, so both are covered. No new strings.

🧪 How to test

  1. Request a signature on any PDF from one signer.
  2. Before anybody signs, open /apps/libresign/p/validation/<uuid> (logged out is fine).
  3. The note reads No digital signatures were found in this document. Before this fix it read "This document is valid".
  4. Repeat with an envelope of two files: the same note appears.
  5. Sign the document and reopen the validation page: it reads This document is valid as before.

🎨 UI changes

  • Validation summary note computed from applied signatures only
  • Screenshots before/after
🏚️ Before 🏡 After
Unsigned document, before Unsigned document, after
Envelope nobody signed, before Envelope nobody signed, after

Signed document, unchanged:

Signed document still valid

  • Components, Unit (with vitest) and/or e2e (with Playwright) tests added - Required

Vitest cases in src/tests/views/Validation.spec.ts: nobody signed, envelope where nobody signed, only signed signers are evaluated, and a signer with a signing date but no cryptographic details. Local runs: npx vitest run src/tests/views/Validation.spec.ts src/tests/views/DocumentValidationLogic.spec.ts (116 passed), npm run ts:check, eslint on the changed files.

Backport

The same logic is on stable33, stable34 and stable35, and the commit cherry-picks cleanly onto them. stable32 has older code that sets "This document is valid" unconditionally, so this change does not apply there as is.

✅ Checklist

🤖 AI (if applicable)

  • The content of this PR was partially or fully generated using AI

The validation page showed "This document is valid" for a document that
nobody has signed yet, and for an envelope where nobody signed. The
summary only returned "No digital signatures were found in this
document" when the signer list was empty; a pending signer carries no
signature data, so none of the error or warning checks matched and the
summary fell through to "valid".

The summary now considers only the signers who have signed (a signing
date or signature validation data). When nobody has signed, it shows
the existing "No digital signatures were found" note; otherwise the
current checks run on the signed signers only.

Fixes LibreSign#8967

Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: André Maia <andrefnkmm@gmail.com>
@maia-andre
maia-andre requested a review from a team as a code owner October 4, 2026 17:41
@YvesCesar
YvesCesar self-requested a review October 17, 2026 17:06

@YvesCesar YvesCesar 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.

@maia-andre

Can you check this review?

Comment thread src/views/Validation.vue Outdated
}

function hasAppliedSignature(signer: ValidationDisplaySigner) {
return Boolean(signer.signed) || signer.signature_validation !== undefined

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.

hasAppliedSignature doesn't consider status, but isCurrentSignerSigned (line 1064) does: !!signer?.signed || Number(signer?.status) === SIGN_REQUEST_STATUS.SIGNED.

CertificateSignersMergeService sets status = 2 on every signer read from the PDF (line 195), even when the signature has no signingTime and no certificate chain could be extracted (then there is no signature_validation either). In that case the summary says "No digital signatures were found" while the signer list below shows the signer as signed.

Could you include the status in the predicate, so the file uses a single definition of "signed"?

function hasAppliedSignature(signer: ValidationDisplaySigner) {
        return Boolean(signer.signed)
                || Number(signer.status) === SIGN_REQUEST_STATUS.SIGNED
                || signer.signature_validation !== undefined
}

And please add a test with { status: 2 } (no signed, no signature_validation) to cover it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @YvesCesar, confirmed: CertificateSignersMergeService sets status = 2 for every signer read from the PDF, so a signature without signingTime or an extractable chain was dropped from the summary. In d02d24b hasAppliedSignature now also accepts Number(signer.status) === SIGN_REQUEST_STATUS.SIGNED, matching isCurrentSignerSigned, and I added a { status: 2 } case (no signed, no signature_validation) to the summary tests.

Comment thread src/views/Validation.vue Outdated

function getDocumentValidationSummary(validationDocument: ValidationDocumentState): DocumentValidationSummary {
const signers = getValidationDocumentSigners(validationDocument)
// Pending signers carry no signature data, so they must not count towards a "valid" verdict

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.

Could you drop this comment? There are no other inline comments in this file, and the function name plus the PR description already explain the intent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in d02d24b, the comment is gone.

…e summary

Signed-off-by: André Maia <andrefnkmm@gmail.com>
@maia-andre
maia-andre requested a review from YvesCesar October 17, 2026 21:55

This branch has not been deployed

No deployments
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.

Validation page says "This document is valid" for a document with no signature

2 participants