Chromad: GetUserKerberosFiles on Chrome startup

Fixes regression introduced in CL:1068184

Also added a browsertest to verify behaviour on user kerberos files
changed D-Bus signal

BUG=chromium:845829
TEST=ExistingUserControllerActiveDirectoryTest.UserKerberosFilesChangedSignalTriggersFileUpdate

Change-Id: Ieeb800f85210c094f34c93a71b0e7502ceae3114
Reviewed-on: https://chromium-review.googlesource.com/1089060Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Roman Sorokin <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567195}

chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc

@@ -99,6 +99,7 @@ AuthPolicyCredentialsManager::AuthPolicyCredentialsManager(Profile* profile)
   StartObserveNetwork();
   account_id_ = user->GetAccountId();
   GetUserStatus();
+  GetUserKerberosFiles();
 
   // Setting environment variables for GSSAPI library.
   std::unique_ptr<base::Environment> env(base::Environment::Create());

chrome/browser/chromeos/login/existing_user_controller_browsertest.cc

@@ -860,6 +860,26 @@ class ExistingUserControllerActiveDirectoryTest
         chromeos::DBusThreadManager::Get()->GetAuthPolicyClient());
   }
 
+  void LoginAdOnline() {
+    ExpectLoginSuccess();
+    UserContext user_context(user_manager::UserType::USER_TYPE_ACTIVE_DIRECTORY,
+                             ad_account_id_);
+    user_context.SetKey(Key(kPassword));
+    user_context.SetUserIDHash(ad_account_id_.GetUserEmail());
+    user_context.SetAuthFlow(UserContext::AUTH_FLOW_ACTIVE_DIRECTORY);
+    ASSERT_EQ(user_manager::UserType::USER_TYPE_ACTIVE_DIRECTORY,
+              user_context.GetUserType());
+    content::WindowedNotificationObserver profile_prepared_observer(
+        chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED,
+        content::NotificationService::AllSources());
+    existing_user_controller()->CompleteLogin(user_context);
+
+    profile_prepared_observer.Wait();
+    KerberosFilesChangeWaiter files_change_waiter(false /* files_must_exist */);
+    files_change_waiter.Wait();
+    CheckKerberosFiles(true /* enable_dns_cname_lookup */);
+  }
+
   void UpdateProviderPolicy(const policy::PolicyMap& policy) {
     policy_provider_.UpdateChromePolicy(policy);
   }
@@ -887,7 +907,7 @@ class ExistingUserControllerActiveDirectoryTest
   std::string GetExpectedKerberosConfig(bool enable_dns_cname_lookup) {
     std::string config(base::StringPrintf(
         kKrb5CnameSettings, enable_dns_cname_lookup ? "true" : "false"));
-    config += "configuration";
+    config += fake_authpolicy_client()->user_kerberos_conf();
     return config;
   }
 
@@ -900,7 +920,7 @@ class ExistingUserControllerActiveDirectoryTest
 
     EXPECT_TRUE(base::ReadFileToString(
         base::FilePath(GetKerberosCredentialsCacheFileName()), &file_contents));
-    EXPECT_EQ(file_contents, "credentials");
+    EXPECT_EQ(file_contents, fake_authpolicy_client()->user_kerberos_creds());
   }
 
   // Applies policy and waits until both config and credentials files changed.
