Move the code to get the robot account ID to DeviceO2TS

This CL is a part of moving access token management to
OAuth2AccessTokenManager.

It is specifically a step toward folding
DeviceO2TSDelegate into DeviceO2TS now that the latter
is no longer an O2TS subclass.

It moves GetRobotAccountId(), CheckRobotAccountId(),
OnServiceAccountIdentityChanged(), and
|service_account_identity_subscription_| which are
related to the robot account ID.

Bug: 967598
Change-Id: I63ffc5c9ee6f56bdadfdaf47b5b2eb870c77187f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1694901
Commit-Queue: Julie Jeongeun Kim <jkim@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676342}

chrome/browser/chromeos/settings/device_oauth2_token_service.cc

@@ -15,6 +15,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/settings/cros_settings_names.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "google_apis/gaia/oauth2_access_token_fetcher.h"
@@ -49,7 +50,14 @@ void DeviceOAuth2TokenService::OnValidationCompleted(
 
 DeviceOAuth2TokenService::DeviceOAuth2TokenService(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
-    PrefService* local_state) {
+    PrefService* local_state)
+    : service_account_identity_subscription_(
+          CrosSettings::Get()->AddSettingsObserver(
+              kServiceAccountIdentity,
+              base::Bind(
+                  &DeviceOAuth2TokenService::OnServiceAccountIdentityChanged,
+                  base::Unretained(this)))),
+      weak_ptr_factory_(this) {
   delegate_ = std::make_unique<DeviceOAuth2TokenServiceDelegate>(
       url_loader_factory, local_state, this);
   token_manager_ = std::make_unique<OAuth2AccessTokenManager>(
@@ -74,13 +82,19 @@ void DeviceOAuth2TokenService::SetAndSaveRefreshToken(
   delegate_->SetAndSaveRefreshToken(refresh_token, result_callback);
 }
 
-std::string DeviceOAuth2TokenService::GetRobotAccountId() const {
-  return delegate_->GetRobotAccountId();
+CoreAccountId DeviceOAuth2TokenService::GetRobotAccountId() const {
+  if (!robot_account_id_for_testing_.empty()) {
+    return robot_account_id_for_testing_;
+  }
+
+  std::string account_id;
+  CrosSettings::Get()->GetString(kServiceAccountIdentity, &account_id);
+  return CoreAccountId(account_id);
 }
 
 void DeviceOAuth2TokenService::set_robot_account_id_for_testing(
     const CoreAccountId& account_id) {
-  delegate_->set_robot_account_id_for_testing(account_id);
+  robot_account_id_for_testing_ = account_id;
 }
 
 void DeviceOAuth2TokenService::SetRefreshTokenAvailableCallback(
@@ -243,4 +257,47 @@ std::vector<CoreAccountId> DeviceOAuth2TokenService::GetAccounts() const {
   return accounts;
 }
 
+void DeviceOAuth2TokenService::OnServiceAccountIdentityChanged() {
+  if (!GetRobotAccountId().empty() && !delegate_->refresh_token_.empty())
+    FireRefreshTokenAvailable(GetRobotAccountId());
+}
+
+void DeviceOAuth2TokenService::CheckRobotAccountId(
+    const CoreAccountId& gaia_robot_id) {
+  // Make sure the value returned by GetRobotAccountId has been validated
+  // against current device settings.
+  switch (CrosSettings::Get()->PrepareTrustedValues(
+      base::Bind(&DeviceOAuth2TokenService::CheckRobotAccountId,
+                 weak_ptr_factory_.GetWeakPtr(), gaia_robot_id))) {
+    case CrosSettingsProvider::TRUSTED:
+      // All good, compare account ids below.
+      break;
+    case CrosSettingsProvider::TEMPORARILY_UNTRUSTED:
+      // The callback passed to PrepareTrustedValues above will trigger a
+      // re-check eventually.
+      return;
+    case CrosSettingsProvider::PERMANENTLY_UNTRUSTED:
+      // There's no trusted account id, which is equivalent to no token present.
+      LOG(WARNING) << "Device settings permanently untrusted.";
+      delegate_->state_ = DeviceOAuth2TokenServiceDelegate::STATE_NO_TOKEN;
+      delegate_->ReportServiceError(GoogleServiceAuthError::USER_NOT_SIGNED_UP);
+      return;
+  }
+
+  CoreAccountId policy_robot_id = GetRobotAccountId();
+  if (policy_robot_id == gaia_robot_id) {
+    delegate_->state_ = DeviceOAuth2TokenServiceDelegate::STATE_TOKEN_VALID;
+    delegate_->ReportServiceError(GoogleServiceAuthError::NONE);
+  } else {
+    if (gaia_robot_id.empty()) {
+      LOG(WARNING) << "Device service account owner in policy is empty.";
+    } else {
+      LOG(WARNING) << "Device service account owner in policy does not match "
+                   << "refresh token owner \"" << gaia_robot_id << "\".";
+    }
+    delegate_->state_ = DeviceOAuth2TokenServiceDelegate::STATE_TOKEN_INVALID;
+    delegate_->ReportServiceError(
+        GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
+  }
+}
 }  // namespace chromeos

chrome/browser/chromeos/settings/device_oauth2_token_service.h

@@ -10,6 +10,8 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h"
 #include "google_apis/gaia/core_account_id.h"
 #include "google_apis/gaia/google_service_auth_error.h"
@@ -52,7 +54,7 @@ class DeviceOAuth2TokenService
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
   // Pull the robot account ID from device policy.
-  virtual std::string GetRobotAccountId() const;
+  CoreAccountId GetRobotAccountId() const;
 
   // Can be used to override the robot account ID for testing purposes. Most
   // common use case is to easily inject a non-empty account ID to make the
@@ -140,6 +142,13 @@ class DeviceOAuth2TokenService
   // Returns a list of accounts based on |state_|.
   std::vector<CoreAccountId> GetAccounts() const;
 
+  // Invoked by CrosSettings when the robot account ID becomes available.
+  void OnServiceAccountIdentityChanged();
+
+  // Checks whether |gaia_robot_id| matches the expected account ID indicated in
+  // device settings.
+  void CheckRobotAccountId(const CoreAccountId& gaia_robot_id);
+
   // TODO(https://crbug.com/967598): Merge DeviceOAuth2TokenServiceDelegate
   // into DeviceOAuth2TokenService.
   std::unique_ptr<DeviceOAuth2TokenServiceDelegate> delegate_;
@@ -153,6 +162,13 @@ class DeviceOAuth2TokenService
   RefreshTokenAvailableCallback on_refresh_token_available_callback_;
   RefreshTokenRevokedCallback on_refresh_token_revoked_callback_;
 
+  std::unique_ptr<CrosSettings::ObserverSubscription>
+      service_account_identity_subscription_;
+
+  CoreAccountId robot_account_id_for_testing_;
+
+  base::WeakPtrFactory<DeviceOAuth2TokenService> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(DeviceOAuth2TokenService);
 };
 

chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.cc

@@ -15,7 +15,6 @@
 #include "chrome/browser/chromeos/settings/token_encryptor.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/cryptohome/system_salt_getter.h"
-#include "chromeos/settings/cros_settings_names.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
@@ -26,11 +25,6 @@
 
 namespace chromeos {
 
-void DeviceOAuth2TokenServiceDelegate::OnServiceAccountIdentityChanged() {
-  if (!GetRobotAccountId().empty() && !refresh_token_.empty())
-    service_->FireRefreshTokenAvailable(GetRobotAccountId());
-}
-
 DeviceOAuth2TokenServiceDelegate::DeviceOAuth2TokenServiceDelegate(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     PrefService* local_state,
@@ -41,12 +35,6 @@ DeviceOAuth2TokenServiceDelegate::DeviceOAuth2TokenServiceDelegate(
       max_refresh_token_validation_retries_(3),
       validation_requested_(false),
       validation_status_delegate_(nullptr),
-      service_account_identity_subscription_(
-          CrosSettings::Get()->AddSettingsObserver(
-              kServiceAccountIdentity,
-              base::Bind(&DeviceOAuth2TokenServiceDelegate::
-                             OnServiceAccountIdentityChanged,
-                         base::Unretained(this)))),
       service_(service),
       weak_ptr_factory_(this) {
   // Pull in the system salt.
@@ -71,8 +59,8 @@ void DeviceOAuth2TokenServiceDelegate::SetAndSaveRefreshToken(
   // If the robot account ID is not available yet, do not announce the token. It
   // will be done from OnServiceAccountIdentityChanged() once the robot account
   // ID becomes available as well.
-  if (!GetRobotAccountId().empty())
-    service_->FireRefreshTokenAvailable(GetRobotAccountId());
+  if (!service_->GetRobotAccountId().empty())
+    service_->FireRefreshTokenAvailable(service_->GetRobotAccountId());
 
   token_save_callbacks_.push_back(result_callback);
   if (!waiting_for_salt) {
@@ -83,16 +71,6 @@ void DeviceOAuth2TokenServiceDelegate::SetAndSaveRefreshToken(
   }
 }
 
-CoreAccountId DeviceOAuth2TokenServiceDelegate::GetRobotAccountId() const {
-  if (!robot_account_id_for_testing_.empty()) {
-    return robot_account_id_for_testing_;
-  }
-
-  std::string account_id;
-  CrosSettings::Get()->GetString(kServiceAccountIdentity, &account_id);
-  return CoreAccountId(account_id);
-}
-
 void DeviceOAuth2TokenServiceDelegate::OnRefreshTokenResponse(
     const std::string& access_token,
     int expires_in_seconds) {
@@ -107,7 +85,7 @@ void DeviceOAuth2TokenServiceDelegate::OnGetTokenInfoResponse(
   token_info->GetString("email", &gaia_robot_id);
   gaia_oauth_client_.reset();
 
-  CheckRobotAccountId(CoreAccountId(gaia_robot_id));
+  service_->CheckRobotAccountId(CoreAccountId(gaia_robot_id));
 }
 
 void DeviceOAuth2TokenServiceDelegate::OnOAuthError() {
@@ -200,46 +178,8 @@ void DeviceOAuth2TokenServiceDelegate::DidGetSystemSalt(
     StartValidation();
 
   // Announce the token.
-  if (!GetRobotAccountId().empty()) {
-    service_->FireRefreshTokenAvailable(GetRobotAccountId());
-  }
-}
-
-void DeviceOAuth2TokenServiceDelegate::CheckRobotAccountId(
-    const CoreAccountId& gaia_robot_id) {
-  // Make sure the value returned by GetRobotAccountId has been validated
-  // against current device settings.
-  switch (CrosSettings::Get()->PrepareTrustedValues(
-      base::Bind(&DeviceOAuth2TokenServiceDelegate::CheckRobotAccountId,
-                 weak_ptr_factory_.GetWeakPtr(), gaia_robot_id))) {
-    case CrosSettingsProvider::TRUSTED:
-      // All good, compare account ids below.
-      break;
-    case CrosSettingsProvider::TEMPORARILY_UNTRUSTED:
-      // The callback passed to PrepareTrustedValues above will trigger a
-      // re-check eventually.
-      return;
-    case CrosSettingsProvider::PERMANENTLY_UNTRUSTED:
-      // There's no trusted account id, which is equivalent to no token present.
-      LOG(WARNING) << "Device settings permanently untrusted.";
-      state_ = STATE_NO_TOKEN;
-      ReportServiceError(GoogleServiceAuthError::USER_NOT_SIGNED_UP);
-      return;
-  }
-
-  CoreAccountId policy_robot_id = GetRobotAccountId();
-  if (policy_robot_id == gaia_robot_id) {
-    state_ = STATE_TOKEN_VALID;
-    ReportServiceError(GoogleServiceAuthError::NONE);
-  } else {
-    if (gaia_robot_id.empty()) {
-      LOG(WARNING) << "Device service account owner in policy is empty.";
-    } else {
-      LOG(WARNING) << "Device service account owner in policy does not match "
-                   << "refresh token owner \"" << gaia_robot_id << "\".";
-    }
-    state_ = STATE_TOKEN_INVALID;
-    ReportServiceError(GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS);
+  if (!service_->GetRobotAccountId().empty()) {
+    service_->FireRefreshTokenAvailable(service_->GetRobotAccountId());
   }
 }
 

chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h

@@ -13,7 +13,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "google_apis/gaia/core_account_id.h"
 #include "google_apis/gaia/gaia_oauth_client.h"
 #include "google_apis/gaia/google_service_auth_error.h"
@@ -52,13 +51,6 @@ class DeviceOAuth2TokenServiceDelegate
   void SetAndSaveRefreshToken(const std::string& refresh_token,
                               const StatusCallback& callback);
 
-  // Pull the robot account ID from device policy.
-  CoreAccountId GetRobotAccountId() const;
-
-  void set_robot_account_id_for_testing(const CoreAccountId& account_id) {
-    robot_account_id_for_testing_ = account_id;
-  }
-
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() const;
   std::unique_ptr<OAuth2AccessTokenFetcher> CreateAccessTokenFetcher(
       const CoreAccountId& account_id,
@@ -98,19 +90,12 @@ class DeviceOAuth2TokenServiceDelegate
     STATE_TOKEN_VALID,
   };
 
-  // Invoked by CrosSettings when the robot account ID becomes available.
-  void OnServiceAccountIdentityChanged();
-
   // Returns the refresh token for the robot account id.
   std::string GetRefreshToken() const;
 
   // Handles completion of the system salt input.
   void DidGetSystemSalt(const std::string& system_salt);
 
-  // Checks whether |gaia_robot_id| matches the expected account ID indicated in
-  // device settings.
-  void CheckRobotAccountId(const CoreAccountId& gaia_robot_id);
-
   // Encrypts and saves the refresh token. Should only be called when the system
   // salt is available.
   void EncryptAndSaveToken();
@@ -155,11 +140,6 @@ class DeviceOAuth2TokenServiceDelegate
 
   std::unique_ptr<gaia::GaiaOAuthClient> gaia_oauth_client_;
 
-  std::unique_ptr<CrosSettings::ObserverSubscription>
-      service_account_identity_subscription_;
-
-  CoreAccountId robot_account_id_for_testing_;
-
   // TODO(https://crbug.com/967598): Completely merge this class into
   // DeviceOAuth2TokenService.
   DeviceOAuth2TokenService* service_;