Settings: Introduce requestCredentialDetails and make note optional

This CL introduces requestCredentialDetails
which returns a PasswordUiEntry. The API is similar to
requestCredentialDetails. Since the requestCredentialDetails API is
still used (when the Password View subpage is disabled), I am keeping as
it is.

PasswordUiEntry now has the note field as optional, as the followup CLs
will put the note behind authentication.

Bug: 1345899
Change-Id: I94066bf4dcd5d474cdc4d51c880fb1d32710cbd6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3784370
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Viktor Semeniuk <vsemeniuk@google.com>
Commit-Queue: Adem Derinel <derinel@google.com>
Cr-Commit-Position: refs/heads/main@{#1032467}

chrome/browser/extensions/api/passwords_private/passwords_private_api.cc

@@ -124,6 +124,40 @@ void PasswordsPrivateRequestPlaintextPasswordFunction::GotPassword(
           ->id)));
 }
 
+// PasswordsPrivateRequestCredentialDetailsFunction
+ResponseAction PasswordsPrivateRequestCredentialDetailsFunction::Run() {
+  auto parameters =
+      api::passwords_private::RequestCredentialDetails::Params::Create(args());
+  EXTENSION_FUNCTION_VALIDATE(parameters);
+
+  GetDelegate(browser_context())
+      ->RequestCredentialDetails(
+          parameters->id,
+          base::BindOnce(&PasswordsPrivateRequestCredentialDetailsFunction::
+                             GotPasswordUiEntry,
+                         this),
+          GetSenderWebContents());
+
+  // GotPasswordUiEntry() might have responded before we reach this point.
+  return did_respond() ? AlreadyResponded() : RespondLater();
+}
+
+void PasswordsPrivateRequestCredentialDetailsFunction::GotPasswordUiEntry(
+    absl::optional<api::passwords_private::PasswordUiEntry> password_ui_entry) {
+  if (password_ui_entry) {
+    Respond(ArgumentList(
+        api::passwords_private::RequestCredentialDetails::Results::Create(
+            std::move(*password_ui_entry))));
+    return;
+  }
+
+  Respond(Error(base::StringPrintf(
+      "Could not obtain password entry. Either the user is not "
+      "authenticated or no credential with id = %d could be found.",
+      api::passwords_private::RequestCredentialDetails::Params::Create(args())
+          ->id)));
+}
+
 // PasswordsPrivateGetSavedPasswordListFunction
 ResponseAction PasswordsPrivateGetSavedPasswordListFunction::Run() {
   // GetList() can immediately call GotList() (which would Respond() before

chrome/browser/extensions/api/passwords_private/passwords_private_api.h

@@ -97,6 +97,23 @@ class PasswordsPrivateRequestPlaintextPasswordFunction
   void GotPassword(absl::optional<std::u16string> password);
 };
 
+class PasswordsPrivateRequestCredentialDetailsFunction
+    : public ExtensionFunction {
+ public:
+  DECLARE_EXTENSION_FUNCTION("passwordsPrivate.requestCredentialDetails",
+                             PASSWORDSPRIVATE_REQUESTCREDENTIALDETAILS)
+ protected:
+  ~PasswordsPrivateRequestCredentialDetailsFunction() override = default;
+
+  // ExtensionFunction overrides.
+  ResponseAction Run() override;
+
+ private:
+  void GotPasswordUiEntry(
+      absl::optional<api::passwords_private::PasswordUiEntry>
+          password_ui_entry);
+};
+
 class PasswordsPrivateGetSavedPasswordListFunction : public ExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("passwordsPrivate.getSavedPasswordList",

chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc

@@ -200,6 +200,15 @@ IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, RequestPlaintextPasswordFails) {
   EXPECT_TRUE(RunPasswordsSubtest("requestPlaintextPasswordFails")) << message_;
 }
 
+IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, RequestCredentialDetails) {
+  EXPECT_TRUE(RunPasswordsSubtest("requestCredentialDetails")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, RequestCredentialDetailsFails) {
+  ResetPlaintextPassword();
+  EXPECT_TRUE(RunPasswordsSubtest("requestCredentialDetailsFails")) << message_;
+}
+
 IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, GetSavedPasswordList) {
   EXPECT_TRUE(RunPasswordsSubtest("getSavedPasswordList")) << message_;
 }

chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h

