Record AccountPasswordsConsent from SigninReauthUI

This CL adds the recording logic for AccountPasswordsConsent to the
SigninReauthUI. This requires:
* Collecting the relevant strings in JS and sending them back to C++.
* Maintaining a mapping between strings and GRD IDs in C++, to convert
  the strings from JS back into IDs.
* Putting the IDs into a AccountPasswordsConsent proto and sending that
  to the ConsentAuditor.

Bug: 1111252
Change-Id: Ic06533fdb361b415398923b12fbc3c289a47e989
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352789
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797726}

chrome/browser/resources/signin/signin_reauth/signin_reauth_app.html

@@ -61,23 +61,31 @@
     padding-inline-start: 16px;
   }
 </style>
+
+<!--
+  Use the 'consent-description' attribute to annotate all the UI elements
+  that are part of the text the user reads before consenting to use passwords
+  from their account. Similarly, use 'consent-confirmation' on the UI element on
+  which user clicks to indicate consent.
+-->
+
 <div id="illustrationContainer">
   <div id="illustration"></div>
   <img src="[[accountImageSrc_]]">
 </div>
 <div id="contentContainer">
-  <h1 id="signinReauthTitle">
+  <h1 id="signinReauthTitle" consent-description>
     $i18n{signinReauthTitle}
   </h1>
   <div class="message-container">
-    <div>$i18n{signinReauthDesc}</div>
+    <div consent-description>$i18n{signinReauthDesc}</div>
   </div>
 </div>
 <div class="action-container">
   <paper-spinner-lite active hidden$="[[!confirmButtonHidden_]]">
   </paper-spinner-lite>
   <cr-button id="confirmButton" class="action-button" on-click="onConfirm_"
-      hidden="[[confirmButtonHidden_]]">
+      hidden="[[confirmButtonHidden_]]" consent-confirmation>
     [[confirmButtonLabel_]]
   </cr-button>
   <cr-button id="cancelButton" on-click="onCancel_"

chrome/browser/resources/signin/signin_reauth/signin_reauth_app.js

@@ -8,6 +8,7 @@ import 'chrome://resources/polymer/v3_0/paper-spinner/paper-spinner-lite.js';
 import './strings.m.js';
 import './signin_shared_css.js';
 
+import {assert, assertNotReached} from 'chrome://resources/js/assert.m.js';
 import {I18nBehavior} from 'chrome://resources/js/i18n_behavior.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {WebUIListenerBehavior} from 'chrome://resources/js/web_ui_listener_behavior.m.js';
