[remoting host] Add timeout to OAuthTokenGetterImpl

gaia::GaiaOAuthClient doesn't call request_->SetTimeoutDuration() when
sending out the request, which might be the cause of b/166817002 -- if
CallWithToken never calls the callback, then all pending authenticated
requests will be blocked.

This CL adds a OneShotTimer to OAuthTokenGetterImpl so that it rejects
the callback if it times out.

Bug: 1123199
Change-Id: Ic05de360983b2540489edbf33aa0eee2f0c5c87f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382488
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803239}

remoting/base/oauth_token_getter_impl.cc

@@ -22,7 +22,11 @@ namespace {
 const int kMaxRetries = 3;
 
 // Time when we we try to update OAuth token before its expiration.
-const int kTokenUpdateTimeBeforeExpirySeconds = 60;
+const int kTokenUpdateTimeBeforeExpirySeconds = 120;
+
+// Max time we wait for the response before giving up.
+constexpr base::TimeDelta kResponseTimeoutDuration =
+    base::TimeDelta::FromSeconds(30);
 
 }  // namespace
 
@@ -31,7 +35,8 @@ OAuthTokenGetterImpl::OAuthTokenGetterImpl(
     const OAuthTokenGetter::CredentialsUpdatedCallback& on_credentials_update,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     bool auto_refresh)
