Revert "Disallow secondary accounts in ARC for Child users"

This reverts commit 85bd68c7.

Reason for revert: for transitions from Child to Regular account, and
from Regular to Child account all secondary accounts are going to be
removed from Chrome OS Account Manager (so we don't need to remove the
accounts from ARC).

Original change's description:
> Disallow secondary accounts in ARC for Child users
>
> Project Beaker on Chrome OS is going to allow child users the addition
> of secondary accounts, but it's going to be limited to web only.
>
> - If user has transitioned from child account to regular -
> TriggerAccountsPushToArc will add accounts to ARC on startup.
> - If user has transitioned from regular account to child -
> HandleSupervisionTransition will remove all secondary accounts from
> ARC.
>
> Update is going to be executed on startup, since to transition to
> another type, user need to sign-out and sign-in again (I think).
>
> ARC CL: http://ag/9615344
>
> Bug: 1017160
> Change-Id: I323a18aca9c9f63e6a3869d9b9dd4dd34432c31c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1876089
> Reviewed-by: Mattias Nissler <mnissler@chromium.org>
> Reviewed-by: Yury Khmel <khmel@chromium.org>
> Commit-Queue: Anastasiia Nikolaienko <anastasiian@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#711171}

Bug: 1017160
Change-Id: Ie44780d372a2c4b9a85327e93be29a2ede0ad9e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1904073Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Commit-Queue: Anastasiia Nikolaienko <anastasiian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714666}

chrome/browser/chromeos/arc/auth/arc_auth_service.cc

@@ -31,7 +31,6 @@
 #include "chrome/browser/ui/settings_window_manager_chromeos.h"
 #include "chrome/browser/ui/webui/signin/inline_login_handler_dialog_chromeos.h"
 #include "chrome/common/webui_url_constants.h"
-#include "chromeos/constants/chromeos_features.h"
 #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/arc/arc_features.h"
 #include "components/arc/arc_prefs.h"
@@ -284,13 +283,6 @@ void ArcAuthService::OnConnectionReady() {
   if (arc::IsArcProvisioned(profile_)) {
     TriggerAccountManagerMigrationsIfRequired(profile_);
     TriggerAccountsPushToArc(false /* filter_primary_account */);
-    if (chromeos::features::IsEduCoexistenceEnabled() &&
-        GetSupervisionTransition(profile_) ==
-            ArcSupervisionTransition::REGULAR_TO_CHILD) {
-      // If profile transitioned from Child to Regular, accounts have already
-      // been pushed to ARC.
-      RemoveSecondaryAccountsFromArc();
-    }
   }
 
   if (pending_get_arc_accounts_callback_)
@@ -565,11 +557,6 @@ void ArcAuthService::OnRefreshTokenUpdatedForAccount(
   if (!arc::IsArcProvisioned(profile_))
     return;
 
-  // For child device accounts do not allow the propagation of secondary
-  // accounts from Chrome OS Account Manager to ARC.
-  if (profile_->IsChild() && !IsPrimaryGaiaAccount(account_info.gaia))
-    return;
-
   if (identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState(
           account_info.account_id)) {
     VLOG(1) << "Ignoring account update due to lack of a valid token: "
@@ -832,13 +819,4 @@ void ArcAuthService::OnMainAccountResolutionStatus(
   UpdateMainAccountResolutionStatus(profile_, status);
 }
 
-void ArcAuthService::RemoveSecondaryAccountsFromArc() {
-  auto* instance = ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->auth(),
-                                               RemoveSecondaryAccounts);
-  if (!instance)
-    return;
-
-  instance->RemoveSecondaryAccounts();
-}
-
 }  // namespace arc

chrome/browser/chromeos/arc/auth/arc_auth_service.h

@@ -193,10 +193,6 @@ class ArcAuthService : public KeyedService,
   // Response for |mojom::GetMainAccountResolutionStatus|.
   void OnMainAccountResolutionStatus(mojom::MainAccountResolutionStatus status);
 
-  // Removes secondary accounts from ARC. For child device accounts propagation
-  // of secondary accounts from Chrome OS Account Manager to ARC is not allowed.
-  void RemoveSecondaryAccountsFromArc();
-
   // Non-owning pointers.
   Profile* const profile_;
   signin::IdentityManager* const identity_manager_;

chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc

@@ -41,7 +41,6 @@
 #include "chrome/browser/ui/app_list/arc/arc_data_removal_dialog.h"
 #include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/constants/chromeos_features.h"
@@ -159,10 +158,6 @@ class FakeAuthInstance : public mojom::AuthInstance {
         mojom::MainAccountResolutionStatus::HASH_CODE_MATCH_SINGLE_ACCOUNT);
   }
 
-  void RemoveSecondaryAccounts() override {
-    ++num_remove_secondary_accounts_calls_;
-  }
-
   mojom::AccountInfo* account_info() { return account_info_.get(); }
 
   mojom::ArcSignInStatus sign_in_status() const { return status_; }
@@ -175,10 +170,6 @@ class FakeAuthInstance : public mojom::AuthInstance {
 
   std::string last_removed_account() const { return last_removed_account_; }
 
-  int num_remove_secondary_accounts_calls() const {
-    return num_remove_secondary_accounts_calls_;
-  }
-
  private:
   void OnAccountInfoResponse(base::OnceClosure done_closure,
                              mojom::ArcSignInStatus status,
@@ -198,21 +189,13 @@ class FakeAuthInstance : public mojom::AuthInstance {
   int num_account_removed_calls_ = 0;
   std::string last_removed_account_;
 
-  int num_remove_secondary_accounts_calls_ = 0;
-
   base::WeakPtrFactory<FakeAuthInstance> weak_ptr_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(FakeAuthInstance);
 };
 
 class ArcAuthServiceTest : public InProcessBrowserTest {
  protected:
-  ArcAuthServiceTest() {
-    scoped_feature_list_.InitWithFeatures(
-        {kEnableChildToRegularTransitionFeature,
-         kEnableRegularToChildTransitionFeature,
-         chromeos::features::kEduCoexistence},
-        {});
-  }
+  ArcAuthServiceTest() = default;
 
   // InProcessBrowserTest:
   ~ArcAuthServiceTest() override = default;
@@ -312,9 +295,7 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
         identity_test_environment_adaptor_->identity_test_env();
     identity_test_env->SetAutomaticIssueOfAccessTokens(true);
     identity_test_env->MakePrimaryAccountAvailable(kFakeUserName);
-  }
 
-  void SetUpAuthService() {
     profile()->GetPrefs()->SetBoolean(prefs::kArcSignedIn, true);
     profile()->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
     MigrateSigninScopedDeviceId(profile());
@@ -383,7 +364,6 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
 
   AccountInfo SetupGaiaAccount(const std::string& email) {
     SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-    SetUpAuthService();
     return SeedAccountInfo(email);
   }
 
@@ -425,7 +405,6 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
   // Not owned.
   ArcAuthService* auth_service_ = nullptr;
   ArcBridgeService* arc_bridge_service_ = nullptr;
-  base::test::ScopedFeatureList scoped_feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcAuthServiceTest);
 };
@@ -436,7 +415,6 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
                        SuccessfulBackgroundFetchViaDeprecatedApi) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          GetFakeAuthTokenResponse());
 
@@ -458,7 +436,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
 // Chrome supplies the info configured in SetAccountAndProfile() method.
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, SuccessfulBackgroundFetch) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          GetFakeAuthTokenResponse());
 
@@ -479,7 +456,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, SuccessfulBackgroundFetch) {
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
                        ReAuthenticatePrimaryAccountSucceeds) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          GetFakeAuthTokenResponse());
 
@@ -500,7 +476,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
                        ReAuthenticatePrimaryAccountFailsForInvalidAccount) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          std::string() /* response */,
                                          net::HTTP_UNAUTHORIZED);
