Skip to content

Investigate provider-to-patient email functionality in CARLOS#3096

Draft
Ben-Heerema wants to merge 1 commit into
developfrom
codex/investigate-provider-patient-email
Draft

Investigate provider-to-patient email functionality in CARLOS#3096
Ben-Heerema wants to merge 1 commit into
developfrom
codex/investigate-provider-patient-email

Conversation

@Ben-Heerema

@Ben-Heerema Ben-Heerema commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR investigates how provider-to-patient email functionality currently works in CARLOS, including the available user flows, backend routing, configuration requirements, and expected behavior in local development.

Scope

  • Identify all UI entry points for provider-to-patient email.
  • Trace the backend actions and services used to compose and send email.
  • Confirm required permissions, patient data, consent, and sender configuration.
  • Verify whether email is available as a direct chart workflow, eForm workflow, or both.
  • Document local development behavior and any expected setup requirements.
  • Capture gaps, confusing UX, or failure modes for follow-up work.

Validation Plan

  • Test the provider workflow from a patient chart.
  • Test the eForm email workflow.
  • Confirm behavior when the patient has no email address.
  • Confirm behavior when consent is missing or not explicit opt-in.
  • Confirm behavior when no sender account is configured.
  • Confirm behavior when SMTP/API delivery is unavailable.
  • Review email logs/status handling after attempted sends.

Notes

Detailed findings will be added as PR comments as each part of the workflow is traced and verified.

Summary by Sourcery

Documentation:

  • Add a specification document outlining the purpose, scope, and validation plan for investigating provider-to-patient email workflows in CARLOS.

Summary by cubic

Adds a documentation spec mapping provider‑to‑patient email in CARLOS: UI entry points, backend routes, required permissions/consent, sender config, and local setup. Includes a validation plan to test chart/eForm flows, edge cases (no email/consent, delivery outages), and review logs/status to capture gaps and failure modes.

Written for commit a5cbedc. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1cc60cc1-4231-48a5-a0e0-30ee5dfa430c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/investigate-provider-patient-email
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch codex/investigate-provider-patient-email

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Ben-Heerema

Ben-Heerema commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Finding: active provider-to-patient email path

The provider-to-patient send path currently appears to be the eForm email workflow:

  1. Provider opens a patient eForm.
  2. Provider clicks Email in the floating eForm toolbar.
  3. The toolbar sets emailEForm=true and saves the eForm.
  4. AddEForm2Action stores the email context in session and redirects to /email/emailComposeAction.
  5. EmailCompose2Action prepares the compose screen with patient recipient, consent status, sender accounts, attachments, encryption settings, and chart-note options.
  6. EmailSend2Action sends using the selected configured sender.

Relevant files:

  • src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js
  • src/main/java/io/github/carlos_emr/carlos/eform/actions/AddEForm2Action.java
  • src/main/java/io/github/carlos_emr/carlos/email/action/EmailCompose2Action.java
  • src/main/java/io/github/carlos_emr/carlos/email/action/EmailSend2Action.java

@sourcery-ai

sourcery-ai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a new investigation spec documenting the scope, validation plan, and placeholder for findings around provider-to-patient email functionality in CARLOS.

File-Level Changes

Change Details Files
Introduce a documentation spec outlining the investigation plan for provider-to-patient email workflows.
  • Create a new markdown spec describing the investigation purpose and scope for provider-to-patient email
  • Document the validation scenarios to exercise different email workflows and edge cases
  • Add a placeholder section indicating that detailed findings will be recorded in pull request comments
docs/specs/provider-patient-email-investigation.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA a5cbedc.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@Ben-Heerema

Copy link
Copy Markdown
Collaborator Author

Finding: CARLOS Messenger is separate from patient email

The internal Messenger UI can associate a demographic/patient with a message, but its recipient model is provider/group based. I did not find evidence that the Messenger compose flow directly sends patient-facing email.

The patient-facing email route found so far is the eForm email workflow, not /messenger/CreateMessage.

