Separate access token caching logic from signaling connector.

remoting/host/oauth_token_getter.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[Sergey Ulanov, 2014/02/08 03:16:36]

ditto

[rmsousa, 2014/02/10 23:07:39]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "remoting/host/oauth_token_getter.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/strings/string_util.h"
+#include "google_apis/google_api_keys.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "remoting/base/logging.h"
+
+namespace remoting {
+
+namespace {
+
+// Maximum number of retries on network/500 errors.
+const int kMaxRetries = 3;
+
+// Time when we we try to update OAuth token before its expiration.
+const int kTokenUpdateTimeBeforeExpirySeconds = 60;
+
+}  // namespace
+
+OAuthTokenGetter::OAuthCredentials::OAuthCredentials(
+    const std::string& login_value,

[Sergey Ulanov, 2014/02/08 03:16:36]

remove _value suffix for this and the next field or add it for the last one.

[rmsousa, 2014/02/10 23:07:39]

Done.

+    const std::string& refresh_token_value,
+    bool is_service_account)
+    : login(login_value),
+      refresh_token(refresh_token_value),
+      is_service_account(is_service_account) {
+}
+
+OAuthTokenGetter::Callbacks::Callbacks(const TokenCallback& on_access_token,
+                                       const base::Closure& on_network_error,
+                                       const base::Closure& on_auth_error)
+    : on_access_token(on_access_token),
+      on_network_error(on_network_error),
+      on_auth_error(on_auth_error) {
+}
+
+OAuthTokenGetter::Callbacks::~Callbacks() {
+}

[Sergey Ulanov, 2014/02/08 03:16:36]

nit: } can be moved to the previous line.

[rmsousa, 2014/02/10 23:07:39]

Class removed

+
+OAuthTokenGetter::OAuthTokenGetter(
+    scoped_ptr<OAuthCredentials> oauth_credentials,
+    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter)
+    : oauth_credentials_(oauth_credentials.Pass()),
+      gaia_oauth_client_(
+          new gaia::GaiaOAuthClient(url_request_context_getter)),
+      url_request_context_getter_(url_request_context_getter),
+      refreshing_oauth_token_(false) {
+}
+
+OAuthTokenGetter::~OAuthTokenGetter() {
+}

[Sergey Ulanov, 2014/02/08 03:16:36]

same here

[rmsousa, 2014/02/10 23:07:39]

Done.

+
+void OAuthTokenGetter::OnGetTokensResponse(const std::string& user_email,
+                                           const std::string& access_token,
+                                           int expires_seconds) {
+  NOTREACHED();
+}
+
+void OAuthTokenGetter::OnRefreshTokenResponse(
+    const std::string& access_token,
+    int expires_seconds) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(oauth_credentials_.get());
+  HOST_LOG << "Received OAuth token.";
+
+  oauth_access_token_ = access_token;
+  auth_token_expiry_time_ = base::Time::Now() +
+      base::TimeDelta::FromSeconds(expires_seconds) -
+      base::TimeDelta::FromSeconds(kTokenUpdateTimeBeforeExpirySeconds);
+
+  gaia_oauth_client_->GetUserEmail(access_token, kMaxRetries, this);
+}
+
+void OAuthTokenGetter::OnGetUserEmailResponse(const std::string& user_email) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(oauth_credentials_.get());
+  HOST_LOG << "Received user info.";
+
+  if (user_email != oauth_credentials_->login) {
+    LOG(ERROR) << "OAuth token and email address do not refer to "
+        "the same account.";
+    OnOAuthError();
+    return;
+  }
+
+  refreshing_oauth_token_ = false;
+
+  // Now that we've refreshed the token and verified that it's for the correct
+  // user account, try to connect using the new token.
+  while (!pending_callbacks_.empty()) {
+    Callbacks callbacks = pending_callbacks_.front();
+    pending_callbacks_.pop();
+    callbacks.on_access_token.Run(user_email, oauth_access_token_);

[Sergey Ulanov, 2014/02/08 03:16:36]

Each callback handler may call this class again and add another callback into
the queue. It might not be problem here, but it will be a problem in error
cases. So instead of copying each separate callback inside the loop (which
doesn't give you anything) you need to move all pending_callbacks_ to a separate
container on stack and call them from there.
As a general callback hygiene measure callback callers should never touch (read
or write) internal state after calling a callback.

[rmsousa, 2014/02/10 23:07:39]

Good point. Done.

+  }
+}
+
+void OAuthTokenGetter::OnOAuthError() {
+  DCHECK(CalledOnValidThread());
+  LOG(ERROR) << "OAuth: invalid credentials.";
+  refreshing_oauth_token_ = false;
+  while (!pending_callbacks_.empty()) {

[Sergey Ulanov, 2014/02/08 03:16:36]

If you use single callback for all results then this loop can be moved to a
helper function and not duplicated in three places.

[rmsousa, 2014/02/10 23:07:39]

Done.

+    Callbacks callbacks = pending_callbacks_.front();
+    pending_callbacks_.pop();
+    callbacks.on_auth_error.Run();
+  }
+}
+
+void OAuthTokenGetter::OnNetworkError(int response_code) {
+  DCHECK(CalledOnValidThread());
+  LOG(ERROR) << "Network error when trying to update OAuth token: "
+             << response_code;
+  refreshing_oauth_token_ = false;
+  while (!pending_callbacks_.empty()) {
+    Callbacks callbacks = pending_callbacks_.front();
+    pending_callbacks_.pop();
+    callbacks.on_network_error.Run();
+  }
+}
+
+void OAuthTokenGetter::CallWithToken(const TokenCallback& on_access_token,
+                                     const base::Closure& on_network_error,
+                                     const base::Closure& on_auth_error) {
+  DCHECK(CalledOnValidThread());
+  bool need_new_auth_token = oauth_credentials_.get() &&
+      (auth_token_expiry_time_.is_null() ||
+       base::Time::Now() >= auth_token_expiry_time_);
+  if (need_new_auth_token) {
+    pending_callbacks_.push(
+        Callbacks(on_access_token, on_network_error, on_auth_error));
+    if (!refreshing_oauth_token_)
+      RefreshOAuthToken();
+  } else {
+    on_access_token.Run(oauth_credentials_->login, oauth_access_token_);
+  }
+}
+
+void OAuthTokenGetter::RefreshOAuthToken() {
+  DCHECK(CalledOnValidThread());
+  HOST_LOG << "Refreshing OAuth token.";
+  DCHECK(!refreshing_oauth_token_);
+
+  // Service accounts use different API keys, as they use the client app flow.
+  google_apis::OAuth2Client oauth2_client;
+  if (oauth_credentials_->is_service_account) {
+    oauth2_client = google_apis::CLIENT_REMOTING_HOST;
+  } else {
+    oauth2_client = google_apis::CLIENT_REMOTING;
+  }
+
+  gaia::OAuthClientInfo client_info = {
+    google_apis::GetOAuth2ClientID(oauth2_client),
+    google_apis::GetOAuth2ClientSecret(oauth2_client),
+    // Redirect URL is only used when getting tokens from auth code. It
+    // is not required when getting access tokens.
+    ""
+  };
+
+  refreshing_oauth_token_ = true;
+  std::vector<std::string> empty_scope_list;  // (Use scope from refresh token.)

[Sergey Ulanov, 2014/02/08 03:16:36]

nit: remove parentheses

[rmsousa, 2014/02/10 23:07:39]

Done.

+  gaia_oauth_client_->RefreshToken(
+      client_info, oauth_credentials_->refresh_token, empty_scope_list,
+      kMaxRetries, this);
+}
+
+}  // namespace remoting