Commit d349f92c authored by David Roger's avatar David Roger Committed by Commit Bot

[signin] Remove the unconsented primary account more aggressively

When the token for the unconsented primary account (UPA) was revoked, the
cookies information becomes stale, and the UPA was not updated until the
cookies were refreshed.

However, in this case the UPA should be removed immediately.

This CL fixes a crash where sync was expecting the UPA to be immediately
reset when the token is revoked.

Change-Id: I4b68206a93e2faed52045b813aab6e0e5f2bca6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824467
Auto-Submit: David Roger <droger@chromium.org>
Commit-Queue: Monica Basta <msalama@chromium.org>
Reviewed-by: default avatarMonica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699784}
parent 48581e94
...@@ -478,17 +478,34 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const { ...@@ -478,17 +478,34 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const {
return base::nullopt; return base::nullopt;
#else #else
AccountsInCookieJarInfo cookie_info = GetAccountsInCookieJar(); AccountsInCookieJarInfo cookie_info = GetAccountsInCookieJar();
if (!cookie_info.accounts_are_fresh)
return base::nullopt; if (AreRefreshTokensLoaded() && GetAccountsWithRefreshTokens().empty())
return CoreAccountInfo();
std::vector<gaia::ListedAccount> cookie_accounts = std::vector<gaia::ListedAccount> cookie_accounts =
cookie_info.signed_in_accounts; cookie_info.signed_in_accounts;
if (cookie_accounts.empty()) if (cookie_info.accounts_are_fresh && cookie_accounts.empty())
return CoreAccountInfo(); return CoreAccountInfo();
if (!AreRefreshTokensLoaded()) if (!AreRefreshTokensLoaded() || !cookie_info.accounts_are_fresh) {
// If cookies or tokens are not loaded, it is not possible to fully compute
// the unconsented primary account. However, if the current unconsented
// primary account is no longer valid, it has to be removed.
CoreAccountId current_account = GetUnconsentedPrimaryAccountId();
if (!current_account.empty()) {
if (AreRefreshTokensLoaded() &&
!HasAccountWithRefreshToken(current_account)) {
return CoreAccountInfo();
}
if (cookie_info.accounts_are_fresh &&
cookie_accounts[0].id != current_account) {
return CoreAccountInfo();
}
}
return base::nullopt; return base::nullopt;
}
// At this point, cookies and tokens are loaded and neither are empty.
const CoreAccountId first_account_id = cookie_accounts[0].id; const CoreAccountId first_account_id = cookie_accounts[0].id;
if (!HasAccountWithRefreshToken(first_account_id)) if (!HasAccountWithRefreshToken(first_account_id))
return CoreAccountInfo(); return CoreAccountInfo();
......
...@@ -1553,6 +1553,58 @@ TEST_F( ...@@ -1553,6 +1553,58 @@ TEST_F(
empty_info); empty_info);
EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), empty_info); EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), empty_info);
} }
TEST_F(IdentityManagerTest,
UnconsentedPrimaryAccountTokenRevokedWithStaleCookies) {
ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT);
// Add an unconsented primary account, incl. proper cookies.
AccountInfo account_info = MakeAccountAvailableWithCookies(
identity_manager(), test_url_loader_factory(), kTestEmail2, kTestGaiaId2);
EXPECT_EQ(kTestEmail2, account_info.email);
EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(),
account_info);
// Make the cookies stale and remove the account.
signin::SetFreshnessOfAccountsInGaiaCookie(identity_manager(), false);
RemoveRefreshTokenForAccount(identity_manager(), account_info.account_id);
AccountsInCookieJarInfo cookie_info =
identity_manager()->GetAccountsInCookieJar();
ASSERT_FALSE(cookie_info.accounts_are_fresh);
// Unconsented account was removed.
EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(),
CoreAccountInfo());
}
TEST_F(IdentityManagerTest,
UnconsentedPrimaryAccountTokenRevokedWithStaleCookiesMultipleAccounts) {
ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT);
// Add two accounts with cookies.
AccountInfo main_account_info =
MakeAccountAvailable(identity_manager(), kTestEmail2);
AccountInfo secondary_account_info =
MakeAccountAvailable(identity_manager(), kTestEmail3);
SetCookieAccounts(
identity_manager(), test_url_loader_factory(),
{{main_account_info.email, main_account_info.gaia},
{secondary_account_info.email, secondary_account_info.gaia}});
EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(),
main_account_info);
// Make the cookies stale and remove the main account.
signin::SetFreshnessOfAccountsInGaiaCookie(identity_manager(), false);
RemoveRefreshTokenForAccount(identity_manager(),
main_account_info.account_id);
AccountsInCookieJarInfo cookie_info =
identity_manager()->GetAccountsInCookieJar();
ASSERT_FALSE(cookie_info.accounts_are_fresh);
// Unconsented account was removed.
EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(),
CoreAccountInfo());
}
#endif #endif
TEST_F(IdentityManagerTest, CallbackSentOnRefreshTokenRemovalOfUnknownAccount) { TEST_F(IdentityManagerTest, CallbackSentOnRefreshTokenRemovalOfUnknownAccount) {
......
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