Move cryptohome callbacks for InSessionPwChange to manager singleton

The class InSessionPasswordChangeHandler manages a dialog where the
user can change their password in-session. If cryptohome password
migration fails, then the handler is notified via a callback. This
callback is where we would help the user to solve the problem.

However, the Handler object will not outlive the dialog, so if the
user closes the dialog, then the callback will not be called, and
the password change will never be fully completed.

To fix this problem, this CL moves the callbacks to a
PrimaryProfileService - the InSessionPasswordChangeManager.
No actual functionality is changed - it just means that these
callbacks (which don't yet do a lot) will still be called even
if the dialog is closed.

Bug: 930109
Change-Id: I520744406251645c6a98b4ea20acd5a19088d1d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1655649
Commit-Queue: A Olsen <olsen@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670203}

chrome/browser/browser_process_platform_part_chromeos.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/chrome_service_name.h"
 #include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h"
+#include "chrome/browser/chromeos/login/saml/in_session_password_change_manager.h"
 #include "chrome/browser/chromeos/login/session/chrome_session_manager.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager_impl.h"
 #include "chrome/browser/chromeos/net/delay_network_call.h"
@@ -160,6 +161,12 @@ void BrowserProcessPlatformPart::InitializePrimaryProfileServices(
   kerberos_credentials_manager_ =
       std::make_unique<chromeos::KerberosCredentialsManager>(
           g_browser_process->local_state(), primary_user);
+
+  DCHECK(!in_session_password_change_manager_);
+  in_session_password_change_manager_ =
+      chromeos::InSessionPasswordChangeManager::CreateIfEnabled(primary_profile,
+                                                                primary_user);
+
   primary_profile_shutdown_subscription_ =
       PrimaryProfileServicesShutdownNotifierFactory::GetInstance()
           ->Get(primary_profile)
@@ -170,6 +177,7 @@ void BrowserProcessPlatformPart::InitializePrimaryProfileServices(
 
 void BrowserProcessPlatformPart::ShutdownPrimaryProfileServices() {
   kerberos_credentials_manager_.reset();
+  in_session_password_change_manager_.reset();
 }
 
 void BrowserProcessPlatformPart::RegisterKeepAlive() {

chrome/browser/browser_process_platform_part_chromeos.h

@@ -22,6 +22,7 @@ class AccountManagerFactory;
 class ChromeSessionManager;
 class ChromeUserManager;
 class KerberosCredentialsManager;
+class InSessionPasswordChangeManager;
 class ProfileHelper;
 class TimeZoneResolver;
 
@@ -123,6 +124,11 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
 
   chromeos::AccountManagerFactory* GetAccountManagerFactory();
 
+  chromeos::InSessionPasswordChangeManager*
+  in_session_password_change_manager() {
+    return in_session_password_change_manager_.get();
+  }
+
  private:
   friend class BrowserProcessPlatformPartTestApi;
 
@@ -164,6 +170,9 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase {
   std::unique_ptr<chromeos::KerberosCredentialsManager>
       kerberos_credentials_manager_;
 
+  std::unique_ptr<chromeos::InSessionPasswordChangeManager>
+      in_session_password_change_manager_;
+
   std::unique_ptr<KeyedServiceShutdownNotifier::Subscription>
       primary_profile_shutdown_subscription_;
 

chrome/browser/chromeos/BUILD.gn

@@ -1253,6 +1253,8 @@ source_set("chromeos") {
     "login/quick_unlock/quick_unlock_utils.h",
     "login/reauth_stats.cc",
     "login/reauth_stats.h",
+    "login/saml/in_session_password_change_manager.cc",
+    "login/saml/in_session_password_change_manager.h",
     "login/saml/saml_offline_signin_limiter.cc",
     "login/saml/saml_offline_signin_limiter.h",
     "login/saml/saml_offline_signin_limiter_factory.cc",

chrome/browser/chromeos/login/saml/in_session_password_change_manager.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/login/saml/in_session_password_change_manager.h"
+
+#include "chrome/browser/chromeos/login/auth/chrome_cryptohome_authenticator.h"
+#include "chrome/browser/chromeos/login/saml/saml_password_expiry_notification.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "chromeos/login/auth/user_context.h"
+#include "components/prefs/pref_service.h"
+#include "components/user_manager/user_manager.h"
+
+namespace chromeos {
+
+// static
+std::unique_ptr<InSessionPasswordChangeManager>
+InSessionPasswordChangeManager::CreateIfEnabled(
+    Profile* primary_profile,
+    const user_manager::User* primary_user) {
+  if (primary_profile->GetPrefs()->GetBoolean(
+          prefs::kSamlInSessionPasswordChangeEnabled)) {
+    return std::make_unique<InSessionPasswordChangeManager>(primary_profile,
+                                                            primary_user);
+  }
+  return nullptr;
+}
+
+InSessionPasswordChangeManager::InSessionPasswordChangeManager(
+    Profile* primary_profile,
+    const user_manager::User* primary_user)
+    : primary_profile_(primary_profile),
+      primary_user_(primary_user),
+      authenticator_(new ChromeCryptohomeAuthenticator(this)) {}
+
+InSessionPasswordChangeManager::~InSessionPasswordChangeManager() {}
+
+void InSessionPasswordChangeManager::ChangePassword(
+    const std::string& old_password,
+    const std::string& new_password) {
+  UserContext user_context(*primary_user_);
+  user_context.SetKey(Key(new_password));
+  authenticator_->MigrateKey(user_context, old_password);
+}
+
+void InSessionPasswordChangeManager::OnAuthSuccess(
+    const UserContext& user_context) {
+  VLOG(3) << "Cryptohome password is changed.";
+  user_manager::UserManager::Get()->SaveForceOnlineSignin(
+      user_context.GetAccountId(), false);
+
+  // Clear expiration time from prefs so that we don't keep nagging the user to
+  // change password (until the SAML provider tells us a new expiration time).
+  SamlPasswordAttributes loaded =
+      SamlPasswordAttributes::LoadFromPrefs(primary_profile_->GetPrefs());
+  SamlPasswordAttributes(
+      /*modified_time=*/base::Time::Now(), /*expiration_time=*/base::Time(),
+      loaded.password_change_url())
+      .SaveToPrefs(primary_profile_->GetPrefs());
+  DismissSamlPasswordExpiryNotification(primary_profile_);
+}
+
+void InSessionPasswordChangeManager::OnAuthFailure(const AuthFailure& error) {
+  // TODO(https://crbug.com/930109): Ask user for the old password.
+  VLOG(1) << "Failed to change cryptohome password: " << error.GetErrorString();
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/login/saml/in_session_password_change_manager.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_SAML_IN_SESSION_PASSWORD_CHANGE_MANAGER_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_SAML_IN_SESSION_PASSWORD_CHANGE_MANAGER_H_
+
+#include <memory>
+#include <string>
+
+#include "base/memory/scoped_refptr.h"
+#include "chromeos/login/auth/auth_status_consumer.h"
+
+class Profile;
+
+namespace user_manager {
+class User;
+}
+
+namespace chromeos {
+class CryptohomeAuthenticator;
+class UserContext;
+
+// Manages the flow of changing a password in-session - handles user
+// response from dialogs, and callbacks from subsystems.
+class InSessionPasswordChangeManager : public AuthStatusConsumer {
+ public:
+  // Returns null if in-session password change is disabled.
+  static std::unique_ptr<InSessionPasswordChangeManager> CreateIfEnabled(
+      Profile* primary_profile,
+      const user_manager::User* primary_user);
+
+  InSessionPasswordChangeManager(Profile* primary_profile,
+                                 const user_manager::User* primary_user);
+  ~InSessionPasswordChangeManager() override;
+
+  // Change cryptohome password for primary user.
+  void ChangePassword(const std::string& old_password,
+                      const std::string& new_password);
+
+  // AuthStatusConsumer:
+  void OnAuthFailure(const AuthFailure& error) override;
+  void OnAuthSuccess(const UserContext& user_context) override;
+
+ private:
+  Profile* primary_profile_;
+  const user_manager::User* primary_user_;
+
+  scoped_refptr<CryptohomeAuthenticator> authenticator_;
+
+  DISALLOW_COPY_AND_ASSIGN(InSessionPasswordChangeManager);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_SAML_IN_SESSION_PASSWORD_CHANGE_MANAGER_H_

chrome/browser/ui/webui/chromeos/insession_password_change_handler_chromeos.cc

@@ -9,7 +9,10 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/values.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_platform_part_chromeos.h"
 #include "chrome/browser/chromeos/login/auth/chrome_cryptohome_authenticator.h"
+#include "chrome/browser/chromeos/login/saml/in_session_password_change_manager.h"
 #include "chrome/browser/chromeos/login/saml/saml_password_expiry_notification.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
@@ -57,12 +60,16 @@ void InSessionPasswordChangeHandler::HandleChangePassword(
                                                           true);
   if (new_passwords.GetList().size() == 1 &&
       old_passwords.GetList().size() > 0) {
-    UserContext user_context(*user);
-    user_context.SetKey(Key(new_passwords.GetList()[0].GetString()));
-    authenticator_ = new ChromeCryptohomeAuthenticator(this);
-    authenticator_->MigrateKey(user_context,
-                               old_passwords.GetList()[0].GetString());
+    auto* in_session_password_change_manager =
+        g_browser_process->platform_part()
+            ->in_session_password_change_manager();
+    CHECK(in_session_password_change_manager);
+    in_session_password_change_manager->ChangePassword(
+        old_passwords.GetList()[0].GetString(),
+        new_passwords.GetList()[0].GetString());
   }
+
+  // TODO(https://crbug.com/930109): Failed to scrape passwords - show dialog.
 }
 
 void InSessionPasswordChangeHandler::RegisterMessages() {
@@ -76,29 +83,4 @@ void InSessionPasswordChangeHandler::RegisterMessages() {
                           weak_factory_.GetWeakPtr()));
 }
 
-void InSessionPasswordChangeHandler::OnAuthSuccess(
-    const UserContext& user_context) {
-  VLOG(3) << "Cryptohome password is changed.";
-  user_manager::UserManager::Get()->SaveForceOnlineSignin(
-      user_context.GetAccountId(), false);
-  authenticator_.reset();
-
-  // Clear expiration time from prefs so that we don't keep nagging the user to
-  // change password (until the SAML provider tells us a new expiration time).
-  Profile* profile = Profile::FromWebUI(web_ui());
-  SamlPasswordAttributes loaded =
-      SamlPasswordAttributes::LoadFromPrefs(profile->GetPrefs());
-  SamlPasswordAttributes(
-      /*modified_time=*/base::Time::Now(), /*expiration_time=*/base::Time(),
-      loaded.password_change_url())
-      .SaveToPrefs(profile->GetPrefs());
-  DismissSamlPasswordExpiryNotification(profile);
-}
-
-void InSessionPasswordChangeHandler::OnAuthFailure(const AuthFailure& error) {
-  // TODO(rsorokin): Ask user for the old password
-  VLOG(1) << "Failed to change cryptohome password: " << error.GetErrorString();
-  authenticator_.reset();
-}
-
 }  // namespace chromeos

chrome/browser/ui/webui/chromeos/insession_password_change_handler_chromeos.h

@@ -7,14 +7,12 @@
 
 #include "base/memory/weak_ptr.h"
 #include "base/values.h"
-#include "chromeos/login/auth/auth_status_consumer.h"
 #include "chromeos/login/auth/cryptohome_authenticator.h"
 #include "content/public/browser/web_ui_message_handler.h"
 
 namespace chromeos {
 
-class InSessionPasswordChangeHandler : public content::WebUIMessageHandler,
-                                       AuthStatusConsumer {
+class InSessionPasswordChangeHandler : public content::WebUIMessageHandler {
  public:
   explicit InSessionPasswordChangeHandler(
       const std::string& password_change_url);
@@ -26,10 +24,6 @@ class InSessionPasswordChangeHandler : public content::WebUIMessageHandler,
   void HandleInitialize(const base::ListValue*);
   void HandleChangePassword(const base::ListValue* passwords);
 
-  // AuthStatusConsumer:
-  void OnAuthFailure(const AuthFailure& error) override;
-  void OnAuthSuccess(const UserContext& user_context) override;
-
  private:
   const std::string password_change_url_;
   scoped_refptr<CryptohomeAuthenticator> authenticator_;