Fix device policy recovery on CrOS login

If the device policy file cannot be loaded on login, the device enters
a state where it allows login process to proceed only if the user logging
in is the owner. To be able to successfully determine if the user is
owner, parallel authenticator has to wait until the certificates
are loaded by CertLoader. The problem is that the CertLoader starts
loading the certificates _after_ the user actually logs in. So if the
authenticator actually waited for the owner status to be resolved, the
login would hang.

This CL adds another state to LoginState::LoggedInState enum (SAFE_MODE)
in which CertLoader will be allowed to start loading the certificates.
When the authenticator detects the policy file corruption, it changes
login state to SAFE MODE (which triggers the certificate loading) and
waits for the DeviceSettingsService to determine whether the current user
is the owner.

Also, removed LoginState::GetLoggedInState. Replaced it's pre-existing
usages with LoginState::IsUserLoggedIn; and added
LoginState::IsInSafeMode to be used here.

BUG=285450
TEST=
1. manually remove device policy file
2. try logging in to a non-owner account -> should fail  
3. try logging in to the owner account -> should succeed
4. try logging in to a non-owner account again -> should succeed

Review URL: https://chromiumcodereview.appspot.com/23684033

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

7 files changed, 91 insertions, 42 deletions

diff --git a/chromeos/cert_loader.cc b/chromeos/cert_loader.cc
index 060d464..62a4f12 100644
--- a/chromeos/cert_loader.cc
+++ b/chromeos/cert_loader.cc
@@ -145,10 +145,14 @@ void CertLoader::MaybeRequestCertificates() {
   if (certificates_requested_ || !crypto_task_runner_.get())
     return;
 
-  const bool logged_in = LoginState::IsInitialized() ?
-      LoginState::Get()->IsUserLoggedIn() : false;
-  VLOG(1) << "RequestCertificates: " << logged_in;
-  if (!logged_in)
+  if (!LoginState::IsInitialized())
+    return;
+
+  bool request_certificates = LoginState::Get()->IsUserLoggedIn() ||
+      LoginState::Get()->IsInSafeMode();
+
+  VLOG(1) << "RequestCertificates: " << request_certificates;
+  if (!request_certificates)
     return;
 
   certificates_requested_ = true;
@@ -159,7 +163,7 @@ void CertLoader::MaybeRequestCertificates() {
     tpm_token_state_ = TPM_DISABLED;
 
   // Treat TPM as disabled for guest users since they do not store certs.
-  if (LoginState::IsInitialized() && LoginState::Get()->IsGuestUser())
+  if (LoginState::Get()->IsGuestUser())
     tpm_token_state_ = TPM_DISABLED;
 
   InitializeTokenAndLoadCertificates();
@@ -387,8 +391,8 @@ void CertLoader::OnCertRemoved(const net::X509Certificate* cert) {
   LoadCertificates();
 }
 
-void CertLoader::LoggedInStateChanged(LoginState::LoggedInState state) {
-  VLOG(1) << "LoggedInStateChanged: " << state;
+void CertLoader::LoggedInStateChanged() {
+  VLOG(1) << "LoggedInStateChanged";
   MaybeRequestCertificates();
 }
 
diff --git a/chromeos/cert_loader.h b/chromeos/cert_loader.h
index bc1123e..9c5ace4 100644
--- a/chromeos/cert_loader.h
+++ b/chromeos/cert_loader.h
@@ -147,7 +147,7 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer,
   virtual void OnCertRemoved(const net::X509Certificate* cert) OVERRIDE;
 
   // LoginState::Observer
-  virtual void LoggedInStateChanged(LoginState::LoggedInState state) OVERRIDE;
+  virtual void LoggedInStateChanged() OVERRIDE;
 
   bool initialize_tpm_for_test_;
 
diff --git a/chromeos/login/login_state.cc b/chromeos/login/login_state.cc
index 8c8c329..57a889b 100644
--- a/chromeos/login/login_state.cc
+++ b/chromeos/login/login_state.cc
@@ -67,22 +67,23 @@ void LoginState::SetLoggedInState(LoggedInState state,
   NotifyObservers();
 }
 
-LoginState::LoggedInState LoginState::GetLoggedInState() const {
-  if (AlwaysLoggedIn())
-    return LOGGED_IN_ACTIVE;
-  return logged_in_state_;
-}
-
 LoginState::LoggedInUserType LoginState::GetLoggedInUserType() const {
   return logged_in_user_type_;
 }
 
 bool LoginState::IsUserLoggedIn() const {
-  return GetLoggedInState() == LOGGED_IN_ACTIVE;
+  if (AlwaysLoggedIn())
+    return true;
+  return logged_in_state_ == LOGGED_IN_ACTIVE;
+}
+
+bool LoginState::IsInSafeMode() const {
+  DCHECK(!AlwaysLoggedIn() || logged_in_state_ != LOGGED_IN_SAFE_MODE);
+  return logged_in_state_ == LOGGED_IN_SAFE_MODE;
 }
 
 bool LoginState::IsGuestUser() const {
-  if (GetLoggedInState() != LOGGED_IN_ACTIVE)
+  if (!IsUserLoggedIn())
     return false;
   switch (logged_in_user_type_) {
     case LOGGED_IN_USER_NONE:
@@ -124,7 +125,7 @@ LoginState::~LoginState() {
 
 void LoginState::NotifyObservers() {
   FOR_EACH_OBSERVER(LoginState::Observer, observer_list_,
-                    LoggedInStateChanged(GetLoggedInState()));
+                    LoggedInStateChanged());
 }
 
 }  // namespace chromeos
diff --git a/chromeos/login/login_state.h b/chromeos/login/login_state.h
index 68966f5..0ef18a7 100644
--- a/chromeos/login/login_state.h
+++ b/chromeos/login/login_state.h
@@ -15,9 +15,10 @@ namespace chromeos {
 class CHROMEOS_EXPORT LoginState {
  public:
   enum LoggedInState {
-    LOGGED_IN_OOBE,    // Out of box experience not completed
-    LOGGED_IN_NONE,    // Not logged in
-    LOGGED_IN_ACTIVE   // A user has logged in
+    LOGGED_IN_OOBE,       // Out of box experience not completed
+    LOGGED_IN_NONE,       // Not logged in
+    LOGGED_IN_SAFE_MODE,  // Not logged in and login not allowed for non-owners
+    LOGGED_IN_ACTIVE      // A user has logged in
   };
 
   enum LoggedInUserType {
@@ -34,7 +35,7 @@ class CHROMEOS_EXPORT LoginState {
   class Observer {
    public:
     // Called when either the login state or the logged in user type changes.
-    virtual void LoggedInStateChanged(LoggedInState state) = 0;
+    virtual void LoggedInStateChanged() = 0;
 
    protected:
     virtual ~Observer() {}
@@ -53,13 +54,16 @@ class CHROMEOS_EXPORT LoginState {
   // Set the logged in state and user type.
   void SetLoggedInState(LoggedInState state, LoggedInUserType type);
 
-  // Get the logged in state / user type.
-  LoggedInState GetLoggedInState() const;
+  // Get the logged in user type.
   LoggedInUserType GetLoggedInUserType() const;
 
-  // Returns true if |logged_in_state_| is active.
+  // Returns true if a user is considered to be logged in.
   bool IsUserLoggedIn() const;
 
+  // Returns true if |logged_in_state_| is safe mode (i.e. the user is not yet
+  // logged in, and only the owner will be allowed to log in).
+  bool IsInSafeMode() const;
+
   // Returns true if logged in and is a guest, retail, public, or kiosk user.
   bool IsGuestUser() const;
 
diff --git a/chromeos/login/login_state_unittest.cc b/chromeos/login/login_state_unittest.cc
index 0665745..d71b9d5 100644
--- a/chromeos/login/login_state_unittest.cc
+++ b/chromeos/login/login_state_unittest.cc
@@ -16,9 +16,8 @@ namespace chromeos {
 class LoginStateTest : public testing::Test,
                        public LoginState::Observer {
  public:
-  LoginStateTest()
-      : logged_in_state_(LoginState::LOGGED_IN_OOBE),
-        logged_in_user_type_(LoginState::LOGGED_IN_USER_NONE) {
+  LoginStateTest() : logged_in_user_type_(LoginState::LOGGED_IN_USER_NONE),
+                     login_state_changes_count_(0) {
   }
   virtual ~LoginStateTest() {
   }
@@ -39,38 +38,81 @@ class LoginStateTest : public testing::Test,
   }
 
   // LoginState::Observer
-  virtual void LoggedInStateChanged(LoginState::LoggedInState state) OVERRIDE {
-    logged_in_state_ = state;
+  virtual void LoggedInStateChanged() OVERRIDE {
+    ++login_state_changes_count_;
     logged_in_user_type_ = LoginState::Get()->GetLoggedInUserType();
   }
 
  protected:
+  // Returns number of times the login state changed since the last call to
+  // this method.
+  unsigned int GetNewLoginStateChangesCount() {
+    unsigned int result = login_state_changes_count_;
+    login_state_changes_count_ = 0;
+    return result;
+  }
+
   base::MessageLoopForUI message_loop_;
-  LoginState::LoggedInState logged_in_state_;
   LoginState::LoggedInUserType logged_in_user_type_;
 
  private:
+  unsigned int login_state_changes_count_;
+
   DISALLOW_COPY_AND_ASSIGN(LoginStateTest);
 };
 
 TEST_F(LoginStateTest, TestLoginState) {
-  EXPECT_EQ(LoginState::LOGGED_IN_OOBE, logged_in_state_);
-  EXPECT_EQ(LoginState::LOGGED_IN_OOBE, LoginState::Get()->GetLoggedInState());
+  EXPECT_FALSE(LoginState::Get()->IsUserLoggedIn());
+  EXPECT_FALSE(LoginState::Get()->IsInSafeMode());
   EXPECT_EQ(LoginState::LOGGED_IN_USER_NONE, logged_in_user_type_);
   EXPECT_EQ(LoginState::LOGGED_IN_USER_NONE,
             LoginState::Get()->GetLoggedInUserType());
+
   // Setting login state to ACTIVE.
   LoginState::Get()->SetLoggedInState(LoginState::LOGGED_IN_ACTIVE,
                                       LoginState::LOGGED_IN_USER_REGULAR);
-  EXPECT_EQ(LoginState::LOGGED_IN_ACTIVE,
-            LoginState::Get()->GetLoggedInState());
   EXPECT_EQ(LoginState::LOGGED_IN_USER_REGULAR,
             LoginState::Get()->GetLoggedInUserType());
   EXPECT_TRUE(LoginState::Get()->IsUserLoggedIn());
+  EXPECT_FALSE(LoginState::Get()->IsInSafeMode());
+
   // Run the message loop, observer should update members.
   message_loop_.RunUntilIdle();
-  EXPECT_EQ(LoginState::LOGGED_IN_ACTIVE, logged_in_state_);
+  EXPECT_EQ(1U, GetNewLoginStateChangesCount());
   EXPECT_EQ(LoginState::LOGGED_IN_USER_REGULAR, logged_in_user_type_);
 }
 
-}  // namespace
+TEST_F(LoginStateTest, TestSafeModeLoginState) {
+  EXPECT_FALSE(LoginState::Get()->IsUserLoggedIn());
+  EXPECT_FALSE(LoginState::Get()->IsInSafeMode());
+  EXPECT_EQ(LoginState::LOGGED_IN_USER_NONE, logged_in_user_type_);
+  EXPECT_EQ(LoginState::LOGGED_IN_USER_NONE,
+            LoginState::Get()->GetLoggedInUserType());
+  // Setting login state to SAFE MODE.
+  LoginState::Get()->SetLoggedInState(LoginState::LOGGED_IN_SAFE_MODE,
+                                      LoginState::LOGGED_IN_USER_NONE);
+  EXPECT_EQ(LoginState::LOGGED_IN_USER_NONE,
+            LoginState::Get()->GetLoggedInUserType());
+  EXPECT_FALSE(LoginState::Get()->IsUserLoggedIn());
+  EXPECT_TRUE(LoginState::Get()->IsInSafeMode());
+
+  // Run the message loop, observer should update members.
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(1U, GetNewLoginStateChangesCount());
+  EXPECT_EQ(LoginState::LOGGED_IN_USER_NONE, logged_in_user_type_);
+
+  // Setting login state to ACTIVE.
+  LoginState::Get()->SetLoggedInState(LoginState::LOGGED_IN_ACTIVE,
+                                      LoginState::LOGGED_IN_USER_OWNER);
+  EXPECT_EQ(LoginState::LOGGED_IN_USER_OWNER,
+            LoginState::Get()->GetLoggedInUserType());
+  EXPECT_TRUE(LoginState::Get()->IsUserLoggedIn());
+  EXPECT_FALSE(LoginState::Get()->IsInSafeMode());
+
+  // Run the message loop, observer should update members.
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(1U, GetNewLoginStateChangesCount());
+  EXPECT_EQ(LoginState::LOGGED_IN_USER_OWNER, logged_in_user_type_);
+}
+
+}  // namespace chromeos
diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc
index dfcba54..6600e56 100644
--- a/chromeos/network/network_connection_handler.cc
+++ b/chromeos/network/network_connection_handler.cc
@@ -157,8 +157,7 @@ void NetworkConnectionHandler::Init(
     NetworkConfigurationHandler* network_configuration_handler) {
   if (LoginState::IsInitialized()) {
     LoginState::Get()->AddObserver(this);
-    logged_in_ =
-        LoginState::Get()->GetLoggedInState() == LoginState::LOGGED_IN_ACTIVE;
+    logged_in_ = LoginState::Get()->IsUserLoggedIn();
   }
   if (CertLoader::IsInitialized()) {
     cert_loader_ = CertLoader::Get();
@@ -175,9 +174,8 @@ void NetworkConnectionHandler::Init(
   network_configuration_handler_ = network_configuration_handler;
 }
 
-void NetworkConnectionHandler::LoggedInStateChanged(
-    LoginState::LoggedInState state) {
-  if (state == LoginState::LOGGED_IN_ACTIVE) {
+void NetworkConnectionHandler::LoggedInStateChanged() {
+  if (LoginState::Get()->IsUserLoggedIn()) {
     logged_in_ = true;
     NET_LOG_EVENT("Logged In", "");
   }
diff --git a/chromeos/network/network_connection_handler.h b/chromeos/network/network_connection_handler.h
index 326e579..d67a41c 100644
--- a/chromeos/network/network_connection_handler.h
+++ b/chromeos/network/network_connection_handler.h
@@ -126,7 +126,7 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
   virtual void NetworkPropertiesUpdated(const NetworkState* network) OVERRIDE;
 
   // LoginState::Observer
-  virtual void LoggedInStateChanged(LoginState::LoggedInState state) OVERRIDE;
+  virtual void LoggedInStateChanged() OVERRIDE;
 
   // CertLoader::Observer
   virtual void OnCertificatesLoaded(const net::CertificateList& cert_list,