From a11eadfb46f6d9656938b66a5439f8d63bac2a4c Mon Sep 17 00:00:00 2001 From: dzhioev Date: Fri, 24 Jul 2015 22:23:58 -0700 Subject: [PATCH] Hotrod shark controller ported to the new GAIA flow. Before this change shark controller fetched two OA2RT using login WebUI context (cookies), and then for each RT fetched an OA2AT. Now it fetches a single OA2RT using the auth code, and fetches two OA2AT using the single RT. TEST=manually (shark<->remora pairing works, regular enrollment works) BUG=471744 Review URL: https://codereview.chromium.org/1235913003 Cr-Commit-Position: refs/heads/master@{#340402} --- .../chromeos/login/enrollment/enrollment_screen.cc | 16 +-- .../enrollment/enterprise_enrollment_helper.h | 17 +-- .../enterprise_enrollment_helper_impl.cc | 135 +++++++-------------- .../enrollment/enterprise_enrollment_helper_impl.h | 27 ++--- .../chromeos/login/saml/saml_browsertest.cc | 6 +- chrome/browser/chromeos/login/startup_utils.cc | 11 +- 6 files changed, 65 insertions(+), 147 deletions(-) diff --git a/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc b/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc index f56114458ff1..d5e7b50f8a09 100644 --- a/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc +++ b/chrome/browser/chromeos/login/enrollment/enrollment_screen.cc @@ -154,6 +154,7 @@ void EnrollmentScreen::EnrollHostRequested(const std::string& auth_token) { void EnrollmentScreen::OnLoginDone(const std::string& user, const std::string& auth_code) { + LOG_IF(ERROR, auth_code.empty()) << "Auth code is empty."; elapsed_timer_.reset(new base::ElapsedTimer()); enrolling_user_domain_ = gaia::ExtractDomainName(user); @@ -162,19 +163,8 @@ void EnrollmentScreen::OnLoginDone(const std::string& user, actor_->ShowEnrollmentSpinnerScreen(); CreateEnrollmentHelper(); - if (auth_code.empty()) { - enrollment_helper_->EnrollUsingProfile( - ProfileHelper::GetSigninProfile(), - shark_controller_ != NULL /* fetch_additional_token */); - } else { - if (shark_controller_) { - // TODO(dzhioev): add shark controller support. http://crbug.com/471744 - NOTIMPLEMENTED(); - OnOtherError(EnterpriseEnrollmentHelper::OTHER_ERROR_FATAL); - return; - } - enrollment_helper_->EnrollUsingAuthCode(auth_code); - } + enrollment_helper_->EnrollUsingAuthCode( + auth_code, shark_controller_ != NULL /* fetch_additional_token */); } void EnrollmentScreen::OnRetry() { diff --git a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper.h b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper.h index cdeecd822f15..184f5a0379c6 100644 --- a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper.h +++ b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper.h @@ -74,24 +74,15 @@ class EnterpriseEnrollmentHelper { virtual ~EnterpriseEnrollmentHelper(); - // Starts enterprise enrollment using |profile|. First tries to fetch an - // authentication token using the |profile|, then tries to enroll the device - // with the received token. - // If |fetch_additional_token| is true, the helper fetches an additional token - // and passes it to the |status_consumer| on successfull enrollment. - // If enrollment fails, you should clear authentication data in |profile| by - // calling ClearAuth before destroying |this|. - // EnrollUsingProfile can be called only once during this object's lifetime, - // and only if neither of EnrollUsing* was called before. - virtual void EnrollUsingProfile(Profile* profile, - bool fetch_additional_token) = 0; - // Starts enterprise enrollment using |auth_code|. First tries to exchange the // auth code to authentication token, then tries to enroll the device with the // received token. + // If |fetch_additional_token| is true, the helper fetches an additional token + // and passes it to the |status_consumer| on successfull enrollment. // EnrollUsingAuthCode can be called only once during this object's lifetime, // and only if neither of EnrollUsing* methods was called before. - virtual void EnrollUsingAuthCode(const std::string& auth_code) = 0; + virtual void EnrollUsingAuthCode(const std::string& auth_code, + bool fetch_additional_token) = 0; // Starts enterprise enrollment using |token|. // EnrollUsingToken can be called only once during this object's lifetime, and diff --git a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc index 59f25124f2d8..8c898f19296a 100644 --- a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc +++ b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.cc @@ -10,7 +10,6 @@ #include "base/message_loop/message_loop.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process_platform_part.h" -#include "chrome/browser/browsing_data/browsing_data_helper.h" #include "chrome/browser/chromeos/login/enrollment/enrollment_uma.h" #include "chrome/browser/chromeos/login/startup_utils.h" #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" @@ -18,7 +17,6 @@ #include "chrome/browser/chromeos/policy/enrollment_status_chromeos.h" #include "chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h" #include "chrome/browser/chromeos/profiles/profile_helper.h" -#include "chrome/browser/profiles/profile.h" #include "components/policy/core/common/cloud/cloud_policy_constants.h" #include "google_apis/gaia/gaia_auth_consumer.h" #include "google_apis/gaia/gaia_auth_fetcher.h" @@ -71,52 +69,29 @@ EnterpriseEnrollmentHelperImpl::EnterpriseEnrollmentHelperImpl( : EnterpriseEnrollmentHelper(status_consumer), enrollment_config_(enrollment_config), enrolling_user_domain_(enrolling_user_domain), - profile_(NULL), - fetch_additional_token_(false), started_(false), - oauth_fetchers_finished_(0), - last_auth_error_(GoogleServiceAuthError::AuthErrorNone()), finished_(false), success_(false), auth_data_cleared_(false), - browsing_data_remover_(NULL), weak_ptr_factory_(this) { } EnterpriseEnrollmentHelperImpl::~EnterpriseEnrollmentHelperImpl() { DCHECK(g_browser_process->IsShuttingDown() || !started_ || - (finished_ && (success_ || !profile_ || auth_data_cleared_))); - if (browsing_data_remover_) - browsing_data_remover_->RemoveObserver(this); -} - -void EnterpriseEnrollmentHelperImpl::EnrollUsingProfile( - Profile* profile, - bool fetch_additional_token) { - DCHECK(!started_); - started_ = true; - profile_ = profile; - fetch_additional_token_ = fetch_additional_token; - oauth_fetchers_.resize(fetch_additional_token_ ? 2 : 1); - for (size_t i = 0; i < oauth_fetchers_.size(); ++i) { - oauth_fetchers_[i] = new policy::PolicyOAuth2TokenFetcher(); - oauth_fetchers_[i]->StartWithSigninContext( - profile_->GetRequestContext(), - g_browser_process->system_request_context(), - base::Bind(&EnterpriseEnrollmentHelperImpl::OnTokenFetched, - weak_ptr_factory_.GetWeakPtr(), i)); - } + (finished_ && (success_ || auth_data_cleared_))); } void EnterpriseEnrollmentHelperImpl::EnrollUsingAuthCode( - const std::string& auth_code) { + const std::string& auth_code, + bool fetch_additional_token) { DCHECK(!started_); started_ = true; - oauth_fetchers_.push_back(new policy::PolicyOAuth2TokenFetcher()); - oauth_fetchers_[0]->StartWithAuthCode( + oauth_fetcher_.reset(new policy::PolicyOAuth2TokenFetcher()); + oauth_fetcher_->StartWithAuthCode( auth_code, g_browser_process->system_request_context(), base::Bind(&EnterpriseEnrollmentHelperImpl::OnTokenFetched, - weak_ptr_factory_.GetWeakPtr(), 0)); + weak_ptr_factory_.GetWeakPtr(), + fetch_additional_token /* is_additional_token */)); } void EnterpriseEnrollmentHelperImpl::EnrollUsingToken( @@ -127,34 +102,27 @@ void EnterpriseEnrollmentHelperImpl::EnrollUsingToken( } void EnterpriseEnrollmentHelperImpl::ClearAuth(const base::Closure& callback) { - for (size_t i = 0; i < oauth_fetchers_.size(); ++i) { - // Do not revoke the additional token if enrollment has finished - // successfully. - if (i == 1 && success_) - continue; - - if (!oauth_fetchers_[i]->oauth2_access_token().empty()) - (new TokenRevoker())->Start(oauth_fetchers_[i]->oauth2_access_token()); - - if (!oauth_fetchers_[i]->oauth2_refresh_token().empty()) - (new TokenRevoker())->Start(oauth_fetchers_[i]->oauth2_refresh_token()); - } - oauth_fetchers_.clear(); - - if (!profile_) { - chromeos::ProfileHelper::Get()->ClearSigninProfile(callback); - return; + // Do not revoke the additional token if enrollment has finished + // successfully. + if (!success_ && additional_token_.length()) + (new TokenRevoker())->Start(additional_token_); + + if (oauth_fetcher_) { + if (!oauth_fetcher_->oauth2_access_token().empty()) + (new TokenRevoker())->Start(oauth_fetcher_->oauth2_access_token()); + + if (!oauth_fetcher_->oauth2_refresh_token().empty()) + (new TokenRevoker())->Start(oauth_fetcher_->oauth2_refresh_token()); + + oauth_fetcher_.reset(); + } else if (oauth_token_.length()) { + // EnrollUsingToken was called. + (new TokenRevoker())->Start(oauth_token_); } - auth_clear_callbacks_.push_back(callback); - if (browsing_data_remover_) - return; - - browsing_data_remover_ = - BrowsingDataRemover::CreateForUnboundedRange(profile_); - browsing_data_remover_->AddObserver(this); - browsing_data_remover_->Remove(BrowsingDataRemover::REMOVE_SITE_DATA, - BrowsingDataHelper::UNPROTECTED_WEB); + chromeos::ProfileHelper::Get()->ClearSigninProfile( + base::Bind(&EnterpriseEnrollmentHelperImpl::OnSigninProfileCleared, + weak_ptr_factory_.GetWeakPtr(), callback)); } void EnterpriseEnrollmentHelperImpl::DoEnrollUsingToken( @@ -220,30 +188,29 @@ void EnterpriseEnrollmentHelperImpl::UpdateDeviceAttributes( } void EnterpriseEnrollmentHelperImpl::OnTokenFetched( - size_t fetcher_index, + bool is_additional_token, const std::string& token, const GoogleServiceAuthError& error) { - CHECK_LT(fetcher_index, oauth_fetchers_.size()); - - if (error.state() != GoogleServiceAuthError::NONE) - last_auth_error_ = error; - - ++oauth_fetchers_finished_; - if (oauth_fetchers_finished_ != oauth_fetchers_.size()) - return; - - if (last_auth_error_.state() != GoogleServiceAuthError::NONE) { - ReportAuthStatus(last_auth_error_); + if (error.state() != GoogleServiceAuthError::NONE) { + ReportAuthStatus(error); finished_ = true; - status_consumer()->OnAuthError(last_auth_error_); + status_consumer()->OnAuthError(error); return; } - if (oauth_fetchers_.size() == 2) - additional_token_ = oauth_fetchers_[1]->oauth2_access_token(); + if (!is_additional_token) { + DoEnrollUsingToken(token); + return; + } - oauth_token_ = oauth_fetchers_[0]->oauth2_access_token(); - DoEnrollUsingToken(oauth_token_); + additional_token_ = token; + std::string refresh_token = oauth_fetcher_->oauth2_refresh_token(); + oauth_fetcher_.reset(new policy::PolicyOAuth2TokenFetcher()); + oauth_fetcher_->StartWithRefreshToken( + refresh_token, g_browser_process->system_request_context(), + base::Bind(&EnterpriseEnrollmentHelperImpl::OnTokenFetched, + weak_ptr_factory_.GetWeakPtr(), + false /* is_additional_token */)); } void EnterpriseEnrollmentHelperImpl::OnEnrollmentFinished( @@ -252,7 +219,6 @@ void EnterpriseEnrollmentHelperImpl::OnEnrollmentFinished( finished_ = true; if (status.status() == policy::EnrollmentStatus::STATUS_SUCCESS) { success_ = true; - DCHECK(!fetch_additional_token_ || !additional_token_.empty()); StartupUtils::MarkOobeCompleted(); status_consumer()->OnDeviceEnrolled(additional_token_); } else { @@ -436,21 +402,10 @@ void EnterpriseEnrollmentHelperImpl::UMA(policy::MetricEnrollment sample) { EnrollmentUMA(sample, enrollment_config_.mode); } -void EnterpriseEnrollmentHelperImpl::OnBrowsingDataRemoverDone() { - browsing_data_remover_->RemoveObserver(this); - - // BrowsingDataRemover deletes itself. - browsing_data_remover_ = nullptr; - +void EnterpriseEnrollmentHelperImpl::OnSigninProfileCleared( + const base::Closure& callback) { auth_data_cleared_ = true; - - std::vector callbacks_to_run; - callbacks_to_run.swap(auth_clear_callbacks_); - for (std::vector::iterator callback(callbacks_to_run.begin()); - callback != callbacks_to_run.end(); - ++callback) { - callback->Run(); - } + callback.Run(); } } // namespace chromeos diff --git a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.h b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.h index e60852a07924..08e5cd147325 100644 --- a/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.h +++ b/chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper_impl.h @@ -13,7 +13,6 @@ #include "base/macros.h" #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" -#include "chrome/browser/browsing_data/browsing_data_remover.h" #include "chrome/browser/chromeos/login/enrollment/enterprise_enrollment_helper.h" #include "chrome/browser/chromeos/policy/enrollment_config.h" #include "components/policy/core/common/cloud/enterprise_metrics.h" @@ -27,8 +26,7 @@ class PolicyOAuth2TokenFetcher; namespace chromeos { -class EnterpriseEnrollmentHelperImpl : public EnterpriseEnrollmentHelper, - public BrowsingDataRemover::Observer { +class EnterpriseEnrollmentHelperImpl : public EnterpriseEnrollmentHelper { public: EnterpriseEnrollmentHelperImpl( EnrollmentStatusConsumer* status_consumer, @@ -37,9 +35,8 @@ class EnterpriseEnrollmentHelperImpl : public EnterpriseEnrollmentHelper, ~EnterpriseEnrollmentHelperImpl() override; // Overridden from EnterpriseEnrollmentHelper: - void EnrollUsingProfile(Profile* profile, - bool fetch_additional_token) override; - void EnrollUsingAuthCode(const std::string& auth_code) override; + void EnrollUsingAuthCode(const std::string& auth_code, + bool fetch_additional_token) override; void EnrollUsingToken(const std::string& token) override; void ClearAuth(const base::Closure& callback) override; void GetDeviceAttributeUpdatePermission() override; @@ -50,7 +47,7 @@ class EnterpriseEnrollmentHelperImpl : public EnterpriseEnrollmentHelper, void DoEnrollUsingToken(const std::string& token); // Handles completion of the OAuth2 token fetch attempt. - void OnTokenFetched(size_t fetcher_index, + void OnTokenFetched(bool is_additional_token, const std::string& token, const GoogleServiceAuthError& error); @@ -70,30 +67,22 @@ class EnterpriseEnrollmentHelperImpl : public EnterpriseEnrollmentHelper, // histogram, depending on |enrollment_mode_|. void UMA(policy::MetricEnrollment sample); - // Overridden from BrowsingDataRemover::Observer: - void OnBrowsingDataRemoverDone() override; + // Called by ProfileHelper when a signin profile clearance has finished. + // |callback| is a callback, that was passed to ClearAuth() before. + void OnSigninProfileCleared(const base::Closure& callback); const policy::EnrollmentConfig enrollment_config_; const std::string enrolling_user_domain_; - Profile* profile_; bool fetch_additional_token_; bool started_; - size_t oauth_fetchers_finished_; - GoogleServiceAuthError last_auth_error_; std::string additional_token_; bool finished_; bool success_; bool auth_data_cleared_; std::string oauth_token_; - // The browsing data remover instance currently active, if any. - BrowsingDataRemover* browsing_data_remover_; - - // The callbacks to invoke after browsing data has been cleared. - std::vector auth_clear_callbacks_; - - ScopedVector oauth_fetchers_; + scoped_ptr oauth_fetcher_; base::WeakPtrFactory weak_ptr_factory_; diff --git a/chrome/browser/chromeos/login/saml/saml_browsertest.cc b/chrome/browser/chromeos/login/saml/saml_browsertest.cc index f5373b00a2cc..42f490687916 100644 --- a/chrome/browser/chromeos/login/saml/saml_browsertest.cc +++ b/chrome/browser/chromeos/login/saml/saml_browsertest.cc @@ -905,10 +905,12 @@ IN_PROC_BROWSER_TEST_P(SAMLEnrollmentTest, WithCredentialsPassingAPI) { WaitForEnrollmentSuccess(); } -// TODO(xiyuan): Update once webview flow is implemented. +// TODO(dzhioev): fix the tests for webview case. http://crbug.com/513955 +/* INSTANTIATE_TEST_CASE_P(SamlSuite, SAMLEnrollmentTest, - testing::Values(false)); + testing::Values(true)); +*/ class SAMLPolicyTest : public SamlTest { public: diff --git a/chrome/browser/chromeos/login/startup_utils.cc b/chrome/browser/chromeos/login/startup_utils.cc index 61d6233eae12..d35e06d0a978 100644 --- a/chrome/browser/chromeos/login/startup_utils.cc +++ b/chrome/browser/chromeos/login/startup_utils.cc @@ -184,20 +184,11 @@ std::string StartupUtils::GetInitialLocale() { // static bool StartupUtils::IsWebviewSigninEnabled() { - const policy::DeviceCloudPolicyManagerChromeOS* policy_manager = - g_browser_process->platform_part() - ->browser_policy_connector_chromeos() - ->GetDeviceCloudPolicyManager(); - - const bool is_shark = - policy_manager ? policy_manager->IsSharkRequisition() : false; - const bool is_webview_disabled_pref = g_browser_process->local_state()->GetBoolean( prefs::kWebviewSigninDisabled); - // TODO(achuith): Remove is_shark when crbug.com/471744 is resolved. - return !is_shark && !IsWebViewDisabledCmdLine() && !is_webview_disabled_pref; + return !IsWebViewDisabledCmdLine() && !is_webview_disabled_pref; } // static -- 2.11.4.GIT