Public Sessions: fetch device robot api token during enterprise enrollment.

chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc

Index: chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
diff --git a/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc b/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
index bd97ade258850b2656b0e13673c57c7041f802e5..3d94f551850a3c356bb490b8b977a4c78f93f348 100644
--- a/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
+++ b/chrome/browser/chromeos/policy/enrollment_handler_chromeos.cc
@@ -7,11 +7,13 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.h"
 #include "chrome/browser/chromeos/policy/enterprise_install_attributes.h"
 #include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h"
 #include "chrome/browser/policy/cloud/cloud_policy_constants.h"
 #include "chrome/browser/policy/cloud/proto/device_management_backend.pb.h"
+#include "google_apis/gaia/gaia_urls.h"
 
 namespace em = enterprise_management;
 
@@ -23,6 +25,8 @@ namespace {
 const int kLockRetryIntervalMs = 500;
 // Maximum time to retry InstallAttrs initialization before we give up.
 const int kLockRetryTimeoutMs = 10 * 60 * 1000;  // 10 minutes.
+// Number of times to retry fetching the device-level API refresh token.
+const int kRobotRefreshTokenFetchRetryCount = 2;
 
 }  // namespace
 
@@ -110,7 +114,7 @@ void EnrollmentHandlerChromeOS::OnRegistrationStateChanged(
   DCHECK_EQ(client_.get(), client);
 
   if (enrollment_step_ == STEP_REGISTRATION && client_->is_registered()) {
-    enrollment_step_ = STEP_POLICY_FETCH,
+    enrollment_step_ = STEP_ROBOT_AUTH_FETCH,
     device_mode_ = client_->device_mode();
     if (device_mode_ == DEVICE_MODE_NOT_SET)
       device_mode_ = DEVICE_MODE_ENTERPRISE;
@@ -120,17 +124,81 @@ void EnrollmentHandlerChromeOS::OnRegistrationStateChanged(
           EnrollmentStatus::STATUS_REGISTRATION_BAD_MODE));
       return;
     }
-    client_->FetchPolicy();
+    client_->FetchRobotAuthTokens(auth_token_);
   } else {
     LOG(FATAL) << "Registration state changed to " << client_->is_registered()
                << " in step " << enrollment_step_;
   }
 }
 