@@ -951,46 +971,14 @@ class ExistingUserControllerActiveDirectoryUserWhitelistTest
 // managed device.
 IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,
                        ActiveDirectoryOnlineLogin_Success) {
-  ExpectLoginSuccess();
-  UserContext user_context(user_manager::UserType::USER_TYPE_ACTIVE_DIRECTORY,
-                           ad_account_id_);
-  user_context.SetKey(Key(kPassword));
-  user_context.SetUserIDHash(ad_account_id_.GetUserEmail());
-  user_context.SetAuthFlow(UserContext::AUTH_FLOW_ACTIVE_DIRECTORY);
-  ASSERT_EQ(user_manager::UserType::USER_TYPE_ACTIVE_DIRECTORY,
-            user_context.GetUserType());
-  content::WindowedNotificationObserver profile_prepared_observer(
-      chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED,
-      content::NotificationService::AllSources());
-  existing_user_controller()->CompleteLogin(user_context);
-
-  profile_prepared_observer.Wait();
-  KerberosFilesChangeWaiter files_change_waiter(false /* files_must_exist */);
-  files_change_waiter.Wait();
-  CheckKerberosFiles(true /* enable_dns_cname_lookup */);
+  LoginAdOnline();
 }
 
 // Tests if DisabledAuthNegotiateCnameLookup changes trigger updating user
 // Kerberos files.
 IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,
                        PolicyChangeTriggersFileUpdate) {
-  ExpectLoginSuccess();
-  UserContext user_context(user_manager::UserType::USER_TYPE_ACTIVE_DIRECTORY,
-                           ad_account_id_);
-  user_context.SetKey(Key(kPassword));
-  user_context.SetUserIDHash(ad_account_id_.GetUserEmail());
-  user_context.SetAuthFlow(UserContext::AUTH_FLOW_ACTIVE_DIRECTORY);
-  ASSERT_EQ(user_manager::UserType::USER_TYPE_ACTIVE_DIRECTORY,
-            user_context.GetUserType());
-  content::WindowedNotificationObserver profile_prepared_observer(
-      chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED,
-      content::NotificationService::AllSources());
-  existing_user_controller()->CompleteLogin(user_context);
-
-  profile_prepared_observer.Wait();
-  KerberosFilesChangeWaiter files_change_waiter(false /* files_must_exist */);
-  files_change_waiter.Wait();
-  CheckKerberosFiles(true /* enable_dns_cname_lookup */);
+  LoginAdOnline();
 
   ApplyPolicyAndWaitFilesChanged(false /* enable_dns_cname_lookup */);
   CheckKerberosFiles(false /* enable_dns_cname_lookup */);
@@ -999,6 +987,18 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,
   CheckKerberosFiles(true /* enable_dns_cname_lookup */);
 }
 
+// Tests if user Kerberos files changed D-Bus signal triggers updating user
+// Kerberos files.
+IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,
+                       UserKerberosFilesChangedSignalTriggersFileUpdate) {
+  LoginAdOnline();
+  KerberosFilesChangeWaiter files_change_waiter(true /* files_must_exist */);
+  fake_authpolicy_client()->SetUserKerberosFiles("new_kerberos_creds",
+                                                 "new_kerberos_config");
+  files_change_waiter.Wait();
+  CheckKerberosFiles(true /* enable_dns_cname_lookup */);
+}
+
 // Tests that Active Directory offline login succeeds on the Active Directory
 // managed device.
 IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,

chromeos/dbus/fake_auth_policy_client.cc

@@ -34,6 +34,8 @@ namespace {
 
 constexpr size_t kMaxMachineNameLength = 15;
 constexpr char kInvalidMachineNameCharacters[] = "\\/:*?\"<>|";
+constexpr char kDefaultKerberosCreds[] = "credentials";
+constexpr char kDefaultKerberosConf[] = "configuration";
 
 void OnStorePolicy(chromeos::AuthPolicyClient::RefreshPolicyCallback callback,
                    bool success) {
@@ -151,6 +153,8 @@ void FakeAuthPolicyClient::AuthenticateUser(
     }
     error = auth_error_;
   }
+  if (error == authpolicy::ERROR_NONE)
+    SetUserKerberosFiles(kDefaultKerberosCreds, kDefaultKerberosConf);
   PostDelayedClosure(base::BindOnce(std::move(callback), error, account_info),
                      dbus_operation_delay_);
 }
