[Chrome OS] Prevent HOSTED accounts from logging in, but still handle CAPTCHA correctly

BUG=chromium-os:7867
TEST=unit tests, install on device and verify that you can clear the CAPTCHAd state for a HOSTED account, and still see the appropriate log message.

Review URL: http://codereview.chromium.org/4109004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64247 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 34 insertions, 18 deletions

diff --git a/chrome/browser/chromeos/login/google_authenticator.cc b/chrome/browser/chromeos/login/google_authenticator.cc
index 3a0d6b2..3ab5640 100644
--- a/chrome/browser/chromeos/login/google_authenticator.cc
+++ b/chrome/browser/chromeos/login/google_authenticator.cc
@@ -58,7 +58,7 @@ const int kPassHashLen = 32;
 GoogleAuthenticator::GoogleAuthenticator(LoginStatusConsumer* consumer)
     : Authenticator(consumer),
       user_manager_(UserManager::Get()),
-      hosted_policy_(GaiaAuthenticator2::HostedAccountsNotAllowed),
+      hosted_policy_(GaiaAuthenticator2::HostedAccountsAllowed),
       unlock_(false),
       try_again_(true),
       checked_for_localaccount_(false) {
@@ -188,20 +188,17 @@ void GoogleAuthenticator::OnClientLoginSuccess(
   VLOG(1) << "Online login successful!";
   ClearClientLoginAttempt();
 
-  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.";
+  if (hosted_policy_ == GaiaAuthenticator2::HostedAccountsAllowed &&
+      !user_manager_->IsKnownUser(username_)) {
+    // First time user, and we don't know if the account is HOSTED or not.
+    // Since we don't allow HOSTED accounts to log in, we need to try
+    // again, without allowing HOSTED accounts.
+    //
+    // NOTE: we used to do this in the opposite order, so that we'd only
+    // try the HOSTED pathway if GOOGLE-only failed.  This breaks CAPTCHA
+    // handling, though.
     hosted_policy_ = GaiaAuthenticator2::HostedAccountsNotAllowed;
+    TryClientLogin();
     return;
   }
   BrowserThread::PostTask(
@@ -227,10 +224,20 @@ 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.
-    hosted_policy_ = GaiaAuthenticator2::HostedAccountsAllowed;
-    TryClientLogin();
+    // This was a first-time login, we already tried allowing HOSTED accounts
+    // and succeeded.  That we've failed with INVALID_GAIA_CREDENTIALS now
+    // indicates that the account is HOSTED.
+    LoginFailure failure_details =
+        LoginFailure::FromNetworkAuthFailure(
+            GoogleServiceAuthError(
+                GoogleServiceAuthError::HOSTED_NOT_ALLOWED));
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        NewRunnableMethod(this,
+                          &GoogleAuthenticator::OnLoginFailure,
+                          failure_details));
+    LOG(WARNING) << "Rejecting valid HOSTED account.";
+    hosted_policy_ = GaiaAuthenticator2::HostedAccountsNotAllowed;
     return;
   }
 
diff --git a/chrome/browser/chromeos/login/google_authenticator_unittest.cc b/chrome/browser/chromeos/login/google_authenticator_unittest.cc
index 1e5211d..956742a 100644
--- a/chrome/browser/chromeos/login/google_authenticator_unittest.cc
+++ b/chrome/browser/chromeos/login/google_authenticator_unittest.cc
@@ -483,6 +483,9 @@ TEST_F(GoogleAuthenticatorTest, OnlineLogin) {
 
   scoped_refptr<GoogleAuthenticator> auth(new GoogleAuthenticator(&consumer));
   PrepForLogin(auth.get());
+  EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_))
+      .WillOnce(Return(true))
+      .RetiresOnSaturation();
   auth->OnClientLoginSuccess(result_);
   message_loop.RunAllPending();
 }
@@ -577,6 +580,10 @@ TEST_F(GoogleAuthenticatorTest, FullLogin) {
   URLFetcher::set_factory(&factory);
 
   scoped_refptr<GoogleAuthenticator> auth(new GoogleAuthenticator(&consumer));
+  EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_))
+      .WillOnce(Return(true))
+      .RetiresOnSaturation();
+  auth->set_user_manager(user_manager_.get());
   auth->AuthenticateToLogin(
       &profile, username_, password_, std::string(), std::string());
 
@@ -615,6 +622,7 @@ TEST_F(GoogleAuthenticatorTest, FullHostedLogin) {
   auth->set_user_manager(user_manager_.get());
   EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_))
       .WillOnce(Return(false))
+      .WillOnce(Return(false))
       .RetiresOnSaturation();
   auth->AuthenticateToLogin(
       &profile, username_, hash_ascii_, std::string(), std::string());
@@ -657,6 +665,7 @@ TEST_F(GoogleAuthenticatorTest, FullHostedLoginFailure) {
   auth->set_user_manager(user_manager_.get());
   EXPECT_CALL(*user_manager_.get(), IsKnownUser(username_))
       .WillOnce(Return(false))
+      .WillOnce(Return(false))
       .RetiresOnSaturation();
   auth->AuthenticateToLogin(
       &profile, username_, hash_ascii_, std::string(), std::string());