Commit da3c8b83 authored by Boris Sazonov's avatar Boris Sazonov Committed by Commit Bot

[Signin][Android] Trigger sign-out when account cookies are cleared

Adds logic to trigger sign-out if account cookies are cleared while
there's only an unconsented primary account.

Bug: 1095112
Change-Id: Iaa65f31ef8da6759cd9afaed2ce9dd6f16e511d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410875
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808561}
parent 5f3c3822
...@@ -726,6 +726,19 @@ public class SigninManager ...@@ -726,6 +726,19 @@ public class SigninManager
notifySignInAllowedChanged(); notifySignInAllowedChanged();
} }
@Override
public void onAccountsCookieDeletedByUserAction() {
// No need to sign out if the user is already signed out.
if (mIdentityManager.getPrimaryAccountInfo(ConsentLevel.NOT_REQUIRED) == null) return;
// If the user consented for sync, then the user should not be signed out.
// Account cookies will be rebuilt by the account reconcilor.
if (mIdentityManager.getPrimaryAccountInfo(ConsentLevel.SYNC) != null) return;
// Clearing account cookies should also sign the user out if the user was not syncing.
signOut(SignoutReason.USER_DELETED_ACCOUNT_COOKIES);
}
/** /**
* Verifies if the account is managed. Callback may be called either * Verifies if the account is managed. Callback may be called either
* synchronously or asynchronously depending on the availability of the * synchronously or asynchronously depending on the availability of the
......
...@@ -36,10 +36,12 @@ import org.chromium.chrome.browser.sync.AndroidSyncSettings; ...@@ -36,10 +36,12 @@ import org.chromium.chrome.browser.sync.AndroidSyncSettings;
import org.chromium.components.signin.AccountTrackerService; import org.chromium.components.signin.AccountTrackerService;
import org.chromium.components.signin.base.CoreAccountId; import org.chromium.components.signin.base.CoreAccountId;
import org.chromium.components.signin.base.CoreAccountInfo; import org.chromium.components.signin.base.CoreAccountInfo;
import org.chromium.components.signin.identitymanager.ClearAccountsAction;
import org.chromium.components.signin.identitymanager.ConsentLevel; import org.chromium.components.signin.identitymanager.ConsentLevel;
import org.chromium.components.signin.identitymanager.IdentityManager; import org.chromium.components.signin.identitymanager.IdentityManager;
import org.chromium.components.signin.identitymanager.IdentityMutator; import org.chromium.components.signin.identitymanager.IdentityMutator;
import org.chromium.components.signin.metrics.SigninAccessPoint; import org.chromium.components.signin.metrics.SigninAccessPoint;
import org.chromium.components.signin.metrics.SignoutDelete;
import org.chromium.components.signin.metrics.SignoutReason; import org.chromium.components.signin.metrics.SignoutReason;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
...@@ -236,6 +238,36 @@ public class SigninManagerTest { ...@@ -236,6 +238,36 @@ public class SigninManagerTest {
verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any()); verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any());
} }
@Test
public void clearingAccountCookiesTriggersSignout() {
// Stub out various native calls. Some of these are verified as never called
// and those stubs simply allow that verification to catch any issues.
doNothing().when(mNativeMock).wipeProfileData(anyLong(), any());
doNothing().when(mNativeMock).wipeGoogleServiceWorkerCaches(anyLong(), any());
// Clearing cookies shouldn't do anything when there's no primary account.
doReturn(null).when(mIdentityManager).getPrimaryAccountInfo(anyInt());
mIdentityManager.onAccountsCookieDeletedByUserAction();
verify(mIdentityMutator, never()).clearPrimaryAccount(anyInt(), anyInt(), anyInt());
// Clearing cookies shouldn't do anything when there's sync account.
doReturn(mAccount).when(mIdentityManager).getPrimaryAccountInfo(anyInt());
mIdentityManager.onAccountsCookieDeletedByUserAction();
verify(mIdentityMutator, never()).clearPrimaryAccount(anyInt(), anyInt(), anyInt());
// Clearing cookies when there's only an unconsented account should trigger sign-out.
doReturn(mAccount).when(mIdentityManager).getPrimaryAccountInfo(ConsentLevel.NOT_REQUIRED);
doReturn(null).when(mIdentityManager).getPrimaryAccountInfo(ConsentLevel.SYNC);
mIdentityManager.onAccountsCookieDeletedByUserAction();
verify(mIdentityMutator)
.clearPrimaryAccount(ClearAccountsAction.DEFAULT,
SignoutReason.USER_DELETED_ACCOUNT_COOKIES, SignoutDelete.IGNORE_METRIC);
// Sign-out triggered by wiping account cookies shouldn't wipe data.
verify(mNativeMock, never()).wipeProfileData(anyLong(), any());
verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any());
}
@Test @Test
public void callbackNotifiedWhenNoOperationIsInProgress() { public void callbackNotifiedWhenNoOperationIsInProgress() {
assertFalse(mSigninManager.isOperationInProgress()); assertFalse(mSigninManager.isOperationInProgress());
......
...@@ -245,6 +245,12 @@ bool IsSignoutDisallowedByPolicy( ...@@ -245,6 +245,12 @@ bool IsSignoutDisallowedByPolicy(
case signin_metrics::ProfileSignout::FORCE_SIGNOUT_ALWAYS_ALLOWED_FOR_TEST: case signin_metrics::ProfileSignout::FORCE_SIGNOUT_ALWAYS_ALLOWED_FOR_TEST:
// Allow signout for tests that want to force it. // Allow signout for tests that want to force it.
return false; return false;
case signin_metrics::ProfileSignout::USER_DELETED_ACCOUNT_COOKIES:
// There's no special-casing for this in ChromeSigninClient, as this only
// happens when there's no sync account and policies aren't enforced.
// PrimaryAccountManager won't actually invoke PreSignOut in this case,
// thus it is fine for ChromeSigninClient to not have any special-casing.
return true;
case signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS: case signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS:
NOTREACHED(); NOTREACHED();
return false; return false;
...@@ -317,6 +323,7 @@ const signin_metrics::ProfileSignout kSignoutSources[] = { ...@@ -317,6 +323,7 @@ const signin_metrics::ProfileSignout kSignoutSources[] = {
signin_metrics::ProfileSignout::ACCOUNT_REMOVED_FROM_DEVICE, signin_metrics::ProfileSignout::ACCOUNT_REMOVED_FROM_DEVICE,
signin_metrics::ProfileSignout::SIGNIN_NOT_ALLOWED_ON_PROFILE_INIT, signin_metrics::ProfileSignout::SIGNIN_NOT_ALLOWED_ON_PROFILE_INIT,
signin_metrics::ProfileSignout::FORCE_SIGNOUT_ALWAYS_ALLOWED_FOR_TEST, signin_metrics::ProfileSignout::FORCE_SIGNOUT_ALWAYS_ALLOWED_FOR_TEST,
signin_metrics::ProfileSignout::USER_DELETED_ACCOUNT_COOKIES,
}; };
static_assert(base::size(kSignoutSources) == static_assert(base::size(kSignoutSources) ==
signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS, signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS,
......
...@@ -118,7 +118,8 @@ public class IdentityManager { ...@@ -118,7 +118,8 @@ public class IdentityManager {
} }
@CalledByNative @CalledByNative
private void onAccountsCookieDeletedByUserAction() { @VisibleForTesting
public void onAccountsCookieDeletedByUserAction() {
for (Observer observer : mObservers) { for (Observer observer : mObservers) {
observer.onAccountsCookieDeletedByUserAction(); observer.onAccountsCookieDeletedByUserAction();
} }
......
...@@ -59,6 +59,9 @@ enum ProfileSignout : int { ...@@ -59,6 +59,9 @@ enum ProfileSignout : int {
SIGNIN_NOT_ALLOWED_ON_PROFILE_INIT, SIGNIN_NOT_ALLOWED_ON_PROFILE_INIT,
// Sign out is forced allowed. Only used for tests. // Sign out is forced allowed. Only used for tests.
FORCE_SIGNOUT_ALWAYS_ALLOWED_FOR_TEST, FORCE_SIGNOUT_ALWAYS_ALLOWED_FOR_TEST,
// User cleared account cookies when there's no sync consent, which has caused
// sign out.
USER_DELETED_ACCOUNT_COOKIES,
// Keep this as the last enum. // Keep this as the last enum.
NUM_PROFILE_SIGNOUT_METRICS, NUM_PROFILE_SIGNOUT_METRICS,
}; };
......
...@@ -65765,6 +65765,10 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf ...@@ -65765,6 +65765,10 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
<int value="11" label="[Tests] Force sign-out"> <int value="11" label="[Tests] Force sign-out">
Signout is forced. Used only for tests. Signout is forced. Used only for tests.
</int> </int>
<int value="12" label="User deleted account cookies">
User cleared account cookies when there's no sync consent, which has caused
sign out.
</int>
</enum> </enum>
<enum name="SigninSource"> <enum name="SigninSource">
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