@@ -52,9 +53,14 @@ Polymer({
     this.signinReauthBrowserProxy_.initialize();
   },
 
-  /** @private */
-  onConfirm_() {
-    this.signinReauthBrowserProxy_.confirm();
+  /**
+   * @param {!Event} e
+   * @private
+   */
+  onConfirm_(e) {
+    this.signinReauthBrowserProxy_.confirm(
+        this.getConsentDescription_(),
+        this.getConsentConfirmation_(e.composedPath()));
   },
 
   /** @private */
@@ -75,4 +81,31 @@ Polymer({
         this.i18n('signinReauthNextLabel') :
         this.i18n('signinReauthConfirmLabel');
   },
+
+  /** @return {!Array<string>} Text of the consent description elements. */
+  getConsentDescription_() {
+    const consentDescription =
+        Array.from(this.shadowRoot.querySelectorAll('[consent-description]'))
+            .filter(element => element.clientWidth * element.clientHeight > 0)
+            .map(element => element.innerHTML.trim());
+    assert(consentDescription);
+    return consentDescription;
+  },
+
+  /**
+   * @param {!Array<!HTMLElement>} path Path of the click event. Must contain
+   *     a consent confirmation element.
+   * @return {string} The text of the consent confirmation element.
+   * @private
+   */
+  getConsentConfirmation_(path) {
+    for (const element of path) {
+      if (element.nodeType !== Node.DOCUMENT_FRAGMENT_NODE &&
+          element.hasAttribute('consent-confirmation')) {
+        return element.innerHTML.trim();
+      }
+    }
+    assertNotReached('No consent confirmation element found.');
+    return '';
+  },
 });

chrome/browser/resources/signin/signin_reauth/signin_reauth_browser_proxy.js

@@ -17,8 +17,12 @@ export class SigninReauthBrowserProxy {
 
   /**
    * Called when the user confirms the signin reauth dialog.
+   * @param {!Array<string>} description Strings that the user was presented
+   *     with in the UI.
+   * @param {string} confirmation Text of the element that the user
+   *     clicked on.
    */
-  confirm() {}
+  confirm(description, confirmation) {}
 
   /**
    * Called when the user cancels the signin reauth.
@@ -34,8 +38,8 @@ export class SigninReauthBrowserProxyImpl {
   }
 
   /** @override */
-  confirm() {
-    chrome.send('confirm');
+  confirm(description, confirmation) {
+    chrome.send('confirm', [description, confirmation]);
   }
 
   /** @override */

chrome/browser/ui/signin_reauth_view_controller.cc

@@ -13,6 +13,7 @@
 #include "base/optional.h"
 #include "base/task_runner.h"
 #include "base/time/time.h"
+#include "chrome/browser/consent_auditor/consent_auditor_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/reauth_result.h"
 #include "chrome/browser/signin/reauth_tab_helper.h"
@@ -21,6 +22,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/webui/signin/signin_reauth_ui.h"
+#include "components/consent_auditor/consent_auditor.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/site_instance.h"
@@ -125,10 +127,15 @@ void SigninReauthViewController::OnModalSigninClosed() {
   CompleteReauth(signin::ReauthResult::kDismissedByUser);
 }
 
-void SigninReauthViewController::OnReauthConfirmed() {
+void SigninReauthViewController::OnReauthConfirmed(
+    sync_pb::UserConsentTypes::AccountPasswordsConsent consent) {
   if (user_confirmed_reauth_)
     return;
 
+  // Cache the consent. It will be actually recorded later, in CompleteReauth(),
+  // if the user successfully completed the reauth.
+  consent_ = consent;
+
   user_confirmed_reauth_ = true;
   user_confirmed_reauth_time_ = base::TimeTicks::Now();
   OnStateChanged();
@@ -219,6 +226,12 @@ void SigninReauthViewController::CompleteReauth(signin::ReauthResult result) {
     raw_reauth_web_contents_ = nullptr;
   }
 
+  if (result == signin::ReauthResult::kSuccess) {
+    CHECK(consent_.has_value());
+    ConsentAuditorFactory::GetForProfile(browser_->profile())
+        ->RecordAccountPasswordsConsent(account_id_, *consent_);
+  }
+
   signin_ui_util::RecordTransactionalReauthResult(access_point_, result);
   if (reauth_callback_)
     std::move(reauth_callback_).Run(result);

chrome/browser/ui/signin_reauth_view_controller.h

@@ -13,6 +13,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/ui/signin_view_controller_delegate.h"
 #include "components/signin/public/base/signin_metrics.h"
+#include "components/sync/protocol/user_consent_types.pb.h"
 #include "google_apis/gaia/core_account_id.h"
 
 class Browser;
@@ -134,7 +135,8 @@ class SigninReauthViewController
   // Called when the user clicks the confirm button in the reauth confirmation
   // dialog.
   // This happens before the Gaia reauth page is shown.
-  void OnReauthConfirmed();
+  void OnReauthConfirmed(
+      sync_pb::UserConsentTypes::AccountPasswordsConsent consent);
   // Called when the user clicks the cancel button in the reauth confirmation
   // dialog.
   // This happens before the Gaia reauth page is shown.
@@ -199,6 +201,7 @@ class SigninReauthViewController
 
   // The state of the reauth flow.
   bool user_confirmed_reauth_ = false;
+  base::Optional<sync_pb::UserConsentTypes::AccountPasswordsConsent> consent_;
   GaiaReauthPageState gaia_reauth_page_state_ = GaiaReauthPageState::kStarted;
   base::Optional<signin::ReauthResult> gaia_reauth_page_result_;
 

chrome/browser/ui/signin_reauth_view_controller_browsertest.cc

@@ -300,6 +300,7 @@ IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
 IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
                        CancelReauthDialog) {
   ShowReauthPrompt();
+  RedirectGaiaChallengeTo(https_server()->GetURL(kReauthDonePath));
   ASSERT_TRUE(login_ui_test_utils::CancelReauthConfirmationDialog(
       browser(), kReauthDialogTimeout));
   EXPECT_EQ(WaitForReauthResult(), signin::ReauthResult::kDismissedByUser);
@@ -312,17 +313,26 @@ IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
 IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
                        GaiaChallengeLoadFailed) {
   ShowReauthPrompt();
-  ASSERT_TRUE(login_ui_test_utils::ConfirmReauthConfirmationDialog(
-      browser(), kReauthDialogTimeout));
+
+  // Make the Gaia page fail to load.
   const GURL target_url = https_server()->GetURL("/close-socket");
   content::TestNavigationObserver target_content_observer(target_url);
   target_content_observer.WatchExistingWebContents();
   RedirectGaiaChallengeTo(target_url);
   target_content_observer.Wait();
+
   EXPECT_TRUE(browser()->signin_view_controller()->ShowsModalDialog());
   EXPECT_FALSE(target_content_observer.last_navigation_succeeded());
 
-  // Check that |kLoadFailed| is returned as the result.
+  // Now confirm the pre-reauth confirmation dialog, and wait for the Gaia page
+  // (an error page in this case) to show up.
+  ReauthTestObserver reauth_observer(signin_reauth_view_controller());
+  ASSERT_TRUE(login_ui_test_utils::ConfirmReauthConfirmationDialog(
+      browser(), kReauthDialogTimeout));
+  reauth_observer.WaitUntilGaiaReauthPageIsShown();
+
+  // Close the modal dialog and check that |kLoadFailed| is returned as the
+  // result.
   SimulateCloseButtonClick();
   EXPECT_EQ(WaitForReauthResult(), signin::ReauthResult::kLoadFailed);
   EXPECT_THAT(
@@ -337,7 +347,7 @@ IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
 // Tests clicking on the confirm button in the reauth dialog. Reauth completes
 // before the confirmation.
 IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
-                       ConfirmReauthDialog_AfterReauthSuccess) {
+                       ConfirmReauthDialog) {
   ShowReauthPrompt();
   RedirectGaiaChallengeTo(https_server()->GetURL(kReauthDonePath));
   ASSERT_TRUE(login_ui_test_utils::ConfirmReauthConfirmationDialog(
@@ -355,27 +365,6 @@ IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
       kReauthGaiaNavigationDurationFromConfirmClickHistogramName, 1);
 }
 
-// Tests clicking on the confirm button in the reauth dialog. Reauth completes
-// after the confirmation.
-IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
-                       ConfirmReauthDialog_BeforeReauthSuccess) {
-  ShowReauthPrompt();
-  ASSERT_TRUE(login_ui_test_utils::ConfirmReauthConfirmationDialog(
-      browser(), kReauthDialogTimeout));
-  RedirectGaiaChallengeTo(https_server()->GetURL(kReauthDonePath));
-  EXPECT_EQ(WaitForReauthResult(), signin::ReauthResult::kSuccess);
-  histogram_tester()->ExpectUniqueSample(
-      kReauthUserActionHistogramName,
-      SigninReauthViewController::UserAction::kClickConfirmButton, 1);
-  histogram_tester()->ExpectUniqueSample(
-      kReauthUserActionToFillPasswordHistogramName,
-      SigninReauthViewController::UserAction::kClickConfirmButton, 1);
-  histogram_tester()->ExpectTotalCount(
-      kReauthGaiaNavigationDurationFromReauthStartHistogramName, 1);
-  histogram_tester()->ExpectTotalCount(
-      kReauthGaiaNavigationDurationFromConfirmClickHistogramName, 1);
-}
-
 // Tests completing the Gaia reauth challenge in a dialog.
 IN_PROC_BROWSER_TEST_F(SigninReauthViewControllerBrowserTest,
                        CompleteReauthInDialog) {

chrome/browser/ui/webui/signin/login_ui_test_utils.cc

@@ -254,10 +254,12 @@ bool IsElementReady(content::WebContents* web_contents,
       "  window.domAutomationController.send('DocumentNotReady');"
       "} else if (%s == null) {"
       "  window.domAutomationController.send('NotFound');"
+      "} else if (%s.hidden) {"
+      "  window.domAutomationController.send('Hidden');"
       "} else {"
       "  window.domAutomationController.send('Ok');"
       "}",
-      element_selector.c_str());
+      element_selector.c_str(), element_selector.c_str());
   EXPECT_TRUE(content::ExecuteScriptAndExtractString(
       web_contents, find_element_js, &message));
   return message == "Ok";

chrome/browser/ui/webui/signin/signin_reauth_handler.cc

@@ -4,13 +4,20 @@
 
 #include "chrome/browser/ui/webui/signin/signin_reauth_handler.h"
 
+#include <vector>
+
 #include "base/bind.h"
+#include "base/logging.h"
 #include "chrome/browser/ui/signin_reauth_view_controller.h"
+#include "components/sync/protocol/user_consent_types.pb.h"
 #include "content/public/browser/web_ui.h"
 #include "ui/base/webui/web_ui_util.h"
 
-SigninReauthHandler::SigninReauthHandler(SigninReauthViewController* controller)
-    : controller_(controller) {
+SigninReauthHandler::SigninReauthHandler(
+    SigninReauthViewController* controller,
+    base::flat_map<std::string, int> string_to_grd_id_map)
+    : controller_(controller),
+      string_to_grd_id_map_(std::move(string_to_grd_id_map)) {
   DCHECK(controller_);
   controller_observer_.Add(controller_);
 }
@@ -63,10 +70,44 @@ void SigninReauthHandler::HandleInitialize(const base::ListValue* args) {
 
 void SigninReauthHandler::HandleConfirm(const base::ListValue* args) {
   if (controller_)
-    controller_->OnReauthConfirmed();
+    controller_->OnReauthConfirmed(BuildConsent(args));
 }
 
 void SigninReauthHandler::HandleCancel(const base::ListValue* args) {
   if (controller_)
     controller_->OnReauthDismissed();
 }
+
+sync_pb::UserConsentTypes::AccountPasswordsConsent
+SigninReauthHandler::BuildConsent(const base::ListValue* args) const {
+  CHECK_EQ(2U, args->GetSize());
+  base::Value::ConstListView consent_description = args->GetList()[0].GetList();
+  const std::string& consent_confirmation = args->GetList()[1].GetString();
+
+  // The strings returned by the WebUI are not free-form, they must belong into
+  // a pre-determined set of strings (stored in |string_to_grd_id_map_|). As
+  // this has privacy and legal implications, CHECK the integrity of the strings
+  // received from the renderer process before recording the consent.
+  std::vector<int> consent_description_ids;
+  for (const base::Value& description : consent_description) {
+    auto iter = string_to_grd_id_map_.find(description.GetString());
+    CHECK(iter != string_to_grd_id_map_.end()) << "Unexpected string:\n"
+                                               << description.GetString();
+    consent_description_ids.push_back(iter->second);
+  }
+
+  auto iter = string_to_grd_id_map_.find(consent_confirmation);
+  CHECK(iter != string_to_grd_id_map_.end()) << "Unexpected string:\n"
+                                             << consent_confirmation;
+  int consent_confirmation_id = iter->second;
+
+  sync_pb::UserConsentTypes::AccountPasswordsConsent consent;
+  consent.set_confirmation_grd_id(consent_confirmation_id);
+  for (int id : consent_description_ids) {
+    consent.add_description_grd_ids(id);
+  }
+  consent.set_status(sync_pb::UserConsentTypes::ConsentStatus::
+                         UserConsentTypes_ConsentStatus_GIVEN);
+
+  return consent;
+}

chrome/browser/ui/webui/signin/signin_reauth_handler.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_SIGNIN_SIGNIN_REAUTH_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_SIGNIN_SIGNIN_REAUTH_HANDLER_H_
 
+#include "base/containers/flat_map.h"
 #include "chrome/browser/ui/signin_reauth_view_controller.h"
 #include "content/public/browser/web_ui_message_handler.h"
 
@@ -17,7 +18,8 @@ class SigninReauthHandler : public content::WebUIMessageHandler,
                             public SigninReauthViewController::Observer {
  public:
   // Creates a SigninReauthHandler for the |controller|.
-  explicit SigninReauthHandler(SigninReauthViewController* controller);
+  SigninReauthHandler(SigninReauthViewController* controller,
+                      base::flat_map<std::string, int> string_to_grd_id_map);
   ~SigninReauthHandler() override;
 
   SigninReauthHandler(const SigninReauthHandler&) = delete;
@@ -46,12 +48,19 @@ class SigninReauthHandler : public content::WebUIMessageHandler,
   virtual void HandleCancel(const base::ListValue* args);
 
  private:
+  sync_pb::UserConsentTypes::AccountPasswordsConsent BuildConsent(
+      const base::ListValue* args) const;
+
   // May be null if |controller_| gets destroyed earlier than |this|.
   SigninReauthViewController* controller_;
 
   ScopedObserver<SigninReauthViewController,
                  SigninReauthViewController::Observer>
       controller_observer_{this};
+
+  // Mapping between strings displayed in the UI corresponding to this handler
+  // and their respective GRD IDs.
+  base::flat_map<std::string, int> string_to_grd_id_map_;
 };
 
 #endif  // CHROME_BROWSER_UI_WEBUI_SIGNIN_SIGNIN_REAUTH_HANDLER_H_

chrome/browser/ui/webui/signin/signin_reauth_ui.cc

@@ -7,6 +7,7 @@
 #include <string>
 
 #include "base/check.h"
+#include "base/containers/flat_map.h"
 #include "base/optional.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
@@ -24,6 +25,7 @@
 #include "content/public/browser/web_ui_data_source.h"
 #include "google_apis/gaia/core_account_id.h"
 #include "services/network/public/mojom/content_security_policy.mojom.h"
+#include "ui/base/l10n/l10n_util.h"
 #include "ui/base/webui/web_ui_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/resources/grit/webui_resources.h"
@@ -114,16 +116,16 @@ SigninReauthUI::SigninReauthUI(content::WebUI* web_ui)
   source->AddResourcePath(
       "images/signin_reauth_illustration_dark.svg",
       IDR_SIGNIN_REAUTH_IMAGES_ACCOUNT_PASSWORDS_REAUTH_ILLUSTRATION_DARK_SVG);
-  source->AddLocalizedString("signinReauthTitle",
-                             GetReauthTitleStringId(access_point));
-  source->AddLocalizedString("signinReauthDesc",
-                             IDS_ACCOUNT_PASSWORDS_REAUTH_DESC);
-  source->AddLocalizedString("signinReauthConfirmLabel",
-                             GetReauthConfirmButtonLabelStringId(access_point));
-  source->AddLocalizedString("signinReauthNextLabel",
-                             IDS_ACCOUNT_PASSWORDS_REAUTH_NEXT_BUTTON_LABEL);
-  source->AddLocalizedString("signinReauthCloseLabel",
-                             IDS_ACCOUNT_PASSWORDS_REAUTH_CLOSE_BUTTON_LABEL);
+  AddStringResource(source, "signinReauthTitle",
+                    GetReauthTitleStringId(access_point));
+  AddStringResource(source, "signinReauthDesc",
+                    IDS_ACCOUNT_PASSWORDS_REAUTH_DESC);
+  AddStringResource(source, "signinReauthConfirmLabel",
+                    GetReauthConfirmButtonLabelStringId(access_point));
+  AddStringResource(source, "signinReauthNextLabel",
+                    IDS_ACCOUNT_PASSWORDS_REAUTH_NEXT_BUTTON_LABEL);
+  AddStringResource(source, "signinReauthCloseLabel",
+                    IDS_ACCOUNT_PASSWORDS_REAUTH_CLOSE_BUTTON_LABEL);
 
   content::WebUIDataSource::Add(profile, source);
 }
@@ -132,8 +134,28 @@ SigninReauthUI::~SigninReauthUI() = default;
 
 void SigninReauthUI::InitializeMessageHandlerWithReauthController(
     SigninReauthViewController* controller) {
-  web_ui()->AddMessageHandler(
-      std::make_unique<SigninReauthHandler>(controller));
+  web_ui()->AddMessageHandler(std::make_unique<SigninReauthHandler>(
+      controller,
+      base::flat_map<std::string, int>(js_localized_string_to_ids_)));
 }
 
 void SigninReauthUI::InitializeMessageHandlerWithBrowser(Browser* browser) {}
+
+void SigninReauthUI::AddStringResource(content::WebUIDataSource* source,
+                                       base::StringPiece name,
+                                       int ids) {
+  source->AddLocalizedString(name, ids);
+
+  // When the strings are passed to the HTML, the Unicode NBSP symbol (\u00A0)
+  // will be automatically replaced with "&nbsp;". This change must be mirrored
+  // in the string-to-ids map. Note that "\u00A0" is actually two characters,
+  // so we must use base::ReplaceSubstrings* rather than base::ReplaceChars.
+  // TODO(treib): De-dupe this with the similar code in SyncConfirmationUI,
+  // SyncConsentScreenHandler, and possibly other places.
+  std::string sanitized_string =
+      base::UTF16ToUTF8(l10n_util::GetStringUTF16(ids));
+  base::ReplaceSubstringsAfterOffset(&sanitized_string, 0, "\u00A0" /* NBSP */,
+                                     "&nbsp;");
+
+  js_localized_string_to_ids_.emplace_back(sanitized_string, ids);
+}

chrome/browser/ui/webui/signin/signin_reauth_ui.h

@@ -5,6 +5,9 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_SIGNIN_SIGNIN_REAUTH_UI_H_
 #define CHROME_BROWSER_UI_WEBUI_SIGNIN_SIGNIN_REAUTH_UI_H_
 
+#include <string>
+#include <vector>
+
 #include "chrome/browser/ui/webui/signin/signin_web_dialog_ui.h"
 
 class Browser;
@@ -12,6 +15,7 @@ class SigninReauthViewController;
 
 namespace content {
 class WebUI;
+class WebUIDataSource;
 }
 
 // WebUI controller for the signin reauth dialog.
@@ -44,6 +48,18 @@ class SigninReauthUI : public SigninWebDialogUI {
   // This class relies on InitializeMessageHandlerWithReauthController() so this
   // method does nothing.
   void InitializeMessageHandlerWithBrowser(Browser* browser) override;
+
+ private:
+  // Adds a string resource with the given GRD |ids| to the WebUI data |source|
+  // named as |name|. Also stores a reverse mapping from the localized version
+  // of the string to the |ids| in order to later pass it to
+  // SigninReauthHandler.
+  void AddStringResource(content::WebUIDataSource* source,
+                         base::StringPiece name,
+                         int ids);
+
+  // For consent auditing.
+  std::vector<std::pair<std::string, int>> js_localized_string_to_ids_;
 };
 
 #endif  // CHROME_BROWSER_UI_WEBUI_SIGNIN_SIGNIN_REAUTH_UI_H_