[PM-25240] Send Access OTP email in MJML format #6411

ike-kottlowski · 2025-10-03T20:42:41Z

🎟️ Tracking

📔 Objective

This work supports the building of *.html.hbs artifacts for the Send Access OTP email using the MJML templating library.

MJML templating is not standard in our code base yet, a feature flag is added in the SendEmailOtpRequestValidator.

Since the HandlebarsMailService is registered as a Singleton we cannot inject the FeatureService since the FeatureService is registered as Scoped. This means we had to update the call site of the email rather than in the HandlebarsMailService.

📸 Screenshots

Responsive email

Full email

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions · 2025-10-03T20:51:43Z

Checkmarx One – Scan Summary & Details – 70be726e-dac7-4254-9c58-4996d4c12028

Great job! No new security vulnerabilities introduced in this pull request

codecov · 2025-10-03T20:54:23Z

Codecov Report

❌ Patch coverage is 36.11111% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.71%. Comparing base (0fb7099) to head (a309001).

Files with missing lines	Patch %	Lines
.../Services/Implementations/HandlebarsMailService.cs	0.00%	18 Missing ⚠️
...re/Services/NoopImplementations/NoopMailService.cs	0.00%	3 Missing ⚠️
.../Core/Models/Mail/Auth/DefaultEmailOtpViewModel.cs	0.00%	1 Missing ⚠️
...idators/SendAccess/SendEmailOtpRequestValidator.cs	92.85%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6411      +/-   ##
==========================================
- Coverage   50.72%   50.71%   -0.01%     
==========================================
  Files        1866     1866              
  Lines       82702    82734      +32     
  Branches     7319     7320       +1     
==========================================
+ Hits        41949    41958       +9     
- Misses      39150    39172      +22     
- Partials     1603     1604       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

(cherry picked from commit 24783ee)

This reverts commit 24783ee.

enmande

Excellent documentation, thank you! 🥳

…ate-otp-email-mjml

…com/bitwarden/server into auth/pm-25240/update-otp-email-mjml

vleague2

Mostly typo fixes and questions!

vleague2 · 2025-10-07T19:32:34Z

src/Core/MailTemplates/Mjml/README.md

+
+There is no change on how we interact with our view models.
+
+The new comes in when we compile the `*.mjml` to `*.html.hbs`. This is the format we use so the handlebars service can apply the variables. This build pipeline process is in progress and may need to be manual done.


Sorry, I can't turn off the editor side of my brain 😂

Suggested change

The new comes in when we compile the `*.mjml` to `*.html.hbs`. This is the format we use so the handlebars service can apply the variables. This build pipeline process is in progress and may need to be manual done.

The new comes in when we compile the `*.mjml` to `*.html.hbs`. This is the format we use so the handlebars service can apply the variables. This build pipeline process is in progress and may need to be manually done.

vleague2 · 2025-10-07T19:33:59Z

src/Core/MailTemplates/Mjml/README.md

+The new comes in when we compile the `*.mjml` to `*.html.hbs`. This is the format we use so the handlebars service can apply the variables. This build pipeline process is in progress and may need to be manual done.
+
+### `txt.hbs`
+There is no change to how we create the `txt.hbs`. MJML does not impact how we create these artifacts.


If this documentation is not temporary, I would suggest documenting the existing behavior here (or pointing to where it exists). Right now it reads like a changelog, which is useful if you already know how it works, but this readme looks like it is the permanent documentation for MJML?

I'll see about adding some documentation about how we handle the text files.

I am trying to keep this readme as much about MJML as possible without muddying the waters with other flows, which is why it's vague here. It might be worth adding a README.md one directory up from this and reference the txt.hbs pattern there since it's a more general code area.

I don't think we need to run text.hbs through mjml since there's significantly less complexity there. But I might be wrong?

*.txt.hbs is not used at all in the mjml build flows. Those are maintained in the same way as before.

This PR is stale please look at this one for more up-to-date docs.

vleague2 · 2025-10-07T19:34:21Z

src/Core/MailTemplates/Mjml/README.md

+```
+This command will parse the email directory for all mjml files and attempt to compile them into `*html.hbs` files and output them into the `out/` directory. This command maintains the structure of the input directories. Meaning if an mjml template is located in `email/auth` then the compiled version will be in `out/auth`.
+
+The script was generated and works as expected. It is more fully featured than it's usage here. If interested take a look.


Suggested change

The script was generated and works as expected. It is more fully featured than it's usage here. If interested take a look.

The script was generated and works as expected. It is more fully featured than its usage here. If interested take a look.

vleague2 · 2025-10-07T19:35:12Z

src/Core/MailTemplates/Mjml/README.md

+Not all MJML tags have the same attributes, it is highly recommended to review the documentation on the official MJML website to understand the usages of each of the tags.
+
+### Custom Tags
+There is currently a `mj-bw-hero` tag that you can use from within your `*.mjml` templates. This is a good example of how to create a component that takes in attribute values allowing us to be more DRY in our development of emails. Since the attributes input is a string we are able to define whatever we need into the component, in this case `mj-bw-hero`.


Suggested change

There is currently a `mj-bw-hero` tag that you can use from within your `*.mjml` templates. This is a good example of how to create a component that takes in attribute values allowing us to be more DRY in our development of emails. Since the attributes input is a string we are able to define whatever we need into the component, in this case `mj-bw-hero`.

There is currently a `mj-bw-hero` tag that you can use from within your `*.mjml` templates. This is a good example of how to create a component that takes in attribute values allowing us to be more DRY in our development of emails. Since the attribute's input is a string we are able to define whatever we need into the component, in this case `mj-bw-hero`.

vleague2 · 2025-10-07T19:36:26Z

src/Core/MailTemplates/Mjml/README.md

+/>
+```
+
+Attributes in Custom Components are defined by the developer. They can be required or optional depending on implementation. See the documentation for more information.


