diff --git a/src/Core/Auth/UserFeatures/TwoFactorAuth/IDeleteTwoFactorWebAuthnCredentialCommand.cs b/src/Core/Auth/UserFeatures/TwoFactorAuth/IDeleteTwoFactorWebAuthnCredentialCommand.cs index 61774eb69e..4ae092635d 100644 --- a/src/Core/Auth/UserFeatures/TwoFactorAuth/IDeleteTwoFactorWebAuthnCredentialCommand.cs +++ b/src/Core/Auth/UserFeatures/TwoFactorAuth/IDeleteTwoFactorWebAuthnCredentialCommand.cs @@ -1,15 +1,18 @@ using Bit.Core.Entities; +using Bit.Core.Services; namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth; public interface IDeleteTwoFactorWebAuthnCredentialCommand { /// - /// Deletes a Two-factor WebAuthn credential by ID. + /// Deletes a single Two-factor WebAuthn credential by ID ("Key{id}"). /// /// The current user. - /// ID of the credential to delete. + /// The ID of the credential to delete ("Key{id}"). /// Whether deletion was successful. + /// Will not delete the last registered credential for a user. To delete the last (or single) + /// registered credential, use Task DeleteTwoFactorWebAuthnCredentialAsync(User user, int id); } diff --git a/src/Core/Auth/UserFeatures/TwoFactorAuth/Implementations/DeleteTwoFactorWebAuthnCredentialCommand.cs b/src/Core/Auth/UserFeatures/TwoFactorAuth/Implementations/DeleteTwoFactorWebAuthnCredentialCommand.cs index 0b572d04ed..8de1e3fde0 100644 --- a/src/Core/Auth/UserFeatures/TwoFactorAuth/Implementations/DeleteTwoFactorWebAuthnCredentialCommand.cs +++ b/src/Core/Auth/UserFeatures/TwoFactorAuth/Implementations/DeleteTwoFactorWebAuthnCredentialCommand.cs @@ -22,7 +22,7 @@ public class DeleteTwoFactorWebAuthnCredentialCommand : IDeleteTwoFactorWebAuthn var keyName = $"Key{id}"; var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); - if (!provider?.MetaData?.ContainsKey(keyName) ?? true) + if (provider?.MetaData == null || !provider.MetaData.ContainsKey(keyName)) { return false; } diff --git a/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/DeleteTwoFactorWebAuthnCredentialCommandTests.cs b/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/DeleteTwoFactorWebAuthnCredentialCommandTests.cs new file mode 100644 index 0000000000..b1dfe46573 --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/DeleteTwoFactorWebAuthnCredentialCommandTests.cs @@ -0,0 +1,138 @@ +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Implementations; +using Bit.Core.Entities; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Fido2NetLib.Objects; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Auth.UserFeatures.TwoFactorAuth; + +[SutProviderCustomize] +public class DeleteTwoFactorWebAuthnCredentialCommandTests +{ + private static void SetupWebAuthnProvider(User user, int credentialCount) + { + var providers = new Dictionary(); + var metadata = new Dictionary(); + + // Add credentials as Key1, Key2, Key3, etc. + for (var i = 1; i <= credentialCount; i++) + { + metadata[$"Key{i}"] = new TwoFactorProvider.WebAuthnData + { + Name = $"Key {i}", + Descriptor = new PublicKeyCredentialDescriptor([(byte)i]), + PublicKey = [(byte)i], + UserHandle = [(byte)i], + SignatureCounter = 0, + CredType = "public-key", + RegDate = DateTime.UtcNow, + AaGuid = Guid.NewGuid() + }; + } + + providers[TwoFactorProviderType.WebAuthn] = new TwoFactorProvider { Enabled = true, MetaData = metadata }; + + user.SetTwoFactorProviders(providers); + } + + /// + /// When the user has multiple WebAuthn credentials and requests deletion of an existing key, + /// the command should remove it, persist via UserService, and return true. + /// + [Theory, BitAutoData] + public async Task DeleteAsync_KeyExistsWithMultipleKeys_RemovesKeyAndReturnsTrue( + SutProvider sutProvider, User user) + { + // Arrange + SetupWebAuthnProvider(user, 3); + var keyIdToDelete = 2; + + // Act + var result = await sutProvider.Sut.DeleteTwoFactorWebAuthnCredentialAsync(user, keyIdToDelete); + + // Assert + Assert.True(result); + + var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); + Assert.NotNull(provider?.MetaData); + Assert.False(provider.MetaData.ContainsKey($"Key{keyIdToDelete}")); + Assert.Equal(2, provider.MetaData.Count); + + await sutProvider.GetDependency().Received(1) + .UpdateTwoFactorProviderAsync(user, TwoFactorProviderType.WebAuthn); + } + + /// + /// When the requested key does not exist, the command should return false + /// and not call UserService. + /// + [Theory, BitAutoData] + public async Task DeleteAsync_KeyDoesNotExist_ReturnsFalse( + SutProvider sutProvider, User user) + { + // Arrange + SetupWebAuthnProvider(user, 2); + var nonExistentKeyId = 99; + + // Act + var result = await sutProvider.Sut.DeleteTwoFactorWebAuthnCredentialAsync(user, nonExistentKeyId); + + // Assert + Assert.False(result); + + await sutProvider.GetDependency().DidNotReceive() + .UpdateTwoFactorProviderAsync(Arg.Any(), Arg.Any()); + } + + /// + /// Users must retain at least one WebAuthn credential. When only one key remains, + /// deletion should be rejected to prevent lockout. + /// + [Theory, BitAutoData] + public async Task DeleteAsync_OnlyOneKeyRemaining_ReturnsFalse( + SutProvider sutProvider, User user) + { + // Arrange + SetupWebAuthnProvider(user, 1); + var keyIdToDelete = 1; + + // Act + var result = await sutProvider.Sut.DeleteTwoFactorWebAuthnCredentialAsync(user, keyIdToDelete); + + // Assert + Assert.False(result); + + // Key should still exist + var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); + Assert.NotNull(provider?.MetaData); + Assert.True(provider.MetaData.ContainsKey($"Key{keyIdToDelete}")); + + await sutProvider.GetDependency().DidNotReceive() + .UpdateTwoFactorProviderAsync(Arg.Any(), Arg.Any()); + } + + /// + /// When the user has no two-factor providers configured, deletion should return false. + /// + [Theory, BitAutoData] + public async Task DeleteAsync_NoProviders_ReturnsFalse( + SutProvider sutProvider, User user) + { + // Arrange - user with no providers (clear any AutoFixture-generated ones) + user.SetTwoFactorProviders(null); + + // Act + var result = await sutProvider.Sut.DeleteTwoFactorWebAuthnCredentialAsync(user, 1); + + // Assert + Assert.False(result); + + await sutProvider.GetDependency().DidNotReceive() + .UpdateTwoFactorProviderAsync(Arg.Any(), Arg.Any()); + } +}