Improve tests

src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs

@@ -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?.SiginedPublicKey))
         {
             throw new InvalidOperationException("No signed public key provided, but the user already has a signature key pair.");
         }
@@ -118,31 +118,25 @@ public class RotateUserAccountKeysCommand(
         }
     }
 
-    void ValidateRotationModelSignatureKeyPairForV1UserAndUpgradeToV2(RotateUserAccountKeysData model, User user, List<UpdateEncryptedDataForKeyRotation> saveEncryptedDataActions)
+    public void ValidateRotationModelSignatureKeyPairForV1UserAndUpgradeToV2(RotateUserAccountKeysData model, User user, List<UpdateEncryptedDataForKeyRotation> saveEncryptedDataActions)
     {
         if (model.AccountKeys.SignatureKeyPairData != null)
         {
-            // user is upgrading
+            if (string.IsNullOrEmpty(model.AccountKeys.PublicKeyEncryptionKeyPairData?.SignedPublicKey))
-            if (string.IsNullOrEmpty(model.AccountKeys.SignatureKeyPairData.VerifyingKey))
             {
-                throw new InvalidOperationException("The provided signing key data does not contain a valid verifying key.");
+                throw new InvalidOperationException("The provided public key encryption key pair data does not contain a valid signed public key.");
-            }
-
-            if (string.IsNullOrEmpty(model.AccountKeys.SignatureKeyPairData.WrappedSigningKey))
-            {
-                throw new InvalidOperationException("The provided signing key data does not contain a valid wrapped signing key.");
             }
             saveEncryptedDataActions.Add(_userSignatureKeyPairRepository.SetUserSignatureKeyPair(user.Id, model.AccountKeys.SignatureKeyPairData));
             user.SignedPublicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey;
         }
     }
 
