From 6c4c7208314174568cd5d0e4bdba747da2d69a51 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 26 May 2026 13:39:12 -0700 Subject: [PATCH 1/7] add v1 path that can receive new types: Auth, Unlock, AccountKeys --- .../Auth/Controllers/AccountsController.cs | 59 +++++++++++++++++-- .../SetInitialPasswordRequestModel.cs | 24 +++++++- .../Api/Request/AccountKeysRequestModel.cs | 29 ++++++++- 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 0542d5b00473..ecd8f0907ed8 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -250,20 +250,67 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel throw new UnauthorizedAccessException(); } - if (model.IsV2Request()) + // V2 encryption - TDE user with "manage account recovery" permission + if ( + model.IsV2Request() && + model.IsTdeSetPasswordRequest() && + _featureService.IsEnabled(FeatureFlagKeys.V2RegistrationTDEJIT)) { - if (model.IsTdeSetPasswordRequest()) + await _tdeSetPasswordCommand.SetMasterPasswordAsync(user, model.ToData()); + return; + } + + // V2 encryption - MP JIT + if ( + model.IsV2Request() && + _featureService.IsEnabled(FeatureFlagKeys.EnableAccountEncryptionV2JitPasswordRegistration)) + { + await _finishSsoJitProvisionMasterPasswordCommand.FinishProvisionAsync(user, model.ToData()); + return; + } + + // V1 encryption - new data properties (MP JIT only - AccountKeys on the request means it's not a TDE user) + // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + if ( + model.MasterPasswordAuthentication != null && + model.MasterPasswordUnlock != null && + model.AccountKeys != null) + { + try { - await _tdeSetPasswordCommand.SetMasterPasswordAsync(user, model.ToData()); + user = model.ToUserV1EncryptionFromNewDataTypes(user); } - else + catch (Exception e) { - await _finishSsoJitProvisionMasterPasswordCommand.FinishProvisionAsync(user, model.ToData()); + ModelState.AddModelError(string.Empty, e.Message); + throw new BadRequestException(ModelState); } + + var result = await _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( + user, + model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + model.MasterPasswordUnlock.MasterKeyWrappedUserKey, + model.OrgIdentifier + ); + + if (result.Succeeded) + { + return; + } + + foreach (var error in result.Errors) + { + ModelState.AddModelError(string.Empty, error.Description); + } + + throw new BadRequestException(ModelState); } else { - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + // V1 encryption - legacy data properties + // TODO: code removal requires that BOTH flags have been removed: + // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) + // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) try { user = model.ToUser(user); diff --git a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs index f88938358bf7..d79782abec12 100644 --- a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs @@ -10,7 +10,9 @@ namespace Bit.Api.Auth.Models.Request.Accounts; public class SetInitialPasswordRequestModel : IValidatableObject { - // TODO will be removed with https://bitwarden.atlassian.net/browse/PM-27327 + // TODO: removal requires that BOTH flags have been removed: + // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) + // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) [Obsolete("Use MasterPasswordAuthentication instead")] [StringLength(300)] public string? MasterPasswordHash { get; set; } @@ -43,7 +45,9 @@ public class SetInitialPasswordRequestModel : IValidatableObject [Required] public required string OrgIdentifier { get; set; } - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + // TODO: removal requires that BOTH flags have been removed: + // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) + // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) public User ToUser(User existingUser) { existingUser.MasterPasswordHint = MasterPasswordHint; @@ -56,6 +60,18 @@ public User ToUser(User existingUser) return existingUser; } + // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + public User ToUserV1EncryptionFromNewDataTypes(User existingUser) + { + existingUser.MasterPasswordHint = MasterPasswordHint; + existingUser.Kdf = MasterPasswordAuthentication!.Kdf.KdfType; + existingUser.KdfIterations = MasterPasswordAuthentication.Kdf.Iterations; + existingUser.KdfMemory = MasterPasswordAuthentication.Kdf.Memory; + existingUser.KdfParallelism = MasterPasswordAuthentication.Kdf.Parallelism; + AccountKeys!.ToUserV1Encryption(existingUser); + return existingUser; + } + public IEnumerable Validate(ValidationContext validationContext) { if (IsV2Request()) @@ -71,7 +87,9 @@ public IEnumerable Validate(ValidationContext validationContex } // V1 registration - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + // TODO: removal requires that BOTH flags have been removed: + // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) + // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) if (string.IsNullOrEmpty(MasterPasswordHash)) { yield return new ValidationResult("MasterPasswordHash must be supplied."); diff --git a/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs b/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs index bdf538e6d8d5..8853172f930d 100644 --- a/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs +++ b/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs @@ -1,4 +1,5 @@ -using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Utilities; namespace Bit.Core.KeyManagement.Models.Api.Request; @@ -12,6 +13,32 @@ public class AccountKeysRequestModel public SignatureKeyPairRequestModel? SignatureKeyPair { get; set; } public SecurityStateModel? SecurityState { get; set; } + // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + public User ToUserV1Encryption(User existingUser) + { + if (string.IsNullOrWhiteSpace(AccountPublicKey) || string.IsNullOrWhiteSpace(UserKeyEncryptedAccountPrivateKey)) + { + throw new InvalidOperationException("Public and private keys are required."); + } + + if (string.IsNullOrWhiteSpace(existingUser.PublicKey) && string.IsNullOrWhiteSpace(existingUser.PrivateKey)) + { + existingUser.PublicKey = AccountPublicKey; + existingUser.PrivateKey = UserKeyEncryptedAccountPrivateKey; + return existingUser; + } + else if (existingUser.PrivateKey != null && + AccountPublicKey == existingUser.PublicKey && + CoreHelpers.FixedTimeEquals(UserKeyEncryptedAccountPrivateKey, existingUser.PrivateKey)) + { + return existingUser; + } + else + { + throw new InvalidOperationException("Cannot replace existing key(s) with new key(s)."); + } + } + public UserAccountKeysData ToAccountKeysData() { // This will be cleaned up, after a compatibility period, at which point PublicKeyEncryptionKeyPair and SignatureKeyPair will be required. From b72bff538d3c541498c96688594f1cff235a67fc Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 26 May 2026 14:04:03 -0700 Subject: [PATCH 2/7] update test names to distinguish between Auth & Unlock vs legacy data --- .../Auth/Controllers/AccountsController.cs | 7 ++- .../SetInitialPasswordRequestModel.cs | 10 +++- .../SetInitialPasswordRequestModelTests.cs | 46 +++++++++---------- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index ecd8f0907ed8..7a40248c2759 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -252,7 +252,7 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel // V2 encryption - TDE user with "manage account recovery" permission if ( - model.IsV2Request() && + model.HasAuthAndUnlockData() && model.IsTdeSetPasswordRequest() && _featureService.IsEnabled(FeatureFlagKeys.V2RegistrationTDEJIT)) { @@ -262,7 +262,7 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel // V2 encryption - MP JIT if ( - model.IsV2Request() && + model.HasAuthAndUnlockData() && _featureService.IsEnabled(FeatureFlagKeys.EnableAccountEncryptionV2JitPasswordRegistration)) { await _finishSsoJitProvisionMasterPasswordCommand.FinishProvisionAsync(user, model.ToData()); @@ -272,8 +272,7 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel // V1 encryption - new data properties (MP JIT only - AccountKeys on the request means it's not a TDE user) // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 if ( - model.MasterPasswordAuthentication != null && - model.MasterPasswordUnlock != null && + model.HasAuthAndUnlockData() && model.AccountKeys != null) { try diff --git a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs index d79782abec12..db41e7d1d94c 100644 --- a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs @@ -74,7 +74,7 @@ public User ToUserV1EncryptionFromNewDataTypes(User existingUser) public IEnumerable Validate(ValidationContext validationContext) { - if (IsV2Request()) + if (HasAuthAndUnlockData()) { // V2 registration - validate KDF equality, salt equality, and KDF settings foreach (var validationResult in KdfSettingsValidator.ValidateAuthenticationAndUnlockData( @@ -133,7 +133,13 @@ public IEnumerable Validate(ValidationContext validationContex } } - public bool IsV2Request() + /// + /// True when the request uses the new data shape (MasterPasswordAuthentication + MasterPasswordUnlock). + /// This is a shape check, NOT a guarantee that V2 encryption will run. It is possible for V1 encryption + /// to run even when the request contains these new data types (see `set-password` endpoint). + /// Feature flags and AccountKeys presence determine the actual flow (V1 or V2). + /// + public bool HasAuthAndUnlockData() { // AccountKeys can be null for TDE users, so we don't check that here return MasterPasswordAuthentication != null && MasterPasswordUnlock != null; diff --git a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs index e62fb62468f1..a51eadbde39c 100644 --- a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs +++ b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs @@ -11,12 +11,12 @@ namespace Bit.Api.Test.Auth.Models.Request.Accounts; public class SetInitialPasswordRequestModelTests { - #region V2 Validation Tests + #region Validation Tests (Auth + Unlock Data) [Theory] [InlineData(KdfType.PBKDF2_SHA256, 600000, null, null)] [InlineData(KdfType.Argon2id, 3, 64, 4)] - public void Validate_V2Request_WithMatchingKdfAndSalt_ReturnsNoErrors(KdfType kdfType, int iterations, int? memory, int? parallelism) + public void Validate_AuthAndUnlockData_WithMatchingKdfAndSalt_ReturnsNoErrors(KdfType kdfType, int iterations, int? memory, int? parallelism) { // Arrange — uses separate KDF object instances with identical values to verify value equality var model = new SetInitialPasswordRequestModel @@ -62,7 +62,7 @@ public void Validate_V2Request_WithMatchingKdfAndSalt_ReturnsNoErrors(KdfType kd [Theory] [BitAutoData] - public void Validate_V2Request_WithMismatchedKdfSettings_ReturnsValidationError(string orgIdentifier) + public void Validate_AuthAndUnlockData_WithMismatchedKdfSettings_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -103,7 +103,7 @@ public void Validate_V2Request_WithMismatchedKdfSettings_ReturnsValidationError( [Theory] [BitAutoData] - public void Validate_V2Request_WithMismatchedSalt_ReturnsValidationError(string orgIdentifier) + public void Validate_AuthAndUnlockData_WithMismatchedSalt_ReturnsValidationError(string orgIdentifier) { // Arrange var kdf = new KdfRequestModel @@ -138,7 +138,7 @@ public void Validate_V2Request_WithMismatchedSalt_ReturnsValidationError(string [Theory] [BitAutoData] - public void Validate_V2Request_WithInvalidAuthenticationKdf_ReturnsValidationError(string orgIdentifier) + public void Validate_AuthAndUnlockData_WithInvalidAuthenticationKdf_ReturnsValidationError(string orgIdentifier) { // Arrange var kdf = new KdfRequestModel @@ -174,11 +174,11 @@ public void Validate_V2Request_WithInvalidAuthenticationKdf_ReturnsValidationErr #endregion - #region V1 Validation Tests (Obsolete) + #region Validation Tests (Legacy Data) (Obsolete) [Theory] [BitAutoData] - public void Validate_V1Request_WithMissingMasterPasswordHash_ReturnsValidationError(string orgIdentifier) + public void Validate_LegacyData_WithMissingMasterPasswordHash_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -198,7 +198,7 @@ public void Validate_V1Request_WithMissingMasterPasswordHash_ReturnsValidationEr [Theory] [BitAutoData] - public void Validate_V1Request_WithMissingKey_ReturnsValidationError(string orgIdentifier) + public void Validate_LegacyData_WithMissingKey_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -218,7 +218,7 @@ public void Validate_V1Request_WithMissingKey_ReturnsValidationError(string orgI [Theory] [BitAutoData] - public void Validate_V1Request_WithMissingKdf_ReturnsValidationError(string orgIdentifier) + public void Validate_LegacyData_WithMissingKdf_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -238,7 +238,7 @@ public void Validate_V1Request_WithMissingKdf_ReturnsValidationError(string orgI [Theory] [BitAutoData] - public void Validate_V1Request_WithMissingKdfIterations_ReturnsValidationError(string orgIdentifier) + public void Validate_LegacyData_WithMissingKdfIterations_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -258,7 +258,7 @@ public void Validate_V1Request_WithMissingKdfIterations_ReturnsValidationError(s [Theory] [BitAutoData] - public void Validate_V1Request_WithArgon2idAndMissingMemory_ReturnsValidationError(string orgIdentifier) + public void Validate_LegacyData_WithArgon2idAndMissingMemory_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -280,7 +280,7 @@ public void Validate_V1Request_WithArgon2idAndMissingMemory_ReturnsValidationErr [Theory] [BitAutoData] - public void Validate_V1Request_WithArgon2idAndMissingParallelism_ReturnsValidationError(string orgIdentifier) + public void Validate_LegacyData_WithArgon2idAndMissingParallelism_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -302,7 +302,7 @@ public void Validate_V1Request_WithArgon2idAndMissingParallelism_ReturnsValidati [Theory] [BitAutoData] - public void Validate_V1Request_WithInvalidKdfSettings_ReturnsValidationError(string orgIdentifier) + public void Validate_LegacyData_WithInvalidKdfSettings_ReturnsValidationError(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -325,7 +325,7 @@ public void Validate_V1Request_WithInvalidKdfSettings_ReturnsValidationError(str [Theory] [InlineData(KdfType.PBKDF2_SHA256, 600000, null, null)] [InlineData(KdfType.Argon2id, 3, 64, 4)] - public void Validate_V1Request_WithValidSettings_ReturnsNoErrors(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) + public void Validate_LegacyData_WithValidSettings_ReturnsNoErrors(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) { // Arrange var model = new SetInitialPasswordRequestModel @@ -348,11 +348,11 @@ public void Validate_V1Request_WithValidSettings_ReturnsNoErrors(KdfType kdfType #endregion - #region IsV2Request Tests + #region HasAuthAndUnlockData Tests [Theory] [BitAutoData] - public void IsV2Request_WithV2Properties_ReturnsTrue(string orgIdentifier) + public void HasAuthAndUnlockData_WithBothPresent_ReturnsTrue(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -381,7 +381,7 @@ public void IsV2Request_WithV2Properties_ReturnsTrue(string orgIdentifier) }; // Act - var result = model.IsV2Request(); + var result = model.HasAuthAndUnlockData(); // Assert Assert.True(result); @@ -389,7 +389,7 @@ public void IsV2Request_WithV2Properties_ReturnsTrue(string orgIdentifier) [Theory] [BitAutoData] - public void IsV2Request_WithoutMasterPasswordAuthentication_ReturnsFalse(string orgIdentifier) + public void HasAuthAndUnlockData_WithoutMasterPasswordAuthentication_ReturnsFalse(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -408,7 +408,7 @@ public void IsV2Request_WithoutMasterPasswordAuthentication_ReturnsFalse(string }; // Act - var result = model.IsV2Request(); + var result = model.HasAuthAndUnlockData(); // Assert Assert.False(result); @@ -416,7 +416,7 @@ public void IsV2Request_WithoutMasterPasswordAuthentication_ReturnsFalse(string [Theory] [BitAutoData] - public void IsV2Request_WithoutMasterPasswordUnlock_ReturnsFalse(string orgIdentifier) + public void HasAuthAndUnlockData_WithoutMasterPasswordUnlock_ReturnsFalse(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -435,7 +435,7 @@ public void IsV2Request_WithoutMasterPasswordUnlock_ReturnsFalse(string orgIdent }; // Act - var result = model.IsV2Request(); + var result = model.HasAuthAndUnlockData(); // Assert Assert.False(result); @@ -443,7 +443,7 @@ public void IsV2Request_WithoutMasterPasswordUnlock_ReturnsFalse(string orgIdent [Theory] [BitAutoData] - public void IsV2Request_WithV1Properties_ReturnsFalse(string orgIdentifier) + public void HasAuthAndUnlockData_WithLegacyPropertiesOnly_ReturnsFalse(string orgIdentifier) { // Arrange var model = new SetInitialPasswordRequestModel @@ -456,7 +456,7 @@ public void IsV2Request_WithV1Properties_ReturnsFalse(string orgIdentifier) }; // Act - var result = model.IsV2Request(); + var result = model.HasAuthAndUnlockData(); // Assert Assert.False(result); From b7a205b116dc5173d3d534e38e1a408c8af1077d Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 26 May 2026 14:23:11 -0700 Subject: [PATCH 3/7] add and update tests in AccountsController --- .../Controllers/AccountsControllerTests.cs | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index db07a1bc4d8c..77c1ebad5d22 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -928,6 +928,7 @@ public async Task PostSetPasswordAsync_V2_WhenUserExistsAndSettingPasswordSuccee { // Arrange UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); + _featureService.IsEnabled(FeatureFlagKeys.EnableAccountEncryptionV2JitPasswordRegistration).Returns(true); _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); _finishSsoJitProvisionMasterPasswordCommand.FinishProvisionAsync(user, Arg.Any()) .Returns(Task.CompletedTask); @@ -954,6 +955,7 @@ public async Task PostSetPasswordAsync_V2_WithTdeSetPassword_ShouldCallTdeSetPas { // Arrange UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel, includeTdeSetPassword: true); + _featureService.IsEnabled(FeatureFlagKeys.V2RegistrationTDEJIT).Returns(true); _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); _tdeSetPasswordCommand.SetMasterPasswordAsync(user, Arg.Any()) .Returns(Task.CompletedTask); @@ -993,6 +995,7 @@ public async Task PostSetPasswordAsync_V2_WhenSettingPasswordFails_ShouldThrowEx { // Arrange UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); + _featureService.IsEnabled(FeatureFlagKeys.EnableAccountEncryptionV2JitPasswordRegistration).Returns(true); _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); _finishSsoJitProvisionMasterPasswordCommand.FinishProvisionAsync(user, Arg.Any()) .Returns(Task.FromException(new Exception("Setting password failed"))); @@ -1001,6 +1004,96 @@ public async Task PostSetPasswordAsync_V2_WhenSettingPasswordFails_ShouldThrowEx await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setInitialPasswordRequestModel)); } + // V1 encryption with new data types (transitional path — V2 flags off, request carries Auth + Unlock + AccountKeys) + // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + [Theory] + [BitAutoData] + public async Task PostSetPasswordAsync_V1WithNewDataTypes_WhenUserExistsAndSettingPasswordSucceeds_ShouldCallV1CommandAsync( + User user, + SetInitialPasswordRequestModel setInitialPasswordRequestModel) + { + // Arrange — V2-shape model, V2 flags left disabled → routes to V1-new-data branch. + // Existing user has no keys so AccountKeys.ToUserV1Encryption takes the "set new keys" branch. + UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); + user.PublicKey = null; + user.PrivateKey = null; + + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( + user, + setInitialPasswordRequestModel.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + setInitialPasswordRequestModel.MasterPasswordUnlock.MasterKeyWrappedUserKey, + setInitialPasswordRequestModel.OrgIdentifier) + .Returns(Task.FromResult(IdentityResult.Success)); + + // Act + await _sut.PostSetPasswordAsync(setInitialPasswordRequestModel); + + // Assert — V1 command called with new-data-type values + await _setInitialMasterPasswordCommandV1.Received(1) + .SetInitialMasterPasswordAsync( + Arg.Is(u => u == user), + Arg.Is(s => s == setInitialPasswordRequestModel.MasterPasswordAuthentication.MasterPasswordAuthenticationHash), + Arg.Is(s => s == setInitialPasswordRequestModel.MasterPasswordUnlock.MasterKeyWrappedUserKey), + Arg.Is(s => s == setInitialPasswordRequestModel.OrgIdentifier)); + + // KDF + keys mapped onto user from the new data types + Assert.Equal(setInitialPasswordRequestModel.MasterPasswordHint, user.MasterPasswordHint); + Assert.Equal(setInitialPasswordRequestModel.MasterPasswordAuthentication.Kdf.KdfType, user.Kdf); + Assert.Equal(setInitialPasswordRequestModel.MasterPasswordAuthentication.Kdf.Iterations, user.KdfIterations); + Assert.Equal(setInitialPasswordRequestModel.AccountKeys.AccountPublicKey, user.PublicKey); + Assert.Equal(setInitialPasswordRequestModel.AccountKeys.UserKeyEncryptedAccountPrivateKey, user.PrivateKey); + + // V2 commands not called + await _finishSsoJitProvisionMasterPasswordCommand.DidNotReceiveWithAnyArgs() + .FinishProvisionAsync(Arg.Any(), Arg.Any()); + await _tdeSetPasswordCommand.DidNotReceiveWithAnyArgs() + .SetMasterPasswordAsync(Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PostSetPasswordAsync_V1WithNewDataTypes_WhenExistingKeysCannotBeReplaced_ShouldThrowBadRequestAsync( + User user, + SetInitialPasswordRequestModel setInitialPasswordRequestModel) + { + // Arrange — V2-shape model + existing user has DIFFERENT keys from the model. + // AccountKeys.ToUserV1Encryption should throw "Cannot replace existing key(s)", caught by the + // controller and rethrown as BadRequestException. + UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); + user.PublicKey = "differentExistingPublicKey"; + user.PrivateKey = "differentExistingPrivateKey"; + + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + + // Act & Assert + await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setInitialPasswordRequestModel)); + + // V1 command must not be invoked when mapping fails + await _setInitialMasterPasswordCommandV1.DidNotReceiveWithAnyArgs() + .SetInitialMasterPasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task PostSetPasswordAsync_V1WithNewDataTypes_WhenSettingPasswordFails_ShouldThrowBadRequestAsync( + User user, + SetInitialPasswordRequestModel setInitialPasswordRequestModel) + { + // Arrange — V2-shape model, fresh user keys, V1 command returns a failed IdentityResult. + UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); + user.PublicKey = null; + user.PrivateKey = null; + + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(IdentityResult.Failed(new IdentityError { Description = "boom" }))); + + // Act & Assert + await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setInitialPasswordRequestModel)); + } + private void UpdateSetInitialPasswordRequestModelToV1(SetInitialPasswordRequestModel model) { model.MasterPasswordAuthentication = null; From e8514d3153f586083b23cd62836a168ef0ce854b Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 26 May 2026 16:11:36 -0700 Subject: [PATCH 4/7] add IsJitMpSetPasswordRequest() helper for clarity --- src/Api/Auth/Controllers/AccountsController.cs | 3 ++- .../Request/Accounts/SetInitialPasswordRequestModel.cs | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 7a40248c2759..84718815d0c3 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -263,6 +263,7 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel // V2 encryption - MP JIT if ( model.HasAuthAndUnlockData() && + model.IsJitMpSetPasswordRequest() && _featureService.IsEnabled(FeatureFlagKeys.EnableAccountEncryptionV2JitPasswordRegistration)) { await _finishSsoJitProvisionMasterPasswordCommand.FinishProvisionAsync(user, model.ToData()); @@ -273,7 +274,7 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 if ( model.HasAuthAndUnlockData() && - model.AccountKeys != null) + model.IsJitMpSetPasswordRequest()) { try { diff --git a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs index db41e7d1d94c..41e6e42f9e9c 100644 --- a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs @@ -150,6 +150,11 @@ public bool IsTdeSetPasswordRequest() return AccountKeys == null; } + public bool IsJitMpSetPasswordRequest() + { + return AccountKeys != null; + } + public SetInitialMasterPasswordDataModel ToData() { return new SetInitialMasterPasswordDataModel From cf73eb669dc2da0c1f3a736aa753527a9fce28ea Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:11:30 -0700 Subject: [PATCH 5/7] transition to new approach: collapse set-password V1 branches; ToUser() falls back from MPAD/MPUD to legacy fields --- .../Auth/Controllers/AccountsController.cs | 87 ++++++------------- .../SetInitialPasswordRequestModel.cs | 32 +++---- .../Api/Request/AccountKeysRequestModel.cs | 29 +------ 3 files changed, 39 insertions(+), 109 deletions(-) diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 84718815d0c3..9b9aa273262c 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -270,75 +270,38 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel return; } - // V1 encryption - new data properties (MP JIT only - AccountKeys on the request means it's not a TDE user) - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 - if ( - model.HasAuthAndUnlockData() && - model.IsJitMpSetPasswordRequest()) + // V1 encryption — handles both modern clients (sending MPAD/MPUD + legacy Keys) and + // legacy clients (sending only legacy fields). The model's ToUser() handles the fallback. + // TODO: removal requires that BOTH flags have been removed: + // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) + // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) + try { - try - { - user = model.ToUserV1EncryptionFromNewDataTypes(user); - } - catch (Exception e) - { - ModelState.AddModelError(string.Empty, e.Message); - throw new BadRequestException(ModelState); - } - - var result = await _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( - user, - model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, - model.MasterPasswordUnlock.MasterKeyWrappedUserKey, - model.OrgIdentifier - ); - - if (result.Succeeded) - { - return; - } - - foreach (var error in result.Errors) - { - ModelState.AddModelError(string.Empty, error.Description); - } - - throw new BadRequestException(ModelState); + user = model.ToUser(user); } - else + catch (Exception e) { - // V1 encryption - legacy data properties - // TODO: code removal requires that BOTH flags have been removed: - // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) - // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) - try - { - user = model.ToUser(user); - } - catch (Exception e) - { - ModelState.AddModelError(string.Empty, e.Message); - throw new BadRequestException(ModelState); - } - - var result = await _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( - user, - model.MasterPasswordHash, - model.Key, - model.OrgIdentifier); + ModelState.AddModelError(string.Empty, e.Message); + throw new BadRequestException(ModelState); + } - if (result.Succeeded) - { - return; - } + var result = await _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( + user, + model.MasterPasswordAuthentication?.MasterPasswordAuthenticationHash ?? model.MasterPasswordHash, + model.MasterPasswordUnlock?.MasterKeyWrappedUserKey ?? model.Key, + model.OrgIdentifier); - foreach (var error in result.Errors) - { - ModelState.AddModelError(string.Empty, error.Description); - } + if (result.Succeeded) + { + return; + } - throw new BadRequestException(ModelState); + foreach (var error in result.Errors) + { + ModelState.AddModelError(string.Empty, error.Description); } + + throw new BadRequestException(ModelState); } [HttpPost("verify-password")] diff --git a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs index 41e6e42f9e9c..7487439d5b0f 100644 --- a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs @@ -45,33 +45,23 @@ public class SetInitialPasswordRequestModel : IValidatableObject [Required] public required string OrgIdentifier { get; set; } + // Reads KDF/key from MasterPasswordAuthentication/MasterPasswordUnlock when present (modern clients), + // and falls back to the top-level legacy properties when not (clients ≤3 releases back). // TODO: removal requires that BOTH flags have been removed: // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) public User ToUser(User existingUser) { existingUser.MasterPasswordHint = MasterPasswordHint; - existingUser.Kdf = Kdf!.Value; - existingUser.KdfIterations = KdfIterations!.Value; - existingUser.KdfMemory = KdfMemory; - existingUser.KdfParallelism = KdfParallelism; - existingUser.Key = Key; + existingUser.Kdf = MasterPasswordAuthentication?.Kdf.KdfType ?? Kdf!.Value; + existingUser.KdfIterations = MasterPasswordAuthentication?.Kdf.Iterations ?? KdfIterations!.Value; + existingUser.KdfMemory = MasterPasswordAuthentication?.Kdf.Memory ?? KdfMemory; + existingUser.KdfParallelism = MasterPasswordAuthentication?.Kdf.Parallelism ?? KdfParallelism; + existingUser.Key = MasterPasswordUnlock?.MasterKeyWrappedUserKey ?? Key; Keys?.ToUser(existingUser); return existingUser; } - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 - public User ToUserV1EncryptionFromNewDataTypes(User existingUser) - { - existingUser.MasterPasswordHint = MasterPasswordHint; - existingUser.Kdf = MasterPasswordAuthentication!.Kdf.KdfType; - existingUser.KdfIterations = MasterPasswordAuthentication.Kdf.Iterations; - existingUser.KdfMemory = MasterPasswordAuthentication.Kdf.Memory; - existingUser.KdfParallelism = MasterPasswordAuthentication.Kdf.Parallelism; - AccountKeys!.ToUserV1Encryption(existingUser); - return existingUser; - } - public IEnumerable Validate(ValidationContext validationContext) { if (HasAuthAndUnlockData()) @@ -145,14 +135,18 @@ public bool HasAuthAndUnlockData() return MasterPasswordAuthentication != null && MasterPasswordUnlock != null; } + // TDE users don't send any key material (their keypair already exists). + // Checks both AccountKeys (new) and Keys (legacy) so the predicate is correct for + // the transitional period where clients may send either key shape. public bool IsTdeSetPasswordRequest() { - return AccountKeys == null; + return AccountKeys == null && Keys == null; } + // MP JIT users send new key material — either via the new AccountKeys shape or the legacy Keys shape. public bool IsJitMpSetPasswordRequest() { - return AccountKeys != null; + return AccountKeys != null || Keys != null; } public SetInitialMasterPasswordDataModel ToData() diff --git a/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs b/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs index 8853172f930d..bdf538e6d8d5 100644 --- a/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs +++ b/src/Core/KeyManagement/Models/Api/Request/AccountKeysRequestModel.cs @@ -1,5 +1,4 @@ -using Bit.Core.Entities; -using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Utilities; namespace Bit.Core.KeyManagement.Models.Api.Request; @@ -13,32 +12,6 @@ public class AccountKeysRequestModel public SignatureKeyPairRequestModel? SignatureKeyPair { get; set; } public SecurityStateModel? SecurityState { get; set; } - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 - public User ToUserV1Encryption(User existingUser) - { - if (string.IsNullOrWhiteSpace(AccountPublicKey) || string.IsNullOrWhiteSpace(UserKeyEncryptedAccountPrivateKey)) - { - throw new InvalidOperationException("Public and private keys are required."); - } - - if (string.IsNullOrWhiteSpace(existingUser.PublicKey) && string.IsNullOrWhiteSpace(existingUser.PrivateKey)) - { - existingUser.PublicKey = AccountPublicKey; - existingUser.PrivateKey = UserKeyEncryptedAccountPrivateKey; - return existingUser; - } - else if (existingUser.PrivateKey != null && - AccountPublicKey == existingUser.PublicKey && - CoreHelpers.FixedTimeEquals(UserKeyEncryptedAccountPrivateKey, existingUser.PrivateKey)) - { - return existingUser; - } - else - { - throw new InvalidOperationException("Cannot replace existing key(s) with new key(s)."); - } - } - public UserAccountKeysData ToAccountKeysData() { // This will be cleaned up, after a compatibility period, at which point PublicKeyEncryptionKeyPair and SignatureKeyPair will be required. From a713c286275def4d54dc75a2a80fdabb595ead30 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:17:39 -0700 Subject: [PATCH 6/7] update set-password controller tests for unified V1 path --- .../Controllers/AccountsControllerTests.cs | 87 ++++++++++--------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index 77c1ebad5d22..72e8b8e2ef10 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -1004,17 +1004,25 @@ public async Task PostSetPasswordAsync_V2_WhenSettingPasswordFails_ShouldThrowEx await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setInitialPasswordRequestModel)); } - // V1 encryption with new data types (transitional path — V2 flags off, request carries Auth + Unlock + AccountKeys) - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + // V1 encryption with new data types (transitional path — V2 flags off, modern client carries MPAD/MPUD + legacy Keys) + // TODO: removal requires that BOTH flags have been removed: + // - https://bitwarden.atlassian.net/browse/PM-27327 (MP) + // - https://bitwarden.atlassian.net/browse/PM-27329 (TDE) [Theory] [BitAutoData] - public async Task PostSetPasswordAsync_V1WithNewDataTypes_WhenUserExistsAndSettingPasswordSucceeds_ShouldCallV1CommandAsync( + public async Task PostSetPasswordAsync_V1_NewClientMpJit_UsesMpadMpudValues_ShouldCallV1CommandAsync( User user, SetInitialPasswordRequestModel setInitialPasswordRequestModel) { - // Arrange — V2-shape model, V2 flags left disabled → routes to V1-new-data branch. - // Existing user has no keys so AccountKeys.ToUserV1Encryption takes the "set new keys" branch. + // Arrange — modern MP JIT client: sends MPAD + MPUD + legacy Keys (no AccountKeys, no V2 flag). + // ToUser() should map KDF from MPAD and the wrapped user key from MPUD; legacy Keys?.ToUser sets the keypair. UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); + setInitialPasswordRequestModel.AccountKeys = null; + setInitialPasswordRequestModel.Keys = new KeysRequestModel + { + PublicKey = "newPublicKey", + EncryptedPrivateKey = "newEncryptedPrivateKey" + }; user.PublicKey = null; user.PrivateKey = null; @@ -1029,7 +1037,7 @@ public async Task PostSetPasswordAsync_V1WithNewDataTypes_WhenUserExistsAndSetti // Act await _sut.PostSetPasswordAsync(setInitialPasswordRequestModel); - // Assert — V1 command called with new-data-type values + // Assert — V1 command called with MPAD hash + MPUD wrapped key (not legacy MasterPasswordHash/Key) await _setInitialMasterPasswordCommandV1.Received(1) .SetInitialMasterPasswordAsync( Arg.Is(u => u == user), @@ -1037,12 +1045,14 @@ await _setInitialMasterPasswordCommandV1.Received(1) Arg.Is(s => s == setInitialPasswordRequestModel.MasterPasswordUnlock.MasterKeyWrappedUserKey), Arg.Is(s => s == setInitialPasswordRequestModel.OrgIdentifier)); - // KDF + keys mapped onto user from the new data types + // KDF mapped from MPAD Assert.Equal(setInitialPasswordRequestModel.MasterPasswordHint, user.MasterPasswordHint); Assert.Equal(setInitialPasswordRequestModel.MasterPasswordAuthentication.Kdf.KdfType, user.Kdf); Assert.Equal(setInitialPasswordRequestModel.MasterPasswordAuthentication.Kdf.Iterations, user.KdfIterations); - Assert.Equal(setInitialPasswordRequestModel.AccountKeys.AccountPublicKey, user.PublicKey); - Assert.Equal(setInitialPasswordRequestModel.AccountKeys.UserKeyEncryptedAccountPrivateKey, user.PrivateKey); + + // Public/private keys mapped from legacy Keys + Assert.Equal("newPublicKey", user.PublicKey); + Assert.Equal("newEncryptedPrivateKey", user.PrivateKey); // V2 commands not called await _finishSsoJitProvisionMasterPasswordCommand.DidNotReceiveWithAnyArgs() @@ -1053,45 +1063,44 @@ await _tdeSetPasswordCommand.DidNotReceiveWithAnyArgs() [Theory] [BitAutoData] - public async Task PostSetPasswordAsync_V1WithNewDataTypes_WhenExistingKeysCannotBeReplaced_ShouldThrowBadRequestAsync( + public async Task PostSetPasswordAsync_V1_NewClientTde_UsesMpadMpudValues_DoesNotMutateExistingKeysAsync( User user, SetInitialPasswordRequestModel setInitialPasswordRequestModel) { - // Arrange — V2-shape model + existing user has DIFFERENT keys from the model. - // AccountKeys.ToUserV1Encryption should throw "Cannot replace existing key(s)", caught by the - // controller and rethrown as BadRequestException. - UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); - user.PublicKey = "differentExistingPublicKey"; - user.PrivateKey = "differentExistingPrivateKey"; - - _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); - - // Act & Assert - await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setInitialPasswordRequestModel)); - - // V1 command must not be invoked when mapping fails - await _setInitialMasterPasswordCommandV1.DidNotReceiveWithAnyArgs() - .SetInitialMasterPasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); - } + // Arrange — modern TDE client: sends MPAD + MPUD with both AccountKeys and Keys null (V2 TDE flag off). + // TDE users already have a keypair; Keys?.ToUser is a no-op and the existing keys are left alone. + UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel, includeTdeSetPassword: true); - [Theory] - [BitAutoData] - public async Task PostSetPasswordAsync_V1WithNewDataTypes_WhenSettingPasswordFails_ShouldThrowBadRequestAsync( - User user, - SetInitialPasswordRequestModel setInitialPasswordRequestModel) - { - // Arrange — V2-shape model, fresh user keys, V1 command returns a failed IdentityResult. - UpdateSetInitialPasswordRequestModelToV2(setInitialPasswordRequestModel); - user.PublicKey = null; - user.PrivateKey = null; + const string existingPublicKey = "tdeUserExistingPublicKey"; + const string existingPrivateKey = "tdeUserExistingPrivateKey"; + user.PublicKey = existingPublicKey; + user.PrivateKey = existingPrivateKey; _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(Task.FromResult(IdentityResult.Failed(new IdentityError { Description = "boom" }))); + .Returns(Task.FromResult(IdentityResult.Success)); - // Act & Assert - await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setInitialPasswordRequestModel)); + // Act + await _sut.PostSetPasswordAsync(setInitialPasswordRequestModel); + + // Assert — V1 command called with MPAD hash + MPUD wrapped key + await _setInitialMasterPasswordCommandV1.Received(1) + .SetInitialMasterPasswordAsync( + Arg.Is(u => u == user), + Arg.Is(s => s == setInitialPasswordRequestModel.MasterPasswordAuthentication.MasterPasswordAuthenticationHash), + Arg.Is(s => s == setInitialPasswordRequestModel.MasterPasswordUnlock.MasterKeyWrappedUserKey), + Arg.Is(s => s == setInitialPasswordRequestModel.OrgIdentifier)); + + // Existing keypair preserved + Assert.Equal(existingPublicKey, user.PublicKey); + Assert.Equal(existingPrivateKey, user.PrivateKey); + + // V2 commands not called + await _finishSsoJitProvisionMasterPasswordCommand.DidNotReceiveWithAnyArgs() + .FinishProvisionAsync(Arg.Any(), Arg.Any()); + await _tdeSetPasswordCommand.DidNotReceiveWithAnyArgs() + .SetMasterPasswordAsync(Arg.Any(), Arg.Any()); } private void UpdateSetInitialPasswordRequestModelToV1(SetInitialPasswordRequestModel model) From 0f724cf71bc6cbdfc7f949c5425eb4307c61afc6 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 2 Jun 2026 13:39:24 -0700 Subject: [PATCH 7/7] add tests for ToUser fallback and updated TDE/JIT MP predicates on SetInitialPasswordRequestModel --- .../SetInitialPasswordRequestModelTests.cs | 263 +++++++++++++++++- 1 file changed, 260 insertions(+), 3 deletions(-) diff --git a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs index a51eadbde39c..2c3e3b0d281c 100644 --- a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs +++ b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs @@ -468,9 +468,9 @@ public void HasAuthAndUnlockData_WithLegacyPropertiesOnly_ReturnsFalse(string or [Theory] [BitAutoData] - public void IsTdeSetPasswordRequest_WithNullAccountKeys_ReturnsTrue(string orgIdentifier) + public void IsTdeSetPasswordRequest_WithBothAccountKeysAndKeysNull_ReturnsTrue(string orgIdentifier) { - // Arrange + // Arrange — TDE user sends no key material at all (they already have a keypair) var model = new SetInitialPasswordRequestModel { OrgIdentifier = orgIdentifier, @@ -494,7 +494,8 @@ public void IsTdeSetPasswordRequest_WithNullAccountKeys_ReturnsTrue(string orgId MasterKeyWrappedUserKey = "wrappedKey", Salt = "salt" }, - AccountKeys = null + AccountKeys = null, + Keys = null }; // Act @@ -546,6 +547,99 @@ public void IsTdeSetPasswordRequest_WithAccountKeys_ReturnsFalse(string orgIdent Assert.False(result); } + [Theory] + [BitAutoData] + public void IsTdeSetPasswordRequest_WithLegacyKeysPresent_ReturnsFalse(string orgIdentifier) + { + // Arrange — MP JIT request shape: AccountKeys null but legacy Keys populated. + // Without checking Keys, this would be misclassified as TDE. + var model = new SetInitialPasswordRequestModel + { + OrgIdentifier = orgIdentifier, + AccountKeys = null, + Keys = new KeysRequestModel + { + PublicKey = "publicKey", + EncryptedPrivateKey = "encryptedPrivateKey" + } + }; + + // Act + var result = model.IsTdeSetPasswordRequest(); + + // Assert + Assert.False(result); + } + + #endregion + + #region IsJitMpSetPasswordRequest Tests + + [Theory] + [BitAutoData] + public void IsJitMpSetPasswordRequest_WithAccountKeys_ReturnsTrue(string orgIdentifier) + { + // Arrange — new client / future MP JIT shape + var model = new SetInitialPasswordRequestModel + { + OrgIdentifier = orgIdentifier, + AccountKeys = new AccountKeysRequestModel + { + UserKeyEncryptedAccountPrivateKey = "privateKey", + AccountPublicKey = "publicKey" + }, + Keys = null + }; + + // Act + var result = model.IsJitMpSetPasswordRequest(); + + // Assert + Assert.True(result); + } + + [Theory] + [BitAutoData] + public void IsJitMpSetPasswordRequest_WithLegacyKeys_ReturnsTrue(string orgIdentifier) + { + // Arrange — modern MP JIT client (Option Y) sends legacy Keys, no AccountKeys + var model = new SetInitialPasswordRequestModel + { + OrgIdentifier = orgIdentifier, + AccountKeys = null, + Keys = new KeysRequestModel + { + PublicKey = "publicKey", + EncryptedPrivateKey = "encryptedPrivateKey" + } + }; + + // Act + var result = model.IsJitMpSetPasswordRequest(); + + // Assert + Assert.True(result); + } + + [Theory] + [BitAutoData] + public void IsJitMpSetPasswordRequest_WithBothAccountKeysAndKeysNull_ReturnsFalse(string orgIdentifier) + { + // Arrange — TDE shape: no key material at all + var model = new SetInitialPasswordRequestModel + { + OrgIdentifier = orgIdentifier, + AccountKeys = null, + Keys = null + }; + + // Act + var result = model.IsJitMpSetPasswordRequest(); + + // Assert + Assert.False(result); + } + #endregion #region ToUser Tests (Obsolete) @@ -624,6 +718,169 @@ public void ToUser_WithoutKeys_MapsPropertiesCorrectly(KdfType kdfType, int kdfI Assert.Null(result.PrivateKey); } + [Theory] + [InlineData(KdfType.PBKDF2_SHA256, 600000, null, null)] + [InlineData(KdfType.Argon2id, 3, 64, 4)] + public void ToUser_WithMasterPasswordAuthAndUnlock_AndKeys_ReadsKdfAndKeyFromNewData( + KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) + { + // Arrange — modern client: MPAD + MPUD + legacy Keys, no top-level legacy KDF/key fields + var existingUser = new User(); + var model = new SetInitialPasswordRequestModel + { + OrgIdentifier = "orgIdentifier", + MasterPasswordHint = "hint", + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = new KdfRequestModel + { + KdfType = kdfType, + Iterations = kdfIterations, + Memory = kdfMemory, + Parallelism = kdfParallelism + }, + MasterPasswordAuthenticationHash = "authHash", + Salt = "salt" + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel + { + KdfType = kdfType, + Iterations = kdfIterations, + Memory = kdfMemory, + Parallelism = kdfParallelism + }, + MasterKeyWrappedUserKey = "wrappedKeyFromMpud", + Salt = "salt" + }, + Keys = new KeysRequestModel + { + PublicKey = "publicKey", + EncryptedPrivateKey = "encryptedPrivateKey" + } + }; + + // Act + var result = model.ToUser(existingUser); + + // Assert — KDF mapped from MPAD, user.Key from MPUD, public/private from legacy Keys + Assert.Same(existingUser, result); + Assert.Equal("hint", result.MasterPasswordHint); + Assert.Equal(kdfType, result.Kdf); + Assert.Equal(kdfIterations, result.KdfIterations); + Assert.Equal(kdfMemory, result.KdfMemory); + Assert.Equal(kdfParallelism, result.KdfParallelism); + Assert.Equal("wrappedKeyFromMpud", result.Key); + Assert.Equal("publicKey", result.PublicKey); + Assert.Equal("encryptedPrivateKey", result.PrivateKey); + } + + [Fact] + public void ToUser_WithBothNewAndLegacyFieldsSet_PrefersNewData() + { + // Arrange — defensive: if a request somehow includes both new and legacy KDF/key fields, + // ToUser should source from MPAD/MPUD, not the legacy top-level properties. + // Uses Argon2id in MPAD so Memory/Parallelism are populated (not null) on the new shape; + // verifies the new values win for every non-nullable field. + var existingUser = new User(); + var model = new SetInitialPasswordRequestModel + { + OrgIdentifier = "orgIdentifier", + MasterPasswordHint = "hint", + + // Legacy top-level (should NOT win) + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000, + KdfMemory = 999, + KdfParallelism = 9, + Key = "legacyKey", + + // New shape (should win) + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = new KdfRequestModel + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }, + MasterPasswordAuthenticationHash = "authHash", + Salt = "salt" + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }, + MasterKeyWrappedUserKey = "wrappedKeyFromMpud", + Salt = "salt" + } + }; + + // Act + var result = model.ToUser(existingUser); + + // Assert — values came from MPAD/MPUD, not legacy fields + Assert.Equal(KdfType.Argon2id, result.Kdf); + Assert.Equal(3, result.KdfIterations); + Assert.Equal(64, result.KdfMemory); + Assert.Equal(4, result.KdfParallelism); + Assert.Equal("wrappedKeyFromMpud", result.Key); + } + + [Fact] + public void ToUser_WithMasterPasswordAuthAndUnlock_AndNullKeys_DoesNotMutateExistingPublicPrivateKey() + { + // Arrange — TDE flow: modern client sends MPAD + MPUD with no key material. + // Existing user has a keypair that must not be replaced. + var existingUser = new User + { + PublicKey = "existingPublicKey", + PrivateKey = "existingPrivateKey" + }; + var model = new SetInitialPasswordRequestModel + { + OrgIdentifier = "orgIdentifier", + MasterPasswordHint = "hint", + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = new KdfRequestModel + { + KdfType = KdfType.PBKDF2_SHA256, + Iterations = 600000 + }, + MasterPasswordAuthenticationHash = "authHash", + Salt = "salt" + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel + { + KdfType = KdfType.PBKDF2_SHA256, + Iterations = 600000 + }, + MasterKeyWrappedUserKey = "wrappedKeyFromMpud", + Salt = "salt" + }, + Keys = null + }; + + // Act + var result = model.ToUser(existingUser); + + // Assert — KDF/Key mapped from new data, public/private kept intact + Assert.Equal(KdfType.PBKDF2_SHA256, result.Kdf); + Assert.Equal("wrappedKeyFromMpud", result.Key); + Assert.Equal("existingPublicKey", result.PublicKey); + Assert.Equal("existingPrivateKey", result.PrivateKey); + } + #endregion #region ToData Tests