Relevant files:

  • src/main/webapp/WEB-INF/jsp/messenger/CreateMessage.jsp
  • src/main/webapp/WEB-INF/classes/struts-messenger.xml
  • src/main/java/io/github/carlos_emr/carlos/managers/MessengerDemographicManagerImpl.java

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds a new specification document, provider-patient-email-investigation.md, to outline the scope and validation plan for investigating provider-to-patient email functionality. The reviewer's feedback focuses on adhering to the repository style guide by using 'CARLOS EMR' in documentation, focusing the investigation on active email pathways rather than decommissioned legacy modules, and documenting findings directly within the file instead of using ephemeral pull request comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@@ -0,0 +1,28 @@
# Provider-to-Patient Email Investigation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

According to the repository style guide, 'CARLOS EMR' should be used in documentation headers and README files.

Suggested change
# Provider-to-Patient Email Investigation
# Provider-to-Patient Email Investigation in CARLOS EMR
References
  1. Use 'CARLOS EMR' in README files, documentation headers, GitHub templates, workflow descriptions, and user-facing messages. (link)


## Purpose

Investigate how provider-to-patient email functionality works in CARLOS, including available user flows, backend routing, configuration requirements, and expected behavior in local development.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

According to the repository style guide, 'CARLOS EMR' is preferred for documentation and user-facing content.

Suggested change
Investigate how provider-to-patient email functionality works in CARLOS, including available user flows, backend routing, configuration requirements, and expected behavior in local development.
Investigate how provider-to-patient email functionality works in CARLOS EMR, including available user flows, backend routing, configuration requirements, and expected behavior in local development.
References
  1. Use 'CARLOS EMR' in README files, documentation headers, GitHub templates, workflow descriptions, and user-facing messages. (link)

## Scope

- Identify UI entry points for provider-to-patient email.
- Trace backend actions and services used to compose and send email.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When tracing the backend actions, please note that the legacy email notifications module was recently removed as part of the active code cleanup (as specified in the repository style guide). The investigation should focus on active email pathways, such as those associated with eForms or other modern modules, to avoid spending effort on decommissioned code.

References
  1. The repository style guide notes that legacy email notifications have been removed as part of the active code cleanup. (link)


## Findings

Detailed findings will be added as pull request comments as the workflow is traced and verified.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To preserve the findings of this investigation for future reference, please document the results directly within this file before merging the pull request, rather than relying solely on ephemeral pull request comments.

Suggested change
Detailed findings will be added as pull request comments as the workflow is traced and verified.
Detailed findings will be documented directly in this file as the workflow is traced and verified.

@Ben-Heerema

Copy link
Copy Markdown
Collaborator Author

Finding: email availability depends on several gates

The eForm toolbar Email button and compose flow depend on these conditions:

  • Current user must have _email privilege.
  • Email consent tracking must be configured.
  • At least one active sender account must exist.
  • The patient demographic record must include at least one valid email address.
  • Consent status is shown in the compose screen; if the patient is not Explicit Opt-In, the eForm toolbar prompts the provider to type Yes before continuing.

If the email system is not configured, the toolbar disables the Email button with the title Consent or Sender Details Not Configured. If the user lacks _email, the toolbar hides the button.

Relevant files:

  • src/main/webapp/WEB-INF/jsp/eform/eformFloatingToolbar/eform_floating_toolbar.jspf
  • src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js
  • src/main/java/io/github/carlos_emr/carlos/managers/EmailComposeManager.java

@Ben-Heerema

Copy link
Copy Markdown
Collaborator Author

Finding: local development SMTP failure is expected without a mail service

In the current local develop runtime, email delivery reached the SMTP send step but failed because the configured sender tried to connect to localhost:25 and no SMTP server is listening there.

Observed error:

Mail server connection failed. Failed messages: org.eclipse.angus.mail.util.MailConnectException: Couldn't connect to host, port: localhost, 25; timeout -1; nested exception is: java.net.ConnectException: Connection refused

Runtime/default properties point email at local SMTP:

email.host=localhost
email.port=25

This confirms the compose/send path is active, but local delivery requires either:

  • a local SMTP/mail-catcher service listening on the configured host/port,
  • a real SMTP sender configuration,
  • or an API sender configuration such as SendGrid.

