[PM-28769] [PM-28768] [PM-28772] Welcome email bug fixes

[ike-kottlowski]

🎟️ Tracking

PM-28768
PM-28769
PM-28772

📔 Objective

PM-28768 - Updated conditional to include PlanType.Families2025 in the logic to send the correct email; added docs in PlanType to alert Billing of the dependency.

PM-28769 - Fixed subtitle to match figma; also updated to use headStyle so styling is only applied once per component rather than each time a component is used.

PM-28772 - Added config to the appsettings.development.json for the SSO project to allow for emails to be sent on local dev. Alerted SRE to possible missing configuration for SSO project.

📸 Screenshots

Individual User

indv_welcome_email.mp4

Organization User + JIT User

JIT_org_Welcome_Email.mp4

Families User

firefox_W3tuKzoSt6.mp4

⏰ 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

[claude]

Claude finished @ike-kottlowski's task —— View job

---

Code Review Complete ✅

Tasks

• Read project guidelines and changed files
• Review PM-28768 fix (Families2025 plan type)
• Review PM-28769 fix (email template styling)
• Review PM-28772 fix (SSO SMTP config)
• Check for security issues and code quality
• Address existing review comments
• Provide comprehensive feedback

---

Summary

This PR successfully addresses three welcome email bugs. The implementation is clean and follows good practices. I've identified one critical issue with the production appsettings.json configuration and several recommendations for improvement.

Critical Issues ❌

1. Localhost SMTP in Production Config

Location: bitwarden_license/src/Sso/appsettings.json:17-20

The SMTP localhost configuration was added to the production appsettings.json file, not just Development. This could cause production email failures.

Issue: The author confirmed this is "only relevant for development environments" but it's in the production config file.

Recommendation: Remove the SMTP config from appsettings.json (production) and keep it only in appsettings.Development.json. Production environments should rely on SendGrid (line 14) or environment-specific configuration overrides.

# In bitwarden_license/src/Sso/appsettings.json
 "mail": {
   "sendGridApiKey": "SECRET",
   "amazonConfigSetName": "Email",
-  "replyToEmail": "[email protected]",
-  "smtp": {
-    "host": "localhost",
-    "port": 10250
-  }
+  "replyToEmail": "[email protected]"
 }

Issues Resolved ✅

PM-28768: Families2025 Plan Type Bug Fix

Location: src/Core/Auth/UserFeatures/Registration/Implementations/RegisterUserCommand.cs:459

Previous Code:

if (organization.PlanType is PlanType.FamiliesAnnually
    or PlanType.FamiliesAnnually2019
    or PlanType.Free)

New Code:

if (organization.PlanType.GetProductTier() is ProductTierType.Free or ProductTierType.Families)

👍 Excellent improvement! This change:

• ✅ Uses the GetProductTier() extension method (addressing amorask-bitwarden's suggestion)
• ✅ Future-proof solution that automatically handles new family plan types
• ✅ More maintainable - no need to update the list when new family plan variants are added
• ✅ Test coverage added: PlanType.FamiliesAnnually2025 test case added (line 1020)
• ✅ Correctly adds required import: using Bit.Core.Billing.Extensions;

This is the superior approach compared to the array-based solution mentioned in review comments.

PM-28769: Email Subtitle Fix

Location: Multiple .mjml files

Changes:

• Fixed subtitle from "Let's get set up to autofill." → "Let's get you set up to autofill."
• Changed apostrophe from ASCII ' (U+0027) to typographic ' (U+2019)

✅ Matches Figma design (confirmed by author)
✅ Consistent across all three welcome email templates
✅ HTML templates regenerated correctly

CSS Fix: The mj-bw-icon-row.js component had pre-existing CSS syntax errors that were correctly fixed in this PR:

Before (broken):

componentHeadStyle = (breakpoint) => {
  return `
    @media only screen and (max-width:${breakpoint}): {  // ← Extra colon
      ".mj-bw-icon-row-text": {                          // ← JSON syntax
        padding-left: "5px !important",                   // ← Quoted values

After (correct):

headStyle = (breakpoint) => {
  return `
    @media only screen and (max-width:${breakpoint}) {   // ✅ Valid media query
      .mj-bw-icon-row-text {                             // ✅ Valid CSS selector
        padding-left: 5px !important;                     // ✅ Unquoted values

👍 This fix resolves the critical CSS syntax error flagged in previous reviews while also renaming the method to match MJML conventions.

PM-28772: SSO Local Development Email Config

Location: bitwarden_license/src/Sso/appsettings.Development.json:29-34

✅ Correctly added to appsettings.Development.json for local development
⚠️ However: Also incorrectly added to production appsettings.json (see Critical Issues above)

Security & Best Practices ✅

• ✅ No security vulnerabilities introduced
• ✅ No PII or sensitive data exposed
• ✅ No changes to authentication or encryption logic
• ✅ Proper use of extension methods for type checking
• ✅ Test coverage maintained and expanded
• ✅ Code follows project style guidelines
• ✅ No code regions used (per CLAUDE.md rules)

Additional Observations

Test Coverage 📝

The test suite correctly validates the new behavior:

• Test added for PlanType.FamiliesAnnually2025 (line 1020)
• Uses the same test data attributes as other family plan types
• Validates that family welcome email is sent for all family plan types

Maintainability Improvements 🌱

The switch to GetProductTier() significantly improves maintainability:

• No need to update multiple locations when new plan types are added
• Leverages existing billing tier abstraction
• Self-documenting code (intent is clearer)

Recommendations

1. Required: Remove SMTP config from production appsettings.json
2. Optional: Consider documenting the email sending flow with the new tier-based logic
3. Optional: Verify SRE has been alerted about SSO email config (mentioned in PR description)

Verdict

Approval Status: ✅ Approve with required changes

Once the SMTP config is removed from production appsettings.json, this PR is good to merge. The core fixes are solid, test coverage is good, and the implementation follows best practices.

---

[github-actions]

Checkmarx One – Scan Summary & Details – 7c855bb0-c859-4e72-abfa-9e2f040a9fc3

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.40%. Comparing base (9573cab) to head (125c6e5).
⚠️ Report is 61 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #6644       +/-   ##
===========================================
+ Coverage   13.38%   53.40%   +40.01%     
===========================================
  Files        1125     1917      +792     
  Lines       49593    85464    +35871     
  Branches     3914     7667     +3753     
===========================================
+ Hits         6640    45644    +39004     
+ Misses      42831    38054     -4777     
- Partials      122     1766     +1644     

☔ 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.

[enmande]

Sorry, this is literally the same question in 6 places. There isn't any occurrence of that copy which didn't change, and I think I understand typographically this is often the preferred character, so I imagine it's intended, but just wanted to check.

[amorask-bitwarden]

This works perfectly fine. If you prefer to not have to manage these in the future, you can use the PlanType.GetProductTierType extension.

if (organization.PlanType.GetProductTierType() is ProductTierType.Free or ProductTierType.Families)

[ike-kottlowski]

Awesome suggestion updated the code!