[Webauthn] Handle duplicate registrations with InvalidStateError

This implements the WebauthN spec change that permits returning an immediate
and specific error if the user consented to use a key but it was already
registered with the relying party. Formerly, we had to wait for a timeout
and return the generic "NotAllowedError". This way, RPs can let the user
know the reason that registration failed.

Bug: 814870
Change-Id: Ib6f0c9cdd5ca7e3f545c8ce6f3c8e641d672783b
Reviewed-on: https://chromium-review.googlesource.com/984725Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546976}

components/password_manager/content/common/credential_manager_mojom_traits.cc

@@ -90,6 +90,7 @@ bool EnumTraits<password_manager::mojom::CredentialManagerError,
     case password_manager::mojom::CredentialManagerError::NOT_ALLOWED:
     case password_manager::mojom::CredentialManagerError::NOT_SUPPORTED:
     case password_manager::mojom::CredentialManagerError::INVALID_DOMAIN:
+    case password_manager::mojom::CredentialManagerError::INVALID_STATE:
     case password_manager::mojom::CredentialManagerError::NOT_IMPLEMENTED:
     case password_manager::mojom::CredentialManagerError::UNKNOWN:
       *output = password_manager::CredentialManagerError::UNKNOWN;

content/browser/webauth/authenticator_impl.cc

@@ -545,7 +545,7 @@ void AuthenticatorImpl::OnRegisterResponse(
   DCHECK(u2f_request_) << "unsupported callback hairpin";
 
   switch (status_code) {
-    case device::FidoReturnCode::kConditionsNotSatisfied:
+    case device::FidoReturnCode::kInvalidState:
       // Duplicate registration: the new credential would be created on an
       // authenticator that already contains one of the credentials in
       // |exclude_credentials|.
@@ -560,6 +560,7 @@ void AuthenticatorImpl::OnRegisterResponse(
           webauth::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR, nullptr);
       return;
     case device::FidoReturnCode::kInvalidParams:
+    case device::FidoReturnCode::kConditionsNotSatisfied:
       NOTREACHED();
       return;
     case device::FidoReturnCode::kSuccess:
@@ -653,6 +654,7 @@ void AuthenticatorImpl::OnSignResponse(
           webauth::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR, nullptr);
       return;
     case device::FidoReturnCode::kInvalidParams:
+    case device::FidoReturnCode::kInvalidState:
       NOTREACHED();
       return;
     case device::FidoReturnCode::kSuccess:

device/fido/fido_constants.h

@@ -20,6 +20,7 @@ enum class FidoReturnCode : uint8_t {
   kFailure,
   kInvalidParams,
   kConditionsNotSatisfied,
+  kInvalidState,
 };
 
 enum class ProtocolVersion {

device/fido/u2f_register.cc

@@ -142,7 +142,7 @@ void U2fRegister::OnTryDevice(bool is_duplicate_registration,
       state_ = State::COMPLETE;
       if (is_duplicate_registration) {
         std::move(completion_callback_)
-            .Run(FidoReturnCode::kConditionsNotSatisfied, base::nullopt);
+            .Run(FidoReturnCode::kInvalidState, base::nullopt);
         break;
       }
       auto response =

device/fido/u2f_register_unittest.cc

@@ -645,7 +645,7 @@ TEST_F(U2fRegisterTest, TestSingleDeviceRegistrationWithDuplicateHandle) {
   discovery()->AddDevice(std::move(device));
 
   register_callback_receiver().WaitForCallback();
-  EXPECT_EQ(FidoReturnCode::kConditionsNotSatisfied,
+  EXPECT_EQ(FidoReturnCode::kInvalidState,
             register_callback_receiver().status());
   EXPECT_FALSE(register_callback_receiver().value());
 }
@@ -697,7 +697,7 @@ TEST_F(U2fRegisterTest, TestMultipleDeviceRegistrationWithDuplicateHandle) {
   discovery()->WaitForCallToStartAndSimulateSuccess();
 
   register_callback_receiver().WaitForCallback();
-  EXPECT_EQ(FidoReturnCode::kConditionsNotSatisfied,
+  EXPECT_EQ(FidoReturnCode::kInvalidState,
             register_callback_receiver().status());
   EXPECT_FALSE(register_callback_receiver().value());
 }

third_party/WebKit/LayoutTests/http/tests/credentialmanager/credentialscontainer-create-basics.html

@@ -124,6 +124,13 @@ promise_test(t => {
       navigator.credentials.create({ publicKey : MAKE_CREDENTIAL_OPTIONS}));
 }, "Verify that not supported error returned by mock is properly handled.");
 
+promise_test(t => {
+  mockAuthenticator.setAuthenticatorStatus(
+      webauth.mojom.AuthenticatorStatus.INVALID_STATE);
+  return promise_rejects(t, "InvalidStateError",
+    navigator.credentials.create({ publicKey : MAKE_CREDENTIAL_OPTIONS}));
+}, "Verify that InvalidState (duplicate registration) returned by mock is properly handled.");
+
 promise_test(_ => {
   mockAuthenticator.reset();
   mockAuthenticator.setDefaultsForSuccessfulMakeCredential();

third_party/WebKit/Source/modules/credentialmanager/CredentialManagerTypeConverters.cpp

@@ -118,6 +118,8 @@ TypeConverter<CredentialManagerError, AuthenticatorStatus>::Convert(
       return CredentialManagerError::PENDING_REQUEST;
     case webauth::mojom::blink::AuthenticatorStatus::INVALID_DOMAIN:
       return CredentialManagerError::INVALID_DOMAIN;
+    case webauth::mojom::blink::AuthenticatorStatus::INVALID_STATE:
+      return CredentialManagerError::INVALID_STATE;
     case webauth::mojom::blink::AuthenticatorStatus::NOT_IMPLEMENTED:
       return CredentialManagerError::NOT_IMPLEMENTED;
     case webauth::mojom::blink::AuthenticatorStatus::SUCCESS:

third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp

@@ -256,6 +256,10 @@ DOMException* CredentialManagerErrorToDOMException(
           "Parameters for this operation are not supported.");
     case CredentialManagerError::INVALID_DOMAIN:
       return DOMException::Create(kSecurityError, "This is an invalid domain.");
+    case CredentialManagerError::INVALID_STATE:
+      return DOMException::Create(
+          kInvalidStateError,
+          "Attempting to register an already-registered key.");
     case CredentialManagerError::NOT_IMPLEMENTED:
       return DOMException::Create(kNotSupportedError, "Not implemented");
     case CredentialManagerError::UNKNOWN:

third_party/WebKit/public/platform/modules/credentialmanager/credential_manager.mojom

@@ -27,6 +27,7 @@ enum CredentialManagerError {
   NOT_ALLOWED,
   NOT_SUPPORTED,
   INVALID_DOMAIN,
+  INVALID_STATE,
   NOT_IMPLEMENTED,
   UNKNOWN
 };

third_party/WebKit/public/platform/modules/webauth/authenticator.mojom

@@ -18,6 +18,7 @@ enum AuthenticatorStatus {
   NOT_ALLOWED_ERROR,
   NOT_SUPPORTED_ERROR,
   INVALID_DOMAIN,
+  INVALID_STATE,
   NOT_IMPLEMENTED,
   UNKNOWN_ERROR,
 };