Commit 83e19511 authored by zelidrag@chromium.org's avatar zelidrag@chromium.org

Perform /ListAccounts check before session merge to see if there is a need for...

Perform /ListAccounts check before session merge to see if there is a need for session merge at all.

BUG=338543
TEST=manual, added OAuth2Test.PRE_PRE_MergeSession to test existing user with fresh cookies, modified OAuth2Test.PRE_MergeSession to test existing user with stale GAIA cookies

Review URL: https://codereview.chromium.org/148463004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247862 0039d316-1c4b-4281-b951-d872f2087c98
parent ba88a975
......@@ -138,7 +138,14 @@ class OAuth2Test : public OobeBaseTest {
SetupGaiaServerWithAccessTokens();
}
void SetupGaiaServerForExistingAccount() {
void SetupGaiaServerForUnexpiredAccount() {
FakeGaia::MergeSessionParams params;
params.email = kTestAccountId;
fake_gaia_->SetMergeSessionParams(params);
SetupGaiaServerWithAccessTokens();
}
void SetupGaiaServerForExpiredAccount() {
FakeGaia::MergeSessionParams params;
params.gaia_uber_token = kTestGaiaUberToken;
params.session_sid_cookie = kTestSession2SIDCookie;
......@@ -147,9 +154,42 @@ class OAuth2Test : public OobeBaseTest {
SetupGaiaServerWithAccessTokens();
}
void LoginAsExistingUser() {
content::WindowedNotificationObserver(
chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
content::NotificationService::AllSources()).Wait();
JsExpect("!!document.querySelector('#account-picker')");
JsExpect("!!document.querySelector('#pod-row')");
EXPECT_EQ(GetOAuthStatusFromLocalState(kTestAccountId),
User::OAUTH2_TOKEN_STATUS_VALID);
EXPECT_TRUE(TryToLogin(kTestAccountId, kTestAccountPassword));
Profile* profile = ProfileManager::GetPrimaryUserProfile();
// Wait for the session merge to finish.
std::set<OAuth2LoginManager::SessionRestoreState> states;
states.insert(OAuth2LoginManager::SESSION_RESTORE_DONE);
states.insert(OAuth2LoginManager::SESSION_RESTORE_FAILED);
states.insert(OAuth2LoginManager::SESSION_RESTORE_CONNECTION_FAILED);
OAuth2LoginManagerStateWaiter merge_session_waiter(profile);
merge_session_waiter.WaitForStates(states);
EXPECT_EQ(merge_session_waiter.final_state(),
OAuth2LoginManager::SESSION_RESTORE_DONE);
// Check for existance of refresh token.
ProfileOAuth2TokenService* token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(profile);
EXPECT_TRUE(token_service->RefreshTokenIsAvailable(kTestAccountId));
EXPECT_EQ(GetOAuthStatusFromLocalState(kTestAccountId),
User::OAUTH2_TOKEN_STATUS_VALID);
}
bool TryToLogin(const std::string& username,
const std::string& password) {
if (!AddUserTosession(username, password))
if (!AddUserToSession(username, password))
return false;
if (const User* active_user = UserManager::Get()->GetActiveUser())
......@@ -183,7 +223,7 @@ class OAuth2Test : public OobeBaseTest {
return OobeBaseTest::profile();
}
bool AddUserTosession(const std::string& username,
bool AddUserToSession(const std::string& username,
const std::string& password) {
ExistingUserController* controller =
ExistingUserController::current_controller();
......@@ -352,12 +392,8 @@ class CookieReader : public base::RefCountedThreadSafe<CookieReader> {
};
// PRE_MergeSession is testing merge session for a new profile.
IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_MergeSession) {
IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_PRE_MergeSession) {
StartNewUserSession();
// Wait for the session merge to finish.
WaitForMergeSessionCompletion(OAuth2LoginManager::SESSION_RESTORE_DONE);
// Check for existance of refresh token.
ProfileOAuth2TokenService* token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(
......@@ -373,37 +409,33 @@ IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_MergeSession) {
EXPECT_EQ(cookie_reader->GetCookieValue("LSID"), kTestSessionLSIDCookie);
}
// MergeSession test is running merge session process for an existing profile
// that was generated in PRE_PRE_PRE_MergeSession test. In this test, we
// are not running /MergeSession process since the /ListAccounts call confirms
// that the session is not stale.
IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_MergeSession) {
SetupGaiaServerForUnexpiredAccount();
SimulateNetworkOnline();
LoginAsExistingUser();
scoped_refptr<CookieReader> cookie_reader(new CookieReader());
cookie_reader->ReadCookies(profile());
// These are still cookie values form the initial session since
// /ListAccounts
EXPECT_EQ(cookie_reader->GetCookieValue("SID"), kTestSessionSIDCookie);
EXPECT_EQ(cookie_reader->GetCookieValue("LSID"), kTestSessionLSIDCookie);
}
// MergeSession test is running merge session process for an existing profile
// that was generated in PRE_PRE_MergeSession test.
IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_MergeSession) {
SetupGaiaServerForExistingAccount();
SetupGaiaServerForExpiredAccount();
SimulateNetworkOnline();
content::WindowedNotificationObserver(
chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
content::NotificationService::AllSources()).Wait();
JsExpect("!!document.querySelector('#account-picker')");
JsExpect("!!document.querySelector('#pod-row')");
EXPECT_EQ(GetOAuthStatusFromLocalState(kTestAccountId),
User::OAUTH2_TOKEN_STATUS_VALID);
EXPECT_TRUE(TryToLogin(kTestAccountId, kTestAccountPassword));
// Wait for the session merge to finish.
WaitForMergeSessionCompletion(OAuth2LoginManager::SESSION_RESTORE_DONE);
// Check for existance of refresh token.
ProfileOAuth2TokenService* token_service =
ProfileOAuth2TokenServiceFactory::GetForProfile(profile());
EXPECT_TRUE(token_service->RefreshTokenIsAvailable(kTestAccountId));
EXPECT_EQ(GetOAuthStatusFromLocalState(kTestAccountId),
User::OAUTH2_TOKEN_STATUS_VALID);
LoginAsExistingUser();
scoped_refptr<CookieReader> cookie_reader(new CookieReader());
cookie_reader->ReadCookies(profile());
// These should be cookie values that we generated by calling /MergeSession,
// since /ListAccounts should have tell us that the initial session cookies
// are stale.
EXPECT_EQ(cookie_reader->GetCookieValue("SID"), kTestSession2SIDCookie);
EXPECT_EQ(cookie_reader->GetCookieValue("LSID"), kTestSession2LSIDCookie);
}
......
......@@ -99,7 +99,7 @@ void OAuth2LoginManager::RestoreSessionFromSavedTokens() {
const std::string& primary_account_id = GetPrimaryAccountId();
if (token_service->RefreshTokenIsAvailable(primary_account_id)) {
LOG(WARNING) << "OAuth2 refresh token is already loaded.";
RestoreSessionCookies();
VerifySessionCookies();
} else {
LOG(WARNING) << "Loading OAuth2 refresh token from database.";
......@@ -145,7 +145,7 @@ void OAuth2LoginManager::OnRefreshTokenAvailable(
// Token is loaded. Undo the flagging before token loading.
UserManager::Get()->SaveUserOAuthStatus(account_id,
User::OAUTH2_TOKEN_STATUS_VALID);
RestoreSessionCookies();
VerifySessionCookies();
}
}
......@@ -243,20 +243,28 @@ void OAuth2LoginManager::OnOAuth2TokensAvailable(
void OAuth2LoginManager::OnOAuth2TokensFetchFailed() {
LOG(ERROR) << "OAuth2 tokens fetch failed!";
UMA_HISTOGRAM_ENUMERATION("OAuth2Login.SessionRestore",
SESSION_RESTORE_TOKEN_FETCH_FAILED,
SESSION_RESTORE_COUNT);
SetSessionRestoreState(OAuth2LoginManager::SESSION_RESTORE_FAILED);
RecordSessionRestoreOutcome(SESSION_RESTORE_TOKEN_FETCH_FAILED,
SESSION_RESTORE_FAILED);
}
void OAuth2LoginManager::RestoreSessionCookies() {
void OAuth2LoginManager::VerifySessionCookies() {
DCHECK(!login_verifier_.get());
SetSessionRestoreState(SESSION_RESTORE_IN_PROGRESS);
login_verifier_.reset(
new OAuth2LoginVerifier(this,
g_browser_process->system_request_context(),
user_profile_->GetRequestContext(),
oauthlogin_access_token_));
if (restore_strategy_ == RESTORE_FROM_SAVED_OAUTH2_REFRESH_TOKEN) {
login_verifier_->VerifyUserCookies(user_profile_);
return;
}
RestoreSessionCookies();
}
void OAuth2LoginManager::RestoreSessionCookies() {
SetSessionRestoreState(SESSION_RESTORE_IN_PROGRESS);
login_verifier_->VerifyProfileTokens(user_profile_);
}
......@@ -268,25 +276,21 @@ void OAuth2LoginManager::Shutdown() {
void OAuth2LoginManager::OnSessionMergeSuccess() {
VLOG(1) << "OAuth2 refresh and/or GAIA token verification succeeded.";
UMA_HISTOGRAM_ENUMERATION("OAuth2Login.SessionRestore",
SESSION_RESTORE_SUCCESS,
SESSION_RESTORE_COUNT);
SetSessionRestoreState(OAuth2LoginManager::SESSION_RESTORE_DONE);
RecordSessionRestoreOutcome(SESSION_RESTORE_SUCCESS,
SESSION_RESTORE_DONE);
}
void OAuth2LoginManager::OnSessionMergeFailure(bool connection_error) {
LOG(ERROR) << "OAuth2 refresh and GAIA token verification failed!"
<< " connection_error: " << connection_error;
UMA_HISTOGRAM_ENUMERATION("OAuth2Login.SessionRestore",
SESSION_RESTORE_MERGE_SESSION_FAILED,
SESSION_RESTORE_COUNT);
SetSessionRestoreState(connection_error ?
OAuth2LoginManager::SESSION_RESTORE_CONNECTION_FAILED :
OAuth2LoginManager::SESSION_RESTORE_FAILED);
RecordSessionRestoreOutcome(SESSION_RESTORE_MERGE_SESSION_FAILED,
connection_error ?
SESSION_RESTORE_CONNECTION_FAILED :
SESSION_RESTORE_FAILED);
}
void OAuth2LoginManager::OnListAccountsSuccess(const std::string& data) {
PostMergeVerificationOutcome outcome = POST_MERGE_SUCCESS;
MergeVerificationOutcome outcome = POST_MERGE_SUCCESS;
// Let's analyze which accounts we see logged in here:
std::vector<std::string> accounts = gaia::ParseListAccountsData(data);
std::string user_email = gaia::CanonicalizeEmail(GetPrimaryAccountId());
......@@ -312,22 +316,63 @@ void OAuth2LoginManager::OnListAccountsSuccess(const std::string& data) {
outcome = POST_MERGE_NO_ACCOUNTS;
}
RecordPostMergeOutcome(outcome);
bool is_pre_merge = (state_ == SESSION_RESTORE_PREPARING);
RecordCookiesCheckOutcome(is_pre_merge, outcome);
// If the primary account is missing during the initial cookie freshness
// check, try to restore GAIA session cookies form the OAuth2 tokens.
if (is_pre_merge) {
if (outcome != POST_MERGE_SUCCESS &&
outcome != POST_MERGE_PRIMARY_NOT_FIRST_ACCOUNT) {
RestoreSessionCookies();
} else {
// We are done with this account, it's GAIA cookies are legit.
RecordSessionRestoreOutcome(SESSION_RESTORE_NOT_NEEDED,
SESSION_RESTORE_DONE);
}
}
}
void OAuth2LoginManager::OnListAccountsFailure(bool connection_error) {
RecordPostMergeOutcome(connection_error ? POST_MERGE_CONNECTION_FAILED :
POST_MERGE_VERIFICATION_FAILED);
bool is_pre_merge = (state_ == SESSION_RESTORE_PREPARING);
RecordCookiesCheckOutcome(
is_pre_merge,
connection_error ? POST_MERGE_CONNECTION_FAILED :
POST_MERGE_VERIFICATION_FAILED);
if (is_pre_merge) {
if (!connection_error) {
// If we failed to get account list, our cookies might be stale so we
// need to attempt to restore them.
RestoreSessionCookies();
} else {
RecordSessionRestoreOutcome(SESSION_RESTORE_LISTACCOUNTS_FAILED,
SESSION_RESTORE_CONNECTION_FAILED);
}
}
}
// static
void OAuth2LoginManager::RecordPostMergeOutcome(
PostMergeVerificationOutcome outcome) {
UMA_HISTOGRAM_ENUMERATION("OAuth2Login.PostMergeVerification",
void OAuth2LoginManager::RecordSessionRestoreOutcome(
SessionRestoreOutcome outcome,
OAuth2LoginManager::SessionRestoreState state) {
UMA_HISTOGRAM_ENUMERATION("OAuth2Login.SessionRestore",
outcome,
POST_MERGE_COUNT);
SESSION_RESTORE_COUNT);
SetSessionRestoreState(state);
}
// static
void OAuth2LoginManager::RecordCookiesCheckOutcome(
bool is_pre_merge,
MergeVerificationOutcome outcome) {
if (is_pre_merge) {
UMA_HISTOGRAM_ENUMERATION("OAuth2Login.PreMergeVerification",
outcome,
POST_MERGE_COUNT);
} else {
UMA_HISTOGRAM_ENUMERATION("OAuth2Login.PostMergeVerification",
outcome,
POST_MERGE_COUNT);
}
}
void OAuth2LoginManager::SetSessionRestoreState(
OAuth2LoginManager::SessionRestoreState state) {
......
......@@ -115,22 +115,25 @@ class OAuth2LoginManager : public BrowserContextKeyedService,
private:
friend class MergeSessionLoadPageTest;
friend class OAuth2Test;
// Session restore outcomes (for UMA).
enum {
enum SessionRestoreOutcome {
SESSION_RESTORE_UNDEFINED = 0,
SESSION_RESTORE_SUCCESS = 1,
SESSION_RESTORE_TOKEN_FETCH_FAILED = 2,
SESSION_RESTORE_NO_REFRESH_TOKEN_FAILED = 3,
SESSION_RESTORE_OAUTHLOGIN_FAILED = 4,
SESSION_RESTORE_MERGE_SESSION_FAILED = 5,
SESSION_RESTORE_COUNT = SESSION_RESTORE_MERGE_SESSION_FAILED,
SESSION_RESTORE_LISTACCOUNTS_FAILED = 6,
SESSION_RESTORE_NOT_NEEDED = 7,
SESSION_RESTORE_COUNT = 8,
};
// Outcomes of post-merge session verification.
// This enum is used for an UMA histogram, and hence new items should only be
// appended at the end.
enum PostMergeVerificationOutcome {
enum MergeVerificationOutcome {
POST_MERGE_UNDEFINED = 0,
POST_MERGE_SUCCESS = 1,
POST_MERGE_NO_ACCOUNTS = 2,
......@@ -191,6 +194,10 @@ class OAuth2LoginManager : public BrowserContextKeyedService,
// Reports when all tokens are loaded.
void ReportOAuth2TokensLoaded();
// Checks if primary account sessions cookies are stale and restores them
// if needed.
void VerifySessionCookies();
// Issue GAIA cookie recovery (MergeSession) from |refresh_token_|.
void RestoreSessionCookies();
......@@ -204,8 +211,15 @@ class OAuth2LoginManager : public BrowserContextKeyedService,
// Testing helper.
void SetSessionRestoreStartForTesting(const base::Time& time);
// Records |outcome| of post merge verification check.
static void RecordPostMergeOutcome(PostMergeVerificationOutcome outcome);
// Records |outcome| of session restore process and sets new |state|.
void RecordSessionRestoreOutcome(SessionRestoreOutcome outcome,
SessionRestoreState state);
// Records |outcome| of merge verification check. |is_pre_merge| specifies
// if this is pre or post merge session verification.
static void RecordCookiesCheckOutcome(
bool is_pre_merge,
MergeVerificationOutcome outcome);
// Keeps the track if we have already reported OAuth2 token being loaded
// by OAuth2TokenService.
......
......@@ -68,26 +68,24 @@ OAuth2LoginVerifier::OAuth2LoginVerifier(
OAuth2LoginVerifier::~OAuth2LoginVerifier() {
}
void OAuth2LoginVerifier::VerifyUserCookies(Profile* profile) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (DelayNetworkCall(base::Bind(&OAuth2LoginVerifier::VerifyUserCookies,
AsWeakPtr(),
profile))) {
return;
}
StartAuthCookiesVerification();
}
void OAuth2LoginVerifier::VerifyProfileTokens(Profile* profile) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
// Delay the verification if the network is not connected or on a captive
// portal.
const NetworkState* default_network =
NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
NetworkPortalDetector* detector = NetworkPortalDetector::Get();
if (!default_network ||
default_network->connection_state() == shill::kStatePortal ||
(detector && detector->GetCaptivePortalState(default_network).status !=
NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE)) {
// If network is offline, defer the token fetching until online.
LOG(WARNING) << "Network is offline. Deferring OAuth2 access token fetch.";
BrowserThread::PostDelayedTask(
BrowserThread::UI,
FROM_HERE,
base::Bind(
&OAuth2LoginVerifier::VerifyProfileTokens, AsWeakPtr(), profile),
base::TimeDelta::FromMilliseconds(kRequestRestartDelay));
if (DelayNetworkCall(base::Bind(&OAuth2LoginVerifier::VerifyProfileTokens,
AsWeakPtr(),
profile))) {
return;
}
......@@ -169,11 +167,11 @@ void OAuth2LoginVerifier::SchedulePostMergeVerification() {
BrowserThread::UI,
FROM_HERE,
base::Bind(
&OAuth2LoginVerifier::StartPostRestoreVerification, AsWeakPtr()),
&OAuth2LoginVerifier::StartAuthCookiesVerification, AsWeakPtr()),
base::TimeDelta::FromMilliseconds(kPostResoreVerificationDelay));
}
void OAuth2LoginVerifier::StartPostRestoreVerification() {
void OAuth2LoginVerifier::StartAuthCookiesVerification() {
gaia_fetcher_.reset(
new GaiaAuthFetcher(this,
std::string(GaiaConstants::kChromeOSSource),
......@@ -239,7 +237,7 @@ void OAuth2LoginVerifier::OnListAccountsFailure(
RetryOnError(
"ListAccounts",
error,
base::Bind(&OAuth2LoginVerifier::StartPostRestoreVerification,
base::Bind(&OAuth2LoginVerifier::StartAuthCookiesVerification,
AsWeakPtr()),
base::Bind(&Delegate::OnListAccountsFailure,
base::Unretained(delegate_)));
......@@ -272,4 +270,27 @@ void OAuth2LoginVerifier::RetryOnError(const char* operation_id,
error_handler.Run(IsConnectionOrServiceError(error));
}
bool OAuth2LoginVerifier::DelayNetworkCall(const base::Closure& callback) {
// Delay the verification if the network is not connected or on a captive
// portal.
const NetworkState* default_network =
NetworkHandler::Get()->network_state_handler()->DefaultNetwork();
NetworkPortalDetector* detector = NetworkPortalDetector::Get();
if (!default_network ||
default_network->connection_state() == shill::kStatePortal ||
(detector && detector->GetCaptivePortalState(default_network).status !=
NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_ONLINE)) {
// If network is offline, defer the token fetching until online.
LOG(WARNING) << "Network is offline. Deferring call.";
BrowserThread::PostDelayedTask(
BrowserThread::UI,
FROM_HERE,
callback,
base::TimeDelta::FromMilliseconds(kRequestRestartDelay));
return true;
}
return false;
}
} // namespace chromeos
......@@ -53,6 +53,9 @@ class OAuth2LoginVerifier : public base::SupportsWeakPtr<OAuth2LoginVerifier>,
const std::string& oauthlogin_access_token);
virtual ~OAuth2LoginVerifier();
// Initiates verification of GAIA cookies in |profile|'s cookie jar.
void VerifyUserCookies(Profile* profile);
// Attempts to restore session from OAuth2 refresh token minting all necesarry
// tokens along the way (OAuth2 access token, SID/LSID, GAIA service token).
void VerifyProfileTokens(Profile* profile);
......@@ -94,8 +97,8 @@ class OAuth2LoginVerifier : public base::SupportsWeakPtr<OAuth2LoginVerifier>,
// hasn't stumped over SID/LSID.
void SchedulePostMergeVerification();
// Starts post merge request verification.
void StartPostRestoreVerification();
// Starts GAIA auth cookies (SID/LSID) verification.
void StartAuthCookiesVerification();
// Decides how to proceed on GAIA |error|. If the error looks temporary,
// retries |task| after certain delay until max retry count is reached.
......@@ -104,6 +107,10 @@ class OAuth2LoginVerifier : public base::SupportsWeakPtr<OAuth2LoginVerifier>,
const base::Closure& task_to_retry,
const ErrorHandler& error_handler);
// Delays operation defined with |callback| based on the current networking
// conditions.
bool DelayNetworkCall(const base::Closure& callback);
OAuth2LoginVerifier::Delegate* delegate_;
scoped_refptr<net::URLRequestContextGetter> system_request_context_;
scoped_refptr<net::URLRequestContextGetter> user_request_context_;
......
......@@ -12238,12 +12238,21 @@ other types of suffix sets.
<histogram name="OAuth2Login.PostMergeVerification"
enum="PostMergeVerificationOutcome">
<summary>
Outcome of Chrome OS GAIA cookie post merge session verification process. It
Outcome of Chrome OS GAIA cookie post-merge session verification process. It
measures how often /MergeSession request collided with browser session
restore process resulting in partially authenticated primary GAIA session.
</summary>
</histogram>
<histogram name="OAuth2Login.PreMergeVerification"
enum="PostMergeVerificationOutcome">
<summary>
Outcome of Chrome OS GAIA cookie pre-merge session verification process. It
measures how often we need to perform /MergeSession request to
re-authenticated exisitng user with GAIA.
</summary>
</histogram>
<histogram name="OAuth2Login.SessionRestore" enum="GaiaSessionRestoreOutcome">
<summary>Outcome of Chrome OS GAIA cookie session restore process.</summary>
</histogram>
......@@ -25872,12 +25881,15 @@ other types of suffix sets.
</enum>
<enum name="GaiaSessionRestoreOutcome" type="int">
<int value="0" label="UNDEFINED"/>
<int value="1" label="SUCCESS"/>
<int value="2" label="RESTORE_TOKEN_FETCH_FAILED"/>
<int value="3" label="NO_REFRESH_TOKEN_FAILED"/>
<int value="4" label="OAUTHLOGIN_FAILED"/>
<int value="5" label="MERGESESSION_FAILED"/>
<int value="0" label="Undefined"/>
<int value="1" label="Success"/>
<int value="2" label="OAuth2 tokens cannot be fetched"/>
<int value="3" label="No local OAuth2 refresh token found"/>
<int value="4" label="OAuthLogin call failed"/>
<int value="5" label="MergeSession call failed"/>
<int value="6" label="ListAccounts call failed"/>
<int value="7" label="No restore needed, fresh cookies found"/>
<int value="8" label="Overflow"/>
</enum>
<enum name="GDataAuthResult" type="int">
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