From edfd2506bf1b5b270168fbab8b114e879abbb828 Mon Sep 17 00:00:00 2001 From: dzhioev Date: Thu, 6 Aug 2015 15:37:18 -0700 Subject: [PATCH] [cleanup] Removed online password check on Chrome OS login. OnlineAttempt and OnlineAttemptHost will be removed in a subsequent CL. BUG=470893 Review URL: https://codereview.chromium.org/1277693002 Cr-Commit-Position: refs/heads/master@{#342218} --- chrome/app/chromeos_strings.grdp | 3 -- .../chromeos/app_mode/kiosk_profile_loader.cc | 5 --- .../chromeos/app_mode/kiosk_profile_loader.h | 1 - .../chromeos/login/auth/chrome_login_performer.cc | 8 ++--- .../chromeos/login/auth/chrome_login_performer.h | 1 - .../chromeos/login/existing_user_controller.cc | 36 ---------------------- .../chromeos/login/existing_user_controller.h | 10 ------ .../login/existing_user_controller_browsertest.cc | 10 +++--- .../browser/chromeos/login/screens/error_screen.cc | 4 --- .../browser/chromeos/login/screens/error_screen.h | 1 - chrome/browser/chromeos/login/ui/login_display.h | 3 -- .../browser/chromeos/login/ui/mock_login_display.h | 1 - .../chromeos/login/ui/webui_login_display.cc | 5 --- .../chromeos/login/ui/webui_login_display.h | 1 - .../chromeos/login/screen_gaia_signin.html | 1 - .../resources/chromeos/login/screen_gaia_signin.js | 12 +------- .../ui/webui/chromeos/login/gaia_screen_handler.cc | 10 ------ .../ui/webui/chromeos/login/gaia_screen_handler.h | 9 ------ .../webui/chromeos/login/signin_screen_handler.cc | 9 ------ .../webui/chromeos/login/signin_screen_handler.h | 2 -- chromeos/login/auth/login_performer.cc | 24 ++------------- chromeos/login/auth/login_performer.h | 17 ++-------- 22 files changed, 11 insertions(+), 162 deletions(-) diff --git a/chrome/app/chromeos_strings.grdp b/chrome/app/chromeos_strings.grdp index 827e5ec7f5f8..394907bb07b4 100644 --- a/chrome/app/chromeos_strings.grdp +++ b/chrome/app/chromeos_strings.grdp @@ -1337,9 +1337,6 @@ Press any key to continue exploring. Sign in, press the tab key to interact with input elements - - You have recently changed the password. Please sign in with the new one. - Please wait... diff --git a/chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc b/chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc index 2004ba389756..76e7b1e6655c 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc +++ b/chrome/browser/chromeos/app_mode/kiosk_profile_loader.cc @@ -178,11 +178,6 @@ void KioskProfileLoader::PolicyLoadFailed() { ReportLaunchResult(KioskAppLaunchError::POLICY_LOAD_FAILED); } -void KioskProfileLoader::OnOnlineChecked( - const std::string& email, bool success) { - NOTREACHED(); -} - void KioskProfileLoader::OnProfilePrepared(Profile* profile, bool browser_launched) { // This object could be deleted any time after successfully reporting diff --git a/chrome/browser/chromeos/app_mode/kiosk_profile_loader.h b/chrome/browser/chromeos/app_mode/kiosk_profile_loader.h index 42796e398b3d..3727ce3bbc55 100644 --- a/chrome/browser/chromeos/app_mode/kiosk_profile_loader.h +++ b/chrome/browser/chromeos/app_mode/kiosk_profile_loader.h @@ -53,7 +53,6 @@ class KioskProfileLoader : public LoginPerformer::Delegate, void OnAuthFailure(const AuthFailure& error) override; void WhiteListCheckFailed(const std::string& email) override; void PolicyLoadFailed() override; - void OnOnlineChecked(const std::string& email, bool success) override; // UserSessionManagerDelegate implementation: void OnProfilePrepared(Profile* profile, bool browser_launched) override; diff --git a/chrome/browser/chromeos/login/auth/chrome_login_performer.cc b/chrome/browser/chromeos/login/auth/chrome_login_performer.cc index 2b5bcc7b3c8f..e913bc17c876 100644 --- a/chrome/browser/chromeos/login/auth/chrome_login_performer.cc +++ b/chrome/browser/chromeos/login/auth/chrome_login_performer.cc @@ -10,7 +10,6 @@ #include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_user_login_flow.h" #include "chrome/browser/chromeos/login/helper.h" #include "chrome/browser/chromeos/login/session/user_session_manager.h" -#include "chrome/browser/chromeos/login/startup_utils.h" #include "chrome/browser/chromeos/login/supervised/supervised_user_authentication.h" #include "chrome/browser/chromeos/login/supervised/supervised_user_constants.h" #include "chrome/browser/chromeos/login/supervised/supervised_user_login_flow.h" @@ -24,11 +23,8 @@ namespace chromeos { ChromeLoginPerformer::ChromeLoginPerformer(Delegate* delegate) - : LoginPerformer(base::ThreadTaskRunnerHandle::Get(), - delegate, - StartupUtils::IsWebviewSigninEnabled()), - weak_factory_(this) { -} + : LoginPerformer(base::ThreadTaskRunnerHandle::Get(), delegate), + weak_factory_(this) {} ChromeLoginPerformer::~ChromeLoginPerformer() { } diff --git a/chrome/browser/chromeos/login/auth/chrome_login_performer.h b/chrome/browser/chromeos/login/auth/chrome_login_performer.h index 800f6a13e9f7..a0b670c5ee68 100644 --- a/chrome/browser/chromeos/login/auth/chrome_login_performer.h +++ b/chrome/browser/chromeos/login/auth/chrome_login_performer.h @@ -15,7 +15,6 @@ #include "chromeos/login/auth/authenticator.h" #include "chromeos/login/auth/extended_authenticator.h" #include "chromeos/login/auth/login_performer.h" -#include "chromeos/login/auth/online_attempt_host.h" #include "chromeos/login/auth/user_context.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc index 74c5eb0033a2..03b88f7b399c 100644 --- a/chrome/browser/chromeos/login/existing_user_controller.cc +++ b/chrome/browser/chromeos/login/existing_user_controller.cc @@ -28,7 +28,6 @@ #include "chrome/browser/chromeos/login/easy_unlock/bootstrap_user_context_initializer.h" #include "chrome/browser/chromeos/login/easy_unlock/bootstrap_user_flow.h" #include "chrome/browser/chromeos/login/helper.h" -#include "chrome/browser/chromeos/login/reauth_stats.h" #include "chrome/browser/chromeos/login/session/user_session_manager.h" #include "chrome/browser/chromeos/login/signin/oauth2_token_initializer.h" #include "chrome/browser/chromeos/login/signin_specifics.h" @@ -156,7 +155,6 @@ ExistingUserController::ExistingUserController(LoginDisplayHost* host) login_display_(host_->CreateLoginDisplay(this)), num_login_attempts_(0), cros_settings_(CrosSettings::Get()), - offline_failed_(false), is_login_in_progress_(false), password_changed_(false), auth_mode_(LoginPerformer::AUTH_MODE_EXTENSION), @@ -516,7 +514,6 @@ void ExistingUserController::ShowTPMError() { // void ExistingUserController::OnAuthFailure(const AuthFailure& failure) { - offline_failed_ = true; guest_mode_url_ = GURL::EmptyGURL(); std::string error = failure.GetErrorString(); @@ -538,8 +535,6 @@ void ExistingUserController::OnAuthFailure(const AuthFailure& failure) { base::TimeDelta::FromMilliseconds(kSafeModeRestartUiDelayMs)); } else if (failure.reason() == AuthFailure::TPM_ERROR) { ShowTPMError(); - } else if (!online_succeeded_for_.empty()) { - ShowGaiaPasswordChanged(online_succeeded_for_); } else if (last_login_attempt_username_ == chromeos::login::kGuestUserName) { // Show no errors, just re-enable input. login_display_->ClearAndEnablePassword(); @@ -592,7 +587,6 @@ void ExistingUserController::OnAuthFailure(const AuthFailure& failure) { void ExistingUserController::OnAuthSuccess(const UserContext& user_context) { is_login_in_progress_ = false; - offline_failed_ = false; login_display_->set_signin_completed(true); // Login performer will be gone so cache this value to use @@ -656,7 +650,6 @@ void ExistingUserController::OnProfilePrepared(Profile* profile, void ExistingUserController::OnOffTheRecordAuthSuccess() { is_login_in_progress_ = false; - offline_failed_ = false; // Mark the device as registered., i.e. the second part of OOBE as completed. if (!StartupUtils::IsDeviceRegistered()) @@ -670,7 +663,6 @@ void ExistingUserController::OnOffTheRecordAuthSuccess() { void ExistingUserController::OnPasswordChangeDetected() { is_login_in_progress_ = false; - offline_failed_ = false; // Must not proceed without signature verification. if (CrosSettingsProvider::TRUSTED != cros_settings_->PrepareTrustedValues( @@ -707,7 +699,6 @@ void ExistingUserController::OnPasswordChangeDetected() { void ExistingUserController::WhiteListCheckFailed(const std::string& email) { PerformLoginFinishedActions(true /* start public session timer */); - offline_failed_ = false; if (StartupUtils::IsWebviewSigninEnabled()) { login_display_->ShowWhitelistCheckFailedError(); @@ -734,20 +725,9 @@ void ExistingUserController::PolicyLoadFailed() { ShowError(IDS_LOGIN_ERROR_OWNER_KEY_LOST, ""); PerformLoginFinishedActions(false /* don't start public session timer */); - offline_failed_ = false; display_email_.clear(); } -void ExistingUserController::OnOnlineChecked(const std::string& username, - bool success) { - if (success && last_login_attempt_username_ == username) { - online_succeeded_for_ = username; - // Wait for login attempt to end, if it hasn't yet. - if (offline_failed_ && !is_login_in_progress_) - ShowGaiaPasswordChanged(username); - } -} - //////////////////////////////////////////////////////////////////////////////// // ExistingUserController, private: @@ -992,18 +972,6 @@ void ExistingUserController::ShowError(int error_id, login_display_->ShowError(error_id, num_login_attempts_, help_topic_id); } -void ExistingUserController::ShowGaiaPasswordChanged( - const std::string& username) { - // Invalidate OAuth token, since it can't be correct after password is - // changed. - user_manager::UserManager::Get()->SaveUserOAuthStatus( - username, user_manager::User::OAUTH2_TOKEN_STATUS_INVALID); - RecordReauthReason(username, ReauthReason::OTHER); - - login_display_->SetUIEnabled(true); - login_display_->ShowGaiaPasswordChanged(username); -} - void ExistingUserController::SendAccessibilityAlert( const std::string& alert_text) { AutomationManagerAura::GetInstance()->HandleAlert( @@ -1049,10 +1017,6 @@ void ExistingUserController::PerformPreLoginActions( if (last_login_attempt_username_ != user_context.GetUserID()) { last_login_attempt_username_ = user_context.GetUserID(); num_login_attempts_ = 0; - - // Also reset state variables, which are used to determine password change. - offline_failed_ = false; - online_succeeded_for_.clear(); } // Guard in cases when we're called twice but login process is still active. diff --git a/chrome/browser/chromeos/login/existing_user_controller.h b/chrome/browser/chromeos/login/existing_user_controller.h index d357155464af..0df5d284cb40 100644 --- a/chrome/browser/chromeos/login/existing_user_controller.h +++ b/chrome/browser/chromeos/login/existing_user_controller.h @@ -148,7 +148,6 @@ class ExistingUserController : public LoginDisplay::Delegate, void OnPasswordChangeDetected() override; void WhiteListCheckFailed(const std::string& email) override; void PolicyLoadFailed() override; - void OnOnlineChecked(const std::string& username, bool success) override; // UserSessionManagerDelegate implementation: void OnProfilePrepared(Profile* profile, bool browser_launched) override; @@ -164,9 +163,6 @@ class ExistingUserController : public LoginDisplay::Delegate, // provided by authenticator, it is not localized. void ShowError(int error_id, const std::string& details); - // Shows Gaia page because password change was detected. - void ShowGaiaPasswordChanged(const std::string& username); - // Handles result of ownership check and starts enterprise or kiosk enrollment // if applicable. void OnEnrollmentOwnershipCheckCompleted( @@ -288,15 +284,9 @@ class ExistingUserController : public LoginDisplay::Delegate, // The displayed email for the next login attempt set by |SetDisplayEmail|. std::string display_email_; - // Whether offline login attempt failed. - bool offline_failed_; - // Whether login attempt is running. bool is_login_in_progress_; - // Whether online login attempt succeeded. - std::string online_succeeded_for_; - // True if password has been changed for user who is completing sign in. // Set in OnLoginSuccess. Before that use LoginPerformer::password_changed(). bool password_changed_; diff --git a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc index 890fa92df173..5fbaa050a9a4 100644 --- a/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc +++ b/chrome/browser/chromeos/login/existing_user_controller_browsertest.cc @@ -158,14 +158,12 @@ class ExistingUserControllerTest : public policy::DevicePolicyCrosBrowserTest { } void TearDownOnMainThread() override { - // ExistingUserController must be deleted before the thread is cleaned up: - // If there is an outstanding login attempt when ExistingUserController is - // deleted, its LoginPerformer instance will be deleted, which in turn - // deletes its OnlineAttemptHost instance. However, OnlineAttemptHost must - // be deleted on the UI thread. - existing_user_controller_.reset(); DevicePolicyCrosBrowserTest::InProcessBrowserTest::TearDownOnMainThread(); + // |existing_user_controller_| has data members that are CrosSettings + // observers. They need to be destructed before CrosSettings. + existing_user_controller_.reset(); + // Test case may be configured with the real user manager but empty user // list initially. So network OOBE screen is initialized. // Need to reset it manually so that we don't end up with CrosSettings diff --git a/chrome/browser/chromeos/login/screens/error_screen.cc b/chrome/browser/chromeos/login/screens/error_screen.cc index 5b36badbfe33..dd4cdb50cb88 100644 --- a/chrome/browser/chromeos/login/screens/error_screen.cc +++ b/chrome/browser/chromeos/login/screens/error_screen.cc @@ -246,10 +246,6 @@ void ErrorScreen::PolicyLoadFailed() { LOG(FATAL); } -void ErrorScreen::OnOnlineChecked(const std::string& username, bool success) { - LOG(FATAL); -} - ErrorScreen::ConnectRequestCallbackSubscription ErrorScreen::RegisterConnectRequestCallback(const base::Closure& callback) { return connect_request_callbacks_.Add(callback); diff --git a/chrome/browser/chromeos/login/screens/error_screen.h b/chrome/browser/chromeos/login/screens/error_screen.h index c1ce3578dfa3..b73284c8dd68 100644 --- a/chrome/browser/chromeos/login/screens/error_screen.h +++ b/chrome/browser/chromeos/login/screens/error_screen.h @@ -62,7 +62,6 @@ class ErrorScreen : public NetworkErrorModel, public LoginPerformer::Delegate { void OnPasswordChangeDetected() override; void WhiteListCheckFailed(const std::string& email) override; void PolicyLoadFailed() override; - void OnOnlineChecked(const std::string& username, bool success) override; // Register a callback to be invoked when the user indicates that an attempt // to connect to the network should be made. diff --git a/chrome/browser/chromeos/login/ui/login_display.h b/chrome/browser/chromeos/login/ui/login_display.h index 7ed2e508fcfa..a60d45673886 100644 --- a/chrome/browser/chromeos/login/ui/login_display.h +++ b/chrome/browser/chromeos/login/ui/login_display.h @@ -131,9 +131,6 @@ class LoginDisplay { // Displays detailed error screen for error with ID |error_id|. virtual void ShowErrorScreen(LoginDisplay::SigninError error_id) = 0; - // Proceed with Gaia flow because password has changed. - virtual void ShowGaiaPasswordChanged(const std::string& username) = 0; - // Show password changed dialog. If |show_password_error| is not null // user already tried to enter old password but it turned out to be incorrect. virtual void ShowPasswordChangedDialog(bool show_password_error, diff --git a/chrome/browser/chromeos/login/ui/mock_login_display.h b/chrome/browser/chromeos/login/ui/mock_login_display.h index dca85a1414c5..4af0487811ac 100644 --- a/chrome/browser/chromeos/login/ui/mock_login_display.h +++ b/chrome/browser/chromeos/login/ui/mock_login_display.h @@ -23,7 +23,6 @@ class MockLoginDisplay : public LoginDisplay { MOCK_METHOD1(SetUIEnabled, void(bool)); MOCK_METHOD3(ShowError, void(int, int, HelpAppLauncher::HelpTopic)); MOCK_METHOD1(ShowErrorScreen, void(LoginDisplay::SigninError)); - MOCK_METHOD1(ShowGaiaPasswordChanged, void(const std::string&)); MOCK_METHOD2(ShowPasswordChangedDialog, void(bool, const std::string&)); MOCK_METHOD1(ShowSigninUI, void(const std::string&)); MOCK_METHOD0(ShowWhitelistCheckFailedError, void(void)); diff --git a/chrome/browser/chromeos/login/ui/webui_login_display.cc b/chrome/browser/chromeos/login/ui/webui_login_display.cc index a39ac27d012e..ec0e6a50e580 100644 --- a/chrome/browser/chromeos/login/ui/webui_login_display.cc +++ b/chrome/browser/chromeos/login/ui/webui_login_display.cc @@ -180,11 +180,6 @@ void WebUILoginDisplay::ShowErrorScreen(LoginDisplay::SigninError error_id) { webui_handler_->ShowErrorScreen(error_id); } -void WebUILoginDisplay::ShowGaiaPasswordChanged(const std::string& username) { - if (webui_handler_) - webui_handler_->ShowGaiaPasswordChanged(username); -} - void WebUILoginDisplay::ShowPasswordChangedDialog(bool show_password_error, const std::string& email) { if (webui_handler_) diff --git a/chrome/browser/chromeos/login/ui/webui_login_display.h b/chrome/browser/chromeos/login/ui/webui_login_display.h index a810af9dc5db..f4883f2d443b 100644 --- a/chrome/browser/chromeos/login/ui/webui_login_display.h +++ b/chrome/browser/chromeos/login/ui/webui_login_display.h @@ -41,7 +41,6 @@ class WebUILoginDisplay : public LoginDisplay, int login_attempts, HelpAppLauncher::HelpTopic help_topic_id) override; void ShowErrorScreen(LoginDisplay::SigninError error_id) override; - void ShowGaiaPasswordChanged(const std::string& username) override; void ShowPasswordChangedDialog(bool show_password_error, const std::string& email) override; void ShowSigninUI(const std::string& email) override; diff --git a/chrome/browser/resources/chromeos/login/screen_gaia_signin.html b/chrome/browser/resources/chromeos/login/screen_gaia_signin.html index 58f75f2dfdc6..481dd5174dc0 100644 --- a/chrome/browser/resources/chromeos/login/screen_gaia_signin.html +++ b/chrome/browser/resources/chromeos/login/screen_gaia_signin.html @@ -15,7 +15,6 @@