Eliminate DeviceOAuth2TokenService inheriting from OAuth2TokenService

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

DeviceOAuth2TokenService stops inheriting from OAuth2TokenService.
Instead, it inherits from OAuth2TokenServiceObserver and
OAuth2AccessTokenManager::Delegate and holds |token_manager_|.
It exposes StartAccessTokenRequest and InvalidateAccessToken.
These methods are handled through |token_manager_|.

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

chrome/browser/chromeos/policy/remote_commands/crd_host_delegate.cc

@@ -210,7 +210,7 @@ void CRDHostDelegate::FetchOAuthToken(
   oauth_success_callback_ = std::move(success_callback);
   error_callback_ = std::move(error_callback);
 
-  oauth_request_ = oauth_service->StartRequest(
+  oauth_request_ = oauth_service->StartAccessTokenRequest(
       oauth_service->GetRobotAccountId(), scopes, this);
 }
 

chrome/browser/chromeos/settings/device_identity_provider.cc

@@ -56,7 +56,7 @@ ActiveAccountAccessTokenFetcherImpl::ActiveAccountAccessTokenFetcherImpl(
     : OAuth2AccessTokenManager::Consumer(oauth_consumer_name),
       callback_(std::move(callback)) {
   access_token_request_ =
-      token_service->StartRequest(active_account_id, scopes, this);
+      token_service->StartAccessTokenRequest(active_account_id, scopes, this);
 }
 
 ActiveAccountAccessTokenFetcherImpl::~ActiveAccountAccessTokenFetcherImpl() {}

chrome/browser/chromeos/settings/device_oauth2_token_service.cc

@@ -17,6 +17,7 @@
 #include "chrome/common/pref_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"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace chromeos {
@@ -48,12 +49,17 @@ void DeviceOAuth2TokenService::OnValidationCompleted(
 
 DeviceOAuth2TokenService::DeviceOAuth2TokenService(
     std::unique_ptr<DeviceOAuth2TokenServiceDelegate> delegate)
-    : OAuth2TokenService(std::move(delegate)) {
-  GetDeviceDelegate()->InitializeWithValidationStatusDelegate(this);
+    : delegate_(std::move(delegate)) {
+  DCHECK(delegate_);
+  delegate_->AddObserver(this);
+  token_manager_ = std::make_unique<OAuth2AccessTokenManager>(
+      this /* OAuth2AccessTokenManager::Delegate* */);
+  delegate_->InitializeWithValidationStatusDelegate(this);
 }
 
 DeviceOAuth2TokenService::~DeviceOAuth2TokenService() {
-  GetDeviceDelegate()->ClearValidationStatusDelegate();
+  delegate_->RemoveObserver(this);
+  delegate_->ClearValidationStatusDelegate();
   FlushPendingRequests(false, GoogleServiceAuthError::REQUEST_CANCELED);
 }
 
@@ -66,16 +72,16 @@ void DeviceOAuth2TokenService::RegisterPrefs(PrefRegistrySimple* registry) {
 void DeviceOAuth2TokenService::SetAndSaveRefreshToken(
     const std::string& refresh_token,
     const StatusCallback& result_callback) {
-  GetDeviceDelegate()->SetAndSaveRefreshToken(refresh_token, result_callback);
+  delegate_->SetAndSaveRefreshToken(refresh_token, result_callback);
 }
 
 std::string DeviceOAuth2TokenService::GetRobotAccountId() const {
-  return GetDeviceDelegate()->GetRobotAccountId();
+  return delegate_->GetRobotAccountId();
 }
 
 void DeviceOAuth2TokenService::set_robot_account_id_for_testing(
     const CoreAccountId& account_id) {
-  GetDeviceDelegate()->set_robot_account_id_for_testing(account_id);
+  delegate_->set_robot_account_id_for_testing(account_id);
 }
 
 void DeviceOAuth2TokenService::SetRefreshTokenAvailableCallback(
@@ -88,16 +94,78 @@ void DeviceOAuth2TokenService::SetRefreshTokenRevokedCallback(
   on_refresh_token_revoked_callback_ = std::move(callback);
 }
 
+std::unique_ptr<OAuth2AccessTokenManager::Request>
+DeviceOAuth2TokenService::StartAccessTokenRequest(
+    const CoreAccountId& account_id,
+    const OAuth2AccessTokenManager::ScopeSet& scopes,
+    OAuth2AccessTokenManager::Consumer* consumer) {
+  return token_manager_->StartRequest(account_id, scopes, consumer);
+}
+
+void DeviceOAuth2TokenService::InvalidateAccessToken(
+    const CoreAccountId& account_id,
+    const OAuth2AccessTokenManager::ScopeSet& scopes,
+    const std::string& access_token) {
+  token_manager_->InvalidateAccessToken(account_id, scopes, access_token);
+}
+
+bool DeviceOAuth2TokenService::RefreshTokenIsAvailable(
+    const CoreAccountId& account_id) const {
+  return delegate_->RefreshTokenIsAvailable(account_id);
+}
+
+OAuth2AccessTokenManager* DeviceOAuth2TokenService::GetAccessTokenManager() {
+  return token_manager_.get();
+}
+
+std::unique_ptr<OAuth2AccessTokenFetcher>
+DeviceOAuth2TokenService::CreateAccessTokenFetcher(
+    const CoreAccountId& account_id,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    OAuth2AccessTokenConsumer* consumer) {
+  return delegate_->CreateAccessTokenFetcher(account_id, url_loader_factory,
+                                             consumer);
+}
+
+bool DeviceOAuth2TokenService::HasRefreshToken(
+    const CoreAccountId& account_id) const {
+  return RefreshTokenIsAvailable(account_id);
+}
+
+bool DeviceOAuth2TokenService::FixRequestErrorIfPossible() {
+  return delegate_->FixRequestErrorIfPossible();
+}
+
+scoped_refptr<network::SharedURLLoaderFactory>
+DeviceOAuth2TokenService::GetURLLoaderFactory() const {
+  return delegate_->GetURLLoaderFactory();
+}
+
+void DeviceOAuth2TokenService::OnAccessTokenInvalidated(
+    const CoreAccountId& account_id,
+    const std::string& client_id,
+    const std::set<std::string>& scopes,
+    const std::string& access_token) {
+  delegate_->OnAccessTokenInvalidated(account_id, client_id, scopes,
+                                      access_token);
+}
+
+void DeviceOAuth2TokenService::OnAccessTokenFetched(
+    const CoreAccountId& account_id,
+    const GoogleServiceAuthError& error) {
+  // Update the auth error state so auth errors are appropriately communicated
+  // to the user.
+  delegate_->UpdateAuthError(account_id, error);
+}
+
 void DeviceOAuth2TokenService::OnRefreshTokenAvailable(
     const CoreAccountId& account_id) {
-  OAuth2TokenService::OnRefreshTokenAvailable(account_id);
   if (on_refresh_token_available_callback_)
     on_refresh_token_available_callback_.Run(account_id);
 }
 
 void DeviceOAuth2TokenService::OnRefreshTokenRevoked(
     const CoreAccountId& account_id) {
-  OAuth2TokenService::OnRefreshTokenRevoked(account_id);
   if (on_refresh_token_revoked_callback_)
     on_refresh_token_revoked_callback_.Run(account_id);
 }
@@ -109,17 +177,17 @@ bool DeviceOAuth2TokenService::HandleAccessTokenFetch(
     const std::string& client_id,
     const std::string& client_secret,
     const OAuth2AccessTokenManager::ScopeSet& scopes) {
-  switch (GetDeviceDelegate()->state_) {
+  switch (delegate_->state_) {
     case DeviceOAuth2TokenServiceDelegate::STATE_VALIDATION_PENDING:
       // If this is the first request for a token, start validation.
-      GetDeviceDelegate()->StartValidation();
+      delegate_->StartValidation();
       FALLTHROUGH;
     case DeviceOAuth2TokenServiceDelegate::STATE_LOADING:
     case DeviceOAuth2TokenServiceDelegate::STATE_VALIDATION_STARTED:
       // Add a pending request that will be satisfied once validation completes.
       pending_requests_.push_back(new PendingRequest(
           request->AsWeakPtr(), client_id, client_secret, scopes));
-      GetDeviceDelegate()->RequestValidation();
+      delegate_->RequestValidation();
       return true;
     case DeviceOAuth2TokenServiceDelegate::STATE_NO_TOKEN:
       FailRequest(request, GoogleServiceAuthError::USER_NOT_SIGNED_UP);
@@ -132,7 +200,7 @@ bool DeviceOAuth2TokenService::HandleAccessTokenFetch(
       return false;
   }
 
-  NOTREACHED() << "Unexpected state " << GetDeviceDelegate()->state_;
+  NOTREACHED() << "Unexpected state " << delegate_->state_;
   return false;
 }
 
@@ -149,10 +217,10 @@ void DeviceOAuth2TokenService::FlushPendingRequests(
       continue;
 
     if (token_is_valid) {
-      GetAccessTokenManager()->FetchOAuth2Token(
+      token_manager_->FetchOAuth2Token(
           scoped_request->request.get(),
           scoped_request->request->GetAccountId(),
-          GetDeviceDelegate()->GetURLLoaderFactory(), scoped_request->client_id,
+          delegate_->GetURLLoaderFactory(), scoped_request->client_id,
           scoped_request->client_secret, scoped_request->scopes);
     } else {
       FailRequest(scoped_request->request.get(), error);
@@ -175,15 +243,4 @@ void DeviceOAuth2TokenService::FailRequest(
                      request->AsWeakPtr(), auth_error,
                      OAuth2AccessTokenConsumer::TokenResponse()));
 }
-
-DeviceOAuth2TokenServiceDelegate*
-DeviceOAuth2TokenService::GetDeviceDelegate() {
-  return static_cast<DeviceOAuth2TokenServiceDelegate*>(GetDelegate());
-}
-
-const DeviceOAuth2TokenServiceDelegate*
-DeviceOAuth2TokenService::GetDeviceDelegate() const {
-  return static_cast<const DeviceOAuth2TokenServiceDelegate*>(GetDelegate());
-}
-
 }  // namespace chromeos

chrome/browser/chromeos/settings/device_oauth2_token_service.h

@@ -11,7 +11,11 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service_delegate.h"
-#include "google_apis/gaia/oauth2_token_service.h"
+#include "google_apis/gaia/core_account_id.h"
+#include "google_apis/gaia/google_service_auth_error.h"
+#include "google_apis/gaia/oauth2_access_token_consumer.h"
+#include "google_apis/gaia/oauth2_access_token_manager.h"
+#include "google_apis/gaia/oauth2_token_service_observer.h"
 
 class PrefRegistrySimple;
 
@@ -20,15 +24,13 @@ namespace chromeos {
 // DeviceOAuth2TokenService retrieves OAuth2 access tokens for a given
 // set of scopes using the device-level OAuth2 any-api refresh token
 // obtained during enterprise device enrollment.
-//
-// See |OAuth2TokenService| for usage details.
-//
 // When using DeviceOAuth2TokenService, a value of |GetRobotAccountId| should
 // be used in places where API expects |account_id|.
 //
 // Note that requests must be made from the UI thread.
 class DeviceOAuth2TokenService
-    : public OAuth2TokenService,
+    : public OAuth2TokenServiceObserver,
+      public OAuth2AccessTokenManager::Delegate,
       public DeviceOAuth2TokenServiceDelegate::ValidationStatusDelegate {
  public:
   typedef base::RepeatingCallback<void(const CoreAccountId& /* account_id */)>
@@ -62,14 +64,45 @@ class DeviceOAuth2TokenService
   // If set, this callback will be invoked when a refresh token is revoked.
   void SetRefreshTokenRevokedCallback(RefreshTokenRevokedCallback callback);
 
-  // OAuth2TokenServiceObserver:
-  // NOTE: OAuth2TokenService already adds itself as an observer, so this class
-  // doesn't actually need to add/remove itself as an O2TSObserver.
-  void OnRefreshTokenAvailable(const CoreAccountId& account_id) override;
-  void OnRefreshTokenRevoked(const CoreAccountId& account_id) override;
+  // Checks in the cache for a valid access token for a specified |account_id|
+  // and |scopes|, and if not found starts a request for an OAuth2 access token
+  // using the OAuth2 refresh token maintained by this instance for that
+  // |account_id|. The caller owns the returned Request.
+  // |scopes| is the set of scopes to get an access token for, |consumer| is
+  // the object that will be called back with results if the returned request
+  // is not deleted.
+  std::unique_ptr<OAuth2AccessTokenManager::Request> StartAccessTokenRequest(
+      const CoreAccountId& account_id,
+      const OAuth2AccessTokenManager::ScopeSet& scopes,
+      OAuth2AccessTokenManager::Consumer* consumer);
 
- protected:
-  // Implementation of OAuth2AccessTokenManager::Delegate
+  // Mark an OAuth2 |access_token| issued for |account_id| and |scopes| as
+  // invalid. This should be done if the token was received from this class,
+  // but was not accepted by the server (e.g., the server returned
+  // 401 Unauthorized). The token will be removed from the cache for the given
+  // scopes.
+  void InvalidateAccessToken(const CoreAccountId& account_id,
+                             const OAuth2AccessTokenManager::ScopeSet& scopes,
+                             const std::string& access_token);
+
+  bool RefreshTokenIsAvailable(const CoreAccountId& account_id) const;
+
+  OAuth2AccessTokenManager* GetAccessTokenManager();
+
+ private:
+  friend class DeviceOAuth2TokenServiceFactory;
+  friend class DeviceOAuth2TokenServiceTest;
+  struct PendingRequest;
+
+  // OAuth2AccessTokenManager::Delegate:
+  std::unique_ptr<OAuth2AccessTokenFetcher> CreateAccessTokenFetcher(
+      const CoreAccountId& account_id,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      OAuth2AccessTokenConsumer* consumer) override;
+  bool HasRefreshToken(const CoreAccountId& account_id) const override;
+  bool FixRequestErrorIfPossible() override;
+  scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory()
+      const override;
   bool HandleAccessTokenFetch(
       OAuth2AccessTokenManager::RequestImpl* request,
       const CoreAccountId& account_id,
@@ -77,11 +110,16 @@ class DeviceOAuth2TokenService
       const std::string& client_id,
       const std::string& client_secret,
       const OAuth2AccessTokenManager::ScopeSet& scopes) override;
+  void OnAccessTokenInvalidated(const CoreAccountId& account_id,
+                                const std::string& client_id,
+                                const std::set<std::string>& scopes,
+                                const std::string& access_token) override;
+  void OnAccessTokenFetched(const CoreAccountId& account_id,
+                            const GoogleServiceAuthError& error) override;
 
- private:
-  friend class DeviceOAuth2TokenServiceFactory;
-  friend class DeviceOAuth2TokenServiceTest;
-  struct PendingRequest;
+  // OAuth2TokenServiceObserver:
+  void OnRefreshTokenAvailable(const CoreAccountId& account_id) override;
+  void OnRefreshTokenRevoked(const CoreAccountId& account_id) override;
 
   // Implementation of
   // DeviceOAuth2TokenServiceDelegate::ValidationStatusDelegate.
@@ -100,8 +138,10 @@ class DeviceOAuth2TokenService
   void FailRequest(OAuth2AccessTokenManager::RequestImpl* request,
                    GoogleServiceAuthError::State error);
 
-  DeviceOAuth2TokenServiceDelegate* GetDeviceDelegate();
-  const DeviceOAuth2TokenServiceDelegate* GetDeviceDelegate() const;
+  // TODO(https://crbug.com/967598): Merge DeviceOAuth2TokenServiceDelegate
+  // into DeviceOAuth2TokenService.
+  std::unique_ptr<DeviceOAuth2TokenServiceDelegate> delegate_;
+  std::unique_ptr<OAuth2AccessTokenManager> token_manager_;
 
   // Currently open requests that are waiting while loading the system salt or
   // validating the token.

chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc

@@ -90,8 +90,9 @@ class DeviceOAuth2TokenServiceTest : public testing::Test {
   }
 
   std::unique_ptr<OAuth2AccessTokenManager::Request> StartTokenRequest() {
-    return oauth2_service_->StartRequest(oauth2_service_->GetRobotAccountId(),
-                                         std::set<std::string>(), &consumer_);
+    return oauth2_service_->StartAccessTokenRequest(
+        oauth2_service_->GetRobotAccountId(), std::set<std::string>(),
+        &consumer_);
   }
 
   void SetUp() override {
@@ -125,7 +126,8 @@ class DeviceOAuth2TokenServiceTest : public testing::Test {
         scoped_testing_local_state_.Get());
     delegate->max_refresh_token_validation_retries_ = 0;
     oauth2_service_.reset(new DeviceOAuth2TokenService(std::move(delegate)));
-    oauth2_service_->set_max_authorization_token_fetch_retries_for_testing(0);
+    oauth2_service_->GetAccessTokenManager()
+        ->set_max_authorization_token_fetch_retries_for_testing(0);
   }
 
   // Utility method to set a value in Local State for the device refresh token
@@ -141,6 +143,10 @@ class DeviceOAuth2TokenServiceTest : public testing::Test {
            "  \"user_id\": \"1234567890\" }";
   }
 
+  DeviceOAuth2TokenServiceDelegate* GetDelegate() {
+    return oauth2_service_->delegate_.get();
+  }
+
   bool RefreshTokenIsAvailable() {
     return oauth2_service_->RefreshTokenIsAvailable(
         oauth2_service_->GetRobotAccountId());
@@ -150,9 +156,7 @@ class DeviceOAuth2TokenServiceTest : public testing::Test {
     if (!RefreshTokenIsAvailable())
       return std::string();
 
-    return static_cast<DeviceOAuth2TokenServiceDelegate*>(
-               oauth2_service_->GetDelegate())
-        ->GetRefreshToken();
+    return GetDelegate()->GetRefreshToken();
   }
 
   // A utility method to return fake URL results, for testing the refresh token
@@ -460,7 +464,7 @@ TEST_F(DeviceOAuth2TokenServiceTest, DoNotAnnounceTokenWithoutAccountID) {
   CreateService();
 
   testing::StrictMock<MockOAuth2TokenServiceObserver> observer;
-  oauth2_service_->AddObserver(&observer);
+  GetDelegate()->AddObserver(&observer);
 
   // Make a token available during enrollment. Verify that the token is not
   // announced yet.
@@ -475,7 +479,7 @@ TEST_F(DeviceOAuth2TokenServiceTest, DoNotAnnounceTokenWithoutAccountID) {
   SetRobotAccountId("robot@example.com");
   testing::Mock::VerifyAndClearExpectations(&observer);
 
-  oauth2_service_->RemoveObserver(&observer);
+  GetDelegate()->RemoveObserver(&observer);
 }
 
 }  // namespace chromeos

chrome/browser/extensions/api/identity/identity_get_auth_token_function.cc

@@ -740,8 +740,8 @@ void IdentityGetAuthTokenFunction::StartDeviceAccessTokenRequest() {
   // request access token for [any-api] instead of login.
   OAuth2AccessTokenManager::ScopeSet scopes;
   scopes.insert(GaiaConstants::kAnyApiOAuth2Scope);
-  device_access_token_request_ =
-      service->StartRequest(service->GetRobotAccountId(), scopes, this);
+  device_access_token_request_ = service->StartAccessTokenRequest(
+      service->GetRobotAccountId(), scopes, this);
 }
 
 bool IdentityGetAuthTokenFunction::IsOriginWhitelistedInPublicSession() {

chrome/browser/ui/webui/print_preview/print_preview_handler.cc

@@ -422,7 +422,8 @@ class PrintPreviewHandler::AccessTokenService
         chromeos::DeviceOAuth2TokenServiceFactory::Get();
     std::string account_id = token_service->GetRobotAccountId();
 
-    device_request_ = token_service->StartRequest(account_id, scopes, this);
+    device_request_ =
+        token_service->StartAccessTokenRequest(account_id, scopes, this);
     device_request_callback_ = std::move(callback);
   }