From 5dd74dbcb31b34e15c3b1abc864e599c1a52a287 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 27 Jan 2025 14:49:35 +0100 Subject: [PATCH 01/32] Implement userkey rotation v2 --- .../Auth/Controllers/AccountsController.cs | 1 + .../Accounts/MasterPasswordUnlockDataModel.cs | 47 +++++++ .../AccountsKeyManagementController.cs | 93 +++++++++++- .../Requests/RotateUserAccountKeysModel.cs | 35 +++++ .../UserServiceCollectionExtensions.cs | 1 + src/Core/Constants.cs | 1 + .../Models/Data/MasterPasswordUnlockData.cs | 33 +++++ .../Models/Data/RotateUserAccountKeysData.cs | 28 ++++ .../UserKey/IRotateUserAccountKeysCommand.cs | 20 +++ .../RotateUserAccountkeysCommand.cs | 133 ++++++++++++++++++ src/Core/Repositories/IUserRepository.cs | 2 + .../Repositories/UserRepository.cs | 61 +++++++- .../Repositories/UserRepository.cs | 45 ++++++ .../AccountsKeyManagementControllerTests.cs | 32 +++++ .../RotateUserAccountKeysCommandTests.cs | 90 ++++++++++++ 15 files changed, 614 insertions(+), 8 deletions(-) create mode 100644 src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs create mode 100644 src/Api/KeyManagement/Models/Requests/RotateUserAccountKeysModel.cs create mode 100644 src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs create mode 100644 src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs create mode 100644 src/Core/KeyManagement/UserKey/IRotateUserAccountKeysCommand.cs create mode 100644 src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs create mode 100644 test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 7990a5a18a5d..33f5c5d9f0ce 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -377,6 +377,7 @@ public async Task PostKdf([FromBody] KdfRequestModel model) throw new BadRequestException(ModelState); } + [Obsolete("Replaced by the safer rotate-user-account-keys endpoint.")] [HttpPost("key")] public async Task PostKey([FromBody] UpdateKeyRequestModel model) { diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs new file mode 100644 index 000000000000..2b46fe8633da --- /dev/null +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs @@ -0,0 +1,47 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.Enums; + +namespace Bit.Api.Auth.Models.Request.Accounts; + +public class MasterPasswordUnlockDataModel : IValidatableObject +{ + [Required] + public KdfType KdfType { get; set; } + [Required] + public int KdfIterations { get; set; } + public int? KdfMemory { get; set; } + public int? KdfParallelism { get; set; } + + [Required] + public string Email { get; set; } + [Required] + [StringLength(300)] + public string MasterKeyServerAuthenticationHash { get; set; } + [Required] + public string MasterKeyEncryptedUserKey { get; set; } + [StringLength(50)] + public string MasterPasswordHint { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (KdfType == KdfType.PBKDF2_SHA256) + { + if (KdfMemory.HasValue || KdfParallelism.HasValue) + { + yield return new ValidationResult("KdfMemory and KdfParallelism must be null for PBKDF2_SHA256", new[] { nameof(KdfMemory), nameof(KdfParallelism) }); + } + } + else if (KdfType == KdfType.Argon2id) + { + if (!KdfMemory.HasValue || !KdfParallelism.HasValue) + { + yield return new ValidationResult("KdfMemory and KdfParallelism must have values for Argon2id", new[] { nameof(KdfMemory), nameof(KdfParallelism) }); + } + } + else + { + yield return new ValidationResult("Invalid KdfType", new[] { nameof(KdfType) }); + } + } + +} diff --git a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs index b8d5e3094931..63f47a1c609c 100644 --- a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs +++ b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs @@ -1,10 +1,24 @@ #nullable enable +using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Models.Request.WebAuthn; +using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; +using Bit.Api.KeyManagement.Validators; +using Bit.Api.Tools.Models.Request; +using Bit.Api.Vault.Models.Request; using Bit.Core; +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.KeyManagement.Commands.Interfaces; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.UserKey; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tools.Entities; +using Bit.Core.Vault.Entities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -19,18 +33,45 @@ public class AccountsKeyManagementController : Controller private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IRegenerateUserAsymmetricKeysCommand _regenerateUserAsymmetricKeysCommand; private readonly IUserService _userService; + private readonly IRotateUserAccountKeysCommand _rotateUserAccountKeysCommand; + private readonly IRotationValidator, IEnumerable> _cipherValidator; + private readonly IRotationValidator, IEnumerable> _folderValidator; + private readonly IRotationValidator, IReadOnlyList> _sendValidator; + private readonly IRotationValidator, IEnumerable> + _emergencyAccessValidator; + private readonly IRotationValidator, + IReadOnlyList> + _organizationUserValidator; + private readonly IRotationValidator, IEnumerable> + _webauthnKeyValidator; public AccountsKeyManagementController(IUserService userService, IFeatureService featureService, IOrganizationUserRepository organizationUserRepository, IEmergencyAccessRepository emergencyAccessRepository, - IRegenerateUserAsymmetricKeysCommand regenerateUserAsymmetricKeysCommand) + IRegenerateUserAsymmetricKeysCommand regenerateUserAsymmetricKeysCommand, + IRotateUserAccountKeysCommand rotateUserKeyCommandV2, + IRotationValidator, IEnumerable> cipherValidator, + IRotationValidator, IEnumerable> folderValidator, + IRotationValidator, IReadOnlyList> sendValidator, + IRotationValidator, IEnumerable> + emergencyAccessValidator, + IRotationValidator, IReadOnlyList> + organizationUserValidator, + IRotationValidator, IEnumerable> webAuthnKeyValidator) { _userService = userService; _featureService = featureService; _regenerateUserAsymmetricKeysCommand = regenerateUserAsymmetricKeysCommand; _organizationUserRepository = organizationUserRepository; _emergencyAccessRepository = emergencyAccessRepository; + _rotateUserAccountKeysCommand = rotateUserKeyCommandV2; + _cipherValidator = cipherValidator; + _folderValidator = folderValidator; + _sendValidator = sendValidator; + _emergencyAccessValidator = emergencyAccessValidator; + _organizationUserValidator = organizationUserValidator; + _webauthnKeyValidator = webAuthnKeyValidator; } [HttpPost("regenerate-keys")] @@ -47,4 +88,54 @@ public async Task RegenerateKeysAsync([FromBody] KeyRegenerationRequestModel req await _regenerateUserAsymmetricKeysCommand.RegenerateKeysAsync(request.ToUserAsymmetricKeys(user.Id), usersOrganizationAccounts, designatedEmergencyAccess); } + + + [HttpPost("rotate-user-account-keys")] + public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysModel model) + { + var user = await _userService.GetUserByPrincipalAsync(User); + if (user == null) + { + throw new UnauthorizedAccessException(); + } + + var unlockData = new MasterPasswordUnlockData + { + KdfType = model.MasterPasswordUnlockData.KdfType, + KdfIterations = model.MasterPasswordUnlockData.KdfIterations, + KdfMemory = model.MasterPasswordUnlockData.KdfMemory, + KdfParallelism = model.MasterPasswordUnlockData.KdfParallelism, + Email = model.MasterPasswordUnlockData.Email, + MasterKeyRemoteAuthenticationHash = model.MasterPasswordUnlockData.MasterKeyServerAuthenticationHash, + MasterKeyEncryptedUserKey = model.MasterPasswordUnlockData.MasterKeyEncryptedUserKey, + MasterPasswordHint = model.MasterPasswordUnlockData.MasterPasswordHint + }; + + var dataModel = new RotateUserAccountKeysData + { + MasterPasswordUnlockData = unlockData, + OldMasterKeyServerAuthenticationHash = model.OldMasterKeyServerAuthenticationHash, + UserKeyEncryptedAccountPrivateKey = model.UserKeyEncryptedAccountPrivateKey, + Ciphers = await _cipherValidator.ValidateAsync(user, model.Ciphers), + Folders = await _folderValidator.ValidateAsync(user, model.Folders), + Sends = await _sendValidator.ValidateAsync(user, model.Sends), + EmergencyAccesses = await _emergencyAccessValidator.ValidateAsync(user, model.EmergencyAccessUnlockData), + OrganizationUsers = await _organizationUserValidator.ValidateAsync(user, model.OrganizationAccountRecoveryUnlockData), + WebAuthnKeys = await _webauthnKeyValidator.ValidateAsync(user, model.PasskeyPrfUnlockData) + }; + + var result = await _rotateUserAccountKeysCommand.RotateUserAccountKeysAsync(user, dataModel); + + if (result.Succeeded) + { + return; + } + + foreach (var error in result.Errors) + { + ModelState.AddModelError(string.Empty, error.Description); + } + + throw new BadRequestException(ModelState); + } } diff --git a/src/Api/KeyManagement/Models/Requests/RotateUserAccountKeysModel.cs b/src/Api/KeyManagement/Models/Requests/RotateUserAccountKeysModel.cs new file mode 100644 index 000000000000..7f9d3d7c921c --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/RotateUserAccountKeysModel.cs @@ -0,0 +1,35 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Request.WebAuthn; +using Bit.Api.Tools.Models.Request; +using Bit.Api.Vault.Models.Request; + +namespace Bit.Api.KeyManagement.Models.Request.Accounts; + +public class RotateUserAccountKeysModel +{ + // Authentication for this requests + [Required] + public string OldMasterKeyServerAuthenticationHash { get; set; } + + // All methods to get to the userkey + [Required] + public MasterPasswordUnlockDataModel MasterPasswordUnlockData { get; set; } + public IEnumerable EmergencyAccessUnlockData { get; set; } + public IEnumerable OrganizationAccountRecoveryUnlockData { get; set; } + public IEnumerable PasskeyPrfUnlockData { get; set; } + + // Other keys encrypted by the userkey + [Required] + public string UserKeyEncryptedAccountPrivateKey { get; set; } + [Required] + public string AccountPublicKey { get; set; } + + // User vault data encrypted by the userkey + public IEnumerable Ciphers { get; set; } + public IEnumerable Folders { get; set; } + public IEnumerable Sends { get; set; } + +} diff --git a/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs index df102c855f23..16a0ef9805db 100644 --- a/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs +++ b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs @@ -32,6 +32,7 @@ public static void AddUserServices(this IServiceCollection services, IGlobalSett public static void AddUserKeyCommands(this IServiceCollection services, IGlobalSettings globalSettings) { services.AddScoped(); + services.AddScoped(); } private static void AddUserPasswordCommands(this IServiceCollection services) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index b4ddd734094b..ee0999df66a1 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -164,6 +164,7 @@ public static class FeatureFlagKeys public const string Argon2Default = "argon2-default"; public const string UsePricingService = "use-pricing-service"; public const string RecordInstallationLastActivityDate = "installation-last-activity-date"; + public const string UserkeyRotationV2 = "userkey-rotation-v2"; public const string EnablePasswordManagerSyncAndroid = "enable-password-manager-sync-android"; public const string EnablePasswordManagerSynciOS = "enable-password-manager-sync-ios"; diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs new file mode 100644 index 000000000000..86545edcc042 --- /dev/null +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs @@ -0,0 +1,33 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; + +namespace Bit.Core.KeyManagement.Models.Data; + +public class MasterPasswordUnlockData +{ + public KdfType KdfType { get; set; } + public int KdfIterations { get; set; } + public int? KdfMemory { get; set; } + public int? KdfParallelism { get; set; } + + public string Email { get; set; } + public string MasterKeyRemoteAuthenticationHash { get; set; } + public string MasterKeyEncryptedUserKey { get; set; } + public string MasterPasswordHint { get; set; } + + public bool ValidateForUser(User user) + { + if (KdfType != user.Kdf || KdfMemory != user.KdfMemory || KdfParallelism != user.KdfParallelism || KdfIterations != user.KdfIterations) + { + return false; + } + else if (Email != user.Email) + { + return false; + } + else + { + return true; + } + } +} diff --git a/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs new file mode 100644 index 000000000000..8e0e728bebd6 --- /dev/null +++ b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs @@ -0,0 +1,28 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; +using Bit.Core.Tools.Entities; +using Bit.Core.Vault.Entities; + +namespace Bit.Core.KeyManagement.Models.Data; + +public class RotateUserAccountKeysData +{ + // Authentication for this requests + public string OldMasterKeyServerAuthenticationHash { get; set; } + + // Other keys encrypted by the userkey + public string UserKeyEncryptedAccountPrivateKey { get; set; } + public string AccountPublicKey { get; set; } + + // All methods to get to the userkey + public MasterPasswordUnlockData MasterPasswordUnlockData { get; set; } + public IEnumerable EmergencyAccesses { get; set; } + public IReadOnlyList OrganizationUsers { get; set; } + public IEnumerable WebAuthnKeys { get; set; } + + // User vault data encrypted by the userkey + public IEnumerable Ciphers { get; set; } + public IEnumerable Folders { get; set; } + public IReadOnlyList Sends { get; set; } +} diff --git a/src/Core/KeyManagement/UserKey/IRotateUserAccountKeysCommand.cs b/src/Core/KeyManagement/UserKey/IRotateUserAccountKeysCommand.cs new file mode 100644 index 000000000000..ec40e7031d28 --- /dev/null +++ b/src/Core/KeyManagement/UserKey/IRotateUserAccountKeysCommand.cs @@ -0,0 +1,20 @@ +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.KeyManagement.UserKey; + +/// +/// Responsible for rotation of a user key and updating database with re-encrypted data +/// +public interface IRotateUserAccountKeysCommand +{ + /// + /// Sets a new user key and updates all encrypted data. + /// + /// All necessary information for rotation. If data is not included, this will lead to the change being rejected. + /// An IdentityResult for verification of the master password hash + /// User must be provided. + /// User KDF settings and email must match the model provided settings. + Task RotateUserAccountKeysAsync(User user, RotateUserAccountKeysData model); +} diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs new file mode 100644 index 000000000000..826fdf70cc8f --- /dev/null +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs @@ -0,0 +1,133 @@ +using Bit.Core.Auth.Repositories; +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Tools.Repositories; +using Bit.Core.Vault.Repositories; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.KeyManagement.UserKey.Implementations; + +/// +public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand +{ + private readonly IUserService _userService; + private readonly IUserRepository _userRepository; + private readonly ICipherRepository _cipherRepository; + private readonly IFolderRepository _folderRepository; + private readonly ISendRepository _sendRepository; + private readonly IEmergencyAccessRepository _emergencyAccessRepository; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IPushNotificationService _pushService; + private readonly IdentityErrorDescriber _identityErrorDescriber; + private readonly IWebAuthnCredentialRepository _credentialRepository; + private readonly IPasswordHasher _passwordHasher; + + /// + /// Instantiates a new + /// + /// Master password hash validation + /// Updates user keys and re-encrypted data if needed + /// Provides a method to update re-encrypted cipher data + /// Provides a method to update re-encrypted folder data + /// Provides a method to update re-encrypted send data + /// Provides a method to update re-encrypted emergency access data + /// Provides a method to update re-encrypted organization user data + /// Hashes the new master password + /// Logs out user from other devices after successful rotation + /// Provides a password mismatch error if master password hash validation fails + /// Provides a method to update re-encrypted WebAuthn keys + public RotateUserAccountKeysCommand(IUserService userService, IUserRepository userRepository, + ICipherRepository cipherRepository, IFolderRepository folderRepository, ISendRepository sendRepository, + IEmergencyAccessRepository emergencyAccessRepository, IOrganizationUserRepository organizationUserRepository, + IPasswordHasher passwordHasher, + IPushNotificationService pushService, IdentityErrorDescriber errors, IWebAuthnCredentialRepository credentialRepository) + { + _userService = userService; + _userRepository = userRepository; + _cipherRepository = cipherRepository; + _folderRepository = folderRepository; + _sendRepository = sendRepository; + _emergencyAccessRepository = emergencyAccessRepository; + _organizationUserRepository = organizationUserRepository; + _pushService = pushService; + _identityErrorDescriber = errors; + _credentialRepository = credentialRepository; + _passwordHasher = passwordHasher; + } + + /// + public async Task RotateUserAccountKeysAsync(User user, RotateUserAccountKeysData model) + { + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + if (!await _userService.CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash)) + { + return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); + } + + var now = DateTime.UtcNow; + user.RevisionDate = user.AccountRevisionDate = now; + user.LastKeyRotationDate = now; + user.SecurityStamp = Guid.NewGuid().ToString(); + + if ( + !model.MasterPasswordUnlockData.ValidateForUser(user) + ) + { + throw new InvalidOperationException("The provided master password unlock data is not valid for this user."); + } + if ( + !model.AccountPublicKey == user.PublicKey + ) + { + throw new InvalidOperationException("The provided account public key does not match the user's current public key, and changing the account asymmetric keypair is currently not supported during rotation."); + } + + user.Key = model.MasterPasswordUnlockData.MasterKeyEncryptedUserKey; + user.PrivateKey = model.UserKeyEncryptedAccountPrivateKey; + user.MasterPassword = _passwordHasher.HashPassword(user, model.MasterPasswordUnlockData.MasterKeyRemoteAuthenticationHash); + + List saveEncryptedDataActions = new(); + if (model.Ciphers.Any()) + { + saveEncryptedDataActions.Add(_cipherRepository.UpdateForKeyRotation(user.Id, model.Ciphers)); + } + + if (model.Folders.Any()) + { + saveEncryptedDataActions.Add(_folderRepository.UpdateForKeyRotation(user.Id, model.Folders)); + } + + if (model.Sends.Any()) + { + saveEncryptedDataActions.Add(_sendRepository.UpdateForKeyRotation(user.Id, model.Sends)); + } + + if (model.EmergencyAccesses.Any()) + { + saveEncryptedDataActions.Add( + _emergencyAccessRepository.UpdateForKeyRotation(user.Id, model.EmergencyAccesses)); + } + + if (model.OrganizationUsers.Any()) + { + saveEncryptedDataActions.Add( + _organizationUserRepository.UpdateForKeyRotation(user.Id, model.OrganizationUsers)); + } + + if (model.WebAuthnKeys.Any()) + { + saveEncryptedDataActions.Add(_credentialRepository.UpdateKeysForRotationAsync(user.Id, model.WebAuthnKeys)); + } + + await _userRepository.UpdateUserKeyAndEncryptedDataV2Async(user, saveEncryptedDataActions); + await _pushService.PushLogOutAsync(user.Id); + return IdentityResult.Success; + } +} diff --git a/src/Core/Repositories/IUserRepository.cs b/src/Core/Repositories/IUserRepository.cs index 040e6e1f49a1..0e59b9998f64 100644 --- a/src/Core/Repositories/IUserRepository.cs +++ b/src/Core/Repositories/IUserRepository.cs @@ -32,5 +32,7 @@ public interface IUserRepository : IRepository /// Registered database calls to update re-encrypted data. Task UpdateUserKeyAndEncryptedDataAsync(User user, IEnumerable updateDataActions); + Task UpdateUserKeyAndEncryptedDataV2Async(User user, + IEnumerable updateDataActions); Task DeleteManyAsync(IEnumerable users); } diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index 227a7c03e5d5..d092c6fbcb66 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -254,6 +254,48 @@ public async Task UpdateUserKeyAndEncryptedDataAsync( } + public async Task UpdateUserKeyAndEncryptedDataV2Async( + User user, + IEnumerable updateDataActions) + { + await using var connection = new SqlConnection(ConnectionString); + connection.Open(); + + await using var transaction = connection.BeginTransaction(); + try + { + if (user.AccountRevisionDate == null) + { + user.AccountRevisionDate = user.RevisionDate; + } + + ProtectData(user); + await connection.ExecuteAsync( + $"[{Schema}].[{Table}_Update]", + user, + transaction: transaction, + commandType: CommandType.StoredProcedure); + + // Update re-encrypted data + foreach (var action in updateDataActions) + { + await action(connection, transaction); + } + transaction.Commit(); + } + catch + { + transaction.Rollback(); + throw; + } + finally + { + connection.Close(); + UnprotectData(user); + } + } + + public async Task> GetManyAsync(IEnumerable ids) { using (var connection = new SqlConnection(ReadOnlyConnectionString)) @@ -295,6 +337,18 @@ private async Task ProtectDataAndSaveAsync(User user, Func saveTask) var originalKey = user.Key; // Protect values + ProtectData(user); + + // Save + await saveTask(); + + // Restore original values + user.MasterPassword = originalMasterPassword; + user.Key = originalKey; + } + + private void ProtectData(User user) + { if (!user.MasterPassword?.StartsWith(Constants.DatabaseFieldProtectedPrefix) ?? false) { user.MasterPassword = string.Concat(Constants.DatabaseFieldProtectedPrefix, @@ -306,13 +360,6 @@ private async Task ProtectDataAndSaveAsync(User user, Func saveTask) user.Key = string.Concat(Constants.DatabaseFieldProtectedPrefix, _dataProtector.Protect(user.Key!)); } - - // Save - await saveTask(); - - // Restore original values - user.MasterPassword = originalMasterPassword; - user.Key = originalKey; } private void UnprotectData(User? user) diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index cbfefb648332..cbab13b1a461 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -194,6 +194,51 @@ public async Task UpdateUserKeyAndEncryptedDataAsync(Core.Entities.User user, } + + public async Task UpdateUserKeyAndEncryptedDataV2Async(Core.Entities.User user, + IEnumerable updateDataActions) + { + using var scope = ServiceScopeFactory.CreateScope(); + var dbContext = GetDatabaseContext(scope); + + await using var transaction = await dbContext.Database.BeginTransactionAsync(); + + try + { + // Update user + var userEntity = await dbContext.Users.FindAsync(user.Id); + if (userEntity == null) + { + throw new ArgumentException("User not found", nameof(user)); + } + + userEntity.SecurityStamp = user.SecurityStamp; + userEntity.Key = user.Key; + userEntity.MasterPassword = user.MasterPassword; + userEntity.PrivateKey = user.PrivateKey; + userEntity.LastKeyRotationDate = user.LastKeyRotationDate; + userEntity.AccountRevisionDate = user.AccountRevisionDate; + userEntity.RevisionDate = user.RevisionDate; + + await dbContext.SaveChangesAsync(); + + // Update re-encrypted data + foreach (var action in updateDataActions) + { + // connection and transaction aren't used in EF + await action(); + } + + await transaction.CommitAsync(); + } + catch + { + await transaction.RollbackAsync(); + throw; + } + + } + public async Task> GetManyAsync(IEnumerable ids) { using (var scope = ServiceScopeFactory.CreateScope()) diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 2615697ad374..7e81aaebca18 100644 --- a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -1,17 +1,28 @@ #nullable enable using System.Security.Claims; +using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Api.Auth.Models.Request; using Bit.Api.KeyManagement.Controllers; +using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; +using Bit.Api.KeyManagement.Validators; +using Bit.Api.Tools.Models.Request; +using Bit.Api.Vault.Models.Request; using Bit.Core; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Data; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.KeyManagement.Commands.Interfaces; using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.UserKey; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tools.Entities; +using Bit.Core.Vault.Entities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; using NSubstitute; using NSubstitute.ReturnsExtensions; using Xunit; @@ -93,4 +104,25 @@ await sutProvider.GetDependency().Received Arg.Is(orgUsers), Arg.Is(accessDetails)); } + + [Theory] + [BitAutoData] + public async Task RotateUserAccountKeysSuccess(SutProvider sutProvider, + RotateUserAccountKeysModel data, User user) + { + sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).Returns(user); + sutProvider.GetDependency().RotateUserAccountKeysAsync(Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Success); + await sutProvider.Sut.RotateUserAccountKeys(data); + await sutProvider.GetDependency, IEnumerable>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.Ciphers)); + await sutProvider.GetDependency, IEnumerable>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.Folders)); + await sutProvider.GetDependency, IReadOnlyList>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.Sends)); + await sutProvider.GetDependency, IEnumerable>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.EmergencyAccessUnlockData)); + await sutProvider.GetDependency, IReadOnlyList>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.OrganizationAccountRecoveryUnlockData)); + } } diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs new file mode 100644 index 000000000000..5452572d7344 --- /dev/null +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -0,0 +1,90 @@ +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.UserKey.Implementations; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.KeyManagement.UserFeatures.UserKey; + +[SutProviderCustomize] +public class RotateUserAccountKeysCommandTests +{ + [Theory, BitAutoData] + public async Task RejectsWrongOldMasterPassword(SutProvider sutProvider, User user, + RotateUserAccountKeysData model) + { + user.Email = model.MasterPasswordUnlockData.Email; + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + .Returns(false); + + var result = await sutProvider.Sut.RotateUserAccountKeysAsync(user, model); + + Assert.NotEqual(IdentityResult.Success, result); + } + + [Theory, BitAutoData] + public async Task RejectsEmailChange(SutProvider sutProvider, User user, + RotateUserAccountKeysData model) + { + user.Kdf = Enums.KdfType.Argon2id; + user.KdfIterations = 3; + user.KdfMemory = 64; + user.KdfParallelism = 4; + + model.MasterPasswordUnlockData.Email = user.Email + ".different-domain"; + model.MasterPasswordUnlockData.KdfType = Enums.KdfType.Argon2id; + model.MasterPasswordUnlockData.KdfIterations = 3; + model.MasterPasswordUnlockData.KdfMemory = 64; + model.MasterPasswordUnlockData.KdfParallelism = 4; + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + .Returns(true); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); + } + + [Theory, BitAutoData] + public async Task RejectsKdfChange(SutProvider sutProvider, User user, + RotateUserAccountKeysData model) + { + user.Kdf = Enums.KdfType.Argon2id; + user.KdfIterations = 3; + user.KdfMemory = 64; + user.KdfParallelism = 4; + + model.MasterPasswordUnlockData.Email = user.Email; + model.MasterPasswordUnlockData.KdfType = Enums.KdfType.PBKDF2_SHA256; + model.MasterPasswordUnlockData.KdfIterations = 600000; + model.MasterPasswordUnlockData.KdfMemory = null; + model.MasterPasswordUnlockData.KdfParallelism = null; + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + .Returns(true); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); + } + + [Theory, BitAutoData] + public async Task RotatesCorrectly(SutProvider sutProvider, User user, + RotateUserAccountKeysData model) + { + user.Kdf = Enums.KdfType.Argon2id; + user.KdfIterations = 3; + user.KdfMemory = 64; + user.KdfParallelism = 4; + + model.MasterPasswordUnlockData.Email = user.Email; + model.MasterPasswordUnlockData.KdfType = Enums.KdfType.Argon2id; + model.MasterPasswordUnlockData.KdfIterations = 3; + model.MasterPasswordUnlockData.KdfMemory = 64; + model.MasterPasswordUnlockData.KdfParallelism = 4; + + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + .Returns(true); + + var result = await sutProvider.Sut.RotateUserAccountKeysAsync(user, model); + + Assert.Equal(IdentityResult.Success, result); + } + +} From 26175b71e8f71d5ad94467de0c5a22361658caa2 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 27 Jan 2025 18:39:46 +0100 Subject: [PATCH 02/32] Update request models --- .../Accounts/MasterPasswordUnlockDataModel.cs | 21 ++++++++++- .../AccountsKeyManagementController.cs | 37 ++++++++----------- .../Requests/AccountKeysRequestModel.cs | 11 ++++++ .../RotateAccountKeysAndDataRequestModel.cs | 15 ++++++++ ...KeysModel.cs => UnlockDataRequestModel.cs} | 20 +--------- .../Models/Requests/UserDataRequestModel.cs | 11 ++++++ .../Models/Data/MasterPasswordUnlockData.cs | 2 +- .../Models/Data/RotateUserAccountKeysData.cs | 2 +- .../RotateUserAccountkeysCommand.cs | 8 ++-- .../AccountsKeyManagementControllerTests.cs | 3 +- .../RotateUserAccountKeysCommandTests.cs | 8 ++-- 11 files changed, 85 insertions(+), 53 deletions(-) create mode 100644 src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs create mode 100644 src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs rename src/Api/KeyManagement/Models/Requests/{RotateUserAccountKeysModel.cs => UnlockDataRequestModel.cs} (54%) create mode 100644 src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs index 2b46fe8633da..7b08f4edfcc8 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs @@ -1,5 +1,6 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Data; namespace Bit.Api.Auth.Models.Request.Accounts; @@ -16,7 +17,7 @@ public class MasterPasswordUnlockDataModel : IValidatableObject public string Email { get; set; } [Required] [StringLength(300)] - public string MasterKeyServerAuthenticationHash { get; set; } + public string MasterKeyAuthenticationHash { get; set; } [Required] public string MasterKeyEncryptedUserKey { get; set; } [StringLength(50)] @@ -44,4 +45,22 @@ public IEnumerable Validate(ValidationContext validationContex } } + public MasterPasswordUnlockData ToUnlockData() + { + var data = new MasterPasswordUnlockData + { + KdfType = KdfType, + KdfIterations = KdfIterations, + KdfMemory = KdfMemory, + KdfParallelism = KdfParallelism, + + Email = Email, + + MasterKeyAuthenticationHash = MasterKeyAuthenticationHash, + MasterKeyEncryptedUserKey = MasterKeyEncryptedUserKey, + MasterPasswordHint = MasterPasswordHint + }; + return data; + } + } diff --git a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs index 63f47a1c609c..83c68991338c 100644 --- a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs +++ b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs @@ -91,7 +91,7 @@ await _regenerateUserAsymmetricKeysCommand.RegenerateKeysAsync(request.ToUserAsy [HttpPost("rotate-user-account-keys")] - public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysModel model) + public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysAndDataRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -99,29 +99,22 @@ public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysModel mo throw new UnauthorizedAccessException(); } - var unlockData = new MasterPasswordUnlockData - { - KdfType = model.MasterPasswordUnlockData.KdfType, - KdfIterations = model.MasterPasswordUnlockData.KdfIterations, - KdfMemory = model.MasterPasswordUnlockData.KdfMemory, - KdfParallelism = model.MasterPasswordUnlockData.KdfParallelism, - Email = model.MasterPasswordUnlockData.Email, - MasterKeyRemoteAuthenticationHash = model.MasterPasswordUnlockData.MasterKeyServerAuthenticationHash, - MasterKeyEncryptedUserKey = model.MasterPasswordUnlockData.MasterKeyEncryptedUserKey, - MasterPasswordHint = model.MasterPasswordUnlockData.MasterPasswordHint - }; - var dataModel = new RotateUserAccountKeysData { - MasterPasswordUnlockData = unlockData, - OldMasterKeyServerAuthenticationHash = model.OldMasterKeyServerAuthenticationHash, - UserKeyEncryptedAccountPrivateKey = model.UserKeyEncryptedAccountPrivateKey, - Ciphers = await _cipherValidator.ValidateAsync(user, model.Ciphers), - Folders = await _folderValidator.ValidateAsync(user, model.Folders), - Sends = await _sendValidator.ValidateAsync(user, model.Sends), - EmergencyAccesses = await _emergencyAccessValidator.ValidateAsync(user, model.EmergencyAccessUnlockData), - OrganizationUsers = await _organizationUserValidator.ValidateAsync(user, model.OrganizationAccountRecoveryUnlockData), - WebAuthnKeys = await _webauthnKeyValidator.ValidateAsync(user, model.PasskeyPrfUnlockData) + OldMasterkeyAuthenticationHash = model.OldMasterkeyAuthenticationHash, + + UserKeyEncryptedAccountPrivateKey = model.AccountKeys.UserKeyEncryptedAccountPrivateKey, + AccountPublicKey = model.AccountKeys.AccountPublicKey, + + MasterPasswordUnlockData = model.AccountUnlockData.MasterPasswordUnlockData.ToUnlockData(), + EmergencyAccesses = await _emergencyAccessValidator.ValidateAsync(user, model.AccountUnlockData.EmergencyAccessUnlockData), + OrganizationUsers = await _organizationUserValidator.ValidateAsync(user, model.AccountUnlockData.OrganizationAccountRecoveryUnlockData), + WebAuthnKeys = await _webauthnKeyValidator.ValidateAsync(user, model.AccountUnlockData.PasskeyUnlockData), + + Ciphers = await _cipherValidator.ValidateAsync(user, model.AccountData.Ciphers), + Folders = await _folderValidator.ValidateAsync(user, model.AccountData.Folders), + Sends = await _sendValidator.ValidateAsync(user, model.AccountData.Sends), + }; var result = await _rotateUserAccountKeysCommand.RotateUserAccountKeysAsync(user, dataModel); diff --git a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs new file mode 100644 index 000000000000..b3daabf1a93f --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs @@ -0,0 +1,11 @@ +using System.ComponentModel.DataAnnotations; + +namespace Bit.Api.KeyManagement.Models.Request.Accounts; + +public class AccountKeysRequestModel +{ + [Required] + public string UserKeyEncryptedAccountPrivateKey { get; set; } + [Required] + public string AccountPublicKey { get; set; } +} diff --git a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs new file mode 100644 index 000000000000..9a99cc2f3cf9 --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs @@ -0,0 +1,15 @@ +using System.ComponentModel.DataAnnotations; + +namespace Bit.Api.KeyManagement.Models.Request.Accounts; + +public class RotateUserAccountKeysAndDataRequestModel +{ + [Required] + public string OldMasterkeyAuthenticationHash { get; set; } + [Required] + public UnlockDataRequestModel AccountUnlockData { get; set; } + [Required] + public AccountKeysRequestModel AccountKeys { get; set; } + [Required] + public AccountDataRequestModel AccountData { get; set; } +} diff --git a/src/Api/KeyManagement/Models/Requests/RotateUserAccountKeysModel.cs b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs similarity index 54% rename from src/Api/KeyManagement/Models/Requests/RotateUserAccountKeysModel.cs rename to src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs index 7f9d3d7c921c..27f8769fa0d0 100644 --- a/src/Api/KeyManagement/Models/Requests/RotateUserAccountKeysModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs @@ -8,28 +8,12 @@ namespace Bit.Api.KeyManagement.Models.Request.Accounts; -public class RotateUserAccountKeysModel +public class UnlockDataRequestModel { - // Authentication for this requests - [Required] - public string OldMasterKeyServerAuthenticationHash { get; set; } - // All methods to get to the userkey [Required] public MasterPasswordUnlockDataModel MasterPasswordUnlockData { get; set; } public IEnumerable EmergencyAccessUnlockData { get; set; } public IEnumerable OrganizationAccountRecoveryUnlockData { get; set; } - public IEnumerable PasskeyPrfUnlockData { get; set; } - - // Other keys encrypted by the userkey - [Required] - public string UserKeyEncryptedAccountPrivateKey { get; set; } - [Required] - public string AccountPublicKey { get; set; } - - // User vault data encrypted by the userkey - public IEnumerable Ciphers { get; set; } - public IEnumerable Folders { get; set; } - public IEnumerable Sends { get; set; } - + public IEnumerable PasskeyUnlockData { get; set; } } diff --git a/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs new file mode 100644 index 000000000000..b7c861b7cbb6 --- /dev/null +++ b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs @@ -0,0 +1,11 @@ +using Bit.Api.Tools.Models.Request; +using Bit.Api.Vault.Models.Request; + +namespace Bit.Api.KeyManagement.Models.Request.Accounts; + +public class AccountDataRequestModel +{ + public IEnumerable Ciphers { get; set; } + public IEnumerable Folders { get; set; } + public IEnumerable Sends { get; set; } +} diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs index 86545edcc042..623b29ea7396 100644 --- a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs @@ -11,7 +11,7 @@ public class MasterPasswordUnlockData public int? KdfParallelism { get; set; } public string Email { get; set; } - public string MasterKeyRemoteAuthenticationHash { get; set; } + public string MasterKeyAuthenticationHash { get; set; } public string MasterKeyEncryptedUserKey { get; set; } public string MasterPasswordHint { get; set; } diff --git a/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs index 8e0e728bebd6..7c3493d7f193 100644 --- a/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs +++ b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs @@ -9,7 +9,7 @@ namespace Bit.Core.KeyManagement.Models.Data; public class RotateUserAccountKeysData { // Authentication for this requests - public string OldMasterKeyServerAuthenticationHash { get; set; } + public string OldMasterkeyAuthenticationHash { get; set; } // Other keys encrypted by the userkey public string UserKeyEncryptedAccountPrivateKey { get; set; } diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs index 826fdf70cc8f..03cc08e3c9f2 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs @@ -66,7 +66,7 @@ public async Task RotateUserAccountKeysAsync(User user, RotateUs throw new ArgumentNullException(nameof(user)); } - if (!await _userService.CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash)) + if (!await _userService.CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash)) { return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); } @@ -83,15 +83,15 @@ public async Task RotateUserAccountKeysAsync(User user, RotateUs throw new InvalidOperationException("The provided master password unlock data is not valid for this user."); } if ( - !model.AccountPublicKey == user.PublicKey + model.AccountPublicKey != user.PublicKey ) { - throw new InvalidOperationException("The provided account public key does not match the user's current public key, and changing the account asymmetric keypair is currently not supported during rotation."); + throw new InvalidOperationException("The provided account public key does not match the user's current public key, and changing the account asymmetric keypair is currently not supported during key rotation."); } user.Key = model.MasterPasswordUnlockData.MasterKeyEncryptedUserKey; user.PrivateKey = model.UserKeyEncryptedAccountPrivateKey; - user.MasterPassword = _passwordHasher.HashPassword(user, model.MasterPasswordUnlockData.MasterKeyRemoteAuthenticationHash); + user.MasterPassword = _passwordHasher.HashPassword(user, model.MasterPasswordUnlockData.MasterKeyAuthenticationHash); List saveEncryptedDataActions = new(); if (model.Ciphers.Any()) diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 7e81aaebca18..a4fea601b66e 100644 --- a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -3,7 +3,6 @@ using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.Auth.Models.Request; using Bit.Api.KeyManagement.Controllers; -using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; using Bit.Api.KeyManagement.Validators; using Bit.Api.Tools.Models.Request; @@ -108,7 +107,7 @@ await sutProvider.GetDependency().Received [Theory] [BitAutoData] public async Task RotateUserAccountKeysSuccess(SutProvider sutProvider, - RotateUserAccountKeysModel data, User user) + RotateUserAccountKeysRequestModel data, User user) { sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).Returns(user); sutProvider.GetDependency().RotateUserAccountKeysAsync(Arg.Any(), Arg.Any()) diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index 5452572d7344..665acd79cf52 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -18,7 +18,7 @@ public async Task RejectsWrongOldMasterPassword(SutProvider().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) .Returns(false); var result = await sutProvider.Sut.RotateUserAccountKeysAsync(user, model); @@ -40,7 +40,7 @@ public async Task RejectsEmailChange(SutProvider s model.MasterPasswordUnlockData.KdfIterations = 3; model.MasterPasswordUnlockData.KdfMemory = 64; model.MasterPasswordUnlockData.KdfParallelism = 4; - sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) .Returns(true); await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); } @@ -59,7 +59,7 @@ public async Task RejectsKdfChange(SutProvider sut model.MasterPasswordUnlockData.KdfIterations = 600000; model.MasterPasswordUnlockData.KdfMemory = null; model.MasterPasswordUnlockData.KdfParallelism = null; - sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) .Returns(true); await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); } @@ -79,7 +79,7 @@ public async Task RotatesCorrectly(SutProvider sut model.MasterPasswordUnlockData.KdfMemory = 64; model.MasterPasswordUnlockData.KdfParallelism = 4; - sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyServerAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) .Returns(true); var result = await sutProvider.Sut.RotateUserAccountKeysAsync(user, model); From ba489dbdbe624183abc6617e613e7fac279da73f Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 27 Jan 2025 18:41:42 +0100 Subject: [PATCH 03/32] Cleanup --- .../Controllers/AccountsKeyManagementController.cs | 3 +-- .../Requests/RotateAccountKeysAndDataRequestModel.cs | 2 +- .../Models/Data/RotateUserAccountKeysData.cs | 2 +- .../Implementations/RotateUserAccountkeysCommand.cs | 2 +- .../UserKey/RotateUserAccountKeysCommandTests.cs | 8 ++++---- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs index 83c68991338c..984a77bdc4d5 100644 --- a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs +++ b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs @@ -101,7 +101,7 @@ public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysAndDataR var dataModel = new RotateUserAccountKeysData { - OldMasterkeyAuthenticationHash = model.OldMasterkeyAuthenticationHash, + OldMasterKeyAuthenticationHash = model.OldMasterKeyAuthenticationHash, UserKeyEncryptedAccountPrivateKey = model.AccountKeys.UserKeyEncryptedAccountPrivateKey, AccountPublicKey = model.AccountKeys.AccountPublicKey, @@ -114,7 +114,6 @@ public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysAndDataR Ciphers = await _cipherValidator.ValidateAsync(user, model.AccountData.Ciphers), Folders = await _folderValidator.ValidateAsync(user, model.AccountData.Folders), Sends = await _sendValidator.ValidateAsync(user, model.AccountData.Sends), - }; var result = await _rotateUserAccountKeysCommand.RotateUserAccountKeysAsync(user, dataModel); diff --git a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs index 9a99cc2f3cf9..89b05ca6962f 100644 --- a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs @@ -5,7 +5,7 @@ namespace Bit.Api.KeyManagement.Models.Request.Accounts; public class RotateUserAccountKeysAndDataRequestModel { [Required] - public string OldMasterkeyAuthenticationHash { get; set; } + public string OldMasterKeyAuthenticationHash { get; set; } [Required] public UnlockDataRequestModel AccountUnlockData { get; set; } [Required] diff --git a/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs index 7c3493d7f193..7cb1c273a37c 100644 --- a/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs +++ b/src/Core/KeyManagement/Models/Data/RotateUserAccountKeysData.cs @@ -9,7 +9,7 @@ namespace Bit.Core.KeyManagement.Models.Data; public class RotateUserAccountKeysData { // Authentication for this requests - public string OldMasterkeyAuthenticationHash { get; set; } + public string OldMasterKeyAuthenticationHash { get; set; } // Other keys encrypted by the userkey public string UserKeyEncryptedAccountPrivateKey { get; set; } diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs index 03cc08e3c9f2..d2cb695ae62e 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs @@ -66,7 +66,7 @@ public async Task RotateUserAccountKeysAsync(User user, RotateUs throw new ArgumentNullException(nameof(user)); } - if (!await _userService.CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash)) + if (!await _userService.CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash)) { return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); } diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index 665acd79cf52..f263a205b09a 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -18,7 +18,7 @@ public async Task RejectsWrongOldMasterPassword(SutProvider().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) .Returns(false); var result = await sutProvider.Sut.RotateUserAccountKeysAsync(user, model); @@ -40,7 +40,7 @@ public async Task RejectsEmailChange(SutProvider s model.MasterPasswordUnlockData.KdfIterations = 3; model.MasterPasswordUnlockData.KdfMemory = 64; model.MasterPasswordUnlockData.KdfParallelism = 4; - sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) .Returns(true); await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); } @@ -59,7 +59,7 @@ public async Task RejectsKdfChange(SutProvider sut model.MasterPasswordUnlockData.KdfIterations = 600000; model.MasterPasswordUnlockData.KdfMemory = null; model.MasterPasswordUnlockData.KdfParallelism = null; - sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) .Returns(true); await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); } @@ -79,7 +79,7 @@ public async Task RotatesCorrectly(SutProvider sut model.MasterPasswordUnlockData.KdfMemory = 64; model.MasterPasswordUnlockData.KdfParallelism = 4; - sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterkeyAuthenticationHash) + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) .Returns(true); var result = await sutProvider.Sut.RotateUserAccountKeysAsync(user, model); From ff44902fe10e8a25d8620f58e2bde2262cfa5fc4 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 27 Jan 2025 20:27:01 +0100 Subject: [PATCH 04/32] Update tests --- .../AccountsKeyManagementController.cs | 3 +- .../AccountsKeyManagementControllerTests.cs | 70 +++++++++++++++++++ .../AccountsKeyManagementControllerTests.cs | 41 ++++++++--- 3 files changed, 103 insertions(+), 11 deletions(-) diff --git a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs index 984a77bdc4d5..2eb761086396 100644 --- a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs +++ b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs @@ -91,7 +91,7 @@ await _regenerateUserAsymmetricKeysCommand.RegenerateKeysAsync(request.ToUserAsy [HttpPost("rotate-user-account-keys")] - public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysAndDataRequestModel model) + public async Task RotateUserAccountKeysAsync([FromBody] RotateUserAccountKeysAndDataRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -117,7 +117,6 @@ public async Task RotateUserAccountKeys([FromBody] RotateUserAccountKeysAndDataR }; var result = await _rotateUserAccountKeysCommand.RotateUserAccountKeysAsync(user, dataModel); - if (result.Succeeded) { return; diff --git a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index ec7ca3746010..2d26eed2638a 100644 --- a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -1,13 +1,19 @@ using System.Net; using Bit.Api.IntegrationTest.Factories; using Bit.Api.IntegrationTest.Helpers; +using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; +using Bit.Api.Vault.Models; +using Bit.Api.Vault.Models.Request; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Billing.Enums; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Repositories; +using Bit.Core.Vault.Enums; using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; using Xunit; namespace Bit.Api.IntegrationTest.KeyManagement.Controllers; @@ -23,6 +29,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture _passwordHasher; private string _ownerEmail = null!; public AccountsKeyManagementControllerTests(ApiApplicationFactory factory) @@ -35,6 +42,7 @@ public AccountsKeyManagementControllerTests(ApiApplicationFactory factory) _userRepository = _factory.GetService(); _emergencyAccessRepository = _factory.GetService(); _organizationUserRepository = _factory.GetService(); + _passwordHasher = _factory.GetService>(); } public async Task InitializeAsync() @@ -161,4 +169,66 @@ private async Task CreateDesignatedEmergencyAccessAsync(EmergencyAccessStatusTyp }; await _emergencyAccessRepository.CreateAsync(emergencyAccess); } + + [Theory] + [BitAutoData] + public async Task RotateUserAccountKeysAsync_NotLoggedIn_Unauthorized(RotateUserAccountKeysAndDataRequestModel request) + { + var response = await _client.PostAsJsonAsync("/accounts/key-management/rotate-user-account-keys", request); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Theory] + [BitAutoData] + public async Task RotateUserAccountKeysAsync_Success(RotateUserAccountKeysAndDataRequestModel request) + { + await _loginHelper.LoginAsync(_ownerEmail); + var user = await _userRepository.GetByEmailAsync(_ownerEmail); + if (user == null) + { + throw new InvalidOperationException("User not found."); + } + + var password = _passwordHasher.HashPassword(user, "newMasterPassword"); + user.MasterPassword = password; + user.PublicKey = "publicKey"; + await _userRepository.ReplaceAsync(user); + + request.AccountUnlockData.MasterPasswordUnlockData.KdfType = user.Kdf; + request.AccountUnlockData.MasterPasswordUnlockData.KdfIterations = user.KdfIterations; + request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory = user.KdfMemory; + request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism = user.KdfParallelism; + request.AccountUnlockData.MasterPasswordUnlockData.Email = user.Email; + request.AccountKeys.AccountPublicKey = "publicKey"; + request.AccountKeys.UserKeyEncryptedAccountPrivateKey = _mockEncryptedString; + + request.OldMasterKeyAuthenticationHash = "newMasterPassword"; + + request.AccountData.Ciphers = + [ + new CipherWithIdRequestModel + { + Id = Guid.NewGuid(), + Type = CipherType.Login, + Name = _mockEncryptedString, + Login = new CipherLoginModel + { + Username = _mockEncryptedString, + Password = _mockEncryptedString, + }, + }, + ]; + request.AccountData.Folders = []; + request.AccountData.Sends = []; + request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey = _mockEncryptedString; + request.AccountUnlockData.PasskeyUnlockData = []; + request.AccountUnlockData.EmergencyAccessUnlockData = []; + request.AccountUnlockData.OrganizationAccountRecoveryUnlockData = []; + + var response = await _client.PostAsJsonAsync("/accounts/key-management/rotate-user-account-keys", request); + var rsp = await response.Content.ReadAsStringAsync(); + Console.WriteLine(rsp); + response.EnsureSuccessStatusCode(); + } } diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index a4fea601b66e..792cf2c960b0 100644 --- a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -2,7 +2,9 @@ using System.Security.Claims; using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Models.Request.WebAuthn; using Bit.Api.KeyManagement.Controllers; +using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; using Bit.Api.KeyManagement.Validators; using Bit.Api.Tools.Models.Request; @@ -107,21 +109,42 @@ await sutProvider.GetDependency().Received [Theory] [BitAutoData] public async Task RotateUserAccountKeysSuccess(SutProvider sutProvider, - RotateUserAccountKeysRequestModel data, User user) + RotateUserAccountKeysAndDataRequestModel data, User user) { sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).Returns(user); sutProvider.GetDependency().RotateUserAccountKeysAsync(Arg.Any(), Arg.Any()) .Returns(IdentityResult.Success); - await sutProvider.Sut.RotateUserAccountKeys(data); + await sutProvider.Sut.RotateUserAccountKeysAsync(data); + + await sutProvider.GetDependency, IEnumerable>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.AccountUnlockData.EmergencyAccessUnlockData)); + await sutProvider.GetDependency, IReadOnlyList>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.AccountUnlockData.OrganizationAccountRecoveryUnlockData)); + await sutProvider.GetDependency, IEnumerable>>().Received(1) + .ValidateAsync(Arg.Any(), Arg.Is(data.AccountUnlockData.PasskeyUnlockData)); + await sutProvider.GetDependency, IEnumerable>>().Received(1) - .ValidateAsync(Arg.Any(), Arg.Is(data.Ciphers)); + .ValidateAsync(Arg.Any(), Arg.Is(data.AccountData.Ciphers)); await sutProvider.GetDependency, IEnumerable>>().Received(1) - .ValidateAsync(Arg.Any(), Arg.Is(data.Folders)); + .ValidateAsync(Arg.Any(), Arg.Is(data.AccountData.Folders)); await sutProvider.GetDependency, IReadOnlyList>>().Received(1) - .ValidateAsync(Arg.Any(), Arg.Is(data.Sends)); - await sutProvider.GetDependency, IEnumerable>>().Received(1) - .ValidateAsync(Arg.Any(), Arg.Is(data.EmergencyAccessUnlockData)); - await sutProvider.GetDependency, IReadOnlyList>>().Received(1) - .ValidateAsync(Arg.Any(), Arg.Is(data.OrganizationAccountRecoveryUnlockData)); + .ValidateAsync(Arg.Any(), Arg.Is(data.AccountData.Sends)); + + await sutProvider.GetDependency().Received(1) + .RotateUserAccountKeysAsync(Arg.Is(user), Arg.Is(d => + d.OldMasterKeyAuthenticationHash == data.OldMasterKeyAuthenticationHash + + && d.MasterPasswordUnlockData.KdfType == data.AccountUnlockData.MasterPasswordUnlockData.KdfType + && d.MasterPasswordUnlockData.KdfIterations == data.AccountUnlockData.MasterPasswordUnlockData.KdfIterations + && d.MasterPasswordUnlockData.KdfMemory == data.AccountUnlockData.MasterPasswordUnlockData.KdfMemory + && d.MasterPasswordUnlockData.KdfParallelism == data.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism + && d.MasterPasswordUnlockData.Email == data.AccountUnlockData.MasterPasswordUnlockData.Email + + && d.MasterPasswordUnlockData.MasterKeyAuthenticationHash == data.AccountUnlockData.MasterPasswordUnlockData.MasterKeyAuthenticationHash + && d.MasterPasswordUnlockData.MasterKeyEncryptedUserKey == data.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey + + && d.AccountPublicKey == data.AccountKeys.AccountPublicKey + && d.UserKeyEncryptedAccountPrivateKey == data.AccountKeys.UserKeyEncryptedAccountPrivateKey + )); } } From 4b7667a45fb34200bbd6765a981fe5dc433364fd Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 28 Jan 2025 11:54:06 +0100 Subject: [PATCH 05/32] Improve test --- .../AccountsKeyManagementControllerTests.cs | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 2d26eed2638a..77236c6c97c2 100644 --- a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -3,6 +3,7 @@ using Bit.Api.IntegrationTest.Helpers; using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; +using Bit.Api.Tools.Models.Request; using Bit.Api.Vault.Models; using Bit.Api.Vault.Models.Request; using Bit.Core.Auth.Entities; @@ -219,16 +220,37 @@ public async Task RotateUserAccountKeysAsync_Success(RotateUserAccountKeysAndDat }, }, ]; - request.AccountData.Folders = []; - request.AccountData.Sends = []; + request.AccountData.Folders = [ + new FolderWithIdRequestModel + { + Id = Guid.NewGuid(), + Name = _mockEncryptedString, + }, + ]; + request.AccountData.Sends = [ + new SendWithIdRequestModel + { + Id = Guid.NewGuid(), + Name = _mockEncryptedString, + Key = _mockEncryptedString, + Disabled = false, + DeletionDate = DateTime.UtcNow.AddDays(1), + }, + ]; request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey = _mockEncryptedString; request.AccountUnlockData.PasskeyUnlockData = []; request.AccountUnlockData.EmergencyAccessUnlockData = []; request.AccountUnlockData.OrganizationAccountRecoveryUnlockData = []; var response = await _client.PostAsJsonAsync("/accounts/key-management/rotate-user-account-keys", request); - var rsp = await response.Content.ReadAsStringAsync(); - Console.WriteLine(rsp); response.EnsureSuccessStatusCode(); + + var userNewState = await _userRepository.GetByEmailAsync(_ownerEmail); + Assert.NotNull(userNewState); + Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.Email, userNewState.Email); + Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfType, userNewState.Kdf); + Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfIterations, userNewState.KdfIterations); + Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory, userNewState.KdfMemory); + Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism, userNewState.KdfParallelism); } } From cd40987ca66649e18aa754554afcc967490b5f8d Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 28 Jan 2025 11:59:38 +0100 Subject: [PATCH 06/32] Add tests --- .../RotateUserAccountKeysCommandTests.cs | 27 ++++++++++++++++++- .../UserKey/RotateUserKeyCommandTests.cs | 2 +- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index f263a205b09a..4ed2de45a6c5 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -8,7 +8,7 @@ using NSubstitute; using Xunit; -namespace Bit.Core.Test.KeyManagement.UserFeatures.UserKey; +namespace Bit.Core.Test.KeyManagement.UserKey; [SutProviderCustomize] public class RotateUserAccountKeysCommandTests @@ -64,6 +64,30 @@ public async Task RejectsKdfChange(SutProvider sut await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); } + + [Theory, BitAutoData] + public async Task RejectsPublicKeyChange(SutProvider sutProvider, User user, + RotateUserAccountKeysData model) + { + user.PublicKey = "old-public"; + user.Kdf = Enums.KdfType.Argon2id; + user.KdfIterations = 3; + user.KdfMemory = 64; + user.KdfParallelism = 4; + + model.AccountPublicKey = "new-public"; + model.MasterPasswordUnlockData.Email = user.Email; + model.MasterPasswordUnlockData.KdfType = Enums.KdfType.Argon2id; + model.MasterPasswordUnlockData.KdfIterations = 3; + model.MasterPasswordUnlockData.KdfMemory = 64; + model.MasterPasswordUnlockData.KdfParallelism = 4; + + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) + .Returns(true); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); + } + [Theory, BitAutoData] public async Task RotatesCorrectly(SutProvider sutProvider, User user, RotateUserAccountKeysData model) @@ -87,4 +111,5 @@ public async Task RotatesCorrectly(SutProvider sut Assert.Equal(IdentityResult.Success, result); } + } diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserKeyCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserKeyCommandTests.cs index 53263d88056c..000fa7e90c89 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserKeyCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserKeyCommandTests.cs @@ -11,7 +11,7 @@ using NSubstitute; using Xunit; -namespace Bit.Core.Test.KeyManagement.UserFeatures.UserKey; +namespace Bit.Core.Test.KeyManagement.UserKey; [SutProviderCustomize] public class RotateUserKeyCommandTests From 73f6f672baf044756222625d81cb2ef41c6303cf Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 28 Jan 2025 12:17:32 +0100 Subject: [PATCH 07/32] Fix formatting --- src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs index 27f8769fa0d0..01468908fd03 100644 --- a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs @@ -3,8 +3,6 @@ using Bit.Api.Auth.Models.Request; using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Request.WebAuthn; -using Bit.Api.Tools.Models.Request; -using Bit.Api.Vault.Models.Request; namespace Bit.Api.KeyManagement.Models.Request.Accounts; From a9391328a34a28734e00bc6407806419cbcb59c8 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 28 Jan 2025 12:20:46 +0100 Subject: [PATCH 08/32] Fix test --- .../KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index 4ed2de45a6c5..d8ec42dc8fe3 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -103,6 +103,8 @@ public async Task RotatesCorrectly(SutProvider sut model.MasterPasswordUnlockData.KdfMemory = 64; model.MasterPasswordUnlockData.KdfParallelism = 4; + model.AccountPublicKey = user.PublicKey; + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) .Returns(true); From 2fe3c27b0bbb6dae415d0340f57c652bdc3f9456 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 28 Jan 2025 14:03:54 +0100 Subject: [PATCH 09/32] Remove whitespace --- .../KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index d8ec42dc8fe3..1c9f59cca4bd 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -112,6 +112,4 @@ public async Task RotatesCorrectly(SutProvider sut Assert.Equal(IdentityResult.Success, result); } - - } From 414775cbec48f24db92aff9a81b97bf46fb03cc0 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 14:03:04 +0100 Subject: [PATCH 10/32] Fix namespace --- .../KeyManagement/Models/Requests/AccountKeysRequestModel.cs | 2 +- .../Models/Requests/RotateAccountKeysAndDataRequestModel.cs | 2 +- src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs | 2 +- src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs index b3daabf1a93f..d87b92660676 100644 --- a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs @@ -1,6 +1,6 @@ using System.ComponentModel.DataAnnotations; -namespace Bit.Api.KeyManagement.Models.Request.Accounts; +namespace Bit.Api.KeyManagement.Models.Request; public class AccountKeysRequestModel { diff --git a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs index 89b05ca6962f..4d733fd8d1c9 100644 --- a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs @@ -1,6 +1,6 @@ using System.ComponentModel.DataAnnotations; -namespace Bit.Api.KeyManagement.Models.Request.Accounts; +namespace Bit.Api.KeyManagement.Models.Request; public class RotateUserAccountKeysAndDataRequestModel { diff --git a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs index 01468908fd03..670fcc891a25 100644 --- a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs @@ -4,7 +4,7 @@ using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Request.WebAuthn; -namespace Bit.Api.KeyManagement.Models.Request.Accounts; +namespace Bit.Api.KeyManagement.Models.Request; public class UnlockDataRequestModel { diff --git a/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs index b7c861b7cbb6..2f67be04368a 100644 --- a/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs @@ -1,7 +1,7 @@ using Bit.Api.Tools.Models.Request; using Bit.Api.Vault.Models.Request; -namespace Bit.Api.KeyManagement.Models.Request.Accounts; +namespace Bit.Api.KeyManagement.Models.Request; public class AccountDataRequestModel { From e5c4c9efcc6c0b8b2df06e3428d1da8f80891a05 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 14:14:04 +0100 Subject: [PATCH 11/32] Enable nullable on models --- .../Models/Requests/AccountKeysRequestModel.cs | 11 ++++------- .../RotateAccountKeysAndDataRequestModel.cs | 15 ++++++--------- .../Models/Requests/UnlockDataRequestModel.cs | 11 +++++------ .../Models/Requests/UserDataRequestModel.cs | 9 +++++---- 4 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs index d87b92660676..01e76810d070 100644 --- a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs @@ -1,11 +1,8 @@ -using System.ComponentModel.DataAnnotations; - -namespace Bit.Api.KeyManagement.Models.Request; +#nullable enable +namespace Bit.Api.KeyManagement.Models.Requests; public class AccountKeysRequestModel { - [Required] - public string UserKeyEncryptedAccountPrivateKey { get; set; } - [Required] - public string AccountPublicKey { get; set; } + public required string UserKeyEncryptedAccountPrivateKey { get; set; } + public required string AccountPublicKey { get; set; } } diff --git a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs index 4d733fd8d1c9..9e7927eaae1d 100644 --- a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs @@ -1,15 +1,12 @@ -using System.ComponentModel.DataAnnotations; +#nullable enable +using Bit.Api.KeyManagement.Models.Requests; namespace Bit.Api.KeyManagement.Models.Request; public class RotateUserAccountKeysAndDataRequestModel { - [Required] - public string OldMasterKeyAuthenticationHash { get; set; } - [Required] - public UnlockDataRequestModel AccountUnlockData { get; set; } - [Required] - public AccountKeysRequestModel AccountKeys { get; set; } - [Required] - public AccountDataRequestModel AccountData { get; set; } + public required string OldMasterKeyAuthenticationHash { get; set; } + public required UnlockDataRequestModel AccountUnlockData { get; set; } + public required AccountKeysRequestModel AccountKeys { get; set; } + public required AccountDataRequestModel AccountData { get; set; } } diff --git a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs index 670fcc891a25..2b6b1de03461 100644 --- a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs @@ -1,4 +1,4 @@ -using System.ComponentModel.DataAnnotations; +#nullable enable using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.Auth.Models.Request; using Bit.Api.Auth.Models.Request.Accounts; @@ -9,9 +9,8 @@ namespace Bit.Api.KeyManagement.Models.Request; public class UnlockDataRequestModel { // All methods to get to the userkey - [Required] - public MasterPasswordUnlockDataModel MasterPasswordUnlockData { get; set; } - public IEnumerable EmergencyAccessUnlockData { get; set; } - public IEnumerable OrganizationAccountRecoveryUnlockData { get; set; } - public IEnumerable PasskeyUnlockData { get; set; } + public required MasterPasswordUnlockDataModel MasterPasswordUnlockData { get; set; } + public required IEnumerable EmergencyAccessUnlockData { get; set; } + public required IEnumerable OrganizationAccountRecoveryUnlockData { get; set; } + public required IEnumerable PasskeyUnlockData { get; set; } } diff --git a/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs index 2f67be04368a..73f67472447e 100644 --- a/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs @@ -1,11 +1,12 @@ -using Bit.Api.Tools.Models.Request; +#nullable enable +using Bit.Api.Tools.Models.Request; using Bit.Api.Vault.Models.Request; namespace Bit.Api.KeyManagement.Models.Request; public class AccountDataRequestModel { - public IEnumerable Ciphers { get; set; } - public IEnumerable Folders { get; set; } - public IEnumerable Sends { get; set; } + public required IEnumerable Ciphers { get; set; } + public required IEnumerable Folders { get; set; } + public required IEnumerable Sends { get; set; } } From fb952dac07f0674d14bab510965c45f0760ff07c Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 14:17:57 +0100 Subject: [PATCH 12/32] Fix build --- .../Controllers/AccountsKeyManagementController.cs | 1 - .../Models/Requests/RotateAccountKeysAndDataRequestModel.cs | 4 +--- .../KeyManagement/Models/Requests/UnlockDataRequestModel.cs | 2 +- src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs | 2 +- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs index 2eb761086396..85e0981f2262 100644 --- a/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs +++ b/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs @@ -2,7 +2,6 @@ using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.Auth.Models.Request; using Bit.Api.Auth.Models.Request.WebAuthn; -using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; using Bit.Api.KeyManagement.Validators; using Bit.Api.Tools.Models.Request; diff --git a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs index 9e7927eaae1d..aa6b18f8f392 100644 --- a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs @@ -1,7 +1,5 @@ #nullable enable -using Bit.Api.KeyManagement.Models.Requests; - -namespace Bit.Api.KeyManagement.Models.Request; +namespace Bit.Api.KeyManagement.Models.Requests; public class RotateUserAccountKeysAndDataRequestModel { diff --git a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs index 2b6b1de03461..5156e2a65545 100644 --- a/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UnlockDataRequestModel.cs @@ -4,7 +4,7 @@ using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Request.WebAuthn; -namespace Bit.Api.KeyManagement.Models.Request; +namespace Bit.Api.KeyManagement.Models.Requests; public class UnlockDataRequestModel { diff --git a/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs index 73f67472447e..f854d82bcc65 100644 --- a/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/UserDataRequestModel.cs @@ -2,7 +2,7 @@ using Bit.Api.Tools.Models.Request; using Bit.Api.Vault.Models.Request; -namespace Bit.Api.KeyManagement.Models.Request; +namespace Bit.Api.KeyManagement.Models.Requests; public class AccountDataRequestModel { From 2495d07a219b7703ad8dd6f1751f9ecb792d093b Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 15:17:51 +0100 Subject: [PATCH 13/32] Add tests and enable nullable on masterpasswordunlockdatamodel --- .../Accounts/MasterPasswordUnlockDataModel.cs | 21 +++---- .../AccountsKeyManagementControllerTests.cs | 1 - .../Request/MasterPasswordUnlockDataModel.cs | 63 +++++++++++++++++++ 3 files changed, 72 insertions(+), 13 deletions(-) create mode 100644 test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs index 7b08f4edfcc8..da6452de2cf0 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs @@ -1,4 +1,6 @@ -using System.ComponentModel.DataAnnotations; +#nullable enable + +using System.ComponentModel.DataAnnotations; using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Data; @@ -6,22 +8,17 @@ namespace Bit.Api.Auth.Models.Request.Accounts; public class MasterPasswordUnlockDataModel : IValidatableObject { - [Required] - public KdfType KdfType { get; set; } - [Required] - public int KdfIterations { get; set; } + public required KdfType KdfType { get; set; } + public required int KdfIterations { get; set; } public int? KdfMemory { get; set; } public int? KdfParallelism { get; set; } - [Required] - public string Email { get; set; } - [Required] + public required string Email { get; set; } [StringLength(300)] - public string MasterKeyAuthenticationHash { get; set; } - [Required] - public string MasterKeyEncryptedUserKey { get; set; } + public required string MasterKeyAuthenticationHash { get; set; } + public required string MasterKeyEncryptedUserKey { get; set; } [StringLength(50)] - public string MasterPasswordHint { get; set; } + public required string MasterPasswordHint { get; set; } public IEnumerable Validate(ValidationContext validationContext) { diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 792cf2c960b0..6ea6a0cc72ba 100644 --- a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -4,7 +4,6 @@ using Bit.Api.Auth.Models.Request; using Bit.Api.Auth.Models.Request.WebAuthn; using Bit.Api.KeyManagement.Controllers; -using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; using Bit.Api.KeyManagement.Validators; using Bit.Api.Tools.Models.Request; diff --git a/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs new file mode 100644 index 000000000000..a7b0b91979df --- /dev/null +++ b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs @@ -0,0 +1,63 @@ +#nullable enable +using System.ComponentModel.DataAnnotations; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Core.Enums; +using Xunit; + +namespace Bit.Api.Test.KeyManagement.Models.Request; + +public class MasterPasswordUnlockDataModelTests +{ + [Theory] + [InlineData(KdfType.PBKDF2_SHA256, 5000, null, null)] + [InlineData(KdfType.PBKDF2_SHA256, 100000, null, null)] + [InlineData(KdfType.PBKDF2_SHA256, 600000, null, null)] + [InlineData(KdfType.Argon2id, 3, 64, 4)] + public void Validate_Success(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) + { + var model = new MasterPasswordUnlockDataModel + { + KdfType = kdfType, + KdfIterations = kdfIterations, + KdfMemory = kdfMemory, + KdfParallelism = kdfParallelism, + Email = "example@example.com", + MasterKeyAuthenticationHash = "hash", + MasterKeyEncryptedUserKey = "key", + MasterPasswordHint = "hint" + }; + var result = Validate(model); + Assert.Empty(result); + } + + [Theory] + [InlineData(KdfType.Argon2id, 1, null, 1)] + [InlineData(KdfType.Argon2id, 1, 64, null)] + [InlineData(KdfType.PBKDF2_SHA256, 5000, 0, null)] + [InlineData(KdfType.PBKDF2_SHA256, 5000, null, 0)] + [InlineData(KdfType.PBKDF2_SHA256, 5000, 0, 0)] + public void Validate_Failure(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) + { + var model = new MasterPasswordUnlockDataModel + { + KdfType = kdfType, + KdfIterations = kdfIterations, + KdfMemory = kdfMemory, + KdfParallelism = kdfParallelism, + Email = "example@example.com", + MasterKeyAuthenticationHash = "hash", + MasterKeyEncryptedUserKey = "key", + MasterPasswordHint = "hint" + }; + var result = Validate(model); + Assert.Single(result); + Assert.NotNull(result.First().ErrorMessage); + } + + private static List Validate(MasterPasswordUnlockDataModel model) + { + var results = new List(); + Validator.TryValidateObject(model, new ValidationContext(model), results, true); + return results; + } +} From f462ef2ab54074c641036df8ea84024750fba288 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 15:47:33 +0100 Subject: [PATCH 14/32] Fix test --- .../Controllers/AccountsKeyManagementControllerTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 77236c6c97c2..7c05e1d68084 100644 --- a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -1,7 +1,6 @@ using System.Net; using Bit.Api.IntegrationTest.Factories; using Bit.Api.IntegrationTest.Helpers; -using Bit.Api.KeyManagement.Models.Request.Accounts; using Bit.Api.KeyManagement.Models.Requests; using Bit.Api.Tools.Models.Request; using Bit.Api.Vault.Models; From bcfe85220ba26c448acddde4a0d6fcc3c9dd38f0 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 19:05:30 +0100 Subject: [PATCH 15/32] Remove rollback --- .../Repositories/UserRepository.cs | 44 +++++++------------ 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index d092c6fbcb66..469ceafe22ca 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -262,37 +262,27 @@ public async Task UpdateUserKeyAndEncryptedDataV2Async( connection.Open(); await using var transaction = connection.BeginTransaction(); - try + if (user.AccountRevisionDate == null) { - if (user.AccountRevisionDate == null) - { - user.AccountRevisionDate = user.RevisionDate; - } - - ProtectData(user); - await connection.ExecuteAsync( - $"[{Schema}].[{Table}_Update]", - user, - transaction: transaction, - commandType: CommandType.StoredProcedure); - - // Update re-encrypted data - foreach (var action in updateDataActions) - { - await action(connection, transaction); - } - transaction.Commit(); - } - catch - { - transaction.Rollback(); - throw; + user.AccountRevisionDate = user.RevisionDate; } - finally + + ProtectData(user); + await connection.ExecuteAsync( + $"[{Schema}].[{Table}_Update]", + user, + transaction: transaction, + commandType: CommandType.StoredProcedure); + + // Update re-encrypted data + foreach (var action in updateDataActions) { - connection.Close(); - UnprotectData(user); + await action(connection, transaction); } + transaction.Commit(); + + connection.Close(); + UnprotectData(user); } From 0567da745cb8432b6d20ffc54e890c3fa050ac9e Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 19:31:19 +0100 Subject: [PATCH 16/32] Add tests --- .../AccountsKeyManagementControllerTests.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 6ea6a0cc72ba..21d9e9e9c271 100644 --- a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -146,4 +146,28 @@ await sutProvider.GetDependency().Received(1) && d.UserKeyEncryptedAccountPrivateKey == data.AccountKeys.UserKeyEncryptedAccountPrivateKey )); } + + + [Theory] + [BitAutoData] + public async Task RotateUserKeyNoUser_Throws(SutProvider sutProvider, + RotateUserAccountKeysAndDataRequestModel data) + { + User? user = null; + sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).Returns(user); + sutProvider.GetDependency().RotateUserAccountKeysAsync(Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Success); + await Assert.ThrowsAsync(() => sutProvider.Sut.RotateUserAccountKeysAsync(data)); + } + + [Theory] + [BitAutoData] + public async Task RotateUserKeyWrongData_Throws(SutProvider sutProvider, + RotateUserAccountKeysAndDataRequestModel data, User user, IdentityErrorDescriber _identityErrorDescriber) + { + sutProvider.GetDependency().GetUserByPrincipalAsync(Arg.Any()).Returns(user); + sutProvider.GetDependency().RotateUserAccountKeysAsync(Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch())); + await Assert.ThrowsAsync(() => sutProvider.Sut.RotateUserAccountKeysAsync(data)); + } } From de5b91454be35a5faf10862a686c8eac1d3a2528 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 19:47:45 +0100 Subject: [PATCH 17/32] Make masterpassword hint optional --- .../Accounts/MasterPasswordUnlockDataModel.cs | 2 +- .../Models/Data/MasterPasswordUnlockData.cs | 11 +++--- .../Repositories/UserRepository.cs | 38 +++++++++++-------- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs index da6452de2cf0..7a5dd0d88976 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs @@ -18,7 +18,7 @@ public class MasterPasswordUnlockDataModel : IValidatableObject public required string MasterKeyAuthenticationHash { get; set; } public required string MasterKeyEncryptedUserKey { get; set; } [StringLength(50)] - public required string MasterPasswordHint { get; set; } + public string? MasterPasswordHint { get; set; } public IEnumerable Validate(ValidationContext validationContext) { diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs index 623b29ea7396..0ddfc0319002 100644 --- a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs @@ -1,4 +1,5 @@ -using Bit.Core.Entities; +#nullable enable +using Bit.Core.Entities; using Bit.Core.Enums; namespace Bit.Core.KeyManagement.Models.Data; @@ -10,10 +11,10 @@ public class MasterPasswordUnlockData public int? KdfMemory { get; set; } public int? KdfParallelism { get; set; } - public string Email { get; set; } - public string MasterKeyAuthenticationHash { get; set; } - public string MasterKeyEncryptedUserKey { get; set; } - public string MasterPasswordHint { get; set; } + public required string Email { get; set; } + public required string MasterKeyAuthenticationHash { get; set; } + public required string MasterKeyEncryptedUserKey { get; set; } + public string? MasterPasswordHint { get; set; } public bool ValidateForUser(User user) { diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index 469ceafe22ca..b234a132815c 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -262,25 +262,33 @@ public async Task UpdateUserKeyAndEncryptedDataV2Async( connection.Open(); await using var transaction = connection.BeginTransaction(); - if (user.AccountRevisionDate == null) + try { - user.AccountRevisionDate = user.RevisionDate; - } + if (user.AccountRevisionDate == null) + { + user.AccountRevisionDate = user.RevisionDate; + } - ProtectData(user); - await connection.ExecuteAsync( - $"[{Schema}].[{Table}_Update]", - user, - transaction: transaction, - commandType: CommandType.StoredProcedure); - - // Update re-encrypted data - foreach (var action in updateDataActions) + ProtectData(user); + await connection.ExecuteAsync( + $"[{Schema}].[{Table}_Update]", + user, + transaction: transaction, + commandType: CommandType.StoredProcedure); + + // Update re-encrypted data + foreach (var action in updateDataActions) + { + await action(connection, transaction); + } + transaction.Commit(); + } + catch { - await action(connection, transaction); + connection.Close(); + UnprotectData(user); + throw; } - transaction.Commit(); - connection.Close(); UnprotectData(user); } From 28200554dbd09bd98c0fdbad2cf937e86af908ee Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 29 Jan 2025 20:07:00 +0100 Subject: [PATCH 18/32] Update user query --- .../Repositories/UserRepository.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index cbab13b1a461..f8918100f71a 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -170,6 +170,17 @@ public async Task UpdateUserKeyAndEncryptedDataAsync(Core.Entities.User user, entity.SecurityStamp = user.SecurityStamp; entity.Key = user.Key; + + entity.Kdf = user.Kdf; + entity.KdfIterations = user.KdfIterations; + entity.KdfMemory = user.KdfMemory; + entity.KdfParallelism = user.KdfParallelism; + + entity.Email = user.Email; + + entity.MasterPassword = user.MasterPassword; + entity.MasterPasswordHint = user.MasterPasswordHint; + entity.PrivateKey = user.PrivateKey; entity.LastKeyRotationDate = user.LastKeyRotationDate; entity.AccountRevisionDate = user.AccountRevisionDate; From 09587622f4accd3728f5bb5d48563bf07c0d7b95 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 30 Jan 2025 14:18:18 +0100 Subject: [PATCH 19/32] Add EF test --- .../Repositories/UserRepository.cs | 22 ++++++++--------- .../Repositories/UserRepositoryTests.cs | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index f8918100f71a..e6f147e7ee37 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -171,16 +171,6 @@ public async Task UpdateUserKeyAndEncryptedDataAsync(Core.Entities.User user, entity.SecurityStamp = user.SecurityStamp; entity.Key = user.Key; - entity.Kdf = user.Kdf; - entity.KdfIterations = user.KdfIterations; - entity.KdfMemory = user.KdfMemory; - entity.KdfParallelism = user.KdfParallelism; - - entity.Email = user.Email; - - entity.MasterPassword = user.MasterPassword; - entity.MasterPasswordHint = user.MasterPasswordHint; - entity.PrivateKey = user.PrivateKey; entity.LastKeyRotationDate = user.LastKeyRotationDate; entity.AccountRevisionDate = user.AccountRevisionDate; @@ -225,8 +215,18 @@ public async Task UpdateUserKeyAndEncryptedDataV2Async(Core.Entities.User user, userEntity.SecurityStamp = user.SecurityStamp; userEntity.Key = user.Key; - userEntity.MasterPassword = user.MasterPassword; userEntity.PrivateKey = user.PrivateKey; + + userEntity.Kdf = user.Kdf; + userEntity.KdfIterations = user.KdfIterations; + userEntity.KdfMemory = user.KdfMemory; + userEntity.KdfParallelism = user.KdfParallelism; + + userEntity.Email = user.Email; + + userEntity.MasterPassword = user.MasterPassword; + userEntity.MasterPasswordHint = user.MasterPasswordHint; + userEntity.LastKeyRotationDate = user.LastKeyRotationDate; userEntity.AccountRevisionDate = user.AccountRevisionDate; userEntity.RevisionDate = user.RevisionDate; diff --git a/test/Infrastructure.EFIntegration.Test/Repositories/UserRepositoryTests.cs b/test/Infrastructure.EFIntegration.Test/Repositories/UserRepositoryTests.cs index 066a550fa87a..151bd47c44a0 100644 --- a/test/Infrastructure.EFIntegration.Test/Repositories/UserRepositoryTests.cs +++ b/test/Infrastructure.EFIntegration.Test/Repositories/UserRepositoryTests.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Entities; using Bit.Core.Entities; +using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Test.AutoFixture.Attributes; using Bit.Infrastructure.EFIntegration.Test.AutoFixture; @@ -289,4 +290,27 @@ public async Task GetBySsoUserAsync_Works_DataMatches(User user, Organization or var distinctItems = returnedList.Distinct(equalityComparer); Assert.True(!distinctItems.Skip(1).Any()); } + + [CiSkippedTheory, EfUserAutoData] + public async Task UpdateUserKeyAndEncryptedDataAsync_Works_DataMatches(User user, SqlRepo.UserRepository sqlUserRepo) + { + var sqlUser = await sqlUserRepo.CreateAsync(user); + sqlUser.Kdf = KdfType.PBKDF2_SHA256; + sqlUser.KdfIterations = 6_000_000; + sqlUser.KdfMemory = 7_000_000; + sqlUser.KdfParallelism = 8_000_000; + sqlUser.MasterPassword = "masterPasswordHash"; + sqlUser.MasterPasswordHint = "masterPasswordHint"; + sqlUser.Email = "example@example.com"; + + await sqlUserRepo.UpdateUserKeyAndEncryptedDataV2Async(sqlUser, []); + var updatedUser = await sqlUserRepo.GetByIdAsync(sqlUser.Id); + Assert.Equal(sqlUser.Kdf, updatedUser.Kdf); + Assert.Equal(sqlUser.KdfIterations, updatedUser.KdfIterations); + Assert.Equal(sqlUser.KdfMemory, updatedUser.KdfMemory); + Assert.Equal(sqlUser.KdfParallelism, updatedUser.KdfParallelism); + Assert.Equal(sqlUser.MasterPassword, updatedUser.MasterPassword); + Assert.Equal(sqlUser.MasterPasswordHint, updatedUser.MasterPasswordHint); + Assert.Equal(sqlUser.Email, updatedUser.Email); + } } From e95e4a20e61696d0be2c7d437ea66e615357ed63 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 30 Jan 2025 15:05:01 +0100 Subject: [PATCH 20/32] Improve test --- .../AccountsKeyManagementControllerTests.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index 21d9e9e9c271..49c4f88cb4ee 100644 --- a/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/test/Api.Test/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs @@ -168,6 +168,14 @@ public async Task RotateUserKeyWrongData_Throws(SutProvider().GetUserByPrincipalAsync(Arg.Any()).Returns(user); sutProvider.GetDependency().RotateUserAccountKeysAsync(Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch())); - await Assert.ThrowsAsync(() => sutProvider.Sut.RotateUserAccountKeysAsync(data)); + try + { + await sutProvider.Sut.RotateUserAccountKeysAsync(data); + Assert.Fail("Should have thrown"); + } + catch (BadRequestException ex) + { + Assert.NotEmpty(ex.ModelState.Values); + } } } From 52626110ea6e55c76540e98d463a63f22c003502 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 30 Jan 2025 15:08:49 +0100 Subject: [PATCH 21/32] Cleanup --- .../Repositories/UserRepository.cs | 57 ++++++++----------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index e6f147e7ee37..127646ed597e 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -204,50 +204,41 @@ public async Task UpdateUserKeyAndEncryptedDataV2Async(Core.Entities.User user, await using var transaction = await dbContext.Database.BeginTransactionAsync(); - try + // Update user + var userEntity = await dbContext.Users.FindAsync(user.Id); + if (userEntity == null) { - // Update user - var userEntity = await dbContext.Users.FindAsync(user.Id); - if (userEntity == null) - { - throw new ArgumentException("User not found", nameof(user)); - } + throw new ArgumentException("User not found", nameof(user)); + } - userEntity.SecurityStamp = user.SecurityStamp; - userEntity.Key = user.Key; - userEntity.PrivateKey = user.PrivateKey; + userEntity.SecurityStamp = user.SecurityStamp; + userEntity.Key = user.Key; + userEntity.PrivateKey = user.PrivateKey; - userEntity.Kdf = user.Kdf; - userEntity.KdfIterations = user.KdfIterations; - userEntity.KdfMemory = user.KdfMemory; - userEntity.KdfParallelism = user.KdfParallelism; + userEntity.Kdf = user.Kdf; + userEntity.KdfIterations = user.KdfIterations; + userEntity.KdfMemory = user.KdfMemory; + userEntity.KdfParallelism = user.KdfParallelism; - userEntity.Email = user.Email; + userEntity.Email = user.Email; - userEntity.MasterPassword = user.MasterPassword; - userEntity.MasterPasswordHint = user.MasterPasswordHint; + userEntity.MasterPassword = user.MasterPassword; + userEntity.MasterPasswordHint = user.MasterPasswordHint; - userEntity.LastKeyRotationDate = user.LastKeyRotationDate; - userEntity.AccountRevisionDate = user.AccountRevisionDate; - userEntity.RevisionDate = user.RevisionDate; + userEntity.LastKeyRotationDate = user.LastKeyRotationDate; + userEntity.AccountRevisionDate = user.AccountRevisionDate; + userEntity.RevisionDate = user.RevisionDate; - await dbContext.SaveChangesAsync(); + await dbContext.SaveChangesAsync(); - // Update re-encrypted data - foreach (var action in updateDataActions) - { - // connection and transaction aren't used in EF - await action(); - } - - await transaction.CommitAsync(); - } - catch + // Update re-encrypted data + foreach (var action in updateDataActions) { - await transaction.RollbackAsync(); - throw; + // connection and transaction aren't used in EF + await action(); } + await transaction.CommitAsync(); } public async Task> GetManyAsync(IEnumerable ids) From 7cbc7a5c3a7215a3c1264b0784111df9ba420e04 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 30 Jan 2025 15:20:26 +0100 Subject: [PATCH 22/32] Set masterpassword hint --- .../UserKey/Implementations/RotateUserAccountkeysCommand.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs index d2cb695ae62e..f4dcf31d5c05 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs @@ -92,6 +92,7 @@ public async Task RotateUserAccountKeysAsync(User user, RotateUs user.Key = model.MasterPasswordUnlockData.MasterKeyEncryptedUserKey; user.PrivateKey = model.UserKeyEncryptedAccountPrivateKey; user.MasterPassword = _passwordHasher.HashPassword(user, model.MasterPasswordUnlockData.MasterKeyAuthenticationHash); + user.MasterPasswordHint = model.MasterPasswordUnlockData.MasterPasswordHint; List saveEncryptedDataActions = new(); if (model.Ciphers.Any()) From a8a59da753defe9042bdda4a9f743785fec30116 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 12:23:42 +0100 Subject: [PATCH 23/32] Remove connection close --- src/Infrastructure.Dapper/Repositories/UserRepository.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index b234a132815c..d3eb866385e3 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -285,11 +285,9 @@ await connection.ExecuteAsync( } catch { - connection.Close(); UnprotectData(user); throw; } - connection.Close(); UnprotectData(user); } From 0ca2caa7cf4c22565ab0ee7308ac1b9eb8b3aba8 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 12:28:26 +0100 Subject: [PATCH 24/32] Add tests for invalid kdf types --- .../Models/Request/MasterPasswordUnlockDataModel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs index a7b0b91979df..0124e06bcf0a 100644 --- a/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs +++ b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs @@ -36,6 +36,8 @@ public void Validate_Success(KdfType kdfType, int kdfIterations, int? kdfMemory, [InlineData(KdfType.PBKDF2_SHA256, 5000, 0, null)] [InlineData(KdfType.PBKDF2_SHA256, 5000, null, 0)] [InlineData(KdfType.PBKDF2_SHA256, 5000, 0, 0)] + [InlineData((KdfType)2, 100000, null, null)] + [InlineData((KdfType)2, 2, 64, 4)] public void Validate_Failure(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) { var model = new MasterPasswordUnlockDataModel From 39bb255b025393d17fb195ef0dc5ae7e06be823c Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 12:29:49 +0100 Subject: [PATCH 25/32] Update test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> --- .../UserKey/RotateUserAccountKeysCommandTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index 1c9f59cca4bd..c7c5425631a1 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -25,7 +25,12 @@ public async Task RejectsWrongOldMasterPassword(SutProvider sutProvider, + RotateUserAccountKeysData model) + { + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(null, model)); + } [Theory, BitAutoData] public async Task RejectsEmailChange(SutProvider sutProvider, User user, RotateUserAccountKeysData model) From b8e8d27e75b8c1d0ff9ee0d00c0ae22f9d59a625 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 12:49:58 +0100 Subject: [PATCH 26/32] Fix formatting --- .../UserKey/RotateUserAccountKeysCommandTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index c7c5425631a1..e677814fc1f7 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -25,9 +25,9 @@ public async Task RejectsWrongOldMasterPassword(SutProvider sutProvider, - RotateUserAccountKeysData model) + RotateUserAccountKeysData model) { await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(null, model)); } From 39201fbe91ba3a2eabdc1bd585f3d50946783875 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 18:30:25 +0100 Subject: [PATCH 27/32] Update src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> --- .../Models/Requests/RotateAccountKeysAndDataRequestModel.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs index aa6b18f8f392..856faf02665a 100644 --- a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs @@ -3,6 +3,7 @@ namespace Bit.Api.KeyManagement.Models.Requests; public class RotateUserAccountKeysAndDataRequestModel { + [StringLength(300)] public required string OldMasterKeyAuthenticationHash { get; set; } public required UnlockDataRequestModel AccountUnlockData { get; set; } public required AccountKeysRequestModel AccountKeys { get; set; } From ab4afa0bd7ac216779ab14529781f8541e80b8e5 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 18:30:33 +0100 Subject: [PATCH 28/32] Update src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> --- .../Models/Request/Accounts/MasterPasswordUnlockDataModel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs index 7a5dd0d88976..4b47c52faa64 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs @@ -13,6 +13,8 @@ public class MasterPasswordUnlockDataModel : IValidatableObject public int? KdfMemory { get; set; } public int? KdfParallelism { get; set; } + [StrictEmailAddress] + [StringLength(256)] public required string Email { get; set; } [StringLength(300)] public required string MasterKeyAuthenticationHash { get; set; } From c64b00716504eb3a486b010657e584af31e7c867 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 18:31:00 +0100 Subject: [PATCH 29/32] Update src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> --- .../Models/Request/Accounts/MasterPasswordUnlockDataModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs index 4b47c52faa64..4657b04bf6e1 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs @@ -18,7 +18,7 @@ public class MasterPasswordUnlockDataModel : IValidatableObject public required string Email { get; set; } [StringLength(300)] public required string MasterKeyAuthenticationHash { get; set; } - public required string MasterKeyEncryptedUserKey { get; set; } + [EncryptedString] public required string MasterKeyEncryptedUserKey { get; set; } [StringLength(50)] public string? MasterPasswordHint { get; set; } From 3b767d8c750cd6cb034fed6fd18788e2392d9f90 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 18:31:07 +0100 Subject: [PATCH 30/32] Update src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs Co-authored-by: Thomas Avery <43214426+Thomas-Avery@users.noreply.github.com> --- .../KeyManagement/Models/Requests/AccountKeysRequestModel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs index 01e76810d070..921f2a273a33 100644 --- a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs @@ -3,6 +3,6 @@ namespace Bit.Api.KeyManagement.Models.Requests; public class AccountKeysRequestModel { - public required string UserKeyEncryptedAccountPrivateKey { get; set; } + [EncryptedString] public required string UserKeyEncryptedAccountPrivateKey { get; set; } public required string AccountPublicKey { get; set; } } From d67b4d6084a78c568232653fb0b78209d00e89b8 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 18:35:49 +0100 Subject: [PATCH 31/32] Fix imports --- .../Models/Request/Accounts/MasterPasswordUnlockDataModel.cs | 1 + .../KeyManagement/Models/Requests/AccountKeysRequestModel.cs | 2 ++ .../Models/Requests/RotateAccountKeysAndDataRequestModel.cs | 2 ++ 3 files changed, 5 insertions(+) diff --git a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs index 4657b04bf6e1..ba57788cec10 100644 --- a/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/MasterPasswordUnlockDataModel.cs @@ -3,6 +3,7 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Utilities; namespace Bit.Api.Auth.Models.Request.Accounts; diff --git a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs index 921f2a273a33..7c7de4d210c0 100644 --- a/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/AccountKeysRequestModel.cs @@ -1,4 +1,6 @@ #nullable enable +using Bit.Core.Utilities; + namespace Bit.Api.KeyManagement.Models.Requests; public class AccountKeysRequestModel diff --git a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs index 856faf02665a..b0b19e2bd3e5 100644 --- a/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs +++ b/src/Api/KeyManagement/Models/Requests/RotateAccountKeysAndDataRequestModel.cs @@ -1,4 +1,6 @@ #nullable enable +using System.ComponentModel.DataAnnotations; + namespace Bit.Api.KeyManagement.Models.Requests; public class RotateUserAccountKeysAndDataRequestModel From 49a6a82894f537f7346177ca4884458720370146 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 31 Jan 2025 19:38:40 +0100 Subject: [PATCH 32/32] Fix tests --- .../Models/Request/MasterPasswordUnlockDataModel.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs index 0124e06bcf0a..4c78c7015a18 100644 --- a/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs +++ b/test/Api.Test/KeyManagement/Models/Request/MasterPasswordUnlockDataModel.cs @@ -8,6 +8,9 @@ namespace Bit.Api.Test.KeyManagement.Models.Request; public class MasterPasswordUnlockDataModelTests { + + readonly string _mockEncryptedString = "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98sp4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg="; + [Theory] [InlineData(KdfType.PBKDF2_SHA256, 5000, null, null)] [InlineData(KdfType.PBKDF2_SHA256, 100000, null, null)] @@ -23,7 +26,7 @@ public void Validate_Success(KdfType kdfType, int kdfIterations, int? kdfMemory, KdfParallelism = kdfParallelism, Email = "example@example.com", MasterKeyAuthenticationHash = "hash", - MasterKeyEncryptedUserKey = "key", + MasterKeyEncryptedUserKey = _mockEncryptedString, MasterPasswordHint = "hint" }; var result = Validate(model); @@ -48,7 +51,7 @@ public void Validate_Failure(KdfType kdfType, int kdfIterations, int? kdfMemory, KdfParallelism = kdfParallelism, Email = "example@example.com", MasterKeyAuthenticationHash = "hash", - MasterKeyEncryptedUserKey = "key", + MasterKeyEncryptedUserKey = _mockEncryptedString, MasterPasswordHint = "hint" }; var result = Validate(model);