Does documentation here refer to MJML official docs or what the dev has documented in a specific component?

vleague2 · 2025-10-07T19:37:05Z

src/Core/MailTemplates/Mjml/README.md

+The script was generated and works as expected. It is more fully featured than it's usage here. If interested take a look.
+
 ## Development
+MJML supports components and you can create your own components by adding them to `.mjmlconfig`. Components are simple JavaScipt that return HTML based on the attributes assigned. (see components/mj-bw-hero.js)


Suggested change

MJML supports components and you can create your own components by adding them to `.mjmlconfig`. Components are simple JavaScipt that return HTML based on the attributes assigned. (see components/mj-bw-hero.js)

MJML supports components and you can create your own components by adding them to `.mjmlconfig`. Components are simple JavaScript that return HTML based on the attributes assigned. (see components/mj-bw-hero.js)

vleague2 · 2025-10-07T19:37:17Z

src/Core/MailTemplates/Mjml/README.md


-MJML supports components and you can create your own components by adding them to `.mjmlconfig`.
+## Implementation Considerations
+We will be using the mjml templates to be generating


This looks like it got cut off?

vleague2 · 2025-10-07T19:41:44Z

src/Core/MailTemplates/Mjml/components/head.mjml

 </mj-style>
+
+<!-- Responsive icon visibility -->
+<mj-style>


So is this file used in all emails, or will other emails have to add this same css selector to get the responsive behavior?

If a mjml tempate wishes to use the styling in the head.mjml template, then it would need to be referenced like this:

<mjml> <mj-head> <mj-include path="../components/head.mjml" /> </mj-head> ... </mjml>

Then the class would be applied to tags using the css-class attribute. In mj-bw-hero.js we use it to hide the logo in the blue header area.

<mj-image src="${this.getAttribute("img-src")}" alt="" width="140px" height="140px" padding="0px" css-class="hide-small-img" />

So the hide-small-img class wouldn't do anything if the template doesn't include the head.mjml template, right? I guess I'm trying to think if having the styling separate from the class usage could cause sneaky bugs if the styling isn't included

I think my recommendation would be to always include the head.mjml as a matter of practice. Which should avoid that issue? Not sure if that's the best approach or not.

If it's expected to always be included then that seems fine to me. Can we document that somewhere?

…ate-otp-email-mjml

… formatting

Added information about build script definitions for MJML.

…ate-otp-email-mjml

vleague2 · 2025-10-14T17:52:35Z

src/Core/MailTemplates/Mjml/components/learn-more-footer.mjml

+<mj-section border-radius="0px 0px 4px 4px" background-color="#f6f6f6" padding="5px 20px 10px 20px">
+  <mj-column width="70%">
+    <mj-text line-height="24px">
+      <h3 style="font-size: 20px; margin: 0; line-height: 28px">


Does this need to be a header element? I think it will be hard to ensure that heading levels are in the correct order / not being skipped with the way the templates are joined together.

Edit -- I see this is pulled from an existing email, so totally fine if we want to defer that problem to solve later

enmande

A non-blocking observation below. If this is even worth mentioning, should it be an update considered to .editorconfig or similar?

enmande · 2025-10-15T19:26:03Z

src/Core/MailTemplates/Handlebars/Auth/SendAccessEmailOtpEmailv2.html.hbs

+
+  </body>
+</html>
+


⛏️ This is only mentioned in contrib specifically in reference to C# files, which this is not to be fair, but I notice this file is marked with no newline.

enmande · 2025-10-15T19:26:49Z

src/Core/MailTemplates/Handlebars/Auth/SendAccessEmailOtpEmailv2.text.hbs

+This code can only be used once and expires in {{Expiry}} minutes. After that you'll need to verify your email again.
+
+Bitwarden Send transmits sensitive, temporary information to others easily and securely. Learn more about Bitwarden Send or sign up to try it today.
+{{/BasicTextLayout}}


⛏️ Same as the other .hbs file comment re: newline.

ike-kottlowski added 6 commits October 1, 2025 16:48

chore: move around files; update links in hrefs;

b8a2c82

feat: first pass for mjml send email OTP

5f09170

docs: update readme

6e9ba8e

chore: rename hero.js to be more meaningful

6830ee3

feat: powershell build script

24783ee

docs: update docs to include section about build:all

4c3b895

