Migrate opt-in auth flow to re-auth flow.

chrome/browser/chromeos/arc/arc_auth_service.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 "chrome/browser/chromeos/arc/arc_auth_service.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 325 matching lines @@
   if (!reenable_arc_)
     return;
 
   // Restart ARC anyway. Let the enterprise reporting instance decide whether
   // the ARC user data wipe is still required or not.
   reenable_arc_ = false;
   VLOG(1) << "Reenable ARC";
   EnableArc();
 }
 
-std::string ArcAuthService::GetAndResetAuthCode() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  std::string auth_code;
-  auth_code_.swap(auth_code);
-  return auth_code;
-}
-
 void ArcAuthService::GetAuthCodeDeprecated0(
     const GetAuthCodeDeprecated0Callback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(!IsOptInVerificationDisabled());
-  // For robot account we must use RequestAccountInfo because it allows to
-  // specify account type.
-  DCHECK(!IsArcKioskMode());
-  callback.Run(GetAndResetAuthCode());
+  NOTREACHED() << "GetAuthCodeDeprecated0() should no longer be callable";
+  callback.Run(std::string());
 }
 
 void ArcAuthService::GetAuthCodeDeprecated(
     const GetAuthCodeDeprecatedCallback& callback) {
   // For robot account we must use RequestAccountInfo because it allows
   // to specify account type.
   DCHECK(!IsArcKioskMode());
   RequestAccountInfoInternal(
       base::MakeUnique<ArcAuthService::AccountInfoNotifier>(callback));
 }
@@ skipping 19 matching lines @@
   instance->OnAccountInfoReady(std::move(account_info));
 }
 
 void ArcAuthService::RequestAccountInfoInternal(
     std::unique_ptr<ArcAuthService::AccountInfoNotifier>
         account_info_notifier) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // No other auth code-related operation may be in progress.
   DCHECK(!account_info_notifier_);
 
