Commit 9417ee25 authored by Jialiu Lin's avatar Jialiu Lin Committed by Commit Bot

Update advanced protection status upon Chrome OS online sign-in

If an OAuth2 token is fetched during Chrome OS sign-in, we update
the advanced protection status accordingly for the primary AccountInfo
(in AccountTrackerService).

Bug: 866620, 882576
Change-Id: I729a7c47b8b3e45589c03df0fe8b8c9ab7c4084e
Reviewed-on: https://chromium-review.googlesource.com/1205107Reviewed-by: default avatarRoger Tawa <rogerta@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Jialiu Lin <jialiul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590427}
parent 8cd1d176
...@@ -374,6 +374,11 @@ UserSessionManager::RlzInitParams CollectRlzParams() { ...@@ -374,6 +374,11 @@ UserSessionManager::RlzInitParams CollectRlzParams() {
} }
#endif #endif
bool IsOnlineSignin(const UserContext& user_context) {
return user_context.GetAuthFlow() == UserContext::AUTH_FLOW_GAIA_WITH_SAML ||
user_context.GetAuthFlow() == UserContext::AUTH_FLOW_GAIA_WITHOUT_SAML;
}
} // namespace } // namespace
UserSessionManagerDelegate::~UserSessionManagerDelegate() {} UserSessionManagerDelegate::~UserSessionManagerDelegate() {}
...@@ -966,10 +971,8 @@ void UserSessionManager::OnSessionRestoreStateChanged( ...@@ -966,10 +971,8 @@ void UserSessionManager::OnSessionRestoreStateChanged(
// Otherwise, auth token dependent code would be in an invalid state. // Otherwise, auth token dependent code would be in an invalid state.
// Important piece such as policy code might be broken because of this and // Important piece such as policy code might be broken because of this and
// subject to an exploit. See http://crbug.com/677312. // subject to an exploit. See http://crbug.com/677312.
const bool is_online_signin = if (IsOnlineSignin(user_context_) &&
user_context_.GetAuthFlow() == UserContext::AUTH_FLOW_GAIA_WITH_SAML || state == OAuth2LoginManager::SESSION_RESTORE_FAILED) {
user_context_.GetAuthFlow() == UserContext::AUTH_FLOW_GAIA_WITHOUT_SAML;
if (is_online_signin && state == OAuth2LoginManager::SESSION_RESTORE_FAILED) {
LOG(ERROR) LOG(ERROR)
<< "Session restore failed for online sign-in, terminating session."; << "Session restore failed for online sign-in, terminating session.";
chrome::AttemptUserExit(); chrome::AttemptUserExit();
...@@ -1235,12 +1238,18 @@ void UserSessionManager::InitProfilePreferences( ...@@ -1235,12 +1238,18 @@ void UserSessionManager::InitProfilePreferences(
bool is_child = user->GetType() == user_manager::USER_TYPE_CHILD; bool is_child = user->GetType() == user_manager::USER_TYPE_CHILD;
DCHECK(is_child == DCHECK(is_child ==
(user_context.GetUserType() == user_manager::USER_TYPE_CHILD)); (user_context.GetUserType() == user_manager::USER_TYPE_CHILD));
AccountTrackerServiceFactory::GetForProfile(profile)->SetIsChildAccount( AccountTrackerService* account_tracker =
account_id, is_child); AccountTrackerServiceFactory::GetForProfile(profile);
account_tracker->SetIsChildAccount(account_id, is_child);
VLOG(1) << "Seed IdentityManager and SigninManagerBase with the " VLOG(1) << "Seed IdentityManager and SigninManagerBase with the "
<< "authenticated account info, success=" << "authenticated account info, success="
<< SigninManagerFactory::GetForProfile(profile)->IsAuthenticated(); << SigninManagerFactory::GetForProfile(profile)->IsAuthenticated();
if (IsOnlineSignin(user_context)) {
account_tracker->SetIsAdvancedProtectionAccount(
account_id, user_context.IsUnderAdvancedProtection());
}
// Backfill GAIA ID in user prefs stored in Local State. // Backfill GAIA ID in user prefs stored in Local State.
std::string tmp_gaia_id; std::string tmp_gaia_id;
if (!user_manager::known_user::FindGaiaID(user_context.GetAccountId(), if (!user_manager::known_user::FindGaiaID(user_context.GetAccountId(),
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "chrome/browser/chromeos/login/wizard_controller.h" #include "chrome/browser/chromeos/login/wizard_controller.h"
#include "chrome/browser/extensions/chrome_extension_test_notification_observer.h" #include "chrome/browser/extensions/chrome_extension_test_notification_observer.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/signin/account_tracker_service_factory.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h" #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "chrome/browser/signin/signin_error_controller_factory.h" #include "chrome/browser/signin/signin_error_controller_factory.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
...@@ -83,6 +84,14 @@ const char kTestSessionSIDCookie[] = "fake-session-SID-cookie"; ...@@ -83,6 +84,14 @@ const char kTestSessionSIDCookie[] = "fake-session-SID-cookie";
const char kTestSessionLSIDCookie[] = "fake-session-LSID-cookie"; const char kTestSessionLSIDCookie[] = "fake-session-LSID-cookie";
const char kTestSession2SIDCookie[] = "fake-session2-SID-cookie"; const char kTestSession2SIDCookie[] = "fake-session2-SID-cookie";
const char kTestSession2LSIDCookie[] = "fake-session2-LSID-cookie"; const char kTestSession2LSIDCookie[] = "fake-session2-LSID-cookie";
const char kTestIdTokenAdvancedProtectionEnabled[] =
"dummy-header."
"eyAic2VydmljZXMiOiBbInRpYSJdIH0=" // payload: { "services": ["tia"] }
".dummy-signature";
const char kTestIdTokenAdvancedProtectionDisabled[] =
"dummy-header."
"eyAic2VydmljZXMiOiBbXSB9" // payload: { "services": [] }
".dummy-signature";
std::string PickAccountId(Profile* profile, std::string PickAccountId(Profile* profile,
const std::string& gaia_id, const std::string& gaia_id,
...@@ -230,7 +239,7 @@ class OAuth2Test : public OobeBaseTest { ...@@ -230,7 +239,7 @@ class OAuth2Test : public OobeBaseTest {
base::Bind(&OAuth2Test::InterceptRequest, base::Unretained(this))); base::Bind(&OAuth2Test::InterceptRequest, base::Unretained(this)));
} }
void SetupGaiaServerForNewAccount() { void SetupGaiaServerForNewAccount(bool is_under_advanced_protection) {
FakeGaia::MergeSessionParams params; FakeGaia::MergeSessionParams params;
params.auth_sid_cookie = kTestAuthSIDCookie; params.auth_sid_cookie = kTestAuthSIDCookie;
params.auth_lsid_cookie = kTestAuthLSIDCookie; params.auth_lsid_cookie = kTestAuthLSIDCookie;
...@@ -240,6 +249,9 @@ class OAuth2Test : public OobeBaseTest { ...@@ -240,6 +249,9 @@ class OAuth2Test : public OobeBaseTest {
params.gaia_uber_token = kTestGaiaUberToken; params.gaia_uber_token = kTestGaiaUberToken;
params.session_sid_cookie = kTestSessionSIDCookie; params.session_sid_cookie = kTestSessionSIDCookie;
params.session_lsid_cookie = kTestSessionLSIDCookie; params.session_lsid_cookie = kTestSessionLSIDCookie;
params.id_token = is_under_advanced_protection
? kTestIdTokenAdvancedProtectionEnabled
: kTestIdTokenAdvancedProtectionDisabled;
fake_gaia_->SetMergeSessionParams(params); fake_gaia_->SetMergeSessionParams(params);
SetupFakeGaiaForLogin(kTestEmail, kTestGaiaId, kTestRefreshToken); SetupFakeGaiaForLogin(kTestEmail, kTestGaiaId, kTestRefreshToken);
} }
...@@ -375,8 +387,9 @@ class OAuth2Test : public OobeBaseTest { ...@@ -375,8 +387,9 @@ class OAuth2Test : public OobeBaseTest {
EXPECT_EQ(merge_session_waiter.final_state(), final_state); EXPECT_EQ(merge_session_waiter.final_state(), final_state);
} }
void StartNewUserSession(bool wait_for_merge) { void StartNewUserSession(bool wait_for_merge,
SetupGaiaServerForNewAccount(); bool is_under_advanced_protection) {
SetupGaiaServerForNewAccount(is_under_advanced_protection);
SimulateNetworkOnline(); SimulateNetworkOnline();
WaitForGaiaPageLoad(); WaitForGaiaPageLoad();
...@@ -479,7 +492,8 @@ class CookieReader : public base::RefCountedThreadSafe<CookieReader> { ...@@ -479,7 +492,8 @@ class CookieReader : public base::RefCountedThreadSafe<CookieReader> {
// PRE_MergeSession is testing merge session for a new profile. // PRE_MergeSession is testing merge session for a new profile.
IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_PRE_MergeSession) { IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_PRE_MergeSession) {
StartNewUserSession(true); StartNewUserSession(/*wait_for_merge=*/true,
/*is_under_advanced_protectionis_true=*/false);
// Check for existence of refresh token. // Check for existence of refresh token.
std::string account_id = PickAccountId(profile(), kTestGaiaId, kTestEmail); std::string account_id = PickAccountId(profile(), kTestGaiaId, kTestEmail);
ProfileOAuth2TokenService* token_service = ProfileOAuth2TokenService* token_service =
...@@ -558,7 +572,8 @@ IN_PROC_BROWSER_TEST_F(OAuth2Test, DISABLED_MergeSession) { ...@@ -558,7 +572,8 @@ IN_PROC_BROWSER_TEST_F(OAuth2Test, DISABLED_MergeSession) {
// Sets up a new user with stored refresh token. // Sets up a new user with stored refresh token.
IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_OverlappingContinueSessionRestore) { IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_OverlappingContinueSessionRestore) {
StartNewUserSession(true); StartNewUserSession(/*wait_for_merge=*/true,
/*is_under_advanced_protectionis_true=*/false);
} }
// Tests that ContinueSessionRestore could be called multiple times. // Tests that ContinueSessionRestore could be called multiple times.
...@@ -642,9 +657,33 @@ IN_PROC_BROWSER_TEST_F(OAuth2Test, TerminateOnBadMergeSessionAfterOnlineAuth) { ...@@ -642,9 +657,33 @@ IN_PROC_BROWSER_TEST_F(OAuth2Test, TerminateOnBadMergeSessionAfterOnlineAuth) {
WaitForMergeSessionCompletion(OAuth2LoginManager::SESSION_RESTORE_FAILED); WaitForMergeSessionCompletion(OAuth2LoginManager::SESSION_RESTORE_FAILED);
} }
IN_PROC_BROWSER_TEST_F(OAuth2Test, VerifyInAdvancedProtectionAfterOnlineAuth) {
StartNewUserSession(/*wait_for_merge=*/true,
/*is_under_advanced_protectionis_true=*/true);
// Verify that AccountInfo is properly updated.
AccountTrackerService* account_tracker =
AccountTrackerServiceFactory::GetInstance()->GetForProfile(profile());
EXPECT_TRUE(
account_tracker->GetAccountInfo(kTestEmail).is_under_advanced_protection);
}
IN_PROC_BROWSER_TEST_F(OAuth2Test,
VerifyNotInAdvancedProtectionAfterOnlineAuth) {
StartNewUserSession(/*wait_for_merge=*/true,
/*is_under_advanced_protectionis_true=*/false);
// Verify that AccountInfo is properly updated.
AccountTrackerService* account_tracker =
AccountTrackerServiceFactory::GetInstance()->GetForProfile(profile());
EXPECT_FALSE(
account_tracker->GetAccountInfo(kTestEmail).is_under_advanced_protection);
}
// Sets up a new user with stored refresh token. // Sets up a new user with stored refresh token.
IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_SetInvalidTokenStatus) { IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_SetInvalidTokenStatus) {
StartNewUserSession(true); StartNewUserSession(/*wait_for_merge=*/true,
/*is_under_advanced_protectionis_true=*/false);
} }
// Tests that an auth error reported by SigninErrorController marks invalid auth // Tests that an auth error reported by SigninErrorController marks invalid auth
...@@ -861,7 +900,8 @@ Browser* FindOrCreateVisibleBrowser(Profile* profile) { ...@@ -861,7 +900,8 @@ Browser* FindOrCreateVisibleBrowser(Profile* profile) {
} }
IN_PROC_BROWSER_TEST_F(MergeSessionTest, PageThrottle) { IN_PROC_BROWSER_TEST_F(MergeSessionTest, PageThrottle) {
StartNewUserSession(false); StartNewUserSession(/*wait_for_merge=*/false,
/*is_under_advanced_protectionis_true=*/false);
// Try to open a page from google.com. // Try to open a page from google.com.
Browser* browser = FindOrCreateVisibleBrowser(profile()); Browser* browser = FindOrCreateVisibleBrowser(profile());
...@@ -907,7 +947,8 @@ IN_PROC_BROWSER_TEST_F(MergeSessionTest, PageThrottle) { ...@@ -907,7 +947,8 @@ IN_PROC_BROWSER_TEST_F(MergeSessionTest, PageThrottle) {
} }
IN_PROC_BROWSER_TEST_F(MergeSessionTest, XHRThrottle) { IN_PROC_BROWSER_TEST_F(MergeSessionTest, XHRThrottle) {
StartNewUserSession(false); StartNewUserSession(/*wait_for_merge=*/false,
/*is_under_advanced_protectionis_true=*/false);
// Wait until we get send merge session request. // Wait until we get send merge session request.
WaitForMergeSessionToStart(); WaitForMergeSessionToStart();
......
...@@ -35,6 +35,8 @@ void OAuth2TokenInitializer::OnOAuth2TokensAvailable( ...@@ -35,6 +35,8 @@ void OAuth2TokenInitializer::OnOAuth2TokensAvailable(
user_context_.SetAuthCode(std::string()); user_context_.SetAuthCode(std::string());
user_context_.SetRefreshToken(result.refresh_token); user_context_.SetRefreshToken(result.refresh_token);
user_context_.SetAccessToken(result.access_token); user_context_.SetAccessToken(result.access_token);
user_context_.SetIsUnderAdvancedProtection(
result.is_under_advanced_protection);
const bool support_usm = const bool support_usm =
base::FeatureList::IsEnabled(features::kCrOSEnableUSMUserService); base::FeatureList::IsEnabled(features::kCrOSEnableUSMUserService);
......
...@@ -143,6 +143,10 @@ bool UserContext::HasCredentials() const { ...@@ -143,6 +143,10 @@ bool UserContext::HasCredentials() const {
!auth_code_.empty(); !auth_code_.empty();
} }
bool UserContext::IsUnderAdvancedProtection() const {
return is_under_advanced_protection_;
}
void UserContext::SetAccountId(const AccountId& account_id) { void UserContext::SetAccountId(const AccountId& account_id) {
account_id_ = account_id; account_id_ = account_id;
} }
...@@ -208,6 +212,11 @@ void UserContext::SetSyncPasswordData( ...@@ -208,6 +212,11 @@ void UserContext::SetSyncPasswordData(
sync_password_data_ = {sync_password_data}; sync_password_data_ = {sync_password_data};
} }
void UserContext::SetIsUnderAdvancedProtection(
bool is_under_advanced_protection) {
is_under_advanced_protection_ = is_under_advanced_protection;
}
void UserContext::ClearSecrets() { void UserContext::ClearSecrets() {
key_.ClearSecret(); key_.ClearSecret();
password_key_.ClearSecret(); password_key_.ClearSecret();
......
...@@ -88,6 +88,9 @@ class CHROMEOS_EXPORT UserContext { ...@@ -88,6 +88,9 @@ class CHROMEOS_EXPORT UserContext {
bool HasCredentials() const; bool HasCredentials() const;
// If this user is under advanced protection.
bool IsUnderAdvancedProtection() const;
void SetAccountId(const AccountId& account_id); void SetAccountId(const AccountId& account_id);
void SetKey(const Key& key); void SetKey(const Key& key);
void SetPasswordKey(const Key& key); void SetPasswordKey(const Key& key);
...@@ -105,6 +108,7 @@ class CHROMEOS_EXPORT UserContext { ...@@ -105,6 +108,7 @@ class CHROMEOS_EXPORT UserContext {
void SetGAPSCookie(const std::string& gaps_cookie); void SetGAPSCookie(const std::string& gaps_cookie);
void SetSyncPasswordData( void SetSyncPasswordData(
const password_manager::PasswordHashData& sync_password_data); const password_manager::PasswordHashData& sync_password_data);
void SetIsUnderAdvancedProtection(bool is_under_advanced_protection);
void ClearSecrets(); void ClearSecrets();
...@@ -126,6 +130,7 @@ class CHROMEOS_EXPORT UserContext { ...@@ -126,6 +130,7 @@ class CHROMEOS_EXPORT UserContext {
std::string public_session_input_method_; std::string public_session_input_method_;
std::string device_id_; std::string device_id_;
std::string gaps_cookie_; std::string gaps_cookie_;
bool is_under_advanced_protection_ = false;
// For password reuse detection use. // For password reuse detection use.
base::Optional<password_manager::PasswordHashData> sync_password_data_; base::Optional<password_manager::PasswordHashData> sync_password_data_;
......
...@@ -730,6 +730,7 @@ void FakeGaia::HandleAuthToken(const HttpRequest& request, ...@@ -730,6 +730,7 @@ void FakeGaia::HandleAuthToken(const HttpRequest& request,
base::DictionaryValue response_dict; base::DictionaryValue response_dict;
response_dict.SetString("access_token", token_info->token); response_dict.SetString("access_token", token_info->token);
response_dict.SetInteger("expires_in", 3600); response_dict.SetInteger("expires_in", 3600);
response_dict.SetString("id_token", token_info->id_token);
FormatJSONResponse(response_dict, http_response); FormatJSONResponse(response_dict, http_response);
return; return;
} }
...@@ -759,6 +760,7 @@ void FakeGaia::HandleTokenInfo(const HttpRequest& request, ...@@ -759,6 +760,7 @@ void FakeGaia::HandleTokenInfo(const HttpRequest& request,
response_dict.SetString("scope", base::JoinString(scope_vector, " ")); response_dict.SetString("scope", base::JoinString(scope_vector, " "));
response_dict.SetInteger("expires_in", token_info->expires_in); response_dict.SetInteger("expires_in", token_info->expires_in);
response_dict.SetString("email", token_info->email); response_dict.SetString("email", token_info->email);
response_dict.SetString("id_token", token_info->id_token);
FormatJSONResponse(response_dict, http_response); FormatJSONResponse(response_dict, http_response);
} else { } else {
http_response->set_code(net::HTTP_BAD_REQUEST); http_response->set_code(net::HTTP_BAD_REQUEST);
...@@ -781,6 +783,7 @@ void FakeGaia::HandleIssueToken(const HttpRequest& request, ...@@ -781,6 +783,7 @@ void FakeGaia::HandleIssueToken(const HttpRequest& request,
response_dict.SetString("expiresIn", response_dict.SetString("expiresIn",
base::IntToString(token_info->expires_in)); base::IntToString(token_info->expires_in));
response_dict.SetString("token", token_info->token); response_dict.SetString("token", token_info->token);
response_dict.SetString("id_token", token_info->id_token);
FormatJSONResponse(response_dict, http_response); FormatJSONResponse(response_dict, http_response);
return; return;
} }
...@@ -819,6 +822,7 @@ void FakeGaia::HandleOAuthUserInfo( ...@@ -819,6 +822,7 @@ void FakeGaia::HandleOAuthUserInfo(
response_dict.SetString("id", GetGaiaIdOfEmail(token_info->email)); response_dict.SetString("id", GetGaiaIdOfEmail(token_info->email));
response_dict.SetString("email", token_info->email); response_dict.SetString("email", token_info->email);
response_dict.SetString("verified_email", token_info->email); response_dict.SetString("verified_email", token_info->email);
response_dict.SetString("id_token", token_info->id_token);
FormatJSONResponse(response_dict, http_response); FormatJSONResponse(response_dict, http_response);
} else { } else {
http_response->set_code(net::HTTP_BAD_REQUEST); http_response->set_code(net::HTTP_BAD_REQUEST);
......
...@@ -49,6 +49,7 @@ class FakeGaia { ...@@ -49,6 +49,7 @@ class FakeGaia {
std::string email; std::string email;
// When set to true, any scope set for issue token request matches |this|. // When set to true, any scope set for issue token request matches |this|.
bool any_scope = false; bool any_scope = false;
std::string id_token;
}; };
// Cookies and tokens for /MergeSession call seqeunce. // Cookies and tokens for /MergeSession call seqeunce.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment