Stop revocation of the old tokens on UpsertAccount

After attaching device id for secondary account sign-ins
(https://crrev.com/c/1859784), old tokens will be revoked
automatically. Revocation of the old tokens for account sign-ins with
device id attached would also result in revocation of the new tokens.

Bug: 1012723
Change-Id: I282aa2117204138cf8d3e5d68c45aee96d6b9cf9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1859783
Commit-Queue: Anastasiia Nikolaienko <anastasiian@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709949}

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -1404,7 +1404,7 @@ void UserSessionManager::InitProfilePreferences(
             AccountManager::AccountKey{
                 gaia_id, account_manager::AccountType::ACCOUNT_TYPE_GAIA},
             user->GetDisplayEmail() /* raw_email */,
-            user_context.GetRefreshToken(), false /* revoke_old_token */);
+            user_context.GetRefreshToken());
       }
       // else: If |user_context| does not contain a refresh token, then we are
       // restoring an existing Profile, in which case the account will be

chromeos/components/account_manager/account_manager.cc

@@ -401,20 +401,18 @@ void AccountManager::RemoveAccountByEmailInternal(const std::string& email) {
 
 void AccountManager::UpsertAccount(const AccountKey& account_key,
                                    const std::string& raw_email,
-                                   const std::string& token,
-                                   bool revoke_old_token) {
+                                   const std::string& token) {
   DCHECK_NE(init_state_, InitializationState::kNotStarted);
   DCHECK(!raw_email.empty());
 
   base::OnceClosure closure = base::BindOnce(
       &AccountManager::UpsertAccountInternal, weak_factory_.GetWeakPtr(),
-      account_key, AccountInfo{raw_email, token}, revoke_old_token);
+      account_key, AccountInfo{raw_email, token});
   RunOnInitialization(std::move(closure));
 }
 
 void AccountManager::UpdateToken(const AccountKey& account_key,
-                                 const std::string& token,
-                                 bool revoke_old_token) {
+                                 const std::string& token) {
   DCHECK_NE(init_state_, InitializationState::kNotStarted);
 
   if (account_key.account_type ==
@@ -422,23 +420,21 @@ void AccountManager::UpdateToken(const AccountKey& account_key,
     DCHECK_EQ(token, kActiveDirectoryDummyToken);
   }
 
-  base::OnceClosure closure = base::BindOnce(
-      &AccountManager::UpdateTokenInternal, weak_factory_.GetWeakPtr(),
-      account_key, token, revoke_old_token);
+  base::OnceClosure closure =
+      base::BindOnce(&AccountManager::UpdateTokenInternal,
+                     weak_factory_.GetWeakPtr(), account_key, token);
   RunOnInitialization(std::move(closure));
 }
 
 void AccountManager::UpdateTokenInternal(const AccountKey& account_key,
-                                         const std::string& token,
-                                         bool revoke_old_token) {
+                                         const std::string& token) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK_EQ(init_state_, InitializationState::kInitialized);
 
   auto it = accounts_.find(account_key);
   DCHECK(it != accounts_.end())
       << "UpdateToken cannot be used for adding accounts";
-  UpsertAccountInternal(account_key, AccountInfo{it->second.raw_email, token},
-                        revoke_old_token);
+  UpsertAccountInternal(account_key, AccountInfo{it->second.raw_email, token});
 }
 
 void AccountManager::UpdateEmail(const AccountKey& account_key,
@@ -460,13 +456,11 @@ void AccountManager::UpdateEmailInternal(const AccountKey& account_key,
   auto it = accounts_.find(account_key);
   DCHECK(it != accounts_.end())
       << "UpdateEmail cannot be used for adding accounts";
-  UpsertAccountInternal(account_key, AccountInfo{raw_email, it->second.token},
-                        false /* revoke_old_token */);
+  UpsertAccountInternal(account_key, AccountInfo{raw_email, it->second.token});
 }
 
 void AccountManager::UpsertAccountInternal(const AccountKey& account_key,
-                                           const AccountInfo& account,
-                                           bool revoke_old_token) {
+                                           const AccountInfo& account) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK_EQ(init_state_, InitializationState::kInitialized);
   DCHECK(account_key.IsValid()) << "Invalid account_key: " << account_key;
@@ -509,9 +503,6 @@ void AccountManager::UpsertAccountInternal(const AccountKey& account_key,
   if (did_token_change) {
     NotifyTokenObservers(Account{account_key, account.raw_email});
   }
-  if (did_token_change && revoke_old_token) {
-    MaybeRevokeTokenOnServer(account_key, old_token);
-  }
 }
 
 void AccountManager::PersistAccountsAsync() {

chromeos/components/account_manager/account_manager.h

@@ -185,24 +185,17 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManager {
   // email id for |account_key|. |raw_email| must not be empty. Use
   // |AccountManager::kActiveDirectoryDummyToken| as the |token| for Active
   // Directory accounts, and |AccountManager::kInvalidToken| for Gaia accounts
-  // with unknown tokens. |revoke_old_token| is an optional parameter that tells
-  // |AccountManager| whether or not to revoke the old token associated with
-  // |account_key| in the event of a token update.
+  // with unknown tokens.
   // Note: This API is idempotent.
   void UpsertAccount(const AccountKey& account_key,
                      const std::string& raw_email,
-                     const std::string& token,
-                     bool revoke_old_token = true);
+                     const std::string& token);
 
   // Updates the token for the account corresponding to the given |account_key|.
   // The account must be known to Account Manager. See |UpsertAccount| for
-  // information about adding an account. |revoke_old_token| is an optional
-  // parameter that tells |AccountManager| whether or not to revoke the old
-  // token associated with |account_key| in the event of a token update.
+  // information about adding an account.
   // Note: This API is idempotent.
-  void UpdateToken(const AccountKey& account_key,
-                   const std::string& token,
-                   bool revoke_old_token = true);
+  void UpdateToken(const AccountKey& account_key, const std::string& token);
 
   // Updates the email associated with |account_key|. The account must be known
   // to Account Manager. See |UpsertAccount| for information about adding an
@@ -318,8 +311,7 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManager {
 
   // Assumes that |AccountManager| initialization (|init_state_|) is complete.
   void UpdateTokenInternal(const AccountKey& account_key,
-                           const std::string& token,
-                           bool revoke_old_token);
+                           const std::string& token);
 
   // Assumes that |AccountManager| initialization (|init_state_|) is complete.
   void UpdateEmailInternal(const AccountKey& account_key,
@@ -330,8 +322,7 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManager {
   // funnel through to this method. Assumes that |AccountManager| initialization
   // (|init_state_|) is complete.
   void UpsertAccountInternal(const AccountKey& account_key,
-                             const AccountInfo& account,
-                             bool revoke_old_token);
+                             const AccountInfo& account);
 
   // Posts a task on |task_runner_|, which is usually a background thread, to
   // persist the current state of |accounts_|.

chromeos/components/account_manager/account_manager_unittest.cc

@@ -431,20 +431,7 @@ TEST_F(AccountManagerTest,
   account_manager_->RemoveAccount(kGaiaAccountKey_);
 }
 
-TEST_F(AccountManagerTest, OldTokenIsRevokedOnTokenUpdateByDefault) {
-  ResetAndInitializeAccountManager();
-  // Only 1 token should be revoked.
-  EXPECT_CALL(*account_manager_.get(), RevokeGaiaTokenOnServer(kGaiaToken))
-      .Times(1);
-  account_manager_->UpsertAccount(kGaiaAccountKey_, kRawUserEmail, kGaiaToken);
-
-  // Update the token.
-  account_manager_->UpdateToken(kGaiaAccountKey_, kNewGaiaToken);
-  task_environment_.RunUntilIdle();
-}
-
-TEST_F(AccountManagerTest,
-       OldTokenIsNotRevokedOnTokenUpdateIfRequestedByClient) {
+TEST_F(AccountManagerTest, OldTokenIsNotRevokedOnTokenUpdateByDefault) {
   ResetAndInitializeAccountManager();
   // Token should not be revoked.
   EXPECT_CALL(*account_manager_.get(), RevokeGaiaTokenOnServer(kGaiaToken))
@@ -452,22 +439,7 @@ TEST_F(AccountManagerTest,
   account_manager_->UpsertAccount(kGaiaAccountKey_, kRawUserEmail, kGaiaToken);
 
   // Update the token.
-  account_manager_->UpdateToken(kGaiaAccountKey_, kNewGaiaToken,
-                                false /* revoke_old_token */);
-  task_environment_.RunUntilIdle();
-}
-
-TEST_F(AccountManagerTest,
-       OldTokenIsNotRevokedOnAccountUpdateIfRequestedByClient) {
-  ResetAndInitializeAccountManager();
-  // Token should not be revoked.
-  EXPECT_CALL(*account_manager_.get(), RevokeGaiaTokenOnServer(kGaiaToken))
-      .Times(0);
-  account_manager_->UpsertAccount(kGaiaAccountKey_, kRawUserEmail, kGaiaToken);
-
-  // Update the token.
-  account_manager_->UpsertAccount(kGaiaAccountKey_, kRawUserEmail,
-                                  kNewGaiaToken, false /* revoke_old_token */);
+  account_manager_->UpdateToken(kGaiaAccountKey_, kNewGaiaToken);
   task_environment_.RunUntilIdle();
 }