Add ManagedUserTokenFetcher to fetch scoped-down tokens.

chrome/browser/managed_mode/managed_user_token_fetcher.cc

Index: chrome/browser/managed_mode/managed_user_token_fetcher.cc
diff --git a/chrome/browser/managed_mode/managed_user_token_fetcher.cc b/chrome/browser/managed_mode/managed_user_token_fetcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..59a33bc9573f42f457e3fa15b1d0a6580f8600a9
--- /dev/null
+++ b/chrome/browser/managed_mode/managed_user_token_fetcher.cc
@@ -0,0 +1,277 @@
+// Copyright 2013 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/managed_mode/managed_user_token_fetcher.h"
+
+#include "base/callback.h"
+#include "base/json/json_reader.h"
+#include "base/logging.h"
+#include "base/string16.h"
+#include "base/stringprintf.h"
+#include "base/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/browser/signin/oauth2_token_service.h"
+#include "google_apis/gaia/gaia_oauth_client.h"
+#include "google_apis/gaia/gaia_urls.h"
+#include "google_apis/gaia/google_service_auth_error.h"
+#include "google_apis/gaia/oauth2_api_call_flow.h"
+#include "net/base/escape.h"
+#include "net/base/load_flags.h"
+#include "net/base/net_errors.h"
+#include "net/http/http_status_code.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_status.h"
+
+using base::Time;
+using gaia::GaiaOAuthClient;
+using net::URLFetcher;
+using net::URLFetcherDelegate;
+using net::URLRequestContextGetter;
+
+namespace {
+
+const int kNumRetries = 1;
+
+static const char kChromeSyncManagedScope[] =
+    "https://www.googleapis.com/auth/chromesync_playpen";
+
+static const char kIssueTokenBodyFormat[] =
+    "client_id=%s"
+    "&scope=&%s"
+    "&response_type=code"

[Andrew T Wilson (Slow), 2013/05/28 13:10:08]

I wonder if it makes sense to extend OAuth2MintTokenFlow to support the extra
parameters you need instead of building your own parallel implementation here?
What do you think?

You mentioned that it makes testing a little trickier, but it sucks to basically
duplicate the same code just because we want to pass in profile_id/profile_name.

[Bernhard Bauer, 2013/05/29 11:41:45]

Well, there are also other problems:
* OAuth2MintTokenFlow uses OAuth2AccessTokenFetcher instead of
OAuth2TokenService, so it doesn't cache the access token
* It fetches an access token, not an authentication code for a refresh token.
* It takes the (custodian's) refresh token, and you said we don't want to expose
that.
* It also takes an extension ID, which we don't have.

If I went ahead and fixed that, I don't think there'd be that much duplication
left.

+    "&profile_id=%s"
+    "&profile_name=%s"
+    "&device_name=%s";
+
+static const char kAuthorizationHeaderFormat[] =
+    "Authorization: Bearer %s";
+
+static const char kCodeKey[] = "code";
+
+class ManagedUserTokenFetcherImpl : public ManagedUserTokenFetcher,

[Andrew T Wilson (Slow), 2013/05/28 13:10:08]

BTW, I like this pattern of keeping the Impl class hidden. Makes it so much
easier to read the public interface. People don't use this pattern enough -
thanks for doing this.

+                                    public OAuth2TokenService::Consumer,
+                                    public URLFetcherDelegate,
+                                    public GaiaOAuthClient::Delegate {
+ public:
+  ManagedUserTokenFetcherImpl(OAuth2TokenService* oauth2_token_service,
+                              URLRequestContextGetter* context);
+  virtual ~ManagedUserTokenFetcherImpl();
+
+  // ManagedUserTokenFetcher implementation:
+  virtual void Start(const std::string& managed_user_id,
+                     const string16& name,
+                     const std::string& device_name,
+                     const TokenCallback& callback) OVERRIDE;
+
+ protected:
+  // OAuth2TokenService::Consumer implementation:
+  virtual void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
+                                 const std::string& access_token,
+                                 const Time& expiration_time) OVERRIDE;
+  virtual void OnGetTokenFailure(const OAuth2TokenService::Request* request,
+                                 const GoogleServiceAuthError& error) OVERRIDE;
+
+  // net::URLFetcherDelegate implementation.
+  virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
+
+  // GaiaOAuthClient::Delegate implementation:
+  virtual void OnGetTokensResponse(const std::string& refresh_token,
+                                   const std::string& access_token,
+                                   int expires_in_seconds) OVERRIDE;
+  virtual void OnRefreshTokenResponse(const std::string& access_token,
+                                      int expires_in_seconds) OVERRIDE;
+  virtual void OnOAuthError() OVERRIDE;
+  virtual void OnNetworkError(int response_code) OVERRIDE;
+
+ private:
+  // Requests an access token, which is the first thing we need. This is where
+  // we restart when the returned access token has expired.
+  void StartFetching();
+
+  void DispatchNetworkError(int error_code);
+  void DispatchGoogleServiceAuthError(GoogleServiceAuthError::State foo);
+
+  OAuth2TokenService* oauth2_token_service_;
+  URLRequestContextGetter* context_;
+
+  std::string device_name_;
+  std::string managed_user_id_;
+  string16 name_;
+  TokenCallback callback_;
+
+  scoped_ptr<OAuth2TokenService::Request> access_token_request_;
+  std::string access_token_;
+  scoped_ptr<URLFetcher> url_fetcher_;
+  scoped_ptr<GaiaOAuthClient> gaia_oauth_client_;
+};
+
+ManagedUserTokenFetcherImpl::ManagedUserTokenFetcherImpl(
+    OAuth2TokenService* oauth2_token_service,
+    URLRequestContextGetter* context)
+    : oauth2_token_service_(oauth2_token_service),
+      context_(context) {}
+
+ManagedUserTokenFetcherImpl::~ManagedUserTokenFetcherImpl() {}
+
+void ManagedUserTokenFetcherImpl::Start(
+    const std::string& managed_user_id,
+    const string16& name,
+    const std::string& device_name,
+    const TokenCallback& callback) {
+  DCHECK(callback_.is_null());
+  managed_user_id_ = managed_user_id;
+  name_ = name;
+  device_name_ = device_name;
+  callback_ = callback;
+  StartFetching();

[Andrew T Wilson (Slow), 2013/05/28 13:10:08]

So, let's imagine that we have a really slow internet connection - this could
take a long time to mint this token. Is there UI displayed to the user? Can the
user cancel it? It seems like there's no way for a caller to cancel an
in-progress request (and, in fact, if there's an existing request already
outstanding, anyone invoking Start() will hit a DCHECK.

[Bernhard Bauer, 2013/05/29 11:41:45]

Yes, at least on desktop. I'll ask about Chrome OS.
Well, in this class the request can be canceled by destroying it. There are some
tests for that.

Now, the way this class is currently used, it is owned by the
ManagedUserRegistrationService, which is a ProfileKeyedService, so it will only
get canceled when the user logs out (on Chrome OS) or Chrome shuts down (on
Desktop). I want to separate the SyncableService from the registration part
anyway though, which would mean that the registration part could be owned by its
client again.

[Andrew T Wilson (Slow), 2013/05/29 12:05:33]

I guess I don't understand the flow, then.

1) User performs some kind of "create managed user" operation in the UI. They
get some kind of progress dialog.
2) Code invokes ManagedUserTokenFetcher::Start()
3) User cancels out of the progress dialog.

