diff --git a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs index 35f791a6b2ac..8d376e168ea4 100644 --- a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs +++ b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs @@ -47,6 +47,7 @@ private readonly IRotationValidator, IEnumerable> _deviceValidator; private readonly IKeyConnectorConfirmationDetailsQuery _keyConnectorConfirmationDetailsQuery; private readonly ISetKeyConnectorKeyCommand _setKeyConnectorKeyCommand; + private readonly IConvertUserToKeyConnectorCommand _convertUserToKeyConnectorCommand; public AccountsKeyManagementController(IUserService userService, IOrganizationUserRepository organizationUserRepository, @@ -64,7 +65,8 @@ public AccountsKeyManagementController(IUserService userService, IRotationValidator, IEnumerable> webAuthnKeyValidator, IRotationValidator, IEnumerable> deviceValidator, - ISetKeyConnectorKeyCommand setKeyConnectorKeyCommand) + ISetKeyConnectorKeyCommand setKeyConnectorKeyCommand, + IConvertUserToKeyConnectorCommand convertUserToKeyConnectorCommand) { _userService = userService; _regenerateUserAsymmetricKeysCommand = regenerateUserAsymmetricKeysCommand; @@ -80,6 +82,7 @@ public AccountsKeyManagementController(IUserService userService, _deviceValidator = deviceValidator; _keyConnectorConfirmationDetailsQuery = keyConnectorConfirmationDetailsQuery; _setKeyConnectorKeyCommand = setKeyConnectorKeyCommand; + _convertUserToKeyConnectorCommand = convertUserToKeyConnectorCommand; } [HttpPost("key-management/regenerate-keys")] @@ -219,7 +222,7 @@ public async Task PostConvertToKeyConnectorAsync() throw new UnauthorizedAccessException(); } - var result = await _userService.ConvertToKeyConnectorAsync(user, null); + var result = await _convertUserToKeyConnectorCommand.ConvertAsync(user); if (result.Succeeded) { return; @@ -242,7 +245,7 @@ public async Task PostEnrollToKeyConnectorAsync([FromBody] KeyConnectorEnrollmen throw new UnauthorizedAccessException(); } - var result = await _userService.ConvertToKeyConnectorAsync(user, model.KeyConnectorKeyWrappedUserKey); + var result = await _convertUserToKeyConnectorCommand.ConvertAsync(user, model.KeyConnectorKeyWrappedUserKey); if (result.Succeeded) { return; diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 46f7c47c1c8e..ff0cc32179ea 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -321,4 +321,32 @@ public interface IMasterPasswordService /// describing validation failures. /// Task> SaveUpdateExistingMasterPasswordAsync(User user, UpdateExistingPasswordData updateExistingData); + + /// + /// Clears the user's master password credential on the object in + /// memory only — nulls and + /// together to preserve the credential/salt invariant. + /// + /// + /// Use when: a flow legitimately removes a user's master password (today, only the + /// Key Connector conversion path). The caller is responsible for persistence and for any + /// related state changes (e.g., setting , writing the + /// Key Connector-wrapped user key, logging an event). + /// + /// + /// + /// Side effects on : + /// + /// MasterPassword → null + /// MasterPasswordSalt → null + /// RevisionDate and AccountRevisionDate → now + /// LastPasswordChangeDate is intentionally NOT updated — this is credential + /// removal, not a password change. + /// + /// + /// + /// + /// The user whose master password credential will be cleared. + /// The mutated . + User PrepareClearMasterPassword(User user); } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 12d3cb228263..da0bc1f3e74c 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -226,6 +226,19 @@ public async Task> SaveUpdateExistingMasterPassword return result.AsT0; } + public User PrepareClearMasterPassword(User user) + { + EnsureUserIsHydrated(user); + + user.MasterPassword = null; + user.MasterPasswordSalt = null; + + var now = _timeProvider.GetUtcNow().UtcDateTime; + user.RevisionDate = user.AccountRevisionDate = now; + + return user; + } + /// /// Server-side hashes the client-supplied master password authentication hash /// () and writes the result to diff --git a/src/Core/KeyManagement/Commands/ConvertUserToKeyConnectorCommand.cs b/src/Core/KeyManagement/Commands/ConvertUserToKeyConnectorCommand.cs new file mode 100644 index 000000000000..fbe09443ccd2 --- /dev/null +++ b/src/Core/KeyManagement/Commands/ConvertUserToKeyConnectorCommand.cs @@ -0,0 +1,79 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Commands.Interfaces; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.KeyManagement.Commands; + +public class ConvertUserToKeyConnectorCommand : IConvertUserToKeyConnectorCommand +{ + private readonly IUserRepository _userRepository; + private readonly IEventService _eventService; + private readonly ICurrentContext _currentContext; + private readonly IMasterPasswordService _masterPasswordService; + private readonly IdentityErrorDescriber _identityErrorDescriber; + private readonly ILogger _logger; + + public ConvertUserToKeyConnectorCommand( + IUserRepository userRepository, + IEventService eventService, + ICurrentContext currentContext, + IMasterPasswordService masterPasswordService, + IdentityErrorDescriber identityErrorDescriber, + ILogger logger) + { + _userRepository = userRepository; + _eventService = eventService; + _currentContext = currentContext; + _masterPasswordService = masterPasswordService; + _identityErrorDescriber = identityErrorDescriber; + _logger = logger; + } + + public async Task ConvertAsync(User user, string? keyConnectorKeyWrappedUserKey = null) + { + ArgumentNullException.ThrowIfNull(user); + + var canUseResult = CheckCanUseKeyConnector(user); + if (canUseResult != null) + { + return canUseResult; + } + + _masterPasswordService.PrepareClearMasterPassword(user); + user.UsesKeyConnector = true; + + if (!string.IsNullOrWhiteSpace(keyConnectorKeyWrappedUserKey)) + { + user.Key = keyConnectorKeyWrappedUserKey; + } + + await _userRepository.ReplaceAsync(user); + await _eventService.LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); + + return IdentityResult.Success; + } + + private IdentityResult? CheckCanUseKeyConnector(User user) + { + if (user.UsesKeyConnector) + { + _logger.LogWarning("Already uses Key Connector."); + return IdentityResult.Failed(_identityErrorDescriber.UserAlreadyHasPassword()); + } + + if (_currentContext.Organizations.Any(u => + u.Type is OrganizationUserType.Owner or OrganizationUserType.Admin)) + { + throw new BadRequestException("Cannot use Key Connector when admin or owner of an organization."); + } + + return null; + } +} diff --git a/src/Core/KeyManagement/Commands/Interfaces/IConvertUserToKeyConnectorCommand.cs b/src/Core/KeyManagement/Commands/Interfaces/IConvertUserToKeyConnectorCommand.cs new file mode 100644 index 000000000000..4d50118bf3fa --- /dev/null +++ b/src/Core/KeyManagement/Commands/Interfaces/IConvertUserToKeyConnectorCommand.cs @@ -0,0 +1,14 @@ +using Bit.Core.Entities; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.KeyManagement.Commands.Interfaces; + +/// +/// Converts an existing master-password user into a Key Connector user — clears the master +/// password credential, marks the account as using Key Connector, optionally rotates the +/// user key wrap to a Key-Connector-wrapped form, persists the user, and emits an event. +/// +public interface IConvertUserToKeyConnectorCommand +{ + Task ConvertAsync(User user, string? keyConnectorKeyWrappedUserKey = null); +} diff --git a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs index 96f990c29981..3fec4ea216d8 100644 --- a/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs +++ b/src/Core/KeyManagement/KeyManagementServiceCollectionExtensions.cs @@ -30,6 +30,7 @@ private static void AddKeyManagementCommands(this IServiceCollection services) services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); } private static void AddKeyManagementQueries(this IServiceCollection services) diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index b805e3a517fe..ce9c6328bc91 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -34,7 +34,6 @@ Task ChangeEmailAsync(User user, string masterPassword, string n // TODO removed with https://bitwarden.atlassian.net/browse/PM-27328 [Obsolete("Use ISetKeyConnectorKeyCommand instead. This method will be removed in a future version.")] Task SetKeyConnectorKeyAsync(User user, string key, string orgIdentifier); - Task ConvertToKeyConnectorAsync(User user, string keyConnectorKeyWrappedUserKey); Task AdminResetPasswordAsync(OrganizationUserType type, Guid orgId, Guid id, string newMasterPassword, string key); [Obsolete("Use IReplaceAdminSetTemporaryPasswordCommand instead. To be removed in PM-33141.")] Task UpdateTempPasswordAsync(User user, string newMasterPassword, string key, string hint); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 1622cce63e9e..446853583f67 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -552,29 +552,6 @@ public async Task SetKeyConnectorKeyAsync(User user, string key, return IdentityResult.Success; } - public async Task ConvertToKeyConnectorAsync(User user, string keyConnectorKeyWrappedUserKey = null) - { - var identityResult = CheckCanUseKeyConnector(user); - if (identityResult != null) - { - return identityResult; - } - - user.RevisionDate = user.AccountRevisionDate = DateTime.UtcNow; - user.MasterPassword = null; - user.UsesKeyConnector = true; - - if (!string.IsNullOrWhiteSpace(keyConnectorKeyWrappedUserKey)) - { - user.Key = keyConnectorKeyWrappedUserKey; - } - - await _userRepository.ReplaceAsync(user); - await _eventService.LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); - - return IdentityResult.Success; - } - private IdentityResult CheckCanUseKeyConnector(User user) { if (user == null) diff --git a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index da5ff282dfba..73a9d3b0276e 100644 --- a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -351,6 +351,7 @@ public async Task PostConvertToKeyConnectorAsync_Success() var user = await _userRepository.GetByEmailAsync(ssoUserEmail); Assert.NotNull(user); Assert.Null(user.MasterPassword); + Assert.Null(user.MasterPasswordSalt); Assert.True(user.UsesKeyConnector); Assert.Equal(DateTime.UtcNow, user.RevisionDate, TimeSpan.FromMinutes(1)); Assert.Equal(DateTime.UtcNow, user.AccountRevisionDate, TimeSpan.FromMinutes(1)); @@ -404,6 +405,7 @@ public async Task PostEnrollToKeyConnectorAsync_Success() var user = await _userRepository.GetByEmailAsync(ssoUserEmail); Assert.NotNull(user); Assert.Null(user.MasterPassword); + Assert.Null(user.MasterPasswordSalt); Assert.True(user.UsesKeyConnector); Assert.Equal(request.KeyConnectorKeyWrappedUserKey, user.Key); Assert.Equal(DateTime.UtcNow, user.RevisionDate, TimeSpan.FromMinutes(1)); diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 458757b9e0f6..f1e7ca923dde 100644 --- a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -477,8 +477,8 @@ public async Task PostConvertToKeyConnectorAsync_UserNull_Throws( await Assert.ThrowsAsync(() => sutProvider.Sut.PostConvertToKeyConnectorAsync()); - await sutProvider.GetDependency().ReceivedWithAnyArgs(0) - .ConvertToKeyConnectorAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .ConvertAsync(Arg.Any(), Arg.Any()); } [Theory] @@ -489,8 +489,8 @@ public async Task PostConvertToKeyConnectorAsync_ConvertToKeyConnectorFails_Thro { sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()) .Returns(expectedUser); - sutProvider.GetDependency() - .ConvertToKeyConnectorAsync(Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .ConvertAsync(Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(new IdentityError { Description = "convert to key connector error" })); var badRequestException = @@ -498,8 +498,8 @@ public async Task PostConvertToKeyConnectorAsync_ConvertToKeyConnectorFails_Thro Assert.Equal(1, badRequestException.ModelState!.ErrorCount); Assert.Equal("convert to key connector error", badRequestException.ModelState.Root.Errors[0].ErrorMessage); - await sutProvider.GetDependency().Received(1) - .ConvertToKeyConnectorAsync(Arg.Is(expectedUser), Arg.Any()); + await sutProvider.GetDependency().Received(1) + .ConvertAsync(Arg.Is(expectedUser), Arg.Any()); } [Theory] @@ -510,14 +510,14 @@ public async Task PostConvertToKeyConnectorAsync_ConvertToKeyConnectorSucceeds_O { sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()) .Returns(expectedUser); - sutProvider.GetDependency() - .ConvertToKeyConnectorAsync(Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .ConvertAsync(Arg.Any(), Arg.Any()) .Returns(IdentityResult.Success); await sutProvider.Sut.PostConvertToKeyConnectorAsync(); - await sutProvider.GetDependency().Received(1) - .ConvertToKeyConnectorAsync(Arg.Is(expectedUser), Arg.Any()); + await sutProvider.GetDependency().Received(1) + .ConvertAsync(Arg.Is(expectedUser), Arg.Any()); } [Theory] @@ -530,8 +530,8 @@ public async Task PostEnrollToKeyConnectorAsync_UserNull_Throws( await Assert.ThrowsAsync(() => sutProvider.Sut.PostEnrollToKeyConnectorAsync(data)); - await sutProvider.GetDependency().ReceivedWithAnyArgs(0) - .ConvertToKeyConnectorAsync(Arg.Any(), Arg.Any()); + await sutProvider.GetDependency().ReceivedWithAnyArgs(0) + .ConvertAsync(Arg.Any(), Arg.Any()); } [Theory] @@ -543,8 +543,8 @@ public async Task PostEnrollToKeyConnectorAsync_ConvertToKeyConnectorFails_Throw { sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()) .Returns(expectedUser); - sutProvider.GetDependency() - .ConvertToKeyConnectorAsync(Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .ConvertAsync(Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(new IdentityError { Description = "convert to key connector error" })); var badRequestException = @@ -552,8 +552,8 @@ public async Task PostEnrollToKeyConnectorAsync_ConvertToKeyConnectorFails_Throw Assert.Equal(1, badRequestException.ModelState!.ErrorCount); Assert.Equal("convert to key connector error", badRequestException.ModelState.Root.Errors[0].ErrorMessage); - await sutProvider.GetDependency().Received(1) - .ConvertToKeyConnectorAsync(Arg.Is(expectedUser), Arg.Is(data.KeyConnectorKeyWrappedUserKey)); + await sutProvider.GetDependency().Received(1) + .ConvertAsync(Arg.Is(expectedUser), Arg.Is(data.KeyConnectorKeyWrappedUserKey)); } [Theory] @@ -565,14 +565,14 @@ public async Task PostEnrollToKeyConnectorAsync_ConvertToKeyConnectorSucceeds_Ok { sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()) .Returns(expectedUser); - sutProvider.GetDependency() - .ConvertToKeyConnectorAsync(Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .ConvertAsync(Arg.Any(), Arg.Any()) .Returns(IdentityResult.Success); await sutProvider.Sut.PostEnrollToKeyConnectorAsync(data); - await sutProvider.GetDependency().Received(1) - .ConvertToKeyConnectorAsync(Arg.Is(expectedUser), Arg.Is(data.KeyConnectorKeyWrappedUserKey)); + await sutProvider.GetDependency().Received(1) + .ConvertAsync(Arg.Is(expectedUser), Arg.Is(data.KeyConnectorKeyWrappedUserKey)); } [Theory] diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index 121751efc274..8de1006293e2 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -1179,4 +1179,35 @@ private static SetInitialPasswordData BuildSetInitialDataForUser(User user) } } + // --- PrepareClearMasterPassword --- + + [Theory, BitAutoData] + public void PrepareClearMasterPassword_HydratedUser_ClearsCredentialAndUpdatesRevisionDates(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.MasterPasswordSalt = "existing-salt"; + var originalLastPasswordChangeDate = user.LastPasswordChangeDate; + + var result = sutProvider.Sut.PrepareClearMasterPassword(user); + + var expectedTime = sutProvider.GetDependency().GetUtcNow().UtcDateTime; + + Assert.Same(user, result); + Assert.Null(user.MasterPassword); + Assert.Null(user.MasterPasswordSalt); + Assert.Equal(expectedTime, user.RevisionDate); + Assert.Equal(expectedTime, user.AccountRevisionDate); + Assert.Equal(originalLastPasswordChangeDate, user.LastPasswordChangeDate); + } + + [Theory, BitAutoData] + public void PrepareClearMasterPassword_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = CreateSutProvider(); + user.Id = default; + + Assert.Throws( + () => sutProvider.Sut.PrepareClearMasterPassword(user)); + } } diff --git a/test/Core.Test/KeyManagement/Commands/ConvertUserToKeyConnectorCommandTests.cs b/test/Core.Test/KeyManagement/Commands/ConvertUserToKeyConnectorCommandTests.cs new file mode 100644 index 000000000000..f00d95f7fb32 --- /dev/null +++ b/test/Core.Test/KeyManagement/Commands/ConvertUserToKeyConnectorCommandTests.cs @@ -0,0 +1,162 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Commands; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.KeyManagement.Commands; + +[SutProviderCustomize] +public class ConvertUserToKeyConnectorCommandTests +{ + [Theory] + [BitAutoData("wrapped-user-key")] + [BitAutoData("2.AOs41Hd8OQiCPXjyJKCiDA==|O6OHgt2U2hJGBSNGnimJmg==|iD33s8B69C8JhYYhSa4V1tArjvLr8eEaGqOV7BRo5Jk=")] + public async Task ConvertAsync_WrappedUserKeyProvided_SetsWrappedUserKey( + string wrappedUserKey, + SutProvider sutProvider, + User user) + { + // Arrange + user.UsesKeyConnector = false; + user.MasterPassword = "master-password"; + user.MasterPasswordSalt = "master-password-salt"; + user.Key = "old-key"; + sutProvider.GetDependency().Organizations = []; + ArrangeMasterPasswordServiceMutation(sutProvider); + + // Act + var result = await sutProvider.Sut.ConvertAsync(user, wrappedUserKey); + + // Assert + Assert.True(result.Succeeded); + Assert.True(user.UsesKeyConnector); + Assert.Null(user.MasterPassword); + Assert.Null(user.MasterPasswordSalt); + Assert.Equal(wrappedUserKey, user.Key); + sutProvider.GetDependency().Received(1) + .PrepareClearMasterPassword(user); + await sutProvider.GetDependency().Received(1) + .ReplaceAsync(Arg.Is(u => + u == user && + u.Key == wrappedUserKey && + u.MasterPassword == null && + u.MasterPasswordSalt == null && + u.UsesKeyConnector)); + await sutProvider.GetDependency().Received(1) + .LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); + } + + [Theory, BitAutoData] + public async Task ConvertAsync_WrappedUserKeyNull_DoesNotOverwriteExistingKey( + SutProvider sutProvider, + User user) + { + // Arrange + const string existingUserKey = "existing-user-key"; + user.UsesKeyConnector = false; + user.MasterPassword = "master-password"; + user.MasterPasswordSalt = "master-password-salt"; + user.Key = existingUserKey; + sutProvider.GetDependency().Organizations = []; + ArrangeMasterPasswordServiceMutation(sutProvider); + + // Act + var result = await sutProvider.Sut.ConvertAsync(user, null); + + // Assert + Assert.True(result.Succeeded); + Assert.True(user.UsesKeyConnector); + Assert.Null(user.MasterPassword); + Assert.Null(user.MasterPasswordSalt); + Assert.Equal(existingUserKey, user.Key); + sutProvider.GetDependency().Received(1) + .PrepareClearMasterPassword(user); + await sutProvider.GetDependency().Received(1) + .ReplaceAsync(Arg.Is(u => + u == user && + u.Key == existingUserKey && + u.MasterPassword == null && + u.MasterPasswordSalt == null && + u.UsesKeyConnector)); + await sutProvider.GetDependency().Received(1) + .LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); + } + + [Theory, BitAutoData] + public async Task ConvertAsync_NullUser_Throws( + SutProvider sutProvider) + { + await Assert.ThrowsAsync( + () => sutProvider.Sut.ConvertAsync(null, null)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ReplaceAsync(Arg.Any()); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogUserEventAsync(Arg.Any(), Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ConvertAsync_UserAlreadyUsesKeyConnector_ReturnsFailure( + SutProvider sutProvider, + User user) + { + user.UsesKeyConnector = true; + sutProvider.GetDependency().Organizations = []; + + var result = await sutProvider.Sut.ConvertAsync(user, null); + + Assert.False(result.Succeeded); + sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .PrepareClearMasterPassword(Arg.Any()); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ReplaceAsync(Arg.Any()); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogUserEventAsync(Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData(OrganizationUserType.Owner)] + [BitAutoData(OrganizationUserType.Admin)] + public async Task ConvertAsync_UserIsOwnerOrAdminOfOrg_ThrowsBadRequest( + OrganizationUserType orgUserType, + SutProvider sutProvider, + User user) + { + user.UsesKeyConnector = false; + sutProvider.GetDependency().Organizations = + [ + new CurrentContextOrganization { Id = Guid.NewGuid(), Type = orgUserType } + ]; + + await Assert.ThrowsAsync( + () => sutProvider.Sut.ConvertAsync(user, null)); + + sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .PrepareClearMasterPassword(Arg.Any()); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ReplaceAsync(Arg.Any()); + } + + // Configures the IMasterPasswordService mock so calling PrepareClearMasterPassword performs + // the real mutation on the user. Tests then assert on both the call and the resulting state + // captured in IUserRepository.ReplaceAsync. + private static void ArrangeMasterPasswordServiceMutation( + SutProvider sutProvider) + { + sutProvider.GetDependency() + .PrepareClearMasterPassword(Arg.Do(u => + { + u.MasterPassword = null; + u.MasterPasswordSalt = null; + })) + .Returns(call => (User)call[0]); + } +} diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 00cd3f27e2db..af2544ee2443 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -15,7 +15,6 @@ using Bit.Core.Billing.Models.Business; using Bit.Core.Billing.Premium.Queries; using Bit.Core.Billing.Services; -using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -460,72 +459,6 @@ public async Task RecoverTwoFactorAsync_IncorrectCode_ReturnsFalse( Assert.NotNull(user.TwoFactorProviders); } - [Theory] - [BitAutoData("wrapped-user-key")] - [BitAutoData("2.AOs41Hd8OQiCPXjyJKCiDA==|O6OHgt2U2hJGBSNGnimJmg==|iD33s8B69C8JhYYhSa4V1tArjvLr8eEaGqOV7BRo5Jk=")] - public async Task ConvertToKeyConnectorAsync_WrappedUserKeyProvided_SetsWrappedUserKey( - string wrappedUserKey, - SutProvider sutProvider, - User user) - { - // Arrange - user.UsesKeyConnector = false; - user.MasterPassword = "master-password"; - user.Key = "old-key"; - sutProvider.GetDependency().Organizations = []; - - // Act - var result = await sutProvider.Sut.ConvertToKeyConnectorAsync(user, wrappedUserKey); - - // Assert - Assert.True(result.Succeeded); - Assert.True(user.UsesKeyConnector); - Assert.Null(user.MasterPassword); - Assert.Equal(wrappedUserKey, user.Key); - Assert.Equal(user.RevisionDate, user.AccountRevisionDate); - await sutProvider.GetDependency().Received(1) - .ReplaceAsync(Arg.Is(u => - u == user && - u.Key == wrappedUserKey && - u.MasterPassword == null && - u.UsesKeyConnector)); - await sutProvider.GetDependency().Received(1) - .LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); - } - - [Theory, BitAutoData] - public async Task ConvertToKeyConnectorAsync_WrappedUserKeyNull_DoesNotOverwriteExistingKey( - SutProvider sutProvider, - User user) - { - // Arrange - const string existingUserKey = "existing-user-key"; - user.UsesKeyConnector = false; - user.MasterPassword = "master-password"; - user.Key = existingUserKey; - sutProvider.GetDependency().Organizations = []; - - // Act - var result = await sutProvider.Sut.ConvertToKeyConnectorAsync(user, null); - - // Assert - Assert.True(result.Succeeded); - Assert.True(user.UsesKeyConnector); - Assert.Null(user.MasterPassword); - Assert.Equal(existingUserKey, user.Key); - Assert.Equal(user.RevisionDate, user.AccountRevisionDate); - - await sutProvider.GetDependency().Received(1) - .ReplaceAsync(Arg.Is(u => - u == user && - u.Key == existingUserKey && - u.MasterPassword == null && - u.UsesKeyConnector)); - - await sutProvider.GetDependency().Received(1) - .LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); - } - private static void SetupUserAndDevice(User user, bool shouldHavePassword) {