@@ -33,6 +33,8 @@ class PasswordsPrivateDelegate : public KeyedService {
  public:
   using PlaintextPasswordCallback =
       base::OnceCallback<void(absl::optional<std::u16string>)>;
+  using RequestCredentialDetailsCallback = base::OnceCallback<void(
+      absl::optional<api::passwords_private::PasswordUiEntry>)>;
 
   using RefreshScriptsIfNecessaryCallback = base::OnceClosure;
 
@@ -118,6 +120,20 @@ class PasswordsPrivateDelegate : public KeyedService {
       PlaintextPasswordCallback callback,
       content::WebContents* web_contents) = 0;
 
+  // Requests the full PasswordUiEntry (with filled password) with the given id.
+  // Returns the full PasswordUiEntry with |callback|. Returns |absl::nullopt|
+  // if no matching credential with |id| is found.
+  // |id| the id created when going over the list of saved passwords.
+  // |reason| The reason why the full PasswordUiEntry is requested.
+  // |callback| The callback that gets invoked with the PasswordUiEntry if it
+  // could be obtained successfully, or absl::nullopt otherwise.
+  // |web_contents| The web content object used as the UI; will be used to show
+  //     an OS-level authentication dialog if necessary.
+  virtual void RequestCredentialDetails(
+      int id,
+      RequestCredentialDetailsCallback callback,
+      content::WebContents* web_contents) = 0;
+
   // Moves a list of passwords currently stored on the device to being stored in
   // the signed-in, non-syncing Google Account. The result of any password is a
   // no-op if any of these is true: |id| is invalid; |id| corresponds to a

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc

@@ -149,6 +149,34 @@ ConvertToPasswordFormStores(
   return {};
 }
 
+extensions::api::passwords_private::PasswordUiEntry
+CreatePasswordUiEntryFromCredentialUiEntry(
+    int id,
+    const CredentialUIEntry& credential) {
+  extensions::api::passwords_private::PasswordUiEntry entry;
+  entry.urls = extensions::CreateUrlCollectionFromCredential(credential);
+  entry.username = base::UTF16ToUTF8(credential.username);
+  // TODO(crbug.com/1345899): Fill the note field after authentication in
+  // OnRequestCredentialDetailsAuthResult
+  entry.note =
+      std::make_unique<std::string>(base::UTF16ToUTF8(credential.note.value));
+  entry.id = id;
+  entry.stored_in = extensions::StoreSetFromCredential(credential);
+  entry.is_android_credential =
+      password_manager::IsValidAndroidFacetURI(credential.signon_realm);
+  if (!credential.federation_origin.opaque()) {
+    std::u16string formatted_origin =
+        url_formatter::FormatOriginForSecurityDisplay(
+            credential.federation_origin,
+            url_formatter::SchemeDisplay::OMIT_CRYPTOGRAPHIC);
+
+    entry.federation_text =
+        std::make_unique<std::string>(l10n_util::GetStringFUTF8(
+            IDS_PASSWORDS_VIA_FEDERATION, formatted_origin));
+  }
+  return entry;
+}
+
 }  // namespace
 
 namespace extensions {
@@ -354,6 +382,23 @@ void PasswordsPrivateDelegateImpl::RequestPlaintextPassword(
           weak_ptr_factory_.GetWeakPtr(), id, reason, std::move(callback)));
 }
 
+void PasswordsPrivateDelegateImpl::RequestCredentialDetails(
+    int id,
+    RequestCredentialDetailsCallback callback,
+    content::WebContents* web_contents) {
+  // Save |web_contents| so that it can be used later when OsReauthCall() is
+  // called. Note: This is safe because the |web_contents| is used before
+  // exiting this method.
+  // TODO(crbug.com/495290): Pass the native window directly to the
+  // reauth-handling code.
+  web_contents_ = web_contents;
+  password_access_authenticator_.EnsureUserIsAuthenticated(
+      GetReauthPurpose(api::passwords_private::PLAINTEXT_REASON_VIEW),
+      base::BindOnce(
+          &PasswordsPrivateDelegateImpl::OnRequestCredentialDetailsAuthResult,
+          weak_ptr_factory_.GetWeakPtr(), id, std::move(callback)));
+}
+
 void PasswordsPrivateDelegateImpl::OsReauthCall(
     password_manager::ReauthPurpose purpose,
     password_manager::PasswordAccessAuthenticator::AuthResultCallback
@@ -393,26 +438,8 @@ void PasswordsPrivateDelegateImpl::SetCredentials(
       current_exception_entry.id = id;
       current_exceptions_.push_back(std::move(current_exception_entry));
     } else {
-      api::passwords_private::PasswordUiEntry entry;
-      entry.urls = CreateUrlCollectionFromCredential(credential);
-      entry.username = base::UTF16ToUTF8(credential.username);
-      entry.note = base::UTF16ToUTF8(credential.note.value);
-      entry.id = id;
-      entry.stored_in = StoreSetFromCredential(credential);
-      entry.is_android_credential =
-          password_manager::IsValidAndroidFacetURI(credential.signon_realm);
-      if (!credential.federation_origin.opaque()) {
-        std::u16string formatted_origin =
-            url_formatter::FormatOriginForSecurityDisplay(
-                credential.federation_origin,
-                url_formatter::SchemeDisplay::OMIT_CRYPTOGRAPHIC);
-
-        entry.federation_text =
-            std::make_unique<std::string>(l10n_util::GetStringFUTF8(
-                IDS_PASSWORDS_VIA_FEDERATION, formatted_origin));
-      }
-
-      current_entries_.push_back(std::move(entry));
+      current_entries_.push_back(
+          CreatePasswordUiEntryFromCredentialUiEntry(id, credential));
     }
   }
 
@@ -648,22 +675,32 @@ void PasswordsPrivateDelegateImpl::OnRequestPlaintextPasswordAuthResult(
   } else {
     std::move(callback).Run(entry->password);
   }
