Pre-emptively reject dummy tokens in ChromeOSOAuth2TokenServiceDelegate

|ChromeOSOAuth2TokenServiceDelegate| should pre-emptively reject access
token fetches and other operations on known dummy Gaia tokens instead of
letting them go over the network and get rejected by Gaia server-side.

This brings feature parity between |ChromeOSOAuth2TokenServiceDelegate|
and |MutableProfileOAuth2TokenServiceDelegate|, which does the same
thing.

Bug: 915628
Test: unit_tests --gtest_filter="*CrOSOAuthDelegateTest*"
Change-Id: I812a847fb1afd040a3ae743a9e491753878ca523
Reviewed-on: https://chromium-review.googlesource.com/c/1437627
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627034}

chrome/browser/chromeos/oauth2_token_service_delegate.cc

@@ -146,7 +146,7 @@ void ChromeOSOAuth2TokenServiceDelegate::UpdateAuthError(
   }
 
   auto it = errors_.find(account_id);
-  if ((it != errors_.end())) {
+  if (it != errors_.end()) {
     // Update the existing error.
     if (error.state() == GoogleServiceAuthError::NONE)
       errors_.erase(it);
@@ -268,13 +268,24 @@ void ChromeOSOAuth2TokenServiceDelegate::OnTokenUpserted(
   // invoked here, regardless. See the comment above |FireAuthErrorChanged| few
   // lines down.
   errors_.erase(account_id);
+  GoogleServiceAuthError error(GoogleServiceAuthError::AuthErrorNone());
+
+  // However, if we know that |account_key| has a dummy token, store a
+  // persistent error against it, so that we can pre-emptively reject access
+  // token requests for it.
+  if (account_manager_->HasDummyGaiaToken(account_key)) {
+    error = GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
+        GoogleServiceAuthError::InvalidGaiaCredentialsReason::
+            CREDENTIALS_REJECTED_BY_CLIENT);
+    errors_.emplace(account_id, AccountErrorStatus{error});
+  }
 
   ScopedBatchChange batch(this);
   FireRefreshTokenAvailable(account_id);
   // See |OAuth2TokenService::Observer::OnAuthErrorChanged|.
   // |OnAuthErrorChanged| must be always called after
   // |OnRefreshTokenAvailable|, when refresh token is updated.
-  FireAuthErrorChanged(account_id, GoogleServiceAuthError::AuthErrorNone());
+  FireAuthErrorChanged(account_id, error);
 }
 
 void ChromeOSOAuth2TokenServiceDelegate::OnAccountRemoved(

chrome/browser/chromeos/oauth2_token_service_delegate_unittest.cc

@@ -84,9 +84,16 @@ class TokenServiceObserver : public OAuth2TokenService::Observer {
 
   void OnRefreshTokenAvailable(const std::string& account_id) override {
     EXPECT_TRUE(is_inside_batch_);
-    // We should not be seeing any cached errors for a freshly updated account.
-    EXPECT_EQ(GoogleServiceAuthError::AuthErrorNone(),
-              delegate_->GetAuthError(account_id));
+    // We should not be seeing any cached errors for a freshly updated account,
+    // except when they have been generated by us (i.e.
+    // CREDENTIALS_REJECTED_BY_CLIENT).
+    const GoogleServiceAuthError error = delegate_->GetAuthError(account_id);
+    EXPECT_TRUE((error == GoogleServiceAuthError::AuthErrorNone()) ||
+                (error.state() ==
+                     GoogleServiceAuthError::State::INVALID_GAIA_CREDENTIALS &&
+                 error.GetInvalidGaiaCredentialsReason() ==
+                     GoogleServiceAuthError::InvalidGaiaCredentialsReason::
+                         CREDENTIALS_REJECTED_BY_CLIENT));
 
     account_ids_.insert(account_id);
 
@@ -273,6 +280,24 @@ TEST_F(CrOSOAuthDelegateTest,
   delegate_->UpdateCredentials(account_info_.account_id, "new-token");
 }
 
+TEST_F(CrOSOAuthDelegateTest, DummyTokensArePreEmptivelyRejected) {
+  TokenServiceObserver observer(delegate_.get());
+  delegate_->UpdateCredentials(account_info_.account_id,
+                               AccountManager::kInvalidToken);
+
+  const GoogleServiceAuthError error =
+      delegate_->GetAuthError(account_info_.account_id);
+  EXPECT_EQ(GoogleServiceAuthError::State::INVALID_GAIA_CREDENTIALS,
+            error.state());
+  EXPECT_EQ(GoogleServiceAuthError::InvalidGaiaCredentialsReason::
+                CREDENTIALS_REJECTED_BY_CLIENT,
+            error.GetInvalidGaiaCredentialsReason());
+
+  // Observer notification should also have notified about the same error.
+  EXPECT_EQ(error, observer.last_err_);
+  EXPECT_EQ(account_info_.account_id, observer.last_err_account_id_);
+}
+
 TEST_F(CrOSOAuthDelegateTest, ObserversAreNotifiedOnCredentialsUpdate) {
   TokenServiceObserver observer(delegate_.get());
   delegate_->UpdateCredentials(account_info_.account_id, kGaiaToken);

chromeos/account_manager/account_manager.cc

@@ -398,6 +398,14 @@ bool AccountManager::IsTokenAvailable(const AccountKey& account_key) const {
          it->second != kActiveDirectoryDummyToken;
 }
 
+bool AccountManager::HasDummyGaiaToken(const AccountKey& account_key) const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK_EQ(init_state_, InitializationState::kInitialized);
+
+  auto it = tokens_.find(account_key);
+  return it != tokens_.end() && it->second == kInvalidToken;
+}
+
 void AccountManager::MaybeRevokeTokenOnServer(const AccountKey& account_key) {
   auto it = tokens_.find(account_key);
   if (it == tokens_.end()) {

chromeos/account_manager/account_manager.h

@@ -174,6 +174,12 @@ class CHROMEOS_EXPORT AccountManager {
   // initialized yet.
   bool IsTokenAvailable(const AccountKey& account_key) const;
 
+  // Returns true if the token stored against |account_key| is a dummy Gaia
+  // token. This is meant to be used only by
+  // |ChromeOSOAuth2TokenServiceDelegate| to pre-emptively reject access token
+  // requests for |account_key|.
+  bool HasDummyGaiaToken(const AccountKey& account_key) const;
+
  private:
   enum InitializationState {
     kNotStarted,   // Initialize has not been called