fix(change-email): Updated with feedback from peers

Author: Patrick-Pimentel-Bitwarden

src/Api/AdminConsole/Controllers/OrganizationUsersController.cs

@@ -25,7 +25,6 @@
 using Bit.Core.AdminConsole.Utilities.v2;
 using Bit.Core.Auth.Enums;
 using Bit.Core.Auth.Repositories;
-using Bit.Core.Auth.UserFeatures.UserEmail.Interfaces;
 using Bit.Core.Billing.Pricing;
 using Bit.Core.Context;
 using Bit.Core.Entities;
@@ -85,7 +84,7 @@ public class OrganizationUsersController : BaseAdminConsoleController
     private readonly IAdminRecoverAccountCommand _adminRecoverAccountCommand;
     private readonly AccountRecoveryV2.IAdminRecoverAccountCommand _adminRecoverAccountCommandV2;
     private readonly ISelfRevokeOrganizationUserCommand _selfRevokeOrganizationUserCommand;
-    private readonly IChangeEmailForPasswordlessUserCommand _changeEmailForPasswordlessUserCommand;
+    private readonly IChangeEmailForPasswordlessOrgUserCommand _changeEmailForPasswordlessOrgUserCommand;
 
     public OrganizationUsersController(IOrganizationRepository organizationRepository,
         IOrganizationUserRepository organizationUserRepository,
@@ -119,7 +118,7 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
         IAutomaticallyConfirmOrganizationUserCommand automaticallyConfirmOrganizationUserCommand,
         V2_RevokeOrganizationUserCommand.IRevokeOrganizationUserCommand revokeOrganizationUserCommandVNext,
         ISelfRevokeOrganizationUserCommand selfRevokeOrganizationUserCommand,
-        IChangeEmailForPasswordlessUserCommand changeEmailForPasswordlessUserCommand)
+        IChangeEmailForPasswordlessOrgUserCommand changeEmailForPasswordlessOrgUserCommand)
     {
         _organizationRepository = organizationRepository;
         _organizationUserRepository = organizationUserRepository;
@@ -153,7 +152,7 @@ public OrganizationUsersController(IOrganizationRepository organizationRepositor
         _adminRecoverAccountCommand = adminRecoverAccountCommand;
         _adminRecoverAccountCommandV2 = adminRecoverAccountCommandV2;
         _selfRevokeOrganizationUserCommand = selfRevokeOrganizationUserCommand;
-        _changeEmailForPasswordlessUserCommand = changeEmailForPasswordlessUserCommand;
+        _changeEmailForPasswordlessOrgUserCommand = changeEmailForPasswordlessOrgUserCommand;
     }
 
     [HttpGet("{id}")]
@@ -574,7 +573,7 @@ public async Task<IResult> ChangeEmailForPasswordlessUser(
             return TypedResults.NotFound();
         }
 
-        await _changeEmailForPasswordlessUserCommand.ChangeOrganizationUserEmailAsync(orgId, targetOrganizationUser, model.NewEmail);
+        await _changeEmailForPasswordlessOrgUserCommand.ChangeOrganizationUserEmailAsync(orgId, targetOrganizationUser, model.NewEmail);
         return TypedResults.NoContent();
     }
 

src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/ChangeEmailForPasswordlessOrgUserCommand.cs

@@ -0,0 +1,47 @@
+using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
+using Bit.Core.Auth.UserFeatures.UserEmail.Interfaces;
+using Bit.Core.Entities;
+using Bit.Core.Exceptions;
+using Bit.Core.Repositories;
+using Bit.Core.Utilities;
+
+namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers;
+
+public class ChangeEmailForPasswordlessOrgUserCommand : IChangeEmailForPasswordlessOrgUserCommand
+{
+    private readonly IUserRepository _userRepository;
+    private readonly IOrganizationDomainRepository _organizationDomainRepository;
+    private readonly IChangeEmailCommand _changeEmailCommand;
+
+    public ChangeEmailForPasswordlessOrgUserCommand(
+        IUserRepository userRepository,
+        IOrganizationDomainRepository organizationDomainRepository,
+        IChangeEmailCommand changeEmailCommand)
+    {
+        _userRepository = userRepository;
+        _organizationDomainRepository = organizationDomainRepository;
+        _changeEmailCommand = changeEmailCommand;
+    }
+
+    public async Task ChangeOrganizationUserEmailAsync(Guid organizationId, OrganizationUser organizationUser, string newEmail)
+    {
+        var user = await _userRepository.GetByIdAsync(organizationUser.UserId!.Value)
+            ?? throw new NotFoundException();
+
+        if (user.HasMasterPassword())
+        {
+            throw new BadRequestException("User has a master password.");
+        }
+
+        var newDomain = CoreHelpers.GetEmailDomain(newEmail);
+        var claimedDomain = await _organizationDomainRepository
+            .GetDomainByOrgIdAndDomainNameAsync(organizationId, newDomain!);
+
+        if (claimedDomain?.VerifiedDate == null)
+        {
+            throw new BadRequestException("The email domain is not claimed by the organization.");
+        }
+
+        await _changeEmailCommand.ChangeEmailAsync(user, newEmail, logOutUser: false);
+    }
+}