+  EmitHistogramsForCredentialAccess(*entry, reason);
+}
 
-  syncer::SyncService* sync_service = nullptr;
-  if (SyncServiceFactory::HasSyncService(profile_)) {
-    sync_service = SyncServiceFactory::GetForProfile(profile_);
+void PasswordsPrivateDelegateImpl::OnRequestCredentialDetailsAuthResult(
+    int id,
+    RequestCredentialDetailsCallback callback,
+    bool authenticated) {
+  if (!authenticated) {
+    std::move(callback).Run(absl::nullopt);
+    return;
   }
-  if (password_manager::sync_util::IsSyncAccountCredential(
-          entry->url, entry->username, sync_service,
-          IdentityManagerFactory::GetForProfile(profile_))) {
-    base::RecordAction(
-        base::UserMetricsAction("PasswordManager_SyncCredentialShown"));
+
+  const CredentialUIEntry* credential = credential_id_generator_.TryGetKey(id);
+  if (!credential) {
+    std::move(callback).Run(absl::nullopt);
+    return;
   }
 
-  UMA_HISTOGRAM_ENUMERATION(
-      "PasswordManager.AccessPasswordInSettings",
-      ConvertPlaintextReason(reason),
-      password_manager::metrics_util::ACCESS_PASSWORD_COUNT);
+  api::passwords_private::PasswordUiEntry password_ui_entry =
+      CreatePasswordUiEntryFromCredentialUiEntry(id, *credential);
+  password_ui_entry.password =
+      std::make_unique<std::string>(base::UTF16ToUTF8(credential->password));
+  std::move(callback).Run(std::move(password_ui_entry));
+
+  EmitHistogramsForCredentialAccess(
+      *credential, api::passwords_private::PLAINTEXT_REASON_VIEW);
 }
 
 void PasswordsPrivateDelegateImpl::OnExportPasswordsAuthResult(
@@ -718,4 +755,24 @@ void PasswordsPrivateDelegateImpl::InitializeIfNecessary() {
   pre_initialization_callbacks_.clear();
 }
 
+void PasswordsPrivateDelegateImpl::EmitHistogramsForCredentialAccess(
+    const CredentialUIEntry& entry,
+    api::passwords_private::PlaintextReason reason) {
+  syncer::SyncService* sync_service = nullptr;
+  if (SyncServiceFactory::HasSyncService(profile_)) {
+    sync_service = SyncServiceFactory::GetForProfile(profile_);
+  }
+  if (password_manager::sync_util::IsSyncAccountCredential(
+          entry.url, entry.username, sync_service,
+          IdentityManagerFactory::GetForProfile(profile_))) {
+    base::RecordAction(
+        base::UserMetricsAction("PasswordManager_SyncCredentialShown"));
+  }
+
+  UMA_HISTOGRAM_ENUMERATION(
+      "PasswordManager.AccessPasswordInSettings",
+      ConvertPlaintextReason(reason),
+      password_manager::metrics_util::ACCESS_PASSWORD_COUNT);
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h

@@ -25,6 +25,7 @@
 #include "components/password_manager/core/browser/password_access_authenticator.h"
 #include "components/password_manager/core/browser/password_account_storage_settings_watcher.h"
 #include "components/password_manager/core/browser/reauth_purpose.h"
+#include "components/password_manager/core/browser/ui/credential_ui_entry.h"
 #include "components/password_manager/core/browser/ui/export_progress_status.h"
 #include "components/password_manager/core/browser/ui/saved_passwords_presenter.h"
 #include "extensions/browser/extension_function.h"
@@ -75,6 +76,9 @@ class PasswordsPrivateDelegateImpl
                                 api::passwords_private::PlaintextReason reason,
                                 PlaintextPasswordCallback callback,
                                 content::WebContents* web_contents) override;
+  void RequestCredentialDetails(int id,
+                                RequestCredentialDetailsCallback callback,
+                                content::WebContents* web_contents) override;
   void MovePasswordsToAccount(const std::vector<int>& ids,
                               content::WebContents* web_contents) override;
   void ImportPasswords(content::WebContents* web_contents) override;
@@ -162,6 +166,12 @@ class PasswordsPrivateDelegateImpl
       PlaintextPasswordCallback callback,
       bool authenticated);
 
+  // Callback for RequestCredentialDetails() after authentication check.
+  void OnRequestCredentialDetailsAuthResult(
+      int id,
+      RequestCredentialDetailsCallback callback,
+      bool authenticated);
+
   // Callback for ExportPasswords() after authentication check.
   void OnExportPasswordsAuthResult(
       base::OnceCallback<void(const std::string&)> accepted_callback,
@@ -179,6 +189,11 @@ class PasswordsPrivateDelegateImpl
       password_manager::PasswordAccessAuthenticator::AuthResultCallback
           callback);
 
+  // Records user action and emits histogram values for retrieving |entry|.
+  void EmitHistogramsForCredentialAccess(
+      const password_manager::CredentialUIEntry& entry,
+      api::passwords_private::PlaintextReason reason);
+
   // Not owned by this class.
   raw_ptr<Profile> profile_;