-    async Task UpdateAccountKeys(RotateUserAccountKeysData model, User user, List<UpdateEncryptedDataForKeyRotation> saveEncryptedDataActions)
+    public async Task UpdateAccountKeys(RotateUserAccountKeysData model, User user, List<UpdateEncryptedDataForKeyRotation> 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
-        var providedPublicKey = model.AccountKeys?.PublicKeyEncryptionKeyPairData?.PublicKey ?? model.AccountPublicKey;
+        var providedPublicKey = 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.");
@@ -153,7 +147,7 @@ public class RotateUserAccountKeysCommand(
         {
             throw new InvalidOperationException("The provided user key encrypted account private key was not wrapped with XChaCha20-Poly1305");
         }
-        if (!isV2User && GetEncryptionType(model.UserKeyEncryptedAccountPrivateKey) != EncryptionType.AesCbc256_HmacSha256_B64)
+        if (!isV2User && model.AccountKeys.SignatureKeyPairData == null && GetEncryptionType(model.UserKeyEncryptedAccountPrivateKey) != EncryptionType.AesCbc256_HmacSha256_B64)
         {
             throw new InvalidOperationException("The provided user key encrypted account private key was not wrapped with AES-256-CBC-HMAC");
         }

src/Infrastructure.EntityFramework/KeyManagement/Repositories/UserSignatureKeyPairRepository.cs

@@ -17,7 +17,7 @@ public class UserSignatureKeyPairRepository(IServiceScopeFactory serviceScopeFac
     {
         await using var scope = ServiceScopeFactory.CreateAsyncScope();
         var dbContext = GetDatabaseContext(scope);
-        var signingKeys = await dbContext.UserSignatureKeyPairs.FindAsync(userId);
+        var signingKeys = await dbContext.UserSignatureKeyPairs.FirstOrDefaultAsync(x => x.UserId == userId);
         if (signingKeys == null)
         {
             return null;

test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs

@@ -12,6 +12,9 @@ using Bit.Core.Auth.Models.Api.Request.Accounts;
 using Bit.Core.Billing.Enums;
 using Bit.Core.Entities;
 using Bit.Core.Enums;
+using Bit.Core.KeyManagement.Entities;
+using Bit.Core.KeyManagement.Enums;
+using Bit.Core.KeyManagement.Repositories;
 using Bit.Core.Repositories;
 using Bit.Core.Vault.Enums;
 using Bit.Test.Common.AutoFixture.Attributes;
@@ -24,6 +27,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
 {
     private static readonly string _mockEncryptedString =
         "2.AOs41Hd8OQiCPXjyJKCiDA==|O6OHgt2U2hJGBSNGnimJmg==|iD33s8B69C8JhYYhSa4V1tArjvLr8eEaGqOV7BRo5Jk=";
+    private static readonly string _mockEncryptedType7String = "7.AOs41Hd8OQiCPXjyJKCiDA==";
 
     private readonly HttpClient _client;
     private readonly IEmergencyAccessRepository _emergencyAccessRepository;
@@ -34,6 +38,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
     private readonly IDeviceRepository _deviceRepository;
     private readonly IPasswordHasher<User> _passwordHasher;
     private readonly IOrganizationRepository _organizationRepository;
+    private readonly IUserSignatureKeyPairRepository _userSignatureKeyPairRepository;
     private string _ownerEmail = null!;
 
     public AccountsKeyManagementControllerTests(ApiApplicationFactory factory)
@@ -49,6 +54,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
         _organizationUserRepository = _factory.GetService<IOrganizationUserRepository>();
         _passwordHasher = _factory.GetService<IPasswordHasher<User>>();
         _organizationRepository = _factory.GetService<IOrganizationRepository>();
+        _userSignatureKeyPairRepository = _factory.GetService<IUserSignatureKeyPairRepository>();
     }
 
     public async Task InitializeAsync()
@@ -200,6 +206,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
         var password = _passwordHasher.HashPassword(user, "newMasterPassword");
         user.MasterPassword = password;
         user.PublicKey = "publicKey";
+        user.PrivateKey = _mockEncryptedString;
         await _userRepository.ReplaceAsync(user);
 
         request.AccountUnlockData.MasterPasswordUnlockData.KdfType = user.Kdf;
@@ -209,6 +216,8 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
         request.AccountUnlockData.MasterPasswordUnlockData.Email = user.Email;
         request.AccountKeys.AccountPublicKey = "publicKey";
         request.AccountKeys.UserKeyEncryptedAccountPrivateKey = _mockEncryptedString;
+        request.AccountKeys.PublicKeyEncryptionKeyPair = null;
+        request.AccountKeys.SignatureKeyPair = null;
 
         request.OldMasterKeyAuthenticationHash = "newMasterPassword";
 
@@ -354,4 +363,191 @@ public class AccountsKeyManagementControllerTests : IClassFixture<ApiApplication
         Assert.Equal(DateTime.UtcNow, user.RevisionDate, TimeSpan.FromMinutes(1));
         Assert.Equal(DateTime.UtcNow, user.AccountRevisionDate, TimeSpan.FromMinutes(1));
     }
+
+    [Theory]
+    [BitAutoData]
+    public async Task RotateV2UserAccountKeysAsync_Success(RotateUserAccountKeysAndDataRequestModel request)
+    {
+        await _loginHelper.LoginAsync(_ownerEmail);
+        var user = await _userRepository.GetByEmailAsync(_ownerEmail);
+        if (user == null)
+        {
+            throw new InvalidOperationException("User not found.");
+        }
+
+        var password = _passwordHasher.HashPassword(user, "newMasterPassword");
+        user.MasterPassword = password;
+        user.PublicKey = "publicKey";
+        user.PrivateKey = _mockEncryptedType7String;
+
+        await _userRepository.ReplaceAsync(user);
+        await _userSignatureKeyPairRepository.CreateAsync(new UserSignatureKeyPair
+        {
+            UserId = user.Id,
+            SignatureAlgorithm = SignatureAlgorithm.Ed25519,
+            SigningKey = _mockEncryptedType7String,
+            VerifyingKey = "verifyingKey",
+        });
+
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfType = user.Kdf;
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfIterations = user.KdfIterations;
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory = user.KdfMemory;
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism = user.KdfParallelism;
+        request.AccountUnlockData.MasterPasswordUnlockData.Email = user.Email;
+        request.AccountKeys.AccountPublicKey = "publicKey";
+        request.AccountKeys.UserKeyEncryptedAccountPrivateKey = _mockEncryptedType7String;
+        request.AccountKeys.PublicKeyEncryptionKeyPair = new PublicKeyEncryptionKeyPairRequestModel
+        {
+            PublicKey = "publicKey",
+            WrappedPrivateKey = _mockEncryptedType7String,
+            SignedPublicKey = "signedPublicKey",
+        };
+        request.AccountKeys.SignatureKeyPair = new SignatureKeyPairRequestModel
+        {
+            SignatureAlgorithm = SignatureAlgorithm.Ed25519,
+            WrappedSigningKey = _mockEncryptedType7String,
+            VerifyingKey = "verifyingKey",
+        };
+
+        request.OldMasterKeyAuthenticationHash = "newMasterPassword";
+
+        request.AccountData.Ciphers =
+        [
+            new CipherWithIdRequestModel
+            {
+                Id = Guid.NewGuid(),
+                Type = CipherType.Login,
+                Name = _mockEncryptedString,
+                Login = new CipherLoginModel
+                {
+                    Username = _mockEncryptedString,
+                    Password = _mockEncryptedString,
+                },
+            },
+        ];
+        request.AccountData.Folders = [
+            new FolderWithIdRequestModel
+            {
+                Id = Guid.NewGuid(),
+                Name = _mockEncryptedString,
+            },
+        ];
+        request.AccountData.Sends = [
+            new SendWithIdRequestModel
+            {
+                Id = Guid.NewGuid(),
+                Name = _mockEncryptedString,
+                Key = _mockEncryptedString,
+                Disabled = false,
+                DeletionDate = DateTime.UtcNow.AddDays(1),
+            },
+        ];
+        request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey = _mockEncryptedString;
+        request.AccountUnlockData.PasskeyUnlockData = [];
+        request.AccountUnlockData.DeviceKeyUnlockData = [];
+        request.AccountUnlockData.EmergencyAccessUnlockData = [];
+        request.AccountUnlockData.OrganizationAccountRecoveryUnlockData = [];
+
+        var response = await _client.PostAsJsonAsync("/accounts/key-management/rotate-user-account-keys", request);
+        var responseMessage = await response.Content.ReadAsStringAsync();
+        response.EnsureSuccessStatusCode();
+
+        var userNewState = await _userRepository.GetByEmailAsync(_ownerEmail);
+        Assert.NotNull(userNewState);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.Email, userNewState.Email);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfType, userNewState.Kdf);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfIterations, userNewState.KdfIterations);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory, userNewState.KdfMemory);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism, userNewState.KdfParallelism);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async Task RotateUpgradeToV2UserAccountKeysAsync_Success(RotateUserAccountKeysAndDataRequestModel request)
+    {
+        await _loginHelper.LoginAsync(_ownerEmail);
+        var user = await _userRepository.GetByEmailAsync(_ownerEmail);
+        if (user == null)
+        {
+            throw new InvalidOperationException("User not found.");
+        }
+
+        var password = _passwordHasher.HashPassword(user, "newMasterPassword");
+        user.MasterPassword = password;
+        user.PublicKey = "publicKey";
+        user.PrivateKey = _mockEncryptedString;
+
+        await _userRepository.ReplaceAsync(user);
+
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfType = user.Kdf;
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfIterations = user.KdfIterations;
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory = user.KdfMemory;
+        request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism = user.KdfParallelism;
+        request.AccountUnlockData.MasterPasswordUnlockData.Email = user.Email;
+        request.AccountKeys.AccountPublicKey = "publicKey";
+        request.AccountKeys.UserKeyEncryptedAccountPrivateKey = _mockEncryptedType7String;
+        request.AccountKeys.PublicKeyEncryptionKeyPair = new PublicKeyEncryptionKeyPairRequestModel
+        {
+            PublicKey = "publicKey",
+            WrappedPrivateKey = _mockEncryptedType7String,
+            SignedPublicKey = "signedPublicKey",
+        };
+        request.AccountKeys.SignatureKeyPair = new SignatureKeyPairRequestModel
+        {
+            SignatureAlgorithm = SignatureAlgorithm.Ed25519,
+            WrappedSigningKey = _mockEncryptedType7String,
+            VerifyingKey = "verifyingKey",
+        };
+
+        request.OldMasterKeyAuthenticationHash = "newMasterPassword";
+
+        request.AccountData.Ciphers =
+        [
+            new CipherWithIdRequestModel
+            {
+                Id = Guid.NewGuid(),
+                Type = CipherType.Login,
+                Name = _mockEncryptedString,
+                Login = new CipherLoginModel
+                {
+                    Username = _mockEncryptedString,
+                    Password = _mockEncryptedString,
+                },
+            },
+        ];
+        request.AccountData.Folders = [
+            new FolderWithIdRequestModel
+            {
+                Id = Guid.NewGuid(),
+                Name = _mockEncryptedString,
+            },
+        ];
+        request.AccountData.Sends = [
+            new SendWithIdRequestModel
+            {
+                Id = Guid.NewGuid(),
+                Name = _mockEncryptedString,
+                Key = _mockEncryptedString,
+                Disabled = false,
+                DeletionDate = DateTime.UtcNow.AddDays(1),
+            },
+        ];
+        request.AccountUnlockData.MasterPasswordUnlockData.MasterKeyEncryptedUserKey = _mockEncryptedString;
+        request.AccountUnlockData.PasskeyUnlockData = [];
+        request.AccountUnlockData.DeviceKeyUnlockData = [];
+        request.AccountUnlockData.EmergencyAccessUnlockData = [];
+        request.AccountUnlockData.OrganizationAccountRecoveryUnlockData = [];
+
+        var response = await _client.PostAsJsonAsync("/accounts/key-management/rotate-user-account-keys", request);
+        var responseMessage = await response.Content.ReadAsStringAsync();
+        response.EnsureSuccessStatusCode();
+
+        var userNewState = await _userRepository.GetByEmailAsync(_ownerEmail);
+        Assert.NotNull(userNewState);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.Email, userNewState.Email);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfType, userNewState.Kdf);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfIterations, userNewState.KdfIterations);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfMemory, userNewState.KdfMemory);
+        Assert.Equal(request.AccountUnlockData.MasterPasswordUnlockData.KdfParallelism, userNewState.KdfParallelism);
+    }
 }

