Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#nullable disable

using Bit.Core.Auth.Repositories;
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.KeyManagement.Repositories;
Expand Down Expand Up @@ -30,7 +32,7 @@ public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand
private readonly IPushNotificationService _pushService;
private readonly IdentityErrorDescriber _identityErrorDescriber;
private readonly IWebAuthnCredentialRepository _credentialRepository;
private readonly IPasswordHasher<User> _passwordHasher;
private readonly IMasterPasswordService _masterPasswordService;
private readonly IUserSignatureKeyPairRepository _userSignatureKeyPairRepository;

/// <summary>
Expand All @@ -44,7 +46,7 @@ public class RotateUserAccountKeysCommand : IRotateUserAccountKeysCommand
/// <param name="emergencyAccessRepository">Provides a method to update re-encrypted emergency access data</param>
/// <param name="organizationUserRepository">Provides a method to update re-encrypted organization user data</param>
/// <param name="deviceRepository">Provides a method to update re-encrypted device keys</param>
/// <param name="passwordHasher">Hashes the new master password</param>
/// <param name="masterPasswordService">Applies master password mutations (hash, key, hint, time markers) on the password-change rotation path</param>
/// <param name="pushService">Logs out user from other devices after successful rotation</param>
/// <param name="errors">Provides a password mismatch error if master password hash validation fails</param>
/// <param name="credentialRepository">Provides a method to update re-encrypted WebAuthn keys</param>
Expand All @@ -53,7 +55,7 @@ public RotateUserAccountKeysCommand(IUserService userService, IUserRepository us
ICipherRepository cipherRepository, IFolderRepository folderRepository, ISendRepository sendRepository,
IEmergencyAccessRepository emergencyAccessRepository, IOrganizationUserRepository organizationUserRepository,
IDeviceRepository deviceRepository,
IPasswordHasher<User> passwordHasher,
IMasterPasswordService masterPasswordService,
IPushNotificationService pushService, IdentityErrorDescriber errors, IWebAuthnCredentialRepository credentialRepository,
IUserSignatureKeyPairRepository userSignatureKeyPairRepository)
{
Expand All @@ -68,7 +70,7 @@ public RotateUserAccountKeysCommand(IUserService userService, IUserRepository us
_pushService = pushService;
_identityErrorDescriber = errors;
_credentialRepository = credentialRepository;
_passwordHasher = passwordHasher;
_masterPasswordService = masterPasswordService;
_userSignatureKeyPairRepository = userSignatureKeyPairRepository;
}

Expand All @@ -90,9 +92,24 @@ public async Task<IdentityResult> PasswordChangeAndRotateUserAccountKeysAsync(Us
List<UpdateEncryptedDataForKeyRotation> saveEncryptedDataActions = [];
var shouldPersistV2UpgradeToken = await BaseRotateUserAccountKeysAsync(model.BaseData, user, saveEncryptedDataActions);

user.Key = model.MasterPasswordUnlockData.MasterKeyWrappedUserKey;
user.MasterPassword = _passwordHasher.HashPassword(user, model.MasterPasswordAuthenticationData.MasterPasswordAuthenticationHash);
user.MasterPasswordHint = model.MasterPasswordHint;
// Delegate the master password mutation (hash, wrapped user key, hint, time markers) to
// MasterPasswordService.
// - RefreshStamp = false: BaseRotateUserAccountKeysAsync already owns SecurityStamp rotation
// with V2-upgrade-token-aware logic; the service must not double-handle it.
// - MasterPasswordHint populated from the request because a password change can update the hint.
var updatePasswordData = new UpdateExistingPasswordData
{
MasterPasswordAuthentication = model.MasterPasswordAuthenticationData,
MasterPasswordUnlock = model.MasterPasswordUnlockData,
ValidatePassword = true,
RefreshStamp = false,
MasterPasswordHint = model.MasterPasswordHint,
};
var preparedResult = await _masterPasswordService.PrepareUpdateExistingMasterPasswordAsync(user, updatePasswordData);
if (preparedResult.TryPickT1(out var errors, out _))
{
return IdentityResult.Failed(errors);
}

await _userRepository.UpdateUserKeyAndEncryptedDataV2Async(user, saveEncryptedDataActions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ public async Task UpdateUserKeyAndEncryptedDataV2Async(Core.Entities.User user,
userEntity.MasterPassword = user.MasterPassword;
userEntity.MasterPasswordHint = user.MasterPasswordHint;

userEntity.LastPasswordChangeDate = user.LastPasswordChangeDate;
userEntity.LastKeyRotationDate = user.LastKeyRotationDate;
userEntity.AccountRevisionDate = user.AccountRevisionDate;
userEntity.RevisionDate = user.RevisionDate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ public async Task PasswordChangeAndRotateUserAccountKeysAsync_Success(RotateUser
SetupRotateUserAccountUnlockData(request, user);
SetupRotateUserAccountData(request);
SetupRotateUserAccountKeys(request, isV2Crypto: false);
var originalMasterPassword = user.MasterPassword;

var response = await _client.PostAsJsonAsync("/accounts/key-management/rotate-user-account-keys", request);
var responseMessage = await response.Content.ReadAsStringAsync();
Expand All @@ -203,6 +204,23 @@ public async Task PasswordChangeAndRotateUserAccountKeysAsync_Success(RotateUser
Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfIterations, userNewState.KdfIterations);
Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory, userNewState.KdfMemory);
Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism, userNewState.KdfParallelism);

// Parity preservation: master-key-wrapped user key and hint are applied from the request
Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey, userNewState.Key);
Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.MasterPasswordHint, userNewState.MasterPasswordHint);

// Parity preservation: master password hash is rewritten and verifies against the new authentication hash
Assert.NotEqual(originalMasterPassword, userNewState.MasterPassword);
Assert.Equal(
PasswordVerificationResult.Success,
_passwordHasher.VerifyHashedPassword(
userNewState,
userNewState.MasterPassword!,
request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyAuthenticationHash));

// PM-38811 parity gap closer: LastPasswordChangeDate is set on this path via MasterPasswordService
Assert.NotNull(userNewState.LastPasswordChangeDate);
Assert.Equal(DateTime.UtcNow, userNewState.LastPasswordChangeDate.Value, TimeSpan.FromMinutes(1));
Comment thread
JaredSnider-Bitwarden marked this conversation as resolved.
Dismissed
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
ο»Ώ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.Enums;
Expand All @@ -19,6 +21,7 @@
using Microsoft.AspNetCore.Identity;
using NSubstitute;
using NSubstitute.ReturnsExtensions;
using OneOf;
using Xunit;

namespace Bit.Core.Test.KeyManagement.UserKey;
Expand Down Expand Up @@ -593,6 +596,87 @@ await sutProvider.GetDependency<IPushNotificationService>().Received(1)
.PushLogOutAsync(user.Id);
}

