0

fido: rename 'Touch ID' to 'platform authentictor' in a few places

To better reflect that it now handles ChromeOS as well.

Also make the has_recognized_platform_authenticator_credential field in
TransportAvailability a base::Optional to reflect that it will be unset
if we're not making a GetAssertionRequest with a present platform
authenticator.

Bug: 1157651
Change-Id: I2fd7d1e88c237c198250cf2e2c5eb0309d014cf7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587559
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840734}
This commit is contained in:
Martin Kreichgauer
2021-01-06 20:03:38 +00:00
committed by Chromium LUCI CQ
parent e2c7dc7720
commit a2eaed9c59
10 changed files with 110 additions and 75 deletions

@ -97,9 +97,9 @@ std::unique_ptr<AuthenticatorRequestSheetView> CreateSheetViewForCurrentStepOf(
std::make_unique<AuthenticatorBlePowerOnManualSheetModel>(
dialog_model));
break;
case Step::kTouchIdIncognitoSpeedBump:
case Step::kPlatformAuthenticatorOffTheRecordInterstitial:
sheet_view = std::make_unique<AuthenticatorRequestSheetView>(
std::make_unique<AuthenticatorTouchIdIncognitoBumpSheetModel>(
std::make_unique<AuthenticatorOffTheRecordInterstitialSheetModel>(
dialog_model));
break;
case Step::kCableActivate:

@ -72,8 +72,8 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
model->SetCurrentStep(
AuthenticatorRequestDialogModel::Step::kBlePowerOnManual);
} else if (name == "touchid_incognito") {
model->SetCurrentStep(
AuthenticatorRequestDialogModel::Step::kTouchIdIncognitoSpeedBump);
model->SetCurrentStep(AuthenticatorRequestDialogModel::Step::
kPlatformAuthenticatorOffTheRecordInterstitial);
} else if (name == "cable_activate") {
model->SetCurrentStep(
AuthenticatorRequestDialogModel::Step::kCableActivate);

@ -470,28 +470,28 @@ void AuthenticatorBlePowerOnAutomaticSheetModel::OnAccept() {
dialog_model()->PowerOnBleAdapter();
}
// AuthenticatorTouchIdIncognitoBumpSheetModel
// AuthenticatorOffTheRecordInterstitialSheetModel
// -----------------------------------------
AuthenticatorTouchIdIncognitoBumpSheetModel::
AuthenticatorTouchIdIncognitoBumpSheetModel(
AuthenticatorOffTheRecordInterstitialSheetModel::
AuthenticatorOffTheRecordInterstitialSheetModel(
AuthenticatorRequestDialogModel* dialog_model)
: AuthenticatorSheetModelBase(dialog_model),
other_transports_menu_model_(std::make_unique<OtherTransportsMenuModel>(
dialog_model,
AuthenticatorTransport::kInternal)) {}
AuthenticatorTouchIdIncognitoBumpSheetModel::
~AuthenticatorTouchIdIncognitoBumpSheetModel() = default;
AuthenticatorOffTheRecordInterstitialSheetModel::
~AuthenticatorOffTheRecordInterstitialSheetModel() = default;
const gfx::VectorIcon&
AuthenticatorTouchIdIncognitoBumpSheetModel::GetStepIllustration(
AuthenticatorOffTheRecordInterstitialSheetModel::GetStepIllustration(
ImageColorScheme color_scheme) const {
return color_scheme == ImageColorScheme::kDark ? kWebauthnPermissionDarkIcon
: kWebauthnPermissionIcon;
}
base::string16 AuthenticatorTouchIdIncognitoBumpSheetModel::GetStepTitle()
base::string16 AuthenticatorOffTheRecordInterstitialSheetModel::GetStepTitle()
const {
#if defined(OS_MAC)
return l10n_util::GetStringFUTF16(IDS_WEBAUTHN_TOUCH_ID_INCOGNITO_BUMP_TITLE,
@ -501,8 +501,8 @@ base::string16 AuthenticatorTouchIdIncognitoBumpSheetModel::GetStepTitle()
#endif // defined(OS_MAC)
}
base::string16 AuthenticatorTouchIdIncognitoBumpSheetModel::GetStepDescription()
const {
base::string16
AuthenticatorOffTheRecordInterstitialSheetModel::GetStepDescription() const {
#if defined(OS_MAC)
return l10n_util::GetStringUTF16(
IDS_WEBAUTHN_TOUCH_ID_INCOGNITO_BUMP_DESCRIPTION);
@ -512,22 +512,22 @@ base::string16 AuthenticatorTouchIdIncognitoBumpSheetModel::GetStepDescription()
}
ui::MenuModel*
AuthenticatorTouchIdIncognitoBumpSheetModel::GetOtherTransportsMenuModel() {
AuthenticatorOffTheRecordInterstitialSheetModel::GetOtherTransportsMenuModel() {
return other_transports_menu_model_.get();
}
bool AuthenticatorTouchIdIncognitoBumpSheetModel::IsAcceptButtonVisible()
bool AuthenticatorOffTheRecordInterstitialSheetModel::IsAcceptButtonVisible()
const {
return true;
}
bool AuthenticatorTouchIdIncognitoBumpSheetModel::IsAcceptButtonEnabled()
bool AuthenticatorOffTheRecordInterstitialSheetModel::IsAcceptButtonEnabled()
const {
return true;
}
base::string16
AuthenticatorTouchIdIncognitoBumpSheetModel::GetAcceptButtonLabel() const {
AuthenticatorOffTheRecordInterstitialSheetModel::GetAcceptButtonLabel() const {
#if defined(OS_MAC)
return l10n_util::GetStringUTF16(
IDS_WEBAUTHN_TOUCH_ID_INCOGNITO_BUMP_CONTINUE);
@ -536,8 +536,8 @@ AuthenticatorTouchIdIncognitoBumpSheetModel::GetAcceptButtonLabel() const {
#endif // defined(OS_MAC)
}
void AuthenticatorTouchIdIncognitoBumpSheetModel::OnAccept() {
dialog_model()->HideDialogAndTryTouchId();
void AuthenticatorOffTheRecordInterstitialSheetModel::OnAccept() {
dialog_model()->HideDialogAndDispatchToPlatformAuthenticator();
}
// AuthenticatorPaaskSheetModel -----------------------------------------

@ -226,12 +226,12 @@ class AuthenticatorBlePowerOnAutomaticSheetModel
bool busy_powering_on_ble_ = false;
};
class AuthenticatorTouchIdIncognitoBumpSheetModel
class AuthenticatorOffTheRecordInterstitialSheetModel
: public AuthenticatorSheetModelBase {
public:
explicit AuthenticatorTouchIdIncognitoBumpSheetModel(
explicit AuthenticatorOffTheRecordInterstitialSheetModel(
AuthenticatorRequestDialogModel* dialog_model);
~AuthenticatorTouchIdIncognitoBumpSheetModel() override;
~AuthenticatorOffTheRecordInterstitialSheetModel() override;
private:
// AuthenticatorSheetModelBase:

@ -43,11 +43,12 @@ base::Optional<device::FidoTransportProtocol> SelectMostLikelyTransport(
return base::nullopt;
}
// Auto advance to Touch ID if the authenticator has a matching credential
// Auto advance to the platform authenticator if it has a matching credential
// for the (possibly empty) allow list.
if (base::Contains(candidate_transports,
device::FidoTransportProtocol::kInternal) &&
transport_availability.has_recognized_mac_touch_id_credential) {
*transport_availability
.has_recognized_platform_authenticator_credential) {
return device::FidoTransportProtocol::kInternal;
}
@ -67,9 +68,10 @@ base::Optional<device::FidoTransportProtocol> SelectMostLikelyTransport(
return base::nullopt;
}
// Auto-advancing to Touch ID based on credential availability has been
// handled above. Hence, at this point it does not have a matching credential
// and should not be advanced to, because it would fail immediately.
// Auto-advancing to platform authenticator based on credential availability
// has been handled above. Hence, at this point it does not have a matching
// credential and should not be advanced to, because it would fail
// immediately.
if (*last_used_transport == device::FidoTransportProtocol::kInternal) {
return base::nullopt;
}
@ -183,7 +185,7 @@ void AuthenticatorRequestDialogModel::StartGuidedFlowForTransport(
SetCurrentStep(Step::kTransportSelection);
break;
case AuthenticatorTransport::kInternal:
StartTouchIdFlow();
StartPlatformAuthenticatorFlow();
break;
case AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy:
EnsureBleAdapterIsPoweredAndContinueWithCable();
@ -279,43 +281,48 @@ void AuthenticatorRequestDialogModel::TryUsbDevice() {
DCHECK_EQ(current_step(), Step::kUsbInsertAndActivate);
}
void AuthenticatorRequestDialogModel::StartTouchIdFlow() {
// Never try Touch ID if the request is known in advance to fail. Proceed to
// a special error screen instead.
void AuthenticatorRequestDialogModel::StartPlatformAuthenticatorFlow() {
// Never try the platform authenticator if the request is known in advance to
// fail. Proceed to a special error screen instead.
if (transport_availability_.request_type ==
device::FidoRequestHandlerBase::RequestType::kGetAssertion &&
!transport_availability_.has_recognized_mac_touch_id_credential) {
SetCurrentStep(Step::kErrorInternalUnrecognized);
return;
device::FidoRequestHandlerBase::RequestType::kGetAssertion) {
DCHECK(transport_availability_
.has_recognized_platform_authenticator_credential);
if (!*transport_availability_
.has_recognized_platform_authenticator_credential) {
SetCurrentStep(Step::kErrorInternalUnrecognized);
return;
}
}
if (transport_availability_.request_type ==
device::FidoRequestHandlerBase::RequestType::kMakeCredential &&
transport_availability_.is_off_the_record_context) {
SetCurrentStep(Step::kTouchIdIncognitoSpeedBump);
SetCurrentStep(Step::kPlatformAuthenticatorOffTheRecordInterstitial);
return;
}
HideDialogAndTryTouchId();
HideDialogAndDispatchToPlatformAuthenticator();
}
void AuthenticatorRequestDialogModel::HideDialogAndTryTouchId() {
void AuthenticatorRequestDialogModel::
HideDialogAndDispatchToPlatformAuthenticator() {
HideDialog();
auto& authenticators =
ephemeral_state_.saved_authenticators_.authenticator_list();
auto touch_id_authenticator_it =
auto platform_authenticator_it =
std::find_if(authenticators.begin(), authenticators.end(),
[](const auto& authenticator) {
return authenticator.transport ==
device::FidoTransportProtocol::kInternal;
});
if (touch_id_authenticator_it == authenticators.end()) {
if (platform_authenticator_it == authenticators.end()) {
return;
}
DispatchRequestAsync(&*touch_id_authenticator_it);
DispatchRequestAsync(&*platform_authenticator_it);
}
void AuthenticatorRequestDialogModel::Cancel() {

@ -72,8 +72,9 @@ class AuthenticatorRequestDialogModel {
kBlePowerOnAutomatic,
kBlePowerOnManual,
// Touch ID.
kTouchIdIncognitoSpeedBump,
// Let the user confirm that they want to create a platform credential in an
// off-the-record browsing context.
kPlatformAuthenticatorOffTheRecordInterstitial,
// Phone as a security key.
kCableActivate,
@ -236,17 +237,18 @@ class AuthenticatorRequestDialogModel {
// Valid action when at step: kUsbInsert.
void TryUsbDevice();
// Tries to use Touch ID -- either because the request requires it or because
// the user told us to. May show an error for unrecognized credential, or an
// Incognito mode interstitial, or proceed straight to the Touch ID prompt.
// Tries to dispatch to the platform authenticator -- either because the
// request requires it or because the user told us to. May show an error for
// unrecognized credential, or an Incognito mode interstitial, or proceed
// straight to the platform authenticator prompt.
//
// Valid action when at all steps.
void StartTouchIdFlow();
void StartPlatformAuthenticatorFlow();
// Proceeds straight to the Touch ID prompt.
// Proceeds straight to the platform authenticator prompt.
//
// Valid action when at all steps.
void HideDialogAndTryTouchId();
void HideDialogAndDispatchToPlatformAuthenticator();
// Cancels the flow as a result of the user clicking `Cancel` on the UI.
//
@ -308,8 +310,8 @@ class AuthenticatorRequestDialogModel {
// credential because of insufficient storage.
void OnAuthenticatorStorageFull();
// To be called when the user denies consent, e.g. by clicking "Cancel" on the
// system Touch ID prompt.
// To be called when the user denies consent, e.g. by canceling out of the
// system's platform authenticator prompt.
void OnUserConsentDenied();
// To be called when the user clicks "Cancel" in the native Windows UI.
@ -426,9 +428,10 @@ class AuthenticatorRequestDialogModel {
// to connect to or conduct WebAuthN request to via the WebAuthN UI.
base::Optional<std::string> selected_authenticator_id_;
// Transport type and id of Mac TouchId and BLE authenticators are cached so
// that the WebAuthN request for the corresponding authenticators can be
// dispatched lazily after the user interacts with the UI element.
// Stores a list of |AuthenticatorReference| values such that a request can
// be dispatched dispatched after some UI interaction. This is useful for
// platform authenticators (and Windows) where dispatch to the authenticator
// immediately results in modal UI to appear.
ObservableAuthenticatorList saved_authenticators_;
// responses_ contains possible accounts to select between.

@ -97,7 +97,7 @@ class AuthenticatorRequestDialogModelTest : public ::testing::Test {
TEST_F(AuthenticatorRequestDialogModelTest, TransportAutoSelection) {
enum class TransportAvailabilityParam {
kHasTouchIdCredential,
kHasPlatformCredential,
kHasWinNativeAuthenticator,
kHasCableExtension,
};
@ -127,7 +127,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportAutoSelection) {
{RequestType::kGetAssertion,
{AuthenticatorTransport::kInternal},
{},
{TransportAvailabilityParam::kHasTouchIdCredential},
{TransportAvailabilityParam::kHasPlatformCredential},
Step::kNotStarted},
{RequestType::kGetAssertion,
{AuthenticatorTransport::kCloudAssistedBluetoothLowEnergy},
@ -154,7 +154,7 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportAutoSelection) {
{RequestType::kGetAssertion,
kAllTransports,
AuthenticatorTransport::kUsbHumanInterfaceDevice,
{TransportAvailabilityParam::kHasTouchIdCredential},
{TransportAvailabilityParam::kHasPlatformCredential},
Step::kNotStarted},
// The KeyChain does not contain an allowed Touch ID credential.
@ -197,13 +197,13 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportAutoSelection) {
{RequestType::kGetAssertion,
{AuthenticatorTransport::kUsbHumanInterfaceDevice},
base::nullopt,
{TransportAvailabilityParam::kHasTouchIdCredential},
{TransportAvailabilityParam::kHasPlatformCredential},
Step::kUsbInsertAndActivate},
{RequestType::kGetAssertion,
{AuthenticatorTransport::kUsbHumanInterfaceDevice,
AuthenticatorTransport::kNearFieldCommunication},
base::nullopt,
{TransportAvailabilityParam::kHasTouchIdCredential},
{TransportAvailabilityParam::kHasPlatformCredential},
Step::kTransportSelection},
// If caBLE is one of the allowed transports, it has second-highest
@ -287,9 +287,9 @@ TEST_F(AuthenticatorRequestDialogModelTest, TransportAutoSelection) {
transports_info.request_type = test_case.request_type;
transports_info.available_transports = test_case.available_transports;
if (base::Contains(test_case.transport_params,
TransportAvailabilityParam::kHasTouchIdCredential))
transports_info.has_recognized_mac_touch_id_credential = true;
transports_info.has_recognized_platform_authenticator_credential =
base::Contains(test_case.transport_params,
TransportAvailabilityParam::kHasPlatformCredential);
if (base::Contains(
test_case.transport_params,

@ -72,7 +72,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoRequestHandlerBase
// relying party.
base::flat_set<FidoTransportProtocol> available_transports;
bool has_recognized_mac_touch_id_credential = false;
// Whether the platform authenticator has a matching credential for the
// request. This is only set for a GetAssertion request and if a platform
// authenticator has been added to the request handler. (The Windows
// WebAuthn API does NOT count as a platform authenticator in this case.)
base::Optional<bool> has_recognized_platform_authenticator_credential;
bool is_ble_powered = false;
bool can_power_on_ble_adapter = false;

@ -82,15 +82,12 @@ class TestObserver : public FidoRequestHandlerBase::Observer {
void WaitForAndExpectAvailableTransportsAre(
base::flat_set<FidoTransportProtocol> expected_transports,
base::Optional<bool> has_recognized_mac_touch_id_credential =
base::nullopt) {
base::Optional<bool> has_platform_credential = base::nullopt) {
auto result = WaitForTransportAvailabilityInfo();
EXPECT_THAT(result.available_transports,
::testing::UnorderedElementsAreArray(expected_transports));
if (has_recognized_mac_touch_id_credential) {
EXPECT_EQ(*has_recognized_mac_touch_id_credential,
result.has_recognized_mac_touch_id_credential);
}
EXPECT_EQ(result.has_recognized_platform_authenticator_credential,
has_platform_credential);
}
protected:
@ -205,6 +202,11 @@ class FakeFidoRequestHandler : public FidoRequestHandlerBase {
}
~FakeFidoRequestHandler() override = default;
void set_has_platform_credential(bool has_platform_credential) {
has_platform_credential_ = has_platform_credential;
}
private:
void DispatchRequest(FidoAuthenticator* authenticator) override {
// FidoRequestHandlerTest uses FakeDiscovery to inject mock devices
// that get wrapped in a FidoDeviceAuthenticator, so we can safely cast
@ -220,7 +222,18 @@ class FakeFidoRequestHandler : public FidoRequestHandlerBase {
weak_factory_.GetWeakPtr(), authenticator)));
}
private:
void AuthenticatorAdded(FidoDiscoveryBase* discovery,
FidoAuthenticator* authenticator) override {
if (authenticator->AuthenticatorTransport() ==
FidoTransportProtocol::kInternal) {
transport_availability_info()
.has_recognized_platform_authenticator_credential =
has_platform_credential_;
}
FidoRequestHandlerBase::AuthenticatorAdded(discovery, authenticator);
}
void HandleResponse(FidoAuthenticator* authenticator,
CtapDeviceResponseCode status,
base::Optional<std::vector<uint8_t>> response) {
@ -246,6 +259,8 @@ class FakeFidoRequestHandler : public FidoRequestHandlerBase {
}
CompletionCallback completion_callback_;
bool has_platform_credential_ = false;
base::WeakPtrFactory<FakeFidoRequestHandler> weak_factory_{this};
};
@ -567,12 +582,13 @@ TEST_F(FidoRequestHandlerTest, TestWithPlatformAuthenticator) {
&fake_discovery_factory_,
base::flat_set<FidoTransportProtocol>({FidoTransportProtocol::kInternal}),
callback().callback());
request_handler->set_has_platform_credential(true);
request_handler->set_observer(&observer);
fake_discovery->AddDevice(std::move(device));
observer.WaitForAndExpectAvailableTransportsAre(
{FidoTransportProtocol::kInternal},
false /* has_recognized_mac_touch_id_credential */);
/*has_platform_credential=*/true);
callback().WaitForCallback();
EXPECT_TRUE(callback().status());

@ -387,17 +387,21 @@ void GetAssertionRequestHandler::AuthenticatorAdded(
// or not. This needs to happen before the base AuthenticatorAdded()
// implementation runs |notify_observer_callback_| for this callback.
if (authenticator->IsTouchIdAuthenticator()) {
transport_availability_info().has_recognized_mac_touch_id_credential =
DCHECK(!transport_availability_info()
.has_recognized_platform_authenticator_credential.has_value());
transport_availability_info()
.has_recognized_platform_authenticator_credential =
static_cast<fido::mac::TouchIdAuthenticator*>(authenticator)
->HasCredentialForGetAssertionRequest(request_);
}
#endif // defined(OS_MAC)
#if BUILDFLAG(IS_CHROMEOS_ASH)
// TODO(martinkr): Put this boolean in a ChromeOS equivalent of
// "has_recognized_mac_touch_id_credential".
if (authenticator->IsChromeOSAuthenticator()) {
transport_availability_info().has_recognized_mac_touch_id_credential =
DCHECK(!transport_availability_info()
.has_recognized_platform_authenticator_credential.has_value());
transport_availability_info()
.has_recognized_platform_authenticator_credential =
static_cast<ChromeOSAuthenticator*>(authenticator)
->HasCredentialForGetAssertionRequest(request_);
}