Commit 17d43e84 authored by Jun Choi's avatar Jun Choi Committed by Commit Bot

Initiate WebAuthN BLE device selection UI and pin UI

When Bluetooth adapter is powered on and user has not previously
connected with any Bluetooth authenticators, show Bluetooth device
selection UI followed by PIN pairing UI. Also, record used
Bluetooth authenticator ID on successful WebAuthN API in order to
trigger WebAuthn BLE pairing UI for case when use has never paired
to Bluetooth authenticator before.

Bug: 877344
Change-Id: I312bdffa618c5f7d446fe24228acbad8e64f2067
Reviewed-on: https://chromium-review.googlesource.com/c/1333767
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: default avatarKim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609741}
parent 7efc56dc
...@@ -33,7 +33,7 @@ class AuthenticatorDialogTest : public DialogBrowserTest { ...@@ -33,7 +33,7 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
AuthenticatorTransport::kNearFieldCommunication, AuthenticatorTransport::kNearFieldCommunication,
AuthenticatorTransport::kInternal, AuthenticatorTransport::kInternal,
AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy}; AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy};
model->StartFlow(std::move(transport_availability), base::nullopt); model->StartFlow(std::move(transport_availability), base::nullopt, nullptr);
// The dialog should immediately close as soon as it is displayed. // The dialog should immediately close as soon as it is displayed.
if (name == "closed") { if (name == "closed") {
......
...@@ -74,7 +74,14 @@ void OtherTransportsMenuModel::ExecuteCommand(int command_id, int event_flags) { ...@@ -74,7 +74,14 @@ void OtherTransportsMenuModel::ExecuteCommand(int command_id, int event_flags) {
static_cast<AuthenticatorTransport>(command_id); static_cast<AuthenticatorTransport>(command_id);
DCHECK(dialog_model_); DCHECK(dialog_model_);
dialog_model_->StartGuidedFlowForTransport(selected_transport); bool pair_with_new_bluetooth_device = false;
if (selected_transport == AuthenticatorTransport::kBluetoothLowEnergy &&
dialog_model_->current_step() ==
AuthenticatorRequestDialogModel::Step::kBleActivate) {
pair_with_new_bluetooth_device = true;
}
dialog_model_->StartGuidedFlowForTransport(selected_transport,
pair_with_new_bluetooth_device);
} }
void OtherTransportsMenuModel::OnModelDestroyed() { void OtherTransportsMenuModel::OnModelDestroyed() {
......
...@@ -491,8 +491,10 @@ gfx::ImageSkia* AuthenticatorBlePinEntrySheetModel::GetStepIllustration() ...@@ -491,8 +491,10 @@ gfx::ImageSkia* AuthenticatorBlePinEntrySheetModel::GetStepIllustration()
base::string16 AuthenticatorBlePinEntrySheetModel::GetStepTitle() const { base::string16 AuthenticatorBlePinEntrySheetModel::GetStepTitle() const {
const auto& authenticator_id = dialog_model()->selected_authenticator_id(); const auto& authenticator_id = dialog_model()->selected_authenticator_id();
DCHECK(authenticator_id);
const auto* ble_authenticator = const auto* ble_authenticator =
dialog_model()->saved_authenticators().GetAuthenticator(authenticator_id); dialog_model()->saved_authenticators().GetAuthenticator(
*authenticator_id);
DCHECK(ble_authenticator); DCHECK(ble_authenticator);
return l10n_util::GetStringFUTF16( return l10n_util::GetStringFUTF16(
IDS_WEBAUTHN_BLE_PIN_ENTRY_TITLE, IDS_WEBAUTHN_BLE_PIN_ENTRY_TITLE,
......
...@@ -15,6 +15,15 @@ ...@@ -15,6 +15,15 @@
namespace { namespace {
bool ShouldShowBlePairingUI(
bool previously_paired_with_bluetooth_authenticator,
bool pair_with_new_device_for_bluetooth_low_energy) {
if (pair_with_new_device_for_bluetooth_low_energy)
return true;
return !previously_paired_with_bluetooth_authenticator;
}
// Attempts to auto-select the most likely transport that will be used to // Attempts to auto-select the most likely transport that will be used to
// service this request, or returns base::nullopt if unsure. // service this request, or returns base::nullopt if unsure.
base::Optional<device::FidoTransportProtocol> SelectMostLikelyTransport( base::Optional<device::FidoTransportProtocol> SelectMostLikelyTransport(
...@@ -87,7 +96,8 @@ void AuthenticatorRequestDialogModel::SetCurrentStep(Step step) { ...@@ -87,7 +96,8 @@ void AuthenticatorRequestDialogModel::SetCurrentStep(Step step) {
void AuthenticatorRequestDialogModel::StartFlow( void AuthenticatorRequestDialogModel::StartFlow(
TransportAvailabilityInfo transport_availability, TransportAvailabilityInfo transport_availability,
base::Optional<device::FidoTransportProtocol> last_used_transport) { base::Optional<device::FidoTransportProtocol> last_used_transport,
const base::ListValue* previously_paired_bluetooth_device_list) {
DCHECK_EQ(current_step(), Step::kNotStarted); DCHECK_EQ(current_step(), Step::kNotStarted);
DCHECK(!transport_availability.disable_embedder_ui); DCHECK(!transport_availability.disable_embedder_ui);
...@@ -97,6 +107,9 @@ void AuthenticatorRequestDialogModel::StartFlow( ...@@ -97,6 +107,9 @@ void AuthenticatorRequestDialogModel::StartFlow(
available_transports_.emplace_back(transport); available_transports_.emplace_back(transport);
} }
previously_paired_with_bluetooth_authenticator_ =
previously_paired_bluetooth_device_list &&
!previously_paired_bluetooth_device_list->GetList().empty();
StartGuidedFlowForMostLikelyTransportOrShowTransportSelection(); StartGuidedFlowForMostLikelyTransportOrShowTransportSelection();
} }
...@@ -117,7 +130,8 @@ void AuthenticatorRequestDialogModel:: ...@@ -117,7 +130,8 @@ void AuthenticatorRequestDialogModel::
} }
void AuthenticatorRequestDialogModel::StartGuidedFlowForTransport( void AuthenticatorRequestDialogModel::StartGuidedFlowForTransport(
AuthenticatorTransport transport) { AuthenticatorTransport transport,
bool pair_with_new_device_for_bluetooth_low_energy) {
DCHECK(current_step() == Step::kTransportSelection || DCHECK(current_step() == Step::kTransportSelection ||
current_step() == Step::kWelcomeScreen || current_step() == Step::kWelcomeScreen ||
current_step() == Step::kUsbInsertAndActivate || current_step() == Step::kUsbInsertAndActivate ||
...@@ -135,9 +149,15 @@ void AuthenticatorRequestDialogModel::StartGuidedFlowForTransport( ...@@ -135,9 +149,15 @@ void AuthenticatorRequestDialogModel::StartGuidedFlowForTransport(
case AuthenticatorTransport::kInternal: case AuthenticatorTransport::kInternal:
StartTouchIdFlow(); StartTouchIdFlow();
break; break;
case AuthenticatorTransport::kBluetoothLowEnergy: case AuthenticatorTransport::kBluetoothLowEnergy: {
EnsureBleAdapterIsPoweredBeforeContinuingWithStep(Step::kBleActivate); Step next_step = ShouldShowBlePairingUI(
previously_paired_with_bluetooth_authenticator_,
pair_with_new_device_for_bluetooth_low_energy)
? Step::kBleDeviceSelection
: Step::kBleActivate;
EnsureBleAdapterIsPoweredBeforeContinuingWithStep(next_step);
break; break;
}
case AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy: case AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy:
EnsureBleAdapterIsPoweredBeforeContinuingWithStep(Step::kCableActivate); EnsureBleAdapterIsPoweredBeforeContinuingWithStep(Step::kCableActivate);
break; break;
...@@ -209,8 +229,9 @@ void AuthenticatorRequestDialogModel::InitiatePairingDevice( ...@@ -209,8 +229,9 @@ void AuthenticatorRequestDialogModel::InitiatePairingDevice(
void AuthenticatorRequestDialogModel::FinishPairingWithPin( void AuthenticatorRequestDialogModel::FinishPairingWithPin(
const base::string16& pin) { const base::string16& pin) {
DCHECK_EQ(current_step(), Step::kBlePinEntry); DCHECK_EQ(current_step(), Step::kBlePinEntry);
DCHECK(selected_authenticator_id_);
const auto* selected_authenticator = const auto* selected_authenticator =
saved_authenticators_.GetAuthenticator(selected_authenticator_id_); saved_authenticators_.GetAuthenticator(*selected_authenticator_id_);
if (!selected_authenticator) { if (!selected_authenticator) {
// TODO(hongjunchoi): Implement an error screen for error encountered when // TODO(hongjunchoi): Implement an error screen for error encountered when
// pairing. // pairing.
...@@ -221,28 +242,32 @@ void AuthenticatorRequestDialogModel::FinishPairingWithPin( ...@@ -221,28 +242,32 @@ void AuthenticatorRequestDialogModel::FinishPairingWithPin(
DCHECK_EQ(device::FidoTransportProtocol::kBluetoothLowEnergy, DCHECK_EQ(device::FidoTransportProtocol::kBluetoothLowEnergy,
selected_authenticator->transport()); selected_authenticator->transport());
ble_pairing_callback_.Run( ble_pairing_callback_.Run(
selected_authenticator_id_, base::UTF16ToUTF8(pin), *selected_authenticator_id_, base::UTF16ToUTF8(pin),
base::BindOnce(&AuthenticatorRequestDialogModel::OnPairingSuccess, base::BindOnce(&AuthenticatorRequestDialogModel::OnPairingSuccess,
weak_factory_.GetWeakPtr(), selected_authenticator_id_), weak_factory_.GetWeakPtr()),
base::BindOnce(&AuthenticatorRequestDialogModel::OnPairingFailure, base::BindOnce(&AuthenticatorRequestDialogModel::OnPairingFailure,
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
SetCurrentStep(Step::kBleVerifying); SetCurrentStep(Step::kBleVerifying);
} }
void AuthenticatorRequestDialogModel::OnPairingSuccess( void AuthenticatorRequestDialogModel::OnPairingSuccess() {
base::StringPiece authenticator_id) {
DCHECK_EQ(current_step(), Step::kBleVerifying); DCHECK_EQ(current_step(), Step::kBleVerifying);
DCHECK(selected_authenticator_id_);
auto* authenticator = auto* authenticator =
saved_authenticators_.GetAuthenticator(authenticator_id); saved_authenticators_.GetAuthenticator(*selected_authenticator_id_);
if (authenticator) if (!authenticator)
return; return;
authenticator->SetIsPaired(true /* is_paired */); authenticator->SetIsPaired(true /* is_paired */);
DCHECK(ble_device_paired_callback_);
ble_device_paired_callback_.Run(*selected_authenticator_id_);
DispatchRequestAsync(authenticator, base::TimeDelta()); DispatchRequestAsync(authenticator, base::TimeDelta());
} }
void AuthenticatorRequestDialogModel::OnPairingFailure() { void AuthenticatorRequestDialogModel::OnPairingFailure() {
DCHECK_EQ(current_step(), Step::kBleVerifying); DCHECK_EQ(current_step(), Step::kBleVerifying);
selected_authenticator_id_.reset();
SetCurrentStep(Step::kBleDeviceSelection); SetCurrentStep(Step::kBleDeviceSelection);
} }
...@@ -357,10 +382,22 @@ void AuthenticatorRequestDialogModel::SetBluetoothAdapterPowerOnCallback( ...@@ -357,10 +382,22 @@ void AuthenticatorRequestDialogModel::SetBluetoothAdapterPowerOnCallback(
void AuthenticatorRequestDialogModel::UpdateAuthenticatorReferenceId( void AuthenticatorRequestDialogModel::UpdateAuthenticatorReferenceId(
base::StringPiece old_authenticator_id, base::StringPiece old_authenticator_id,
std::string new_authenticator_id) { std::string new_authenticator_id) {
// Bluetooth authenticator address may be changed during pairing process after
// the user chose device to pair during device selection UI. Thus, change
// |selected_authenticator_id_| as well.
if (selected_authenticator_id_ &&
*selected_authenticator_id_ == old_authenticator_id)
selected_authenticator_id_ = new_authenticator_id;
saved_authenticators_.ChangeAuthenticatorId(old_authenticator_id, saved_authenticators_.ChangeAuthenticatorId(old_authenticator_id,
std::move(new_authenticator_id)); std::move(new_authenticator_id));
} }
void AuthenticatorRequestDialogModel::SetBleDevicePairedCallback(
BleDevicePairedCallback ble_device_paired_callback) {
ble_device_paired_callback_ = std::move(ble_device_paired_callback);
}
void AuthenticatorRequestDialogModel::AddAuthenticator( void AuthenticatorRequestDialogModel::AddAuthenticator(
const device::FidoAuthenticator& authenticator) { const device::FidoAuthenticator& authenticator) {
if (!authenticator.AuthenticatorTransport()) { if (!authenticator.AuthenticatorTransport()) {
...@@ -369,10 +406,18 @@ void AuthenticatorRequestDialogModel::AddAuthenticator( ...@@ -369,10 +406,18 @@ void AuthenticatorRequestDialogModel::AddAuthenticator(
// suppressed for this authenticator, so we can simply ignore it here. // suppressed for this authenticator, so we can simply ignore it here.
return; return;
} }
saved_authenticators_.AddAuthenticator(AuthenticatorReference(
AuthenticatorReference authenticator_reference(
authenticator.GetId(), authenticator.GetDisplayName(), authenticator.GetId(), authenticator.GetDisplayName(),
*authenticator.AuthenticatorTransport(), authenticator.IsInPairingMode(), *authenticator.AuthenticatorTransport(), authenticator.IsInPairingMode(),
authenticator.IsPaired())); authenticator.IsPaired());
if (authenticator_reference.is_paired() &&
authenticator_reference.transport() ==
AuthenticatorTransport::kBluetoothLowEnergy) {
DispatchRequestAsync(&authenticator_reference, base::TimeDelta());
}
saved_authenticators_.AddAuthenticator(std::move(authenticator_reference));
} }
void AuthenticatorRequestDialogModel::RemoveAuthenticator( void AuthenticatorRequestDialogModel::RemoveAuthenticator(
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/optional.h" #include "base/optional.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/values.h"
#include "chrome/browser/webauthn/authenticator_reference.h" #include "chrome/browser/webauthn/authenticator_reference.h"
#include "chrome/browser/webauthn/authenticator_transport.h" #include "chrome/browser/webauthn/authenticator_transport.h"
#include "chrome/browser/webauthn/observable_authenticator_list.h" #include "chrome/browser/webauthn/observable_authenticator_list.h"
...@@ -31,6 +32,7 @@ class AuthenticatorRequestDialogModel { ...@@ -31,6 +32,7 @@ class AuthenticatorRequestDialogModel {
public: public:
using RequestCallback = device::FidoRequestHandlerBase::RequestCallback; using RequestCallback = device::FidoRequestHandlerBase::RequestCallback;
using BlePairingCallback = device::FidoRequestHandlerBase::BlePairingCallback; using BlePairingCallback = device::FidoRequestHandlerBase::BlePairingCallback;
using BleDevicePairedCallback = base::RepeatingCallback<void(std::string)>;
using TransportAvailabilityInfo = using TransportAvailabilityInfo =
device::FidoRequestHandlerBase::TransportAvailabilityInfo; device::FidoRequestHandlerBase::TransportAvailabilityInfo;
...@@ -128,8 +130,7 @@ class AuthenticatorRequestDialogModel { ...@@ -128,8 +130,7 @@ class AuthenticatorRequestDialogModel {
return transport_availability()->is_ble_powered; return transport_availability()->is_ble_powered;
} }
const std::string& selected_authenticator_id() const { const base::Optional<std::string>& selected_authenticator_id() const {
DCHECK_EQ(Step::kBlePinEntry, current_step());
return selected_authenticator_id_; return selected_authenticator_id_;
} }
...@@ -139,7 +140,8 @@ class AuthenticatorRequestDialogModel { ...@@ -139,7 +140,8 @@ class AuthenticatorRequestDialogModel {
// Valid action when at step: kNotStarted. // Valid action when at step: kNotStarted.
void StartFlow( void StartFlow(
TransportAvailabilityInfo transport_availability, TransportAvailabilityInfo transport_availability,
base::Optional<device::FidoTransportProtocol> last_used_transport); base::Optional<device::FidoTransportProtocol> last_used_transport,
const base::ListValue* previously_paired_bluetooth_device_list);
// Starts the UX flow. Tries to figure out the most likely transport to be // Starts the UX flow. Tries to figure out the most likely transport to be
// used, and starts the guided flow for that transport; or shows the manual // used, and starts the guided flow for that transport; or shows the manual
...@@ -155,7 +157,9 @@ class AuthenticatorRequestDialogModel { ...@@ -155,7 +157,9 @@ class AuthenticatorRequestDialogModel {
// Valid action when at step: kNotStarted, kWelcomeScreen, // Valid action when at step: kNotStarted, kWelcomeScreen,
// kTransportSelection, and steps where the other transports menu is shown, // kTransportSelection, and steps where the other transports menu is shown,
// namely, kUsbInsertAndActivate, kTouchId, kBleActivate, kCableActivate. // namely, kUsbInsertAndActivate, kTouchId, kBleActivate, kCableActivate.
void StartGuidedFlowForTransport(AuthenticatorTransport transport); void StartGuidedFlowForTransport(
AuthenticatorTransport transport,
bool pair_with_new_device_for_bluetooth_low_energy = false);
// Ensures that the Bluetooth adapter is powered before proceeding to |step|. // Ensures that the Bluetooth adapter is powered before proceeding to |step|.
// -- If the adapter is powered, advanced directly to |step|. // -- If the adapter is powered, advanced directly to |step|.
...@@ -198,7 +202,7 @@ class AuthenticatorRequestDialogModel { ...@@ -198,7 +202,7 @@ class AuthenticatorRequestDialogModel {
// Dispatches WebAuthN request to successfully paired Bluetooth authenticator. // Dispatches WebAuthN request to successfully paired Bluetooth authenticator.
// //
// Valid action when at step: kBleVerifying. // Valid action when at step: kBleVerifying.
void OnPairingSuccess(base::StringPiece authenticator_id); void OnPairingSuccess();
// Returns to Bluetooth device selection modal. // Returns to Bluetooth device selection modal.
// //
...@@ -259,6 +263,9 @@ class AuthenticatorRequestDialogModel { ...@@ -259,6 +263,9 @@ class AuthenticatorRequestDialogModel {
void SetBluetoothAdapterPowerOnCallback( void SetBluetoothAdapterPowerOnCallback(
base::RepeatingClosure bluetooth_adapter_power_on_callback); base::RepeatingClosure bluetooth_adapter_power_on_callback);
void SetBleDevicePairedCallback(
BleDevicePairedCallback ble_device_paired_callback);
void UpdateAuthenticatorReferenceId(base::StringPiece old_authenticator_id, void UpdateAuthenticatorReferenceId(base::StringPiece old_authenticator_id,
std::string new_authenticator_id); std::string new_authenticator_id);
void AddAuthenticator(const device::FidoAuthenticator& authenticator); void AddAuthenticator(const device::FidoAuthenticator& authenticator);
...@@ -290,6 +297,11 @@ class AuthenticatorRequestDialogModel { ...@@ -290,6 +297,11 @@ class AuthenticatorRequestDialogModel {
// kBlePowerOnAutomatic. // kBlePowerOnAutomatic.
base::Optional<Step> next_step_once_ble_powered_; base::Optional<Step> next_step_once_ble_powered_;
// Determines whether Bluetooth device selection UI and pin pairing UI should
// be shown. We proceed directly to Step::kBleVerifying if the user has paired
// with a bluetooth authenticator previously.
bool previously_paired_with_bluetooth_authenticator_ = false;
base::ObserverList<Observer>::Unchecked observers_; base::ObserverList<Observer>::Unchecked observers_;
// These fields are only filled out when the UX flow is started. // These fields are only filled out when the UX flow is started.
...@@ -304,11 +316,12 @@ class AuthenticatorRequestDialogModel { ...@@ -304,11 +316,12 @@ class AuthenticatorRequestDialogModel {
// Represents the id of the Bluetooth authenticator that the user is trying to // Represents the id of the Bluetooth authenticator that the user is trying to
// connect to or conduct WebAuthN request to via the WebAuthN UI. // connect to or conduct WebAuthN request to via the WebAuthN UI.
std::string selected_authenticator_id_; base::Optional<std::string> selected_authenticator_id_;
RequestCallback request_callback_; RequestCallback request_callback_;
BlePairingCallback ble_pairing_callback_; BlePairingCallback ble_pairing_callback_;
base::RepeatingClosure bluetooth_adapter_power_on_callback_; base::RepeatingClosure bluetooth_adapter_power_on_callback_;
BleDevicePairedCallback ble_device_paired_callback_;
base::WeakPtrFactory<AuthenticatorRequestDialogModel> weak_factory_; base::WeakPtrFactory<AuthenticatorRequestDialogModel> weak_factory_;
......
...@@ -20,6 +20,8 @@ ...@@ -20,6 +20,8 @@
namespace { namespace {
constexpr char kTestPairedAuthenticatorId[] = "ble:11-22-33-44";
const base::flat_set<AuthenticatorTransport> kAllTransports = { const base::flat_set<AuthenticatorTransport> kAllTransports = {
AuthenticatorTransport::kUsbHumanInterfaceDevice, AuthenticatorTransport::kUsbHumanInterfaceDevice,
AuthenticatorTransport::kNearFieldCommunication, AuthenticatorTransport::kNearFieldCommunication,
...@@ -82,12 +84,17 @@ class AuthenticatorRequestDialogModelTest : public ::testing::Test { ...@@ -82,12 +84,17 @@ class AuthenticatorRequestDialogModelTest : public ::testing::Test {
using Step = AuthenticatorRequestDialogModel::Step; using Step = AuthenticatorRequestDialogModel::Step;
using RequestType = ::device::FidoRequestHandlerBase::RequestType; using RequestType = ::device::FidoRequestHandlerBase::RequestType;
AuthenticatorRequestDialogModelTest() {} AuthenticatorRequestDialogModelTest() {
test_paired_device_list_.Append(
std::make_unique<base::Value>(kTestPairedAuthenticatorId));
}
~AuthenticatorRequestDialogModelTest() override {} ~AuthenticatorRequestDialogModelTest() override {}
protected: protected:
base::test::ScopedTaskEnvironment task_environment_{ base::test::ScopedTaskEnvironment task_environment_{
base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME}; base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME};
base::ListValue test_paired_device_list_;
private: private:
DISALLOW_COPY_AND_ASSIGN(AuthenticatorRequestDialogModelTest); DISALLOW_COPY_AND_ASSIGN(AuthenticatorRequestDialogModelTest);
...@@ -264,7 +271,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportAutoSelection) { ...@@ -264,7 +271,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportAutoSelection) {
test_case.has_touch_id_credential; test_case.has_touch_id_credential;
AuthenticatorRequestDialogModel model; AuthenticatorRequestDialogModel model;
model.StartFlow(std::move(transports_info), test_case.last_used_transport); model.StartFlow(std::move(transports_info), test_case.last_used_transport,
&test_paired_device_list_);
EXPECT_EQ(test_case.expected_first_step, model.current_step()); EXPECT_EQ(test_case.expected_first_step, model.current_step());
if (model.current_step() == Step::kTransportSelection) if (model.current_step() == Step::kTransportSelection)
...@@ -280,7 +288,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportList) { ...@@ -280,7 +288,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportList) {
transports_info.available_transports = kAllTransports; transports_info.available_transports = kAllTransports;
AuthenticatorRequestDialogModel model; AuthenticatorRequestDialogModel model;
model.StartFlow(std::move(transports_info), base::nullopt); model.StartFlow(std::move(transports_info), base::nullopt,
&test_paired_device_list_);
EXPECT_THAT(model.available_transports(), EXPECT_THAT(model.available_transports(),
::testing::UnorderedElementsAre( ::testing::UnorderedElementsAre(
AuthenticatorTransport::kUsbHumanInterfaceDevice, AuthenticatorTransport::kUsbHumanInterfaceDevice,
...@@ -297,7 +306,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, NoAvailableTransports) { ...@@ -297,7 +306,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, NoAvailableTransports) {
EXPECT_CALL(mock_observer, OnStepTransition()); EXPECT_CALL(mock_observer, OnStepTransition());
model.StartFlow(TransportAvailabilityInfo(), model.StartFlow(TransportAvailabilityInfo(),
AuthenticatorTransport::kInternal); AuthenticatorTransport::kInternal, &test_paired_device_list_);
EXPECT_EQ(Step::kErrorNoAvailableTransports, model.current_step()); EXPECT_EQ(Step::kErrorNoAvailableTransports, model.current_step());
testing::Mock::VerifyAndClearExpectations(&mock_observer); testing::Mock::VerifyAndClearExpectations(&mock_observer);
...@@ -335,7 +344,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, PostMortems) { ...@@ -335,7 +344,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, PostMortems) {
transports_info.available_transports = kAllTransportsWithoutCable; transports_info.available_transports = kAllTransportsWithoutCable;
EXPECT_CALL(mock_observer, OnStepTransition()); EXPECT_CALL(mock_observer, OnStepTransition());
model.StartFlow(std::move(transports_info), base::nullopt); model.StartFlow(std::move(transports_info), base::nullopt,
&test_paired_device_list_);
EXPECT_EQ(Step::kTransportSelection, model.current_step()); EXPECT_EQ(Step::kTransportSelection, model.current_step());
testing::Mock::VerifyAndClearExpectations(&mock_observer); testing::Mock::VerifyAndClearExpectations(&mock_observer);
...@@ -354,6 +364,35 @@ TEST_F(AuthenticatorRequestDialogModelTest, PostMortems) { ...@@ -354,6 +364,35 @@ TEST_F(AuthenticatorRequestDialogModelTest, PostMortems) {
} }
} }
TEST_F(AuthenticatorRequestDialogModelTest, BlePairingFlow) {
const struct {
AuthenticatorTransport transport;
const base::ListValue* paired_device_address_list;
Step expected_final_step;
} kTestCases[] = {
{AuthenticatorTransport::kBluetoothLowEnergy, nullptr,
Step::kBleDeviceSelection},
{AuthenticatorTransport::kBluetoothLowEnergy, &test_paired_device_list_,
Step::kBleActivate},
};
for (const auto test_case : kTestCases) {
TransportAvailabilityInfo transports_info;
transports_info.available_transports = {test_case.transport};
transports_info.can_power_on_ble_adapter = true;
transports_info.is_ble_powered = true;
BluetoothAdapterPowerOnCallbackReceiver power_receiver;
AuthenticatorRequestDialogModel model;
model.SetBluetoothAdapterPowerOnCallback(power_receiver.GetCallback());
model.StartFlow(std::move(transports_info), base::nullopt,
test_case.paired_device_address_list);
EXPECT_EQ(test_case.expected_final_step, model.current_step());
EXPECT_TRUE(model.ble_adapter_is_powered());
EXPECT_FALSE(power_receiver.was_called());
}
}
TEST_F(AuthenticatorRequestDialogModelTest, BleAdapaterAlreadyPowered) { TEST_F(AuthenticatorRequestDialogModelTest, BleAdapaterAlreadyPowered) {
const struct { const struct {
AuthenticatorTransport transport; AuthenticatorTransport transport;
...@@ -373,7 +412,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, BleAdapaterAlreadyPowered) { ...@@ -373,7 +412,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, BleAdapaterAlreadyPowered) {
BluetoothAdapterPowerOnCallbackReceiver power_receiver; BluetoothAdapterPowerOnCallbackReceiver power_receiver;
AuthenticatorRequestDialogModel model; AuthenticatorRequestDialogModel model;
model.SetBluetoothAdapterPowerOnCallback(power_receiver.GetCallback()); model.SetBluetoothAdapterPowerOnCallback(power_receiver.GetCallback());
model.StartFlow(std::move(transports_info), base::nullopt); model.StartFlow(std::move(transports_info), base::nullopt,
&test_paired_device_list_);
EXPECT_EQ(test_case.expected_final_step, model.current_step()); EXPECT_EQ(test_case.expected_final_step, model.current_step());
EXPECT_TRUE(model.ble_adapter_is_powered()); EXPECT_TRUE(model.ble_adapter_is_powered());
EXPECT_FALSE(power_receiver.was_called()); EXPECT_FALSE(power_receiver.was_called());
...@@ -402,7 +442,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, ...@@ -402,7 +442,8 @@ TEST_F(AuthenticatorRequestDialogModelTest,
AuthenticatorRequestDialogModel model; AuthenticatorRequestDialogModel model;
model.AddObserver(&mock_observer); model.AddObserver(&mock_observer);
model.SetBluetoothAdapterPowerOnCallback(power_receiver.GetCallback()); model.SetBluetoothAdapterPowerOnCallback(power_receiver.GetCallback());
model.StartFlow(std::move(transports_info), base::nullopt); model.StartFlow(std::move(transports_info), base::nullopt,
&test_paired_device_list_);
EXPECT_EQ(Step::kBlePowerOnManual, model.current_step()); EXPECT_EQ(Step::kBlePowerOnManual, model.current_step());
EXPECT_FALSE(model.ble_adapter_is_powered()); EXPECT_FALSE(model.ble_adapter_is_powered());
...@@ -441,7 +482,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, ...@@ -441,7 +482,8 @@ TEST_F(AuthenticatorRequestDialogModelTest,
BluetoothAdapterPowerOnCallbackReceiver power_receiver; BluetoothAdapterPowerOnCallbackReceiver power_receiver;
AuthenticatorRequestDialogModel model; AuthenticatorRequestDialogModel model;
model.SetBluetoothAdapterPowerOnCallback(power_receiver.GetCallback()); model.SetBluetoothAdapterPowerOnCallback(power_receiver.GetCallback());
model.StartFlow(std::move(transports_info), base::nullopt); model.StartFlow(std::move(transports_info), base::nullopt,
&test_paired_device_list_);
EXPECT_EQ(Step::kBlePowerOnAutomatic, model.current_step()); EXPECT_EQ(Step::kBlePowerOnAutomatic, model.current_step());
...@@ -478,7 +520,8 @@ TEST_F(AuthenticatorRequestDialogModelTest, ...@@ -478,7 +520,8 @@ TEST_F(AuthenticatorRequestDialogModelTest,
AuthenticatorTransport::kInternal, false /* is_in_pairing_mode */, AuthenticatorTransport::kInternal, false /* is_in_pairing_mode */,
false /* is_paired */)); false /* is_paired */));
model.StartFlow(std::move(transports_info), base::nullopt); model.StartFlow(std::move(transports_info), base::nullopt,
&test_paired_device_list_);
EXPECT_EQ(AuthenticatorRequestDialogModel::Step::kTransportSelection, EXPECT_EQ(AuthenticatorRequestDialogModel::Step::kTransportSelection,
model.current_step()); model.current_step());
EXPECT_EQ(0, num_called); EXPECT_EQ(0, num_called);
......
...@@ -150,6 +150,10 @@ void ChromeAuthenticatorRequestDelegate::RegisterActionCallbacks( ...@@ -150,6 +150,10 @@ void ChromeAuthenticatorRequestDelegate::RegisterActionCallbacks(
transient_dialog_model_holder_->SetBluetoothAdapterPowerOnCallback( transient_dialog_model_holder_->SetBluetoothAdapterPowerOnCallback(
bluetooth_adapter_power_on_callback); bluetooth_adapter_power_on_callback);
transient_dialog_model_holder_->SetBlePairingCallback(ble_pairing_callback); transient_dialog_model_holder_->SetBlePairingCallback(ble_pairing_callback);
transient_dialog_model_holder_->SetBleDevicePairedCallback(
base::BindRepeating(
&ChromeAuthenticatorRequestDelegate::AddFidoBleDeviceToPairedList,
weak_ptr_factory_.GetWeakPtr()));
weak_dialog_model_ = transient_dialog_model_holder_.get(); weak_dialog_model_ = transient_dialog_model_holder_.get();
weak_dialog_model_->AddObserver(this); weak_dialog_model_->AddObserver(this);
...@@ -258,6 +262,22 @@ void ChromeAuthenticatorRequestDelegate::UpdateLastTransportUsed( ...@@ -258,6 +262,22 @@ void ChromeAuthenticatorRequestDelegate::UpdateLastTransportUsed(
Profile::FromBrowserContext(browser_context())->GetPrefs(); Profile::FromBrowserContext(browser_context())->GetPrefs();
prefs->SetString(kWebAuthnLastTransportUsedPrefName, prefs->SetString(kWebAuthnLastTransportUsedPrefName,
device::ToString(transport)); device::ToString(transport));
if (!weak_dialog_model_)
return;
// We already invoke AddFidoBleDeviceToPairedList() on
// AuthenticatorRequestDialogModel::OnPairingSuccess(). We invoke the function
// here once more to take into account the case when user pairs Bluetooth
// authenticator separately via system OS rather than using Chrome WebAuthn
// UI. AddFidoBleDeviceToPairedList() handles the case when duplicate
// authenticator id is being stored.
const auto& selected_bluetooth_authenticator_id =
weak_dialog_model_->selected_authenticator_id();
if (transport == device::FidoTransportProtocol::kBluetoothLowEnergy &&
selected_bluetooth_authenticator_id) {
AddFidoBleDeviceToPairedList(*selected_bluetooth_authenticator_id);
}
} }
void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated( void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated(
...@@ -272,7 +292,8 @@ void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated( ...@@ -272,7 +292,8 @@ void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated(
return; return;
DCHECK(weak_dialog_model_); DCHECK(weak_dialog_model_);
weak_dialog_model_->StartFlow(std::move(data), GetLastTransportUsed()); weak_dialog_model_->StartFlow(std::move(data), GetLastTransportUsed(),
GetPreviouslyPairedFidoBleDeviceIds());
DCHECK(transient_dialog_model_holder_); DCHECK(transient_dialog_model_holder_);
ShowAuthenticatorRequestDialog( ShowAuthenticatorRequestDialog(
...@@ -283,17 +304,18 @@ void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated( ...@@ -283,17 +304,18 @@ void ChromeAuthenticatorRequestDelegate::OnTransportAvailabilityEnumerated(
bool ChromeAuthenticatorRequestDelegate::EmbedderControlsAuthenticatorDispatch( bool ChromeAuthenticatorRequestDelegate::EmbedderControlsAuthenticatorDispatch(
const device::FidoAuthenticator& authenticator) { const device::FidoAuthenticator& authenticator) {
// TODO(hongjunchoi): Change this so that requests for BLE authenticators
// are not dispatched immediately if WebAuthN UI is enabled.
if (!IsWebAuthnUiEnabled()) if (!IsWebAuthnUiEnabled())
return false; return false;
// On macOS, a native dialog is shown for the Touch ID authenticator // On macOS, a native dialog is shown for the Touch ID authenticator
// immediately after dispatch to that authenticator. This dialog must not // immediately after dispatch to that authenticator. This dialog must not
// be triggered before Chrome's WebAuthn UI has advanced accordingly. // be triggered before Chrome's WebAuthn UI has advanced accordingly.
return authenticator.AuthenticatorTransport() && // Also, connection to Bluetooth authenticators should not be established
*authenticator.AuthenticatorTransport() == // before user explicitly chooses to use a BLE device as it can trigger
device::FidoTransportProtocol::kInternal; // OS native pairing UI.
const auto& transport = authenticator.AuthenticatorTransport();
return transport &&
(*transport == device::FidoTransportProtocol::kInternal ||
*transport == device::FidoTransportProtocol::kBluetoothLowEnergy);
} }
void ChromeAuthenticatorRequestDelegate::FidoAuthenticatorAdded( void ChromeAuthenticatorRequestDelegate::FidoAuthenticatorAdded(
...@@ -358,19 +380,21 @@ void ChromeAuthenticatorRequestDelegate::OnCancelRequest() { ...@@ -358,19 +380,21 @@ void ChromeAuthenticatorRequestDelegate::OnCancelRequest() {
} }
void ChromeAuthenticatorRequestDelegate::AddFidoBleDeviceToPairedList( void ChromeAuthenticatorRequestDelegate::AddFidoBleDeviceToPairedList(
std::string device_address) { std::string ble_authenticator_id) {
ListPrefUpdate update( ListPrefUpdate update(
Profile::FromBrowserContext(browser_context())->GetPrefs(), Profile::FromBrowserContext(browser_context())->GetPrefs(),
kWebAuthnBlePairedMacAddressesPrefName); kWebAuthnBlePairedMacAddressesPrefName);
bool already_contains_address = std::any_of( bool already_contains_address = std::any_of(
update->begin(), update->end(), [&device_address](const auto& value) { update->begin(), update->end(),
return value.is_string() && value.GetString() == device_address; [&ble_authenticator_id](const auto& value) {
return value.is_string() && value.GetString() == ble_authenticator_id;
}); });
if (already_contains_address) if (already_contains_address)
return; return;
update->Append(std::make_unique<base::Value>(std::move(device_address))); update->Append(
std::make_unique<base::Value>(std::move(ble_authenticator_id)));
} }
base::Optional<device::FidoTransportProtocol> base::Optional<device::FidoTransportProtocol>
...@@ -382,7 +406,7 @@ ChromeAuthenticatorRequestDelegate::GetLastTransportUsed() const { ...@@ -382,7 +406,7 @@ ChromeAuthenticatorRequestDelegate::GetLastTransportUsed() const {
} }
const base::ListValue* const base::ListValue*
ChromeAuthenticatorRequestDelegate::GetPreviouslyPairedFidoBleDeviceAddresses() ChromeAuthenticatorRequestDelegate::GetPreviouslyPairedFidoBleDeviceIds()
const { const {
PrefService* prefs = PrefService* prefs =
Profile::FromBrowserContext(browser_context())->GetPrefs(); Profile::FromBrowserContext(browser_context())->GetPrefs();
......
...@@ -103,9 +103,9 @@ class ChromeAuthenticatorRequestDelegate ...@@ -103,9 +103,9 @@ class ChromeAuthenticatorRequestDelegate
void OnModelDestroyed() override; void OnModelDestroyed() override;
void OnCancelRequest() override; void OnCancelRequest() override;
void AddFidoBleDeviceToPairedList(std::string device_address); void AddFidoBleDeviceToPairedList(std::string ble_authenticator_id);
base::Optional<device::FidoTransportProtocol> GetLastTransportUsed() const; base::Optional<device::FidoTransportProtocol> GetLastTransportUsed() const;
const base::ListValue* GetPreviouslyPairedFidoBleDeviceAddresses() const; const base::ListValue* GetPreviouslyPairedFidoBleDeviceIds() const;
bool IsWebAuthnUiEnabled() const; bool IsWebAuthnUiEnabled() const;
content::RenderFrameHost* const render_frame_host_; content::RenderFrameHost* const render_frame_host_;
......
...@@ -31,14 +31,13 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest, ...@@ -31,14 +31,13 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest,
ChromeAuthenticatorRequestDelegate delegate(main_rfh()); ChromeAuthenticatorRequestDelegate delegate(main_rfh());
auto* const address_list = auto* const address_list = delegate.GetPreviouslyPairedFidoBleDeviceIds();
delegate.GetPreviouslyPairedFidoBleDeviceAddresses();
ASSERT_TRUE(address_list); ASSERT_TRUE(address_list);
EXPECT_TRUE(address_list->empty()); EXPECT_TRUE(address_list->empty());
delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress); delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress);
const auto* updated_address_list = const auto* updated_address_list =
delegate.GetPreviouslyPairedFidoBleDeviceAddresses(); delegate.GetPreviouslyPairedFidoBleDeviceIds();
ASSERT_TRUE(updated_address_list); ASSERT_TRUE(updated_address_list);
ASSERT_EQ(1u, updated_address_list->GetSize()); ASSERT_EQ(1u, updated_address_list->GetSize());
...@@ -48,13 +47,13 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest, ...@@ -48,13 +47,13 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest,
delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress); delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress);
const auto* address_list_with_duplicate_address_added = const auto* address_list_with_duplicate_address_added =
delegate.GetPreviouslyPairedFidoBleDeviceAddresses(); delegate.GetPreviouslyPairedFidoBleDeviceIds();
ASSERT_TRUE(address_list_with_duplicate_address_added); ASSERT_TRUE(address_list_with_duplicate_address_added);
EXPECT_EQ(1u, address_list_with_duplicate_address_added->GetSize()); EXPECT_EQ(1u, address_list_with_duplicate_address_added->GetSize());
delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress2); delegate.AddFidoBleDeviceToPairedList(kTestPairedDeviceAddress2);
const auto* address_list_with_two_addresses = const auto* address_list_with_two_addresses =
delegate.GetPreviouslyPairedFidoBleDeviceAddresses(); delegate.GetPreviouslyPairedFidoBleDeviceIds();
ASSERT_TRUE(address_list_with_two_addresses); ASSERT_TRUE(address_list_with_two_addresses);
ASSERT_EQ(2u, address_list_with_two_addresses->GetSize()); ASSERT_EQ(2u, address_list_with_two_addresses->GetSize());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment