webauthn: show BLE permissions dialog on macOS

If Chrome doesn't have Bluetooth permission on macOS, macOS will show a
prompt to the user to grant it. But if the user declines that prompt
then it's recorded that Chrome should be denied Bluetooth permission and
the prompt will never appear again.

In this situation, Bluetooth API calls "work", they just never receive
any data. So it looks like Chrome (or Bluetooth) is broken.

This change adds a dialog step to prompt the user to open System
Preferences and grant Chrome permission if it ends up in this state.
It's limited to macOS <= 12 because macOS 13 renames Preferences and
breaks the ability to open it to a specific page (at least in beta 1).

Change-Id: I7fb037b6ab4492cae1c1ef6584cc9607b8780801
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3685220
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/heads/main@{#1018385}

chrome/app/generated_resources.grd

@@ -12287,6 +12287,13 @@ Please help our engineers fix this problem. Tell us what happened right before y
       <message name="IDS_WEBAUTHN_BLUETOOTH_POWER_ON_MANUAL_NEXT" desc="Button text. The user clicks this once they manually turned on Bluetooth, so that Chrome would retry connecting to security keys over Bluetooth.">
         Try again
       </message>
+
+      <if expr="is_macosx">
+        <message name="IDS_WEBAUTHN_BLUETOOTH_PERMISSION" desc="Body text in a dialog that appears when the user has requested some function that depends on Bluetooth, but Chrome doesn't have permission to use Bluetooth. The action on this dialog is a button that will open the macOS System Preferences so that the user can grant permission.">
+          Chrome needs permission to use Bluetooth to connect to your device
+        </message>
+      </if>
+
       <message name="IDS_WEBAUTHN_TRANSPORT_POPUP_LABEL" desc="Menu item text. The user selects this to verify their identity on a web site (i.e. sign in) using a different hardware-based authentication mechanism from what they have selected previously.">
         Choose another option
       </message>

chrome/app/generated_resources_grd/IDS_WEBAUTHN_BLUETOOTH_PERMISSION.png.sha1

@@ -0,0 +1 @@
+06813bb2bca58aa7df2c91be2659a6e796912e44

chrome/browser/ui/views/webauthn/authenticator_request_sheet_view.cc

@@ -172,13 +172,15 @@ AuthenticatorRequestSheetView::CreateContentsBelowIllustration() {
       views::LayoutProvider::Get()->GetDistanceMetric(
           views::DISTANCE_RELATED_CONTROL_VERTICAL)));
 
