Updated SigningUsersCsv view to send emails to signers#6
Conversation
chris-adam
force-pushed
the
SE-235/signers_emailing
branch
from
84d8a9f to
dc207de
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds registry-backed signing email content and paraphéo URL settings, a new interactive signing-users template, refactors the SigningUsersCsv view to support CSV export and email sending, and wires registry accessors and install-time defaults; also updates test runtime dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as Browser/UI
participant View as SigningUsersCsv
participant Registry as ConfigRegistry
participant Mail as MailHost
User->>Browser: Open signing users page
Browser->>View: GET -> render index template
View->>Registry: (read) paraphéo_url, signing_users_email_content
View-->>Browser: template with users data (includes duplicates)
User->>Browser: Select users + choose action (download or send)
Browser->>View: POST action + selected IDs
View->>View: _get_selected_userids()
alt action == send_emails
View->>Registry: get_registry_signing_users_email_content()
Registry-->>View: email template
loop per selected user
View->>Mail: send_email(to, personalized_content)
Mail-->>View: delivery status
end
View-->>Browser: redirect with send results
else action == download_csv
View->>View: _download_csv() -> build CSV response
View-->>Browser: CSV file response
end
Browser-->>User: show results or start download
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/imio/esign/browser/templates/signing_users.pt`:
- Around line 198-382: The JS currently sends send_emails via GET
(window.location.href) which breaks CSRF and URL limits; change
performAction/send confirmation flow to submit the existing POST form
("users-form") instead: gather selected IDs from getSelectedUserIds(), populate
or update a hidden input named "selected_users" with the JSON-encoded array, set
the form.action (or include an "action" hidden field) to "send_emails", ensure
the Plone authenticator token is present as a hidden "_authenticator" input in
the template, and call form.submit() (replace the GET usage in performAction and
the confirmEmailBtn click handler with this POST submission).
In `@src/imio/esign/browser/views.py`:
- Around line 449-458: The _get_selected_userids method should guard json.loads
against malformed input: wrap the json.loads(selected) call in a try/except that
catches json.JSONDecodeError (and optionally ValueError) and falls back to
returning an empty list (or the comma-split fallback) instead of raising; ensure
the function still handles the non-JSON branch (the existing return [uid.strip()
...]) and log or silently ignore the parse error as appropriate so a bad
selected_users query parameter does not cause a 500 in the view.
🧹 Nitpick comments (1)
src/imio/esign/browser/settings.py (1)
81-104: Validate email template placeholders at save time.The email content is later formatted with
.format(**user_data). A typo like{full_name}will raiseKeyErrorduring sending and fail every email. Consider a schema constraint to validate placeholders up front.✅ Suggested constraint to validate allowed placeholders
+from string import Formatter ... def validate_vat_number(va_nb): """Validate the VAT number format. It should start with BE followed by 10 digits, with the last 2 digits being a control checksum of the first 8 digits.""" if not va_nb: return True @@ return True + + +def validate_signing_users_email_content(value): + """Allow only known placeholders in the email template.""" + allowed = {"fullname", "firstname", "lastname", "email", "userid"} + for _, field, _, _ in Formatter().parse(value or ""): + if field and field not in allowed: + raise Invalid( + _("Unknown placeholder: ${field}", mapping={"field": field}) + ) + return True ... signing_users_email_content = schema.Text( title=_("Email content for signing users"), description=_( "Email content sent to users when inviting them to Parapheo. " "Use {fullname}, {firstname}, {lastname}, {email}, {userid} as placeholders." ), required=False, + constraint=validate_signing_users_email_content, default=u"""Hello {fullname},
chris-adam
force-pushed
the
SE-235/signers_emailing
branch
from
f59e5fd to
c7e8fcf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/imio/esign/browser/settings.py`:
- Around line 41-48: The validator validate_signing_users_email_content shadows
the translation function _ by using _ as loop variables from
Formatter().parse(value or ""), causing a TypeError when calling _(...) later;
rename the loop variables (e.g., use ignored1, field, ignored2, ignored3 or
similar) so only the actual placeholder name is bound to field and the global _
remains callable, then keep the existing Invalid(...) call that uses _("Unknown
placeholder: ${field}", mapping={"field": field}).
🧹 Nitpick comments (1)
src/imio/esign/browser/views.py (1)
549-551: Avoid unused variable noise.
duplicatesis unused; rename to_duplicatesor omit it to keep lint clean.
chris-adam
force-pushed
the
SE-235/signers_emailing
branch
from
c7e8fcf to
d567660
Compare
src/imio/esign/browser/settings.py
Outdated
| ), | ||
| required=False, | ||
| constraint=validate_signing_users_email_content, | ||
| default=u"""Hello {fullname}, |
There was a problem hiding this comment.
c'est mieux de laisser vide la valeur par défaut et de la compléter dans le setup de dmsmail (là où on change déjà les autres paramètres)
There was a problem hiding this comment.
| metal:use-macro="context/main_template/macros/master" | ||
| i18n:domain="imio.esign"> | ||
|
|
||
| <metal:block fill-slot="content-core"> |
There was a problem hiding this comment.
faudrait supprimer la barre verte, intitulé, etc, donc voir si on peut utiliser un slot plus global
| required=False, | ||
| ) | ||
|
|
||
| signing_users_email_content = schema.Text( |
There was a problem hiding this comment.
Tu peux en faire un champ riche (comme omail_email_signature de nos settings) et utiliser du tal pour rendre les variables dynamiques. Cela permettra de mettre en forme certaines choses si nécessaire.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/imio/esign/browser/templates/signing_users.pt (1)
245-275: Add basic ARIA + focus handling for the confirmation modal.The modal lacks ARIA semantics and focus management, which makes keyboard and screen‑reader usage harder. A small ARIA+focus pass would improve accessibility without changing behavior.
♿ Suggested tweak
- <div id="email-confirmation-modal"> - <div class="modal-content"> - <h2 i18n:translate="">Confirm Email Sending</h2> + <div id="email-confirmation-modal" + role="dialog" + aria-modal="true" + aria-labelledby="email-modal-title"> + <div class="modal-content" tabindex="-1"> + <h2 id="email-modal-title" i18n:translate="">Confirm Email Sending</h2> @@ } else if (action === 'send_emails') { // Show confirmation modal document.getElementById('email-count').textContent = selected.length; modal.style.display = 'block'; + confirmEmailBtn.focus(); } @@ cancelEmailBtn.addEventListener('click', function() { modal.style.display = 'none'; + sendEmailBtn.focus(); }); @@ if (e.target === modal) { modal.style.display = 'none'; + sendEmailBtn.focus(); } });Also applies to: 328-333, 382-399
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/imio/esign/browser/templates/signing_users.pt` around lines 245 - 275, Add basic ARIA roles and focus management to the confirmation modal: give the modal container (id="email-confirmation-modal") role="dialog" with aria-modal="true" plus aria-labelledby referencing the h2 and aria-describedby referencing the explanatory paragraph or the span id="email-count"; ensure the modal itself is focusable (tabindex="-1") and programmatically move focus into it when opened and return focus to the triggering element when closed; wire keyboard handling on the modal to close on Escape and keep tab focus trapped between the focusable controls (ids cancel-email-btn and confirm-email-btn) until close; also toggle aria-hidden on the rest of the page when the modal is open.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/imio/esign/browser/settings.py`:
- Around line 42-50: The validate_signing_users_email_content validator
currently calls Formatter().parse(value) which can raise ValueError on malformed
format strings; wrap the parse loop in a try/except that catches ValueError and
re-raises an Invalid with a clear message (e.g. "Malformed template" or similar)
so malformed templates surface as validation errors; keep the existing behavior
of checking placeholders against the allowed set
{"fullname","firstname","lastname","email","userid"} and raising Invalid for
unknown placeholders inside validate_signing_users_email_content.
In `@src/imio/esign/browser/templates/signing_users.pt`:
- Around line 202-206: The client-side "empty selection" alert is hardcoded;
make it translatable by adding a translatable data attribute on the form or the
hidden input (e.g., on the element with id "users-form" or the hidden input id
"selected-users") containing the localized message via TAL/i18n, then update the
JS that reads document.getElementById("users-form") or
document.getElementById("selected-users") to fetch that data attribute and use
it for the alert instead of a hardcoded English string; apply the same pattern
for the other occurrence around ids "form-action" / the second block (lines
~316-321).
---
Nitpick comments:
In `@src/imio/esign/browser/templates/signing_users.pt`:
- Around line 245-275: Add basic ARIA roles and focus management to the
confirmation modal: give the modal container (id="email-confirmation-modal")
role="dialog" with aria-modal="true" plus aria-labelledby referencing the h2 and
aria-describedby referencing the explanatory paragraph or the span
id="email-count"; ensure the modal itself is focusable (tabindex="-1") and
programmatically move focus into it when opened and return focus to the
triggering element when closed; wire keyboard handling on the modal to close on
Escape and keep tab focus trapped between the focusable controls (ids
cancel-email-btn and confirm-email-btn) until close; also toggle aria-hidden on
the rest of the page when the modal is open.
src/imio/esign/browser/settings.py
Outdated
| def validate_signing_users_email_content(value): | ||
| """Allow only known placeholders in the email template.""" | ||
| allowed = {"fullname", "firstname", "lastname", "email", "userid"} | ||
| for _ignored1, field, _ignored2, _ignored3 in Formatter().parse(value or ""): | ||
| if field and field not in allowed: | ||
| raise Invalid( | ||
| _("Unknown placeholder: ${field}", mapping={"field": field}) | ||
| ) | ||
| return True |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that Formatter().parse() raises ValueError on malformed format strings
python3 - <<'PY'
from string import Formatter
samples = ["Hello {name", "Hello }", "Hello {name}}"]
for s in samples:
try:
list(Formatter().parse(s))
print("OK:", s)
except Exception as e:
print(type(e).__name__, "for", repr(s))
PYRepository: IMIO/imio.esign
Length of output: 143
🏁 Script executed:
# Check the actual file to confirm code location and implementation
if [ -f "src/imio/esign/browser/settings.py" ]; then
wc -l src/imio/esign/browser/settings.py
echo "---"
sed -n '40,55p' src/imio/esign/browser/settings.py
else
echo "File not found at expected location. Searching..."
find . -name "settings.py" -path "*/esign/browser/*" 2>/dev/null
fiRepository: IMIO/imio.esign
Length of output: 612
Handle malformed templates to avoid uncaught ValueError.
Formatter().parse() raises ValueError on malformed format strings (e.g., unmatched braces). When used as a schema constraint, this will surface as a server error instead of a validation error. Catch it and re‑raise Invalid with a friendly message.
🔧 Suggested fix
def validate_signing_users_email_content(value):
"""Allow only known placeholders in the email template."""
allowed = {"fullname", "firstname", "lastname", "email", "userid"}
- for _ignored1, field, _ignored2, _ignored3 in Formatter().parse(value or ""):
- if field and field not in allowed:
- raise Invalid(
- _("Unknown placeholder: ${field}", mapping={"field": field})
- )
+ try:
+ for _ignored1, field, _ignored2, _ignored3 in Formatter().parse(value or ""):
+ if field and field not in allowed:
+ raise Invalid(
+ _("Unknown placeholder: ${field}", mapping={"field": field})
+ )
+ except ValueError as exc:
+ raise Invalid(_("Invalid email template format.")) from exc
return True📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def validate_signing_users_email_content(value): | |
| """Allow only known placeholders in the email template.""" | |
| allowed = {"fullname", "firstname", "lastname", "email", "userid"} | |
| for _ignored1, field, _ignored2, _ignored3 in Formatter().parse(value or ""): | |
| if field and field not in allowed: | |
| raise Invalid( | |
| _("Unknown placeholder: ${field}", mapping={"field": field}) | |
| ) | |
| return True | |
| def validate_signing_users_email_content(value): | |
| """Allow only known placeholders in the email template.""" | |
| allowed = {"fullname", "firstname", "lastname", "email", "userid"} | |
| try: | |
| for _ignored1, field, _ignored2, _ignored3 in Formatter().parse(value or ""): | |
| if field and field not in allowed: | |
| raise Invalid( | |
| _("Unknown placeholder: ${field}", mapping={"field": field}) | |
| ) | |
| except ValueError as exc: | |
| raise Invalid(_("Invalid email template format.")) from exc | |
| return True |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/imio/esign/browser/settings.py` around lines 42 - 50, The
validate_signing_users_email_content validator currently calls
Formatter().parse(value) which can raise ValueError on malformed format strings;
wrap the parse loop in a try/except that catches ValueError and re-raises an
Invalid with a clear message (e.g. "Malformed template" or similar) so malformed
templates surface as validation errors; keep the existing behavior of checking
placeholders against the allowed set
{"fullname","firstname","lastname","email","userid"} and raising Invalid for
unknown placeholders inside validate_signing_users_email_content.
There was a problem hiding this comment.
That's fine because this is a setting that will only be edited by the support team or developers, not end users
There was a problem hiding this comment.
@chris-adam, understood! I've noted that for admin-only configuration settings in this project, verbose error handling for malformed input is not required.
(\\_/)
(•_•)
/>📝 Noted for future reference!
✏️ Learnings added
Learnt from: chris-adam
Repo: IMIO/imio.esign PR: 6
File: src/imio/esign/browser/settings.py:42-50
Timestamp: 2026-02-17T12:37:05.819Z
Learning: In the imio.esign project, for configuration settings that are only edited by the support team or developers (not end users), it's acceptable to allow technical errors like ValueError to surface as server errors rather than wrapping them in user-friendly validation messages.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| <form id="users-form" method="post" tal:attributes="action string:${context/absolute_url}/@@signing-users-csv"> | ||
| <!-- Hidden inputs for form submission --> | ||
| <input type="hidden" name="_authenticator" tal:attributes="value context/@@authenticator/token" /> | ||
| <input type="hidden" name="action" id="form-action" value="" /> | ||
| <input type="hidden" name="selected_users" id="selected-users" value="" /> |
There was a problem hiding this comment.
Localize the client‑side empty‑selection alert.
The JS alert is hardcoded in English, so it won’t be translated. Consider storing the message as a translatable attribute and reading it in JS.
🔧 Suggested fix
- <form id="users-form" method="post" tal:attributes="action string:${context/absolute_url}/@@signing-users-csv">
+ <form id="users-form"
+ method="post"
+ tal:attributes="action string:${context/absolute_url}/@@signing-users-csv"
+ data-empty-selection-msg="Please select at least one user."
+ i18n:attributes="data-empty-selection-msg">
@@
if (selected.length === 0) {
- alert('Please select at least one user.');
+ var emptySelectionMsg = form.getAttribute('data-empty-selection-msg')
+ || 'Please select at least one user.';
+ alert(emptySelectionMsg);
return;
}Also applies to: 316-321
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/imio/esign/browser/templates/signing_users.pt` around lines 202 - 206,
The client-side "empty selection" alert is hardcoded; make it translatable by
adding a translatable data attribute on the form or the hidden input (e.g., on
the element with id "users-form" or the hidden input id "selected-users")
containing the localized message via TAL/i18n, then update the JS that reads
document.getElementById("users-form") or
document.getElementById("selected-users") to fetch that data attribute and use
it for the alert instead of a hardcoded English string; apply the same pattern
for the other occurrence around ids "form-action" / the second block (lines
~316-321).
There was a problem hiding this comment.
That's fine because this view will only be used by the support team or developers, not end users
There was a problem hiding this comment.
@chris-adam, understood! That makes perfect sense for an internal support/developer tool. I'll note this for future reference.
/)/)
(o.o)
(> <) Got it!
✏️ Learnings added
Learnt from: chris-adam
Repo: IMIO/imio.esign PR: 6
File: src/imio/esign/browser/templates/signing_users.pt:202-206
Timestamp: 2026-02-17T12:37:45.338Z
Learning: The signing-users-csv view in imio.esign (src/imio/esign/browser/templates/signing_users.pt and related views.py) is an internal tool used only by support team and developers, not end users, so internationalization is not required for this view.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/imio/esign/setuphandlers.py (1)
26-28: Avoid hardcoding a test Paraphéo URL as a default.If admins forget to update it, emails could point signers to the wrong environment. Consider leaving it empty (forcing explicit configuration) or deriving a site-specific default.
♻️ Suggested adjustment
- if not get_registry_parapheo_url(): - set_registry_parapheo_url("https://simplycosi-1-test.trustsigneurope.com/login?tenantName=IMIO") + if not get_registry_parapheo_url(): + # Leave blank to force explicit configuration in the control panel + set_registry_parapheo_url("")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/imio/esign/setuphandlers.py` around lines 26 - 28, The code currently sets a hardcoded test Paraphéo URL via set_registry_parapheo_url when get_registry_parapheo_url() is empty; remove this hardcoded default and instead leave the registry Paraphéo URL unset (e.g., do not call set_registry_parapheo_url with a test URL) or derive a site-appropriate default from site/config helpers (use site.getSiteURL() or the relevant site configuration) so admins must explicitly configure a Paraphéo endpoint; update the logic around get_registry_parapheo_url and set_registry_parapheo_url in setuphandlers.py accordingly and ensure any code that consumes the URL handles an empty/missing value gracefully.src/imio/esign/browser/settings.py (1)
81-85: Prefer schema.URI for parapheo_url validation.This value is used in email links; using a URI field (or a constraint) helps prevent invalid URLs from being stored.
♻️ Suggested change
- parapheo_url = schema.TextLine( + parapheo_url = schema.URI( title=_("Parapheo url"), description=_("Used in signers email template."), required=False, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/imio/esign/browser/settings.py` around lines 81 - 85, The parapheo_url field currently uses schema.TextLine which doesn't validate URLs; update the field definition named parapheo_url to use schema.URI (or schema.TextLine with a URI constraint) so stored values are valid URIs for use in email links; modify the declaration of parapheo_url accordingly (replacing schema.TextLine with schema.URI and keeping title/description/required options) and run tests that exercise settings parsing to ensure no regressions.src/imio/esign/browser/views.py (1)
417-427: Avoid O(n²) duplicate checks when building the user list.
any(...)overall_users_dataeach iteration scales poorly for large user bases. Asetkeeps this O(n).♻️ Suggested refactor
- all_users_data = [] + all_users_data = [] + seen_userids = set() @@ - if any(u["userid"] == userid for u in all_users_data): - continue + if userid in seen_userids: + continue + seen_userids.add(userid)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/imio/esign/browser/views.py` around lines 417 - 427, The duplicate-check over all_users_data using any(...) inside the loop is O(n²); introduce a seen_userids = set() before iterating over acl_users.searchUsers() and replace the any(...) check with a membership test (if userid in seen_userids: continue), then add userid to seen_userids when you append to all_users_data; this keeps email_registry logic intact and makes building all_users_data O(n).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/imio/esign/browser/views.py`:
- Around line 519-527: The exported CSV can contain user-controlled fields that
begin with Excel formula chars (=, +, -, @); update the writer loop that writes
rows for selected_users (the block calling writer.writerow with keys
"userid","email","lastname","firstname","fullname" and using safe_encode) to
sanitize each field by detecting if the encoded string starts with any of those
characters and, if so, prefix it (e.g., a single quote) to neutralize formula
evaluation before writing; implement this sanitization as a small helper (or
inline check) applied to safe_encode(user_data["..."]) for each field.
- Around line 536-543: The _send_emails method accepts actions via query params
and must enforce POST + CSRF validation: add a check at the top of _send_emails
to require request.method == "POST" and call CheckAuthenticator(self.request) to
validate the _authenticator token (import CheckAuthenticator from
plone.protect), and if either check fails show a warning via
api.portal.show_message and redirect back (same behavior as when no
selected_userids); apply the same change to _download_csv and keep using
_get_selected_userids for selected users.
In `@test-4.3.cfg`:
- Around line 79-84: The ipaddress==1.0.23 pin in the config must be explicitly
justified or upgraded: verify whether Python 2.7 support (Plone 4.3) actually
requires the backport ipaddress and, if it is required, add a comment
documenting the known CVEs (CVE-2021-29921, CVE-2020-14422, CVE-2024-4032) and
the accepted risk/mitigations; if the environment can run a newer Python or
doesn’t need the backport, remove or upgrade the ipaddress pin. Also re-evaluate
the other pins: leave cryptography==3.3.2 only if you still require that
specific CVE-fix, consider upgrading cffi==1.15.1 only if compatibility allows,
and attempt to move pycparser from 2.21 to a newer compatible release (e.g.,
3.x) or document why 2.21 must be pinned. Ensure these changes reference the
package names ipaddress, cryptography, cffi, and pycparser in the config and
include short justification comments for any non-upgraded pins.
---
Duplicate comments:
In `@src/imio/esign/browser/views.py`:
- Around line 491-495: The JSON parsing for "selected" should only return a
list: in the try block where json.loads(selected) is returned (the code handling
selected_users/selected), assign the parsed value to a variable, check
isinstance(parsed, list) and return it only if it's a list; otherwise return an
empty list (and keep the except clause returning [] for decode/type errors).
Update the return flow in the function/method that handles
selected_users/selected to use this parsed-and-validated list check.
---
Nitpick comments:
In `@src/imio/esign/browser/settings.py`:
- Around line 81-85: The parapheo_url field currently uses schema.TextLine which
doesn't validate URLs; update the field definition named parapheo_url to use
schema.URI (or schema.TextLine with a URI constraint) so stored values are valid
URIs for use in email links; modify the declaration of parapheo_url accordingly
(replacing schema.TextLine with schema.URI and keeping
title/description/required options) and run tests that exercise settings parsing
to ensure no regressions.
In `@src/imio/esign/browser/views.py`:
- Around line 417-427: The duplicate-check over all_users_data using any(...)
inside the loop is O(n²); introduce a seen_userids = set() before iterating over
acl_users.searchUsers() and replace the any(...) check with a membership test
(if userid in seen_userids: continue), then add userid to seen_userids when you
append to all_users_data; this keeps email_registry logic intact and makes
building all_users_data O(n).
In `@src/imio/esign/setuphandlers.py`:
- Around line 26-28: The code currently sets a hardcoded test Paraphéo URL via
set_registry_parapheo_url when get_registry_parapheo_url() is empty; remove this
hardcoded default and instead leave the registry Paraphéo URL unset (e.g., do
not call set_registry_parapheo_url with a test URL) or derive a site-appropriate
default from site/config helpers (use site.getSiteURL() or the relevant site
configuration) so admins must explicitly configure a Paraphéo endpoint; update
the logic around get_registry_parapheo_url and set_registry_parapheo_url in
setuphandlers.py accordingly and ensure any code that consumes the URL handles
an empty/missing value gracefully.
| for user_data in selected_users: | ||
| writer.writerow( | ||
| { | ||
| "userid": safe_encode(user_data["userid"]), | ||
| "email": safe_encode(user_data["email"]), | ||
| "lastname": safe_encode(user_data["lastname"]), | ||
| "firstname": safe_encode(user_data["firstname"]), | ||
| "fullname": safe_encode(user_data["fullname"]), | ||
| } |
There was a problem hiding this comment.
Mitigate CSV formula injection in exported data.
User-controlled fields can start with =, +, -, or @, which may execute formulas when opened in Excel. Prefix such values to neutralize.
🛡️ Suggested fix
- for user_data in selected_users:
+ def _csv_safe(value):
+ value = safe_encode(value)
+ if value and value[0] in ("=", "+", "-", "@"):
+ return "'" + value
+ return value
+
+ for user_data in selected_users:
writer.writerow(
{
- "userid": safe_encode(user_data["userid"]),
- "email": safe_encode(user_data["email"]),
- "lastname": safe_encode(user_data["lastname"]),
- "firstname": safe_encode(user_data["firstname"]),
- "fullname": safe_encode(user_data["fullname"]),
+ "userid": _csv_safe(user_data["userid"]),
+ "email": _csv_safe(user_data["email"]),
+ "lastname": _csv_safe(user_data["lastname"]),
+ "firstname": _csv_safe(user_data["firstname"]),
+ "fullname": _csv_safe(user_data["fullname"]),
}
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for user_data in selected_users: | |
| writer.writerow( | |
| { | |
| "userid": safe_encode(user_data["userid"]), | |
| "email": safe_encode(user_data["email"]), | |
| "lastname": safe_encode(user_data["lastname"]), | |
| "firstname": safe_encode(user_data["firstname"]), | |
| "fullname": safe_encode(user_data["fullname"]), | |
| } | |
| def _csv_safe(value): | |
| value = safe_encode(value) | |
| if value and value[0] in ("=", "+", "-", "@"): | |
| return "'" + value | |
| return value | |
| for user_data in selected_users: | |
| writer.writerow( | |
| { | |
| "userid": _csv_safe(user_data["userid"]), | |
| "email": _csv_safe(user_data["email"]), | |
| "lastname": _csv_safe(user_data["lastname"]), | |
| "firstname": _csv_safe(user_data["firstname"]), | |
| "fullname": _csv_safe(user_data["fullname"]), | |
| } | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/imio/esign/browser/views.py` around lines 519 - 527, The exported CSV can
contain user-controlled fields that begin with Excel formula chars (=, +, -, @);
update the writer loop that writes rows for selected_users (the block calling
writer.writerow with keys "userid","email","lastname","firstname","fullname" and
using safe_encode) to sanitize each field by detecting if the encoded string
starts with any of those characters and, if so, prefix it (e.g., a single quote)
to neutralize formula evaluation before writing; implement this sanitization as
a small helper (or inline check) applied to safe_encode(user_data["..."]) for
each field.
| def _send_emails(self): | ||
| """Send emails to selected users.""" | ||
| selected_userids = self._get_selected_userids() | ||
|
|
||
| response = self.request.RESPONSE | ||
| response.setHeader("Content-Type", "text/html; charset=utf-8") | ||
| if not selected_userids: | ||
| api.portal.show_message(_("No users selected for email sending."), request=self.request, type="warning") | ||
| return self.request.RESPONSE.redirect(self.context.absolute_url() + "/@@signing-users-csv") | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the signing-users-csv template and inspect its form structure
rg -n "signing-users-csv|POST|authenticator|method" src/imio/esign/browser/templates/ -A 3 -B 1Repository: IMIO/imio.esign
Length of output: 1368
🏁 Script executed:
#!/bin/bash
# Check the view registration and how _send_emails is called
rg -n "_send_emails|signing-users" src/imio/esign/browser/views.py -B 5 -A 10Repository: IMIO/imio.esign
Length of output: 3817
🏁 Script executed:
#!/bin/bash
# Check for CheckAuthenticator usage in the codebase
rg -n "CheckAuthenticator" src/ --type py -B 2 -A 2Repository: IMIO/imio.esign
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Check the full _send_emails method and surrounding context
sed -n '530,560p' src/imio/esign/browser/views.pyRepository: IMIO/imio.esign
Length of output: 1519
🏁 Script executed:
#!/bin/bash
# Check if _download_csv has similar vulnerability
sed -n '398,407p' src/imio/esign/browser/views.pyRepository: IMIO/imio.esign
Length of output: 349
🏁 Script executed:
#!/bin/bash
# Check the full view class structure and __call__ method
rg -n "class.*View|def __call__|def index" src/imio/esign/browser/views.py | head -20Repository: IMIO/imio.esign
Length of output: 644
🏁 Script executed:
#!/antml:bash
# Check for existing POST/CSRF patterns in similar Plone views
rg -n "request.method.*POST|CheckAuthenticator" src/imio/esign/browser/ -A 2 -B 2Repository: IMIO/imio.esign
Length of output: 41
🏁 Script executed:
#!/bin/bash
# Verify plone.protect exists and CheckAuthenticator is available
rg -n "from plone" src/imio/esign/browser/views.py | head -10Repository: IMIO/imio.esign
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Check the full __call__ method of SigningUsersCsv view
sed -n '389,410p' src/imio/esign/browser/views.pyRepository: IMIO/imio.esign
Length of output: 778
🌐 Web query:
plone.protect CheckAuthenticator CSRF validation
💡 Result:
plone.protect’s CheckAuthenticator is the standard “manual CSRF check” you use in views/handlers that perform writes (especially writes outside the ZODB transaction, e.g., external API calls). If the request does not contain a valid CSRF token, it raises Unauthorized. [1][2]
How CSRF validation works (what CheckAuthenticator checks)
A request is considered valid when it includes a valid token provided by Plone’s authenticator machinery, typically:
- A hidden form field named
_authenticator(usually injected into forms via@@authenticator/authenticator), or - The HTTP header
X-CSRF-TOKEN(common for AJAX). [1]
Plone also has automatic CSRF protection (commit-time validation): a request that modified the ZODB won’t be allowed to commit unless a valid token is present. This happens at transaction commit time, not necessarily when your view starts executing. [2]
Common ways to use CheckAuthenticator
1) Decorator (typical)
from plone.protect import CheckAuthenticator, protect
`@protect`(CheckAuthenticator)
def my_write_view(self):
...[1][2]
2) Call it directly
from plone.protect import CheckAuthenticator
def __call__(self):
CheckAuthenticator(self.request) # raises Unauthorized if invalid/missing
...[1]
3) Verify via the @@authenticator view
from zope.component import getMultiAdapter
from zExceptions import Unauthorized
auth = getMultiAdapter((self.context, self.request), name="authenticator")
if not auth.verify():
raise Unauthorized[1]
How to include/provide a valid token
In a page template (classic)
<span tal:replace="structure context/@@authenticator/authenticator" />This renders the hidden _authenticator input. [1]
Programmatically (e.g., tests / custom client)
from plone.protect.authenticator import createToken
token = createToken()Then send it as _authenticator=token (form) or X-CSRF-TOKEN: token (header). [1]
Notes for tests
If your tests fail with Unauthorized, you can set the token on the test request (and often ensure POST): [1]
self.request.form["_authenticator"] = createToken()
self.request.environ["REQUEST_METHOD"] = "POST"Sources: plone.protect README on PyPI [1]; Plone 6 CSRF docs [2].
Add POST and CSRF token validation to prevent CSRF/unintended triggers.
Email sending is non-idempotent and currently accessible via query parameters (e.g., ?action=send_emails), which bypasses both the POST requirement and CSRF token validation. The form properly includes _authenticator, but the view uses request.get("action") which accepts GET parameters.
Add these checks to _send_emails():
Suggested fix
def _send_emails(self):
"""Send emails to selected users."""
+ if self.request.method != "POST":
+ return self.request.RESPONSE.redirect(self.context.absolute_url() + "/@@signing-users-csv")
+ CheckAuthenticator(self.request)
selected_userids = self._get_selected_userids()Add the import:
from plone.protect import CheckAuthenticatorNote: _download_csv() has the same vulnerability and should receive the same fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/imio/esign/browser/views.py` around lines 536 - 543, The _send_emails
method accepts actions via query params and must enforce POST + CSRF validation:
add a check at the top of _send_emails to require request.method == "POST" and
call CheckAuthenticator(self.request) to validate the _authenticator token
(import CheckAuthenticator from plone.protect), and if either check fails show a
warning via api.portal.show_message and redirect back (same behavior as when no
selected_userids); apply the same change to _download_csv and keep using
_get_selected_userids for selected users.
| # Required by: | ||
| # imio.helpers==1.3.11.dev0 | ||
| cryptography = 3.3.2 | ||
| cffi = 1.15.1 | ||
| ipaddress = 1.0.23 | ||
| pycparser = 2.21 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
cryptography 3.3.2 CVE security advisory vulnerability
💡 Result:
cryptography 3.3.2 (released 2021-02-07) is a security-fix release for CVE-2020-36242. [1]
- CVE: CVE-2020-36242 [2]
- Affected versions:
cryptographybefore 3.3.2 (i.e., upgrade is required if you’re on <3.3.2). [1][2] - Issue summary: Certain sequences of
CipherContext.update()calls while symmetrically encrypting very large payloads (multi‑GB, e.g., >2GB; demonstrated with Fernet) could trigger an integer overflow, which could lead to a buffer overflow (crash/DoS and potentially worse depending on context). [1][2] - Fix / remediation: Upgrade to
cryptography3.3.2 or newer (or vendor backport). [1][3]
Sources: Cryptography 3.3.2 changelog [1], NVD entry for CVE-2020-36242 [2], Debian security tracker [3].
References
- Changelog — Cryptography 3.3.2 (SECURITY ISSUE, CVE-2020-36242)
- NVD — CVE-2020-36242
- Debian security tracker — CVE-2020-36242
🌐 Web query:
cffi 1.15.1 CVE security advisory vulnerability
💡 Result:
For the upstream PyPI package cffi version 1.15.1 (released Jun 30, 2022), I couldn’t find any published CVEs/security advisories that are specific to cffi==1.15.1. Both Snyk and Safety currently list no known security issues/vulnerabilities for cffi. [1] [2] [3]
Be careful not to confuse cffi with similarly named packages like curl-cffi (a different project that has advisories related to libcurl). [4] [5]
Also note that some Linux distributions may publish security advisories for their OS package named python-cffi (which may track different versions/patches than PyPI cffi); for example, SUSE issued an update for python-cffi (their packaged version 1.11.5) referencing CVE-2023-23931. That advisory is not about PyPI cffi==1.15.1. [6]
Sources
[1] PyPI release page for cffi==1.15.1 (release date) (pypi.org)
[2] Snyk cffi package page (“No known security issues”) (security.snyk.io)
[3] Safety DB cffi package page (“No known vulnerabilities found”) (data.safetycli.com)
[4] GitLab Advisory Database for curl-cffi (separate package) (advisories.gitlab.com)
[5] Resolved Security write-up for GHSA affecting curl-cffi (resolvedsecurity.com)
[6] SUSE security update for python-cffi referencing CVE-2023-23931 (suse.com)
Citations:
- 1: https://pypi.org/project/cffi/1.15.1/?utm_source=openai
- 2: https://security.snyk.io/package/pip/cffi?utm_source=openai
- 3: https://data.safetycli.com/packages/pypi/cffi/?utm_source=openai
- 4: https://advisories.gitlab.com/pkg/pypi/curl-cffi/?utm_source=openai
- 5: https://www.resolvedsecurity.com/vulnerability-catalog/GHSA-3vpc-4p9p-47hc?utm_source=openai
- 6: https://www.suse.com/support/update/announcement/2023/suse-su-20230838-1/
🌐 Web query:
Plone 4.3 supported Python versions compatibility
💡 Result:
Plone 4.3 is compatible with Python 2.7 (and is not intended to run on Python 2.6; Plone 4.1 was the last line documented for Python 2.6). [1], [2]
🌐 Web query:
ipaddress 1.0.23 Python package security
💡 Result:
ipaddress==1.0.23 is an old backport of Python’s standard-library ipaddress module (meant for Python 2.6/2.7/3.2) and its last PyPI release was Oct 18, 2019. [1] From a security/maintenance standpoint, that age alone is a risk signal.
Known security concerns that may apply
- CVE-2021-29921 (octal-string parsing / improper input validation): Reported as affecting
ipaddress(PyPI package) and can enable SSRF/RFI/LFI-style issues in applications that rely onipaddressto validate/filter attacker-controlled IP input; Snyk reports no fixed version of the PyPI package. [2] - CVE-2020-14422 (hash-collision DoS):
IPv4Interface/IPv6Interfacehashing could cause extreme dictionary hash collisions (DoS risk) in vulnerable implementations; this was fixed in CPython and also patched by some distros (e.g., SUSE’spython-ipaddressRPM), but that doesn’t guarantee the PyPIipaddress==1.0.23wheel you get frompipcontains the fix. [3][4] - CVE-2024-4032 (misclassification of
is_private/is_global): CPython’sipaddresshad outdated special-purpose range data that could lead to incorrect security policy decisions; fixed in Python 3.12.4+ (and 3.13 alpha at the time). If you’re using the PyPI backport, you should assume you won’t get these registry updates unless the backport is actively maintained (it isn’t). [5][6]
Practical guidance
- If you are on Python 3.3+, prefer the stdlib
ipaddressand remove/uninstall the PyPI backport to avoid outdated behavior and “shadowing” the stdlib module. [1] - If your code uses
ipaddressfor security decisions (allow/deny by “private”, “global”, etc.), ensure you’re on a CPython version that includes the CVE-2024-4032 fix (e.g., 3.12.4+) and re-test that logic. [5] - If you’re stuck on EOL Python and must keep the backport, treat
ipaddress==1.0.23as potentially vulnerable/unpatched for the above items and consider vendoring/patching or moving to a maintained alternative.
Sources: PyPI project info for ipaddress 1.0.23 [1]; Snyk advisory for CVE-2021-29921 affecting ipaddress [2]; CPython issue tracker for CVE-2020-14422 [3] and SUSE advisory showing a downstream patch [4]; Ubuntu notice for CVE-2024-4032 and its fix in newer CPython versions [5]; Python docs indicating changed/updated is_private/is_global behavior in newer versions [6].
References: [1] (pypi.org) [2] (security.snyk.io) [3] (bugs.python.org) [4] (suse.com) [5] (ubuntu.com) [6] (docs.python.org)
Citations:
- 1: https://pypi.org/project/ipaddress/?utm_source=openai
- 2: https://security.snyk.io/vuln/SNYK-PYTHON-IPADDRESS-1290073?utm_source=openai
- 3: https://bugs.python.org/issue41004?utm_source=openai
- 4: https://www.suse.com/support/update/announcement/2020/suse-su-20202157-1/?utm_source=openai
- 5: https://ubuntu.com/security/notices/USN-6941-1?utm_source=openai
- 6: https://docs.python.org/3/library/ipaddress.html?utm_source=openai
🌐 Web query:
pycparser 2.21 CVE security vulnerability
💡 Result:
Web search failed: Server error: no LLM provider could handle the message
🌐 Web query:
pycparser 2.21 latest version release date maintenance status
💡 Result:
- Is pycparser 2.21 the latest? No. The latest release on PyPI is pycparser 3.0, released January 21, 2026. (pypi.org)
- pycparser 2.21 release date: November 6, 2021. (piwheels.org)
- Maintenance status: Actively maintained (there have been multiple newer upstream releases after 2.21: 2.22 (Mar 30, 2024), 2.23 (Sep 9, 2025), and 3.0 (Jan 21, 2026)). (pypi.org)
Citations:
- 1: https://pypi.org/project/pycparser/?utm_source=openai
- 2: https://www.piwheels.org/project/pycparser?utm_source=openai
- 3: https://pypi.org/project/pycparser/?utm_source=openai
Confirm ipaddress 1.0.23 and reassess other pins.
- cryptography 3.3.2: This is the security fix for CVE-2020-36242; no action needed.
- cffi 1.15.1: No known CVEs; acceptable.
- ipaddress 1.0.23: This backport (last updated Oct 2019) has confirmed CVEs: CVE-2021-29921 (octal-string parsing SSRF), CVE-2020-14422 (hash-collision DoS), and CVE-2024-4032 (is_private/is_global misclassification). For Python 2.7 (Plone 4.3 only), the backport may be unavoidable, but ensure it's justified and document the risk.
- pycparser 2.21: Over 4 years old but actively maintained (latest is 3.0); consider upgrading if compatible with your environment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test-4.3.cfg` around lines 79 - 84, The ipaddress==1.0.23 pin in the config
must be explicitly justified or upgraded: verify whether Python 2.7 support
(Plone 4.3) actually requires the backport ipaddress and, if it is required, add
a comment documenting the known CVEs (CVE-2021-29921, CVE-2020-14422,
CVE-2024-4032) and the accepted risk/mitigations; if the environment can run a
newer Python or doesn’t need the backport, remove or upgrade the ipaddress pin.
Also re-evaluate the other pins: leave cryptography==3.3.2 only if you still
require that specific CVE-fix, consider upgrading cffi==1.15.1 only if
compatibility allows, and attempt to move pycparser from 2.21 to a newer
compatible release (e.g., 3.x) or document why 2.21 must be pinned. Ensure these
changes reference the package names ipaddress, cryptography, cffi, and pycparser
in the config and include short justification comments for any non-upgraded
pins.
2 participants
Jen ai profité pour tester l'IA vu que c'est une vue qui ne devrait pas beaucoup évoluer et qui ne sera utilisée que pa nous. Je suis bien sûr repassé derrière
Summary by CodeRabbit
New Features
Chores