[CHrome OS] HOSTED accounts will not be allowed to log in

Instead of allowing HOSTED accounts to log in, but be flagged, we are instead detecting them and blocking them. Now, when a HOSTED account tries to log in, AuthenticateToLogin will call OnLoginFailure with USER_NOT_SIGNED_UP, indicating that the account in question is not allowed to use this device. BUG=chromium-os:7867 TEST=unit tests, install on device and attempt to log in with a HOSTED account. See that it fails. Review URL: http://codereview.chromium.org/3973010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63771 0039d316-1c4b-4281-b951-d872f2087c98
cbchan · Oct 25, 2010 · 992848f · 992848f
1 parent 553b4aa
commit 992848f
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 60 deletions.
diff --git 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
@@ -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
@@ -567,10 +567,6 @@ TEST_F(GoogleAuthenticatorTest, FullLogin) {
   URLFetcher::set_factory(&factory);
 
   scoped_refptr<GoogleAuthenticator> 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<GoogleAuthenticator> 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<GoogleAuthenticator> 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<GoogleAuthenticator> 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<GoogleAuthenticator> 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
@@ -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
@@ -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.