webauthn: include credential ID in pre-discovery data.

In order to support conditional UI, platform authenticators can
"pre-discover" credentials, i.e. return their metadata before any user
verification.

That metadata didn't include the credential ID but, on Windows, we're
going to need the credential ID in order to trigger a webauthn.dll call
for the specific credential.

This change switches the type of the metadata to a newly added type,
`DiscoverableCredentialMetadata`, which is morally a pair of credential ID
and user entity data.

Change-Id: Ie62902976f4d3cf9990e553e8071e41c9cb39832
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3554481
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Auto-Submit: Adam Langley <agl@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986935}

chrome/browser/password_manager/chrome_webauthn_credentials_delegate.cc

@@ -9,6 +9,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "content/public/common/content_features.h"
+#include "device/fido/discoverable_credential_metadata.h"
 
 #if !BUILDFLAG(IS_ANDROID)
 #include "chrome/browser/webauthn/authenticator_request_scheduler.h"
@@ -89,25 +90,26 @@ void ChromeWebAuthnCredentialsDelegate::RetrieveWebAuthnSuggestions(
 }
 
 void ChromeWebAuthnCredentialsDelegate::OnCredentialsReceived(
-    const std::vector<device::PublicKeyCredentialUserEntity>& credentials) {
+    const std::vector<device::DiscoverableCredentialMetadata>& credentials) {
   std::vector<autofill::Suggestion> suggestions;
   for (const auto& credential : credentials) {
     std::u16string name;
-    if (credential.display_name && !credential.display_name->empty()) {
-      name = base::UTF8ToUTF16(*credential.display_name);
+    if (credential.user.display_name &&
+        !credential.user.display_name->empty()) {
+      name = base::UTF8ToUTF16(*credential.user.display_name);
     } else {
       // TODO(crbug.com/1179014): go through UX review, choose a string, and
       // i18n it.
       name = u"Unknown account";
     }
     autofill::Suggestion suggestion(std::move(name));
-    if (credential.name) {
-      suggestion.label = base::UTF8ToUTF16(*credential.name);
+    if (credential.user.name) {
+      suggestion.label = base::UTF8ToUTF16(*credential.user.name);
     }
     suggestion.icon = "fingerprint";
     suggestion.frontend_id = autofill::POPUP_ITEM_ID_WEBAUTHN_CREDENTIAL;
     suggestion.backend_id =
-        std::string(credential.id.begin(), credential.id.end());
+        std::string(credential.user.id.begin(), credential.user.id.end());
     suggestions.push_back(std::move(suggestion));
   }
   suggestions_ = std::move(suggestions);

chrome/browser/password_manager/chrome_webauthn_credentials_delegate.h

@@ -11,7 +11,7 @@
 #include "components/password_manager/core/browser/webauthn_credentials_delegate.h"
 
 namespace device {
-class PublicKeyCredentialUserEntity;
+class DiscoverableCredentialMetadata;
 }
 
 class ChromePasswordManagerClient;
@@ -44,7 +44,7 @@ class ChromeWebAuthnCredentialsDelegate
   // Callback for providing a list of WebAuthn user entities that can be
   // provided as autofill suggestions.
   void OnCredentialsReceived(
-      const std::vector<device::PublicKeyCredentialUserEntity>& credentials);
+      const std::vector<device::DiscoverableCredentialMetadata>& credentials);
 
   // List of autofill suggestions populated from an authenticator from a call
   // to RetrieveWebAuthnSuggestions, and returned to the client via

chrome/browser/password_manager/chrome_webauthn_credentials_delegate_unittest.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/webauthn/authenticator_request_scheduler.h"
 #include "chrome/browser/webauthn/chrome_authenticator_request_delegate.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "device/fido/discoverable_credential_metadata.h"
 #include "device/fido/public_key_credential_user_entity.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -76,11 +77,18 @@ class ChromeWebAuthnCredentialsDelegateTest
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
-  void SetUserList(std::vector<device::PublicKeyCredentialUserEntity> users) {
+  void SetCredList(std::vector<device::PublicKeyCredentialUserEntity> users) {
+    std::vector<device::DiscoverableCredentialMetadata> creds;
+    std::vector<uint8_t> cred_id(1);
+    for (size_t i = 0; i < users.size(); i++) {
+      cred_id[0] = static_cast<uint8_t>(i);
+      creds.emplace_back(cred_id, std::move(users[i]));
+    }
+
     dialog_model()->StartFlow(
         AuthenticatorRequestDialogModel::TransportAvailabilityInfo(),
         /*use_location_bar_bubble=*/true, /*prefer_native_api=*/false);
-    dialog_model()->ReplaceUserListForTesting(users);
+    dialog_model()->ReplaceCredListForTesting(std::move(creds));
   }
 
   raw_ptr<AuthenticatorRequestDialogModel> dialog_model() {
@@ -102,7 +110,7 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest, RetrieveCredentials) {
   users.emplace_back(device::PublicKeyCredentialUserEntity(
       UserId2(), UserName2(), DisplayName2(), absl::nullopt));
 
-  SetUserList(users);
+  SetCredList(users);
 
   credentials_delegate_->RetrieveWebAuthnSuggestions(base::BindOnce([]() {}));
   task_environment()->RunUntilIdle();
@@ -134,7 +142,7 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest,
   users.emplace_back(device::PublicKeyCredentialUserEntity(
       UserId1(), UserName1(), std::string(), absl::nullopt));
 
-  SetUserList(users);
+  SetCredList(users);
 
   credentials_delegate_->RetrieveWebAuthnSuggestions(base::BindOnce([]() {}));
   task_environment()->RunUntilIdle();
@@ -154,7 +162,7 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest,
   users.emplace_back(device::PublicKeyCredentialUserEntity(
       UserId1(), absl::nullopt, DisplayName1(), absl::nullopt));
 
-  SetUserList(users);
+  SetCredList(users);
 
   credentials_delegate_->RetrieveWebAuthnSuggestions(base::BindOnce([]() {}));
   task_environment()->RunUntilIdle();
@@ -171,7 +179,7 @@ TEST_F(ChromeWebAuthnCredentialsDelegateTest, SelectCredential) {
   users.emplace_back(device::PublicKeyCredentialUserEntity(
       UserId1(), UserName1(), DisplayName1(), absl::nullopt));
 
-  SetUserList(users);
+  SetCredList(users);
 
   credentials_delegate_->SelectWebAuthnCredential("1234");
 

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

@@ -20,7 +20,7 @@ std::pair<std::unique_ptr<views::View>,
 AuthenticatorSelectAccountSheetView::BuildStepSpecificContent() {
   return std::make_pair(
       std::make_unique<HoverListView>(std::make_unique<AccountHoverListModel>(
-          &model()->dialog_model()->users(), this)),
+          &model()->dialog_model()->creds(), this)),
       AutoFocus::kYes);
 }
 

chrome/browser/ui/webauthn/account_hover_list_model.cc

@@ -8,14 +8,14 @@
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/grit/generated_resources.h"
 #include "device/fido/authenticator_get_assertion_response.h"
-#include "device/fido/public_key_credential_user_entity.h"
+#include "device/fido/discoverable_credential_metadata.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/paint_vector_icon.h"
 
 AccountHoverListModel::AccountHoverListModel(
-    const std::vector<device::PublicKeyCredentialUserEntity>* users_list,
+    const std::vector<device::DiscoverableCredentialMetadata>* creds,
     Delegate* delegate)
-    : users_list_(users_list), delegate_(delegate) {}
+    : creds_(creds), delegate_(delegate) {}
 
 AccountHoverListModel::~AccountHoverListModel() = default;
 
@@ -36,21 +36,21 @@ std::vector<int> AccountHoverListModel::GetThrobberTags() const {
 }
 
 std::vector<int> AccountHoverListModel::GetButtonTags() const {
-  std::vector<int> tag_list(users_list_->size());
-  for (size_t i = 0; i < users_list_->size(); ++i)
+  std::vector<int> tag_list(creds_->size());
+  for (size_t i = 0; i < creds_->size(); ++i)
     tag_list[i] = i;
   return tag_list;
 }
 
 std::u16string AccountHoverListModel::GetItemText(int item_tag) const {
-  const device::PublicKeyCredentialUserEntity& user = users_list_->at(item_tag);
+  const device::PublicKeyCredentialUserEntity& user = creds_->at(item_tag).user;
   if (user.display_name && !user.display_name->empty())
     return base::UTF8ToUTF16(user.display_name.value());
   return l10n_util::GetStringUTF16(IDS_WEBAUTHN_UNKNOWN_ACCOUNT);
 }
 
 std::u16string AccountHoverListModel::GetDescriptionText(int item_tag) const {
-  const device::PublicKeyCredentialUserEntity& user = users_list_->at(item_tag);
+  const device::PublicKeyCredentialUserEntity& user = creds_->at(item_tag).user;
   return base::UTF8ToUTF16(user.name.value_or(""));
 }
 
@@ -63,7 +63,7 @@ void AccountHoverListModel::OnListItemSelected(int item_tag) {
 }
 
 size_t AccountHoverListModel::GetPreferredItemCount() const {
-  return users_list_->size();
+  return creds_->size();
 }
 
 bool AccountHoverListModel::StyleForTwoLines() const {

chrome/browser/ui/webauthn/account_hover_list_model.h

@@ -12,7 +12,7 @@
 #include "chrome/browser/ui/webauthn/hover_list_model.h"
 
 namespace device {
-class PublicKeyCredentialUserEntity;
+class DiscoverableCredentialMetadata;
 }
 
 class AccountHoverListModel : public HoverListModel {
@@ -25,7 +25,7 @@ class AccountHoverListModel : public HoverListModel {
   };
 
   AccountHoverListModel(
-      const std::vector<device::PublicKeyCredentialUserEntity>* users_list,
+      const std::vector<device::DiscoverableCredentialMetadata>* creds,
       Delegate* delegate);
 
   AccountHoverListModel(const AccountHoverListModel&) = delete;
@@ -47,7 +47,7 @@ class AccountHoverListModel : public HoverListModel {
   bool StyleForTwoLines() const override;
 
  private:
-  raw_ptr<const std::vector<device::PublicKeyCredentialUserEntity>> users_list_;
+  raw_ptr<const std::vector<device::DiscoverableCredentialMetadata>> creds_;
   const raw_ptr<Delegate> delegate_;
 };
 

chrome/browser/ui/webauthn/authenticator_dialog_browsertest.cc

@@ -249,6 +249,8 @@ class AuthenticatorDialogTest : public DialogBrowserTest {
         device::PublicKeyCredentialUserEntity user({1, 2, 3, 4});
         user.name = info.first;
         user.display_name = info.second;
+        response.credential = device::PublicKeyCredentialDescriptor(
+            device::CredentialType::kPublicKey, {1, 2, 3, 4});
         response.user_entity = std::move(user);
         responses.emplace_back(std::move(response));
       }

chrome/browser/ui/webauthn/sheet_models.cc

@@ -24,6 +24,7 @@
 #include "components/strings/grit/components_strings.h"
 #include "components/url_formatter/elide_url.h"
 #include "device/fido/authenticator_get_assertion_response.h"
+#include "device/fido/discoverable_credential_metadata.h"
 #include "device/fido/features.h"
 #include "device/fido/fido_types.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -1073,7 +1074,7 @@ AuthenticatorSelectAccountSheetModel::~AuthenticatorSelectAccountSheetModel() =
 
 void AuthenticatorSelectAccountSheetModel::SetCurrentSelection(int selected) {
   DCHECK_LE(0, selected);
-  DCHECK_LT(static_cast<size_t>(selected), dialog_model()->users().size());
+  DCHECK_LT(static_cast<size_t>(selected), dialog_model()->creds().size());
   selected_ = selected;
 }
 

chrome/browser/webauthn/authenticator_request_dialog_model.cc

@@ -18,6 +18,7 @@
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/vector_icons/vector_icons.h"
+#include "device/fido/discoverable_credential_metadata.h"
 #include "device/fido/features.h"
 #include "device/fido/fido_authenticator.h"
 #include "device/fido/pin.h"
@@ -147,7 +148,7 @@ AuthenticatorRequestDialogModel::PairedPhone::operator=(const PairedPhone&) =
 void AuthenticatorRequestDialogModel::EphemeralState::Reset() {
   selected_authenticator_id_ = absl::nullopt;
   saved_authenticators_.RemoveAllAuthenticators();
-  users_.clear();
+  creds_.clear();
 }
 
 AuthenticatorRequestDialogModel::AuthenticatorRequestDialogModel(
@@ -559,9 +560,10 @@ void AuthenticatorRequestDialogModel::SelectAccount(
     return;
   }
   ephemeral_state_.responses_ = std::move(responses);
-  ephemeral_state_.users_ = {};
+  ephemeral_state_.creds_ = {};
   for (const auto& response : ephemeral_state_.responses_) {
-    ephemeral_state_.users_.push_back(*response.user_entity);
+    ephemeral_state_.creds_.emplace_back(device::DiscoverableCredentialMetadata(
+        response.credential->id, *response.user_entity));
   }
   selection_callback_ = std::move(callback);
   SetCurrentStep(Step::kSelectAccount);
@@ -576,17 +578,17 @@ void AuthenticatorRequestDialogModel::OnAccountSelected(size_t index) {
 
   device::AuthenticatorGetAssertionResponse response =
       std::move(ephemeral_state_.responses_.at(index));
-  ephemeral_state_.users_.clear();
+  ephemeral_state_.creds_.clear();
   ephemeral_state_.responses_.clear();
   std::move(selection_callback_).Run(std::move(response));
 }
 
 void AuthenticatorRequestDialogModel::OnAccountPreselected(
     const std::vector<uint8_t>& id) {
-  for (const auto& account : users()) {
-    if (account.id == id) {
-      preselected_account_ = std::move(account);
-      ephemeral_state_.users_.clear();
+  for (const auto& cred : creds()) {
+    if (cred.user.id == id) {
+      preselected_account_ = std::move(cred.user);
+      ephemeral_state_.creds_.clear();
       HideDialogAndDispatchToPlatformAuthenticator();
       return;
     }
@@ -693,9 +695,9 @@ void AuthenticatorRequestDialogModel::RequestAttestationPermission(
 
 void AuthenticatorRequestDialogModel::GetCredentialListForConditionalUi(
     base::OnceCallback<void(
-        const std::vector<device::PublicKeyCredentialUserEntity>&)> callback) {
+        const std::vector<device::DiscoverableCredentialMetadata>&)> callback) {
   if (current_step() == Step::kLocationBarBubble) {
-    std::move(callback).Run(ephemeral_state_.users_);
+    std::move(callback).Run(ephemeral_state_.creds_);
     return;
   }
 
@@ -739,9 +741,9 @@ std::vector<std::string> AuthenticatorRequestDialogModel::paired_phone_names()
   return names;
 }
 
-void AuthenticatorRequestDialogModel::ReplaceUserListForTesting(
-    std::vector<device::PublicKeyCredentialUserEntity> users) {
-  ephemeral_state_.users_ = std::move(users);
+void AuthenticatorRequestDialogModel::ReplaceCredListForTesting(
+    std::vector<device::DiscoverableCredentialMetadata> creds) {
+  ephemeral_state_.creds_ = std::move(creds);
 }
 
 absl::optional<device::PublicKeyCredentialUserEntity>
@@ -863,14 +865,14 @@ void AuthenticatorRequestDialogModel::ContactPhoneAfterBleIsPowered(
 }
 
 void AuthenticatorRequestDialogModel::StartLocationBarBubbleRequest() {
-  ephemeral_state_.users_ = {};
-  for (const auto& user :
+  ephemeral_state_.creds_ = {};
+  for (const auto& cred :
        transport_availability_.recognized_platform_authenticator_credentials) {
-    ephemeral_state_.users_.emplace_back(user);
+    ephemeral_state_.creds_.emplace_back(cred);
   }
 
   if (conditional_ui_user_list_callback_) {
-    std::move(conditional_ui_user_list_callback_).Run(ephemeral_state_.users_);
+    std::move(conditional_ui_user_list_callback_).Run(ephemeral_state_.creds_);
   }
 
   SetCurrentStep(Step::kLocationBarBubble);