IdentityManager: Introduce access token fetching for arbitrary accounts

The next-generation C++ APIs for interacting with the user's Google
identities currently do not give the ability to fetch access tokens
for arbitrary accounts; rather, there exists only the
PrimaryAccountAccessTokenFetcher, which provides user-friendly access
token fetching for the primary account.

This CL introduces an analogous AccessTokenFetcher class that allows
such user-friendly access token fetching for arbitrary accounts. This
class is currently built on top of the Token Service, as it remains
future work to expose access token fetching APIs on IdentityManager
itself. Once that future work is completed, AccessTokenFetcher will
be ported to be layered on top of IdentityManager. In the meantime,
we introduce IdentityManager::CreateAccessTokenFetcherForAccount()
to allow consumers of IdentityManager to create an AccessTokenFetcher.
Note that our intention is that even in the long-term consumers will
use AccessTokenFetcher and PrimaryAccountAccessTokenFetcher as the
interfaces for fetching access tokens, rather than interacting directly
with IdentityManager's lower-level APIs.

This CL also ports PrimaryAccountAccessTokenFetcher to internally use
AccessTokenFetcher rather than directly fetching access tokens from
ProfileOAuth2TokenService.

API design note: It would be reasonable to build PAATF's idea of an
"access token fetching mode" into AccessTokenFetcher: in this context,
kWaitUntilAvailable would simply mean waiting until the refresh token
was available. This CL does not add such a mode. However, for Sync's
use case having the mode will likely be useful (Sync will alternately
use the primary account and an arbitrary account, and it would be
ergonomic to be able to fetch access tokens in both contexts using
AccessTokenFetcher). Hence, followup work will move this mode down
into AccessTokenFetcher.

Bug: 729547
Change-Id: Id296adcd1567d6cbfbaf6ffd1dafb718c1f01ed7
Reviewed-on: https://chromium-review.googlesource.com/1104118
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569281}

services/identity/BUILD.gn

@@ -51,6 +51,7 @@ source_set("tests") {
   ]
   sources = [
     "identity_manager_impl_unittest.cc",
+    "public/cpp/access_token_fetcher_unittest.cc",
     "public/cpp/identity_manager_unittest.cc",
     "public/cpp/primary_account_access_token_fetcher_unittest.cc",
   ]

services/identity/public/cpp/BUILD.gn

