Skip to content

Commit

Permalink
[WebAuthn] Add PasskeyCredential type to the Password Manager
Browse files Browse the repository at this point in the history 
Currently, WebAuthnCredentialsDelegate provides autofill::Suggestions
to the password autofill code to represent passkeys. This is not an
appropriate use of a type that is intended to be part of the autofill
UI.

This change moves the type currently used in Touch To Fill,
TouchToFillWebAuthnCredential, into components/password_manager and
renames it to PasskeyCredential. PasskeyCredential becomes used to
represent passkeys in password manager code until they need to be
converted to another type for display (Suggestion for autofill,
and the Java passkey class for Touch To Fill).

This also replaces the term 'WebAuthn credential' within the
password manager with 'passkey', since this is becoming the more
recognizable term, and the only WebAuthn credentials that can be
shown in autofill are passkeys.

Bug: 1355568
Change-Id: If12f2cc415806031a924cd85ad82fd90c9a746ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195487
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100391}
  • Loading branch information
@kenrb
kenrb authored and Chromium LUCI CQ committed Feb 2, 2023
1 parent 6e109f3 commit 294fad1
Show file tree
Hide file tree
Showing 33 changed files with 410 additions and 457 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3257,8 +3257,6 @@ static_library("browser") {
"touch_to_fill/touch_to_fill_controller_delegate.h",
"touch_to_fill/touch_to_fill_controller_webauthn_delegate.cc",
"touch_to_fill/touch_to_fill_controller_webauthn_delegate.h",
"touch_to_fill/touch_to_fill_webauthn_credential.cc",
"touch_to_fill/touch_to_fill_webauthn_credential.h",
"translate/android/translate_bridge.cc",
"translate/android/translate_bridge.h",
"usb/android/usb_bridge.cc",
Expand Down
25 changes: 5 additions & 20 deletions chrome/browser/password_manager/chrome_password_manager_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
#include "chrome/browser/safe_browsing/user_interaction_observer.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/sync/sync_service_factory.h"
#include "chrome/browser/touch_to_fill/touch_to_fill_webauthn_credential.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/ui/passwords/password_generation_popup_controller_impl.h"
#include "chrome/browser/ui/passwords/passwords_client_ui_delegate.h"
Expand Down Expand Up @@ -72,6 +71,7 @@
#include "components/password_manager/core/browser/http_auth_manager.h"
#include "components/password_manager/core/browser/http_auth_manager_impl.h"
#include "components/password_manager/core/browser/leak_detection_dialog_utils.h"
#include "components/password_manager/core/browser/passkey_credential.h"
#include "components/password_manager/core/browser/password_bubble_experiment.h"
#include "components/password_manager/core/browser/password_change_success_tracker.h"
#include "components/password_manager/core/browser/password_form.h"
Expand Down Expand Up @@ -468,32 +468,17 @@ void ChromePasswordManagerClient::ShowPasswordManagerErrorMessage(
void ChromePasswordManagerClient::ShowTouchToFill(
PasswordManagerDriver* driver,
autofill::mojom::SubmissionReadinessState submission_readiness) {
std::vector<TouchToFillWebAuthnCredential> webauthn_credentials;
auto* webauthn_delegate = GetWebAuthnCredentialsDelegateForDriver(driver);
if (webauthn_delegate) {
const absl::optional<std::vector<autofill::Suggestion>>& suggestions =
webauthn_delegate->GetWebAuthnSuggestions();
if (suggestions.has_value()) {
base::ranges::transform(*suggestions,
std::back_inserter(webauthn_credentials),
[](const auto& suggestion) {
return TouchToFillWebAuthnCredential(
TouchToFillWebAuthnCredential::Username(
suggestion.main_text.value),
TouchToFillWebAuthnCredential::BackendId(
(suggestion.template GetPayload<
autofill::Suggestion::BackendId>())
.value()));
});
}
std::vector<password_manager::PasskeyCredential> passkeys;
if (webauthn_delegate && webauthn_delegate->GetPasskeys().has_value()) {
passkeys = *webauthn_delegate->GetPasskeys();
}

GetOrCreateTouchToFillController()->Show(
credential_cache_
.GetCredentialStore(url::Origin::Create(
driver->GetLastCommittedURL().DeprecatedGetOriginAsURL()))
.GetCredentials(),
webauthn_credentials,
passkeys,
std::make_unique<TouchToFillControllerAutofillDelegate>(
this, GetBiometricAuthenticator(), driver->AsWeakPtr(),
submission_readiness));
Expand Down
48 changes: 21 additions & 27 deletions chrome/browser/password_manager/chrome_webauthn_credentials_delegate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/functional/callback.h"
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "components/password_manager/core/browser/passkey_credential.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/browser/web_contents.h"
Expand All @@ -24,6 +25,8 @@
#include "chrome/browser/webauthn/android/webauthn_request_delegate_android.h"
#endif

using password_manager::PasskeyCredential;

ChromeWebAuthnCredentialsDelegate::ChromeWebAuthnCredentialsDelegate(
content::WebContents* web_contents)
: web_contents_(web_contents) {}
Expand All @@ -42,8 +45,8 @@ void ChromeWebAuthnCredentialsDelegate::LaunchWebAuthnFlow() {
#endif // !BUILDFLAG(IS_ANDROID)
}

void ChromeWebAuthnCredentialsDelegate::SelectWebAuthnCredential(
std::string backend_id) {
void ChromeWebAuthnCredentialsDelegate::SelectPasskey(
const std::string& backend_id) {
// `backend_id` is the base64-encoded credential ID. See
// `OnCredentialsReceived()` for where these are encoded.
absl::optional<std::vector<uint8_t>> selected_credential_id =
Expand All @@ -68,25 +71,25 @@ void ChromeWebAuthnCredentialsDelegate::SelectWebAuthnCredential(
#endif // BUILDFLAG(IS_ANDROID)
}

const absl::optional<std::vector<autofill::Suggestion>>&
ChromeWebAuthnCredentialsDelegate::GetWebAuthnSuggestions() const {
return suggestions_;
const absl::optional<std::vector<PasskeyCredential>>&
ChromeWebAuthnCredentialsDelegate::GetPasskeys() const {
return passkeys_;
}

void ChromeWebAuthnCredentialsDelegate::RetrieveWebAuthnSuggestions(
void ChromeWebAuthnCredentialsDelegate::RetrievePasskeys(
base::OnceClosure callback) {
if (suggestions_.has_value()) {
if (passkeys_.has_value()) {
// Entries were already populated from the WebAuthn request.
std::move(callback).Run();
return;
}

retrieve_suggestions_callback_ = std::move(callback);
retrieve_passkeys_callback_ = std::move(callback);
}

void ChromeWebAuthnCredentialsDelegate::OnCredentialsReceived(
const std::vector<device::DiscoverableCredentialMetadata>& credentials) {
std::vector<autofill::Suggestion> suggestions;
std::vector<PasskeyCredential> passkeys;

for (const auto& credential : credentials) {
std::u16string name;
Expand All @@ -95,30 +98,21 @@ void ChromeWebAuthnCredentialsDelegate::OnCredentialsReceived(
} else {
name = l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_EMPTY_LOGIN);
}
autofill::Suggestion suggestion(std::move(name));

std::u16string label = l10n_util::GetStringUTF16(
password_manager::GetPlatformAuthenticatorLabel());
if (!label.empty()) {
suggestion.labels = {{autofill::Suggestion::Text(label)}};
}
suggestion.icon = "globeIcon";
suggestion.frontend_id = autofill::POPUP_ITEM_ID_WEBAUTHN_CREDENTIAL;
suggestion.payload =
autofill::Suggestion::BackendId(base::Base64Encode(credential.cred_id));
suggestions.push_back(std::move(suggestion));
passkeys.emplace_back(
PasskeyCredential::Username(name),
PasskeyCredential::BackendId(base::Base64Encode(credential.cred_id)));
}

suggestions_ = std::move(suggestions);
passkeys_ = std::move(passkeys);

if (retrieve_suggestions_callback_) {
std::move(retrieve_suggestions_callback_).Run();
if (retrieve_passkeys_callback_) {
std::move(retrieve_passkeys_callback_).Run();
}
}

void ChromeWebAuthnCredentialsDelegate::NotifyWebAuthnRequestAborted() {
suggestions_ = absl::nullopt;
if (retrieve_suggestions_callback_) {
std::move(retrieve_suggestions_callback_).Run();
passkeys_ = absl::nullopt;
if (retrieve_passkeys_callback_) {
std::move(retrieve_passkeys_callback_).Run();
}
}
20 changes: 10 additions & 10 deletions chrome/browser/password_manager/chrome_webauthn_credentials_delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/functional/callback.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "components/password_manager/core/browser/passkey_credential.h"
#include "components/password_manager/core/browser/webauthn_credentials_delegate.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

Expand All @@ -33,10 +34,10 @@ class ChromeWebAuthnCredentialsDelegate

// password_manager::WebAuthnCredentialsDelegate:
void LaunchWebAuthnFlow() override;
void SelectWebAuthnCredential(std::string backend_id) override;
const absl::optional<std::vector<autofill::Suggestion>>&
GetWebAuthnSuggestions() const override;
void RetrieveWebAuthnSuggestions(base::OnceClosure callback) override;
void SelectPasskey(const std::string& backend_id) override;
const absl::optional<std::vector<password_manager::PasskeyCredential>>&
GetPasskeys() const override;
void RetrievePasskeys(base::OnceClosure callback) override;

// Method for providing a list of WebAuthn user entities that can be provided
// as autofill suggestions. This is called when a WebAuthn Conditional UI
Expand All @@ -52,14 +53,13 @@ class ChromeWebAuthnCredentialsDelegate
const raw_ptr<content::WebContents> web_contents_;

private:
// List of autofill suggestions populated from an authenticator from a call
// to RetrieveWebAuthnSuggestions, and returned to the client via
// GetWebAuthnSuggestions.
// |suggestions_| is nullopt until populated by a WebAuthn request, and reset
// List of passkeys populated from an authenticator from a call to
// RetrievePasskeys, and returned to the client via GetPasskeys.
// |passkeys_| is nullopt until populated by a WebAuthn request, and reset
// to nullopt when the request is cancelled.
absl::optional<std::vector<autofill::Suggestion>> suggestions_;
absl::optional<std::vector<password_manager::PasskeyCredential>> passkeys_;

base::OnceClosure retrieve_suggestions_callback_;
base::OnceClosure retrieve_passkeys_callback_;

base::WeakPtrFactory<ChromeWebAuthnCredentialsDelegate> weak_ptr_factory_{
this};
Expand Down
81 changes: 38 additions & 43 deletions chrome/browser/password_manager/chrome_webauthn_credentials_delegate_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#include "chrome/browser/password_manager/chrome_webauthn_credentials_delegate_factory.h"
#include "chrome/browser/webauthn/authenticator_request_dialog_model.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "components/password_manager/core/browser/password_ui_utils.h"
#include "components/password_manager/core/browser/passkey_credential.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/test/web_contents_tester.h"
#include "device/fido/discoverable_credential_metadata.h"
Expand All @@ -38,6 +38,8 @@
#include "chrome/browser/webauthn/android/webauthn_request_delegate_android.h"
#endif

using password_manager::PasskeyCredential;

namespace {

constexpr uint8_t kUserId1[] = {'1', '2', '3', '4'};
Expand Down Expand Up @@ -156,11 +158,9 @@ class ChromeWebAuthnCredentialsDelegateTest
#endif
};

// Testing retrieving suggestions when there are 2 public key credentials
// Testing retrieving passkeys when there are 2 public key credentials
// present.
TEST_F(ChromeWebAuthnCredentialsDelegateTest, RetrieveCredentials) {
const std::u16string kPlatformAuthenticatorLabel = l10n_util::GetStringUTF16(
password_manager::GetPlatformAuthenticatorLabel());
std::vector<device::DiscoverableCredentialMetadata> users;
users.emplace_back(kRpId, CredId1(),
device::PublicKeyCredentialUserEntity(
Expand All @@ -171,24 +171,22 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest, RetrieveCredentials) {

credentials_delegate_->OnCredentialsReceived(users);

auto suggestions = credentials_delegate_->GetWebAuthnSuggestions();
EXPECT_TRUE(suggestions.has_value());
EXPECT_EQ(suggestions->size(), 2u);
ASSERT_EQ((*suggestions)[0].labels.size(), 1u);
ASSERT_EQ((*suggestions)[1].labels.size(), 1u);
ASSERT_EQ((*suggestions)[0].labels[0].size(), 1u);
ASSERT_EQ((*suggestions)[1].labels[0].size(), 1u);
EXPECT_EQ((*suggestions)[0].main_text.value, base::UTF8ToUTF16(UserName1()));
EXPECT_EQ((*suggestions)[0].labels[0][0].value, kPlatformAuthenticatorLabel);
EXPECT_EQ((*suggestions)[1].main_text.value, base::UTF8ToUTF16(UserName2()));
EXPECT_EQ((*suggestions)[1].labels[0][0].value, kPlatformAuthenticatorLabel);
auto passkeys = credentials_delegate_->GetPasskeys();
ASSERT_TRUE(passkeys.has_value());
EXPECT_THAT(
*passkeys,
testing::ElementsAre(
PasskeyCredential(
PasskeyCredential::Username(base::UTF8ToUTF16(UserName1())),
PasskeyCredential::BackendId(base::Base64Encode(CredId1()))),
PasskeyCredential(
PasskeyCredential::Username(base::UTF8ToUTF16(UserName2())),
PasskeyCredential::BackendId(base::Base64Encode(CredId2())))));
}

// Testing retrieving suggestions when the credentials are not received until
// afterward.
TEST_F(ChromeWebAuthnCredentialsDelegateTest, RetrieveCredentialsDelayed) {
const std::u16string kPlatformAuthenticatorLabel = l10n_util::GetStringUTF16(
password_manager::GetPlatformAuthenticatorLabel());
std::vector<device::DiscoverableCredentialMetadata> users;
users.emplace_back(kRpId, CredId1(),
device::PublicKeyCredentialUserEntity(
Expand All @@ -199,24 +197,24 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest, RetrieveCredentialsDelayed) {

credentials_delegate_->OnCredentialsReceived(users);

auto suggestions = credentials_delegate_->GetWebAuthnSuggestions();
EXPECT_TRUE(suggestions.has_value());
EXPECT_EQ(suggestions->size(), 2u);
ASSERT_EQ((*suggestions)[0].labels.size(), 1u);
ASSERT_EQ((*suggestions)[1].labels.size(), 1u);
ASSERT_EQ((*suggestions)[0].labels[0].size(), 1u);
ASSERT_EQ((*suggestions)[1].labels[0].size(), 1u);
EXPECT_EQ((*suggestions)[0].main_text.value, base::UTF8ToUTF16(UserName1()));
EXPECT_EQ((*suggestions)[0].labels[0][0].value, kPlatformAuthenticatorLabel);
EXPECT_EQ((*suggestions)[1].main_text.value, base::UTF8ToUTF16(UserName2()));
EXPECT_EQ((*suggestions)[1].labels[0][0].value, kPlatformAuthenticatorLabel);
auto passkeys = credentials_delegate_->GetPasskeys();
ASSERT_TRUE(passkeys.has_value());
EXPECT_THAT(
*passkeys,
testing::ElementsAre(
PasskeyCredential(
PasskeyCredential::Username(base::UTF8ToUTF16(UserName1())),
PasskeyCredential::BackendId(base::Base64Encode(CredId1()))),
PasskeyCredential(
PasskeyCredential::Username(base::UTF8ToUTF16(UserName2())),
PasskeyCredential::BackendId(base::Base64Encode(CredId2())))));
}

// Testing retrieving suggestions when there are no public key credentials
// present.
TEST_F(ChromeWebAuthnCredentialsDelegateTest,
RetrieveCredentialsWithEmptyList) {
auto suggestions = credentials_delegate_->GetWebAuthnSuggestions();
auto suggestions = credentials_delegate_->GetPasskeys();
EXPECT_FALSE(suggestions.has_value());
}

Expand All @@ -226,22 +224,20 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest,
RetrieveCredentialWithNoUserName) {
const std::u16string kErrorLabel =
l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_EMPTY_LOGIN);
const std::u16string kPlatformAuthenticatorLabel = l10n_util::GetStringUTF16(
password_manager::GetPlatformAuthenticatorLabel());
std::vector<device::DiscoverableCredentialMetadata> users;
users.emplace_back(kRpId, CredId1(),
device::PublicKeyCredentialUserEntity(
UserId1(), absl::nullopt, DisplayName1()));

credentials_delegate_->OnCredentialsReceived(users);

auto suggestions = credentials_delegate_->GetWebAuthnSuggestions();
EXPECT_TRUE(suggestions.has_value());
EXPECT_EQ(suggestions->size(), 1u);
EXPECT_EQ((*suggestions)[0].main_text.value, kErrorLabel);
ASSERT_EQ((*suggestions)[0].labels.size(), 1u);
ASSERT_EQ((*suggestions)[0].labels[0].size(), 1u);
EXPECT_EQ((*suggestions)[0].labels[0][0].value, kPlatformAuthenticatorLabel);
auto passkeys = credentials_delegate_->GetPasskeys();
ASSERT_TRUE(passkeys.has_value());
EXPECT_THAT(
*passkeys,
testing::ElementsAre(PasskeyCredential(
PasskeyCredential::Username(kErrorLabel),
PasskeyCredential::BackendId(base::Base64Encode(CredId1())))));
}

// Testing selection of a credential.
Expand All @@ -266,8 +262,7 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest, SelectCredential) {
}));
#endif

credentials_delegate_->SelectWebAuthnCredential(
base::Base64Encode(CredId2()));
credentials_delegate_->SelectPasskey(base::Base64Encode(CredId2()));

#if BUILDFLAG(IS_ANDROID)
auto credential_id = GetSelectedId();
Expand All @@ -283,15 +278,15 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest, AbortRequest) {
UserId1(), UserName1(), DisplayName1()));
credentials_delegate_->OnCredentialsReceived(users);
credentials_delegate_->NotifyWebAuthnRequestAborted();
EXPECT_FALSE(credentials_delegate_->GetWebAuthnSuggestions());
EXPECT_FALSE(credentials_delegate_->GetPasskeys());
}

// Test aborting a request when a retrieve suggestions callback is pending.
TEST_F(ChromeWebAuthnCredentialsDelegateTest, AbortRequestPendingCallback) {
device::test::TestCallbackReceiver<> callback;
credentials_delegate_->RetrieveWebAuthnSuggestions(callback.callback());
credentials_delegate_->RetrievePasskeys(callback.callback());
EXPECT_FALSE(callback.was_called());
credentials_delegate_->NotifyWebAuthnRequestAborted();
EXPECT_TRUE(callback.was_called());
EXPECT_FALSE(credentials_delegate_->GetWebAuthnSuggestions());
EXPECT_FALSE(credentials_delegate_->GetPasskeys());
}

0 comments on commit 294fad1

Please sign in to comment.