[PM-27882] Add SendOrganizationConfirmationCommand #6743

JimmyVo16 · 2025-12-16T22:44:07Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-27882

📔 Objective

Add SendOrganizationConfirmationCommand to send out organization confirmation emails using the newly generated MJML templates.
Update some mailer subjects to support dynamic text.
Register handlebar shared layouts.
Add tests.

📸 Screenshots

⏰ 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-12-16T22:55:27Z

Checkmarx One – Scan Summary & Details – 08031b65-4506-48ee-a556-ccd76341c7fe

New Issues (3)

Checkmarx found the following issues in this Pull Request

#	Issue	Source File / Package	Checkmarx Insight
1	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1511	details Method at line 1511 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
2	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1387	details Method at line 1387 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
3	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 245	details Method at line 245 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows t... Attack Vector

Fixed Issues (1)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	~~CSRF~~	/src/Api/Vault/Controllers/CiphersController.cs: 732

This reverts commit 98812dd.

JimmyVo16 · 2025-12-18T17:19:26Z

src/Core/Platform/Mail/Mailer/HandlebarMailRenderer.cs

-        handlebars.RegisterTemplate("FullHtmlLayout", layoutSource);
+
+        // Register Full layouts
+        var fullHtmlLayoutSource = await ReadSourceAsync(assembly, "Bit.Core.MailTemplates.Handlebars.Layouts.Full.html.hbs");


We need this so that shared layouts for the text.hbs files will work. For example, TitleContactUs.text.hbs for OrganizationConfirmationEnterpriseTeamsView.text.hbs

codecov · 2025-12-18T17:47:13Z

Codecov Report

❌ Patch coverage is 99.15966% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 55.01%. Comparing base (1b17d99) to head (c86f5ef).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...onfirmation/SendOrganizationConfirmationCommand.cs	98.68%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6743      +/-   ##
==========================================
+ Coverage   54.96%   55.01%   +0.04%     
==========================================
  Files        1930     1934       +4     
  Lines       85507    85616     +109     
  Branches     7654     7660       +6     
==========================================
+ Hits        47002    47100      +98     
- Misses      36718    36728      +10     
- Partials     1787     1788       +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.

JimmyVo16 · 2025-12-18T21:08:11Z

src/Core/Platform/Mail/Mailer/BaseMail.cs

    /// The subject of the email.
    /// </summary>
-    public abstract string Subject { get; }
+    public abstract string Subject { get; set; }


The subject for the organization confirmation email is dynamic since we have to include the organization name in it. I had to update this to include a set, which caused a cascading effect.

JimmyVo16 · 2025-12-18T21:12:49Z

.../Models/Mail/Mailer/OrganizationConfirmation/OrganizationConfirmationFamilyFreeView.text.hbs

The ViewModel names need to match the templates. Our templates use kebab-case (MJML), while the ViewModels use PascalCase. We should standardize these conventions, but this is outside the AC team’s scope. The owner of the mailer and MJML should decide this.

I don’t recommend creating a script to map one naming convention to another. That adds complexity and could introduce more issues over time.

…command

claude · 2025-12-18T21:47:51Z

Claude finished @JimmyVo16's task in 2m 22s —— View job

Code Review Complete

Overall Assessment: REQUEST CHANGES - One critical bug found

Critical Issue

Missing accessSecretsManager parameter in auto-confirm flow

In src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AutoConfirmUser/AutomaticallyConfirmOrganizationUserCommand.cs:200, the call to SendConfirmationAsync is missing the accessSecretsManager parameter:

await sendOrganizationConfirmationCommand.SendConfirmationAsync(organization, userEmail);

Should be:

await sendOrganizationConfirmationCommand.SendConfirmationAsync(organization, userEmail, accessSecretsManager);

Impact: Users who have Secrets Manager access and are auto-confirmed will receive the wrong vault URL in their confirmation email (standard vault URL instead of Secrets Manager URL).

Fix: Pass request.OrganizationUser.AccessSecretsManager (available at line 150) to the method call.

Note: ConfirmOrganizationUserCommand.cs:366 correctly passes this parameter, so this appears to be an oversight.

Additional Observations

Missing test coverage: No test verifies that accessSecretsManager parameter correctly changes the WebVaultUrl. Consider adding a test to ensure the parameter properly affects URL selection.

Question about HTML decoding: SendOrganizationConfirmationCommand.cs:26 uses WebUtility.HtmlDecode on organization names before passing to Handlebars templates (which auto-escape). If names aren't stored encoded in the database, this decode has no effect. If they are encoded, this decode-then-reencode pattern seems unnecessary.

Positive Notes

Excellent test coverage for plan routing, multiple recipients, and edge cases
Clean separation of concerns with dedicated command
Proper feature flag gating for safe rollout
Good security practices (path traversal protection in HandlebarMailRenderer)
Well-structured template system with shared layouts

