webauth: clean up AuthenticatorCommon::SignalFailureToRequestDelegate()

This removes a giant switch on FidoReturnCode that's better handled
higher up the stack. Also resolve a stale TODO while we're at it.

Bug: 876109
Change-Id: I84bd8aa0a0c73e0a59dad7a673f041ded6019e4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764760Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691737}

content/browser/webauth/authenticator_common.cc

@@ -1014,8 +1014,10 @@ void AuthenticatorCommon::OnRegisterResponse(
       // authenticator that already contains one of the credentials in
       // |exclude_credentials|.
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kKeyAlreadyRegistered);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kKeyAlreadyRegistered,
+          blink::mojom::AuthenticatorStatus::CREDENTIAL_EXCLUDED);
       return;
     case device::FidoReturnCode::kAuthenticatorResponseInvalid:
       // The response from the authenticator was corrupted.
@@ -1025,41 +1027,53 @@ void AuthenticatorCommon::OnRegisterResponse(
           Focus::kDoCheck);
       return;
     case device::FidoReturnCode::kUserConsentButCredentialNotRecognized:
-      // TODO(crbug/876109): This isn't strictly unreachable.
-      NOTREACHED();
+      NOTREACHED() << "This should only be reachable for assertions";
+      InvokeCallbackAndCleanup(
+          std::move(make_credential_response_callback_),
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR, nullptr,
+          Focus::kDoCheck);
       return;
     case device::FidoReturnCode::kUserConsentDenied:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kUserConsentDenied);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kUserConsentDenied,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kSoftPINBlock:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kSoftPINBlock);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kSoftPINBlock,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kHardPINBlock:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kHardPINBlock);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kHardPINBlock,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorRemovedDuringPINEntry:
       SignalFailureToRequestDelegate(
           authenticator,
           AuthenticatorRequestClientDelegate::InterestingFailureReason::
-              kAuthenticatorRemovedDuringPINEntry);
+              kAuthenticatorRemovedDuringPINEntry,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorMissingResidentKeys:
       SignalFailureToRequestDelegate(
           authenticator,
           AuthenticatorRequestClientDelegate::InterestingFailureReason::
-              kAuthenticatorMissingResidentKeys);
+              kAuthenticatorMissingResidentKeys,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorMissingUserVerification:
       SignalFailureToRequestDelegate(
           authenticator,
           AuthenticatorRequestClientDelegate::InterestingFailureReason::
-              kAuthenticatorMissingUserVerification);
+              kAuthenticatorMissingUserVerification,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorMissingCredentialManagement:
       NOTREACHED()
@@ -1078,8 +1092,10 @@ void AuthenticatorCommon::OnRegisterResponse(
       return;
     case device::FidoReturnCode::kStorageFull:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kStorageFull);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kStorageFull,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kSuccess:
       DCHECK(response_data.has_value());
@@ -1226,8 +1242,10 @@ void AuthenticatorCommon::OnSignResponse(
   switch (status_code) {
     case device::FidoReturnCode::kUserConsentButCredentialNotRecognized:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kKeyNotRegistered);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kKeyNotRegistered,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorResponseInvalid:
       // The response from the authenticator was corrupted.
@@ -1236,41 +1254,52 @@ void AuthenticatorCommon::OnSignResponse(
           blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kUserConsentButCredentialExcluded:
-      // TODO(crbug/876109): This isn't strictly unreachable.
-      NOTREACHED();
+      NOTREACHED() << "This should only be reachable for registrations";
+      InvokeCallbackAndCleanup(
+          std::move(get_assertion_response_callback_),
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kUserConsentDenied:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kUserConsentDenied);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kUserConsentDenied,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kSoftPINBlock:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kSoftPINBlock);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kSoftPINBlock,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kHardPINBlock:
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kHardPINBlock);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kHardPINBlock,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorRemovedDuringPINEntry:
       SignalFailureToRequestDelegate(
           authenticator,
           AuthenticatorRequestClientDelegate::InterestingFailureReason::
-              kAuthenticatorRemovedDuringPINEntry);
+              kAuthenticatorRemovedDuringPINEntry,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorMissingResidentKeys:
       SignalFailureToRequestDelegate(
           authenticator,
           AuthenticatorRequestClientDelegate::InterestingFailureReason::
-              kAuthenticatorMissingResidentKeys);
+              kAuthenticatorMissingResidentKeys,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorMissingUserVerification:
       SignalFailureToRequestDelegate(
           authenticator,
           AuthenticatorRequestClientDelegate::InterestingFailureReason::
-              kAuthenticatorMissingUserVerification);
+              kAuthenticatorMissingUserVerification,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kAuthenticatorMissingCredentialManagement:
       NOTREACHED()
@@ -1288,8 +1317,10 @@ void AuthenticatorCommon::OnSignResponse(
     case device::FidoReturnCode::kStorageFull:
       NOTREACHED() << "Should not be possible for assertions.";
       SignalFailureToRequestDelegate(
-          authenticator, AuthenticatorRequestClientDelegate::
-                             InterestingFailureReason::kStorageFull);
+          authenticator,
+          AuthenticatorRequestClientDelegate::InterestingFailureReason::
+              kStorageFull,
+          blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
       return;
     case device::FidoReturnCode::kSuccess:
       DCHECK(response_data.has_value());
@@ -1338,52 +1369,8 @@ void AuthenticatorCommon::OnAccountSelected(
 
 void AuthenticatorCommon::SignalFailureToRequestDelegate(
     const ::device::FidoAuthenticator* authenticator,
-    AuthenticatorRequestClientDelegate::InterestingFailureReason reason) {
-  blink::mojom::AuthenticatorStatus status =
-      blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-
-  switch (reason) {
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kKeyAlreadyRegistered:
-      status = blink::mojom::AuthenticatorStatus::CREDENTIAL_EXCLUDED;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kKeyNotRegistered:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::kTimeout:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kSoftPINBlock:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kHardPINBlock:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kAuthenticatorRemovedDuringPINEntry:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kAuthenticatorMissingResidentKeys:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kAuthenticatorMissingUserVerification:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kStorageFull:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-    case AuthenticatorRequestClientDelegate::InterestingFailureReason::
-        kUserConsentDenied:
-      status = blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR;
-      break;
-  }
-
+    AuthenticatorRequestClientDelegate::InterestingFailureReason reason,
+    blink::mojom::AuthenticatorStatus status) {
   error_awaiting_user_acknowledgement_ = status;
 
   // If WebAuthnUi is enabled, this error blocks until after receiving user
@@ -1408,7 +1395,8 @@ void AuthenticatorCommon::OnTimeout() {
 
   SignalFailureToRequestDelegate(
       /*authenticator=*/nullptr,
-      AuthenticatorRequestClientDelegate::InterestingFailureReason::kTimeout);
+      AuthenticatorRequestClientDelegate::InterestingFailureReason::kTimeout,
+      blink::mojom::AuthenticatorStatus::NOT_ALLOWED_ERROR);
 }
 
 void AuthenticatorCommon::CancelWithStatus(

content/browser/webauth/authenticator_common.h

@@ -167,12 +167,13 @@ class CONTENT_EXPORT AuthenticatorCommon {
   // account from the options.
   void OnAccountSelected(device::AuthenticatorGetAssertionResponse response);
 
-  // Decides whether or not UI is present that needs to block on user
-  // acknowledgement before returning the error, and handles the error
-  // appropriately.
+  // Signals to the request delegate that the request has failed for |reason|.
+  // The request delegate decides whether to present the user with a visual
+  // error before the request is finally resolved with |status|.
   void SignalFailureToRequestDelegate(
       const ::device::FidoAuthenticator* authenticator,
-      AuthenticatorRequestClientDelegate::InterestingFailureReason reason);
+      AuthenticatorRequestClientDelegate::InterestingFailureReason reason,
+      blink::mojom::AuthenticatorStatus status);
 
   void InvokeCallbackAndCleanup(
       blink::mojom::Authenticator::MakeCredentialCallback callback,