Do not revoke old refresh token on server-side during an update

The refresh token is updated to a new valid one in case of reauth. In this case
the old and the new refresh tokens have the same device ID.

When revoking a refresh token on the server, Gaia revokes all the refresh tokens
that have the same device ID.

This CL avoid revoking the old refresh token when to avoid Gaia revoking the new
token soon after a reauth.

Bug: 865189
Change-Id: I9c2a64f30e14a5376c530f3b511c212b70738f3a
Reviewed-on: https://chromium-review.googlesource.com/1145073
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577947}

chrome/browser/signin/dice_browsertest.cc

@@ -694,9 +694,9 @@ IN_PROC_BROWSER_TEST_F(DiceBrowserTest, Reauth) {
   SendRefreshTokenResponse();
   EXPECT_EQ(GetMainAccountID(),
             GetSigninManager()->GetAuthenticatedAccountId());
-  // Old token must be revoked silently.
+
+  // Old token must not be revoked (see http://crbug.com/865189).
   EXPECT_EQ(0, token_revoked_notification_count_);
-  WaitForTokenRevokedCount(1);
 
   EXPECT_EQ(1, reconcilor_blocked_count_);
   WaitForReconcilorUnblockedCount(1);
@@ -896,9 +896,10 @@ IN_PROC_BROWSER_TEST_F(DiceFixAuthErrorsBrowserTest, ReauthFixAuthError) {
   SendRefreshTokenResponse();
   EXPECT_EQ(GetMainAccountID(),
             GetSigninManager()->GetAuthenticatedAccountId());
-  // Old token must be revoked silently.
+
+  // Old token must not be revoked (see http://crbug.com/865189).
   EXPECT_EQ(0, token_revoked_notification_count_);
-  WaitForTokenRevokedCount(1);
+
   EXPECT_EQ(1, reconcilor_blocked_count_);
   WaitForReconcilorUnblockedCount(1);
 }

chrome/browser/signin/mutable_profile_oauth2_token_service_delegate.cc

@@ -761,8 +761,9 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory(
   DCHECK(!account_id.empty());
   DCHECK(!refresh_token.empty());
 
+  bool is_refresh_token_invalidated = refresh_token == kInvalidRefreshToken;
   GoogleServiceAuthError error =
-      (refresh_token == kInvalidRefreshToken)
+      is_refresh_token_invalidated
           ? GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
                 GoogleServiceAuthError::InvalidGaiaCredentialsReason::
                     CREDENTIALS_REJECTED_BY_CLIENT)
@@ -775,7 +776,21 @@ void MutableProfileOAuth2TokenServiceDelegate::UpdateCredentialsInMemory(
     DCHECK_NE(refresh_token, refresh_tokens_[account_id]->refresh_token());
     VLOG(1) << "MutablePO2TS::UpdateCredentials; Refresh Token was present. "
             << "account_id=" << account_id;
-    RevokeCredentialsOnServer(refresh_tokens_[account_id]->refresh_token());
+
+    // The old refresh token must be revoked on the server only when it is
+    // invalidated.
+    //
+    // The refresh token is updated to a new valid one in case of reauth.
+    // In the reauth case the old and the new refresh tokens have the same
+    // device ID. When revoking a refresh token on the server, Gaia revokes
+    // all the refresh tokens that have the same device ID.
+    // Therefore, the old refresh token must not be revoked on the server
+    // when it is updated to a new valid one (otherwise the new refresh token
+    // would also be invalidated server-side).
+    // See http://crbug.com/865189 for more information about this regression.
+    if (is_refresh_token_invalidated)
+      RevokeCredentialsOnServer(refresh_tokens_[account_id]->refresh_token());
+
     refresh_tokens_[account_id]->set_refresh_token(refresh_token);
     UpdateAuthError(account_id, error);
   } else {

chrome/browser/signin/mutable_profile_oauth2_token_service_delegate_unittest.cc

@@ -760,9 +760,10 @@ TEST_F(MutableProfileOAuth2TokenServiceDelegateTest, RevokeOnUpdate) {
   EXPECT_TRUE(oauth2_service_delegate_->server_revokes_.empty());
   ExpectOneTokenAvailableNotification();
 
-  // Update the token.
+  // Updating the token does not revoke the old one.
+  // Regression test for http://crbug.com/865189
   oauth2_service_delegate_->UpdateCredentials("account_id", "refresh_token2");
-  EXPECT_EQ(1u, oauth2_service_delegate_->server_revokes_.size());
+  EXPECT_TRUE(oauth2_service_delegate_->server_revokes_.empty());
   ExpectOneTokenAvailableNotification();
 
   // Flush the server revokes.