@@ -182,8 +186,8 @@ void FakeAuthPolicyClient::GetUserKerberosFiles(
     const std::string& object_guid,
     GetUserKerberosFilesCallback callback) {
   authpolicy::KerberosFiles files;
-  files.set_krb5cc("credentials");
-  files.set_krb5conf("configuration");
+  files.set_krb5cc(user_kerberos_creds());
+  files.set_krb5conf(user_kerberos_conf());
   PostDelayedClosure(
       base::BindOnce(std::move(callback), authpolicy::ERROR_NONE, files),
       dbus_operation_delay_);
@@ -252,12 +256,11 @@ void FakeAuthPolicyClient::ConnectToSignal(
     const std::string& signal_name,
     dbus::ObjectProxy::SignalCallback signal_callback,
     dbus::ObjectProxy::OnConnectedCallback on_connected_callback) {
+  DCHECK_EQ(authpolicy::kUserKerberosFilesChangedSignal, signal_name);
+  DCHECK(!user_kerberos_files_changed_callback_);
+  user_kerberos_files_changed_callback_ = signal_callback;
   std::move(on_connected_callback)
       .Run(authpolicy::kAuthPolicyInterface, signal_name, true /* success */);
-  PostDelayedClosure(
-      base::BindOnce(RunSignalCallback, authpolicy::kAuthPolicyInterface,
-                     signal_name, signal_callback),
-      dbus_operation_delay_);
 }
 
 void FakeAuthPolicyClient::WaitForServiceToBeAvailable(
@@ -271,6 +274,22 @@ void FakeAuthPolicyClient::WaitForServiceToBeAvailable(
   wait_for_service_to_be_available_callbacks_.push_back(std::move(callback));
 }
 
+void FakeAuthPolicyClient::SetUserKerberosFiles(const std::string& creds,
+                                                const std::string& conf) {
+  const bool run_signal =
+      user_kerberos_files_changed_callback_ &&
+      (creds != user_kerberos_creds_ || conf != user_kerberos_conf_);
+  user_kerberos_creds_ = creds;
+  user_kerberos_conf_ = conf;
+  if (run_signal) {
+    PostDelayedClosure(
+        base::BindOnce(RunSignalCallback, authpolicy::kAuthPolicyInterface,
+                       authpolicy::kUserKerberosFilesChangedSignal,
+                       user_kerberos_files_changed_callback_),
+        dbus_operation_delay_);
+  }
+}
+
 void FakeAuthPolicyClient::SetStarted(bool started) {
   started_ = started;
   if (started_) {

chromeos/dbus/fake_auth_policy_client.h

@@ -68,6 +68,13 @@ class CHROMEOS_EXPORT FakeAuthPolicyClient : public AuthPolicyClient {
   void WaitForServiceToBeAvailable(
       dbus::ObjectProxy::WaitForServiceToBeAvailableCallback callback) override;
 
+  // Runs |user_kerberos_files_changed_callback_| if callback is set and files
+  // changed.
+  void SetUserKerberosFiles(const std::string& kerberos_creds,
+                            const std::string& kerberos_conf);
+  const std::string& user_kerberos_conf() { return user_kerberos_conf_; }
+  const std::string& user_kerberos_creds() { return user_kerberos_creds_; }
+
   // Mark service as started. It's getting started by the
   // UpstartClient::StartAuthPolicyService on the Active Directory managed
   // devices. If |started| is true, it triggers calling
@@ -127,6 +134,9 @@ class CHROMEOS_EXPORT FakeAuthPolicyClient : public AuthPolicyClient {
   std::string given_name_;
   std::string machine_name_;
   std::string dm_token_;
+  std::string user_kerberos_creds_;
+  std::string user_kerberos_conf_;
+  dbus::ObjectProxy::SignalCallback user_kerberos_files_changed_callback_;
   authpolicy::ActiveDirectoryUserStatus::PasswordStatus password_status_ =
       authpolicy::ActiveDirectoryUserStatus::PASSWORD_VALID;
   authpolicy::ActiveDirectoryUserStatus::TgtStatus tgt_status_ =