Now what? There's an operation in progress in ManagedUserTokenFetcher - how do
we cancel it? In theory, the caller could free the ManagedUserTokenFetcher, but
since it's passed in to ManagedUserService there's no way for ManagedUserService
to free it and obtain a new one.

[Bernhard Bauer, 2013/05/29 12:56:55]

Yes. I'm planning on adding that as soon as Pam has implemented the new managed
user creation flow on desktop, but I don't want to change the interface she's
using while her CL is in flight.

If you want to, I can add a CancelRegistration() method to ManagedUserService,
but it's going to change soon anyway.

[Andrew T Wilson (Slow), 2013/05/29 13:12:17]

Adding a TODO in the header file is sufficient for now.

[Bernhard Bauer, 2013/05/29 14:11:15]

Done.

+}
+
+void ManagedUserTokenFetcherImpl::StartFetching() {
+  // An empty set of scopes means the same scope as the refresh token.

[Andrew T Wilson (Slow), 2013/05/28 13:10:08]

We should document this in OAuth2TokenService (that "no scopes" == "same scope
as the refresh token") if we're going to rely on this behavior.

[Bernhard Bauer, 2013/05/29 11:41:45]

Ok. Alternatively, I could pass in the login scope explicitly?

[Andrew T Wilson (Slow), 2013/05/29 12:05:33]

Passing in an explicit scope would be ideal.

[Bernhard Bauer, 2013/05/29 12:56:55]

Done.

+  OAuth2TokenService::ScopeSet scopes;
+  access_token_request_ = oauth2_token_service_->StartRequest(scopes, this);
+}
+
+void ManagedUserTokenFetcherImpl::OnGetTokenSuccess(
+    const OAuth2TokenService::Request* request,
+    const std::string& access_token,
+    const Time& expiration_time) {
+  DCHECK_EQ(access_token_request_.get(), request);
+  access_token_ = access_token;
+
+  GURL url(GaiaUrls::GetInstance()->oauth2_issue_token_url());
+  // GaiaOAuthClient uses id 0, so we use 1 to distinguish the requests in
+  // unit tests.
+  int id = 1;

[Andrew T Wilson (Slow), 2013/05/28 13:10:08]

nit: const int id

[Bernhard Bauer, 2013/05/29 11:41:45]

Done.

+
+  url_fetcher_.reset(URLFetcher::Create(id, url, URLFetcher::POST, this));
+
+  url_fetcher_->SetRequestContext(context_);
+  url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
+                             net::LOAD_DO_NOT_SAVE_COOKIES);
+  url_fetcher_->SetAutomaticallyRetryOnNetworkChanges(kNumRetries);
+  url_fetcher_->AddExtraRequestHeader(
+      base::StringPrintf(kAuthorizationHeaderFormat, access_token.c_str()));
+
+  std::string body = base::StringPrintf(
+      kIssueTokenBodyFormat,
+      net::EscapeUrlEncodedData(
+          GaiaUrls::GetInstance()->oauth2_chrome_client_id(), true).c_str(),
+      net::EscapeUrlEncodedData(kChromeSyncManagedScope, true).c_str(),
+      net::EscapeUrlEncodedData(managed_user_id_, true).c_str(),
+      net::EscapeUrlEncodedData(UTF16ToUTF8(name_), true).c_str(),
+      net::EscapeUrlEncodedData(device_name_, true).c_str());
+  url_fetcher_->SetUploadData("application/x-www-form-urlencoded", body);
+
+  url_fetcher_->Start();
+}
+
+void ManagedUserTokenFetcherImpl::OnGetTokenFailure(
+    const OAuth2TokenService::Request* request,
+    const GoogleServiceAuthError& error) {
+  DCHECK_EQ(access_token_request_.get(), request);
+  callback_.Run(error, std::string());
+  callback_.Reset();
+}
+
+void ManagedUserTokenFetcherImpl::OnURLFetchComplete(
+    const URLFetcher* source) {
+  const net::URLRequestStatus& status = source->GetStatus();
+  if (!status.is_success()) {
+    DispatchNetworkError(status.error());
+    return;
+  }
+
+  int response_code = source->GetResponseCode();
+  if (response_code == net::HTTP_UNAUTHORIZED) {
+    oauth2_token_service_->InvalidateToken(OAuth2TokenService::ScopeSet(),
+                                           access_token_);
+    StartFetching();

[Andrew T Wilson (Slow), 2013/05/28 13:10:08]

If there's some bug that causes the server to permanently return 401s, this will
loop forever and hammer the server. Do we want to put a limit on the number of
retries?

[Bernhard Bauer, 2013/05/29 11:41:45]

Good idea! Done.

+    return;
+  }
+
+  if (response_code != net::HTTP_OK) {
+    // TODO(bauerb): We should return the HTTP response code somehow.
+    LOG(WARNING) << "HTTP error " << response_code;
+    DispatchGoogleServiceAuthError(GoogleServiceAuthError::CONNECTION_FAILED);
+    return;
+  }
+
+  std::string response_body;
+  source->GetResponseAsString(&response_body);
+  scoped_ptr<base::Value> value(base::JSONReader::Read(response_body));
+  DictionaryValue* dict = NULL;
+  if (!value.get() || !value->GetAsDictionary(&dict)) {
+    DispatchNetworkError(net::ERR_INVALID_RESPONSE);
+    return;
+  }
+  std::string auth_code;
+  if (!dict->GetString(kCodeKey, &auth_code)) {
+    DispatchNetworkError(net::ERR_INVALID_RESPONSE);
+    return;
+  }
+
+  gaia::OAuthClientInfo client_info;
+  GaiaUrls* urls = GaiaUrls::GetInstance();
+  client_info.client_id = urls->oauth2_chrome_client_id();
+  client_info.client_secret = urls->oauth2_chrome_client_secret();
+  gaia_oauth_client_.reset(
+      new gaia::GaiaOAuthClient(GaiaUrls::GetInstance()->oauth2_token_url(),
+                                context_));
+  gaia_oauth_client_->GetTokensFromAuthCode(client_info, auth_code, kNumRetries,
+                                            this);
+}
+
+void ManagedUserTokenFetcherImpl::OnGetTokensResponse(
+    const std::string& refresh_token,
+    const std::string& access_token,
+    int expires_in_seconds) {
+  // TODO(bauerb): It would be nice if we could pass the access token as well,
+  // so we don't need to fetch another one immediately.
+  GoogleServiceAuthError error(GoogleServiceAuthError::NONE);

[Andrew T Wilson (Slow), 2013/05/29 12:05:33]

Call DispatchGoogleServiceAuthError(GoogleServiceAuthError::NONE) here instead?
The idea is that the callback would be invoked/cleared from exactly one place.

[Bernhard Bauer, 2013/05/29 12:56:55]

Done.

+  callback_.Run(error, refresh_token);
+  callback_.Reset();
+}
+
+void ManagedUserTokenFetcherImpl::OnRefreshTokenResponse(
+    const std::string& access_token,
+    int expires_in_seconds) {
+  NOTREACHED();
+}
+
+void ManagedUserTokenFetcherImpl::OnOAuthError() {
+  DispatchGoogleServiceAuthError(GoogleServiceAuthError::CONNECTION_FAILED);
+}
+
+void ManagedUserTokenFetcherImpl::OnNetworkError(int response_code) {
+  // TODO(bauerb): We should return the HTTP response code somehow.
+  LOG(WARNING) << "HTTP error " << response_code;

[Andrew T Wilson (Slow), 2013/05/29 12:05:33]

I'm always leery of adding LOG() statements since they bloat the release client.
Maybe this should be a DLOG()?

[Bernhard Bauer, 2013/05/29 12:56:55]

Done. It does mean that you can only see the HTTP response code in a Debug build
however if something goes wrong.

+  DispatchGoogleServiceAuthError(GoogleServiceAuthError::CONNECTION_FAILED);
+}
+
+void ManagedUserTokenFetcherImpl::DispatchNetworkError(int error_code) {
+  callback_.Run(GoogleServiceAuthError::FromConnectionError(error_code),

[Andrew T Wilson (Slow), 2013/05/28 13:10:08]

Maybe change this to:

{
 
DispatchGoogleServiceAuthError(GoogleServiceAuthError::FromConnectionError(error_code))
}

?

[Bernhard Bauer, 2013/05/29 11:41:45]

Do you mean adding an overload that takes a GoogleServiceAuthError (as opposed
to a GoogleServiceAuthError::State)? Or changing the signature?

[Andrew T Wilson (Slow), 2013/05/29 12:05:33]

I mean that instead of calling callback_.Run() and callback_.Reset() here, you
might as well call DispatchGoogleServiceAuthError(). That way all the error
handling code that invokes a callback goes through a single routine which makes
it easier to maintain.

[Bernhard Bauer, 2013/05/29 12:56:55]

Done.

+                std::string());
+  callback_.Reset();
+}
+
+void ManagedUserTokenFetcherImpl::DispatchGoogleServiceAuthError(
+    GoogleServiceAuthError::State state) {
+  GoogleServiceAuthError error(state);
+  callback_.Run(error, std::string());
+  callback_.Reset();
+}
+
+}  // namespace
+
+// static
+scoped_ptr<ManagedUserTokenFetcher> ManagedUserTokenFetcher::Create(
+    OAuth2TokenService* oauth2_token_service,
+    URLRequestContextGetter* context) {
+  scoped_ptr<ManagedUserTokenFetcher> fetcher(
+      new ManagedUserTokenFetcherImpl(oauth2_token_service, context));
+  return fetcher.Pass();
+}
+
+ManagedUserTokenFetcher::~ManagedUserTokenFetcher() {}