test/Core.Test/KeyManagement/UserKey/RotateUserAccountKeysCommandTests.cs

@@ -1,4 +1,5 @@
 using Bit.Core.Entities;
+using Bit.Core.KeyManagement.Enums;
 using Bit.Core.KeyManagement.Models.Data;
 using Bit.Core.KeyManagement.Repositories;
 using Bit.Core.KeyManagement.UserKey.Implementations;
@@ -7,6 +8,7 @@ using Bit.Test.Common.AutoFixture;
 using Bit.Test.Common.AutoFixture.Attributes;
 using Microsoft.AspNetCore.Identity;
 using NSubstitute;
+using NSubstitute.ReturnsExtensions;
 using Xunit;
 
 namespace Bit.Core.Test.KeyManagement.UserKey;
@@ -194,4 +196,75 @@ public class RotateUserAccountKeysCommandTests
         Assert.Equal(IdentityResult.Success, result);
     }
 
+
+    [Theory, BitAutoData]
+    public async Task UpdateAccountKeys_ThrowsIfPublicKeyChangesAsync(SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, RotateUserAccountKeysData model)
+    {
+        user.PrivateKey = "2.xxx";
+        sutProvider.GetDependency<IUserSignatureKeyPairRepository>()
+            .GetByUserIdAsync(user.Id)
+            .ReturnsNull();
+        user.PublicKey = "old-public";
+        model.AccountKeys.PublicKeyEncryptionKeyPairData.PublicKey = "new-public";
+        var saveEncryptedDataActions = new List<Core.KeyManagement.UserKey.UpdateEncryptedDataForKeyRotation>();
+        await Assert.ThrowsAsync<InvalidOperationException>(async () => await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions));
+    }
+
+    [Theory, BitAutoData]
+    public async Task UpdateAccountKeys_ThrowsIfPrivateKeyWrappedWithNotXchacha20ForV2UserAsync(SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, RotateUserAccountKeysData model)
+    {
+        user.PrivateKey = "7.xxx";
+        sutProvider.GetDependency<IUserSignatureKeyPairRepository>()
+            .GetByUserIdAsync(user.Id)
+            .Returns(new SignatureKeyPairData(SignatureAlgorithm.Ed25519, "7.xxx", "public"));
+        user.PublicKey = "public";
+        model.UserKeyEncryptedAccountPrivateKey = "2.xxx";
+        model.AccountPublicKey = user.PublicKey;
+        model.AccountKeys.PublicKeyEncryptionKeyPairData = new PublicKeyEncryptionKeyPairData("2.xxx", "public", null);
+        model.AccountKeys.SignatureKeyPairData = null;
+        var saveEncryptedDataActions = new List<Core.KeyManagement.UserKey.UpdateEncryptedDataForKeyRotation>();
+        await Assert.ThrowsAsync<InvalidOperationException>(async () => await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions));
+    }
+
+    [Theory, BitAutoData]
+    public async Task UpdateAccountKeys_ThrowsIfIsV1UserAndPrivateKeyIsWrappedWithNotAesCbcHmacAsync(SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, RotateUserAccountKeysData model)
+    {
+        user.PrivateKey = "2.xxx";
+        sutProvider.GetDependency<IUserSignatureKeyPairRepository>()
+            .GetByUserIdAsync(user.Id)
+            .ReturnsNull();
+        user.PublicKey = "public";
+        model.UserKeyEncryptedAccountPrivateKey = "7.xxx";
+        model.AccountPublicKey = "public";
+        model.AccountKeys.PublicKeyEncryptionKeyPairData = null;
+        model.AccountKeys.SignatureKeyPairData = null;
+        var saveEncryptedDataActions = new List<Core.KeyManagement.UserKey.UpdateEncryptedDataForKeyRotation>();
+        await Assert.ThrowsAsync<InvalidOperationException>(async () => await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions));
+    }
+
+    [Theory, BitAutoData]
+    public async Task UpdateAccountKeys_SuccessAsync(SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, RotateUserAccountKeysData model)
+    {
+        user.PrivateKey = "2.xxx";
+        sutProvider.GetDependency<IUserSignatureKeyPairRepository>()
+            .GetByUserIdAsync(user.Id)
+            .Returns(new SignatureKeyPairData(SignatureAlgorithm.Ed25519, "7.xxx", "public"));
+        user.PublicKey = "public";
+        model.AccountKeys.PublicKeyEncryptionKeyPairData.PublicKey = "public";
+        model.UserKeyEncryptedAccountPrivateKey = "2.xxx";
+        var saveEncryptedDataActions = new List<Core.KeyManagement.UserKey.UpdateEncryptedDataForKeyRotation>();
+        await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions);
+        Assert.NotEmpty(saveEncryptedDataActions);
+    }
+
+
+    [Theory, BitAutoData]
+    public void ValidateRotationModelSignatureKeyPairForv1UserAndUpgradeToV2_NoSignedPublicKeyThrows(SutProvider<RotateUserAccountKeysCommand> sutProvider, User user, RotateUserAccountKeysData model)
+    {
+        model.AccountKeys.SignatureKeyPairData = new SignatureKeyPairData(SignatureAlgorithm.Ed25519, "signingKey", "verifyingKey");
+        model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey = null;
+        var encryptedDataActions = new List<Core.KeyManagement.UserKey.UpdateEncryptedDataForKeyRotation>();
+        var excepction = Assert.Throws<InvalidOperationException>(() => sutProvider.Sut.ValidateRotationModelSignatureKeyPairForV1UserAndUpgradeToV2(model, user, encryptedDataActions));
+        Assert.Equal("The provided public key encryption key pair data does not contain a valid signed public key.", excepction.Message);
+    }
 }