Relevant files/config:

  • /root/carlos.properties
  • src/main/resources/carlos.properties
  • src/main/java/io/github/carlos_emr/carlos/email/helpers/SMTPEmailSender.java
  • src/main/java/io/github/carlos_emr/carlos/email/core/EmailSender.java

@Ben-Heerema

Copy link
Copy Markdown
Collaborator Author

Finding: no general chart-level email composer found yet

I found the eForm-launched patient email workflow and the admin resend workflow, but I did not find a general provider chart action that opens a blank patient email composer directly from the chart without eForm context.

This may be a product gap or a workflow decision worth confirming:

  • If the intended behavior is only Email from eForm, the UI/docs should make that clear.
  • If providers should be able to send a plain message to a patient from the chart, that appears to need a separate direct-entry flow.
  • The existing email compose JSP already has a DIRECT transaction mode, but the normal patient-facing entry point found so far is eForm-based.

Relevant files:

  • src/main/webapp/WEB-INF/jsp/email/emailCompose.jsp
  • src/main/java/io/github/carlos_emr/carlos/email/action/EmailSend2Action.java
  • src/main/java/io/github/carlos_emr/carlos/email/admin/ManageEmails2Action.java

@Ben-Heerema Ben-Heerema self-assigned this Jun 30, 2026
Signed-off-by: Ben <ben@maplecreekmedical.ca>
@Ben-Heerema Ben-Heerema force-pushed the codex/investigate-provider-patient-email branch from 2693ee6 to a5cbedc Compare June 30, 2026 13:38
@sonarqubecloud

Copy link
Copy Markdown

@Ben-Heerema Ben-Heerema added discussion Open for community discussion Don't Merge labels Jun 30, 2026
@Ben-Heerema

Copy link
Copy Markdown
Collaborator Author

Current general steps to access email screen:

  1. Login as usual
  2. Open patient search (I used FAKE-FOREMAN)
  3. Open patients chart
  4. Open EChart
  5. Click the "+" on eForm
  6. Click on Rich Text Letter (an old eForm)
    It should open the following, which may also need a UI update:
image
  1. Click Email in the bottom bar
  2. It will not have consent set up for FAKE-FOREMAN, so just type "yes" to get past consent.
    It will open the following which needs some UI updates:
image

@Ben-Heerema

Ben-Heerema commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator Author

Current thoughts:
Add a warning that subject is not encrypted
Email UI seems to be broken/needs touch ups to make usability better.
While encryption is correctly turned on by default there should be at least one or two warnings when you turn it off.
This takes 5 pop-ups to access, should it be moved? into msg?
Change config for postfix so it writes to a file and give instructions to enable in dev container

@Ben-Heerema

Copy link
Copy Markdown
Collaborator Author

Finding: email compose UI layout issue is existing develop behavior

The weird email compose layout in the screenshot does not come from this investigation PR. The PR diff only adds docs/specs/provider-patient-email-investigation.md; it does not modify the email UI.

The immediate compose-screen cause is in emailCompose.jsp: the top-level form is rendered as a Bootstrap flex container:

<form id="emailComposeForm" class="d-flex flex-wrap align-items-center gap-2" ...>

All of the major sections (From, To, Subject, Body, Encryption, Additional options, Attachments) are direct child .card elements with no explicit width. In Bootstrap flex layout, those cards become content-sized flex items and wrap wherever they fit. That produces the scattered/masonry-like layout shown in the screenshot: From and To on one row, Subject beside Body, Encryption floating to the right, and the lower cards wrapping independently.

