Add preference to store address of paired FIDO BLE device

WebAuthN BLE UI flow should guide the user through pairing UX if no FIDO
BLE devices have been paired with the system previously. If there is at
least one FIDO BLE device that is paired beforehand, we should default
to "activate BLE security key" flow. As so, implement logic to store
address of BLE security keys.

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

chrome/browser/webauthn/chrome_authenticator_request_delegate.cc

@@ -75,6 +75,9 @@ static const char kWebAuthnTouchIdMetadataSecretPrefName[] =
 static const char kWebAuthnLastTransportUsedPrefName[] =
     "webauthn.last_transport_used";
 
+static const char kWebAuthnBlePairedMacAddressesPrefName[] =
+    "webauthn.ble.paired_mac_addresses";
+
 // static
 void ChromeAuthenticatorRequestDelegate::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
@@ -85,6 +88,8 @@ void ChromeAuthenticatorRequestDelegate::RegisterProfilePrefs(
 
   registry->RegisterStringPref(kWebAuthnLastTransportUsedPrefName,
                                std::string());
+  registry->RegisterListPref(kWebAuthnBlePairedMacAddressesPrefName,
+                             std::make_unique<base::ListValue>());
 }
 
 ChromeAuthenticatorRequestDelegate::ChromeAuthenticatorRequestDelegate(
@@ -105,14 +110,6 @@ ChromeAuthenticatorRequestDelegate::~ChromeAuthenticatorRequestDelegate() {
   }
 }
 
-base::Optional<device::FidoTransportProtocol>
-ChromeAuthenticatorRequestDelegate::GetLastTransportUsed() const {
-  PrefService* prefs =
-      Profile::FromBrowserContext(browser_context())->GetPrefs();
-  return device::ConvertToFidoTransportProtocol(
-      prefs->GetString(kWebAuthnLastTransportUsedPrefName));
-}
-
 base::WeakPtr<ChromeAuthenticatorRequestDelegate>
 ChromeAuthenticatorRequestDelegate::AsWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
@@ -347,3 +344,35 @@ void ChromeAuthenticatorRequestDelegate::OnCancelRequest() {
   DCHECK(cancel_callback_);
   std::move(cancel_callback_).Run();
 }
+
+void ChromeAuthenticatorRequestDelegate::AddFidoBleDeviceToPairedList(
+    std::string device_address) {
+  ListPrefUpdate update(
+      Profile::FromBrowserContext(browser_context())->GetPrefs(),
+      kWebAuthnBlePairedMacAddressesPrefName);
+  bool already_contains_address = std::any_of(
+      update->begin(), update->end(), [&device_address](const auto& value) {
+        return value.is_string() && value.GetString() == device_address;
+      });
+
+  if (already_contains_address)
+    return;
+
+  update->Append(std::make_unique<base::Value>(std::move(device_address)));
+}
+
+base::Optional<device::FidoTransportProtocol>
+ChromeAuthenticatorRequestDelegate::GetLastTransportUsed() const {
+  PrefService* prefs =
+      Profile::FromBrowserContext(browser_context())->GetPrefs();
+  return device::ConvertToFidoTransportProtocol(
+      prefs->GetString(kWebAuthnLastTransportUsedPrefName));
+}
+
+const base::ListValue*
+ChromeAuthenticatorRequestDelegate::GetPreviouslyPairedFidoBleDeviceAddresses()
+    const {
+  PrefService* prefs =
+      Profile::FromBrowserContext(browser_context())->GetPrefs();
+  return prefs->GetList(kWebAuthnBlePairedMacAddressesPrefName);
+}

chrome/browser/webauthn/chrome_authenticator_request_delegate.h

@@ -13,6 +13,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/strings/string_piece.h"
+#include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/webauthn/authenticator_request_dialog_model.h"
 #include "content/public/browser/authenticator_request_client_delegate.h"
@@ -54,10 +55,14 @@ class ChromeAuthenticatorRequestDelegate
       const override;
 #endif  // defined(OS_MACOSX)
 
