[Signin] Handle multiple concurrent Dice responses

chrome/browser/signin/dice_response_handler.cc

 // Copyright 2017 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 "chrome/browser/signin/dice_response_handler.h"
 
+#include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/keyed_service/content/browser_context_keyed_service_factory.h"
 #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_client.h"
@@ skipping 40 matching lines @@
     Profile* profile = static_cast<Profile*>(context);
     return new DiceResponseHandler(
         ChromeSigninClientFactory::GetForProfile(profile),
         ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
         AccountTrackerServiceFactory::GetForProfile(profile));
   }
 };
 
 }  // namespace
 
+////////////////////////////////////////////////////////////////////////////////
+// DiceTokenFetcher
+////////////////////////////////////////////////////////////////////////////////
+
+DiceResponseHandler::DiceTokenFetcher::DiceTokenFetcher(
+    const std::string& gaia_id,
+    const std::string& email,
+    const std::string& authorization_code,
+    SigninClient* signin_client,
+    DiceResponseHandler* dice_response_handler)
+    : gaia_id_(gaia_id),
+      email_(email),
+      dice_response_handler_(dice_response_handler) {
+  gaia_auth_fetcher_ = signin_client->CreateGaiaAuthFetcher(
+      this, GaiaConstants::kChromeSource,
+      signin_client->GetURLRequestContext());
+  gaia_auth_fetcher_->StartAuthCodeForOAuth2TokenExchange(authorization_code);
+
+  // TODO(droger): The token exchange must complete quickly or be cancelled. Add
+  // a timeout logic.
+}
+
+DiceResponseHandler::DiceTokenFetcher::~DiceTokenFetcher() {}
+
+void DiceResponseHandler::DiceTokenFetcher::OnClientOAuthSuccess(
+    const GaiaAuthConsumer::ClientOAuthResult& result) {
+  dice_response_handler_->OnTokenExchangeSuccess(this, gaia_id_, email_,
+                                                 result);
+}
+
+void DiceResponseHandler::DiceTokenFetcher::OnClientOAuthFailure(
+    const GoogleServiceAuthError& error) {
+  dice_response_handler_->OnTokenExchangeFailure(this, error);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// DiceResponseHandler
+////////////////////////////////////////////////////////////////////////////////
+
 // static
 DiceResponseHandler* DiceResponseHandler::GetForProfile(Profile* profile) {
   return DiceResponseHandlerFactory::GetForProfile(profile);
 }
 
 DiceResponseHandler::DiceResponseHandler(
     SigninClient* signin_client,
     ProfileOAuth2TokenService* profile_oauth2_token_service,
     AccountTrackerService* account_tracker_service)
     : signin_client_(signin_client),
@@ skipping 26 matching lines @@
   }
 
   NOTREACHED();
   return;
 }
 
 void DiceResponseHandler::ProcessDiceSigninHeader(
     const std::string& gaia_id,
     const std::string& email,
     const std::string& authorization_code) {
-  DCHECK(!gaia_auth_fetcher_);
-  DCHECK(gaia_id_.empty());
-  DCHECK(email_.empty());
-  gaia_id_ = gaia_id;
-  email_ = email;
-  gaia_auth_fetcher_ = signin_client_->CreateGaiaAuthFetcher(
-      this, GaiaConstants::kChromeSource,
-      signin_client_->GetURLRequestContext());
-  gaia_auth_fetcher_->StartAuthCodeForOAuth2TokenExchange(authorization_code);
+  for (auto it = token_fetchers_.begin(); it != token_fetchers_.end(); ++it) {
+    if ((it->get()->gaia_id() == gaia_id) && (it->get()->email() == email))

[msarda, 2017/06/22 09:51:31]

This is a tricky case - in this case the Gaia cookie contains an account that
was reauthenticated while a token request was in progress. It is not clear to me
if the old authorization code can still be used. Also, it is not clear to me how
this would work once the cookie and the refresh tokens are bound one to another.

I think it should be safer to remove the old token fetcher and start a new one
with a fresher authorization code.

WDYT?

[droger, 2017/06/22 11:54:20]

Thanks fro bringing that up.

Summary of offline discussion:
1) we can cancel the request if the authorization code is the same
2) if requests are different and we receive a different token, then we should
probably revoke the old token. Note that this should be done even if the
requests don't happen at the same time, and thus belongs to another layer
(probably the token service: UpdateCredentials should revoke the old
credentials). Filed https://crbug.com/735898

+      return;  // There is already a request in flight with the same parameters.
+  }
 
-  // TODO(droger): The token exchange must complete quickly or be cancelled. Add
-  // a timeout logic.
+  token_fetchers_.push_back(base::MakeUnique<DiceTokenFetcher>(
+      gaia_id, email, authorization_code, signin_client_, this));
 }
 
-void DiceResponseHandler::OnClientOAuthSuccess(
-    const ClientOAuthResult& result) {
+void DiceResponseHandler::DeleteTokenFetcher(DiceTokenFetcher* token_fetcher) {
+  for (auto it = token_fetchers_.begin(); it != token_fetchers_.end(); ++it) {
+    if (it->get() == token_fetcher) {
+      token_fetchers_.erase(it);
+      return;
+    }
+  }
+  NOTREACHED();
+}
+
+void DiceResponseHandler::OnTokenExchangeSuccess(
+    DiceTokenFetcher* token_fetcher,
+    const std::string& gaia_id,
+    const std::string& email,
+    const GaiaAuthConsumer::ClientOAuthResult& result) {
   std::string account_id =
-      account_tracker_service_->SeedAccountInfo(gaia_id_, email_);
+      account_tracker_service_->SeedAccountInfo(gaia_id, email);
   profile_oauth2_token_service_->UpdateCredentials(account_id,
                                                    result.refresh_token);
-  gaia_id_.clear();
-  email_.clear();
-  gaia_auth_fetcher_.reset();
+  DeleteTokenFetcher(token_fetcher);
 }
 
-void DiceResponseHandler::OnClientOAuthFailure(
+void DiceResponseHandler::OnTokenExchangeFailure(
+    DiceTokenFetcher* token_fetcher,
     const GoogleServiceAuthError& error) {
   // TODO(droger): Handle authentication errors.
   DLOG(ERROR) << "Dice OAuth failed: " << error.error_message();
-  gaia_id_.clear();
-  email_.clear();
-  gaia_auth_fetcher_.reset();
+  DeleteTokenFetcher(token_fetcher);
 }