Handling of multiple concurrent requests from different clients in OAuth2TokenService

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>
@@ skipping 82 matching lines @@
     // startup.
     virtual void OnRefreshTokensLoaded() {}
     // Called after all refresh tokens are removed from OAuth2TokenService.
     virtual void OnRefreshTokensCleared() {}
    protected:
     virtual ~Observer() {}
   };
 
   // A set of scopes in OAuth2 authentication.
   typedef std::set<std::string> ScopeSet;
+  typedef std::pair<std::string, ScopeSet> ClientScopeSet;
 
   OAuth2TokenService();
   virtual ~OAuth2TokenService();
 
   // Add or remove observers of this token service.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
   // 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);
 
+#if defined(OS_CHROMEOS)
   // This method does the same as |StartRequest| except it uses |client_id| and
   // |client_secret| to identify OAuth client app instead of using
   // Chrome's default values.
   virtual scoped_ptr<Request> StartRequestForClient(
       const std::string& client_id,
       const std::string& client_secret,
       const ScopeSet& scopes,
       Consumer* consumer);
+#endif

[(NOT FOR CODE REVIEWS), 2013/08/07 19:56:49]

+Michael and Filip.

I think this should be in all version of chrome.  This would be useful for the
chrome identity js api.

[Michael Courage, 2013/08/07 21:07:31]

Our input to a token request is (extension_id, client_id, scopes), so I suspect
we'd add a new public method with those parameters, which would call to the
common StartRequestForClientWithContext.

[zel, 2013/08/08 01:34:24]

To answer the first question, atwilson@ ask me to #ifdef this for ChromeOS in
the bug. I can easily remove this.

I can also easily add extension_id to the list of parameters. We can maybe make
it more generic - something like |request_origin|. WDYT?

 
   // This method does the same as |StartRequest| except it uses the request
   // context given by |getter| instead of using the one returned by
   // |GetRequestContext| implemented by derived classes.
   virtual scoped_ptr<Request> StartRequestWithContext(
       net::URLRequestContextGetter* getter,
       const ScopeSet& scopes,
       Consumer* consumer);
 
   // Returns true if a refresh token exists. If false, calls to
@@ skipping 35 matching lines @@
   // Subclasses should return the refresh token maintained.
   // If no token is available, return an empty string.
   virtual std::string GetRefreshToken() = 0;
 
   // 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,
+  virtual void RegisterCacheEntry(const std::string& client_id,
+                                  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);
+  bool HasCacheEntry(const ClientScopeSet& client_scopes);
 
   // Posts a task to fire the Consumer callback with the cached token.  Must
   // Must only be called if HasCacheEntry() returns true.
-  scoped_ptr<Request> StartCacheLookupRequest(const ScopeSet& scopes,
-                                              Consumer* consumer);
+  scoped_ptr<Request> StartCacheLookupRequest(
+      const ClientScopeSet& client_scopes,
+      Consumer* consumer);
 
   // Clears the internal token cache.
   void ClearCache();
 
   // Cancels all requests that are currently in progress.
   void CancelAllRequests();
 
   // Cancels all requests related to a given refresh token.
   void CancelRequestsForToken(const std::string& refresh_token);
 
   // Called by subclasses to notify observers.
   void FireRefreshTokenAvailable(const std::string& account_id);
   void FireRefreshTokenRevoked(const std::string& account_id,
                                const GoogleServiceAuthError& error);
   void FireRefreshTokensLoaded();
   void FireRefreshTokensCleared();
 
  private:
   // Derived classes must provide a request context used for fetching access
   // tokens with the |StartRequest| method.
   virtual net::URLRequestContextGetter* GetRequestContext() = 0;
 
   // Class that fetches an OAuth2 access token for a given set of scopes and
   // OAuth2 refresh token.

[(NOT FOR CODE REVIEWS), 2013/08/07 19:56:49]

Add comment about client_id too?

   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;
   };
 
   // This method does the same as |StartRequestWithContext| except it
   // uses |client_id| and |client_secret| to identify OAuth
   // client app instead of using Chrome's default values.
   scoped_ptr<Request> StartRequestForClientWithContext(
       net::URLRequestContextGetter* getter,
       const std::string& client_id,
       const std::string& client_secret,
       const ScopeSet& scopes,
       Consumer* consumer);
 
   // Returns a currently valid OAuth2 access token for the given set of scopes,
   // or NULL if none have been cached. Note the user of this method should
-  // ensure no entry with the same |scopes| is added before the usage of the
-  // returned entry is done.
-  const CacheEntry* GetCacheEntry(const ScopeSet& scopes);
+  // ensure no entry with the same |client_scopes| is added before the usage of
+  // the returned entry is done.
+  const CacheEntry* GetCacheEntry(const ClientScopeSet& client_scopes);
 
 
   // Removes an access token for the given set of scopes from the cache.
   // Returns true if the entry was removed, otherwise false.
-  bool RemoveCacheEntry(const OAuth2TokenService::ScopeSet& scopes,
+  bool RemoveCacheEntry(const ClientScopeSet& client_scopes,
                         const std::string& token_to_remove);
 
 
   // Called when |fetcher| finishes fetching.
   void OnFetchComplete(Fetcher* fetcher);
 
   // Called when a number of fetchers need to be canceled.
   void CancelFetchers(std::vector<Fetcher*> fetchers_to_cancel);
 
   // The cache of currently valid tokens.
-  typedef std::map<ScopeSet, CacheEntry> TokenCache;
+  typedef std::map<ClientScopeSet, CacheEntry> TokenCache;
   TokenCache token_cache_;
 
-  // The parameters (refresh token and scope set) used to fetch an OAuth2 access
-  // token.
-  typedef std::pair<std::string, ScopeSet> FetchParameters;
+  // The parameters (client_id, refresh token and scope set) used to fetch an
+  // OAuth2 access token.
+  typedef std::pair<std::pair<std::string /* client_id */,
+                              std::string /* refresh_token */>,
+                    ScopeSet> FetchParameters;

[(NOT FOR CODE REVIEWS), 2013/08/07 19:56:49]

Would be clearer to declare a struct with 3 distinct members.

[fgorski, 2013/08/07 21:36:59]

Alternative solution would be to use ClientScopeSet, as that is how we are
keying the TokenCache:
std::pair<std::string /*refresh_token*/, ClientScopeSet>
(Fetcher would not have to expose client_id, but a ClientScopeSet).

[zel, 2013/08/08 01:34:24]

I've used more complex key now and made this a struct - PTAL.

   // A map from fetch parameters to a fetcher that is fetching an OAuth2 access
   // token using these parameters.
   std::map<FetchParameters, Fetcher*> pending_fetchers_;
 
   // List of observers to notify when token availiability changes.
   // Makes sure list is empty on destruction.
   ObserverList<Observer, true> observer_list_;
 
   // Maximum number of retries in fetching an OAuth2 access token.
   static int max_fetch_retry_num_;
 
   DISALLOW_COPY_AND_ASSIGN(OAuth2TokenService);
 };
 
 #endif  // CHROME_BROWSER_SIGNIN_OAUTH2_TOKEN_SERVICE_H_