-    : intermediate_credentials_(std::move(intermediate_credentials)),
+    : url_loader_factory_(url_loader_factory),
+      intermediate_credentials_(std::move(intermediate_credentials)),
       gaia_oauth_client_(new gaia::GaiaOAuthClient(url_loader_factory)),
       credentials_updated_callback_(on_credentials_update) {
   if (auto_refresh) {
@@ -43,7 +48,8 @@ OAuthTokenGetterImpl::OAuthTokenGetterImpl(
     std::unique_ptr<OAuthAuthorizationCredentials> authorization_credentials,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     bool auto_refresh)
-    : authorization_credentials_(std::move(authorization_credentials)),
+    : url_loader_factory_(url_loader_factory),
+      authorization_credentials_(std::move(authorization_credentials)),
       gaia_oauth_client_(new gaia::GaiaOAuthClient(url_loader_factory)) {
   if (auto_refresh) {
     refresh_timer_.reset(new base::OneShotTimer());
@@ -142,7 +148,7 @@ void OAuthTokenGetterImpl::NotifyTokenCallbacks(
     const std::string& access_token) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  response_pending_ = false;
+  SetResponsePending(false);
 
   base::queue<TokenCallback> callbacks;
   callbacks.swap(pending_callbacks_);
@@ -189,7 +195,7 @@ void OAuthTokenGetterImpl::CallWithToken(TokenCallback on_access_token) {
   pending_callbacks_.push(std::move(on_access_token));
 
   if (intermediate_credentials_) {
-    if (!response_pending_) {
+    if (!IsResponsePending()) {
       GetOauthTokensFromAuthCode();
     }
   } else {
@@ -199,15 +205,13 @@ void OAuthTokenGetterImpl::CallWithToken(TokenCallback on_access_token) {
         (!authorization_credentials_->is_service_account && !email_verified_);
 
     if (need_new_auth_token) {
-      if (!response_pending_) {
+      if (!IsResponsePending()) {
         RefreshAccessToken();
       }
     } else {
-      // If |response_pending_| is true here, |oauth_access_token_| is
-      // up-to-date but not yet exchanged (it might not have the needed scopes).
-      // In that case, wait for token-exchange to complete before returning the
-      // token.
-      if (!response_pending_) {
+      // If IsResponsePending() is true here, |on_access_token| will be called
+      // when the response is received.
+      if (!IsResponsePending()) {
         NotifyTokenCallbacks(OAuthTokenGetterImpl::SUCCESS,
                              authorization_credentials_->login,
                              oauth_access_token_);
@@ -228,7 +232,7 @@ base::WeakPtr<OAuthTokenGetterImpl> OAuthTokenGetterImpl::GetWeakPtr() {
 void OAuthTokenGetterImpl::GetOauthTokensFromAuthCode() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VLOG(1) << "Fetching OAuth token from Auth Code.";
-  DCHECK(!response_pending_);
+  DCHECK(!IsResponsePending());
 
   // Service accounts use different API keys, as they use the client app flow.
   google_apis::OAuth2Client oauth2_client =
@@ -245,7 +249,7 @@ void OAuthTokenGetterImpl::GetOauthTokensFromAuthCode() {
       google_apis::GetOAuth2ClientID(oauth2_client),
       google_apis::GetOAuth2ClientSecret(oauth2_client), redirect_uri};
 
-  response_pending_ = true;
+  SetResponsePending(true);
 
   gaia_oauth_client_->GetTokensFromAuthCode(
       client_info, intermediate_credentials_->authorization_code, kMaxRetries,
@@ -255,7 +259,7 @@ void OAuthTokenGetterImpl::GetOauthTokensFromAuthCode() {
 void OAuthTokenGetterImpl::RefreshAccessToken() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VLOG(1) << "Refreshing OAuth Access token.";
-  DCHECK(!response_pending_);
+  DCHECK(!IsResponsePending());
 
   // Service accounts use different API keys, as they use the client app flow.
   google_apis::OAuth2Client oauth2_client =
@@ -270,11 +274,38 @@ void OAuthTokenGetterImpl::RefreshAccessToken() {
       // is not required when getting access tokens from refresh tokens.
       ""};
 
-  response_pending_ = true;
+  SetResponsePending(true);
   std::vector<std::string> empty_scope_list;  // Use scope from refresh token.
   gaia_oauth_client_->RefreshToken(client_info,
                                    authorization_credentials_->refresh_token,
                                    empty_scope_list, kMaxRetries, this);
 }
 
+bool OAuthTokenGetterImpl::IsResponsePending() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return response_timeout_timer_.IsRunning();
+}
+
+void OAuthTokenGetterImpl::SetResponsePending(bool is_pending) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (is_pending) {
+    if (IsResponsePending()) {
+      LOG(DFATAL) << "The response is already pending.";
+      return;
+    }
+    response_timeout_timer_.Start(FROM_HERE, kResponseTimeoutDuration, this,
+                                  &OAuthTokenGetterImpl::OnResponseTimeout);
+  } else {
+    response_timeout_timer_.Stop();
+  }
+}
+
+void OAuthTokenGetterImpl::OnResponseTimeout() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  LOG(ERROR) << "GaiaOAuthClient response timeout";
+  gaia_oauth_client_ =
+      std::make_unique<gaia::GaiaOAuthClient>(url_loader_factory_);
+  NotifyTokenCallbacks(OAuthTokenGetterImpl::NETWORK_ERROR, {}, {});
+}
+
 }  // namespace remoting

remoting/base/oauth_token_getter_impl.h

@@ -67,18 +67,23 @@ class OAuthTokenGetterImpl : public OAuthTokenGetter,
   void GetOauthTokensFromAuthCode();
   void RefreshAccessToken();
 
+  bool IsResponsePending() const;
+  void SetResponsePending(bool is_pending);
+  void OnResponseTimeout();
+
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
   std::unique_ptr<OAuthIntermediateCredentials> intermediate_credentials_;
   std::unique_ptr<OAuthAuthorizationCredentials> authorization_credentials_;
   std::unique_ptr<gaia::GaiaOAuthClient> gaia_oauth_client_;
   OAuthTokenGetter::CredentialsUpdatedCallback credentials_updated_callback_;
 
-  bool response_pending_ = false;
   bool email_verified_ = false;
   bool email_discovery_ = false;
   std::string oauth_access_token_;
   base::Time access_token_expiry_time_;
   base::queue<OAuthTokenGetter::TokenCallback> pending_callbacks_;
   std::unique_ptr<base::OneShotTimer> refresh_timer_;
+  base::OneShotTimer response_timeout_timer_;
 
   SEQUENCE_CHECKER(sequence_checker_);