@@ -517,7 +492,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, FetchSecondaryAccountInfoSucceeds) {
   // Add a Secondary Account.
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   SeedAccountInfo(kSecondaryAccountEmail);
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          GetFakeAuthTokenResponse());
@@ -541,7 +515,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
                        FetchSecondaryAccountInfoFailsForInvalidAccounts) {
   // Add a Secondary Account.
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   SeedAccountInfo(kSecondaryAccountEmail);
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          std::string() /* response */,
@@ -561,7 +534,6 @@ IN_PROC_BROWSER_TEST_F(
     ArcAuthServiceTest,
     FetchSecondaryAccountInfoReturnsErrorForNotFoundAccounts) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   // Don't add account with kSecondaryAccountEmail.
 
   base::RunLoop run_loop;
@@ -576,7 +548,6 @@ IN_PROC_BROWSER_TEST_F(
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, FetchGoogleAccountsFromArc) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
 
   EXPECT_FALSE(arc_google_accounts_callback_called());
   RequestGoogleAccountsInArc();
@@ -592,7 +563,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, FetchGoogleAccountsFromArc) {
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
                        FetchGoogleAccountsFromArcWorksAcrossConnectionResets) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
 
   // Close the connection.
   arc_bridge_service().auth()->CloseInstance(&auth_instance());
@@ -617,7 +587,6 @@ IN_PROC_BROWSER_TEST_F(
     ArcAuthServiceTest,
     PrimaryAccountReauthIsNotAttemptedJustAfterProvisioning) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   const int initial_num_calls = auth_instance().num_account_upserted_calls();
   // Our test setup manually sets the device as provisioned and invokes
   // |ArcAuthService::OnConnectionReady|. Hence, we would have received an
@@ -658,7 +627,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, AccountUpdatesArePropagated) {
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, AccountRemovalsArePropagated) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SetUpAuthService();
   SeedAccountInfo(kSecondaryAccountEmail);
 
   EXPECT_EQ(0, auth_instance().num_account_removed_calls());
@@ -748,7 +716,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
   chromeos::DemoSession::StartIfInDemoMode();
 
   SetAccountAndProfile(user_manager::USER_TYPE_PUBLIC_ACCOUNT);