This appears to be fallout from the Bootstrap migration commit c83b95c2bcc (update: migrate Bootstrap 3.0.0 and 5.0.2 to Bootstrap 5.3.3 across project (#635)). Before that, the form used class="form-inline"; under Bootstrap 5 that class is effectively gone, but the form still behaved like a normal block element, so block-level cards would naturally stack/stretch. The migration replaced it with d-flex flex-wrap align-items-center gap-2, which changed the layout model for the entire compose form.

Suggested direction:

  • Remove the flex layout from #emailComposeForm and make it a normal vertical compose form, or wrap cards in an explicit Bootstrap grid.
  • Give top-level sections intentional width, for example w-100 for full-width rows or col-* inside a row g-3 if some sections should sit side-by-side.
  • Rework narrow internal label columns like col-sm-1 in the To and encryption cards. They are too narrow for labels such as Email(s) and Encrypt Attachments, which is why labels visually collide/truncate in the screenshot.
  • Move the encryption switch out of the <h5> text flow into a flex card header, e.g. header title on the left and switch on the right.
  • Consider adding explicit warning copy for unencrypted fields: the UI currently warns that the body is unencrypted, but the subject line is also normal email metadata and should be treated as unencrypted.

Separate issue: Rich Text Letter screenshot shows injected logout script as content

The first screenshot also shows a raw <script>(function(){...SessionHeartbeat...})()</script> block inside the Rich Text Letter editor content. That looks like LogoutBroadcastFilter output leaking into the editable eForm document.

Relevant path:

  • efmformadd_data.jsp prints thisEForm.getFormHtml() directly as the eForm HTML response.
  • LogoutBroadcastFilter is mapped broadly to /* for REQUEST and FORWARD and currently only excludes /logoutPage and /status/SessionHeartbeat.
  • The filter appends the session heartbeat/logout script to authenticated text/html responses.
  • Rich Text Letter uses an iframe/editor that reads and writes document body content through innerHTML/textContent. If the filter appends application chrome script into that eForm document, the editor can treat it as letter content instead of page chrome.

This should be tracked separately from the email compose layout fix. Likely fixes are either adding targeted exclusions for eForm content/template document routes, or changing the injection strategy so the logout script is only added to app shell pages where it cannot become provider-authored document content.

Relevant files:

  • src/main/webapp/WEB-INF/jsp/email/emailCompose.jsp
  • src/main/webapp/WEB-INF/web.xml
  • src/main/java/io/github/carlos_emr/carlos/app/LogoutBroadcastFilter.java
  • src/main/webapp/WEB-INF/jsp/eform/efmformadd_data.jsp
  • src/main/webapp/WEB-INF/eform-assets/editControl2.js

Ben-Heerema added a commit that referenced this pull request Jun 30, 2026
Apply reviewer (gemini, cubic) and SonarCloud feedback within the existing
scope:

- Render the disabled-encryption warning as a Bootstrap alert (alert-danger,
  role=alert) instead of a .card-footer placed between the card header and
  body, which produced double borders / stacked gray backgrounds and was
  semantically incorrect.
- Drop the now-unused .encryptionDisabledWarning CSS rule (the alert supplies
  the danger styling).
- Make the unencrypted subject and body PHI warnings visually prominent with
  text-danger and icon spacing (me-2).
- Associate the encryption password/clue/encrypt-attachments labels with their
  controls via for= to clear SonarCloud Web:S6853 accessibility findings.

Refs #3096

Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Ben-Heerema added a commit that referenced this pull request Jun 30, 2026
…ed section

- Place the To (recipient) and From (sender) cards side by side in a
  responsive row, To first/leftmost, using equal-height cards so the row
  stays tidy as recipients are added. Switch both to a label-on-top layout
  and attach the recipient remove button as an input-group.
- Reword the unencrypted-body warning to direct staff to put PHI in the
  encrypted section below (instead of the plain, unencrypted body), across
  all five locale bundles.

Refs #3096

Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Ben-Heerema added a commit that referenced this pull request Jun 30, 2026
Address review/CI findings on the restructured To card:

- Encode the patient name and recipient email (visible inputs and the
  submitted hidden field) with forHtmlAttribute for the HTML attribute
  context, preventing attribute-injection/XSS from demographic-derived values.
- Replace ${loop.index + 1} with the equivalent space-free ${loop.count} in
  the recipient id/for attributes so the generated id has no space character
  (github-code-quality 'malformed id attribute').

Refs #3096

Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Ben-Heerema added a commit that referenced this pull request Jun 30, 2026
Apply the two remaining defense-in-depth cleanups in the email compose JSP:

- Replace ${loop.index + 1} with the equivalent space-free ${loop.count} in
  the attachment accordion ids (header/body/object) and their data-bs-target /
  aria-controls / aria-labelledby references, so generated id attributes carry
  no space character. Rendered ids and the accordion wiring are unchanged.
- Encode the hidden emailConsentStatus value with forHtmlAttribute for the
  attribute context, consistent with the other recipient fields.

Refs #3096

Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Ben-Heerema added a commit that referenced this pull request Jun 30, 2026
The PDF-preview <object> had no child content, so assistive tech and browsers
that cannot render the inline preview were left with nothing (SonarCloud
Web:S5264). Add a fallback paragraph showing the attachment file name, encoded
with forHtml. Fallback only renders when the <object> cannot display the PDF;
the inline preview and accordion behaviour are unchanged.

Refs #3096

Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
yingbull pushed a commit that referenced this pull request Jul 2, 2026
…UX (#3100)

* fix: restore provider-to-patient email compose layout and encryption UX

The email compose form was migrated to a flex container
(d-flex flex-wrap), turning each major .card section into a content-sized
flex item that wrapped into a scattered, masonry-like layout. Restore a
deliberate vertical compose layout and clean up cramped controls:

- Replace the flex form root with a scoped .email-compose-form (max-width
  980px, full-width stacked cards).
- Widen cramped To labels (col-sm-1 -> col-sm-3) and encryption labels
  (col-sm-2 -> col-sm-3) and vertically align rows.
- Move the encryption toggle out of the <h5> into a flex card-header.
- Add a visible 'subject is not encrypted' PHI warning (only the body was
  warned before).
- Add an explicit warning whenever encryption is toggled off.

New i18n keys (email.compose.msg.unencryptedSubject,
email.compose.msg.encryptionDisabledWarning) added to all five locale
bundles. Output uses existing fmt:message vars; no user input is rendered.

Refs #3096


- Render the disabled-encryption warning as a Bootstrap alert (alert-danger,
  role=alert) instead of a .card-footer placed between the card header and
  body, which produced double borders / stacked gray backgrounds and was
  semantically incorrect.
- Drop the now-unused .encryptionDisabledWarning CSS rule (the alert supplies
  the danger styling).
- Make the unencrypted subject and body PHI warnings visually prominent with
  text-danger and icon spacing (me-2).
- Associate the encryption password/clue/encrypt-attachments labels with their
  controls via for= to clear SonarCloud Web:S6853 accessibility findings.

Refs #3096

Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>

* feat: pair To/From side by side and point body PHI warning to encrypted section

- Place the To (recipient) and From (sender) cards side by side in a
  responsive row, To first/leftmost, using equal-height cards so the row
  stays tidy as recipients are added. Switch both to a label-on-top layout
  and attach the recipient remove button as an input-group.
- Reword the unencrypted-body warning to direct staff to put PHI in the
  encrypted section below (instead of the plain, unencrypted body), across
  all five locale bundles.


- Encode the patient name and recipient email (visible inputs and the
  submitted hidden field) with forHtmlAttribute for the HTML attribute
  context, preventing attribute-injection/XSS from demographic-derived values.
- Replace ${loop.index + 1} with the equivalent space-free ${loop.count} in
  the recipient id/for attributes so the generated id has no space character
  (github-code-quality 'malformed id attribute').


The PDF-preview <object> had no child content, so assistive tech and browsers
that cannot render the inline preview were left with nothing (SonarCloud
Web:S5264). Add a fallback paragraph showing the attachment file name, encoded
with forHtml. Fallback only renders when the <object> cannot display the PDF;
the inline preview and accordion behaviour are unchanged.

Refs #3096

Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
@Ben-Heerema

Copy link
Copy Markdown
Collaborator Author

Production-readiness assessment: provider-to-patient email

Following up on the investigation findings above, here is a code-level assessment of what stands between the current email feature and production use with real patient data. This is based on reading the actual send path (EmailCompose2ActionEmailSend2ActionEmailManagerEmailSender and its SMTP/SendGrid helpers), not just the UI behavior.

TL;DR: the plumbing works and privilege gating is solid (_email checks are layered through actions, managers, and senders), but there are five hard blockers — mostly PIPEDA/PHI compliance — plus operational gaps around configuration, reliability, and cleanup. None of the currently in-flight PRs (#3097, #3100, #3101) address the blockers.


Blockers (must fix before any real patient email)

1. Consent is displayed but never enforced.
EmailComposeManager.getEmailConsentStatus() (lines 292–315) resolves the patient's ConsentType via PatientConsentManager, but the result is only rendered as a warning banner in emailCompose.jsp and echoed back as a hidden field. Nothing on the server blocks a send when the status is Explicit Opt-Out or Unknown:

  • EmailSend2Action.sendEmail() / prepareEmailFields() (lines 196–265) never read consent.
  • EmailManager.sendEmail() (lines 125–147) never checks consent.
  • The client-side validateForm() in emailCompose.jsp (lines 691–731) validates sender/recipient/subject/body only — no consent check — and the isEmailAutoSend path (lines 790–795) can auto-submit the form on page load with no consent gate at all.
  • The consent status at time of send is not recorded on the EmailLog, so a send to an opted-out patient succeeds and leaves no audit trail of the override.

This is the single biggest compliance gap. Enforcement belongs in EmailManager.sendEmail() (server-side, not JS), with the consent status persisted to the log and an explicit, audited override path if the product wants one.

2. The "encryption" scheme is weak and self-defeating.
The encryption toggle does not encrypt the email. It password-protects PDF attachments via PDFBox (EmailManager.encryptEmail(), lines 452–472; PDFEncryptionUtil). The problems:

  • The password is DOB(YYYYMMDD) + HIN (EmailComposeManager.createEmailPDFPassword(), lines 444–447) — low-entropy and derived entirely from PHI.
  • The cleartext email body includes a clue that states that exact recipe: "your date of birth in the format YYYYMMDD followed by the 10 digits of your health insurance number" (EmailCompose2Action.java:246). Anyone who intercepts the email is told how to construct the password.
  • The message body itself is never encrypted — transport TLS only. There is no S/MIME or PGP.
  • The password is stored in plaintext in emailLog.password (EmailLog.java:129), surfaced in the Manage Emails admin UI (EmailStatusResult.getPassword()), and written into the chart note by EmailNoteUtil (lines 216–285).

This needs a design decision, not a patch: random per-message passwords delivered out-of-band (SMS/phone), a portal-link model, or dropping the "encrypted" claim from the UI entirely. PR #3100's warnings are honest UX but don't change the underlying scheme.

3. EmailSend2Action accepts GET.
execute() routes purely on the method request parameter with no HTTP-verb check (EmailSend2Action.java:79–91), so a crafted GET can send an email and persist state — CSRF exposure. This violates the repo's mutator GET/HEAD-rejection contract; the fix should register the action in MutatorActionGetRejectionContractTest alongside the verb check.

4. Credentials and secrets stored in plaintext.
SMTP passwords and SendGrid API keys live unencrypted in the emailConfig.configDetails TEXT column (EmailConfig.java:42–43). Combined with the plaintext PDF passwords in emailLog (see #2), a database read exposes both transport credentials and every "encrypted" attachment's password.

5. PHI temp files are never cleaned up.
Every compose/send/resend renders patient PDFs to the temp directory (ConvertToEdoc.saveAsTempPDF, encrypted copies as encryptedPDF_<millis>.pdf via PDFEncryptionUtil.java:22), and nothing ever deletes them — no Files.delete, no deleteOnExit, no cleanup job. Unencrypted patient PDFs accumulate on disk indefinitely. (The only deletion in the flow is removeEFormData removing the eForm DB record — unrelated to temp files.)


Operational gaps

6. No admin UI for sender accounts. configureEmail.jsp (behind ViewConfigureEmail2Action) is a static documentation page with sample JSON; EmailConfig rows must be inserted into the DB by hand, with no validation until send time. A null/missing sender config throws an IllegalArgumentException from prepareEmailForOutbox() (EmailManager.java:173–179) that sendEmail()'s catch (EmailSendingException) does not catch — so misconfiguration surfaces as an HTTP 500 rather than a graceful failed-status log.

7. Synchronous, fire-once sending. EmailManager.sendEmail() runs the full network send in the HTTP request thread. No queue, no async, no retry, no rate limiting. A slow SMTP server blocks Tomcat workers; failures are terminal until an admin manually resends from Manage Emails.

8. No attachment size limits. No per-attachment or total-message cap before the SMTP send or the SendGrid Base64 encode — oversized messages just fail at the provider (SendGrid ~30 MB) with a runtime error.

9. Deliverability/ops prerequisites (outside the code). A production sender domain with SPF/DKIM/DMARC, a real relay or SendGrid account, and monitoring of FAILED rows in the email log. PR #3097's Postfix capture covers the devcontainer only.


Hardening (important, not blocking)

10. Test coverage on the core path is nearly empty. EmailSend2ActionTest has a single test (the cancel() redirect). There are no tests for EmailManager, EmailComposeManager, EmailSender, either SMTP sender, APISendGridEmailSender, or EmailNoteUtil, and no Playwright email flow in scripts/ or package.json.

11. Assorted send-path issues:

  • EmailAttachmentSettings.sanitizeSubject() strips CRLF on the eForm compose entry path, but the actual send path reads subjectEmail/recipients straight from request.getParameter with no sanitization (EmailSend2Action.java:224–238). JavaMail provides some protection; it should not be the only layer.
  • Recipient email is emitted unencoded in emailCompose.jsp:298 and :302 (value="${receiverEmail}") — low risk since the values are RFC-validated from the demographic record, but inconsistent with the null-safe encoding used everywhere else on that page.
  • APISendGridEmailSender sets setNeedClientAuth(true) on a throwaway copy of default SSL parameters (lines 166–168) — a misapplied server-side mTLS flag that does nothing — and redundantly embeds the API key in the JSON request body (lines 267–273).
  • EmailManager.java:144 logs the raw provider exception; SMTP/SendGrid error text can echo recipient addresses into log4j — a plausible PHI leak channel.

Positive findings, for balance

  • _email privilege checks are consistently applied at action, manager, and sender layers, with per-type checks (_eform/_edoc/_lab/_hrm/_form) on attachment preparation.
  • External SMTP enforces STARTTLS (required, TLSv1.2) rather than making it optional.
  • PathValidationUtils is used correctly for attachment file resolution and secure temp-file creation.
  • Attachment filenames are sanitized to generic attachment_NNN.pdf, keeping PHI out of filenames.
  • EmailLog + LogAction auditing and the chart-note integration (EmailNoteUtil) are a solid foundation.

Suggested sequencing

  1. Merge the three in-flight PRs (feat: capture devcontainer email to local file #3097, fix: restore provider-to-patient email compose layout and encryption UX #3100, fix: exclude eForm document routes from logout broadcast injection #3101) — green, review-blocked only.
  2. Fix consent enforcement (Hotfix/01232026 #1), GET rejection (catch up to dev #3), and temp-file cleanup (chore: cleanups of basic whitespace checks and enable checkstyle #5) — well-scoped, established codebase patterns exist for all three.
  3. Decide the encryption/password design ([Penify]: Documentation for commit - 2be0cc2 #2) and secrets-at-rest approach ([Penify]: Documentation for commit - c90f5bb #4) — these need product/security input before code.
  4. Sender-account admin UI (Checkstyle #6) and async/retry sending (chore: updates relating to contextpath carlos #7).
  5. Test coverage (Hibernate 6 Prep: Migrate uniqueResult() to getSingleResult() #10) and the hardening list (Hibernate 6 Prep: Migrate typed setters (setInteger, setString, etc.) to setParameter() #11), then deliverability ops (Hibernate 6 Prep: Migrate Query.list() to getResultList() #9).

Also still open from this investigation: whether a direct chart-level "email patient" entry point (outside the eForm flow) is a product requirement — that remains a question for discussion, not a defect.


Assessment generated with Claude Code; file/line references verified against develop as of 2026-07-02.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discussion Open for community discussion Don't Merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant