Commit af6a20e4 authored by Jun Choi's avatar Jun Choi Committed by Commit Bot

Save previously used transport type for WebAuthn UI

Use Chromium pref as a persistant key-value storage to save previously
used transport type. This will enable us to skip "Welcome" dialog if we
know that the user has already used certain transport type before.

Bug: 868212
Change-Id: Iff732f2d990291cfe1465107fc01d08b3cf3e019
Reviewed-on: https://chromium-review.googlesource.com/1152559
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Reviewed-by: default avatarKim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580028}
parent b4c53a5c
...@@ -32,7 +32,6 @@ ...@@ -32,7 +32,6 @@
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "device/fido/fido_transport_protocol.h"
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
#include "device/fido/mac/credential_metadata.h" #include "device/fido/mac/credential_metadata.h"
...@@ -64,6 +63,26 @@ bool IsWebAuthnUiEnabled() { ...@@ -64,6 +63,26 @@ bool IsWebAuthnUiEnabled() {
switches::kWebAuthenticationUI); switches::kWebAuthenticationUI);
} }
#if !defined(OS_ANDROID)
void SetInitialUiModelBasedOnPreviouslyUsedTransport(
AuthenticatorRequestDialogModel* model,
base::Optional<device::FidoTransportProtocol> previous_transport) {
if (!previous_transport)
return;
// TODO(hongjunchoi): Add UI component for defaulting to BLE, Cable and Mac
// TouchID transports.
switch (*previous_transport) {
case device::FidoTransportProtocol::kUsbHumanInterfaceDevice:
model->SetCurrentStep(AuthenticatorRequestDialogModel::Step::
kUsbInsertAndActivateOnRegister);
break;
default:
return;
}
}
#endif
} // namespace } // namespace
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -71,6 +90,9 @@ static const char kWebAuthnTouchIdMetadataSecretPrefName[] = ...@@ -71,6 +90,9 @@ static const char kWebAuthnTouchIdMetadataSecretPrefName[] =
"webauthn.touchid.metadata_secret"; "webauthn.touchid.metadata_secret";
#endif #endif
static const char kWebAuthnLastTransportUsedPrefName[] =
"webauthn.last_transport_used";
// static // static
void ChromeAuthenticatorRequestDelegate::RegisterProfilePrefs( void ChromeAuthenticatorRequestDelegate::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) { user_prefs::PrefRegistrySyncable* registry) {
...@@ -78,6 +100,9 @@ void ChromeAuthenticatorRequestDelegate::RegisterProfilePrefs( ...@@ -78,6 +100,9 @@ void ChromeAuthenticatorRequestDelegate::RegisterProfilePrefs(
registry->RegisterStringPref(kWebAuthnTouchIdMetadataSecretPrefName, registry->RegisterStringPref(kWebAuthnTouchIdMetadataSecretPrefName,
std::string()); std::string());
#endif #endif
registry->RegisterStringPref(kWebAuthnLastTransportUsedPrefName,
std::string());
} }
ChromeAuthenticatorRequestDelegate::ChromeAuthenticatorRequestDelegate( ChromeAuthenticatorRequestDelegate::ChromeAuthenticatorRequestDelegate(
...@@ -116,7 +141,10 @@ void ChromeAuthenticatorRequestDelegate::DidStartRequest() { ...@@ -116,7 +141,10 @@ void ChromeAuthenticatorRequestDelegate::DidStartRequest() {
auto dialog_model = std::make_unique<AuthenticatorRequestDialogModel>(); auto dialog_model = std::make_unique<AuthenticatorRequestDialogModel>();
weak_dialog_model_ = dialog_model.get(); weak_dialog_model_ = dialog_model.get();
SetInitialUiModelBasedOnPreviouslyUsedTransport(weak_dialog_model_,
GetLastTransportUsed());
weak_dialog_model_->AddObserver(this); weak_dialog_model_->AddObserver(this);
ShowAuthenticatorRequestDialog( ShowAuthenticatorRequestDialog(
content::WebContents::FromRenderFrameHost(render_frame_host()), content::WebContents::FromRenderFrameHost(render_frame_host()),
std::move(dialog_model)); std::move(dialog_model));
...@@ -220,6 +248,14 @@ ChromeAuthenticatorRequestDelegate::GetTouchIdAuthenticatorConfig() const { ...@@ -220,6 +248,14 @@ ChromeAuthenticatorRequestDelegate::GetTouchIdAuthenticatorConfig() const {
} }
#endif #endif
base::Optional<device::FidoTransportProtocol>
ChromeAuthenticatorRequestDelegate::GetLastTransportUsed() const {
PrefService* prefs =
Profile::FromBrowserContext(browser_context())->GetPrefs();
return device::ConvertToFidoTransportProtocol(
prefs->GetString(kWebAuthnLastTransportUsedPrefName));
}
void ChromeAuthenticatorRequestDelegate::BluetoothAdapterIsAvailable() { void ChromeAuthenticatorRequestDelegate::BluetoothAdapterIsAvailable() {
if (!IsWebAuthnUiEnabled()) if (!IsWebAuthnUiEnabled())
return; return;
...@@ -268,6 +304,14 @@ void ChromeAuthenticatorRequestDelegate::FidoAuthenticatorRemoved( ...@@ -268,6 +304,14 @@ void ChromeAuthenticatorRequestDelegate::FidoAuthenticatorRemoved(
saved_authenticators.end()); saved_authenticators.end());
} }
void ChromeAuthenticatorRequestDelegate::UpdateLastTransportUsed(
device::FidoTransportProtocol transport) {
PrefService* prefs =
Profile::FromBrowserContext(browser_context())->GetPrefs();
prefs->SetString(kWebAuthnLastTransportUsedPrefName,
device::ToString(transport));
}
void ChromeAuthenticatorRequestDelegate::OnModelDestroyed() { void ChromeAuthenticatorRequestDelegate::OnModelDestroyed() {
DCHECK(weak_dialog_model_); DCHECK(weak_dialog_model_);
weak_dialog_model_ = nullptr; weak_dialog_model_ = nullptr;
......
...@@ -7,12 +7,15 @@ ...@@ -7,12 +7,15 @@
#include <string> #include <string>
#include "base/gtest_prod_util.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/webauthn/authenticator_request_dialog_model.h" #include "chrome/browser/webauthn/authenticator_request_dialog_model.h"
#include "content/public/browser/authenticator_request_client_delegate.h" #include "content/public/browser/authenticator_request_client_delegate.h"
#include "device/fido/fido_request_handler_base.h" #include "device/fido/fido_request_handler_base.h"
#include "device/fido/fido_transport_protocol.h"
class Profile; class Profile;
...@@ -49,6 +52,7 @@ class ChromeAuthenticatorRequestDelegate ...@@ -49,6 +52,7 @@ class ChromeAuthenticatorRequestDelegate
const override; const override;
#endif // defined(OS_MACOSX) #endif // defined(OS_MACOSX)
base::Optional<device::FidoTransportProtocol> GetLastTransportUsed() const;
base::WeakPtr<ChromeAuthenticatorRequestDelegate> AsWeakPtr(); base::WeakPtr<ChromeAuthenticatorRequestDelegate> AsWeakPtr();
private: private:
...@@ -69,6 +73,8 @@ class ChromeAuthenticatorRequestDelegate ...@@ -69,6 +73,8 @@ class ChromeAuthenticatorRequestDelegate
void FidoAuthenticatorAdded( void FidoAuthenticatorAdded(
const device::FidoAuthenticator& authenticator) override; const device::FidoAuthenticator& authenticator) override;
void FidoAuthenticatorRemoved(base::StringPiece device_id) override; void FidoAuthenticatorRemoved(base::StringPiece device_id) override;
void UpdateLastTransportUsed(
device::FidoTransportProtocol transport) override;
// AuthenticatorRequestDialogModel::Observer: // AuthenticatorRequestDialogModel::Observer:
void OnModelDestroyed() override; void OnModelDestroyed() override;
......
...@@ -10,16 +10,21 @@ ...@@ -10,16 +10,21 @@
#include "content/public/test/web_contents_tester.h" #include "content/public/test/web_contents_tester.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace content {
namespace { namespace {
class ChromeAuthenticatorRequestDelegateTest class ChromeAuthenticatorRequestDelegateTest
: public ChromeRenderViewHostTestHarness {}; : public ChromeRenderViewHostTestHarness {};
TEST_F(ChromeAuthenticatorRequestDelegateTest, TestDefaultTransportPrefType) {
ChromeAuthenticatorRequestDelegate delegate(main_rfh());
EXPECT_FALSE(delegate.GetLastTransportUsed());
}
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
std::string TouchIdMetadataSecret( std::string TouchIdMetadataSecret(
const ChromeAuthenticatorRequestDelegate& delegate) { const ChromeAuthenticatorRequestDelegate& delegate) {
base::Optional<AuthenticatorRequestClientDelegate::TouchIdAuthenticatorConfig> base::Optional<
content::AuthenticatorRequestClientDelegate::TouchIdAuthenticatorConfig>
config = delegate.GetTouchIdAuthenticatorConfig(); config = delegate.GetTouchIdAuthenticatorConfig();
return config->metadata_secret; return config->metadata_secret;
} }
...@@ -45,8 +50,8 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest, ...@@ -45,8 +50,8 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest,
// Different profiles have different secrets. (No way to reset // Different profiles have different secrets. (No way to reset
// browser_context(), so we have to create our own.) // browser_context(), so we have to create our own.)
auto browser_context = base::WrapUnique(CreateBrowserContext()); auto browser_context = base::WrapUnique(CreateBrowserContext());
auto web_contents = auto web_contents = content::WebContentsTester::CreateTestWebContents(
WebContentsTester::CreateTestWebContents(browser_context.get(), nullptr); browser_context.get(), nullptr);
EXPECT_NE( EXPECT_NE(
TouchIdMetadataSecret(ChromeAuthenticatorRequestDelegate(main_rfh())), TouchIdMetadataSecret(ChromeAuthenticatorRequestDelegate(main_rfh())),
TouchIdMetadataSecret( TouchIdMetadataSecret(
...@@ -59,4 +64,3 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest, ...@@ -59,4 +64,3 @@ TEST_F(ChromeAuthenticatorRequestDelegateTest,
#endif #endif
} // namespace } // namespace
} // namespace content
...@@ -48,4 +48,7 @@ void AuthenticatorRequestClientDelegate::FidoAuthenticatorAdded( ...@@ -48,4 +48,7 @@ void AuthenticatorRequestClientDelegate::FidoAuthenticatorAdded(
void AuthenticatorRequestClientDelegate::FidoAuthenticatorRemoved( void AuthenticatorRequestClientDelegate::FidoAuthenticatorRemoved(
base::StringPiece device_id) {} base::StringPiece device_id) {}
void AuthenticatorRequestClientDelegate::UpdateLastTransportUsed(
device::FidoTransportProtocol transport) {}
} // namespace content } // namespace content
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "device/fido/fido_request_handler_base.h" #include "device/fido/fido_request_handler_base.h"
#include "device/fido/fido_transport_protocol.h"
namespace device { namespace device {
class FidoAuthenticator; class FidoAuthenticator;
...@@ -82,6 +83,10 @@ class CONTENT_EXPORT AuthenticatorRequestClientDelegate ...@@ -82,6 +83,10 @@ class CONTENT_EXPORT AuthenticatorRequestClientDelegate
GetTouchIdAuthenticatorConfig() const; GetTouchIdAuthenticatorConfig() const;
#endif #endif
// Saves transport type the user used during WebAuthN API so that the
// WebAuthN UI will default to the same transport type during next API call.
virtual void UpdateLastTransportUsed(device::FidoTransportProtocol transport);
// device::FidoRequestHandlerBase::AuthenticatorMapObserver: // device::FidoRequestHandlerBase::AuthenticatorMapObserver:
void BluetoothAdapterIsAvailable() override; void BluetoothAdapterIsAvailable() override;
void FidoAuthenticatorAdded( void FidoAuthenticatorAdded(
......
...@@ -75,6 +75,7 @@ component("fido") { ...@@ -75,6 +75,7 @@ component("fido") {
"fido_request_handler_base.h", "fido_request_handler_base.h",
"fido_task.cc", "fido_task.cc",
"fido_task.h", "fido_task.h",
"fido_transport_protocol.cc",
"fido_transport_protocol.h", "fido_transport_protocol.h",
"get_assertion_request_handler.cc", "get_assertion_request_handler.cc",
"get_assertion_request_handler.h", "get_assertion_request_handler.h",
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "device/fido/fido_transport_protocol.h"
namespace device {
const char kUsbHumanInterfaceDevice[] = "hid";
const char kNearFieldCommunication[] = "nfc";
const char kBluetoothLowEnergy[] = "ble";
const char kCloudAssistedBluetoothLowEnergy[] = "cable";
const char kInternal[] = "internal";
base::Optional<FidoTransportProtocol> ConvertToFidoTransportProtocol(
base::StringPiece protocol) {
if (protocol == kUsbHumanInterfaceDevice)
return FidoTransportProtocol::kUsbHumanInterfaceDevice;
else if (protocol == kNearFieldCommunication)
return FidoTransportProtocol::kNearFieldCommunication;
else if (protocol == kBluetoothLowEnergy)
return FidoTransportProtocol::kBluetoothLowEnergy;
else if (protocol == kCloudAssistedBluetoothLowEnergy)
return FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy;
else if (protocol == kInternal)
return FidoTransportProtocol::kInternal;
else
return base::nullopt;
}
COMPONENT_EXPORT(DEVICE_FIDO)
std::string ToString(FidoTransportProtocol protocol) {
switch (protocol) {
case FidoTransportProtocol::kUsbHumanInterfaceDevice:
return kUsbHumanInterfaceDevice;
case FidoTransportProtocol::kNearFieldCommunication:
return kNearFieldCommunication;
case FidoTransportProtocol::kBluetoothLowEnergy:
return kBluetoothLowEnergy;
case FidoTransportProtocol::kCloudAssistedBluetoothLowEnergy:
return kCloudAssistedBluetoothLowEnergy;
case FidoTransportProtocol::kInternal:
return kInternal;
}
NOTREACHED();
return "";
}
} // namespace device
...@@ -5,11 +5,17 @@ ...@@ -5,11 +5,17 @@
#ifndef DEVICE_FIDO_FIDO_TRANSPORT_PROTOCOL_H_ #ifndef DEVICE_FIDO_FIDO_TRANSPORT_PROTOCOL_H_
#define DEVICE_FIDO_FIDO_TRANSPORT_PROTOCOL_H_ #define DEVICE_FIDO_FIDO_TRANSPORT_PROTOCOL_H_
#include <string>
#include "base/component_export.h"
#include "base/optional.h"
#include "base/strings/string_piece.h"
namespace device { namespace device {
// This enum represents the transport protocols over which Fido WebAuthN API is // This enum represents the transport protocols over which Fido WebAuthN API is
// currently supported. // currently supported.
enum class FidoTransportProtocol { enum class FidoTransportProtocol : uint8_t {
kUsbHumanInterfaceDevice, kUsbHumanInterfaceDevice,
kNearFieldCommunication, kNearFieldCommunication,
kBluetoothLowEnergy, kBluetoothLowEnergy,
...@@ -17,6 +23,20 @@ enum class FidoTransportProtocol { ...@@ -17,6 +23,20 @@ enum class FidoTransportProtocol {
kInternal, kInternal,
}; };
// String representation of above FidoTransportProtocol enum.
extern const char kUsbHumanInterfaceDevice[];
extern const char kNearFieldCommunication[];
extern const char kBluetoothLowEnergy[];
extern const char kCloudAssistedBluetoothLowEnergy[];
extern const char kInternal[];
COMPONENT_EXPORT(DEVICE_FIDO)
base::Optional<FidoTransportProtocol> ConvertToFidoTransportProtocol(
base::StringPiece protocol);
COMPONENT_EXPORT(DEVICE_FIDO)
std::string ToString(FidoTransportProtocol protocol);
} // namespace device } // namespace device
#endif // DEVICE_FIDO_FIDO_TRANSPORT_PROTOCOL_H_ #endif // DEVICE_FIDO_FIDO_TRANSPORT_PROTOCOL_H_
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