Plumb Bluetooth authenticator display name to WebAuthN UI

Currently, the only identifier for showing Bluetooth authenticators on
WebAuthN UI modals is the authenticator ID which represents the MAC id
of the device. As this is not user friendly, add human-readable
authenticator_display_name to AuthenticatorReference.

Bug: 877344
Change-Id: I2d3b35db669d8f8f61e9989870122a211d80e80f
Reviewed-on: https://chromium-review.googlesource.com/1247681
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595626}

chrome/browser/webauthn/authenticator_request_dialog_model.cc

@@ -72,8 +72,11 @@ base::Optional<device::FidoTransportProtocol> SelectMostLikelyTransport(
 
 AuthenticatorRequestDialogModel::AuthenticatorReference::AuthenticatorReference(
     base::StringPiece authenticator_id,
+    base::StringPiece16 authenticator_display_name,
     device::FidoTransportProtocol transport)
-    : authenticator_id(authenticator_id), transport(transport) {}
+    : authenticator_id(authenticator_id),
+      authenticator_display_name(authenticator_display_name),
+      transport(transport) {}
 AuthenticatorRequestDialogModel::AuthenticatorReference::AuthenticatorReference(
     AuthenticatorReference&& data) = default;
 AuthenticatorRequestDialogModel::AuthenticatorReference&

chrome/browser/webauthn/authenticator_request_dialog_model.h

@@ -10,6 +10,7 @@
 
 #include "base/observer_list.h"
 #include "base/optional.h"
+#include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
 #include "chrome/browser/webauthn/authenticator_transport.h"
 #include "chrome/browser/webauthn/transport_list_model.h"
@@ -72,11 +73,13 @@ class AuthenticatorRequestDialogModel {
     kCableActivate,
   };
 
-  // Encapsulates information about authenticators that have been found but are
-  // in inactive state because we want to dispatch the requests after receiving
-  // confirmation from the user via the WebAuthN UI flow.
+  // Encapsulates information about authenticators that have been found and
+  // whose request is controlled by the UI embedder because we want to dispatch
+  // the requests after receiving confirmation from the user via the WebAuthN UI
+  // flow.
   struct AuthenticatorReference {
     AuthenticatorReference(base::StringPiece device_id,
+                           base::StringPiece16 authenticator_display_name,
                            device::FidoTransportProtocol transport);
     AuthenticatorReference(AuthenticatorReference&& data);
     AuthenticatorReference& operator=(AuthenticatorReference&& other);
@@ -84,6 +87,7 @@ class AuthenticatorRequestDialogModel {
     ~AuthenticatorReference();
 
     std::string authenticator_id;
+    base::string16 authenticator_display_name;
     device::FidoTransportProtocol transport;
     bool dispatched = false;
   };

chrome/browser/webauthn/authenticator_request_dialog_model_unittest.cc

@@ -474,7 +474,9 @@ TEST_F(AuthenticatorRequestDialogModelTest,
       &num_called));
   model.saved_authenticators().emplace_back(
       AuthenticatorRequestDialogModel::AuthenticatorReference(
-          "authenticator", AuthenticatorTransport::kInternal));
+          "authenticator" /* authenticator_id */,
+          base::string16() /* authenticator_display_name */,
+          AuthenticatorTransport::kInternal));
 
   model.StartFlow(std::move(transports_info), base::nullopt);
   EXPECT_EQ(AuthenticatorRequestDialogModel::Step::kTransportSelection,

chrome/browser/webauthn/chrome_authenticator_request_delegate.cc

@@ -300,7 +300,8 @@ void ChromeAuthenticatorRequestDelegate::FidoAuthenticatorAdded(
     return;
 
   weak_dialog_model_->saved_authenticators().emplace_back(
-      authenticator.GetId(), authenticator.AuthenticatorTransport());
+      authenticator.GetId(), authenticator.GetDisplayName(),
+      authenticator.AuthenticatorTransport());
 }
 
 void ChromeAuthenticatorRequestDelegate::FidoAuthenticatorRemoved(

device/fido/ble/fido_ble_device.cc

@@ -67,6 +67,14 @@ std::string FidoBleDevice::GetId() const {
   return GetId(connection_->address());
 }
 
+base::string16 FidoBleDevice::GetDisplayName() const {
+  auto* device = connection_->GetBleDevice();
+  if (!device)
+    return base::string16();
+
+  return device->GetNameForDisplay();
+}
+
 FidoTransportProtocol FidoBleDevice::DeviceTransport() const {
   return FidoTransportProtocol::kBluetoothLowEnergy;
 }

device/fido/ble/fido_ble_device.h

@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
 #include "base/timer/timer.h"
 #include "device/fido/ble/fido_ble_connection.h"
@@ -42,6 +43,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoBleDevice : public FidoDevice {
   void TryWink(WinkCallback callback) override;
   void Cancel() override;
   std::string GetId() const override;
+  base::string16 GetDisplayName() const override;
   FidoTransportProtocol DeviceTransport() const override;
 
   // Returns whether or not the underlying BLE device is currently in pairing

device/fido/fido_authenticator.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/strings/string16.h"
 #include "device/fido/authenticator_get_assertion_response.h"
 #include "device/fido/authenticator_make_credential_response.h"
 #include "device/fido/fido_transport_protocol.h"
@@ -48,6 +49,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoAuthenticator {
                             GetAssertionCallback callback) = 0;
   virtual void Cancel() = 0;
   virtual std::string GetId() const = 0;
+  virtual base::string16 GetDisplayName() const = 0;
   virtual const AuthenticatorSupportedOptions& Options() const = 0;
   virtual FidoTransportProtocol AuthenticatorTransport() const = 0;
   virtual base::WeakPtr<FidoAuthenticator> GetWeakPtr() = 0;

device/fido/fido_device.cc

@@ -18,6 +18,11 @@ namespace device {
 FidoDevice::FidoDevice() = default;
 FidoDevice::~FidoDevice() = default;
 
+base::string16 FidoDevice::GetDisplayName() const {
+  const auto id = GetId();
+  return base::string16(id.begin(), id.end());
+}
+
 void FidoDevice::DiscoverSupportedProtocolAndDeviceInfo(
     base::OnceClosure done) {
   if (base::FeatureList::IsEnabled(kNewCtap2Device)) {

device/fido/fido_device.h

@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/strings/string16.h"
 #include "device/fido/authenticator_get_info_response.h"
 #include "device/fido/fido_constants.h"
 #include "device/fido/fido_transport_protocol.h"
@@ -59,6 +60,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDevice {
   virtual void TryWink(WinkCallback callback) = 0;
   virtual void Cancel() = 0;
   virtual std::string GetId() const = 0;
+  virtual base::string16 GetDisplayName() const;
   virtual FidoTransportProtocol DeviceTransport() const = 0;
   virtual base::WeakPtr<FidoDevice> GetWeakPtr() = 0;
 

device/fido/fido_device_authenticator.cc

@@ -60,6 +60,10 @@ std::string FidoDeviceAuthenticator::GetId() const {
   return device_->GetId();
 }
 
+base::string16 FidoDeviceAuthenticator::GetDisplayName() const {
+  return device_->GetDisplayName();
+}
+
 const AuthenticatorSupportedOptions& FidoDeviceAuthenticator::Options() const {
   static const AuthenticatorSupportedOptions default_options;
   switch (device_->supported_protocol()) {

device/fido/fido_device_authenticator.h

@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "base/strings/string16.h"
 #include "device/fido/fido_authenticator.h"
 
 namespace device {
@@ -40,6 +41,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDeviceAuthenticator
                     GetAssertionCallback callback) override;
   void Cancel() override;
   std::string GetId() const override;
+  base::string16 GetDisplayName() const override;
   const AuthenticatorSupportedOptions& Options() const override;
   FidoTransportProtocol AuthenticatorTransport() const override;
   base::WeakPtr<FidoAuthenticator> GetWeakPtr() override;

device/fido/mac/authenticator.h

@@ -12,6 +12,7 @@
 #include "base/mac/availability.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/strings/string16.h"
 #include "base/strings/string_piece_forward.h"
 #include "device/fido/fido_authenticator.h"
 #include "device/fido/fido_transport_protocol.h"
@@ -56,6 +57,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) TouchIdAuthenticator
                     GetAssertionCallback callback) override;
   void Cancel() override;
   std::string GetId() const override;
+  base::string16 GetDisplayName() const override;
   const AuthenticatorSupportedOptions& Options() const override;
   FidoTransportProtocol AuthenticatorTransport() const override;
   base::WeakPtr<FidoAuthenticator> GetWeakPtr() override;

device/fido/mac/authenticator.mm

@@ -132,6 +132,10 @@ std::string TouchIdAuthenticator::GetId() const {
   return "TouchIdAuthenticator";
 }
 
+base::string16 TouchIdAuthenticator::GetDisplayName() const {
+  return base::string16();
+}
+
 FidoTransportProtocol TouchIdAuthenticator::AuthenticatorTransport() const {
   return FidoTransportProtocol::kInternal;
 }