Fix device policy recovery on CrOS login

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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223168

[tbarzic, 2013-09-11 23:52:31]

Please, take a look.

Steven: an owner for chromeos/
Dmitry: an owner for login flow

[stevenjb, 2013-09-12 00:48:01]

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_state.h
File chromeos/login/login_state.h (right):

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
chromeos/login/login_state.h:60: // Returns true if an user is considered to be
logged in.
nit: 'a user' (English sucks)

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
File chromeos/login/login_state_unittest.cc (right):

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
chromeos/login/login_state_unittest.cc:40: virtual void LoggedInStateChanged()
OVERRIDE {
We should explicitly track that the observer gets triggered here, and test for
that explicitly, instead of just relying on logged_in_user_type_ getting set
correctly.

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
chromeos/login/login_state_unittest.cc:58: // Setting login state to SAFE MODE.
We should probably test safe mode separately, since normally LoginState won't go
-> SAFE -> ACTIVE, i.e. we should test going -> ACTIVE directly.

[tbarzic, 2013-09-12 01:18:12]

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_state.h
File chromeos/login/login_state.h (right):

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
chromeos/login/login_state.h:60: // Returns true if an user is considered to be
logged in.
On 2013/09/12 00:48:01, stevenjb wrote:
> nit: 'a user' (English sucks)

:)

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
File chromeos/login/login_state_unittest.cc (right):

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
chromeos/login/login_state_unittest.cc:40: virtual void LoggedInStateChanged()
OVERRIDE {
On 2013/09/12 00:48:01, stevenjb wrote:
> We should explicitly track that the observer gets triggered here, and test for
> that explicitly, instead of just relying on logged_in_user_type_ getting set
> correctly.

Done.

https://codereview.chromium.org/23684033/diff/60001/chromeos/login/login_stat...
chromeos/login/login_state_unittest.cc:58: // Setting login state to SAFE MODE.
On 2013/09/12 00:48:01, stevenjb wrote:
> We should probably test safe mode separately, since normally LoginState won't
go
> -> SAFE -> ACTIVE, i.e. we should test going -> ACTIVE directly.

Done.

[stevenjb, 2013-09-12 17:01:10]

lgtm

[Dmitry Polukhin, 2013-09-12 17:18:43]

lgtm

[pastarmovj, 2013-09-13 10:13:30]

lgtm. I remember we actually shuffled some code back then to make the keystore
start loading in time when in safe mode so I guess this change is the equivalent
in the new keystore world.

[commit-bot: I haz the power, 2013-09-13 18:47:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/23684033/74001

[commit-bot: I haz the power, 2013-09-13 18:56:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tbarzic@chromium.org/23684033/6001

[commit-bot: I haz the power, 2013-09-13 23:36:40]

Change committed as 223168