From 8bbdd7fe3091cad539cc84ad11c5cbb660df1f29 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Mon, 9 Jun 2025 14:14:09 +0200 Subject: [PATCH] Improve tests --- .../RotateUserAccountkeysCommand.cs | 20 +- .../UserSignatureKeyPairRepository.cs | 2 +- .../AccountsKeyManagementControllerTests.cs | 196 ++++++++++++++++++ .../RotateUserAccountKeysCommandTests.cs | 73 +++++++ 4 files changed, 277 insertions(+), 14 deletions(-) diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs index 7041084943..dea3436435 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountkeysCommand.cs +++ b/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 saveEncryptedDataActions) + public void ValidateRotationModelSignatureKeyPairForV1UserAndUpgradeToV2(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) { if (model.AccountKeys.SignatureKeyPairData != null) { - // user is upgrading - if (string.IsNullOrEmpty(model.AccountKeys.SignatureKeyPairData.VerifyingKey)) + if (string.IsNullOrEmpty(model.AccountKeys.PublicKeyEncryptionKeyPairData?.SignedPublicKey)) { - throw new InvalidOperationException("The provided signing key data does not contain a valid verifying key."); - } - - if (string.IsNullOrEmpty(model.AccountKeys.SignatureKeyPairData.WrappedSigningKey)) - { - throw new InvalidOperationException("The provided signing key data does not contain a valid wrapped signing key."); + throw new InvalidOperationException("The provided public key encryption key pair data does not contain a valid signed public key."); } saveEncryptedDataActions.Add(_userSignatureKeyPairRepository.SetUserSignatureKeyPair(user.Id, model.AccountKeys.SignatureKeyPairData)); user.SignedPublicKey = model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey; } } - async Task UpdateAccountKeys(RotateUserAccountKeysData model, User user, List saveEncryptedDataActions) + public 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 - 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"); } diff --git a/src/Infrastructure.EntityFramework/KeyManagement/Repositories/UserSignatureKeyPairRepository.cs b/src/Infrastructure.EntityFramework/KeyManagement/Repositories/UserSignatureKeyPairRepository.cs index 0dfb7e6ac1..57df01cde9 100644 --- a/src/Infrastructure.EntityFramework/KeyManagement/Repositories/UserSignatureKeyPairRepository.cs +++ b/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; diff --git a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs b/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs index bf27d7f0d1..7130b85507 100644 --- a/test/Api.IntegrationTest/KeyManagement/Controllers/AccountsKeyManagementControllerTests.cs +++ b/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 _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(); _passwordHasher = _factory.GetService>(); _organizationRepository = _factory.GetService(); + _userSignatureKeyPairRepository = _factory.GetService(); } public async Task InitializeAsync() @@ -200,6 +206,7 @@ public class AccountsKeyManagementControllerTests : IClassFixture sutProvider, User user, RotateUserAccountKeysData model) + { + user.PrivateKey = "2.xxx"; + sutProvider.GetDependency() + .GetByUserIdAsync(user.Id) + .ReturnsNull(); + user.PublicKey = "old-public"; + model.AccountKeys.PublicKeyEncryptionKeyPairData.PublicKey = "new-public"; + var saveEncryptedDataActions = new List(); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions)); + } + + [Theory, BitAutoData] + public async Task UpdateAccountKeys_ThrowsIfPrivateKeyWrappedWithNotXchacha20ForV2UserAsync(SutProvider sutProvider, User user, RotateUserAccountKeysData model) + { + user.PrivateKey = "7.xxx"; + sutProvider.GetDependency() + .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(); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions)); + } + + [Theory, BitAutoData] + public async Task UpdateAccountKeys_ThrowsIfIsV1UserAndPrivateKeyIsWrappedWithNotAesCbcHmacAsync(SutProvider sutProvider, User user, RotateUserAccountKeysData model) + { + user.PrivateKey = "2.xxx"; + sutProvider.GetDependency() + .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(); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions)); + } + + [Theory, BitAutoData] + public async Task UpdateAccountKeys_SuccessAsync(SutProvider sutProvider, User user, RotateUserAccountKeysData model) + { + user.PrivateKey = "2.xxx"; + sutProvider.GetDependency() + .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(); + await sutProvider.Sut.UpdateAccountKeys(model, user, saveEncryptedDataActions); + Assert.NotEmpty(saveEncryptedDataActions); + } + + + [Theory, BitAutoData] + public void ValidateRotationModelSignatureKeyPairForv1UserAndUpgradeToV2_NoSignedPublicKeyThrows(SutProvider sutProvider, User user, RotateUserAccountKeysData model) + { + model.AccountKeys.SignatureKeyPairData = new SignatureKeyPairData(SignatureAlgorithm.Ed25519, "signingKey", "verifyingKey"); + model.AccountKeys.PublicKeyEncryptionKeyPairData.SignedPublicKey = null; + var encryptedDataActions = new List(); + var excepction = Assert.Throws(() => 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); + } }