-  SetUpAuthService();
 
   test_url_loader_factory()->SetInterceptor(
       base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
@@ -776,7 +743,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest, GetDemoAccount) {
   chromeos::DemoSession::StartIfInDemoMode();
 
   SetAccountAndProfile(user_manager::USER_TYPE_PUBLIC_ACCOUNT);
-  SetUpAuthService();
 
   test_url_loader_factory()->SetInterceptor(
       base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
@@ -803,7 +769,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
   chromeos::DemoSession::StartIfInDemoMode();
 
   SetAccountAndProfile(user_manager::USER_TYPE_PUBLIC_ACCOUNT);
-  SetUpAuthService();
 
   base::RunLoop run_loop;
   auth_instance().RequestAccountInfoDeprecated(run_loop.QuitClosure());
@@ -824,7 +789,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest, GetOfflineDemoAccount) {
   chromeos::DemoSession::StartIfInDemoMode();
 
   SetAccountAndProfile(user_manager::USER_TYPE_PUBLIC_ACCOUNT);
-  SetUpAuthService();
 
   base::RunLoop run_loop;
   auth_instance().RequestPrimaryAccountInfo(run_loop.QuitClosure());
@@ -846,7 +810,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
   chromeos::DemoSession::StartIfInDemoMode();
 
   SetAccountAndProfile(user_manager::USER_TYPE_PUBLIC_ACCOUNT);
-  SetUpAuthService();
 
   test_url_loader_factory()->SetInterceptor(
       base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
@@ -874,7 +837,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
   chromeos::DemoSession::StartIfInDemoMode();
 
   SetAccountAndProfile(user_manager::USER_TYPE_PUBLIC_ACCOUNT);
-  SetUpAuthService();
 
   test_url_loader_factory()->SetInterceptor(
       base::BindLambdaForTesting([&](const network::ResourceRequest& request) {
@@ -898,7 +860,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
 IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
                        RequestPublicAccountInfo) {
   SetAccountAndProfile(user_manager::USER_TYPE_PUBLIC_ACCOUNT);
-  SetUpAuthService();
   profile()->GetProfilePolicyConnector()->OverrideIsManagedForTesting(true);
 
   test_url_loader_factory()->SetInterceptor(
@@ -924,7 +885,6 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
 // SetAccountAndProfile() above.
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, ChildAccountFetchViaDeprecatedApi) {
   SetAccountAndProfile(user_manager::USER_TYPE_CHILD);
-  SetUpAuthService();
   EXPECT_TRUE(profile()->IsChild());
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          GetFakeAuthTokenResponse());
@@ -947,7 +907,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, ChildAccountFetchViaDeprecatedApi) {
 // Chrome supplies the info configured in SetAccountAndProfile() above.
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, ChildAccountFetch) {
   SetAccountAndProfile(user_manager::USER_TYPE_CHILD);
-  SetUpAuthService();
   EXPECT_TRUE(profile()->IsChild());
   test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
                                          GetFakeAuthTokenResponse());
@@ -968,7 +927,6 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, ChildAccountFetch) {
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, ChildTransition) {
   SetAccountAndProfile(user_manager::USER_TYPE_CHILD);
-  SetUpAuthService();
 
   ArcSessionManager* session = ArcSessionManager::Get();
   ASSERT_TRUE(session);
@@ -1074,38 +1032,4 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, ChildTransition) {
   }
 }
 
-IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
-                       ChildUserSecondaryAccountsNotPropagated) {
-  SetAccountAndProfile(user_manager::USER_TYPE_CHILD);
-  SetUpAuthService();
-  SeedAccountInfo(kSecondaryAccountEmail);
-  EXPECT_TRUE(profile()->IsChild());
-  EXPECT_EQ(1, auth_instance().num_account_upserted_calls());
-}
-
-IN_PROC_BROWSER_TEST_F(
-    ArcAuthServiceTest,
-    RegularToChildTransitionShouldRemoveSecondaryAccountsFromArc) {
-  SetAccountAndProfile(user_manager::USER_TYPE_CHILD);
-  SeedAccountInfo(kSecondaryAccountEmail);
-  profile()->GetPrefs()->SetInteger(
-      arc::prefs::kArcSupervisionTransition,
-      static_cast<int>(arc::ArcSupervisionTransition::REGULAR_TO_CHILD));
-  SetUpAuthService();
-  EXPECT_EQ(1, auth_instance().num_account_upserted_calls());
-  EXPECT_EQ(1, auth_instance().num_remove_secondary_accounts_calls());
-}
-
-IN_PROC_BROWSER_TEST_F(
-    ArcAuthServiceTest,
-    ChildToRegularTransitionShouldAddSecondaryAccountsToArc) {
-  SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
-  SeedAccountInfo(kSecondaryAccountEmail);
-  profile()->GetPrefs()->SetInteger(
-      arc::prefs::kArcSupervisionTransition,
-      static_cast<int>(arc::ArcSupervisionTransition::CHILD_TO_REGULAR));
-  SetUpAuthService();
-  EXPECT_EQ(2, auth_instance().num_account_upserted_calls());
-}
-
 }  // namespace arc

components/arc/mojom/auth.mojom

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Next MinVersion: 24
+// Next MinVersion: 23
 
 module arc.mojom;
 
@@ -338,7 +338,7 @@ interface AuthHost {
   [MinVersion=20] HandleUpdateCredentialsRequest@17(string account_name);
 };
 
-// Next Method ID: 7
+// Next Method ID: 6
 interface AuthInstance {
   // DEPRECATED: Please use Init@2 instead.
   InitDeprecated@0(AuthHost host_ptr);
@@ -371,12 +371,4 @@ interface AuthInstance {
   // Gets resolution status of main account for statistics reporting.
   [MinVersion=22] GetMainAccountResolutionStatus@5()
       => (MainAccountResolutionStatus status);
-
-  // A notification that all secondary accounts should be removed from ARC.
-  // Equivalent to calling |OnAccountUpdated| with AccountUpdateType::REMOVAL
-  // for each secondary account and in case of inconsistency between Chrome OS
-  // Account Manager and ARC accounts, inconsistent accounts will be also
-  // removed from ARC. Should be called on transition from Regular to Child
-  // account type.
-  [MinVersion=23] RemoveSecondaryAccounts@6();
 };