ike-kottlowski and others added 7 commits October 6, 2025 12:14

feat: powershell build script

0085ff9

(cherry picked from commit 24783ee)

feat: update package.json

0805cce

fix: prettier version

1be934d

Revert "feat: powershell build script"

8a2e505

This reverts commit 24783ee.

feat: add watch functionality to have parity with build.sh

a016e6f

feat: update img src

5535096

Merge branch 'main' into auth/pm-25240/update-otp-email-mjml

768794b

ike-kottlowski marked this pull request as ready for review October 6, 2025 20:31

ike-kottlowski requested review from a team as code owners October 6, 2025 20:31

ike-kottlowski requested review from enmande and vleague2 October 6, 2025 20:31

enmande previously approved these changes Oct 7, 2025

View reviewed changes

ike-kottlowski added 5 commits October 7, 2025 09:43

feat: add watch and hbs output to build script

57a007d

docs: update readme for MJML

57b2fe3

docs: styling and clarity

b9a57ab

Merge branch 'auth/pm-26551/mjml-build-script' into auth/pm-25240/upd…

28580ba

…ate-otp-email-mjml

Merge branch 'auth/pm-25240/update-otp-email-mjml' of https://github.…

efad705

…com/bitwarden/server into auth/pm-25240/update-otp-email-mjml

vleague2 reviewed Oct 7, 2025

View reviewed changes

ike-kottlowski added 2 commits October 7, 2025 17:21

docs: adding docs for email templates

a6d8984

docs: update based on feedback

f44e71a

ike-kottlowski marked this pull request as draft October 7, 2025 21:33

ike-kottlowski and others added 6 commits October 7, 2025 17:34

Merge branch 'auth/pm-26551/mjml-build-script' into auth/pm-25240/upd…

9987de3

…ate-otp-email-mjml

feat: change powershell to node build script for cross platform; also…

7828a08

… formatting

Merge branch 'main' into auth/pm-26551/mjml-build-script

115ca4b

docs: updated based on feedback

33251e0

Update README with build script details

d50eb5d

Added information about build script definitions for MJML.

Merge branch 'auth/pm-26551/mjml-build-script' into auth/pm-25240/upd…

ce732e0

…ate-otp-email-mjml

ike-kottlowski dismissed enmande’s stale review via ce732e0 October 9, 2025 15:23

Merge branch 'main' into auth/pm-25240/update-otp-email-mjml

dc15e85

ike-kottlowski requested review from enmande and vleague2 October 10, 2025 19:25

ike-kottlowski marked this pull request as ready for review October 10, 2025 19:26

ike-kottlowski added 6 commits October 10, 2025 18:58

feat: update send email

93f6457

feat: add v2 email for Send OTP

ca2dd28

feat: update email view model

9d8d5cc

feat: add feature flag and update call site in request validator

7e8f921

chore: dotnet format

111c77b

test: fix tests

7520fbb

vleague2 reviewed Oct 14, 2025

View reviewed changes

Merge branch 'main' into auth/pm-25240/update-otp-email-mjml

a309001

enmande approved these changes Oct 15, 2025

View reviewed changes


		There is no change on how we interact with our view models.

		The new comes in when we compile the `.mjml` to `.html.hbs`. This is the format we use so the handlebars service can apply the variables. This build pipeline process is in progress and may need to be manual done.

	The script was generated and works as expected. It is more fully featured than it's usage here. If interested take a look.
	The script was generated and works as expected. It is more fully featured than its usage here. If interested take a look.

	There is currently a `mj-bw-hero` tag that you can use from within your `*.mjml` templates. This is a good example of how to create a component that takes in attribute values allowing us to be more DRY in our development of emails. Since the attributes input is a string we are able to define whatever we need into the component, in this case `mj-bw-hero`.
	There is currently a `mj-bw-hero` tag that you can use from within your `*.mjml` templates. This is a good example of how to create a component that takes in attribute values allowing us to be more DRY in our development of emails. Since the attribute's input is a string we are able to define whatever we need into the component, in this case `mj-bw-hero`.

	MJML supports components and you can create your own components by adding them to `.mjmlconfig`. Components are simple JavaScipt that return HTML based on the attributes assigned. (see components/mj-bw-hero.js)
	MJML supports components and you can create your own components by adding them to `.mjmlconfig`. Components are simple JavaScript that return HTML based on the attributes assigned. (see components/mj-bw-hero.js)

+                </body>
+              </html>
+                    No newline at end of file

[PM-25240] Send Access OTP email in MJML format #6411

Are you sure you want to change the base?

[PM-25240] Send Access OTP email in MJML format #6411

Uh oh!

Conversation

ike-kottlowski commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

Responsive email

Full email

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

github-actions bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Great job! No new security vulnerabilities introduced in this pull request

Uh oh!

codecov bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

enmande left a comment

Choose a reason for hiding this comment

Uh oh!

vleague2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enmande left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ike-kottlowski commented Oct 3, 2025 •

edited

Loading

github-actions bot commented Oct 3, 2025 •

edited

Loading

codecov bot commented Oct 3, 2025 •

edited

Loading