Repository navigation
feat(esign): bind signing links to their recipient and record consent - #35
Merged
Merged
Conversation
A link sent to one recipient now carries an email_sig parameter: an HMAC-SHA256 of the doctype, the document name and the normalized email, keyed with a secret derived from the site's encryption key. The signing page and accept check it in constant time, after the share key, and the audit trail records the email as verified via the recipient link (signer_email_verified, verified_via, verified_email). A new Web Form setting, Require Verified Email, signs only through such a link: a missing or tampered binding is refused with a clear message, the email field is pre-filled from the link and read-only, and accept writes and records the bound address whatever the form submits. Send for eSign then sends each recipient in To their own email and link, and refuses CC, BCC and Send me a copy. Without the setting, a single recipient's link is bound only when nobody else gets the message. The signer's name is now only what they typed into the Signer Name Field. The fallback to the User whose email matched is removed; without a name field, a logged-in user signing as themselves is still named after their own User. The signing page has a required consent checkbox, "I agree to sign this document electronically", whose text a web form can replace (Consent Text). accept refuses to sign without it, and the consent text and time are recorded in the audit data, on the timeline card and in the notification email, which also show "Email verified via recipient link". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F
…ng flow - frappe's own web form accept is whitelisted for guests as well, and it asks the web form's has_web_form_permission before saving. The eSign override answered "write" from the share key, so a guest with any signing link could post there and update the document without the consent, without the recipient binding and with any email under Require Verified Email. The share key now grants a write only inside this app's accept. - The whitelisted get_esign_link returned a bound link for any address to anyone with email permission, letting them sign as that address, verified. Over HTTP it now carries the email parameter but no binding; Jinja and Python callers that pass the document still get it. - Send for eSign stored the bound link in the Communication, which every reader of the document can open, and returned it to the sender. Both now carry the link without email_sig; only the email to the recipient has it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F
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.
What changes
Signer attribution that holds up under US ESIGN / UETA: the signing link can vouch for the recipient's email, a web form can require that, the name recorded is only the one the signer typed, and the signer gives explicit consent to sign electronically.
Recipient-bound links. A link sent to one recipient carries
email_sig, an HMAC-SHA256 ofdoctype ‖ document name ‖ normalized email(trimmed, lower-cased), next to the existingkeyandemail.get_esign_link(..., recipient_email)and a single-recipient Send for eSign add it. The signing page (get_context) andaccept(from the referrer, like the key) check it withhmac.compare_digest. The share-key check is unchanged and still runs first.Require Verified Email (new Web Form Check, in the Signer section). With it on:
email_sig("This document can only be signed through the link emailed to its signer…") and a changed one ("This signing link has been changed, or it was not sent for this document…"). Both render as frappe's 403 message page.acceptrefuses the same, before anything is written, and also refuses a new document from the web form, which no link can name;Email) is pre-filled from the bound address and marked read-only inweb_form_doc.acceptwrites the bound address into that field whatever was submitted, and records it;signer_email_verified: true,verified_via: "recipient link"andverified_email.With it off, behaviour is as before, except that a valid binding is still checked and recorded:
signer_email_verifiedis true when the recorded email is the bound one. If the signer typed a different address, the bound one is kept asverified_emailand shown as "Link sent to …, verified via recipient link".Signer name. The fallback that named the signer after the User whose email matched is removed. The name is only what the signer typed into the Signer Name Field. Without that field, a logged-in user whose signer email is their own login is named after their own User. Nobody else is named.
Consent. The signing page has a required checkbox, "I agree to sign this document electronically.", whose text a web form can replace (new Consent Text, Small Text). The client checks it and posts
esign_consent.acceptrefuses to sign without it ("To sign, tick the box: …"). The server-side text (the web form's override or the translated default) and the server time it was received are recorded asconsent_textandconsent_timestamp.Display. The timeline card and the notification email's audit table show "Email verified via recipient link" (or "Link sent to …") and the consent with its time. The signing page shows "Signing as , verified by your email link" when the link is bound.
Multi-recipient Send for eSign: one link per recipient
With Require Verified Email on, Send for eSign sends one email per recipient in To, each with that recipient's own bound link, to that recipient alone. I picked this over refusing, because:
The same reasoning means a bound link must never reach anyone but its recipient. Anyone holding it could sign as that recipient and be recorded as verified. So with the setting on, CC, BCC and Send me a copy are refused, with a message saying to put every signer in To. With the setting off, nothing is refused: a single recipient's link carries
emailas before, but getsemail_sigonly when nobody else (CC, BCC, Send me a copy) gets the message. Each per-recipient email is recorded as its own Communication. The response keepscommunicationandesign_link(the first) and addscommunications. The dialog's link preview says when a web form sends per-recipient links.Security reasoning
k = HMAC-SHA256(site encryption_key, "esign_webforms:recipient-email-binding"). The key is secret, unique to the site and separate from other uses, and the encryption key itself is never used directly. frappe createsencryption_keyon first use (frappe.utils.password.get_encryption_key). Nothing new to configure.hmac.compare_digeston bytes (a non-ASCII value cannot raise). A missing, shortened, changed or moved binding getsNone.get_esign_linkover HTTP still needs email permission but returns no binding (see Review fixes).acceptoverwrites the email field with the bound address, and the consent text recorded is the server's, not anything the client sent.Upgrade impact
esign_require_verified_email, default off;esign_consent_text), synced on migrate. Two existing custom fields get a newinsert_after.acceptrefuses a post withoutesign_consent. The signing page sends it once assets are rebuilt (bench build/bench update). A browser still running a cached old bundle, or any custom client posting toacceptdirectly, gets "To sign, tick the box…" until it sendsesign_consent=1.&email_sig=+ 64 hex). A single-recipient send with CC/BCC/Send me a copy still pre-fillsemailbut is not verified._collect_audit_dataand the README. Merging this one makes fix(audit): name a signer after a User only for their own login #34 unnecessary.Verification
frappe-test --ci: 144 tests pass, coverage 98.9 %, testmap complete, composition and ty clean.prek run --all-files: clean.TestEmailBinding): valid binding; the HMAC construction and per-site key; normalization; tampered (another email, another document, changed, shortened, non-ASCII) and missing parts;compare_digestused.TestVerifiedSigningPage): bound prefill and the verified note; unbound not called verified; required setting refusing a missing or tampered binding and a missing share key; field locked, including a custom Signer Email Field; consent checkbox with default and escaped custom text.TestVerifiedAccept): consent required (0,"0","",None); consent text and time recorded; the required setting refusing a missing or tampered binding and a new document; submitted email overridden server side, including via the Signer Email Field; recorded verified without the setting; a differing typed email recorded unverified, withverified_email.TestSignerName): no User-name fallback for a guest, a bound link or an empty name field; a logged-in user only as themselves.TestSendEsignEmail): single-recipient bound link; no binding when CC/BCC/copy are present; per-recipient emails with their own bound links; refusals for CC/BCC/copy and bad addresses before anything is sent.make_email_binding/get_bound_email/build_esign_link, or assert behaviourdeveloplacks, so they fail ondevelop.frappe-shots --recreate-site: signing page (ticked consent), signature pad, audit trail (verified email and consent rows) and signer settings (Require Verified Email, Consent Text) regenerated. Send for eSign was unchanged.Review fixes (second commit)
An adversarial review found three ways around the binding and the consent. All are fixed with regression tests that fail on the first commit (6 failures and 1 error in
test_web_form, 3 failures intest_esign):frappe.website.doctype.web_form.web_form.acceptis whitelisted for guests and askshas_web_form_permission(..., "write")before saving. The eSign override answered that from the share key, so a guest with any signing link could post there (key in the query or in the Referer) and update the document with no consent, no binding, and any email even under Require Verified Email. The share key now grants a write only inside this app'saccept(a request flag set around its own permission check); reads are unchanged. Test:test_frappes_own_accept_cannot_skip_the_signing_checks,test_share_key_writes_only_through_esign_accept.get_esign_linkover HTTP minted bound links for any address. Anyone with email permission could fetch a link bound toalice@…without emailing Alice and sign as her, recorded as verified. Over HTTP (JSON or dictdoc) the link now carriesemailbut neveremail_sig; Jinja and Python callers that pass the Document still get the binding. Test:test_request_payload_never_gets_a_binding.esign_link. Both now carry the link withoutemail_sig(strip_email_binding); only the email itself has it. Test:test_bound_link_reaches_only_the_recipient.Also checked, no change needed: HMAC forgery or replay across documents and addresses (doctype, name and normalized email are all covered); timing (
compare_digest, with only non-secret length checks first); consent skipping throughaccept(enforced server side); notification and timeline escaping of the new values; the desk dialog (it never used the returnedesign_link).README (Send for eSign, Links in templates, How It Works, a new Limitations entry on where a Jinja-bound link ends up, such as a Notification's Communication) and docs/README.md are updated. No visible UI changed, so the screenshots stay as they are.
Review verification:
frappe-test --ci149 tests pass, coverage 98.7 %, testmap, composition and ty clean;prek run --all-filesclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01SvdEeMiewirZ3ifFoUix5F