-  const std::string auth_code = GetAndResetAuthCode();
-  const bool is_enforced = !IsOptInVerificationDisabled();
-  if (!auth_code.empty() || !is_enforced) {
-    account_info_notifier->Notify(is_enforced, auth_code, GetAccountType(),
+  if (IsOptInVerificationDisabled()) {
+    account_info_notifier->Notify(false /* = is_enforced */, std::string(),
+                                  GetAccountType(),
                                   policy_util::IsAccountManaged(profile_));
     return;
   }
 
   account_info_notifier_ = std::move(account_info_notifier);
 
   if (IsArcKioskMode()) {
     arc_robot_auth_.reset(new ArcRobotAuth());
     arc_robot_auth_->FetchRobotAuthCode(
         base::Bind(&ArcAuthService::OnRobotAuthCodeFetched,
@@ skipping 192 matching lines @@
   PrefServiceSyncableFromProfile(profile_)->AddSyncedPrefObserver(
       prefs::kArcEnabled, this);
 
   context_.reset(new ArcAuthContext(this, profile_));
 
   // In case UI is disabled we assume that ARC is opted-in. For ARC Kiosk we
   // skip ToS because it is very likely that near the device there will be
   // no one who is eligible to accept them. We skip if Android management check
   // because there are no managed human users for Kiosk exist.
   if (IsOptInVerificationDisabled() || IsArcKioskMode()) {
-    auth_code_.clear();
     StartArc();
     return;
   }
 
   if (!g_disable_ui_for_testing ||
       g_enable_check_android_management_for_testing) {
     ArcAndroidManagementChecker::StartClient();
   }
   pref_change_registrar_.Init(profile_->GetPrefs());
   pref_change_registrar_.Add(
@@ skipping 111 matching lines @@
 
   if (!arc_enabled) {
     StopArc();
     RemoveArcData();
     return;
   }
 
   if (state_ == State::ACTIVE)
     return;
   CloseUI();
-  auth_code_.clear();
 
   if (!profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn)) {
     if (profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)) {
-      // Need pre-fetch auth code and start Arc.
-      SetState(State::FETCHING_CODE);
-      PrepareContextForAuthCodeRequest();
+      StartArc();
     } else {
       // Need pre-fetch auth code and show OptIn UI if needed.
       StartUI();
     }
   } else {
     // Ready to start Arc, but check Android management in parallel.
     StartArc();
     // Note: Because the callback may be called in synchronous way (i.e. called
     // on the same stack), StartCheck() needs to be called *after* StartArc().
     // Otherwise, DisableArc() which may be called in
@@ skipping 68 matching lines @@
   reenable_arc_ = true;
   StopArc();
 }
 
 void ArcAuthService::StartArc() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   arc_bridge_service()->RequestStart();
   SetState(State::ACTIVE);
 }
 
-void ArcAuthService::SetAuthCodeAndStartArc(const std::string& auth_code) {
+void ArcAuthService::OnAuthCodeObtained(const std::string& auth_code) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!auth_code.empty());
 
-  if (IsAuthCodeRequest()) {
-    account_info_notifier_->Notify(!IsOptInVerificationDisabled(), auth_code,
-                                   GetAccountType(),
-                                   policy_util::IsAccountManaged(profile_));
-    account_info_notifier_.reset();
-    return;
-  }
-
-  if (state_ != State::SHOWING_TERMS_OF_SERVICE &&
-      state_ != State::CHECKING_ANDROID_MANAGEMENT &&
-      state_ != State::FETCHING_CODE) {
-    ShutdownBridgeAndCloseUI();
-    return;
-  }
-
-  sign_in_time_ = base::Time::Now();
-  VLOG(1) << "Starting ARC for first sign in.";
-
-  SetUIPage(ArcSupportHost::UIPage::START_PROGRESS, base::string16());
-  ShutdownBridge();
-  auth_code_ = auth_code;
-  arc_sign_in_timer_.Start(FROM_HERE, kArcSignInTimeout,
-                           base::Bind(&ArcAuthService::OnArcSignInTimeout,
-                                      weak_ptr_factory_.GetWeakPtr()));
-  StartArc();
+  account_info_notifier_->Notify(!IsOptInVerificationDisabled(), auth_code,
+                                 GetAccountType(),
+                                 policy_util::IsAccountManaged(profile_));
+  account_info_notifier_.reset();
 }
 
 void ArcAuthService::OnArcSignInTimeout() {
   LOG(ERROR) << "Timed out waiting for first sign in.";
   OnSignInFailedInternal(ProvisioningResult::OVERALL_SIGN_IN_TIMEOUT);
 }
 
 void ArcAuthService::CancelAuthCode() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (state_ == State::NOT_INITIALIZED) {
     NOTREACHED();
     return;
   }
 
   // In case |state_| is ACTIVE, |ui_page_| can be START_PROGRESS (which means
   // normal Arc booting) or  ERROR or ERROR_WITH_FEEDBACK (in case Arc can not
   // be started). If Arc is booting normally dont't stop it on progress close.
-  if ((state_ != State::FETCHING_CODE &&
-       state_ != State::SHOWING_TERMS_OF_SERVICE &&
+  if ((state_ != State::SHOWING_TERMS_OF_SERVICE &&
        state_ != State::CHECKING_ANDROID_MANAGEMENT) &&
       ui_page_ != ArcSupportHost::UIPage::ERROR &&
       ui_page_ != ArcSupportHost::UIPage::ERROR_WITH_FEEDBACK) {
     return;
   }
 
   // Update UMA with user cancel only if error is not currently shown.
   if (ui_page_ != ArcSupportHost::UIPage::NO_PAGE &&
       ui_page_ != ArcSupportHost::UIPage::ERROR &&
       ui_page_ != ArcSupportHost::UIPage::ERROR_WITH_FEEDBACK) {
@@ skipping 59 matching lines @@
         ArcSupportHost::UIPage::ERROR,
         l10n_util::GetStringUTF16(IDS_ARC_SIGN_IN_SERVICE_UNAVAILABLE_ERROR));
     return;
   }
 
   SetState(State::SHOWING_TERMS_OF_SERVICE);
   ShowUI(ArcSupportHost::UIPage::TERMS, base::string16());
 }
 
 void ArcAuthService::OnPrepareContextFailed() {
-  DCHECK_EQ(state_, State::FETCHING_CODE);
-
   ShutdownBridgeAndShowUI(
       ArcSupportHost::UIPage::ERROR,
       l10n_util::GetStringUTF16(IDS_ARC_SERVER_COMMUNICATION_ERROR));
   UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
 }
 
 void ArcAuthService::OnAuthCodeSuccess(const std::string& auth_code) {
-  SetAuthCodeAndStartArc(auth_code);
+  OnAuthCodeObtained(auth_code);
 }
 
 void ArcAuthService::OnAuthCodeFailed() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK_EQ(state_, State::FETCHING_CODE);
   ShutdownBridgeAndShowUI(
       ArcSupportHost::UIPage::ERROR,
       l10n_util::GetStringUTF16(IDS_ARC_SERVER_COMMUNICATION_ERROR));
   UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
 }
 
 void ArcAuthService::StartArcAndroidManagementCheck() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(arc_bridge_service()->stopped());
   SetState(State::CHECKING_ANDROID_MANAGEMENT);
 
   android_management_checker_.reset(new ArcAndroidManagementChecker(
       profile_, context_->token_service(), context_->account_id(),
       false /* retry_on_error */));
   android_management_checker_->StartCheck(
       base::Bind(&ArcAuthService::OnAndroidManagementChecked,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ArcAuthService::OnAndroidManagementChecked(
     policy::AndroidManagementClient::Result result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK_EQ(state_, State::CHECKING_ANDROID_MANAGEMENT);
 
   switch (result) {
     case policy::AndroidManagementClient::Result::UNMANAGED:
-      if (IsOptInVerificationDisabled()) {
-        StartArc();
-      } else {
-        // TODO(hidehiko): Merge this prefetching into re-auth case.
-        SetState(State::FETCHING_CODE);
-        PrepareContextForAuthCodeRequest();
-      }
+      VLOG(1) << "Starting ARC for first sign in.";
+      sign_in_time_ = base::Time::Now();
+      arc_sign_in_timer_.Start(FROM_HERE, kArcSignInTimeout,
+                               base::Bind(&ArcAuthService::OnArcSignInTimeout,
+                                          weak_ptr_factory_.GetWeakPtr()));
+      StartArc();
       break;
     case policy::AndroidManagementClient::Result::MANAGED:
       ShutdownBridgeAndShowUI(
           ArcSupportHost::UIPage::ERROR,
           l10n_util::GetStringUTF16(IDS_ARC_ANDROID_MANAGEMENT_REQUIRED_ERROR));
       UpdateOptInCancelUMA(OptInCancelReason::ANDROID_MANAGEMENT_REQUIRED);
       break;
     case policy::AndroidManagementClient::Result::ERROR:
       ShutdownBridgeAndShowUI(
           ArcSupportHost::UIPage::ERROR,
@@ skipping 52 matching lines @@
   // This is ARC support's UI event callback, so this is called only when
   // the UI is visible. The condition to open the UI is
   // !g_disable_ui_for_testing && !IsOptInVerificationDisabled() (see ShowUI())
   // and in the case, preference_handler_ should be always created (see
   // OnPrimaryUserProfilePrepared()),
   // TODO(hidehiko): Simplify the logic with the code restructuring.
   DCHECK(preference_handler_);
   preference_handler_->EnableMetrics(is_metrics_enabled);
   preference_handler_->EnableBackupRestore(is_backup_and_restore_enabled);
   preference_handler_->EnableLocationService(is_location_service_enabled);
+  SetUIPage(ArcSupportHost::UIPage::START_PROGRESS, base::string16());
   StartArcAndroidManagementCheck();
 }
 
 void ArcAuthService::OnAuthSucceeded(const std::string& auth_code) {
-  SetAuthCodeAndStartArc(auth_code);
+  OnAuthCodeObtained(auth_code);
 }
 
 void ArcAuthService::OnRetryClicked() {
   UpdateOptInActionUMA(OptInActionType::RETRY);
-  // ERROR_WITH_FEEDBACK is set in OnSignInFailed(). In the case, we postpone
-  // to stop the ARC to obtain the internal state.
-  // Here, on retry, stop it.
-  if (ui_page_ == ArcSupportHost::UIPage::ERROR_WITH_FEEDBACK)
+
+  // TODO(hidehiko): Simplify the retry logic.
+  if (!profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)) {
+    // If the user has not yet agreed on Terms of Service, then show it.
+    ShowUI(ArcSupportHost::UIPage::TERMS, base::string16());
+  } else if (ui_page_ == ArcSupportHost::UIPage::ERROR_WITH_FEEDBACK) {
+    // ERROR_WITH_FEEDBACK is set in OnSignInFailed(). In the case, stopping
+    // ARC was postponed to contain its internal state into the report.
+    // Here, on retry, stop it, then restart.
+    SetUIPage(ArcSupportHost::UIPage::START_PROGRESS, base::string16());

[Luis Héctor Chávez, 2016/11/14 17:21:36]

DCHECK_EQ(State::ACTIVE, state_); ?

[hidehiko, 2016/11/15 11:26:47]

Done.

     ShutdownBridge();

[hidehiko, 2016/11/14 16:57:00]

FYI: this implementation seems to hit some hidden race around session manager.
ArcStopInstance DBus signal looks sent to Chrome twice asynchronously so that
restarting is not yet working. I'll investigate it more tomorrow on my local TZ.

-
-  switch (state_) {
-    case State::NOT_INITIALIZED:
-      NOTREACHED();
-      break;
-    case State::STOPPED:
-    case State::SHOWING_TERMS_OF_SERVICE:
-      ShowUI(ArcSupportHost::UIPage::TERMS, base::string16());
-      break;
-    case State::CHECKING_ANDROID_MANAGEMENT:
-      StartArcAndroidManagementCheck();
-      break;
-    case State::FETCHING_CODE:
-    case State::ACTIVE:
-      PrepareContextForAuthCodeRequest();
-      break;
+    reenable_arc_ = true;
+  } else if (state_ == State::ACTIVE) {
+    // This happens when ARC support Chrome app reports an error on "Sign in"
+    // page.
+    // TODO(hidehiko): Currently, due to the existing code structure, we need
+    // to call PrepareContextForAuthCodeRequest() always. However, to fetch
+    // an authtoken via LSO page, it is not necessary to call PrepareContext().
+    // Instead, it is possible to show LSO page, immediately.
+    SetUIPage(ArcSupportHost::UIPage::START_PROGRESS, base::string16());
+    PrepareContextForAuthCodeRequest();
+  } else {
+    // Otherwise, we restart the ARC. Note: this is the first boot case.

[Luis Héctor Chávez, 2016/11/14 17:21:36]

nit: s/the ARC/ARC/g

[hidehiko, 2016/11/15 11:26:47]

Done.

+    // For second or later boot, either ERROR_WITH_FEEDBACK case or ACTIVE
+    // case must hit.
+    SetUIPage(ArcSupportHost::UIPage::START_PROGRESS, base::string16());
+    StartArcAndroidManagementCheck();
   }
 }
 
 void ArcAuthService::OnSendFeedbackClicked() {
   chrome::OpenFeedbackDialog(nullptr);
 }
 
 void ArcAuthService::OnMetricsModeChanged(bool enabled, bool managed) {
   if (!support_host_)
     return;
@@ skipping 15 matching lines @@
 std::ostream& operator<<(std::ostream& os, const ArcAuthService::State& state) {
   switch (state) {
     case ArcAuthService::State::NOT_INITIALIZED:
       return os << "NOT_INITIALIZED";
     case ArcAuthService::State::STOPPED:
       return os << "STOPPED";
     case ArcAuthService::State::SHOWING_TERMS_OF_SERVICE:
       return os << "SHOWING_TERMS_OF_SERVICE";
     case ArcAuthService::State::CHECKING_ANDROID_MANAGEMENT:
       return os << "CHECKING_ANDROID_MANAGEMENT";
-    case ArcAuthService::State::FETCHING_CODE:
-      return os << "FETCHING_CODE";
     case ArcAuthService::State::ACTIVE:
       return os << "ACTIVE";
     default:
       NOTREACHED();
       return os;
   }
 }
 
 }  // namespace arc