diff --git a/chrome/browser/chromeos/login/google_authenticator.cc b/chrome/browser/chromeos/login/google_authenticator.cc index 6834b4247fd6d3..6cd7b8a5f67ac0 100644 --- a/chrome/browser/chromeos/login/google_authenticator.cc +++ b/chrome/browser/chromeos/login/google_authenticator.cc @@ -138,17 +138,6 @@ bool GoogleAuthenticator::AuthenticateToLogin( profile->GetRequestContext())); // Will be used for retries. PrepareClientLoginAttempt(password, login_token, login_captcha); - // If this is the first time this user has tried to login, and she has - // a HOSTED account, we want to encourage opting in to namespace unification. - // Thus, we first need to detect whether this is a HOSTED account or not. - // Only way to do that is to _try_ authing as a HOSTED account. That said, - // we don't want to put ALL users through two authentication attempts. - // So, we detect if this is a first-time login, and then try to auth - // as a pure GOOGLE account. If that fails, we'll try again with HOSTED. - // If this user has logged in before, just accept either. - hosted_policy_ = (!user_manager_->IsKnownUser(username) ? - GaiaAuthenticator2::HostedAccountsNotAllowed : - GaiaAuthenticator2::HostedAccountsAllowed); TryClientLogin(); return true; } @@ -198,16 +187,27 @@ void GoogleAuthenticator::OnClientLoginSuccess( VLOG(1) << "Online login successful!"; ClearClientLoginAttempt(); - GaiaAuthConsumer::ClientLoginResult new_creds(credentials); - new_creds.is_hosted = - (hosted_policy_ == GaiaAuthenticator2::HostedAccountsAllowed && - !user_manager_->IsKnownUser(username_)); - + if (hosted_policy_ == GaiaAuthenticator2::HostedAccountsAllowed) { + // We don't allow HOSTED accounts to log in. Call OnLoginFailure() + // with an appropriate LoginFailure. + LoginFailure failure_details = + LoginFailure::FromNetworkAuthFailure( + GoogleServiceAuthError( + GoogleServiceAuthError::HOSTED_NOT_ALLOWED)); + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + NewRunnableMethod(this, + &GoogleAuthenticator::OnLoginFailure, + failure_details)); + VLOG(1) << "Rejecting valid HOSTED account."; + hosted_policy_ = GaiaAuthenticator2::HostedAccountsNotAllowed; + return; + } BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, NewRunnableMethod(this, &GoogleAuthenticator::OnLoginSuccess, - new_creds, false)); + credentials, false)); } void GoogleAuthenticator::OnClientLoginFailure( @@ -224,6 +224,7 @@ void GoogleAuthenticator::OnClientLoginFailure( } if (error.state() == GoogleServiceAuthError::INVALID_GAIA_CREDENTIALS && + !user_manager_->IsKnownUser(username_) && hosted_policy_ != GaiaAuthenticator2::HostedAccountsAllowed) { // if this was a first-time login, then we may have failed because the // account is HOSTED. Try again, but allowing HOSTED accounts. @@ -282,7 +283,6 @@ void GoogleAuthenticator::OnLoginSuccess( ascii_hash_.c_str(), &mount_error))) { BootTimesLoader::Get()->AddLoginTimeMarker("CryptohomeMounted", true); - VLOG_IF(1, credentials.is_hosted) << "Authenticating hosted account"; consumer_->OnLoginSuccess(username_, credentials, request_pending); } else if (!unlock_ && mount_error == chromeos::kCryptohomeMountErrorKeyFailure) { diff --git a/chrome/browser/chromeos/login/google_authenticator.h b/chrome/browser/chromeos/login/google_authenticator.h index 6316029828dd34..5b14924f4a37e2 100644 --- a/chrome/browser/chromeos/login/google_authenticator.h +++ b/chrome/browser/chromeos/login/google_authenticator.h @@ -44,6 +44,10 @@ class GoogleAuthenticator : public Authenticator, public GaiaAuthConsumer { // Optionally could pass CAPTCHA challenge token - |login_token| and // |login_captcha| string that user has entered. // + // NOTE: We do not allow HOSTED accounts to log in. In the event that + // we are asked to authenticate valid HOSTED account creds, we will + // call OnLoginFailure() with HOSTED_NOT_ALLOWED. + // // Returns true if the attempt gets sent successfully and false if not. bool AuthenticateToLogin(Profile* profile, const std::string& username, diff --git a/chrome/browser/chromeos/login/google_authenticator_unittest.cc b/chrome/browser/chromeos/login/google_authenticator_unittest.cc index 07eea9ef8ba710..50da187d5a9815 100644 --- a/chrome/browser/chromeos/login/google_authenticator_unittest.cc +++ b/chrome/browser/chromeos/login/google_authenticator_unittest.cc @@ -567,10 +567,6 @@ TEST_F(GoogleAuthenticatorTest, FullLogin) { URLFetcher::set_factory(&factory); scoped_refptr auth(new GoogleAuthenticator(&consumer)); - auth->set_user_manager(user_manager_.get()); - EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_)) - .Times(2) - .WillRepeatedly(Return(true)); auth->AuthenticateToLogin( &profile, username_, hash_ascii_, std::string(), std::string()); @@ -583,15 +579,18 @@ TEST_F(GoogleAuthenticatorTest, FullHostedLogin) { BrowserThread ui_thread(BrowserThread::UI, &message_loop); chromeos::CryptohomeBlob salt_v(fake_hash_, fake_hash_ + sizeof(fake_hash_)); - GaiaAuthConsumer::ClientLoginResult hosted_result(true, "", "", "", ""); + LoginFailure failure_details = + LoginFailure::FromNetworkAuthFailure( + GoogleServiceAuthError( + GoogleServiceAuthError::HOSTED_NOT_ALLOWED)); MockConsumer consumer; - EXPECT_CALL(consumer, OnLoginSuccess(username_, Eq(hosted_result), false)) - .Times(1) - .RetiresOnSaturation(); - EXPECT_CALL(*mock_library_, Mount(username_, _, _)) - .WillOnce(Return(true)) + EXPECT_CALL(consumer, OnLoginFailure(failure_details)) + .WillOnce(Invoke(MockConsumer::OnFailQuit)) .RetiresOnSaturation(); + // A failure case, but we still want the test to finish gracefully. + ON_CALL(consumer, OnLoginSuccess(username_, _, _)) + .WillByDefault(Invoke(MockConsumer::OnSuccessQuitAndFail)); EXPECT_CALL(*mock_library_, GetSystemSalt()) .WillOnce(Return(salt_v)) @@ -605,8 +604,8 @@ TEST_F(GoogleAuthenticatorTest, FullHostedLogin) { scoped_refptr auth(new GoogleAuthenticator(&consumer)); auth->set_user_manager(user_manager_.get()); EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_)) - .Times(2) - .WillRepeatedly(Return(false)); + .WillOnce(Return(false)) + .RetiresOnSaturation(); auth->AuthenticateToLogin( &profile, username_, hash_ascii_, std::string(), std::string()); @@ -622,17 +621,19 @@ TEST_F(GoogleAuthenticatorTest, FullHostedLoginFailure) { BrowserThread ui_thread(BrowserThread::UI, &message_loop); chromeos::CryptohomeBlob salt_v(fake_hash_, fake_hash_ + sizeof(fake_hash_)); - GaiaAuthConsumer::ClientLoginResult hosted_result(true, "", "", "", ""); + LoginFailure failure_details = + LoginFailure::FromNetworkAuthFailure( + GoogleServiceAuthError( + GoogleServiceAuthError::HOSTED_NOT_ALLOWED)); MockConsumer consumer; - EXPECT_CALL(consumer, OnLoginFailure(_)) + EXPECT_CALL(consumer, OnLoginFailure(failure_details)) .WillOnce(Invoke(MockConsumer::OnFailQuit)) .RetiresOnSaturation(); // A failure case, but we still want the test to finish gracefully. ON_CALL(consumer, OnLoginSuccess(username_, _, _)) .WillByDefault(Invoke(MockConsumer::OnSuccessQuitAndFail)); - EXPECT_CALL(*mock_library_, GetSystemSalt()) .WillOnce(Return(salt_v)) .RetiresOnSaturation(); @@ -645,8 +646,8 @@ TEST_F(GoogleAuthenticatorTest, FullHostedLoginFailure) { scoped_refptr auth(new GoogleAuthenticator(&consumer)); auth->set_user_manager(user_manager_.get()); EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_)) - .Times(2) - .WillRepeatedly(Return(false)); + .WillOnce(Return(false)) + .RetiresOnSaturation(); auth->AuthenticateToLogin( &profile, username_, hash_ascii_, std::string(), std::string()); @@ -692,10 +693,6 @@ TEST_F(GoogleAuthenticatorTest, CancelLogin) { URLFetcher::set_factory(&factory); scoped_refptr auth(new GoogleAuthenticator(&consumer)); - auth->set_user_manager(user_manager_.get()); - EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_)) - .WillOnce(Return(true)); - // For when |auth| tries to load the localaccount file. BrowserThread file_thread(BrowserThread::FILE); file_thread.Start(); @@ -747,10 +744,6 @@ TEST_F(GoogleAuthenticatorTest, CancelLoginAlreadyGotLocalaccount) { URLFetcher::set_factory(&factory); scoped_refptr auth(new GoogleAuthenticator(&consumer)); - auth->set_user_manager(user_manager_.get()); - EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_)) - .WillOnce(Return(true)); - // This time, instead of allowing |auth| to go get the localaccount file // itself, we simulate the case where the file is already loaded, which // happens when this isn't the first login since chrome started. diff --git a/chrome/common/net/gaia/gaia_auth_consumer.h b/chrome/common/net/gaia/gaia_auth_consumer.h index d063c8c4faa33e..429ab7f9be90be 100644 --- a/chrome/common/net/gaia/gaia_auth_consumer.h +++ b/chrome/common/net/gaia/gaia_auth_consumer.h @@ -15,37 +15,23 @@ class GoogleServiceAuthError; class GaiaAuthConsumer { public: struct ClientLoginResult { - inline ClientLoginResult() - : is_hosted(false) {} + inline ClientLoginResult() {} inline ClientLoginResult(const std::string& new_sid, const std::string& new_lsid, const std::string& new_token, const std::string& new_data) - : is_hosted(false), - sid(new_sid), - lsid(new_lsid), - token(new_token), - data(new_data) {} - inline ClientLoginResult(const bool hosted, - const std::string& new_sid, - const std::string& new_lsid, - const std::string& new_token, - const std::string& new_data) - : is_hosted(hosted), - sid(new_sid), + : sid(new_sid), lsid(new_lsid), token(new_token), data(new_data) {} inline bool operator==(const ClientLoginResult &b) const { - return is_hosted == b.is_hosted && - sid == b.sid && + return sid == b.sid && lsid == b.lsid && token == b.token && data == b.data; } - bool is_hosted; std::string sid; std::string lsid; std::string token; diff --git a/chrome/common/net/gaia/google_service_auth_error.h b/chrome/common/net/gaia/google_service_auth_error.h index fa6f76b264f217..032f59f184694f 100644 --- a/chrome/common/net/gaia/google_service_auth_error.h +++ b/chrome/common/net/gaia/google_service_auth_error.h @@ -71,6 +71,10 @@ class GoogleServiceAuthError { // The requestor of the authentication step cancelled the request // prior to completion. REQUEST_CANCELED = 9, + + // The user has provided a HOSTED account, when this service requires + // a GOOGLE account. + HOSTED_NOT_ALLOWED = 10, }; // Additional data for CAPTCHA_REQUIRED errors.