@@ -4,6 +4,8 @@
 
 source_set("cpp") {
   sources = [
+    "access_token_fetcher.cc",
+    "access_token_fetcher.h",
     "identity_manager.cc",
     "identity_manager.h",
     "primary_account_access_token_fetcher.cc",

services/identity/public/cpp/access_token_fetcher.cc

+// Copyright 2018 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 "services/identity/public/cpp/access_token_fetcher.h"
+
+#include <utility>
+
+#include "base/logging.h"
+
+namespace identity {
+
+AccessTokenFetcher::AccessTokenFetcher(
+    const std::string& account_id,
+    const std::string& oauth_consumer_name,
+    OAuth2TokenService* token_service,
+    const OAuth2TokenService::ScopeSet& scopes,
+    TokenCallback callback)
+    : OAuth2TokenService::Consumer(oauth_consumer_name),
+      account_id_(account_id),
+      token_service_(token_service),
+      scopes_(scopes),
+      callback_(std::move(callback)) {
+  // TODO(843510): Consider making the request to ProfileOAuth2TokenService
+  // asynchronously once there are no direct clients of PO2TS (i.e., PO2TS is
+  // used only by this class and IdentityManager).
+  access_token_request_ =
+      token_service_->StartRequest(account_id_, scopes_, this);
+}
+
+AccessTokenFetcher::~AccessTokenFetcher() {}
+
+void AccessTokenFetcher::OnGetTokenSuccess(
+    const OAuth2TokenService::Request* request,
+    const std::string& access_token,
+    const base::Time& expiration_time) {
+  DCHECK_EQ(request, access_token_request_.get());
+  std::unique_ptr<OAuth2TokenService::Request> request_deleter(
+      std::move(access_token_request_));
+
+  RunCallbackAndMaybeDie(GoogleServiceAuthError::AuthErrorNone(), access_token);
+
+  // Potentially dead after the above invocation; nothing to do except return.
+}
+
+void AccessTokenFetcher::OnGetTokenFailure(
+    const OAuth2TokenService::Request* request,
+    const GoogleServiceAuthError& error) {
+  DCHECK_EQ(request, access_token_request_.get());
+  std::unique_ptr<OAuth2TokenService::Request> request_deleter(
+      std::move(access_token_request_));
+
+  RunCallbackAndMaybeDie(error, std::string());
+
+  // Potentially dead after the above invocation; nothing to do except return.
+}
+
+void AccessTokenFetcher::RunCallbackAndMaybeDie(
+    const GoogleServiceAuthError& error,
+    const std::string& access_token) {
+  // Per the contract of this class, it is allowed for consumers to delete this
+  // object from within the callback that is run below. Hence, it is not safe to
+  // add any code below this call.
+  std::move(callback_).Run(error, access_token);
+}
+
+}  // namespace identity

services/identity/public/cpp/access_token_fetcher.h

+// Copyright 2018 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 SERVICES_IDENTITY_PUBLIC_CPP_ACCESS_TOKEN_FETCHER_H_
+#define SERVICES_IDENTITY_PUBLIC_CPP_ACCESS_TOKEN_FETCHER_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "google_apis/gaia/google_service_auth_error.h"
+#include "google_apis/gaia/oauth2_token_service.h"
+
+namespace identity {
+
+// Helper class to ease the task of obtaining an OAuth2 access token for a
+// given account.
+// May only be used on the UI thread.
+class AccessTokenFetcher : public OAuth2TokenService::Observer,
+                           public OAuth2TokenService::Consumer {
+ public:
+  // Callback for when a request completes (successful or not). On successful
+  // requests, |error| is NONE and |access_token| contains the obtained OAuth2
+  // access token. On failed requests, |error| contains the actual error and
+  // |access_token| is empty.
+  // NOTE: At the time that this method is invoked, it is safe for the client to
+  // destroy the AccessTokenFetcher instance that is invoking this callback.
+  using TokenCallback = base::OnceCallback<void(GoogleServiceAuthError error,
+                                                std::string access_token)>;
+
+  // Instantiates a fetcher and immediately starts the process of obtaining an
+  // OAuth2 access token for |account_id| and |scopes|. The |callback| is called
+  // once the request completes (successful or not). If the AccessTokenFetcher
+  // is destroyed before the process completes, the callback is not called.
+  AccessTokenFetcher(const std::string& account_id,
+                     const std::string& oauth_consumer_name,
+                     OAuth2TokenService* token_service,
+                     const OAuth2TokenService::ScopeSet& scopes,
+                     TokenCallback callback);
+
+  ~AccessTokenFetcher() override;
+
+ private:
+  // OAuth2TokenService::Consumer implementation.
+  void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
+                         const std::string& access_token,
+                         const base::Time& expiration_time) override;
+  void OnGetTokenFailure(const OAuth2TokenService::Request* request,
+                         const GoogleServiceAuthError& error) override;
+
+  // Invokes |callback_| with (|error|, |access_token|). Per the contract of
+  // this class, it is allowed for clients to delete this object as part of the
+  // invocation of |callback_|. Hence, this object must assume that it is dead
+  // after invoking this method and must not run any more code.
+  void RunCallbackAndMaybeDie(const GoogleServiceAuthError& error,
+                              const std::string& access_token);
+
+  std::string account_id_;
+  OAuth2TokenService* token_service_;
+  OAuth2TokenService::ScopeSet scopes_;
+
+  // NOTE: This callback should only be invoked from |RunCallbackAndMaybeDie|,
+  // as invoking it has the potential to destroy this object per this class's
+  // contract.
+  TokenCallback callback_;
+
+  std::unique_ptr<OAuth2TokenService::Request> access_token_request_;
+
+  DISALLOW_COPY_AND_ASSIGN(AccessTokenFetcher);
+};
+
+}  // namespace identity
+
+#endif  // SERVICES_IDENTITY_PUBLIC_CPP_ACCESS_TOKEN_FETCHER_H_

services/identity/public/cpp/identity_manager.cc

@@ -83,11 +83,22 @@ bool IdentityManager::HasPrimaryAccount() {
   return !primary_account_info_.account_id.empty();
 }
 
+std::unique_ptr<AccessTokenFetcher>
+IdentityManager::CreateAccessTokenFetcherForAccount(
+    const std::string& account_id,
+    const std::string& oauth_consumer_name,
+    const OAuth2TokenService::ScopeSet& scopes,
+    AccessTokenFetcher::TokenCallback callback) {
+  return std::make_unique<AccessTokenFetcher>(account_id, oauth_consumer_name,
+                                              token_service_, scopes,
+                                              std::move(callback));
+}
+
 std::unique_ptr<PrimaryAccountAccessTokenFetcher>
 IdentityManager::CreateAccessTokenFetcherForPrimaryAccount(
     const std::string& oauth_consumer_name,
     const OAuth2TokenService::ScopeSet& scopes,
-    PrimaryAccountAccessTokenFetcher::TokenCallback callback,
+    AccessTokenFetcher::TokenCallback callback,
     PrimaryAccountAccessTokenFetcher::Mode mode) {
   return std::make_unique<PrimaryAccountAccessTokenFetcher>(
       oauth_consumer_name, signin_manager_, token_service_, scopes,

services/identity/public/cpp/identity_manager.h

@@ -10,6 +10,7 @@
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_manager_base.h"
+#include "services/identity/public/cpp/access_token_fetcher.h"
 #include "services/identity/public/cpp/primary_account_access_token_fetcher.h"
 
 #if !defined(OS_CHROMEOS)
@@ -119,12 +120,19 @@ class IdentityManager : public SigninManagerBase::Observer,
   // primary account info has a valid account ID.
   bool HasPrimaryAccount();
 
+  // Creates an AccessTokenFetcher given the passed-in information.
+  std::unique_ptr<AccessTokenFetcher> CreateAccessTokenFetcherForAccount(
+      const std::string& account_id,
+      const std::string& oauth_consumer_name,
+      const OAuth2TokenService::ScopeSet& scopes,
+      AccessTokenFetcher::TokenCallback callback);
+
   // Creates a PrimaryAccountAccessTokenFetcher given the passed-in information.
   std::unique_ptr<PrimaryAccountAccessTokenFetcher>
   CreateAccessTokenFetcherForPrimaryAccount(
       const std::string& oauth_consumer_name,
       const OAuth2TokenService::ScopeSet& scopes,
-      PrimaryAccountAccessTokenFetcher::TokenCallback callback,
+      AccessTokenFetcher::TokenCallback callback,
       PrimaryAccountAccessTokenFetcher::Mode mode);
 
   // If an entry exists in the Identity Service's cache corresponding to the

services/identity/public/cpp/identity_manager_unittest.cc

@@ -315,7 +315,9 @@ class TestIdentityManagerDiagnosticsObserver
     token_requestor_account_id_ = account_id;
     token_requestor_consumer_id_ = consumer_id;
     token_requestor_scopes_ = scopes;
-    std::move(on_access_token_requested_callback_).Run();
+
+    if (on_access_token_requested_callback_)
+      std::move(on_access_token_requested_callback_).Run();
   }
 
   IdentityManager* identity_manager_;
@@ -499,9 +501,20 @@ TEST_F(IdentityManagerTest, RemoveAccessTokenFromCache) {
   run_loop.Run();
 }
 
+TEST_F(IdentityManagerTest, CreateAccessTokenFetcher) {
+  std::set<std::string> scopes{"scope"};
+  AccessTokenFetcher::TokenCallback callback = base::BindOnce(
+      [](GoogleServiceAuthError error, std::string access_token) {});
+  std::unique_ptr<AccessTokenFetcher> token_fetcher =
+      identity_manager()->CreateAccessTokenFetcherForAccount(
+          identity_manager()->GetPrimaryAccountInfo().account_id,
+          "dummy_consumer", scopes, std::move(callback));
+  EXPECT_TRUE(token_fetcher);
+}
+
 TEST_F(IdentityManagerTest, CreateAccessTokenFetcherForPrimaryAccount) {
   std::set<std::string> scopes{"scope"};
-  PrimaryAccountAccessTokenFetcher::TokenCallback callback = base::BindOnce(
+  AccessTokenFetcher::TokenCallback callback = base::BindOnce(
       [](GoogleServiceAuthError error, std::string access_token) {});
   std::unique_ptr<PrimaryAccountAccessTokenFetcher> token_fetcher =
       identity_manager()->CreateAccessTokenFetcherForPrimaryAccount(
@@ -520,7 +533,7 @@ TEST_F(IdentityManagerTest, ObserveAccessTokenFetch) {
   token_service()->UpdateCredentials(account_id, "refresh_token");
 
   std::set<std::string> scopes{"scope"};
-  PrimaryAccountAccessTokenFetcher::TokenCallback callback = base::BindOnce(
+  AccessTokenFetcher::TokenCallback callback = base::BindOnce(
       [](GoogleServiceAuthError error, std::string access_token) {});
   std::unique_ptr<PrimaryAccountAccessTokenFetcher> token_fetcher =
       identity_manager()->CreateAccessTokenFetcherForPrimaryAccount(

services/identity/public/cpp/primary_account_access_token_fetcher.cc

@@ -15,9 +15,9 @@ PrimaryAccountAccessTokenFetcher::PrimaryAccountAccessTokenFetcher(
     SigninManagerBase* signin_manager,
     OAuth2TokenService* token_service,
     const OAuth2TokenService::ScopeSet& scopes,
-    TokenCallback callback,
+    AccessTokenFetcher::TokenCallback callback,
     Mode mode)
-    : OAuth2TokenService::Consumer(oauth_consumer_name),
+    : oauth_consumer_name_(oauth_consumer_name),
       signin_manager_(signin_manager),
       token_service_(token_service),
       scopes_(scopes),
@@ -59,13 +59,14 @@ void PrimaryAccountAccessTokenFetcher::StartAccessTokenRequest() {
   // Note: We might get here even in cases where we know that there's no refresh
   // token. We're requesting an access token anyway, so that the token service
   // will generate an appropriate error code that we can return to the client.
-  DCHECK(!access_token_request_);
-
-  // TODO(843510): Consider making the request to ProfileOAuth2TokenService
-  // asynchronously once there are no direct clients of PO2TS (i.e., PO2TS is
-  // used only by this class and IdentityManager).
-  access_token_request_ = token_service_->StartRequest(
-      signin_manager_->GetAuthenticatedAccountId(), scopes_, this);
+  DCHECK(!access_token_fetcher_);
+
+  access_token_fetcher_ = std::make_unique<AccessTokenFetcher>(
+      signin_manager_->GetAuthenticatedAccountId(), oauth_consumer_name_,
+      token_service_, scopes_,
+      base::BindOnce(
+          &PrimaryAccountAccessTokenFetcher::OnAccessTokenFetchComplete,
+          base::Unretained(this)));
 }
 
 void PrimaryAccountAccessTokenFetcher::GoogleSigninSucceeded(
@@ -91,25 +92,10 @@ void PrimaryAccountAccessTokenFetcher::ProcessSigninStateChange() {
   StartAccessTokenRequest();
 }
 
-void PrimaryAccountAccessTokenFetcher::OnGetTokenSuccess(
-    const OAuth2TokenService::Request* request,
-    const std::string& access_token,
-    const base::Time& expiration_time) {
-  DCHECK_EQ(request, access_token_request_.get());
-  std::unique_ptr<OAuth2TokenService::Request> request_deleter(
-      std::move(access_token_request_));
-
-  RunCallbackAndMaybeDie(GoogleServiceAuthError::AuthErrorNone(), access_token);
-
-  // Potentially dead after the above invocation; nothing to do except return.
-}
-
-void PrimaryAccountAccessTokenFetcher::OnGetTokenFailure(
-    const OAuth2TokenService::Request* request,
-    const GoogleServiceAuthError& error) {
-  DCHECK_EQ(request, access_token_request_.get());
-  std::unique_ptr<OAuth2TokenService::Request> request_deleter(
-      std::move(access_token_request_));
+void PrimaryAccountAccessTokenFetcher::OnAccessTokenFetchComplete(
+    GoogleServiceAuthError error,
+    std::string access_token) {
+  access_token_fetcher_.reset();
 
   // There is a special case for Android that RefreshTokenIsAvailable and
   // StartRequest are called to pre-fetch the account image and name before
@@ -130,14 +116,6 @@ void PrimaryAccountAccessTokenFetcher::OnGetTokenFailure(
     return;
   }
 
-  RunCallbackAndMaybeDie(error, std::string());
-
-  // Potentially dead after the above invocation; nothing to do except return.
-}
-
-void PrimaryAccountAccessTokenFetcher::RunCallbackAndMaybeDie(
-    const GoogleServiceAuthError& error,
-    const std::string& access_token) {
   // Per the contract of this class, it is allowed for consumers to delete this
   // object from within the callback that is run below. Hence, it is not safe to
   // add any code below this call.

services/identity/public/cpp/primary_account_access_token_fetcher.h

@@ -14,6 +14,7 @@
 #include "components/signin/core/browser/signin_manager_base.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "google_apis/gaia/oauth2_token_service.h"
+#include "services/identity/public/cpp/access_token_fetcher.h"
 
 namespace identity {
 
@@ -23,19 +24,8 @@ namespace identity {
 // transient error.
 // May only be used on the UI thread.
 class PrimaryAccountAccessTokenFetcher : public SigninManagerBase::Observer,
-                                         public OAuth2TokenService::Observer,
-                                         public OAuth2TokenService::Consumer {
+                                         public OAuth2TokenService::Observer {
  public:
-  // Callback for when a request completes (successful or not). On successful
-  // requests, |error| is NONE and |access_token| contains the obtained OAuth2
-  // access token. On failed requests, |error| contains the actual error and
-  // |access_token| is empty.
-  // NOTE: At the time that this method is invoked, it is safe for the client to
-  // destroy the PrimaryAccountAccessTokenFetcher instance that is invoking
-  // this callback.
-  using TokenCallback = base::OnceCallback<void(GoogleServiceAuthError error,
-                                                std::string access_token)>;
-
   // Specifies how this instance should behave:
   // |kImmediate|: Makes one-shot immediate request.
   // |kWaitUntilAvailable|: Waits for the primary account to be available
@@ -53,7 +43,7 @@ class PrimaryAccountAccessTokenFetcher : public SigninManagerBase::Observer,
                                    SigninManagerBase* signin_manager,
                                    OAuth2TokenService* token_service,
                                    const OAuth2TokenService::ScopeSet& scopes,
-                                   TokenCallback callback,
+                                   AccessTokenFetcher::TokenCallback callback,
                                    Mode mode);
 
   ~PrimaryAccountAccessTokenFetcher() override;
@@ -76,35 +66,28 @@ class PrimaryAccountAccessTokenFetcher : public SigninManagerBase::Observer,
   // request if so. Should only be called in mode |kWaitUntilAvailable|.
   void ProcessSigninStateChange();
 
-  // OAuth2TokenService::Consumer implementation.
-  void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
-                         const std::string& access_token,
-                         const base::Time& expiration_time) override;
-  void OnGetTokenFailure(const OAuth2TokenService::Request* request,
-                         const GoogleServiceAuthError& error) override;
-
-  // Invokes |callback_| with (|error|, |access_token|). Per the contract of
-  // this class, it is allowed for clients to delete this object as part of the
-  // invocation of |callback_|. Hence, this object must assume that it is dead
-  // after invoking this method and must not run any more code.
-  void RunCallbackAndMaybeDie(const GoogleServiceAuthError& error,
-                              const std::string& access_token);
+  // Invoked by |fetcher_| when an access token request completes.
+  void OnAccessTokenFetchComplete(GoogleServiceAuthError error,
+                                  std::string access_token);
 
+  std::string oauth_consumer_name_;
   SigninManagerBase* signin_manager_;
   OAuth2TokenService* token_service_;
   OAuth2TokenService::ScopeSet scopes_;
 
-  // NOTE: This callback should only be invoked from |RunCallbackAndMaybeDie|,
-  // as invoking it has the potential to destroy this object per this class's
-  // contract.
-  TokenCallback callback_;
+  // Per the contract of this class, it is allowed for clients to delete this
+  // object as part of the invocation of |callback_|. Hence, this object must
+  // assume that it is dead after invoking |callback_| and must not run any more
+  // code.
+  AccessTokenFetcher::TokenCallback callback_;
 
   ScopedObserver<SigninManagerBase, PrimaryAccountAccessTokenFetcher>
       signin_manager_observer_;
   ScopedObserver<OAuth2TokenService, PrimaryAccountAccessTokenFetcher>
       token_service_observer_;
 
-  std::unique_ptr<OAuth2TokenService::Request> access_token_request_;
+  // Internal fetcher that does the actual access token request.
+  std::unique_ptr<AccessTokenFetcher> access_token_fetcher_;
 
   // When a token request gets canceled, we want to retry once.
   bool access_token_retried_;

services/identity/public/cpp/primary_account_access_token_fetcher_unittest.cc

@@ -57,7 +57,7 @@ class PrimaryAccountAccessTokenFetcherTest
       public OAuth2TokenService::DiagnosticsObserver {
  public:
   using TestTokenCallback =
-      StrictMock<MockCallback<PrimaryAccountAccessTokenFetcher::TokenCallback>>;
+      StrictMock<MockCallback<AccessTokenFetcher::TokenCallback>>;
 
   PrimaryAccountAccessTokenFetcherTest() : signin_client_(&pref_service_) {
     AccountTrackerService::RegisterPrefs(pref_service_.registry());
@@ -88,7 +88,7 @@ class PrimaryAccountAccessTokenFetcherTest
   }
 
   std::unique_ptr<PrimaryAccountAccessTokenFetcher> CreateFetcher(
-      PrimaryAccountAccessTokenFetcher::TokenCallback callback,
+      AccessTokenFetcher::TokenCallback callback,
       PrimaryAccountAccessTokenFetcher::Mode mode) {
     std::set<std::string> scopes{"scope"};
     return std::make_unique<PrimaryAccountAccessTokenFetcher>(