[Theory, BitAutoData]
public async Task PasswordChangeAndRotateUserAccountKeysAsync_DelegatesToMasterPasswordService(
SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, PasswordChangeAndRotateUserAccountKeysData model)
{
// Arrange
SetTestKdfAndSaltForUserAndModel(user, model);
var signatureRepository = sutProvider.GetDependency<IUserSignatureKeyPairRepository>();
SetV1ExistingUser(user, signatureRepository);
SetV1ModelUser(model.BaseData);

sutProvider.GetDependency<IUserService>()
.CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash)
.Returns(true);
sutProvider.GetDependency<IMasterPasswordService>()
.PrepareUpdateExistingMasterPasswordAsync(user, Arg.Any<UpdateExistingPasswordData>())
.Returns(OneOf<User, IdentityError[]>.FromT0(user));

// Act
var result = await sutProvider.Sut.PasswordChangeAndRotateUserAccountKeysAsync(user, model);

// Assert - delegation occurred with correct data shape
Assert.Equal(IdentityResult.Success, result);
await sutProvider.GetDependency<IMasterPasswordService>().Received(1)
.PrepareUpdateExistingMasterPasswordAsync(user, Arg.Is<UpdateExistingPasswordData>(d =>
d.MasterPasswordAuthentication == model.MasterPasswordAuthenticationData &&
d.MasterPasswordUnlock == model.MasterPasswordUnlockData &&
d.MasterPasswordHint == model.MasterPasswordHint &&
d.ValidatePassword == true &&
Comment thread
JaredSnider-Bitwarden marked this conversation as resolved.
Dismissed
d.RefreshStamp == false));
Comment thread
JaredSnider-Bitwarden marked this conversation as resolved.
Dismissed
}

[Theory, BitAutoData]
public async Task PasswordChangeAndRotateUserAccountKeysAsync_MasterPasswordServiceFails_ReturnsFailedAndSkipsPersist(
SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, PasswordChangeAndRotateUserAccountKeysData model)
{
// Arrange
SetTestKdfAndSaltForUserAndModel(user, model);
var signatureRepository = sutProvider.GetDependency<IUserSignatureKeyPairRepository>();
SetV1ExistingUser(user, signatureRepository);
SetV1ModelUser(model.BaseData);

sutProvider.GetDependency<IUserService>()
.CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash)
.Returns(true);

var serviceError = new IdentityError { Code = "SomeError", Description = "Service rejected" };
sutProvider.GetDependency<IMasterPasswordService>()
.PrepareUpdateExistingMasterPasswordAsync(user, Arg.Any<UpdateExistingPasswordData>())
.Returns(OneOf<User, IdentityError[]>.FromT1(new[] { serviceError }));

// Act
var result = await sutProvider.Sut.PasswordChangeAndRotateUserAccountKeysAsync(user, model);

// Assert - failure surfaced and side effects skipped
Assert.False(result.Succeeded);
Assert.Contains(result.Errors, e => e.Code == "SomeError");
await sutProvider.GetDependency<IUserRepository>().DidNotReceive()
.UpdateUserKeyAndEncryptedDataV2Async(Arg.Any<User>(), Arg.Any<IEnumerable<UpdateEncryptedDataForKeyRotation>>());
await sutProvider.GetDependency<IPushNotificationService>().DidNotReceive()
.PushLogOutAsync(Arg.Any<Guid>(), Arg.Any<bool>(), Arg.Any<PushNotificationLogOutReason?>());
}

[Theory, BitAutoData]
public async Task PasswordChangeAndRotateUserAccountKeysAsync_WrongOldPassword_DoesNotCallMasterPasswordService(
SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, PasswordChangeAndRotateUserAccountKeysData model)
{
// Arrange
user.Email = model.MasterPasswordUnlockData.Salt;
sutProvider.GetDependency<IUserService>()
.CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash)
.Returns(false);

// Act
var result = await sutProvider.Sut.PasswordChangeAndRotateUserAccountKeysAsync(user, model);

// Assert - rejection short-circuits before the MP service
Assert.False(result.Succeeded);
await sutProvider.GetDependency<IMasterPasswordService>().DidNotReceive()
.PrepareUpdateExistingMasterPasswordAsync(Arg.Any<User>(), Arg.Any<UpdateExistingPasswordData>());
}

[Theory]
[BitAutoData]
public async Task MasterPasswordRotateUserAccountKeysAsync_MissingUser_Throws(
Expand Down
Loading