[PM-38796] Create link confirmation endpoint#7907
Conversation
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7907 +/- ##
==========================================
+ Coverage 61.36% 61.40% +0.04%
==========================================
Files 2244 2248 +4
Lines 98776 98975 +199
Branches 8928 8941 +13
==========================================
+ Hits 60610 60778 +168
- Misses 36019 36050 +31
Partials 2147 2147 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
JimmyVo16
added
ai-review
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the new invite-link confirmation endpoint ( Code Review DetailsNo new blocking findings. Verified the operator precedence in Minor, non-blocking observation (not posted inline): |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// <summary> | ||
| /// Confirms an existing membership (a pending email invitation or an accepted membership) by linking | ||
| /// it to the user, releasing the org key, and moving it straight to <see cref="OrganizationUserStatusType.Confirmed"/>. | ||
| /// Persisting via <c>ReplaceAsync</c> bumps the user's account revision date so their other devices sync. | ||
| /// </summary> | ||
| private async Task<CommandResult<OrganizationUser>> ConfirmExistingMembershipAsync( | ||
| OrganizationUser existingOrganizationUser, User user, string orgUserKey) | ||
| { | ||
| existingOrganizationUser.Status = OrganizationUserStatusType.Confirmed; | ||
| existingOrganizationUser.UserId = user.Id; | ||
| existingOrganizationUser.Email = null; | ||
| existingOrganizationUser.Key = orgUserKey; | ||
|
|
||
| await organizationUserRepository.ReplaceAsync(existingOrganizationUser); | ||
|
|
||
| return existingOrganizationUser; | ||
| } |
There was a problem hiding this comment.
❓ QUESTION: Is the omission of a device push-sync intentional here?
Details
The other confirm-to-Confirmed flows push an org-keys sync so the user's already-signed-in devices pick up the new org key immediately:
ConfirmOrganizationUserCommand→PushSyncOrgKeysAsync(plus device-registration cleanup)AutomaticallyConfirmOrganizationUserCommand→PushSyncOrganizationKeysAsync
This command releases the org key (Key = orgUserKey) and moves the membership straight to Confirmed, but injects no IPushNotificationService. The doc comment notes the ReplaceAsync revision-date bump handles other-device sync, but that relies on the next poll rather than an immediate push, and CreateConfirmedMembershipAsync (new membership via CreateAsync) has no equivalent note. Was relying solely on the revision-date bump — with no PushSyncOrgKeysAsync — a deliberate choice for this endpoint?
| private async Task<CommandResult<OrganizationUser>> ConfirmExistingMembershipAsync( | ||
| OrganizationUser existingOrganizationUser, User user, string orgUserKey) | ||
| { | ||
| existingOrganizationUser.Status = OrganizationUserStatusType.Confirmed; | ||
| existingOrganizationUser.UserId = user.Id; | ||
| existingOrganizationUser.Email = null; | ||
| existingOrganizationUser.Key = orgUserKey; | ||
|
|
||
| await organizationUserRepository.ReplaceAsync(existingOrganizationUser); | ||
|
|
||
| return existingOrganizationUser; | ||
| } |
There was a problem hiding this comment.
Details and fix
ConfirmExistingMembershipAsync moves any existing membership straight to Confirmed without checking the Free-plan admin limit. An email invitation can carry Admin/Owner Type, so a user who already administers one Free org could confirm a second Admin/Owner invite here and bypass the invariant.
The sibling AcceptOrganizationInviteLinkCommand.AcceptExistingInviteAsync guards exactly this case via ValidateFreeOrganizationAdminLimitAsync, with the comment "so the invite link can't bypass it." ConfirmOrganizationUserCommand enforces the same rule (GetCountByFreeOrganizationAdminUserAsync). Consider mirroring that check before confirming an existing Admin/Owner membership on a Free plan:
if (existingOrganizationUser.Type is OrganizationUserType.Owner or OrganizationUserType.Admin &&
organization.PlanType == PlanType.Free &&
await organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(user.Id) > 0)
{
return new ConfirmOnlyOneFreeOrganizationAdminAllowed();
}1 participant
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-38796
https://bitwarden.atlassian.net/browse/PM-40048
📔 Objective
📸 Screenshots