diff --git a/src/Core/Services/Fido2AuthenticatorService.cs b/src/Core/Services/Fido2AuthenticatorService.cs index db535173f..a48f0cfd7 100644 --- a/src/Core/Services/Fido2AuthenticatorService.cs +++ b/src/Core/Services/Fido2AuthenticatorService.cs @@ -56,13 +56,14 @@ namespace Bit.Core.Services }); var cipherId = response.CipherId; + var userVerified = response.UserVerified; string credentialId; - // if (cipherId === undefined) { - // this.logService?.warning( - // `[Fido2Authenticator] Aborting because user confirmation was not recieved.`, - // ); - // throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.NotAllowed); - // } + if (cipherId == null) { + _logService.Info( + "[Fido2Authenticator] Aborting because user confirmation was not recieved." + ); + throw new NotAllowedError(); + } try { var (publicKey, privateKey) = await _cryptoFunctionService.EcdsaGenerateKeyPairAsync(CryptoEcdsaAlgorithm.P256Sha256); @@ -71,15 +72,12 @@ namespace Bit.Core.Services var encrypted = await _cipherService.GetAsync(cipherId); var cipher = await encrypted.DecryptAsync(); - // if ( - // !userVerified && - // (params.requireUserVerification || cipher.reprompt !== CipherRepromptType.None) - // ) { - // this.logService?.warning( - // `[Fido2Authenticator] Aborting because user verification was unsuccessful.`, - // ); - // throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.NotAllowed); - // } + if (!userVerified && (makeCredentialParams.RequireUserVerification || cipher.Reprompt != CipherRepromptType.None)) { + _logService.Info( + "[Fido2Authenticator] Aborting because user verification was unsuccessful." + ); + throw new NotAllowedError(); + } cipher.Login.Fido2Credentials = [fido2Credential]; var reencrypted = await _cipherService.EncryptAsync(cipher); diff --git a/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs b/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs index 608189376..5a86cad2a 100644 --- a/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs +++ b/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs @@ -21,8 +21,22 @@ using NSubstitute.Extensions; namespace Bit.Core.Test.Services { - public class Fido2AuthenticatorMakeCredentialTests + public class Fido2AuthenticatorMakeCredentialTests : IDisposable { + private Cipher _encryptedCipher; + + public Fido2AuthenticatorMakeCredentialTests() { + var cryptoServiceMock = Substitute.For(); + ServiceContainer.Register(typeof(CryptoService), cryptoServiceMock); + + _encryptedCipher = CreateCipher(); + } + + public void Dispose() + { + ServiceContainer.Reset(); + } + #region invalid input parameters // Spec: Check if at least one of the specified combinations of PublicKeyCredentialType and cryptographic parameters in credTypesAndPubKeyAlgs is supported. If not, return an error code equivalent to "NotSupportedError" and terminate the operation. @@ -171,12 +185,16 @@ namespace Bit.Core.Test.Services sutProvider.GetDependency().EcdsaGenerateKeyPairAsync(Arg.Any()) .Returns((RandomBytes(32), RandomBytes(32))); sutProvider.GetDependency().GetAllDecryptedAsync().Returns(ciphers); + sutProvider.GetDependency().ConfirmNewCredentialAsync(Arg.Any()).Returns(new Fido2ConfirmNewCredentialResult { + CipherId = null, + UserVerified = false + }); // Arrange mParams.RequireUserVerification = true; // Act - await sutProvider.Sut.MakeCredentialAsync(mParams); + await Assert.ThrowsAsync(() => sutProvider.Sut.MakeCredentialAsync(mParams)); // Assert await sutProvider.GetDependency().Received().ConfirmNewCredentialAsync(Arg.Is( @@ -209,7 +227,7 @@ namespace Bit.Core.Test.Services mParams.RequireUserVerification = false; // Act - await sutProvider.Sut.MakeCredentialAsync(mParams); + await Assert.ThrowsAsync(() => sutProvider.Sut.MakeCredentialAsync(mParams)); // Assert await sutProvider.GetDependency().Received().ConfirmNewCredentialAsync(Arg.Is( @@ -219,7 +237,7 @@ namespace Bit.Core.Test.Services [Theory] [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })] - public async Task MakeCredentialAsync_RequestsUserVerification_RequestConfirmedByUser(SutProvider sutProvider, Fido2AuthenticatorMakeCredentialParams mParams, Cipher encryptedCipher) + public async Task MakeCredentialAsync_RequestsUserVerification_RequestConfirmedByUser(SutProvider sutProvider, Fido2AuthenticatorMakeCredentialParams mParams) { // Common Arrange mParams.CredTypesAndPubKeyAlgs = [ @@ -232,17 +250,15 @@ namespace Bit.Core.Test.Services mParams.RequireUserVerification = false; sutProvider.GetDependency().EcdsaGenerateKeyPairAsync(Arg.Any()) .Returns((RandomBytes(32), RandomBytes(32))); - var cryptoServiceMock = Substitute.For(); - ServiceContainer.Register(typeof(CryptoService), cryptoServiceMock); - encryptedCipher.Key = null; - encryptedCipher.Attachments = []; + _encryptedCipher.Key = null; + _encryptedCipher.Attachments = []; // Arrange mParams.RequireResidentKey = false; - sutProvider.GetDependency().EncryptAsync(Arg.Any()).Returns(encryptedCipher); - sutProvider.GetDependency().GetAsync(Arg.Is(encryptedCipher.Id)).Returns(encryptedCipher); + sutProvider.GetDependency().EncryptAsync(Arg.Any()).Returns(_encryptedCipher); + sutProvider.GetDependency().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher); sutProvider.GetDependency().ConfirmNewCredentialAsync(Arg.Any()).Returns(new Fido2ConfirmNewCredentialResult { - CipherId = encryptedCipher.Id, + CipherId = _encryptedCipher.Id, UserVerified = false }); @@ -263,8 +279,106 @@ namespace Bit.Core.Test.Services // c.Login.MainFido2Credential.UserDisplayName == mParams.UserEntity.DisplayName && c.Login.MainFido2Credential.DiscoverableValue == false )); - await sutProvider.GetDependency().Received().SaveWithServerAsync(encryptedCipher); + await sutProvider.GetDependency().Received().SaveWithServerAsync(_encryptedCipher); } + + [Theory] + [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })] + // Spec: If the user does not consent or if user verification fails, return an error code equivalent to "NotAllowedError" and terminate the operation. + public async Task MakeCredentialAsync_ThrowsNotAllowed_RequestNotConfirmedByUser(SutProvider sutProvider, Fido2AuthenticatorMakeCredentialParams mParams) + { + // Common Arrange + mParams.CredTypesAndPubKeyAlgs = [ + new PublicKeyCredentialAlgorithmDescriptor { + Type = "public-key", + Algorithm = -7 // ES256 + } + ]; + mParams.RpEntity = new PublicKeyCredentialRpEntity { Id = "bitwarden.com" }; + mParams.RequireUserVerification = false; + sutProvider.GetDependency().EcdsaGenerateKeyPairAsync(Arg.Any()) + .Returns((RandomBytes(32), RandomBytes(32))); + + // Arrange + sutProvider.GetDependency().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher); + sutProvider.GetDependency().ConfirmNewCredentialAsync(Arg.Any()).Returns(new Fido2ConfirmNewCredentialResult { + CipherId = null, + UserVerified = false + }); + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.MakeCredentialAsync(mParams)); + } + + [Theory] + [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })] + public async Task MakeCredentialAsync_ThrowsNotAllowed_NoUserVerificationWhenRequiredByParams(SutProvider sutProvider, Fido2AuthenticatorMakeCredentialParams mParams) + { + // Common Arrange + mParams.CredTypesAndPubKeyAlgs = [ + new PublicKeyCredentialAlgorithmDescriptor { + Type = "public-key", + Algorithm = -7 // ES256 + } + ]; + mParams.RpEntity = new PublicKeyCredentialRpEntity { Id = "bitwarden.com" }; + mParams.RequireUserVerification = true; + sutProvider.GetDependency().EcdsaGenerateKeyPairAsync(Arg.Any()) + .Returns((RandomBytes(32), RandomBytes(32))); + + // Arrange + sutProvider.GetDependency().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher); + sutProvider.GetDependency().ConfirmNewCredentialAsync(Arg.Any()).Returns(new Fido2ConfirmNewCredentialResult { + CipherId = _encryptedCipher.Id, + UserVerified = false + }); + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.MakeCredentialAsync(mParams)); + } + + [Theory] + [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })] + public async Task MakeCredentialAsync_ThrowsNotAllowed_NoUserVerificationForCipherWithReprompt(SutProvider sutProvider, Fido2AuthenticatorMakeCredentialParams mParams) + { + // Common Arrange + mParams.CredTypesAndPubKeyAlgs = [ + new PublicKeyCredentialAlgorithmDescriptor { + Type = "public-key", + Algorithm = -7 // ES256 + } + ]; + mParams.RpEntity = new PublicKeyCredentialRpEntity { Id = "bitwarden.com" }; + mParams.RequireUserVerification = false; + sutProvider.GetDependency().EcdsaGenerateKeyPairAsync(Arg.Any()) + .Returns((RandomBytes(32), RandomBytes(32))); + _encryptedCipher.Reprompt = CipherRepromptType.Password; + + // Arrange + sutProvider.GetDependency().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher); + sutProvider.GetDependency().ConfirmNewCredentialAsync(Arg.Any()).Returns(new Fido2ConfirmNewCredentialResult { + CipherId = _encryptedCipher.Id, + UserVerified = false + }); + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.MakeCredentialAsync(mParams)); + } + + // /** Spec: If any error occurred while creating the new credential object, return an error code equivalent to "UnknownError" and terminate the operation. */ + // it("should throw unkown error if creation fails", async () => { + // const _encryptedCipher = Symbol(); + // userInterfaceSession.confirmNewCredential.mockResolvedValue({ + // cipherId: existingCipher.id, + // userVerified: false, + // }); + // cipherService.encrypt.mockResolvedValue(_encryptedCipher as unknown as Cipher); + // cipherService.updateWithServer.mockRejectedValue(new Error("Internal error")); + + // const result = async () => await authenticator.makeCredential(params, tab); + + // await expect(result).rejects.toThrowError(Fido2AuthenticatorErrorCode.Unknown); + // }); #endregion @@ -294,5 +408,14 @@ namespace Bit.Core.Test.Services } }; } + + private Cipher CreateCipher() + { + return new Cipher { + Id = Guid.NewGuid().ToString(), + Type = CipherType.Login, + Login = new Login {} + }; + } } }