-  base::Optional<device::FidoTransportProtocol> GetLastTransportUsed() const;
   base::WeakPtr<ChromeAuthenticatorRequestDelegate> AsWeakPtr();
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(ChromeAuthenticatorRequestDelegateTest,
+                           TestTransportPrefType);
+  FRIEND_TEST_ALL_PREFIXES(ChromeAuthenticatorRequestDelegateTest,
+                           TestPairedDeviceAddressPreference);
+
   content::RenderFrameHost* render_frame_host() const {
     return render_frame_host_;
   }
@@ -94,6 +99,10 @@ class ChromeAuthenticatorRequestDelegate
   void OnModelDestroyed() override;
   void OnCancelRequest() override;
 
+  void AddFidoBleDeviceToPairedList(std::string device_address);
+  base::Optional<device::FidoTransportProtocol> GetLastTransportUsed() const;
+  const base::ListValue* GetPreviouslyPairedFidoBleDeviceAddresses() const;
+
   content::RenderFrameHost* const render_frame_host_;
   AuthenticatorRequestDialogModel* weak_dialog_model_ = nullptr;
   // Holds ownership of AuthenticatorRequestDialogModel until

chrome/browser/webauthn/chrome_authenticator_request_delegate_unittest.cc

@@ -4,20 +4,64 @@
 
 #include "chrome/browser/webauthn/chrome_authenticator_request_delegate.h"
 
+#include <utility>
+
 #include "build/build_config.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/test/web_contents_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-namespace {
-
 class ChromeAuthenticatorRequestDelegateTest
     : public ChromeRenderViewHostTestHarness {};
 
-TEST_F(ChromeAuthenticatorRequestDelegateTest, TestDefaultTransportPrefType) {
+TEST_F(ChromeAuthenticatorRequestDelegateTest, TestTransportPrefType) {
   ChromeAuthenticatorRequestDelegate delegate(main_rfh());
   EXPECT_FALSE(delegate.GetLastTransportUsed());
+  delegate.UpdateLastTransportUsed(device::FidoTransportProtocol::kInternal);
+  const auto transport = delegate.GetLastTransportUsed();
+  ASSERT_TRUE(transport);
+  EXPECT_EQ(device::FidoTransportProtocol::kInternal, transport);
+}
+
+TEST_F(ChromeAuthenticatorRequestDelegateTest,
+       TestPairedDeviceAddressPreference) {
+  static constexpr char kTestPairedDeviceAddress[] = "paired_device_address";
+  static constexpr char kTestPairedDeviceAddress2[] = "paired_device_address2";
+
+  ChromeAuthenticatorRequestDelegate delegate(main_rfh());
+
+  auto* const address_list =
+      delegate.GetPreviouslyPairedFidoBleDeviceAddresses();
+  ASSERT_TRUE(address_list);
+  EXPECT_TRUE(address_list->empty());
+
+  delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress);
+  const auto* updated_address_list =
+      delegate.GetPreviouslyPairedFidoBleDeviceAddresses();
+  ASSERT_TRUE(updated_address_list);
+  ASSERT_EQ(1u, updated_address_list->GetSize());
+
+  const auto& address_value = updated_address_list->GetList().at(0);
+  ASSERT_TRUE(address_value.is_string());
+  EXPECT_EQ(kTestPairedDeviceAddress, address_value.GetString());
+
+  delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress);
+  const auto* address_list_with_duplicate_address_added =
+      delegate.GetPreviouslyPairedFidoBleDeviceAddresses();
+  ASSERT_TRUE(address_list_with_duplicate_address_added);
+  EXPECT_EQ(1u, address_list_with_duplicate_address_added->GetSize());
+
+  delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress2);
+  const auto* address_list_with_two_addresses =
+      delegate.GetPreviouslyPairedFidoBleDeviceAddresses();
+  ASSERT_TRUE(address_list_with_two_addresses);
+
+  ASSERT_EQ(2u, address_list_with_two_addresses->GetSize());
+  const auto& second_address_value =
+      address_list_with_two_addresses->GetList().at(1);
+  ASSERT_TRUE(second_address_value.is_string());
+  EXPECT_EQ(kTestPairedDeviceAddress2, second_address_value.GetString());
 }
 
 #if defined(OS_MACOSX)
@@ -63,4 +107,3 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest,
 }
 #endif
 
-}  // namespace