From 3c0ca482a9489cb975dd4c3eac22715e4ad7f626 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 9 Jun 2025 11:55:52 +0200 Subject: [PATCH] Apply fixes --- .../RotateUserAccountkeysCommand.cs | 15 ++--- .../RotateUserAccountKeysCommandTests.cs | 60 +++++++++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs index 06ef3ce9a7..7041084943 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs @@ -65,7 +65,7 @@ public class RotateUserAccountKeysCommand( List saveEncryptedDataActions = []; - UpdateAccountKeys(model, user, saveEncryptedDataActions); + await UpdateAccountKeys(model, user, saveEncryptedDataActions); UpdateUnlockMethods(model, user, saveEncryptedDataActions); UpdateUserData(model, user, saveEncryptedDataActions); @@ -97,10 +97,10 @@ public class RotateUserAccountKeysCommand( throw new InvalidOperationException("User is in an invalid state for key rotation. User has a signature key pair, but the private key is not in v2 format, or vice versa."); } - async void ValidateRotationModelSignatureKeyPairForV2User(RotateUserAccountKeysData model, User user) + async Task ValidateRotationModelSignatureKeyPairForV2User(RotateUserAccountKeysData model, User user) { var currentSignatureKeyPair = await _userSignatureKeyPairRepository.GetByUserIdAsync(user.Id); - if (model.AccountKeys.SignatureKeyPairData == null) + if (model.AccountKeys == null || model.AccountKeys.SignatureKeyPairData == null) { throw new InvalidOperationException("The provided signing key data is null, but the user already has signing keys."); } @@ -108,7 +108,7 @@ public class RotateUserAccountKeysCommand( { throw new InvalidOperationException("The provided signing key data does not match the user's current signing key data."); } - if (string.IsNullOrEmpty(model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey)) + if (string.IsNullOrEmpty(model.AccountKeys.PublicKeyEncryptionKeyPairData?.SignedPublicKey)) { throw new InvalidOperationException("No signed public key provided, but the user already has a signature key pair."); } @@ -137,12 +137,13 @@ public class RotateUserAccountKeysCommand( } } - async void UpdateAccountKeys(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) + async Task UpdateAccountKeys(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) { var isV2User = await IsV2EncryptionUserAsync(user); // Changing the public key encryption key pair is not supported during key rotation for now; so this ensures it is not accidentally changed - if (model.AccountKeys.PublicKeyEncryptionKeyPairData.PublicKey != user.PublicKey) + var providedPublicKey = model.AccountKeys?.PublicKeyEncryptionKeyPairData?.PublicKey ?? model.AccountPublicKey; + if (providedPublicKey != user.PublicKey) { throw new InvalidOperationException("The provided account public key does not match the user's current public key, and changing the account asymmetric keypair is currently not supported during key rotation."); } @@ -162,7 +163,7 @@ public class RotateUserAccountKeysCommand( if (isV2User) { - ValidateRotationModelSignatureKeyPairForV2User(model, user); + await ValidateRotationModelSignatureKeyPairForV2User(model, user); } else { diff --git a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs index e677814fc1..f8d6d37ea3 100644 --- a/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs +++ b/test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs @@ -1,5 +1,6 @@ using Bit.Core.Entities; using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.KeyManagement.Repositories; using Bit.Core.KeyManagement.UserKey.Implementations; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; @@ -25,12 +26,14 @@ public class RotateUserAccountKeysCommandTests Assert.NotEqual(IdentityResult.Success, result); } + [Theory, BitAutoData] public async Task ThrowsWhenUserIsNull(SutProvider sutProvider, RotateUserAccountKeysData model) { await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(null, model)); } + [Theory, BitAutoData] public async Task RejectsEmailChange(SutProvider sutProvider, User user, RotateUserAccountKeysData model) @@ -75,6 +78,7 @@ public class RotateUserAccountKeysCommandTests RotateUserAccountKeysData model) { user.PublicKey = "old-public"; + user.PrivateKey = "2.xxx"; user.Kdf = Enums.KdfType.Argon2id; user.KdfIterations = 3; user.KdfMemory = 64; @@ -117,4 +121,60 @@ public class RotateUserAccountKeysCommandTests Assert.Equal(IdentityResult.Success, result); } + + [Theory, BitAutoData] + public async Task ThrowsWhenSignatureKeyPairMissingForV2User(SutProvider sutProvider, User user, + RotateUserAccountKeysData model) + { + // Simulate v2 user (e.g., by setting a property or flag, depending on implementation) + user.Kdf = Enums.KdfType.Argon2id; + user.KdfIterations = 3; + user.KdfMemory = 64; + user.KdfParallelism = 4; + user.PublicKey = "v2-public-key"; + // Remove signature key pair + if (model.AccountKeys != null) + { + model.AccountKeys.SignatureKeyPairData = null; + } + model.MasterPasswordUnlockData.Email = user.Email; + model.MasterPasswordUnlockData.KdfType = Enums.KdfType.Argon2id; + model.MasterPasswordUnlockData.KdfIterations = 3; + model.MasterPasswordUnlockData.KdfMemory = 64; + model.MasterPasswordUnlockData.KdfParallelism = 4; + model.AccountPublicKey = user.PublicKey; + sutProvider.GetDependency().GetByUserIdAsync(user.Id) + .Returns((SignatureKeyPairData)null); + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) + .Returns(true); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RotateUserAccountKeysAsync(user, model)); + } + + [Theory, BitAutoData] + public async Task DoesNotThrowWhenSignatureKeyPairPresentForV2User(SutProvider sutProvider, User user, + RotateUserAccountKeysData model) + { + user.Kdf = Enums.KdfType.Argon2id; + user.KdfIterations = 3; + user.KdfMemory = 64; + user.KdfParallelism = 4; + user.PublicKey = "v2-public-key"; + // Ensure signature key pair is present + if (model.AccountKeys != null) + { + model.AccountKeys.SignatureKeyPairData = new SignatureKeyPairData( + Bit.Core.KeyManagement.Enums.SignatureAlgorithm.Ed25519, "dummyWrappedSigningKey", "dummyVerifyingKey"); + } + model.MasterPasswordUnlockData.Email = user.Email; + model.MasterPasswordUnlockData.KdfType = Enums.KdfType.Argon2id; + model.MasterPasswordUnlockData.KdfIterations = 3; + model.MasterPasswordUnlockData.KdfMemory = 64; + model.MasterPasswordUnlockData.KdfParallelism = 4; + model.AccountPublicKey = user.PublicKey; + sutProvider.GetDependency().CheckPasswordAsync(user, model.OldMasterKeyAuthenticationHash) + .Returns(true); + var result = await sutProvider.Sut.RotateUserAccountKeysAsync(user, model); + Assert.Equal(IdentityResult.Success, result); + } + }