[PM-35395] MasterPasswordService Key Management Integration

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)
         {

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;
 
 /// <summary>
-/// Dual-shape request: validation accepts either the legacy
-/// (<see cref="NewMasterPasswordHash"/>, <see cref="Key"/>) or new
-/// (<see cref="AuthenticationData"/>, <see cref="UnlockData"/>) payload so the wire contract
-/// can stabilize ahead of caller wiring. <c>PostKdf</c> currently honors only the new shape;
-/// legacy-shape dispatch arrives with <c>ChangeKdfCommand</c>'s dual-path refactor. All legacy
-/// fields are removed in PM-33141.
+/// Request model for changing the KDF settings for a user's account.
 /// </summary>
 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<ValidationResult> 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;
         }
     }
 }

src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs → src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingKdfConfigurationData.cs

@@ -5,16 +5,16 @@
 namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data;
 
 /// <summary>
-/// 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.
 ///
 /// <para>
 /// Use when: constructing a call to
-/// <see cref="Interfaces.IMasterPasswordService.SaveUpdateExistingMasterPasswordAndKdfAsync"/>
+/// <see cref="Interfaces.IMasterPasswordService.SaveUpdateExistingKdfConfigurationAsync"/>
 /// (KDF rotation flows). For hash-only updates, use <see cref="UpdateExistingPasswordData"/> instead.
 /// </para>
 /// </summary>
-public class UpdateExistingPasswordAndKdfData
+public class UpdateExistingKdfConfigurationData
 {
     public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; }
     public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; }

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
 /// <see cref="Interfaces.IMasterPasswordService.PrepareUpdateExistingMasterPasswordAsync"/> or
 /// <see cref="Interfaces.IMasterPasswordService.SaveUpdateExistingMasterPasswordAsync"/>.
-/// For KDF rotation, use <see cref="UpdateExistingPasswordAndKdfData"/> instead.
+/// For KDF rotation, use <see cref="UpdateExistingKdfConfigurationData"/> instead.
 /// </para>
 /// </summary>
 public class UpdateExistingPasswordData

src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs

@@ -255,11 +255,12 @@ public interface IMasterPasswordService
     Task<OneOf<User, IdentityError[]>> PrepareUpdateExistingMasterPasswordAsync(User user, UpdateExistingPasswordData updateExistingData);
 
     /// <summary>
-    /// 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.
     ///
     /// <para>
-    /// Use when: rotating KDF parameters.
+    /// Use when: updating KDF parameters.
     /// </para>
     ///
     /// <para>
@@ -275,18 +276,18 @@ public interface IMasterPasswordService
     /// <param name="user">
     /// 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
-    /// <see cref="UpdateExistingPasswordAndKdfData.ValidateDataForUser"/>.
+    /// <see cref="UpdateExistingKdfConfigurationData.ValidateDataForUser"/>.
     /// </param>
     /// <param name="updateExistingData">
-    /// Cryptographic and authentication data for the updated password and KDF parameters,
-    /// including <c>MasterPasswordUnlock</c>, <c>MasterPasswordAuthentication</c>,
+    /// Cryptographic and authentication data for KDF configuration, including
+    /// <c>MasterPasswordUnlock</c>, <c>MasterPasswordAuthentication</c>,
     /// and control flags <c>ValidatePassword</c> and <c>RefreshStamp</c>.
     /// </param>
     /// <returns>
     /// On success, the modified <see cref="User"/>. On failure, an array of
     /// <see cref="IdentityError"/> describing validation failures.
     /// </returns>
-    Task<OneOf<User, IdentityError[]>> SaveUpdateExistingMasterPasswordAndKdfAsync(User user, UpdateExistingPasswordAndKdfData updateExistingData);
+    Task<OneOf<User, IdentityError[]>> SaveUpdateExistingKdfConfigurationAsync(User user, UpdateExistingKdfConfigurationData updateExistingData);
 
     /// <summary>
     /// Applies a new master password over the user's existing one and persists the updated user

src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs

@@ -171,9 +171,9 @@ public async Task<OneOf<User, IdentityError[]>> PrepareUpdateExistingMasterPassw
         return user;
     }
 
-    public async Task<OneOf<User, IdentityError[]>> SaveUpdateExistingMasterPasswordAndKdfAsync(
+    public async Task<OneOf<User, IdentityError[]>> SaveUpdateExistingKdfConfigurationAsync(
         User user,
-        UpdateExistingPasswordAndKdfData updateExistingData)
+        UpdateExistingKdfConfigurationData updateExistingData)
     {
         EnsureUserIsHydrated(user);
         updateExistingData.ValidateDataForUser(user);
@@ -199,9 +199,10 @@ public async Task<OneOf<User, IdentityError[]>> 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;
 

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<ChangeKdfCommand> _logger;
     private readonly IFeatureService _featureService;
 
     public ChangeKdfCommand(IUserService userService, IPushNotificationService pushService,
-        IUserRepository userRepository, IdentityErrorDescriber describer, ILogger<ChangeKdfCommand> 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<IdentityResult> 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<IdentityResult> 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);

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<string, object?>
+        {
+            ["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]

test/Api.Test/Auth/Controllers/AccountsControllerTests.cs

@@ -696,34 +696,6 @@ public async Task PostKdf_UserNotFound_ShouldFail(ChangeKdfRequestModel model)
         await Assert.ThrowsAsync<UnauthorizedAccessException>(() => _sut.PostKdf(model));
     }
 
-    [Theory]
-    [BitAutoData]
-    public async Task PostKdf_WithNullAuthenticationData_ShouldFail(
-        User user, ChangeKdfRequestModel model)
-    {
-        _userService.GetUserByPrincipalAsync(Arg.Any<ClaimsPrincipal>()).Returns(Task.FromResult(user));
-        model.AuthenticationData = null;
-
-        // Act
-        var exception = await Assert.ThrowsAsync<BadRequestException>(() => _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<ClaimsPrincipal>()).Returns(Task.FromResult(user));
-        model.UnlockData = null;
-
-        // Act
-        var exception = await Assert.ThrowsAsync<BadRequestException>(() => _sut.PostKdf(model));
-
-        Assert.Contains("AuthenticationData and UnlockData must be provided.", exception.Message);
-    }
-
     [Theory]
     [BitAutoData]
     public async Task PostKdf_ChangeKdfFailed_ShouldFail(