diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 95b83a8f259c..0542d5b00473 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -324,11 +324,6 @@ public async Task PostKdf([FromBody] ChangeKdfRequestModel model) throw new UnauthorizedAccessException(); } - if (model.AuthenticationData == null || model.UnlockData == null) - { - throw new BadRequestException("AuthenticationData and UnlockData must be provided."); - } - var result = await _changeKdfCommand.ChangeKdfAsync(user, model.MasterPasswordHash, model.AuthenticationData.ToData(), model.UnlockData.ToData()); if (result.Succeeded) { diff --git a/src/Api/Auth/Models/Request/Accounts/ChangeKdfRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/ChangeKdfRequestModel.cs index 6496f45ca0ab..bb5bc6a5a648 100644 --- a/src/Api/Auth/Models/Request/Accounts/ChangeKdfRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/ChangeKdfRequestModel.cs @@ -1,51 +1,36 @@ using System.ComponentModel.DataAnnotations; -using Bit.Core.Auth.Utilities; using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.Utilities; namespace Bit.Api.Auth.Models.Request.Accounts; /// -/// Dual-shape request: validation accepts either the legacy -/// (, ) or new -/// (, ) payload so the wire contract -/// can stabilize ahead of caller wiring. PostKdf currently honors only the new shape; -/// legacy-shape dispatch arrives with ChangeKdfCommand's dual-path refactor. All legacy -/// fields are removed in PM-33141. +/// Request model for changing the KDF settings for a user's account. /// public class ChangeKdfRequestModel : IValidatableObject { + // The current master password hash; proves user has access to the MP [Required] public required string MasterPasswordHash { get; set; } - [Obsolete("To be removed in PM-33141")] - [StringLength(300)] - public string? NewMasterPasswordHash { get; set; } - [Obsolete("To be removed in PM-33141")] - public string? Key { get; set; } - // Should be made required in PM-33141 - public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } - // Should be made required in PM-33141 - public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } + [Required] + public required MasterPasswordAuthenticationDataRequestModel AuthenticationData { get; set; } + [Required] + public required MasterPasswordUnlockDataRequestModel UnlockData { get; set; } public IEnumerable Validate(ValidationContext validationContext) { - var hasNewPayloads = AuthenticationData is not null && UnlockData is not null; - var hasLegacyPayloads = NewMasterPasswordHash is not null && Key is not null; - - foreach (var validationResult in MasterPasswordPayloadVariantValidator.ValidatePresence( - hasNewPayloads, hasLegacyPayloads)) + // [Required] on AuthenticationData/UnlockData reports null-field errors via ModelState; + // this method only runs cross-field validation when both are present. + if (AuthenticationData == null || UnlockData == null) { - yield return validationResult; + yield break; } - if (hasNewPayloads) + foreach (var validationResult in KdfSettingsValidator.ValidateAuthenticationAndUnlockData( + AuthenticationData.ToData(), UnlockData.ToData())) { - foreach (var validationResult in KdfSettingsValidator.ValidateAuthenticationAndUnlockData( - AuthenticationData!.ToData(), UnlockData!.ToData())) - { - yield return validationResult; - } + yield return validationResult; } } } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingKdfConfigurationData.cs similarity index 94% rename from src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs rename to src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingKdfConfigurationData.cs index ed9812a890cc..2077c40d073b 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingKdfConfigurationData.cs @@ -5,16 +5,16 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; /// -/// Input data for updating a user's existing master password hash together with new KDF parameters. +/// Input data for updating a user's KDF configuration together with the re-derived authentication hash. /// Salt is validated as unchanged; KDF validation is intentionally skipped since KDF is being replaced. /// /// /// Use when: constructing a call to -/// +/// /// (KDF rotation flows). For hash-only updates, use instead. /// /// -public class UpdateExistingPasswordAndKdfData +public class UpdateExistingKdfConfigurationData { public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs index 0f3d9c3934c5..8e2002470656 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs @@ -13,7 +13,7 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; /// Use when: constructing a call to /// or /// . -/// For KDF rotation, use instead. +/// For KDF rotation, use instead. /// /// public class UpdateExistingPasswordData diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 46f7c47c1c8e..6d3e0ca4c701 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -255,11 +255,12 @@ public interface IMasterPasswordService Task> PrepareUpdateExistingMasterPasswordAsync(User user, UpdateExistingPasswordData updateExistingData); /// - /// Applies a new master password and updated KDF parameters over the user's existing ones - /// and persists the updated user to the database. KDF validation is intentionally skipped. + /// Updates the user's KDF parameters and persists the updated user to the database. + /// The user's master password is not changed; the + /// authentication hash is re-derived from the existing password using the new KDF parameters. /// /// - /// Use when: rotating KDF parameters. + /// Use when: updating KDF parameters. /// /// /// @@ -275,18 +276,18 @@ public interface IMasterPasswordService /// /// The user object to mutate and persist. Must already have a master password; /// must not be a Key Connector user. Salt must be unchanged. Validated via - /// . + /// . /// /// - /// Cryptographic and authentication data for the updated password and KDF parameters, - /// including MasterPasswordUnlock, MasterPasswordAuthentication, + /// Cryptographic and authentication data for KDF configuration, including + /// MasterPasswordUnlock, MasterPasswordAuthentication, /// and control flags ValidatePassword and RefreshStamp. /// /// /// On success, the modified . On failure, an array of /// describing validation failures. /// - Task> SaveUpdateExistingMasterPasswordAndKdfAsync(User user, UpdateExistingPasswordAndKdfData updateExistingData); + Task> SaveUpdateExistingKdfConfigurationAsync(User user, UpdateExistingKdfConfigurationData updateExistingData); /// /// Applies a new master password over the user's existing one and persists the updated user diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 12d3cb228263..cc8a1c94b0a7 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -171,9 +171,9 @@ public async Task> PrepareUpdateExistingMasterPassw return user; } - public async Task> SaveUpdateExistingMasterPasswordAndKdfAsync( + public async Task> SaveUpdateExistingKdfConfigurationAsync( User user, - UpdateExistingPasswordAndKdfData updateExistingData) + UpdateExistingKdfConfigurationData updateExistingData) { EnsureUserIsHydrated(user); updateExistingData.ValidateDataForUser(user); @@ -199,9 +199,10 @@ public async Task> SaveUpdateExistingMasterPassword // Always override the master password hint, even if it's null user.MasterPasswordHint = updateExistingData.MasterPasswordHint; - // Update time markers on the user + // Update time markers on the user. + // LastPasswordChangeDate is intentionally not set: KDF rotation re-derives the authentication + // hash from the same password using new KDF parameters — the user's password has not changed. var now = _timeProvider.GetUtcNow().UtcDateTime; - user.LastPasswordChangeDate = now; user.LastKdfChangeDate = now; user.RevisionDate = user.AccountRevisionDate = now; diff --git a/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs b/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs index 17cb20723ec3..000bc5a347a7 100644 --- a/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs +++ b/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs @@ -1,13 +1,13 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Platform.Push; -using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Utilities; using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.Logging; namespace Bit.Core.KeyManagement.Kdf.Implementations; @@ -16,20 +16,18 @@ public class ChangeKdfCommand : IChangeKdfCommand { private readonly IUserService _userService; private readonly IPushNotificationService _pushService; - private readonly IUserRepository _userRepository; + private readonly IMasterPasswordService _masterPasswordService; private readonly IdentityErrorDescriber _identityErrorDescriber; - private readonly ILogger _logger; private readonly IFeatureService _featureService; public ChangeKdfCommand(IUserService userService, IPushNotificationService pushService, - IUserRepository userRepository, IdentityErrorDescriber describer, ILogger logger, + IMasterPasswordService masterPasswordService, IdentityErrorDescriber describer, IFeatureService featureService) { _userService = userService; _pushService = pushService; - _userRepository = userRepository; + _masterPasswordService = masterPasswordService; _identityErrorDescriber = describer; - _logger = logger; _featureService = featureService; } @@ -44,7 +42,8 @@ public async Task ChangeKdfAsync(User user, string masterPasswor // Validate to prevent user account from becoming un-decryptable from invalid parameters // - // Prevent a de-synced salt value from creating an un-decryptable unlock method + // Prevent a de-synced salt value from creating an un-decryptable unlock method. + // Also checked in the MasterPasswordService via UpdateExistingKdfConfigurationData.ValidateDataForUser. authenticationData.ValidateSaltUnchangedForUser(user); unlockData.ValidateSaltUnchangedForUser(user); @@ -62,42 +61,21 @@ public async Task ChangeKdfAsync(User user, string masterPasswor var logoutOnKdfChange = !_featureService.IsEnabled(FeatureFlagKeys.NoLogoutOnKdfChange); - // Update the user with the new KDF settings - // This updates the authentication data and unlock data for the user separately. Currently these still - // use shared values for KDF settings and salt. - // The authentication hash, and the unlock data each are dependent on: - // - The master password (entered by the user every time) - // - The KDF settings (iterations, memory, parallelism) - // - The salt - // These combinations - (password, authentication hash, KDF settings, salt) and (password, unlock data, KDF settings, salt) - // must remain consistent to unlock correctly. + var data = new UpdateExistingKdfConfigurationData + { + MasterPasswordAuthentication = authenticationData, + MasterPasswordUnlock = unlockData, + ValidatePassword = true, + RefreshStamp = logoutOnKdfChange, + MasterPasswordHint = user.MasterPasswordHint, // KDF rotation does not change the hint; carry existing value through + }; - // Authentication - // Note: This mutates the user but does not yet save it to DB. That is done atomically, later. - // This entire operation MUST be atomic to prevent a user from being locked out of their account. - // Salt is ensured to be the same as unlock data, and the value stored in the account and not updated. - // KDF is ensured to be the same as unlock data above and updated below. - var result = await _userService.UpdatePasswordHash(user, authenticationData.MasterPasswordAuthenticationHash, - refreshStamp: logoutOnKdfChange); - if (!result.Succeeded) + var result = await _masterPasswordService.SaveUpdateExistingKdfConfigurationAsync(user, data); + if (result.TryPickT1(out var errors, out _)) { - _logger.LogWarning("Change KDF failed for user {userId}.", user.Id); - return result; + return IdentityResult.Failed(errors); } - // Salt is ensured to be the same as authentication data, and the value stored in the account, and is not updated. - // Kdf - These will be seperated in the future, but for now are ensured to be the same as authentication data above. - user.Key = unlockData.MasterKeyWrappedUserKey; - user.Kdf = unlockData.Kdf.KdfType; - user.KdfIterations = unlockData.Kdf.Iterations; - user.KdfMemory = unlockData.Kdf.Memory; - user.KdfParallelism = unlockData.Kdf.Parallelism; - - var now = DateTime.UtcNow; - user.RevisionDate = user.AccountRevisionDate = now; - user.LastKdfChangeDate = now; - - await _userRepository.ReplaceAsync(user); if (logoutOnKdfChange) { await _pushService.PushLogOutAsync(user.Id); diff --git a/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs b/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs index bace8350cd7f..fd858983f1b6 100644 --- a/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs +++ b/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs @@ -239,6 +239,31 @@ public async Task PostKdf_Unauthorized_ReturnsUnauthorized() Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); } + [Theory] + [InlineData("authenticationData")] + [InlineData("unlockData")] + [InlineData("masterPasswordHash")] + public async Task PostKdf_MissingRequiredJsonKey_BadRequest(string fieldToOmit) + { + await _loginHelper.LoginAsync(_ownerEmail); + + var payload = new Dictionary + { + ["masterPasswordHash"] = _masterPasswordHash, + ["authenticationData"] = BuildAuthData(_defaultKdfRequest, _ownerEmail), + ["unlockData"] = BuildUnlockData(_defaultKdfRequest, _ownerEmail) + }; + payload.Remove(fieldToOmit); + + using var message = new HttpRequestMessage(HttpMethod.Post, "/accounts/kdf"); + message.Content = JsonContent.Create(payload); + var response = await _client.SendAsync(message); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains($"missing required properties including: '{fieldToOmit}'", content); + } + [Theory] [InlineData(false, true)] [InlineData(true, false)] @@ -260,7 +285,14 @@ public async Task PostKdf_AuthenticationDataOrUnlockDataNull_BadRequest(bool aut Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("AuthenticationData and UnlockData must be provided.", content); + if (authenticationDataNull) + { + Assert.Contains("The AuthenticationData field is required.", content); + } + if (unlockDataNull) + { + Assert.Contains("The UnlockData field is required.", content); + } } [Fact] diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index ba3760a2baf0..db07a1bc4d8c 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -696,34 +696,6 @@ public async Task PostKdf_UserNotFound_ShouldFail(ChangeKdfRequestModel model) await Assert.ThrowsAsync(() => _sut.PostKdf(model)); } - [Theory] - [BitAutoData] - public async Task PostKdf_WithNullAuthenticationData_ShouldFail( - User user, ChangeKdfRequestModel model) - { - _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); - model.AuthenticationData = null; - - // Act - var exception = await Assert.ThrowsAsync(() => _sut.PostKdf(model)); - - Assert.Contains("AuthenticationData and UnlockData must be provided.", exception.Message); - } - - [Theory] - [BitAutoData] - public async Task PostKdf_WithNullUnlockData_ShouldFail( - User user, ChangeKdfRequestModel model) - { - _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); - model.UnlockData = null; - - // Act - var exception = await Assert.ThrowsAsync(() => _sut.PostKdf(model)); - - Assert.Contains("AuthenticationData and UnlockData must be provided.", exception.Message); - } - [Theory] [BitAutoData] public async Task PostKdf_ChangeKdfFailed_ShouldFail( diff --git a/test/Api.Test/Auth/Models/Request/Accounts/ChangeKdfRequestModelTests.cs b/test/Api.Test/Auth/Models/Request/Accounts/ChangeKdfRequestModelTests.cs index 91b43b4d21bb..67d56577d06e 100644 --- a/test/Api.Test/Auth/Models/Request/Accounts/ChangeKdfRequestModelTests.cs +++ b/test/Api.Test/Auth/Models/Request/Accounts/ChangeKdfRequestModelTests.cs @@ -12,7 +12,7 @@ public class ChangeKdfRequestModelTests [Theory] [BitAutoData(KdfType.PBKDF2_SHA256, 600000, null, null)] [BitAutoData(KdfType.Argon2id, 3, 64, 4)] - public void Validate_NewPayloadsOnly_NoErrors( + public void Validate_SubmitWithAuthnAndUnlockData_NoErrors( KdfType kdfType, int iterations, int? memory, int? parallelism) { var kdf = new KdfRequestModel @@ -46,46 +46,35 @@ public void Validate_NewPayloadsOnly_NoErrors( } [Theory] - [BitAutoData] - public void Validate_NewPayloadsOnly_WithMismatchedKdfSettings_ReturnsKdfValidationError( + [BitAutoData(true, false)] + [BitAutoData(false, true)] + [BitAutoData(true, true)] + public void Validate_EitherFieldNull_ReturnsNoErrors(bool authNull, bool unlockNull, string masterPasswordHash) { - var authKdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600000 }; - var unlockKdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 650000 }; - - var model = new ChangeKdfRequestModel - { - MasterPasswordHash = masterPasswordHash, - AuthenticationData = new MasterPasswordAuthenticationDataRequestModel - { - Kdf = authKdf, - MasterPasswordAuthenticationHash = "authHash", - Salt = "salt" - }, - UnlockData = new MasterPasswordUnlockDataRequestModel - { - Kdf = unlockKdf, - MasterKeyWrappedUserKey = "wrappedKey", - Salt = "salt" - } - }; - - var result = model.Validate(new ValidationContext(model)).ToList(); - - Assert.Single(result); - Assert.Contains("must have the same KDF configuration", result[0].ErrorMessage); - } + // Defensive guard: [Required] reports null-field errors via ModelState, so Validate() + // must not throw or report when either cross-field input is null. + var kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600000 }; - [Theory] - [BitAutoData] - public void Validate_LegacyPayloadsOnly_NoErrors( - string masterPasswordHash, string newHash, string key) - { var model = new ChangeKdfRequestModel { MasterPasswordHash = masterPasswordHash, - NewMasterPasswordHash = newHash, - Key = key + AuthenticationData = authNull + ? null + : new MasterPasswordAuthenticationDataRequestModel + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "authHash", + Salt = "salt" + }, + UnlockData = unlockNull + ? null + : new MasterPasswordUnlockDataRequestModel + { + Kdf = kdf, + MasterKeyWrappedUserKey = "wrappedKey", + Salt = "salt" + } }; var result = model.Validate(new ValidationContext(model)).ToList(); @@ -95,62 +84,24 @@ public void Validate_LegacyPayloadsOnly_NoErrors( [Theory] [BitAutoData] - public void Validate_BothNewAndLegacyPayloads_NoErrors( - string masterPasswordHash, string newHash, string key) + public void Validate_WithMismatchedKdfSettings_ReturnsKdfValidationError( + string masterPasswordHash) { - var kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600000 }; + var authKdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600000 }; + var unlockKdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 650000 }; var model = new ChangeKdfRequestModel { MasterPasswordHash = masterPasswordHash, - NewMasterPasswordHash = newHash, - Key = key, AuthenticationData = new MasterPasswordAuthenticationDataRequestModel { - Kdf = kdf, + Kdf = authKdf, MasterPasswordAuthenticationHash = "authHash", Salt = "salt" }, UnlockData = new MasterPasswordUnlockDataRequestModel { - Kdf = kdf, - MasterKeyWrappedUserKey = "wrappedKey", - Salt = "salt" - } - }; - - var result = model.Validate(new ValidationContext(model)).ToList(); - - Assert.Empty(result); - } - - [Theory] - [BitAutoData] - public void Validate_NeitherNewNorLegacyPayloads_ReturnsError(string masterPasswordHash) - { - var model = new ChangeKdfRequestModel - { - MasterPasswordHash = masterPasswordHash - }; - - var result = model.Validate(new ValidationContext(model)).ToList(); - - Assert.Single(result); - Assert.Contains("Must provide either", result[0].ErrorMessage); - } - - [Theory] - [BitAutoData] - public void Validate_OnlyUnlockData_ReturnsError(string masterPasswordHash) - { - var kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600000 }; - - var model = new ChangeKdfRequestModel - { - MasterPasswordHash = masterPasswordHash, - UnlockData = new MasterPasswordUnlockDataRequestModel - { - Kdf = kdf, + Kdf = unlockKdf, MasterKeyWrappedUserKey = "wrappedKey", Salt = "salt" } @@ -159,29 +110,6 @@ public void Validate_OnlyUnlockData_ReturnsError(string masterPasswordHash) var result = model.Validate(new ValidationContext(model)).ToList(); Assert.Single(result); - Assert.Contains("Must provide either", result[0].ErrorMessage); - } - - [Theory] - [BitAutoData] - public void Validate_OnlyAuthenticationData_ReturnsError(string masterPasswordHash) - { - var kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = 600000 }; - - var model = new ChangeKdfRequestModel - { - MasterPasswordHash = masterPasswordHash, - AuthenticationData = new MasterPasswordAuthenticationDataRequestModel - { - Kdf = kdf, - MasterPasswordAuthenticationHash = "authHash", - Salt = "salt" - } - }; - - var result = model.Validate(new ValidationContext(model)).ToList(); - - Assert.Single(result); - Assert.Contains("Must provide either", result[0].ErrorMessage); + Assert.Contains("must have the same KDF configuration", result[0].ErrorMessage); } } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingKdfConfigurationDataTests.cs similarity index 93% rename from test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs rename to test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingKdfConfigurationDataTests.cs index 40c8e3946a24..0d4ba54073de 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingKdfConfigurationDataTests.cs @@ -7,7 +7,7 @@ namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword.Data; -public class UpdateExistingPasswordAndKdfDataTests +public class UpdateExistingKdfConfigurationDataTests { private static User BuildValidUser() { @@ -24,7 +24,7 @@ private static User BuildValidUser() }; } - private static UpdateExistingPasswordAndKdfData BuildData(User user, string? saltOverride = null) + private static UpdateExistingKdfConfigurationData BuildData(User user, string? saltOverride = null) { var salt = saltOverride ?? user.GetMasterPasswordSalt(); var newKdf = new KdfSettings @@ -34,7 +34,7 @@ private static UpdateExistingPasswordAndKdfData BuildData(User user, string? sal Memory = 64, Parallelism = 4 }; - return new UpdateExistingPasswordAndKdfData + return new UpdateExistingKdfConfigurationData { MasterPasswordUnlock = new MasterPasswordUnlockData { @@ -100,7 +100,7 @@ public void ValidateDataForUser_Throws_WhenSaltMismatch_ValidatesBothFieldsIndep var user = BuildValidUser(); var correctSalt = user.GetMasterPasswordSalt(); var newKdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 3, Memory = 64, Parallelism = 4 }; - var data = new UpdateExistingPasswordAndKdfData + var data = new UpdateExistingKdfConfigurationData { MasterPasswordUnlock = new MasterPasswordUnlockData { diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index 121751efc274..43f28a22c475 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -106,7 +106,7 @@ private static UpdateExistingPasswordData BuildUpdateExistingPasswordData(User u }; } - private static UpdateExistingPasswordAndKdfData BuildUpdateExistingPasswordAndKdfData(User user, + private static UpdateExistingKdfConfigurationData BuildUpdateExistingKdfConfigurationData(User user, KdfSettings? newKdf = null, string? hint = null, bool validatePassword = false, bool refreshStamp = false) { var salt = user.GetMasterPasswordSalt(); @@ -117,7 +117,7 @@ private static UpdateExistingPasswordAndKdfData BuildUpdateExistingPasswordAndKd Memory = 64, Parallelism = 4 }; - return new UpdateExistingPasswordAndKdfData + return new UpdateExistingKdfConfigurationData { MasterPasswordUnlock = new MasterPasswordUnlockData { @@ -759,21 +759,22 @@ await Assert.ThrowsAsync( () => sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data)); } - // --- SaveUpdateExistingMasterPasswordAndKdf --- + // --- SaveUpdateExistingKdfConfiguration --- [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_Success(User user) + public async Task SaveUpdateExistingKdfConfiguration_Success(User user) { var sutProvider = CreateSutProvider(); user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingPasswordAndKdfData(user, hint: "test-hint"); + var data = BuildUpdateExistingKdfConfigurationData(user, hint: "test-hint"); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); + var originalLastPasswordChangeDate = user.LastPasswordChangeDate; - var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + var result = await sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data); var expectedTime = sutProvider.GetDependency().GetUtcNow().UtcDateTime; @@ -782,15 +783,20 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_Success(User user) Assert.Equal(data.MasterPasswordUnlock.Kdf.KdfType, user.Kdf); Assert.Equal(data.MasterPasswordUnlock.Kdf.Iterations, user.KdfIterations); Assert.Equal("test-hint", user.MasterPasswordHint); - Assert.Equal(expectedTime, user.LastPasswordChangeDate); Assert.Equal(expectedTime, user.LastKdfChangeDate); + // LastPasswordChangeDate marks a user's action to change the password; + // the fact that the hash-of-hash which is stored in the database changes + // as a result of KDF update does not affect a change to this date; + // doing so would confuse the data. LastKdfChangeDate neatly separates + // this concern. + Assert.Equal(originalLastPasswordChangeDate, user.LastPasswordChangeDate); Assert.Equal(expectedTime, user.RevisionDate); Assert.Equal(user.RevisionDate, user.AccountRevisionDate); await sutProvider.GetDependency().Received().ReplaceAsync(user); } [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesPbkdf2ToArgon2id(User user) + public async Task SaveUpdateExistingKdfConfiguration_RotatesPbkdf2ToArgon2id(User user) { var sutProvider = CreateSutProvider(); user.MasterPassword = "existing-hash"; @@ -807,12 +813,12 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesPbkdf2ToArgon2id Memory = 64, Parallelism = 4 }; - var data = BuildUpdateExistingPasswordAndKdfData(user, newKdf: newKdf); + var data = BuildUpdateExistingKdfConfigurationData(user, newKdf: newKdf); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); - var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + var result = await sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data); Assert.True(result.IsT0); Assert.Equal(KdfType.Argon2id, user.Kdf); @@ -822,7 +828,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesPbkdf2ToArgon2id } [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesArgon2idToPbkdf2(User user) + public async Task SaveUpdateExistingKdfConfiguration_RotatesArgon2idToPbkdf2(User user) { var sutProvider = CreateSutProvider(); user.MasterPassword = "existing-hash"; @@ -839,12 +845,12 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesArgon2idToPbkdf2 Memory = null, Parallelism = null }; - var data = BuildUpdateExistingPasswordAndKdfData(user, newKdf: newKdf); + var data = BuildUpdateExistingKdfConfigurationData(user, newKdf: newKdf); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); - var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + var result = await sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data); Assert.True(result.IsT0); Assert.Equal(KdfType.PBKDF2_SHA256, user.Kdf); @@ -854,7 +860,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesArgon2idToPbkdf2 } [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_WhenValidationFails_ReturnsErrorsAndDoesNotPersist(User user) + public async Task SaveUpdateExistingKdfConfiguration_WhenValidationFails_ReturnsErrorsAndDoesNotPersist(User user) { var error = new IdentityError { Code = "pwd-invalid", Description = "Password is too weak." }; var validator = Substitute.For>(); @@ -866,9 +872,9 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_WhenValidationFails_Ret user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingPasswordAndKdfData(user, validatePassword: true); + var data = BuildUpdateExistingKdfConfigurationData(user, validatePassword: true); - var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + var result = await sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data); Assert.True(result.IsT1); Assert.NotEmpty(result.AsT1); @@ -876,7 +882,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_WhenValidationFails_Ret } [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenSaltChanged(User user) + public async Task SaveUpdateExistingKdfConfiguration_ThrowsWhenSaltChanged(User user) { var sutProvider = CreateSutProvider(); user.MasterPassword = "existing-hash"; @@ -890,7 +896,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenSaltChanged(U Memory = 64, Parallelism = 4 }; - var data = new UpdateExistingPasswordAndKdfData + var data = new UpdateExistingKdfConfigurationData { MasterPasswordUnlock = new MasterPasswordUnlockData { @@ -909,51 +915,51 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenSaltChanged(U }; await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + () => sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data)); } [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenNoExistingPassword(User user) + public async Task SaveUpdateExistingKdfConfiguration_ThrowsWhenNoExistingPassword(User user) { var sutProvider = CreateSutProvider(); user.MasterPassword = null; - var data = BuildUpdateExistingPasswordAndKdfData(user); + var data = BuildUpdateExistingKdfConfigurationData(user); await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + () => sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data)); } [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenUserNotHydrated(User user) + public async Task SaveUpdateExistingKdfConfiguration_ThrowsWhenUserNotHydrated(User user) { var sutProvider = CreateSutProvider(); user.Id = default; user.MasterPassword = "existing-hash"; - var data = BuildUpdateExistingPasswordAndKdfData(user); + var data = BuildUpdateExistingKdfConfigurationData(user); await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + () => sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data)); } [Theory, BitAutoData] - public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsForKeyConnectorUser(User user) + public async Task SaveUpdateExistingKdfConfiguration_ThrowsForKeyConnectorUser(User user) { var sutProvider = CreateSutProvider(); user.MasterPassword = "existing-hash"; user.UsesKeyConnector = true; - var data = BuildUpdateExistingPasswordAndKdfData(user); + var data = BuildUpdateExistingKdfConfigurationData(user); await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + () => sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data)); } [Theory] [InlineData(true)] [InlineData(false)] - public async Task SaveUpdateExistingMasterPasswordAndKdf_SecurityStampRotation_HonorsRefreshStampFlag(bool refreshStamp) + public async Task SaveUpdateExistingKdfConfiguration_SecurityStampRotation_HonorsRefreshStampFlag(bool refreshStamp) { var sutProvider = CreateSutProvider(); var user = new User @@ -967,12 +973,12 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_SecurityStampRotation_H KdfIterations = 600000, SecurityStamp = "original-stamp" }; - var data = BuildUpdateExistingPasswordAndKdfData(user, refreshStamp: refreshStamp); + var data = BuildUpdateExistingKdfConfigurationData(user, refreshStamp: refreshStamp); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("hash"); - await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + await sutProvider.Sut.SaveUpdateExistingKdfConfigurationAsync(user, data); Assert.Equal(refreshStamp, user.SecurityStamp != "original-stamp"); } diff --git a/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs b/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs index 991935b92896..099beb774648 100644 --- a/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs +++ b/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs @@ -1,17 +1,19 @@ #nullable enable +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.KeyManagement.Kdf.Implementations; using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Platform.Push; -using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Identity; using NSubstitute; +using OneOf; using Xunit; namespace Bit.Core.Test.KeyManagement.Kdf; @@ -21,14 +23,69 @@ public class ChangeKdfCommandTests { [Theory] [BitAutoData] - public async Task ChangeKdfAsync_ChangesKdfAsync(SutProvider sutProvider, User user) + public async Task ChangeKdfAsync_ServiceReturnsUser_ReturnsIdentityResultSuccess( + SutProvider sutProvider, User user) { - sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); - sutProvider.GetDependency().UpdatePasswordHash(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(IdentityResult.Success)); + var kdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "auth-hash", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "wrapped-key", + Salt = user.GetMasterPasswordSalt() + }; + sutProvider.GetDependency().CheckPasswordAsync(user, Arg.Any()) + .Returns(true); + sutProvider.GetDependency() + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Any()) + .Returns(OneOf.FromT0(user)); + + var result = await sutProvider.Sut.ChangeKdfAsync(user, "current-password", authenticationData, unlockData); + + Assert.True(result.Succeeded); + } + + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_ServiceReturnsErrors_ReturnsIdentityResultFailed( + SutProvider sutProvider, User user) + { + var kdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; + var authenticationData = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = "auth-hash", + Salt = user.GetMasterPasswordSalt() + }; + var unlockData = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = "wrapped-key", + Salt = user.GetMasterPasswordSalt() + }; + var serviceError = new IdentityError { Code = "SomeError", Description = "Service error" }; + sutProvider.GetDependency().CheckPasswordAsync(user, Arg.Any()) + .Returns(true); + sutProvider.GetDependency() + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Any()) + .Returns(OneOf.FromT1(new[] { serviceError })); + + var result = await sutProvider.Sut.ChangeKdfAsync(user, "current-password", authenticationData, unlockData); + + Assert.False(result.Succeeded); + Assert.Contains(result.Errors, e => e.Code == "SomeError"); + } - var kdf = new KdfSettings { KdfType = Enums.KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; + [Theory] + [BitAutoData] + public async Task ChangeKdfAsync_ChangesKdfAsync(SutProvider sutProvider, User user) + { + var kdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; var authenticationData = new MasterPasswordAuthenticationData { Kdf = kdf, @@ -41,23 +98,27 @@ public async Task ChangeKdfAsync_ChangesKdfAsync(SutProvider s MasterKeyWrappedUserKey = "masterKeyWrappedUserKey", Salt = user.GetMasterPasswordSalt() }; + sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) + .Returns(true); + sutProvider.GetDependency() + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Any()) + .Returns(OneOf.FromT0(user)); await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); - await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => - u.Id == user.Id - && u.Kdf == Enums.KdfType.Argon2id - && u.KdfIterations == 4 - && u.KdfMemory == 512 - && u.KdfParallelism == 4 - )); + await sutProvider.GetDependency().Received(1) + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Is(d => + d.MasterPasswordAuthentication == authenticationData && + d.MasterPasswordUnlock == unlockData && + d.ValidatePassword == true && + d.MasterPasswordHint == user.MasterPasswordHint)); } [Theory] [BitAutoData] public async Task ChangeKdfAsync_UserIsNull_ThrowsArgumentNullException(SutProvider sutProvider) { - var kdf = new KdfSettings { KdfType = Enums.KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; + var kdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; var authenticationData = new MasterPasswordAuthenticationData { Kdf = kdf, @@ -83,7 +144,7 @@ public async Task ChangeKdfAsync_WrongPassword_ReturnsPasswordMismatch(SutProvid sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(false)); - var kdf = new KdfSettings { KdfType = Enums.KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; + var kdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; var authenticationData = new MasterPasswordAuthenticationData { Kdf = kdf, @@ -110,7 +171,7 @@ public async Task { var constantKdf = new KdfSettings { - KdfType = Enums.KdfType.Argon2id, + KdfType = KdfType.Argon2id, Iterations = 5, Memory = 1024, Parallelism = 4 @@ -128,24 +189,17 @@ public async Task Salt = user.GetMasterPasswordSalt() }; sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); - sutProvider.GetDependency() - .UpdatePasswordHash(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(IdentityResult.Success)); + .Returns(true); sutProvider.GetDependency().IsEnabled(Arg.Any()).Returns(false); + sutProvider.GetDependency() + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Any()) + .Returns(OneOf.FromT0(user)); await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); - await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => - u.Id == user.Id - && u.Kdf == constantKdf.KdfType - && u.KdfIterations == constantKdf.Iterations - && u.KdfMemory == constantKdf.Memory - && u.KdfParallelism == constantKdf.Parallelism - && u.Key == "new-wrapped-key" - )); - await sutProvider.GetDependency().Received(1).UpdatePasswordHash(user, - authenticationData.MasterPasswordAuthenticationHash, validatePassword: true, refreshStamp: true); + await sutProvider.GetDependency().Received(1) + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Is(d => + d.RefreshStamp == true)); await sutProvider.GetDependency().Received(1).PushLogOutAsync(user.Id); sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.NoLogoutOnKdfChange); } @@ -158,7 +212,7 @@ public async Task { var constantKdf = new KdfSettings { - KdfType = Enums.KdfType.Argon2id, + KdfType = KdfType.Argon2id, Iterations = 5, Memory = 1024, Parallelism = 4 @@ -176,24 +230,17 @@ public async Task Salt = user.GetMasterPasswordSalt() }; sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); - sutProvider.GetDependency() - .UpdatePasswordHash(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(IdentityResult.Success)); + .Returns(true); sutProvider.GetDependency().IsEnabled(Arg.Any()).Returns(true); + sutProvider.GetDependency() + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Any()) + .Returns(OneOf.FromT0(user)); await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); - await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => - u.Id == user.Id - && u.Kdf == constantKdf.KdfType - && u.KdfIterations == constantKdf.Iterations - && u.KdfMemory == constantKdf.Memory - && u.KdfParallelism == constantKdf.Parallelism - && u.Key == "new-wrapped-key" - )); - await sutProvider.GetDependency().Received(1).UpdatePasswordHash(user, - authenticationData.MasterPasswordAuthenticationHash, validatePassword: true, refreshStamp: false); + await sutProvider.GetDependency().Received(1) + .SaveUpdateExistingKdfConfigurationAsync(user, Arg.Is(d => + d.RefreshStamp == false)); await sutProvider.GetDependency().Received(1) .PushLogOutAsync(user.Id, false, PushNotificationLogOutReason.KdfChange); await sutProvider.GetDependency().Received(1).PushSyncSettingsAsync(user.Id); @@ -212,7 +259,7 @@ public async Task ChangeKdfAsync_KdfNotEqualBetweenAuthAndUnlock_ThrowsBadReques { Kdf = new KdfSettings { - KdfType = Enums.KdfType.Argon2id, + KdfType = KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 @@ -222,7 +269,7 @@ public async Task ChangeKdfAsync_KdfNotEqualBetweenAuthAndUnlock_ThrowsBadReques }; var unlockData = new MasterPasswordUnlockData { - Kdf = new KdfSettings { KdfType = Enums.KdfType.PBKDF2_SHA256, Iterations = 100000 }, + Kdf = new KdfSettings { KdfType = KdfType.PBKDF2_SHA256, Iterations = 100000 }, MasterKeyWrappedUserKey = "new-wrapped-key", Salt = user.GetMasterPasswordSalt() }; @@ -236,7 +283,7 @@ public async Task ChangeKdfAsync_AuthDataSaltMismatch_Throws(SutProvider().CheckPasswordAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(true); var authenticationData = new MasterPasswordAuthenticationData { @@ -252,6 +299,9 @@ public async Task ChangeKdfAsync_AuthDataSaltMismatch_Throws(SutProvider(async () => await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData)); + await sutProvider.GetDependency() + .Received(0) + .SaveUpdateExistingKdfConfigurationAsync(Arg.Any(), Arg.Any()); } [Theory] @@ -260,7 +310,7 @@ public async Task ChangeKdfAsync_UnlockDataSaltMismatch_Throws(SutProvider().CheckPasswordAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); + .Returns(true); var authenticationData = new MasterPasswordAuthenticationData { @@ -276,36 +326,9 @@ public async Task ChangeKdfAsync_UnlockDataSaltMismatch_Throws(SutProvider(async () => await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData)); - } - - [Theory] - [BitAutoData] - public async Task ChangeKdfAsync_UpdatePasswordHashFails_ReturnsFailure(SutProvider sutProvider, - User user) - { - sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(true)); - var failedResult = IdentityResult.Failed(new IdentityError { Code = "TestFail", Description = "Test fail" }); - sutProvider.GetDependency().UpdatePasswordHash(Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(failedResult)); - - var kdf = new KdfSettings { KdfType = Enums.KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; - var authenticationData = new MasterPasswordAuthenticationData - { - Kdf = kdf, - MasterPasswordAuthenticationHash = "newMasterPassword", - Salt = user.GetMasterPasswordSalt() - }; - var unlockData = new MasterPasswordUnlockData - { - Kdf = kdf, - MasterKeyWrappedUserKey = "masterKeyWrappedUserKey", - Salt = user.GetMasterPasswordSalt() - }; - - var result = await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); - - Assert.False(result.Succeeded); + await sutProvider.GetDependency() + .Received(0) + .SaveUpdateExistingKdfConfigurationAsync(Arg.Any(), Arg.Any()); } [Theory] @@ -316,10 +339,9 @@ public async Task ChangeKdfAsync_InvalidKdfSettings_ThrowsBadRequestException( sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); - // Create invalid KDF settings (iterations too low for PBKDF2) var invalidKdf = new KdfSettings { - KdfType = Enums.KdfType.PBKDF2_SHA256, + KdfType = KdfType.PBKDF2_SHA256, Iterations = 1000, // This is below the minimum of 600,000 Memory = null, Parallelism = null @@ -355,7 +377,7 @@ public async Task ChangeKdfAsync_InvalidArgon2Settings_ThrowsBadRequestException // Create invalid Argon2 KDF settings (memory too high) var invalidKdf = new KdfSettings { - KdfType = Enums.KdfType.Argon2id, + KdfType = KdfType.Argon2id, Iterations = 3, // Valid Memory = 2048, // This is above the maximum of 1024 Parallelism = 4 // Valid