Device robot refresh token integrity validation.

chrome/browser/signin/oauth2_token_service.h

 // Copyright 2012 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_SIGNIN_OAUTH2_TOKEN_SERVICE_H_
 #define CHROME_BROWSER_SIGNIN_OAUTH2_TOKEN_SERVICE_H_
 
 #include <map>
 #include <set>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time.h"
 
 namespace base {
 class Time;
 }
 
 namespace net {
 class URLRequestContextGetter;
 }
 
 class GoogleServiceAuthError;
 
 // Abstract base class for a service that fetches and caches OAuth2 access
-// tokens. Concrete subclasses should implement GetRefreshToken to return
+// tokens. Concrete subclasses should implement StartGetRefreshToken to provide
 // the appropriate refresh token.
 //
 // All calls are expected from the UI thread.
 //
 // To use this service, call StartRequest() with a given set of scopes and a
 // consumer of the request results. The consumer is required to outlive the
 // request. The request can be deleted. The consumer may be called back
 // asynchronously with the fetch results.
 //
 // - If the consumer is not called back before the request is deleted, it will
@@ skipping 40 matching lines @@
 
   // Checks in the cache for a valid access token, and if not found starts
   // a request for an OAuth2 access token using the OAuth2 refresh token
   // maintained by this instance. 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.
   virtual scoped_ptr<Request> StartRequest(const ScopeSet& scopes,
                                            Consumer* consumer);
 
-  // Returns true if a refresh token exists. If false, calls to
-  // |StartRequest| will result in a Consumer::OnGetTokenFailure callback.
-  bool RefreshTokenIsAvailable();
-
   // Mark an OAuth2 access token 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.
   virtual void InvalidateToken(const ScopeSet& scopes,
                                const std::string& invalid_token);
 
   // Return the current number of entries in the cache.
   int cache_size_for_testing() const;
 
  protected:
+  // Interface that StartGetRefreshToken calls back into.
+  class RefreshTokenValidationConsumer {
+   public:
+    virtual void OnRefreshTokenValidationComplete(
+        const std::string& refresh_token,
+        bool is_valid) = 0;
+  };
+
+  // Implements a cancelable |OAuth2TokenService::Request|, which should be
+  // operated on the UI thread.
+  class RequestImpl : public base::SupportsWeakPtr<RequestImpl>,
+                      public Request {
+   public:
+    // |consumer| is required to outlive this.
+    explicit RequestImpl(Consumer* consumer);
+    virtual ~RequestImpl();
+
+    // Informs |consumer_| that this request is completed.
+    void InformConsumer(const GoogleServiceAuthError& error,
+                        const std::string& access_token,
+                        const base::Time& expiration_date);
+
+   private:
+    // |consumer_| to call back when this request completes.
+    Consumer* const consumer_;
+  };
+
   // Subclasses should return the refresh token maintained.
   // If no token is available, return an empty string.
   virtual std::string GetRefreshToken() = 0;
 