…ChangeEmailForPasswordlessUserCommand.cs …ngeEmailForPasswordlessOrgUserCommand.cssrc/Core/Auth/UserFeatures/UserEmail/Interfaces/IChangeEmailForPasswordlessUserCommand.cs renamed to src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IChangeEmailForPasswordlessOrgUserCommand.cs src/Core/Auth/UserFeatures/UserEmail/Interfaces/IChangeEmailForPasswordlessUserCommand.cs renamed to src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IChangeEmailForPasswordlessOrgUserCommand.cs

@@ -1,8 +1,8 @@
 using Bit.Core.Entities;
 
-namespace Bit.Core.Auth.UserFeatures.UserEmail.Interfaces;
+namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces;
 
-public interface IChangeEmailForPasswordlessUserCommand
+public interface IChangeEmailForPasswordlessOrgUserCommand
 {
     Task ChangeOrganizationUserEmailAsync(Guid organizationId, OrganizationUser organizationUser, string newEmail);
 }

src/Core/Auth/UserFeatures/UserEmail/ChangeEmailCommand.cs

@@ -0,0 +1,69 @@
+using Bit.Core.Auth.UserFeatures.UserEmail.Interfaces;
+using Bit.Core.Billing.Services;
+using Bit.Core.Entities;
+using Bit.Core.Enums;
+using Bit.Core.Exceptions;
+using Bit.Core.Platform.Push;
+using Bit.Core.Repositories;
+
+namespace Bit.Core.Auth.UserFeatures.UserEmail;
+
+public class ChangeEmailCommand : IChangeEmailCommand
+{
+    private readonly IUserRepository _userRepository;
+    private readonly IStripeSyncService _stripeSyncService;
+    private readonly IPushNotificationService _pushNotificationService;
+
+    public ChangeEmailCommand(
+        IUserRepository userRepository,
+        IStripeSyncService stripeSyncService,
+        IPushNotificationService pushNotificationService)
+    {
+        _userRepository = userRepository;
+        _stripeSyncService = stripeSyncService;
+        _pushNotificationService = pushNotificationService;
+    }
+
+    public async Task ChangeEmailAsync(User user, string newEmail, bool logOutUser = true)
+    {
+        // Querying by email exposes a limited account-enumeration vector: a distinct error response
+        // ("Email already in use.") vs. success lets a caller infer whether a Bitwarden account exists
+        // at a given address. Callers are responsible for enforcing access controls before reaching this
+        // point that bound who can probe and which addresses are reachable.
+        var existingUser = await _userRepository.GetByEmailAsync(newEmail);
+        if (existingUser != null && existingUser.Id != user.Id)
+        {
+            throw new BadRequestException("Email already in use.");
+        }
+
+        var previousEmail = user.Email;
+        var now = DateTime.UtcNow;
+        user.Email = newEmail;
+        user.EmailVerified = true;
+        user.RevisionDate = user.AccountRevisionDate = now;
+        user.LastEmailChangeDate = now;
+        await _userRepository.ReplaceAsync(user);
+
+        if (user.Gateway == GatewayType.Stripe && user.GatewayCustomerId != null)
+        {
+            try
+            {
+                await _stripeSyncService.UpdateCustomerEmailAddressAsync(
+                    user.GatewayCustomerId,
+                    user.BillingEmailAddress()!);
+            }
+            catch
+            {
+                user.Email = previousEmail;
+                user.RevisionDate = user.AccountRevisionDate = DateTime.UtcNow;
+                await _userRepository.ReplaceAsync(user);
+                throw;
+            }
+        }
+
+        if (logOutUser)
+        {
+            await _pushNotificationService.PushLogOutAsync(user.Id);
+        }
+    }
+}

src/Core/Auth/UserFeatures/UserEmail/ChangeEmailForPasswordlessUserCommand.cs

@@ -0,0 +1,8 @@
+using Bit.Core.Entities;
+
+namespace Bit.Core.Auth.UserFeatures.UserEmail.Interfaces;
+
+public interface IChangeEmailCommand
+{
+    Task ChangeEmailAsync(User user, string newEmail, bool logOutUser = true);
+}

src/Core/Auth/UserFeatures/UserEmail/Interfaces/IChangeEmailCommand.cs

@@ -91,6 +91,6 @@ private static void AddSsoQueries(this IServiceCollection services)
 
     private static void AddUserEmailCommands(this IServiceCollection services)
     {
-        services.AddScoped<IChangeEmailForPasswordlessUserCommand, ChangeEmailForPasswordlessUserCommand>();
+        services.AddScoped<IChangeEmailCommand, ChangeEmailCommand>();
     }
 }

src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs

@@ -163,6 +163,7 @@ private static void AddOrganizationUserCommands(this IServiceCollection services
         services.AddScoped<V2_RevokeUsersCommand.IRevokeOrganizationUserValidator, V2_RevokeUsersCommand.RevokeOrganizationUsersValidator>();
 
         services.AddScoped<ISelfRevokeOrganizationUserCommand, SelfRevokeOrganizationUserCommand>();
+        services.AddScoped<IChangeEmailForPasswordlessOrgUserCommand, ChangeEmailForPasswordlessOrgUserCommand>();
     }
 
     private static void AddOrganizationApiKeyCommandsQueries(this IServiceCollection services)