Use SigninClient::IsClearPrimaryAccountAllowed to prevent signout for

supervised users. Bug: 1324567 Change-Id: Id893f427f5a523efc18d8e99f81977df9cc71c2b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4926469 Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com> Reviewed-by: Boris Sazonov <bsazonov@chromium.org> Commit-Queue: Nohemi Fernandez <fernandex@chromium.org> Cr-Commit-Position: refs/heads/main@{#1209363}
chromium · Oct 13, 2023 · f4acc35 · f4acc35
1 parent 6919353
commit f4acc35
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 24 deletions.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManagerImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManagerImpl.java
@@ -186,9 +186,10 @@ public boolean isSyncOptInAllowed() {
     /** Returns true if sign out can be started now. */
     @Override
     public boolean isSignOutAllowed() {
-        return mSignOutState == null && mSignInState == null
+        return mSignOutState == null
+                && mSignInState == null
                 && mIdentityManager.getPrimaryAccountInfo(ConsentLevel.SIGNIN) != null
-                && !Profile.getLastUsedRegularProfile().isChild();
+                && mIdentityManager.isClearPrimaryAccountAllowed();
     }
 
     /**

diff --git a/...ndroid/javatests/src/org/chromium/chrome/browser/signin/SigninSignoutIntegrationTest.java b/...ndroid/javatests/src/org/chromium/chrome/browser/signin/SigninSignoutIntegrationTest.java
@@ -145,10 +145,16 @@ public void testSignIn() {
                 () -> mSigninManager.getIdentityManager().hasPrimaryAccount(ConsentLevel.SYNC));
         verify(mSignInStateObserverMock).onSignedIn();
         verify(mSignInStateObserverMock, never()).onSignedOut();
-        TestThreadUtils.runOnUiThreadBlocking(() -> {
-            Assert.assertEquals(coreAccountInfo,
-                    mSigninManager.getIdentityManager().getPrimaryAccountInfo(ConsentLevel.SYNC));
-        });
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    Assert.assertEquals(
+                            coreAccountInfo,
+                            mSigninManager
+                                    .getIdentityManager()
+                                    .getPrimaryAccountInfo(ConsentLevel.SYNC));
+                    Assert.assertTrue(
+                            mSigninManager.getIdentityManager().isClearPrimaryAccountAllowed());
+                });
         signinHistogram.assertExpected(
                 "Signin should be recorded with the settings page as the access point.");
         syncHistogram.assertExpected(
@@ -275,10 +281,16 @@ public void testChildAccountSignInAndSync() {
 
         CriteriaHelper.pollUiThread(
                 () -> mSigninManager.getIdentityManager().hasPrimaryAccount(ConsentLevel.SYNC));
-        TestThreadUtils.runOnUiThreadBlocking(() -> {
-            Assert.assertEquals(coreChildAccountInfo,
-                    mSigninManager.getIdentityManager().getPrimaryAccountInfo(ConsentLevel.SYNC));
-        });
+        TestThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    Assert.assertEquals(
+                            coreChildAccountInfo,
+                            mSigninManager
+                                    .getIdentityManager()
+                                    .getPrimaryAccountInfo(ConsentLevel.SYNC));
+                    Assert.assertFalse(
+                            mSigninManager.getIdentityManager().isClearPrimaryAccountAllowed());
+                });
         verify(mSignInStateObserverMock, times(2)).onSignedIn();
         verify(mSignInStateObserverMock, never()).onSignedOut();
         onView(withText(R.string.account_management_sign_out)).check(doesNotExist());

diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninManagerImplTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninManagerImplTest.java
@@ -131,7 +131,6 @@ public void setUp() {
         when(mNativeMock.isSigninAllowedByPolicy(NATIVE_SIGNIN_MANAGER)).thenReturn(true);
         // Pretend Google Play services are available as it is required for the sign-in
         when(mExternalAuthUtils.isGooglePlayServicesMissing(any())).thenReturn(false);
-        when(mProfile.isChild()).thenReturn(false);
         doAnswer(invocation -> {
             Runnable runnable = invocation.getArgument(0);
             runnable.run();
@@ -143,6 +142,8 @@ public void setUp() {
         when(mIdentityManagerNativeMock.findExtendedAccountInfoByEmailAddress(
                      NATIVE_IDENTITY_MANAGER, ACCOUNT_INFO.getEmail()))
                 .thenReturn(ACCOUNT_INFO);
+        when(mIdentityManagerNativeMock.isClearPrimaryAccountAllowed(NATIVE_IDENTITY_MANAGER))
+                .thenReturn(true);
 
         AccountManagerFacadeProvider.setInstanceForTests(mFakeAccountManagerFacade);
 
@@ -527,7 +528,8 @@ public void signOutNotAllowedForChildAccounts() {
         when(mIdentityManagerNativeMock.getPrimaryAccountInfo(
                      eq(NATIVE_IDENTITY_MANAGER), anyInt()))
                 .thenReturn(ACCOUNT_INFO);
-        when(mProfile.isChild()).thenReturn(true);
+        when(mIdentityManagerNativeMock.isClearPrimaryAccountAllowed(NATIVE_IDENTITY_MANAGER))
+                .thenReturn(false);
 
         assertFalse(mSigninManager.isSignOutAllowed());
     }

diff --git a/...blic/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java b/...blic/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java
@@ -17,6 +17,7 @@
 import org.chromium.components.signin.base.CoreAccountInfo;
 
 import java.util.List;
+
 /**
  * IdentityManager provides access to native IdentityManager's public API to java components.
  */
@@ -183,6 +184,11 @@ public void refreshAccountInfoIfStale(List<CoreAccountInfo> accountInfos) {
         }
     }
 
+    /** Returns true if the primary account can be cleared/removed from the browser. */
+    public boolean isClearPrimaryAccountAllowed() {
+        return IdentityManagerJni.get().isClearPrimaryAccountAllowed(mNativeIdentityManager);
+    }
+
     /**
      * Called by native to invalidate an OAuth2 token. Please note that the token is invalidated
      * asynchronously.
@@ -207,5 +213,7 @@ public interface Natives {
         AccountInfo findExtendedAccountInfoByEmailAddress(long nativeIdentityManager, String email);
         CoreAccountInfo[] getAccountsWithRefreshTokens(long nativeIdentityManager);
         void refreshAccountInfoIfStale(long nativeIdentityManager, CoreAccountId coreAccountId);
+
+        boolean isClearPrimaryAccountAllowed(long nativeIdentityManager);
     }
 }
diff --git a/components/signin/public/identity_manager/identity_manager.cc b/components/signin/public/identity_manager/identity_manager.cc
@@ -114,9 +114,7 @@ IdentityManager::IdentityManager(IdentityManager::InitParameters&& parameters)
           std::move(parameters.gaia_cookie_manager_service)),
       primary_account_manager_(std::move(parameters.primary_account_manager)),
       account_fetcher_service_(std::move(parameters.account_fetcher_service)),
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
       signin_client_(parameters.signin_client),
-#endif
 #if BUILDFLAG(IS_CHROMEOS)
       account_manager_facade_(parameters.account_manager_facade),
 #endif
@@ -129,6 +127,7 @@ IdentityManager::IdentityManager(IdentityManager::InitParameters&& parameters)
       should_verify_scope_access_(parameters.should_verify_scope_access) {
   DCHECK(account_fetcher_service_);
   DCHECK(diagnostics_provider_);
+  DCHECK(signin_client_);
 
   primary_account_manager_observation_.Observe(primary_account_manager_.get());
   token_service_observation_.Observe(token_service_.get());
@@ -490,6 +489,11 @@ IdentityManager::GetAccountsWithRefreshTokens(JNIEnv* env) const {
   }
   return array;
 }
+
+jboolean IdentityManager::IsClearPrimaryAccountAllowed(JNIEnv* env) const {
+  return signin_client_->IsClearPrimaryAccountAllowed(
+      HasPrimaryAccount(signin::ConsentLevel::kSync));
+}
 #endif
 
 AccountInfo IdentityManager::FindExtendedPrimaryAccountInfo(

diff --git a/components/signin/public/identity_manager/identity_manager.h b/components/signin/public/identity_manager/identity_manager.h
@@ -359,9 +359,7 @@ class IdentityManager : public KeyedService,
     AccountConsistencyMethod account_consistency =
         AccountConsistencyMethod::kDisabled;
     bool should_verify_scope_access = true;
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
     raw_ptr<SigninClient> signin_client = nullptr;
-#endif
 #if BUILDFLAG(IS_CHROMEOS)
     raw_ptr<account_manager::AccountManagerFacade, DanglingUntriaged>
         account_manager_facade = nullptr;
@@ -452,6 +450,9 @@ class IdentityManager : public KeyedService,
   void RefreshAccountInfoIfStale(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& j_core_account_id);
+
+  // Returns true if the browser allows the primary account to be cleared.
+  jboolean IsClearPrimaryAccountAllowed(JNIEnv* env) const;
 #endif
 
  private:
@@ -647,9 +648,7 @@ class IdentityManager : public KeyedService,
   std::unique_ptr<GaiaCookieManagerService> gaia_cookie_manager_service_;
   std::unique_ptr<PrimaryAccountManager> primary_account_manager_;
   std::unique_ptr<AccountFetcherService> account_fetcher_service_;
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
   const raw_ptr<SigninClient> signin_client_;
-#endif
 #if BUILDFLAG(IS_CHROMEOS)
   const raw_ptr<account_manager::AccountManagerFacade, DanglingUntriaged>
       account_manager_facade_;

diff --git a/components/signin/public/identity_manager/identity_manager_builder.cc b/components/signin/public/identity_manager/identity_manager_builder.cc
@@ -202,9 +202,7 @@ IdentityManager::InitParameters BuildIdentityManagerInitParameters(
   init_params.primary_account_manager = std::move(primary_account_manager);
   init_params.token_service = std::move(token_service);
   init_params.account_consistency = params->account_consistency;
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
   init_params.signin_client = params->signin_client;
-#endif
 #if BUILDFLAG(IS_CHROMEOS)
   init_params.account_manager_facade = params->account_manager_facade;
 #endif

diff --git a/components/signin/public/identity_manager/identity_manager_unittest.cc b/components/signin/public/identity_manager/identity_manager_unittest.cc
@@ -456,9 +456,7 @@ class IdentityManagerTest : public testing::Test {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
     init_params.account_manager_facade = account_manager_facade_.get();
 #endif
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
     init_params.signin_client = &signin_client_;
-#endif
     init_params.account_fetcher_service = std::move(account_fetcher_service);
     init_params.account_tracker_service = std::move(account_tracker_service);
     init_params.gaia_cookie_manager_service =

diff --git a/components/signin/public/identity_manager/identity_test_environment.cc b/components/signin/public/identity_manager/identity_test_environment.cc
@@ -383,9 +383,7 @@ IdentityTestEnvironment::FinishBuildIdentityManagerForTests(
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   init_params.account_manager_facade = account_manager_facade;
 #endif
-#if BUILDFLAG(IS_CHROMEOS_LACROS)
   init_params.signin_client = signin_client;
-#endif
 
   return std::make_unique<IdentityManager>(std::move(init_params));
 }