r-tome

Looks good! Just a few thoughts

r-tome · 2025-12-22T11:58:58Z

...onsole/Models/Mail/Mailer/OrganizationConfirmation/OrganizationConfirmationFamilyFreeView.cs

+
+namespace Bit.Core.AdminConsole.Models.Mail.Mailer.OrganizationConfirmation;
+
+#nullable enable


This can be removed

r-tome · 2025-12-22T12:00:09Z

...nFeatures/OrganizationUsers/OrganizationConfirmation/ISendOrganizationConfirmationCommand.cs

+
+namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.OrganizationConfirmation;
+
+#nullable enable


Not necessary

...AdminConsole/Models/Mail/Mailer/OrganizationConfirmation/OrganizationConfirmationBaseView.cs

...e/Models/Mail/Mailer/OrganizationConfirmation/OrganizationConfirmationEnterpriseTeamsView.cs

r-tome · 2025-12-22T12:08:23Z

src/Core/Core.csproj

  </ItemGroup>

  <ItemGroup>
+    <Folder Include="MailTemplates\Handlebars\MJML\AdminConsole\OrganizationConfirmation\" />


❓ Is this necessary?

r-tome · 2025-12-22T12:09:13Z

...onFeatures/OrganizationUsers/OrganizationConfirmation/SendOrganizationConfirmationCommand.cs

+            View = new OrganizationConfirmationEnterpriseTeamsView
+            {
+                OrganizationName = organizationName,
+                TitleFirst = "You're confirmed as a member of ",


Consider placing this hardcoded string in a const variable because it is repeated below

Yeah, normally I abstract them when it hits three, but we can do it now.

…command

amorask-bitwarden

Looks like Billing changes are just formatting, so looks good for us.

[PM-27882] Add SendOrganizationConfirmationCommand

8f26c52

JimmyVo16 added 6 commits December 17, 2025 10:13

[PM-27882] undo

c363d80

[PM-27882] Update the subject property to be dynamic.

f9ed23f

[PM-27882] Register Handlebars layouts

98812dd

Revert "[PM-27882] Register Handlebars layouts"

f8e7a3b

This reverts commit 98812dd.

[PM-27882] Register handle bar layouts. For real

00c806a

[PM-27882] Add WebVaultUrl

2a12770

JimmyVo16 commented Dec 18, 2025

View reviewed changes

[PM-27882] fix tests

9435748

JimmyVo16 commented Dec 18, 2025

View reviewed changes

[PM-27882] Add tests for feature flag check

d8282e1

JimmyVo16 self-assigned this Dec 18, 2025

JimmyVo16 marked this pull request as ready for review December 18, 2025 21:46

JimmyVo16 requested review from a team as code owners December 18, 2025 21:46

JimmyVo16 requested review from addisonbeck, amorask-bitwarden and r-tome December 18, 2025 21:46

Merge branch 'main' into ac/pm-27882/add-sendorganizationconfirmation…

342c197

…command

JimmyVo16 added 2 commits December 19, 2025 09:34

[PM-27882] refactor accessSecretsManager

94b29ab

[PM-27882] undo

20da464

r-tome reviewed Dec 22, 2025

View reviewed changes

JimmyVo16 added 3 commits January 5, 2026 14:35

Merge branch 'main' into ac/pm-27882/add-sendorganizationconfirmation…

945576c

…command

[PM-27882] code review

7494800

[PM-27882] code review

d93d100

JimmyVo16 requested a review from r-tome January 5, 2026 20:07

Merge branch 'main' into ac/pm-27882/add-sendorganizationconfirmation…

c86f5ef

…command

r-tome approved these changes Jan 6, 2026

View reviewed changes

amorask-bitwarden approved these changes Jan 6, 2026

View reviewed changes

addisonbeck approved these changes Jan 6, 2026

View reviewed changes

JimmyVo16 merged commit 63784e1 into main Jan 6, 2026
51 checks passed

JimmyVo16 deleted the ac/pm-27882/add-sendorganizationconfirmationcommand branch January 6, 2026 21:43


		namespace Bit.Core.AdminConsole.Models.Mail.Mailer.OrganizationConfirmation;

		#nullable enable


		namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.OrganizationConfirmation;

		#nullable enable

[PM-27882] Add SendOrganizationConfirmationCommand #6743

[PM-27882] Add SendOrganizationConfirmationCommand #6743

Conversation

JimmyVo16 commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Complete

Critical Issue

Additional Observations

Positive Notes

Uh oh!

r-tome 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!

Uh oh!

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!

amorask-bitwarden left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JimmyVo16 commented Dec 16, 2025 •

edited

Loading

github-actions bot commented Dec 16, 2025 •

edited

Loading

codecov bot commented Dec 18, 2025 •

edited

Loading

claude bot commented Dec 18, 2025 •

edited

Loading