[PM-5731] feat: add user verification checks

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) {
+            if (cipherId == null) {
-            //     this.logService?.warning(
+                _logService.Info(
-            //     `[Fido2Authenticator] Aborting because user confirmation was not recieved.`,
+                    "[Fido2Authenticator] Aborting because user confirmation was not recieved."
-            //     );
+                );
-            //     throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.NotAllowed);
+                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 (
+                if (!userVerified && (makeCredentialParams.RequireUserVerification || cipher.Reprompt != CipherRepromptType.None)) {
-                // !userVerified &&
+                    _logService.Info(
-                // (params.requireUserVerification || cipher.reprompt !== CipherRepromptType.None)
+                        "[Fido2Authenticator] Aborting because user verification was unsuccessful."
-                // ) {
+                    );
-                // this.logService?.warning(
+                    throw new NotAllowedError();
-                //     `[Fido2Authenticator] Aborting because user verification was unsuccessful.`,
+                }
-                // );
-                // throw new Fido2AuthenticatorError(Fido2AuthenticatorErrorCode.NotAllowed);
-                // }
 
                 cipher.Login.Fido2Credentials = [fido2Credential];
                 var reencrypted = await _cipherService.EncryptAsync(cipher);

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<ICryptoService>();
+            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<ICryptoFunctionService>().EcdsaGenerateKeyPairAsync(Arg.Any<CryptoEcdsaAlgorithm>())
                 .Returns((RandomBytes(32), RandomBytes(32)));
             sutProvider.GetDependency<ICipherService>().GetAllDecryptedAsync().Returns(ciphers);
+            sutProvider.GetDependency<IFido2UserInterface>().ConfirmNewCredentialAsync(Arg.Any<Fido2ConfirmNewCredentialParams>()).Returns(new Fido2ConfirmNewCredentialResult {
+                CipherId = null,
+                UserVerified = false
+            });
 
             // Arrange
             mParams.RequireUserVerification = true;
 
             // Act
-            await sutProvider.Sut.MakeCredentialAsync(mParams);
+            await Assert.ThrowsAsync<NotAllowedError>(() => sutProvider.Sut.MakeCredentialAsync(mParams));
 
             // Assert
             await sutProvider.GetDependency<IFido2UserInterface>().Received().ConfirmNewCredentialAsync(Arg.Is<Fido2ConfirmNewCredentialParams>(
@@ -209,7 +227,7 @@ namespace Bit.Core.Test.Services
             mParams.RequireUserVerification = false;
 
             // Act
-            await sutProvider.Sut.MakeCredentialAsync(mParams);
+            await Assert.ThrowsAsync<NotAllowedError>(() => sutProvider.Sut.MakeCredentialAsync(mParams));
 
             // Assert
             await sutProvider.GetDependency<IFido2UserInterface>().Received().ConfirmNewCredentialAsync(Arg.Is<Fido2ConfirmNewCredentialParams>(
@@ -219,7 +237,7 @@ namespace Bit.Core.Test.Services
 
         [Theory]
         [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })]
-        public async Task MakeCredentialAsync_RequestsUserVerification_RequestConfirmedByUser(SutProvider<Fido2AuthenticatorService> sutProvider, Fido2AuthenticatorMakeCredentialParams mParams, Cipher encryptedCipher)
+        public async Task MakeCredentialAsync_RequestsUserVerification_RequestConfirmedByUser(SutProvider<Fido2AuthenticatorService> sutProvider, Fido2AuthenticatorMakeCredentialParams mParams)
         {
             // Common Arrange
             mParams.CredTypesAndPubKeyAlgs = [
@@ -232,17 +250,15 @@ namespace Bit.Core.Test.Services
             mParams.RequireUserVerification = false;
             sutProvider.GetDependency<ICryptoFunctionService>().EcdsaGenerateKeyPairAsync(Arg.Any<CryptoEcdsaAlgorithm>())
                 .Returns((RandomBytes(32), RandomBytes(32)));
-            var cryptoServiceMock = Substitute.For<ICryptoService>();
+            _encryptedCipher.Key = null;
-            ServiceContainer.Register(typeof(CryptoService), cryptoServiceMock);
+            _encryptedCipher.Attachments = [];
-            encryptedCipher.Key = null;
-            encryptedCipher.Attachments = [];
 
             // Arrange
             mParams.RequireResidentKey = false;
-            sutProvider.GetDependency<ICipherService>().EncryptAsync(Arg.Any<CipherView>()).Returns(encryptedCipher);
+            sutProvider.GetDependency<ICipherService>().EncryptAsync(Arg.Any<CipherView>()).Returns(_encryptedCipher);
-            sutProvider.GetDependency<ICipherService>().GetAsync(Arg.Is(encryptedCipher.Id)).Returns(encryptedCipher);
+            sutProvider.GetDependency<ICipherService>().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher);
             sutProvider.GetDependency<IFido2UserInterface>().ConfirmNewCredentialAsync(Arg.Any<Fido2ConfirmNewCredentialParams>()).Returns(new Fido2ConfirmNewCredentialResult {
-                CipherId = encryptedCipher.Id,
+                CipherId = _encryptedCipher.Id,
                 UserVerified = false
             });
 
@@ -263,9 +279,107 @@ namespace Bit.Core.Test.Services
                     // c.Login.MainFido2Credential.UserDisplayName == mParams.UserEntity.DisplayName &&
                     c.Login.MainFido2Credential.DiscoverableValue == false
             ));
-            await sutProvider.GetDependency<ICipherService>().Received().SaveWithServerAsync(encryptedCipher);
+            await sutProvider.GetDependency<ICipherService>().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<Fido2AuthenticatorService> 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<ICryptoFunctionService>().EcdsaGenerateKeyPairAsync(Arg.Any<CryptoEcdsaAlgorithm>())
+                .Returns((RandomBytes(32), RandomBytes(32)));
+
+            // Arrange
+            sutProvider.GetDependency<ICipherService>().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher);
+            sutProvider.GetDependency<IFido2UserInterface>().ConfirmNewCredentialAsync(Arg.Any<Fido2ConfirmNewCredentialParams>()).Returns(new Fido2ConfirmNewCredentialResult {
+                CipherId = null,
+                UserVerified = false
+            });
+
+            // Act & Assert
+            await Assert.ThrowsAsync<NotAllowedError>(() => sutProvider.Sut.MakeCredentialAsync(mParams));
+        }
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })]
+        public async Task MakeCredentialAsync_ThrowsNotAllowed_NoUserVerificationWhenRequiredByParams(SutProvider<Fido2AuthenticatorService> 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<ICryptoFunctionService>().EcdsaGenerateKeyPairAsync(Arg.Any<CryptoEcdsaAlgorithm>())
+                .Returns((RandomBytes(32), RandomBytes(32)));
+
+            // Arrange
+            sutProvider.GetDependency<ICipherService>().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher);
+            sutProvider.GetDependency<IFido2UserInterface>().ConfirmNewCredentialAsync(Arg.Any<Fido2ConfirmNewCredentialParams>()).Returns(new Fido2ConfirmNewCredentialResult {
+                CipherId = _encryptedCipher.Id,
+                UserVerified = false
+            });
+
+            // Act & Assert
+            await Assert.ThrowsAsync<NotAllowedError>(() => sutProvider.Sut.MakeCredentialAsync(mParams));
+        }
+
+        [Theory]
+        [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })]
+        public async Task MakeCredentialAsync_ThrowsNotAllowed_NoUserVerificationForCipherWithReprompt(SutProvider<Fido2AuthenticatorService> 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<ICryptoFunctionService>().EcdsaGenerateKeyPairAsync(Arg.Any<CryptoEcdsaAlgorithm>())
+                .Returns((RandomBytes(32), RandomBytes(32)));
+            _encryptedCipher.Reprompt = CipherRepromptType.Password;
+
+            // Arrange
+            sutProvider.GetDependency<ICipherService>().GetAsync(Arg.Is(_encryptedCipher.Id)).Returns(_encryptedCipher);
+            sutProvider.GetDependency<IFido2UserInterface>().ConfirmNewCredentialAsync(Arg.Any<Fido2ConfirmNewCredentialParams>()).Returns(new Fido2ConfirmNewCredentialResult {
+                CipherId = _encryptedCipher.Id,
+                UserVerified = false
+            });
+
+            // Act & Assert
+            await Assert.ThrowsAsync<NotAllowedError>(() => 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
 
         private byte[] RandomBytes(int length)
@@ -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 {}
+            };
+        }
     }
 }