Stop using authenticated account in PrimaryAccountManager

This CL moves away from using the notion of an authenticated account in
favor of the primary account in the PrimaryAccountManager:
* GetAuthenticatedAccount() -> GetPrimaryAccount(ConsentLevel::kSync)
* GetGetUnconsentedPrimaryAccount() ->
     GetPrimaryAccount(ConsentLevel::kNotRequired)
* SetAuthenticatedAccountInfo() -> SetPrimarySyncAccountInfo()
* Signin() -> SetSyncPrimaryAccountInfo()

Bug: 1158855

Change-Id: Ibd5e5e91ad91d8decf425c6e39b82a7aad5a1e34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601490
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839332}

components/signin/internal/identity_manager/accounts_mutator_impl.cc

@@ -100,7 +100,7 @@ void AccountsMutatorImpl::InvalidateRefreshTokenForPrimaryAccount(
 #endif
   DCHECK(primary_account_manager_->HasPrimaryAccount(ConsentLevel::kSync));
   CoreAccountInfo primary_account_info =
-      primary_account_manager_->GetAuthenticatedAccountInfo();
+      primary_account_manager_->GetPrimaryAccountInfo(ConsentLevel::kSync);
   AddOrUpdateAccount(primary_account_info.gaia, primary_account_info.email,
                      GaiaConstants::kInvalidRefreshToken,
                      primary_account_info.is_under_advanced_protection, source);

components/signin/internal/identity_manager/primary_account_manager.cc

@@ -113,10 +113,10 @@ void PrimaryAccountManager::Initialize(PrefService* local_state) {
       account_tracker_service_->GetAccountInfo(account_id);
   if (consented) {
     DCHECK(!account_info.account_id.empty());
-    // First reset the state, because SetAuthenticatedAccountInfo can only be
-    // called if the user is not already signed in.
-    SetPrimaryAccountInternal(CoreAccountInfo(), /*consented=*/false);
-    SetAuthenticatedAccountInfo(account_info);
+    // First reset the state, because SetSyncPrimaryAccountInternal() can
+    // only be called if there is no primary account.
+    SetPrimaryAccountInternal(CoreAccountInfo(), /*consented_to_sync=*/false);
+    SetSyncPrimaryAccountInternal(account_info);
   } else {
     SetPrimaryAccountInternal(account_info, consented);
   }
@@ -127,32 +127,30 @@ void PrimaryAccountManager::Initialize(PrefService* local_state) {
   // It is important to only load credentials after starting to observe the
   // token service.
   token_service_->AddObserver(this);
-  token_service_->LoadCredentials(GetAuthenticatedAccountId());
+  token_service_->LoadCredentials(
+      GetPrimaryAccountId(signin::ConsentLevel::kSync));
 }
 
 bool PrimaryAccountManager::IsInitialized() const {
   return initialized_;
 }
 
-CoreAccountInfo PrimaryAccountManager::GetAuthenticatedAccountInfo() const {
-  if (!HasPrimaryAccount(signin::ConsentLevel::kSync))
+CoreAccountInfo PrimaryAccountManager::GetPrimaryAccountInfo(
+    signin::ConsentLevel consent_level) const {
+  if (!HasPrimaryAccount(consent_level))
     return CoreAccountInfo();
   return primary_account_info();
 }
 
-CoreAccountId PrimaryAccountManager::GetAuthenticatedAccountId() const {
-  return GetAuthenticatedAccountInfo().account_id;
-}
-
-CoreAccountInfo PrimaryAccountManager::GetUnconsentedPrimaryAccountInfo()
-    const {
-  return primary_account_info();
+CoreAccountId PrimaryAccountManager::GetPrimaryAccountId(
+    signin::ConsentLevel consent_level) const {
+  return GetPrimaryAccountInfo(consent_level).account_id;
 }
 
 void PrimaryAccountManager::SetUnconsentedPrimaryAccountInfo(
-    CoreAccountInfo account_info) {
+    const CoreAccountInfo& account_info) {
   if (HasPrimaryAccount(signin::ConsentLevel::kSync)) {
-    DCHECK_EQ(account_info, GetAuthenticatedAccountInfo());
+    DCHECK_EQ(account_info, GetPrimaryAccountInfo(signin::ConsentLevel::kSync));
     return;
   }
 
@@ -163,7 +161,7 @@ void PrimaryAccountManager::SetUnconsentedPrimaryAccountInfo(
     FirePrimaryAccountChanged(previous_state);
 }
 
-void PrimaryAccountManager::SetAuthenticatedAccountInfo(
+void PrimaryAccountManager::SetSyncPrimaryAccountInternal(
     const CoreAccountInfo& account_info) {
   DCHECK(!account_info.account_id.empty());
   DCHECK(!HasPrimaryAccount(signin::ConsentLevel::kSync));
@@ -192,7 +190,7 @@ void PrimaryAccountManager::SetAuthenticatedAccountInfo(
   client_->GetPrefs()->SetString(prefs::kGoogleServicesLastUsername,
                                  account_info.email);
 
-  // Commit authenticated account info immediately so that it does not get lost
+  // Commit primary sync account info immediately so that it does not get lost
   // if Chrome crashes before the next commit interval.
   client_->GetPrefs()->CommitPendingWrite();
 }
@@ -230,24 +228,29 @@ bool PrimaryAccountManager::HasPrimaryAccount(
   }
 }
 
-void PrimaryAccountManager::SignIn(const std::string& username) {
-  CoreAccountInfo info =
-      account_tracker_service_->FindAccountInfoByEmail(username);
-  DCHECK(!info.gaia.empty());
-  DCHECK(!info.email.empty());
-  DCHECK(!info.account_id.empty());
+void PrimaryAccountManager::SetSyncPrimaryAccountInfo(
+    const CoreAccountInfo& account_info) {
+#if DCHECK_IS_ON()
+  DCHECK(!account_info.account_id.empty());
+  DCHECK(!account_info.gaia.empty());
+  DCHECK(!account_info.email.empty());
+  DCHECK(!account_tracker_service_->GetAccountInfo(account_info.account_id)
+              .IsEmpty())
+      << "Account should have been seeded before being set as primary account";
+#endif
   if (HasPrimaryAccount(signin::ConsentLevel::kSync)) {
-    DCHECK_EQ(info.account_id, GetAuthenticatedAccountId())
-        << "Changing the authenticated account while it is not allowed.";
+    DCHECK_EQ(account_info.account_id,
+              GetPrimaryAccountId(signin::ConsentLevel::kSync))
+        << "Changing the primary sync account is not allowed.";
     return;
   }
 
   PrimaryAccountChangeEvent::State previous_state = GetPrimaryAccountState();
-  SetAuthenticatedAccountInfo(info);
+  SetSyncPrimaryAccountInternal(account_info);
   FirePrimaryAccountChanged(previous_state);
 }
 
-void PrimaryAccountManager::UpdateAuthenticatedAccountInfo() {
+void PrimaryAccountManager::UpdateSyncPrimaryAccountInfo() {
   DCHECK(!primary_account_info().account_id.empty());
   DCHECK(HasPrimaryAccount(signin::ConsentLevel::kSync));
   const CoreAccountInfo info = account_tracker_service_->GetAccountInfo(
@@ -389,9 +392,10 @@ void PrimaryAccountManager::OnRefreshTokensLoaded() {
   if (token_service_->HasLoadCredentialsFinishedWithNoErrors()) {
     std::vector<AccountInfo> accounts_in_tracker_service =
         account_tracker_service_->GetAccounts();
-    const CoreAccountId authenticated_account_id = GetAuthenticatedAccountId();
+    const CoreAccountId sync_account_id =
+        GetPrimaryAccountId(signin::ConsentLevel::kSync);
     for (const auto& account : accounts_in_tracker_service) {
-      if (authenticated_account_id != account.account_id &&
+      if (sync_account_id != account.account_id &&
           !token_service_->RefreshTokenIsAvailable(account.account_id)) {
         VLOG(0) << "Removed account from account tracker service: "
                 << account.account_id;

components/signin/internal/identity_manager/primary_account_manager.h

@@ -78,48 +78,52 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
   void Initialize(PrefService* local_state);
   bool IsInitialized() const;
 
-  // If a user has previously signed in (and has not signed out), this returns
-  // the know information of the account. Otherwise, it returns an empty struct.
-  CoreAccountInfo GetAuthenticatedAccountInfo() const;
-
-  // If a user has previously signed in (and has not signed out), this returns
-  // the account id. Otherwise, it returns an empty CoreAccountId.  This id is
-  // the G+/Focus obfuscated gaia id of the user. It can be used to uniquely
-  // identify an account, so for example as a key to map accounts to data. For
-  // code that needs a unique id to represent the connected account, call this
-  // method. Example: the AccountStatusMap type in
-  // MutableProfileOAuth2TokenService. For code that needs to know the
-  // normalized email address of the connected account, use
-  // GetAuthenticatedAccountInfo().email.  Example: to show the string
-  // "Signed in as XXX" in the hotdog menu.
-  CoreAccountId GetAuthenticatedAccountId() const;
-
   // Returns whether the user's primary account is available. If consent is
   // |ConsentLevel::kSync| then true implies that the user has blessed this
   // account for sync.
   bool HasPrimaryAccount(signin::ConsentLevel consent_level) const;
 
+  // Provides access to the core information of the user's primary account.
+  // The primary account may or may not be blessed with the sync consent.
+  // Returns an empty struct if no such info is available, either because there
+  // is no primary account yet or because the user signed out or the |consent|
+  // level required |ConsentLevel::kSync| was not granted.
+  // Returns a non-empty struct if the primary account exists and was granted
+  // the required consent level.
+  CoreAccountInfo GetPrimaryAccountInfo(
+      signin::ConsentLevel consent_level) const;
+
+  // Provides access to the account ID of the user's primary account. Simple
+  // convenience wrapper over GetPrimaryAccountInfo().account_id.
+  CoreAccountId GetPrimaryAccountId(signin::ConsentLevel consent_level) const;
+
   // Signs a user in. PrimaryAccountManager assumes that |username| can be used
   // to look up the corresponding account_id and gaia_id for this email.
-  void SignIn(const std::string& username);
+  void SetSyncPrimaryAccountInfo(const CoreAccountInfo& account_info);
+
+  // Sets the unconsented primary account. The unconsented primary account can
+  // only be changed if the user has not consented for sync If the user has
+  // consented for sync already, then use ClearPrimaryAccount() or RevokeSync()
+  // instead.
+  void SetUnconsentedPrimaryAccountInfo(const CoreAccountInfo& account_info);
 
-  // Updates the authenticated account information from AccountTrackerService.
-  void UpdateAuthenticatedAccountInfo();
+  // Updates the primary account information from AccountTrackerService.
+  void UpdateSyncPrimaryAccountInfo();
 
   // Signout API surfaces (not supported on ChromeOS, where signout is not
   // permitted).
 #if !BUILDFLAG(IS_CHROMEOS_ASH)
-  // Signs a user out, removing the preference, erasing all keys
-  // associated with the authenticated user, and canceling all auth in progress.
-  // It removes all accounts from Chrome by revoking all refresh tokens.
+  // Clears the primary account, erasing all keys associated with the primary
+  // account (also cancels all auth in progress).
+  // It removes all accounts from the identity manager by revoking all refresh
+  // tokens.
   void ClearPrimaryAccount(signin_metrics::ProfileSignout signout_source_metric,
                            signin_metrics::SignoutDelete signout_delete_metric);
 
 #endif  // !BUILDFLAG(IS_CHROMEOS_ASH)
 
-  // Signs a user out, removing the preference, erasing all keys
-  // associated with the authenticated user, and canceling all auth in progress.
-  // Does not remove the accounts from the token service.
+  // Rovokes the sync consent but leaves the primary account and the rest of
+  // the accounts untouched.
   void RevokeSyncConsent(signin_metrics::ProfileSignout signout_source_metric,
                          signin_metrics::SignoutDelete signout_delete_metric);
 
@@ -127,25 +131,14 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
 
-  // Provides access to the core information of the user's unconsented primary
-  // account. Returns an empty info, if there is no such account.
-  CoreAccountInfo GetUnconsentedPrimaryAccountInfo() const;
-
-  // Sets the unconsented primary account. The unconsented primary account can
-  // only be changed if the user is not authenticated. If the user is
-  // authenticated, use Signout() instead.
-  void SetUnconsentedPrimaryAccountInfo(CoreAccountInfo account_info);
-
  private:
-  // Sets the authenticated user's account id, when the user has consented to
-  // sync.
-  // If the user is already authenticated with the same account id, then this
-  // method is a no-op.
-  // It is forbidden to call this method if the user is already authenticated
-  // with a different account (this method will DCHECK in that case).
-  // |account_id| must not be empty. To log the user out, use
-  // ClearAuthenticatedAccountId() instead.
-  void SetAuthenticatedAccountInfo(const CoreAccountInfo& account_info);
+  // Sets the primary account id, when the user has consented to sync.
+  // If the user has consented for sync with the same account, then this method
+  // is a no-op.
+  // It is forbidden to call this method if the user has already consented for
+  // sync  with a different account (this method will DCHECK in that case).
+  // |account_id| must not be empty.
+  void SetSyncPrimaryAccountInternal(const CoreAccountInfo& account_info);
 
   // Sets |primary_account_info_| and updates the associated preferences.
   void SetPrimaryAccountInternal(const CoreAccountInfo& account_info,
@@ -189,8 +182,8 @@ class PrimaryAccountManager : public ProfileOAuth2TokenServiceObserver {
 
   bool initialized_ = false;
 
-  // Account id after successful authentication. The account may or may not be
-  // consented to Sync.
+  // The primary account information. The account may or may not be consented
+  // for Sync.
   // Must be kept in sync with prefs. Use SetPrimaryAccountInternal() to change
   // this field.
   CoreAccountInfo primary_account_info_;

components/signin/internal/identity_manager/primary_account_mutator_impl.cc

@@ -62,7 +62,7 @@ bool PrimaryAccountMutatorImpl::SetPrimaryAccount(
   // TODO(crbug.com/889899): should check that the account email is allowed.
 #endif
 
-  primary_account_manager_->SignIn(account_info.email);
+  primary_account_manager_->SetSyncPrimaryAccountInfo(account_info);
   return true;
 }
 
@@ -101,7 +101,7 @@ bool PrimaryAccountMutatorImpl::RevokeConsentShouldClearPrimaryAccount() const {
       // TODO(msarda): The logic in this function is platform specific and we
       // should consider moving it to |SigninManager|.
       return token_service_->RefreshTokenHasError(
-          primary_account_manager_->GetAuthenticatedAccountId());
+          primary_account_manager_->GetPrimaryAccountId(ConsentLevel::kSync));
     case AccountConsistencyMethod::kDisabled:
     case AccountConsistencyMethod::kMirror:
       return true;

components/signin/internal/identity_manager/primary_account_policy_manager_impl.cc

@@ -40,8 +40,8 @@ void PrimaryAccountPolicyManagerImpl::InitializePolicy(
           &PrimaryAccountPolicyManagerImpl::OnSigninAllowedPrefChanged,
           base::Unretained(this), primary_account_manager));
 
-  CoreAccountInfo account_info =
-      primary_account_manager->GetAuthenticatedAccountInfo();
+  CoreAccountInfo account_info = primary_account_manager->GetPrimaryAccountInfo(
+      signin::ConsentLevel::kSync);
   if (!account_info.account_id.empty() &&
       (!IsAllowedUsername(account_info.email) || !IsSigninAllowed())) {
     // User is signed in, but the username is invalid or signin is no longer
@@ -73,7 +73,9 @@ void PrimaryAccountPolicyManagerImpl::OnGoogleServicesUsernamePatternChanged(
     PrimaryAccountManager* primary_account_manager) {
   if (primary_account_manager->HasPrimaryAccount(signin::ConsentLevel::kSync) &&
       !IsAllowedUsername(
-          primary_account_manager->GetAuthenticatedAccountInfo().email)) {
+          primary_account_manager
+              ->GetPrimaryAccountInfo(signin::ConsentLevel::kSync)
+              .email)) {
     // Signed in user is invalid according to the current policy so sign
     // the user out.
     primary_account_manager->ClearPrimaryAccount(

components/signin/public/identity_manager/identity_manager.cc

@@ -115,10 +115,7 @@ void IdentityManager::RemoveObserver(Observer* observer) {
 // TODO(862619) change return type to base::Optional<CoreAccountInfo>
 CoreAccountInfo IdentityManager::GetPrimaryAccountInfo(
     ConsentLevel consent) const {
-  if (consent == ConsentLevel::kNotRequired) {
-    return primary_account_manager_->GetUnconsentedPrimaryAccountInfo();
-  }
-  return primary_account_manager_->GetAuthenticatedAccountInfo();
+  return primary_account_manager_->GetPrimaryAccountInfo(consent);
 }
 
 CoreAccountId IdentityManager::GetPrimaryAccountId(ConsentLevel consent) const {
@@ -683,7 +680,7 @@ void IdentityManager::OnAccountUpdated(const AccountInfo& info) {
   if (HasPrimaryAccount()) {
     const CoreAccountId primary_account_id = GetPrimaryAccountId();
     if (primary_account_id == info.account_id) {
-      primary_account_manager_->UpdateAuthenticatedAccountInfo();
+      primary_account_manager_->UpdateSyncPrimaryAccountInfo();
     }
   }
 

components/signin/public/identity_manager/identity_manager_unittest.cc

@@ -410,8 +410,10 @@ class IdentityManagerTest : public testing::Test {
 
     if (primary_account_manager_setup ==
         PrimaryAccountManagerSetup::kWithAuthenticatedAccout) {
-      account_tracker_service->SeedAccountInfo(kTestGaiaId, kTestEmail);
-      primary_account_manager->SignIn(kTestEmail);
+      CoreAccountId account_id =
+          account_tracker_service->SeedAccountInfo(kTestGaiaId, kTestEmail);
+      primary_account_manager->SetSyncPrimaryAccountInfo(
+          account_tracker_service->GetAccountInfo(account_id));
     }
 
     IdentityManager::InitParameters init_params;
@@ -1219,7 +1221,8 @@ TEST_F(IdentityManagerTest, RemoveAccessTokenFromCache) {
 
   identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
                                                                   kTestEmail);
-  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
+  identity_manager()->GetPrimaryAccountMutator()->SetPrimaryAccount(
+      primary_account_id());
   UpdateCredentials(primary_account_id(), kTestGaiaId, kTestEmail,
                     "refresh_token");
 
@@ -1260,7 +1263,8 @@ TEST_F(IdentityManagerTest,
 
   identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
                                                                   kTestEmail);
-  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
+  identity_manager()->GetPrimaryAccountMutator()->SetPrimaryAccount(
+      primary_account_id());
   UpdateCredentials(primary_account_id(), kTestGaiaId, kTestEmail,
                     "refresh_token");
 
@@ -1352,7 +1356,8 @@ TEST_F(IdentityManagerTest, ObserveAccessTokenFetch) {
 
   identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
                                                                   kTestEmail);
-  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
+  identity_manager()->GetPrimaryAccountMutator()->SetPrimaryAccount(
+      primary_account_id());
   UpdateCredentials(primary_account_id(), kTestGaiaId, kTestEmail,
                     "refresh_token");
 
@@ -1407,7 +1412,8 @@ TEST_F(IdentityManagerTest,
 
   identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
                                                                   kTestEmail);
-  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
+  identity_manager()->GetPrimaryAccountMutator()->SetPrimaryAccount(
+      primary_account_id());
   UpdateCredentials(primary_account_id(), kTestGaiaId, kTestEmail,
                     "refresh_token");
   token_service()->set_auto_post_fetch_response_on_message_loop(true);
@@ -2097,7 +2103,8 @@ TEST_F(IdentityManagerTest,
        BatchChangeObserversAreNotifiedOnCredentialsUpdate) {
   identity_manager()->GetAccountTrackerService()->SeedAccountInfo(kTestGaiaId,
                                                                   kTestEmail);
-  identity_manager()->GetPrimaryAccountManager()->SignIn(kTestEmail);
+  identity_manager()->GetPrimaryAccountMutator()->SetPrimaryAccount(
+      primary_account_id());
   UpdateCredentials(primary_account_id(), kTestGaiaId, kTestEmail,
                     "refresh_token");
 

components/signin/public/identity_manager/identity_test_utils.cc

@@ -125,7 +125,7 @@ CoreAccountInfo SetPrimaryAccount(IdentityManager* identity_manager,
       EnsureAccountExists(identity_manager->GetAccountTrackerService(), email);
   DCHECK(!account_info.gaia.empty());
 
-  primary_account_manager->SignIn(email);
+  primary_account_manager->SetSyncPrimaryAccountInfo(account_info);
 
   DCHECK(primary_account_manager->HasPrimaryAccount(ConsentLevel::kSync));
   DCHECK(identity_manager->HasPrimaryAccount());