+void EnrollmentHandlerChromeOS::OnRobotAuthCodesFetched(
+    CloudPolicyClient* client) {
+  DCHECK_EQ(client_.get(), client);
+  CHECK_EQ(STEP_ROBOT_AUTH_FETCH, enrollment_step_);
+
+  enrollment_step_ = STEP_ROBOT_AUTH_REFRESH;
+
+  gaia::OAuthClientInfo client_info;
+  client_info.client_id = GaiaUrls::GetInstance()->oauth2_chrome_client_id();
+  client_info.client_secret =
+      GaiaUrls::GetInstance()->oauth2_chrome_client_secret();
+
+  // Use the system request context to avoid sending user cookies.
+  gaia_oauth_client_.reset(new gaia::GaiaOAuthClient(
+      gaia::kGaiaOAuth2Url,
+      // TODO: use DeviceManagementRequestContextGetter?  It seems to just use
+      // the system_request_context internally, an manually return the IO thread

[Mattias Nissler (ping if slow), 2013/03/19 06:33:08]

grammar

[David Roche, 2013/04/02 01:59:25]

Done.

+      // message loop, but the system_request_context already does this?  What
+      // does DeviceManagementRequestContextGetter change?

[Mattias Nissler (ping if slow), 2013/03/19 06:33:08]

It uses a different user agent, and disables all cookie handling. In this case,
system_request_context is probably fine. But please do pass that dependency in
for better testability.

[David Roche, 2013/04/02 01:59:25]

It would make sense for testability if a test depended on injecting a custom
value.  Grep'ing through the code, I can't find an instance of any tests doing
this.  I also based this direct use of
g_browser_process->system_request_context() on the implementation of
device_management_service.cc, which led me to assume that this usage pattern was
okay.

I can directly pass in the dependency if you feel strongly about it, but it
seems like it won't add any additional value?

[Mattias Nissler (ping if slow), 2013/04/02 14:16:46]

OK, tell you what: You make sure the test for this (i.e.
DeviceCloudPolicyManagerChromeOSEnrollmentTest) has proper coverage and passes.
If the reference to the global here is the best way to achieve that, I'm not to
complain :)

[David Roche, 2013/04/04 01:39:53]

Ah, yes, updating tests.  That is on the agenda for tomorrow.

+      g_browser_process->system_request_context()));
+  gaia_oauth_client_->GetTokensFromAuthCode(client_info,
+                                            client->robot_api_auth_code(),
+                                            kRobotRefreshTokenFetchRetryCount,

[Mattias Nissler (ping if slow), 2013/03/19 06:33:08]

why would we have to retry at all?

[David Roche, 2013/04/02 01:59:25]

I retry b/c other bits of enrollment seem to retry, and it appeared that it was
considered a good thing: https://codereview.chromium.org/11572044

[Mattias Nissler (ping if slow), 2013/04/02 14:16:46]

That's an entirely separate issue that covers our back when the network config
changes while we're registering or fetching policy. That's not the case here
though, so unless we have a good reason why we need this, my vote is to not
retry - chances are retrying doesn't help in this case. 

[David Roche, 2013/04/04 01:39:53]

Done.

+                                            this);
+}
+
+// GaiaOAuthClient::Delegate callback for OAuth2 refresh token fetched.
+void EnrollmentHandlerChromeOS::OnGetTokensResponse(
+    const std::string& refresh_token,
+    const std::string& access_token,
+    int expires_in_seconds) {
+  CHECK_EQ(STEP_ROBOT_AUTH_REFRESH, enrollment_step_);
+
+  enrollment_step_ = STEP_POLICY_FETCH,
+
+  // TODO: persist token in DeviceOAuth2TokenService when CL 12647008 lands.
+
+  client_->FetchPolicy();
+}
+
+// GaiaOAuthClient::Delegate
+void EnrollmentHandlerChromeOS::OnRefreshTokenResponse(
+    const std::string& access_token,
+    int expires_in_seconds) {
+  // We never use the code that should trigger this callback.
+  NOTREACHED() << "Unexpected callback invoked";

[Mattias Nissler (ping if slow), 2013/03/19 06:33:08]

either need to notify the delegate, or do a LOG(FATAL) (i.e. crash)

[David Roche, 2013/04/02 01:59:25]

Done.

+}
+
+// GaiaOAuthClient::Delegate OAuth2 error when fetching refresh token request.
+void EnrollmentHandlerChromeOS::OnOAuthError() {
+  ReportResult(EnrollmentStatus::ForStatus(
+      EnrollmentStatus::STATUS_ROBOT_REFRESH_FETCH_FAILED));
+}
+
+// GaiaOAuthClient::Delegate network error when fetching refresh token.
+void EnrollmentHandlerChromeOS::OnNetworkError(int response_code) {
+  LOG(ERROR) << "Network error while fetching API refresh token: "
+             << response_code;
+  ReportResult(EnrollmentStatus::ForStatus(
+      EnrollmentStatus::STATUS_ROBOT_REFRESH_FETCH_FAILED));
+}
+
 void EnrollmentHandlerChromeOS::OnClientError(CloudPolicyClient* client) {
   DCHECK_EQ(client_.get(), client);
 
-  if (enrollment_step_ < STEP_POLICY_FETCH)
+  if (enrollment_step_ == STEP_ROBOT_AUTH_FETCH)
+    ReportResult(EnrollmentStatus::ForRobotAuthError(client_->status()));
+  else if (enrollment_step_ < STEP_POLICY_FETCH)
     ReportResult(EnrollmentStatus::ForRegistrationError(client_->status()));
   else
     ReportResult(EnrollmentStatus::ForFetchError(client_->status()));
@@ -140,6 +208,9 @@ void EnrollmentHandlerChromeOS::OnStoreLoaded(CloudPolicyStore* store) {
   DCHECK_EQ(store_, store);
 
   if (enrollment_step_ == STEP_LOADING_STORE) {
+    // If the |store_| wasn't initialized when StartEnrollment() was
+    // called, then AttemptRegistration() bails silently.  This gets
+    // registration rolling again after the store finishes loading.
     AttemptRegistration();
   } else if (enrollment_step_ == STEP_STORE_POLICY) {
     ReportResult(EnrollmentStatus::ForStatus(EnrollmentStatus::STATUS_SUCCESS));