-  auto title_label = std::make_unique<views::Label>(
-      model()->GetStepTitle(), views::style::CONTEXT_DIALOG_TITLE,
-      views::style::STYLE_PRIMARY);
-  title_label->SetMultiLine(true);
-  title_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-  title_label->SetAllowCharacterBreak(true);
-  label_container->AddChildView(title_label.release());
+  const std::u16string title = model()->GetStepTitle();
+  if (!title.empty()) {
+    auto title_label = std::make_unique<views::Label>(
+        title, views::style::CONTEXT_DIALOG_TITLE, views::style::STYLE_PRIMARY);
+    title_label->SetMultiLine(true);
+    title_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+    title_label->SetAllowCharacterBreak(true);
+    label_container->AddChildView(title_label.release());
+  }
 
   std::u16string description = model()->GetStepDescription();
   if (!description.empty()) {

chrome/browser/ui/views/webauthn/sheet_view_factory.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/check.h"
+#include "build/build_config.h"
 #include "chrome/browser/ui/autofill/payments/webauthn_dialog_model.h"
 #include "chrome/browser/ui/views/webauthn/authenticator_bio_enrollment_sheet_view.h"
 #include "chrome/browser/ui/views/webauthn/authenticator_client_pin_entry_sheet_view.h"
@@ -127,6 +128,13 @@ std::unique_ptr<AuthenticatorRequestSheetView> CreateSheetViewForCurrentStepOf(
           std::make_unique<AuthenticatorBlePowerOnManualSheetModel>(
               dialog_model));
       break;
+#if BUILDFLAG(IS_MAC)
+    case Step::kBlePermissionMac:
+      sheet_view = std::make_unique<AuthenticatorRequestSheetView>(
+          std::make_unique<AuthenticatorBlePermissionMacSheetModel>(
+              dialog_model));
+      break;
+#endif
     case Step::kOffTheRecordInterstitial:
       sheet_view = std::make_unique<AuthenticatorRequestSheetView>(
           std::make_unique<AuthenticatorOffTheRecordInterstitialSheetModel>(

chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc

@@ -268,6 +268,12 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
       model_->SetCurrentStepForTesting(
           AuthenticatorRequestDialogModel::Step::kCableActivate);
     }
+#if BUILDFLAG(IS_MAC)
+    else if (name == "ble_permission_mac") {
+      model_->SetCurrentStepForTesting(
+          AuthenticatorRequestDialogModel::Step::kBlePermissionMac);
+    }
+#endif
 
 #define EXP_SHEET(x)                                                    \
   else if (name == "server_link_sheet_" #x) {                           \
@@ -480,3 +486,9 @@ EXP_SHEET(ARM_4)
 EXP_SHEET(ARM_5)
 EXP_SHEET(ARM_6)
 #undef EXP_SHEET
+
+#if BUILDFLAG(IS_MAC)
+IN_PROC_BROWSER_TEST_F(AuthenticatorDialogTest, InvokeUi_ble_permission_mac) {
+  ShowAndVerifyUi();
+}
+#endif

chrome/browser/ui/webauthn/sheet_models.cc

@@ -495,6 +495,52 @@ void AuthenticatorBlePowerOnAutomaticSheetModel::OnAccept() {
   dialog_model()->PowerOnBleAdapter();
 }
 
+#if BUILDFLAG(IS_MAC)
+
+// AuthenticatorBlePermissionMacSheetModel
+// ------------------------------------
+
+const gfx::VectorIcon&
+AuthenticatorBlePermissionMacSheetModel::GetStepIllustration(
+    ImageColorScheme color_scheme) const {
+  return color_scheme == ImageColorScheme::kDark
+             ? kWebauthnErrorBluetoothDarkIcon
+             : kWebauthnErrorBluetoothIcon;
+}
+
+std::u16string AuthenticatorBlePermissionMacSheetModel::GetStepTitle() const {
+  // An empty title causes the title View to be omitted.
+  return u"";
+}
+
+std::u16string AuthenticatorBlePermissionMacSheetModel::GetStepDescription()
+    const {
+  return l10n_util::GetStringUTF16(IDS_WEBAUTHN_BLUETOOTH_PERMISSION);
+}
+
+bool AuthenticatorBlePermissionMacSheetModel::IsAcceptButtonVisible() const {
+  return true;
+}
+
+bool AuthenticatorBlePermissionMacSheetModel::IsAcceptButtonEnabled() const {
+  return true;
+}
+
+bool AuthenticatorBlePermissionMacSheetModel::IsCancelButtonVisible() const {
+  return false;
+}
+
+std::u16string AuthenticatorBlePermissionMacSheetModel::GetAcceptButtonLabel()
+    const {
+  return l10n_util::GetStringUTF16(IDS_OPEN_PREFERENCES_LINK);
+}
+
+void AuthenticatorBlePermissionMacSheetModel::OnAccept() {
+  dialog_model()->OpenBlePreferences();
+}
+
+#endif  // IS_MAC
+
 // AuthenticatorOffTheRecordInterstitialSheetModel
 // -----------------------------------------
 

chrome/browser/ui/webauthn/sheet_models.h

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/memory/raw_ptr.h"
+#include "build/build_config.h"
 #include "chrome/browser/ui/webauthn/authenticator_request_sheet_model.h"
 #include "chrome/browser/ui/webauthn/transport_hover_list_model.h"
 #include "chrome/browser/webauthn/authenticator_request_dialog_model.h"
@@ -232,6 +233,28 @@ class AuthenticatorBlePowerOnAutomaticSheetModel
   bool busy_powering_on_ble_ = false;
 };
 
+#if BUILDFLAG(IS_MAC)
+
+class AuthenticatorBlePermissionMacSheetModel
+    : public AuthenticatorSheetModelBase {
+ public:
+  using AuthenticatorSheetModelBase::AuthenticatorSheetModelBase;
+
+ private:
+  // AuthenticatorSheetModelBase:
+  const gfx::VectorIcon& GetStepIllustration(
+      ImageColorScheme color_scheme) const override;
+  std::u16string GetStepTitle() const override;
+  std::u16string GetStepDescription() const override;
+  bool IsAcceptButtonVisible() const override;
+  bool IsAcceptButtonEnabled() const override;
+  bool IsCancelButtonVisible() const override;
+  std::u16string GetAcceptButtonLabel() const override;
+  void OnAccept() override;
+};
+
+#endif  // IS_MAC
+
 class AuthenticatorOffTheRecordInterstitialSheetModel
     : public AuthenticatorSheetModelBase {
  public:

chrome/browser/webauthn/authenticator_request_dialog_model.cc

@@ -12,6 +12,7 @@
 #include "base/containers/contains.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/observer_list.h"
+#include "base/process/launch.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "build/build_config.h"
@@ -27,6 +28,10 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/text_elider.h"
 
+#if BUILDFLAG(IS_MAC)
+#include "base/mac/mac_util.h"
+#endif
+
 namespace {
 
 // BleEvent enumerates user-visible BLE events.
@@ -262,6 +267,21 @@ void AuthenticatorRequestDialogModel::
          current_step() == Step::kOffTheRecordInterstitial ||
          current_step() == Step::kNotStarted);
 
+#if BUILDFLAG(IS_MAC)
+  // The BLE permission screen is only shown on macOS <= 12 because:
+  //    * System Preferences has been renamed to System Settings, so the
+  //      string on the button would need to be changed.
+  //    * Opening Preferences/Settings at the BLE permissions page is broken.
+  if (transport_availability()->ble_access_denied &&
+      base::mac::IsAtMostOS12()) {
+    // |step| is not saved because macOS asks the user to restart Chrome
+    // after permission has been granted. So the user will end up retrying
+    // the whole WebAuthn request in the new process.
+    SetCurrentStep(Step::kBlePermissionMac);
+    return;
+  }
+#endif
+
   if (ble_adapter_is_powered()) {
     base::UmaHistogramEnumeration("WebAuthentication.BLEUserEvents",
                                   BleEvent::kAlreadyPowered);
@@ -303,6 +323,19 @@ void AuthenticatorRequestDialogModel::PowerOnBleAdapter() {
   bluetooth_adapter_power_on_callback_.Run();
 }
 
+#if BUILDFLAG(IS_MAC)
+void AuthenticatorRequestDialogModel::OpenBlePreferences() {
+  DCHECK_EQ(current_step(), Step::kBlePermissionMac);
+
+  base::LaunchOptions opts;
+  opts.disclaim_responsibility = true;
+  base::LaunchProcess({"open",
+                       "x-apple.systempreferences:com.apple.preference."
+                       "security?Privacy_Bluetooth"},
+                      opts);
+}
+#endif  // IS_MAC
+
 void AuthenticatorRequestDialogModel::TryUsbDevice() {
   DCHECK_EQ(current_step(), Step::kUsbInsertAndActivate);
 }

chrome/browser/webauthn/authenticator_request_dialog_model.h

@@ -86,6 +86,9 @@ class AuthenticatorRequestDialogModel {
     // Bluetooth Low Energy (BLE).
     kBlePowerOnAutomatic,
     kBlePowerOnManual,
+#if BUILDFLAG(IS_MAC)
+    kBlePermissionMac,
+#endif
 
     // Let the user confirm that they want to create a credential in an
     // off-the-record browsing context. Used for platform and caBLE credentials,
@@ -345,6 +348,11 @@ class AuthenticatorRequestDialogModel {
   // Valid action when at step: kBlePowerOnAutomatic.
   void PowerOnBleAdapter();
 
+  // Open the system dialog to grant BLE permission to Chrome.
+  //
+  // Valid action when at step: kBlePermissionMac.
+  void OpenBlePreferences();
+
   // Tries if a USB device is present -- the user claims they plugged it in.
   //
   // Valid action when at step: kUsbInsert.

device/fido/cable/fido_cable_discovery.cc

@@ -278,8 +278,11 @@ void FidoCableDiscovery::OnGetAdapter(scoped_refptr<BluetoothAdapter> adapter) {
   switch (fido::mac::ProcessIsSigned()) {
     case fido::mac::CodeSigningState::kSigned:
       FIDO_LOG(DEBUG) << "Bluetooth authorized: "
-                      << (adapter_->GetOsPermissionStatus() !=
-                          BluetoothAdapter::PermissionStatus::kDenied);
+                      << static_cast<int>(adapter_->GetOsPermissionStatus());
+      if (adapter_->GetOsPermissionStatus() ==
+          BluetoothAdapter::PermissionStatus::kDenied) {
+        observer()->BleDenied();
+      }
       break;
     case fido::mac::CodeSigningState::kNotSigned:
       FIDO_LOG(DEBUG)

device/fido/fido_discovery_base.h

@@ -42,6 +42,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDiscoveryBase {
                                     FidoAuthenticator* authenticator) = 0;
     virtual void AuthenticatorRemoved(FidoDiscoveryBase* discovery,
                                       FidoAuthenticator* authenticator) = 0;
+
+    // BleDenied is called if the user has denied access to the BLE hardware.
+    // This is macOS-specific and, unlike information like the power state, this
+    // information is only available once the caBLE discovery has opened the BLE
+    // adaptor. Thus the signal is plumbed via this observer interface.
+    virtual void BleDenied() {}
   };
 
   // Start authenticator discovery. The Observer must have been set before this

device/fido/fido_request_handler_base.cc

@@ -387,6 +387,10 @@ void FidoRequestHandlerBase::AuthenticatorAdded(
 #endif  // BUILDFLAG(IS_WIN)
 }
 
+void FidoRequestHandlerBase::BleDenied() {
+  transport_availability_info_.ble_access_denied = true;
+}
+
 void FidoRequestHandlerBase::GetPlatformCredentialStatus(
     FidoAuthenticator* platform_authenticator) {
   transport_availability_callback_readiness_

device/fido/fido_request_handler_base.h

@@ -89,6 +89,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
     bool is_ble_powered = false;
     bool can_power_on_ble_adapter = false;
 
+    // ble_access_denied is set to true if Chromium does not have permission
+    // to use the BLE adaptor. Resolving this is a platform-specific operation.
+    bool ble_access_denied = false;
+
     // Indicates whether the native Windows WebAuthn API is available.
     // Dispatching to it should be controlled by the embedder.
     //
@@ -294,6 +298,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
                           FidoAuthenticator* authenticator) override;
   void AuthenticatorRemoved(FidoDiscoveryBase* discovery,
                             FidoAuthenticator* authenticator) override;
+  void BleDenied() override;
 
   // GetPlatformCredentialStatus is called to learn whether a platform
   // authenticator has credentials responsive to the current request. If this