Adding the AccessTokenFetcher and Environment class to the app remoting test

remoting/test/access_token_fetcher.cc

+// Copyright 2015 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 "remoting/test/access_token_fetcher.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop.h"
+#include "base/thread_task_runner_handle.h"
+#include "google_apis/gaia/gaia_constants.h"
+#include "google_apis/google_api_keys.h"
+#include "net/url_request/url_fetcher.h"
+#include "remoting/base/url_request_context_getter.h"
+
+namespace {
+const int kMaxGetTokensRetries = 3;
+const char kOauthRedirectUrl[] =
+    "https://chromoting-oauth.talkgadget."
+    "google.com/talkgadget/oauth/chrome-remote-desktop/dev";
+
+// Populate a vector with the required permissions for app remoting.
+const std::vector<std::string> kAppRemotingScopeVector = {
+  "https://www.googleapis.com/auth/appremoting.runapplication",
+  "https://www.googleapis.com/auth/googletalk",
+  "https://www.googleapis.com/auth/userinfo.email",
+  "https://docs.google.com/feeds",
+  "https://www.googleapis.com/auth/drive"
+};
+}
+
+namespace remoting {
+namespace test {
+
+AccessTokenFetcher::AccessTokenFetcher() {
+  oauth_client_info_ = {
+      google_apis::GetOAuth2ClientID(google_apis::CLIENT_REMOTING),
+      google_apis::GetOAuth2ClientSecret(google_apis::CLIENT_REMOTING),
+      kOauthRedirectUrl};
+}
+
+AccessTokenFetcher::~AccessTokenFetcher() {
+}
+
+void AccessTokenFetcher::GetAccessTokenFromAuthCode(
+    const std::string& auth_code,
+    const AccessTokenCallback& callback) {
+  DCHECK(!auth_code.empty());
+  DCHECK(!callback.is_null());

[Wez, 2015/02/19 22:00:23]

nit: Typically we leave a blank line between DCHECKs on preconditions, and
actual function logic.

Now that you're DCHECK()ing these preconditions I'd suggest doing the same for
access_token_callback_.is_null(), since that is another precondition - the
caller is broken if we reach here while a request is already in progress.

[joedow, 2015/02/20 02:58:35]

Done.

+  if (!access_token_callback_.is_null()) {
+    LOG(ERROR) << "Access token request is already in progress";
+    return;
+  }
+
+  DVLOG(2) << "Calling GetTokensFromAuthCode to exchange auth_code for token";
+
+  access_token_.clear();
+  refresh_token_.clear();
+  access_token_callback_ = callback;
+
+  // |auth_client_| objects can only be used for one service call so we need to

[Wez, 2015/02/19 22:00:23]

What's a "service call"? Do you mean "request to GAIA", or something else?

|auth_client_| is a member, not a type, so it doesn't make sense to refer to
"|auth_client_| objects" - do you mean "OAuthClientClient instances" for
example?

[joedow, 2015/02/20 02:58:35]

Done.

+  // create a new one before each gaia service request.

[Wez, 2015/02/19 22:00:23]

Not sure what a "gaia service request" is?

Also, gaia -> GAIA here and everywhere else in this code.

Could this simply be "Create a new GaiaOAuthClient for each request" for
example?

[joedow, 2015/02/20 02:58:35]

Done.

+  CreateNewGaiaOAuthClientInstance();
+  auth_client_->GetTokensFromAuthCode(
+      oauth_client_info_, auth_code, kMaxGetTokensRetries,
+      this);  // GaiaOAuthClient::Delegate* delegate
+}
+
+void AccessTokenFetcher::GetAccessTokenFromRefreshToken(
+    const std::string& refresh_token,
+    const AccessTokenCallback& callback) {
+  DCHECK(!refresh_token.empty());
+  DCHECK(!callback.is_null());

[Wez, 2015/02/19 22:00:23]

See above

[joedow, 2015/02/20 02:58:35]

Done.

+  if (!access_token_callback_.is_null()) {
+    LOG(ERROR) << "Access token request is already in progress";
+    return;
+  }
+
+  DVLOG(2) << "Calling RefreshToken to generate a new access token";
+
+  access_token_.clear();
+  refresh_token_ = refresh_token;
+  access_token_callback_ = callback;
+
+  // |auth_client_| objects can only be used for one service call so we need to
+  // create a new one before each gaia service request.

[Wez, 2015/02/19 22:00:23]

See above

[joedow, 2015/02/20 02:58:35]

Done.

+  CreateNewGaiaOAuthClientInstance();
+  auth_client_->RefreshToken(
+      oauth_client_info_,
+      refresh_token_,
+      kAppRemotingScopeVector,
+      kMaxGetTokensRetries,
+      this);  // GaiaOAuthClient::Delegate* delegate
+}
+
+void AccessTokenFetcher::CreateNewGaiaOAuthClientInstance() {
+  scoped_refptr<remoting::URLRequestContextGetter> request_context_getter;
+  request_context_getter = new remoting::URLRequestContextGetter(
+      base::ThreadTaskRunnerHandle::Get(),   // network_runner
+      base::ThreadTaskRunnerHandle::Get());  // file_runner
+
+  auth_client_.reset(new gaia::GaiaOAuthClient(request_context_getter.get()));

[Wez, 2015/02/19 22:00:23]

When you reset this member, any existing GaiaOAuthClient instance will be
destroyed, even if it has a request pending - is that safe & correct?

[joedow, 2015/02/20 02:58:35]

A caller cannot make two concurrent requests (I had a condition check and have
replaced it was a DCHECK() in the two GetAccessTokenFrom* calls).  And the code
in the class only creates a new client in the Validate function which is called
after the previous call has completed.

That said, the GaiaOAuthClient could always be destroyed with a pending request
if the object is torn down so I'd expect that it should handle that (since there
isn't much a consumer of the class could do to prevent it from being destroyed
if it is also being torn down).

[Wez, 2015/02/23 20:17:14]

Agreed; it was really the AccessTokenFetcher caller semantics I was questioning,
since if you tear down an existing GaiaOAuthClient then you'll never get the
callbacks you'd have expected from it.

+}
+
+void AccessTokenFetcher::OnGetTokensResponse(const std::string& refresh_token,
+                                             const std::string& access_token,
+                                             int expires_in_seconds) {
+  DVLOG(1) << "AccessTokenFetcher::OnGetTokensResponse() Called";
+  DVLOG(1) << "--refresh_token: " << refresh_token;
+  DVLOG(1) << "--access_token: " << access_token;
+  DVLOG(1) << "--expires_in_seconds: " << expires_in_seconds;
+
+  refresh_token_ = refresh_token;
+  access_token_ = access_token;
+
+  ValidateAccessToken();
+}
+
+void AccessTokenFetcher::OnRefreshTokenResponse(const std::string& access_token,
+                                                int expires_in_seconds) {
+  DVLOG(1) << "AccessTokenFetcher::OnRefreshTokenResponse() Called";
+  DVLOG(1) << "--access_token: " << access_token;
+  DVLOG(1) << "--expires_in_seconds: " << expires_in_seconds;
+
+  access_token_ = access_token;
+
+  ValidateAccessToken();
+}
+
+void AccessTokenFetcher::OnGetUserEmailResponse(const std::string& user_email) {
+  // This callback should not be called as we do not request the user's email.
+  NOTREACHED();
+}
+
+void AccessTokenFetcher::OnGetUserIdResponse(const std::string& user_id) {
+  // This callback should not be called as we do not request the user's id.
+  NOTREACHED();
+}
+
+void AccessTokenFetcher::OnGetUserInfoResponse(
+    scoped_ptr<base::DictionaryValue> user_info) {
+  // This callback should not be called as we do not request user info.
+  NOTREACHED();
+}
+
+void AccessTokenFetcher::OnGetTokenInfoResponse(
+    scoped_ptr<base::DictionaryValue> token_info) {
+  DVLOG(1) << "AccessTokenFetcher::OnGetTokenInfoResponse() Called";
+
+  std::string error_string;
+  std::string error_description;
+
+  // Check to see if the token_info we received had any errors,
+  // otherwise we will assume that it is valid for our purposes.
+  if (token_info->HasKey("error")) {
+    token_info->GetString("error", &error_string);
+    token_info->GetString("error_description", &error_description);
+
+    LOG(ERROR) << "OnGetTokenInfoResponse returned an error. "
+               << ", "
+               << "error: " << error_string << ", "
+               << "description: " << error_description;
+    access_token_.clear();
+    refresh_token_.clear();
+  } else {
+    DVLOG(1) << "Access Token has been validated";
+  }
+
+  access_token_callback_.Run(access_token_, refresh_token_);
+  access_token_callback_.Reset();
+}
+
+void AccessTokenFetcher::OnOAuthError() {
+  LOG(ERROR) << "AccessTokenFetcher::OnOAuthError() Called";
+
+  access_token_.clear();
+  refresh_token_.clear();
+
+  access_token_callback_.Run(access_token_, refresh_token_);
+  access_token_callback_.Reset();
+}
+
+void AccessTokenFetcher::OnNetworkError(int response_code) {
+  LOG(ERROR) << "AccessTokenFetcher::OnNetworkError() Called";
+  LOG(ERROR) << "response code: " << response_code;
+
+  access_token_.clear();
+  refresh_token_.clear();
+
+  access_token_callback_.Run(access_token_, refresh_token_);
+  access_token_callback_.Reset();
+}
+
+void AccessTokenFetcher::ValidateAccessToken() {
+  DVLOG(2) << "Calling GetTokenInfo to validate access token";
+
+  // |auth_client_| objects can only be used for one service call so we need to
+  // create a new one before each gaia service request.
+  CreateNewGaiaOAuthClientInstance();
+  auth_client_->GetTokenInfo(
+      access_token_,
+      kMaxGetTokensRetries,
+      this);  // GaiaOAuthClient::Delegate* delegate
+}
+
+}  // namespace test
+}  // namespace remoting