Auth/PM-40521 - Registration - Optionally carry SealedOpenOrgInviteData through email verification#8037
Draft
JaredSnider-Bitwarden wants to merge 4 commits into
Conversation
…to verification email Adds an optional passthrough field on the anonymous registration-start endpoint (POST identity/accounts/register/send-verification-email) that carries an opaque wire artifact produced by the SDK's seal_open_org_invite_data (PM-40520). The server never parses the value — it flows from the request model through the command layer into IMailService.SendRegistrationVerificationEmailAsync and is appended to the verification-email URL fragment so the verification-email tab can unseal it after auto-login. Key details: - Threaded as a trailing optional parameter so existing callers compile unchanged; four call sites updated for arity (controller, tokenable-agnostic command, HandlebarsMailService, NoopMailService, plus the IdentityApplication test factory's IMailService substitution). - Not woven into the RegistrationEmailVerificationTokenable payload — the tokenable is a stateless IDataProtectorTokenFactory-protected blob with no server-side row to attach to. The sealed data rides the mail model directly. - URL append uses '&' (the fragment-scoped query is already opened by 'token') and sits after fromMarketing so today's non-open-invite flow is byte-for-byte unchanged when the field is absent. - Anti-enumeration branch (existing account) silently discards the sealed data — matches today's 200+null shape, per the tasks.md known limitation. - 4096-char MaxLength cap: measured a realistic SDK output at 1202 chars, so 4096 provides ~3.4x headroom while tightly bounding an anonymous request body. Coverage: 4 new RegisterVerifyEmail URL unit tests (present/absent, and combined with fromMarketing), 2 new command unit tests (new-user forwards / existing-user discards), and 3 new AccountsController integration tests exercising the endpoint with the field present, oversized (400), and against the existing-user short-circuit branch.
github-actions
Bot
added
the
platform-community
Contributor
ℹ️ Limited Platform familiarityThis PR touches a platform-community area. The Platform team owns this code but does not actively develop it, so a Platform review here may be less authoritative than usual. |
Contributor
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the addition of an optional Code Review DetailsNo findings at or above the confidence threshold. Notes considered and intentionally not posted as inline comments:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an optional opaque SealedOpenOrgInviteData passthrough from the anonymous registration-start endpoint through command + mail layers into the verification-email URL fragment, enabling the client to unseal open-invite data after auto-login.
Changes:
- Extends
RegisterSendVerificationEmailRequestModel,ISendVerificationEmailForRegistrationCommand, andIMailService.SendRegistrationVerificationEmailAsyncwith an optionalsealedOpenOrgInviteDataparameter and threads it through controller/command/mail implementations. - Updates
RegisterVerifyEmail.Urlto appendsealedOpenOrgInviteDatawithin the fragment-scoped query when present. - Adds/updates unit + integration tests to cover URL composition, forwarding behavior, and MaxLength validation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs | Updates IMailService substitute setup to match new mail method arity. |
| test/Identity.IntegrationTest/Controllers/AccountsControllerTests.cs | Adds integration tests for passthrough present, oversized validation, and existing-user discard behavior. |
| test/Core.Test/Auth/UserFeatures/Registration/SendVerificationEmailForRegistrationCommandTests.cs | Updates existing assertions for new mail signature; adds new forwarding/discard unit tests. |
| test/Core.Test/Auth/Models/Mail/RegisterVerifyEmailTests.cs | Adds new unit tests validating verification URL construction with/without sealed data and fromMarketing. |
| src/Identity/Controllers/AccountsController.cs | Threads SealedOpenOrgInviteData from request model into the registration verification command call. |
| src/Core/Platform/Mail/NoopMailService.cs | Extends noop implementation signature with optional sealedOpenOrgInviteData. |
| src/Core/Platform/Mail/IMailService.cs | Extends interface method signature to include optional sealedOpenOrgInviteData. |
| src/Core/Platform/Mail/HandlebarsMailService.cs | Encodes and passes sealed data into the mail model used by the Handlebars templates. |
| src/Core/Auth/UserFeatures/Registration/ISendVerificationEmailForRegistrationCommand.cs | Extends command interface signature and documents new parameter behavior. |
| src/Core/Auth/UserFeatures/Registration/Implementations/SendVerificationEmailForRegistrationCommand.cs | Forwards sealed data to mail service for new-user branch; discards it on existing-user branch. |
| src/Core/Auth/Models/Mail/RegisterVerifyEmail.cs | Appends sealedOpenOrgInviteData to the verification URL fragment when non-empty. |
| src/Core/Auth/Models/Api/Request/Accounts/RegisterSendVerificationEmailRequestModel.cs | Adds optional SealedOpenOrgInviteData with a 4096 max length constraint and documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+15
to
+16
| /// Silently discarded on the existing-account anti-enumeration branch (matching the 200+null | ||
| /// shape today's flow returns for that branch). |
Comment on lines
+90
to
+93
| // with no server-side row to attach to. The value flows straight to the mail service | ||
| // and is appended to the URL fragment. On the existing-user short-circuit branch | ||
| // below, the sealed data is silently discarded, matching today's 200+null anti- | ||
| // enumeration behavior. |
Comment on lines
+31
to
+32
| [MaxLength(SealedOpenOrgInviteDataMaxLength)] | ||
| public string? SealedOpenOrgInviteData { get; set; } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8037 +/- ##
===========================================
+ Coverage 14.96% 62.51% +47.55%
===========================================
Files 1389 2301 +912
Lines 60360 100156 +39796
Branches 4793 9027 +4234
===========================================
+ Hits 9030 62613 +53583
+ Misses 51175 35361 -15814
- Partials 155 2182 +2027 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
JaredSnider-Bitwarden
changed the title
Copilot review feedback: the doc and inline comment described the existing-user short-circuit as "200+null", but the controller actually returns 204 NoContent. Rephrase to describe the intent (anti-enumeration — response indistinguishable from the new-account branch) without pinning to a specific HTTP status.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
This reverts commit fbbb10a.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-40521
Tech breakdown: https://github.com/bitwarden/tech-breakdowns/tree/main/auth/PM-39706-open-invite-registration-crossing
📔 Objective
Add the optional ability to registration start to carry
SealedOpenOrgInviteDatathrough the email verification process in order to securely deliver theSealedOpenOrgInviteDataback to the web vault for the user to complete registration and then be able to unseal the data and attempt to accept the invite and join the org.📸 Screenshots