+  // Subclasses can optionally implement this method to validate the given
+  // refresh token.  Return true if the validation is started and the
+  // callback should be expected, or false if validation is skipped and the
+  // token should be used directly.  This method may be invoked in parallel;
+  // subclasses must ensure that all consumers are notified of the results.
+  virtual bool StartRefreshTokenValidation(

[Mattias Nissler (ping if slow), 2013/06/17 05:34:17]

I wonder whether making the concept of refresh token validation visible on the
OAuth2TokenService level is actually a good idea. Have you thought about just
making GetRefreshToken asynchronous, i.e.:

void GetRefreshToken(base::Callback<void(const std::string&)>);

Then, you could hide away the validation part in DeviceOAuth2TokenService.

[David Roche, 2013/06/18 04:12:08]

I started doing that initially, but unfortunately it got to be messy.  The
problem is that the refresh token getter is on the token-service singleton,
while the async/concurrent fetch requests are on the inner Fetcher class.  So,
to stay with the current design of OAuth2TokenService, the fetching should stay
in the Fetcher, since it automatically takes care of handling concurrent
fetches, etc.  Then the obvious idea is to continue with the attempt to make
GetRefreshToken async, but call it from the Fetcher.  This was my attempt #2,
but it had issues b/c the OAuth2TokenService keeps the map of in-flight Fetchers
indexed by refresh_token/scopes pair.  So, I need the refresh token value before
the Fetcher's CreateAndStart method is called.  I considered a big refactor of
the OAuth2TokenService logic around Fetcher lifecycle tracking, so that it was a
2-phase affair -- the first tracking a list of Fetchers that were getting the
refresh token (indexed only by scope) and the 2nd phase of Fetchers that had a
refresh token and were fetching the access token (indexed by refresh token /
scope pair).  But, in the end that seemed like a lot of changes.  So, what you
see is attempt #3 ... keeping the change as simple as possible by having the
fetcher created with the refresh token initially (so it can be added to the
existing map), and then optionally validating the token in the Fetcher. 
Conceptually, validating the refresh token is something that other subclasses
could want to do in the future, so it seemed reasonable enough.  Although, I
agree with your initial design instinct ... it would have been nice to hide the
validation in DeviceOAuth2TokenService, if it could have been done relatively
easily.

+      const std::string refresh_token,
+      RefreshTokenValidationConsumer* consumer);
+
   // Subclasses can override if they want to report errors to the user.
   virtual void UpdateAuthError(const GoogleServiceAuthError& error);
 
   // Add a new entry to the cache.
   // Subclasses can override if there are implementation-specific reasons
   // that an access token should ever not be cached.
   virtual void RegisterCacheEntry(const std::string& refresh_token,
                                   const ScopeSet& scopes,
                                   const std::string& access_token,
                                   const base::Time& expiration_date);
 
   // Returns true if GetCacheEntry would return a valid cache entry for the
   // given scopes.
   bool HasCacheEntry(const ScopeSet& scopes);
 
-  // Posts a task to fire the Consumer callback with the cached token.  Must
-  // only be called if HasCacheEntry() returns true.
+  // Posts a task to fetch the cached token for the given in-flight Request.
+  // Must only be called if HasCacheEntry() returns true.
   scoped_ptr<Request> StartCacheLookupRequest(const ScopeSet& scopes,
-                                              Consumer* consumer);
+                                              scoped_ptr<RequestImpl> request);
 
   // Clears the internal token cache.
   void ClearCache();
 
-  // Implements a cancelable |OAuth2TokenService::Request|, which should be
-  // operated on the UI thread.
-  class RequestImpl : public base::SupportsWeakPtr<RequestImpl>,
-                      public Request {
-   public:
-    // |consumer| is required to outlive this.
-    explicit RequestImpl(Consumer* consumer);
-    virtual ~RequestImpl();
-
-    // Informs |consumer_| that this request is completed.
-    void InformConsumer(const GoogleServiceAuthError& error,
-                        const std::string& access_token,
-                        const base::Time& expiration_date);
-
-   private:
-    // |consumer_| to call back when this request completes.
-    Consumer* const consumer_;
-  };
-
  private:
   // Class that fetches an OAuth2 access token for a given set of scopes and
   // OAuth2 refresh token.
   class Fetcher;
   friend class Fetcher;
 
   // Struct that contains the information of an OAuth2 access token.
   struct CacheEntry {
     std::string access_token;
     base::Time expiration_date;
@@ skipping 26 matching lines @@
   // token.
   typedef std::pair<std::string, ScopeSet> FetchParameters;
   // A map from fetch parameters to a fetcher that is fetching an OAuth2 access
   // token using these parameters.
   std::map<FetchParameters, Fetcher*> pending_fetchers_;
 
   DISALLOW_COPY_AND_ASSIGN(OAuth2TokenService);
 };
 
 #endif  // CHROME_BROWSER_SIGNIN_OAUTH2_TOKEN_SERVICE_H_