Notify ARC about persistent error during sign-in

On secondary account sign-in, notify ARC that particular error is
persistent and there is no reason to retry the sign-in request.
Currently persistent errors are when user token become invalid or when
requested account is missing in Chrome OS.

Bug: b/146792085
Change-Id: I09addfb53968d83782390a7977c22ea1ba6b01d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2108723
Commit-Queue: Andrei Salavei <solovey@google.com>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760024}

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

@@ -247,6 +247,15 @@ std::string GetAccountName(Profile* profile) {
   }
 }
 
+void OnFetchPrimaryAccountInfoCompleted(
+    ArcAuthService::RequestAccountInfoCallback callback,
+    bool persistent_error,
+    mojom::ArcSignInStatus status,
+    mojom::AccountInfoPtr account_info) {
+  std::move(callback).Run(std::move(status), std::move(account_info),
+                          persistent_error);
+}
+
 }  // namespace
 
 // static
@@ -477,7 +486,13 @@ void ArcAuthService::RequestAccountInfo(const std::string& account_name,
     return;
   }
 
-  FetchPrimaryAccountInfo(false /* initial_signin */, std::move(callback));
+  // TODO(solovey): Check secondary account ARC sign-in statistics and send
+  // |persistent_error| == true for primary account for cases when refresh token
+  // has persistent error.
+  FetchPrimaryAccountInfo(
+      false /* initial_signin */,
+      base::BindOnce(&OnFetchPrimaryAccountInfoCompleted, std::move(callback),
+                     false /* persistent_error */));
 }
 
 void ArcAuthService::FetchPrimaryAccountInfo(
@@ -733,13 +748,22 @@ void ArcAuthService::FetchSecondaryAccountInfo(
   if (!account_info.has_value()) {
     // Account is in ARC, but not in Chrome OS Account Manager.
     std::move(callback).Run(mojom::ArcSignInStatus::CHROME_ACCOUNT_NOT_FOUND,
-                            nullptr);
+                            nullptr /* account_info */,
+                            true /* persistent_error */);
     return;
   }
 
   const CoreAccountId& account_id = account_info->account_id;
   DCHECK(!account_id.empty());
 
+  if (identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState(
+          account_id)) {
+    std::move(callback).Run(
+        mojom::ArcSignInStatus::CHROME_SERVER_COMMUNICATION_ERROR,
+        nullptr /* account_info */, true /* persistent_error */);
+    return;
+  }
+
   std::unique_ptr<ArcBackgroundAuthCodeFetcher> fetcher =
       CreateArcBackgroundAuthCodeFetcher(account_id,
                                          false /* initial_signin */);
@@ -769,11 +793,29 @@ void ArcAuthService::OnSecondaryAccountAuthCodeFetched(
         mojom::ArcSignInStatus::SUCCESS,
         CreateAccountInfo(true /* is_enforced */, auth_code, account_name,
                           mojom::ChromeAccountType::USER_ACCOUNT,
-                          false /* is_managed */));
-  } else {
+                          false /* is_managed */),
+        false /* persistent_error*/);
+    return;
+  }
+
+  base::Optional<AccountInfo> account_info =
+      identity_manager_
+          ->FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
+              account_name);
+  // Take care of the case when the user removes an account immediately after
+  // adding/re-authenticating it.
+  if (account_info.has_value()) {
+    const bool is_persistent_error =
+        identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState(
+            account_info->account_id);
     std::move(callback).Run(
-        mojom::ArcSignInStatus::CHROME_SERVER_COMMUNICATION_ERROR, nullptr);
+        mojom::ArcSignInStatus::CHROME_SERVER_COMMUNICATION_ERROR,
+        nullptr /* account_info */, is_persistent_error);
+    return;
   }
+
+  std::move(callback).Run(mojom::ArcSignInStatus::CHROME_ACCOUNT_NOT_FOUND,
+                          nullptr /* account_info */, true);
 }
 
 void ArcAuthService::DeletePendingTokenRequest(ArcFetcherBase* fetcher) {

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

@@ -63,6 +63,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/identity_manager/accounts_mutator.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
+#include "components/signin/public/identity_manager/identity_test_utils.h"
 #include "components/user_manager/scoped_user_manager.h"
 #include "components/user_manager/user_manager.h"
 #include "components/user_manager/user_names.h"
@@ -134,7 +135,7 @@ class FakeAuthInstance : public mojom::AuthInstance {
 
   void RequestPrimaryAccountInfo(base::OnceClosure done_closure) {
     host_->RequestPrimaryAccountInfo(base::BindOnce(
-        &FakeAuthInstance::OnAccountInfoResponse,
+        &FakeAuthInstance::OnPrimaryAccountInfoResponse,
         weak_ptr_factory_.GetWeakPtr(), std::move(done_closure)));
   }
 
@@ -163,6 +164,8 @@ class FakeAuthInstance : public mojom::AuthInstance {
 
   mojom::ArcSignInStatus sign_in_status() const { return status_; }
 
+  bool sign_in_persistent_error() const { return persistent_error_; }
+
   int num_account_upserted_calls() const { return num_account_upserted_calls_; }
 
   std::string last_upserted_account() const { return last_upserted_account_; }
@@ -172,16 +175,27 @@ class FakeAuthInstance : public mojom::AuthInstance {
   std::string last_removed_account() const { return last_removed_account_; }
 
  private:
+  void OnPrimaryAccountInfoResponse(base::OnceClosure done_closure,
+                                    mojom::ArcSignInStatus status,
+                                    mojom::AccountInfoPtr account_info) {
+    account_info_ = std::move(account_info);
+    status_ = status;
+    std::move(done_closure).Run();
+  }
+
   void OnAccountInfoResponse(base::OnceClosure done_closure,
                              mojom::ArcSignInStatus status,
-                             mojom::AccountInfoPtr account_info) {
-    account_info_ = std::move(account_info);
+                             mojom::AccountInfoPtr account_info,
+                             bool persistent_error) {
     status_ = status;
+    account_info_ = std::move(account_info);
+    persistent_error_ = persistent_error;
     std::move(done_closure).Run();
   }
 
   mojom::AuthHostPtr host_;
   mojom::ArcSignInStatus status_;
+  bool persistent_error_;
   mojom::AccountInfoPtr account_info_;
   base::OnceClosure done_closure_;
 
@@ -362,6 +376,13 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
         ->SetRefreshTokenForAccount(account_id);
   }
 
+  void UpdatePersistentErrorOfRefreshTokenForAccount(
+      const CoreAccountId& account_id,
+      const GoogleServiceAuthError& error) {
+    identity_test_environment_adaptor_->identity_test_env()
+        ->UpdatePersistentErrorOfRefreshTokenForAccount(account_id, error);
+  }
+
   void RequestGoogleAccountsInArc() {
     arc_google_accounts_.clear();
     arc_google_accounts_callback_called_ = false;
@@ -559,6 +580,7 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
             auth_instance().account_info()->account_type);
   EXPECT_FALSE(auth_instance().account_info()->enrollment_token);
   EXPECT_FALSE(auth_instance().account_info()->is_managed);
+  EXPECT_FALSE(auth_instance().sign_in_persistent_error());
 }
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
@@ -597,6 +619,7 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, FetchSecondaryAccountInfoSucceeds) {
             auth_instance().account_info()->account_type);
   EXPECT_FALSE(auth_instance().account_info()->enrollment_token);
   EXPECT_FALSE(auth_instance().account_info()->is_managed);
+  EXPECT_FALSE(auth_instance().sign_in_persistent_error());
 }
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
@@ -618,6 +641,45 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
             auth_instance().sign_in_status());
 }
 
+IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
+                       FetchSecondaryAccountInfoInvalidRefreshToken) {
+  const AccountInfo account_info = SetupGaiaAccount(kSecondaryAccountEmail);
+  SetInvalidRefreshTokenForAccount(account_info.account_id);
+  test_url_loader_factory()->AddResponse(arc::kAuthTokenExchangeEndPoint,
+                                         std::string() /* response */,
+                                         net::HTTP_UNAUTHORIZED);
+
+  base::RunLoop run_loop;
+  auth_instance().RequestAccountInfo(kSecondaryAccountEmail,
+                                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_FALSE(auth_instance().account_info());
+  EXPECT_EQ(mojom::ArcSignInStatus::CHROME_SERVER_COMMUNICATION_ERROR,
+            auth_instance().sign_in_status());
+  EXPECT_TRUE(auth_instance().sign_in_persistent_error());
+}
+
+IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
+                       FetchSecondaryAccountRefreshTokenHasPersistentError) {
+  const AccountInfo account_info = SetupGaiaAccount(kSecondaryAccountEmail);
+  UpdatePersistentErrorOfRefreshTokenForAccount(
+      account_info.account_id,
+      GoogleServiceAuthError::FromInvalidGaiaCredentialsReason(
+          GoogleServiceAuthError::InvalidGaiaCredentialsReason::
+              CREDENTIALS_REJECTED_BY_SERVER));
+
+  base::RunLoop run_loop;
+  auth_instance().RequestAccountInfo(kSecondaryAccountEmail,
+                                     run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_FALSE(auth_instance().account_info());
+  EXPECT_EQ(mojom::ArcSignInStatus::CHROME_SERVER_COMMUNICATION_ERROR,
+            auth_instance().sign_in_status());
+  EXPECT_TRUE(auth_instance().sign_in_persistent_error());
+}
+
 IN_PROC_BROWSER_TEST_F(
     ArcAuthServiceTest,
     FetchSecondaryAccountInfoReturnsErrorForNotFoundAccounts) {
@@ -632,6 +694,7 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_FALSE(auth_instance().account_info());
   EXPECT_EQ(mojom::ArcSignInStatus::CHROME_ACCOUNT_NOT_FOUND,
             auth_instance().sign_in_status());
+  EXPECT_TRUE(auth_instance().sign_in_persistent_error());
 }
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, FetchGoogleAccountsFromArc) {
@@ -966,6 +1029,7 @@ IN_PROC_BROWSER_TEST_F(ArcRobotAccountAuthServiceTest,
             auth_instance().account_info()->account_type);
   EXPECT_FALSE(auth_instance().account_info()->enrollment_token);
   EXPECT_TRUE(auth_instance().account_info()->is_managed);
+  EXPECT_FALSE(auth_instance().sign_in_persistent_error());
 }
 
 // Tests that when ARC requests account info for a child account, via

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: 25
 
 module arc.mojom;
 
@@ -318,8 +318,12 @@ interface AuthHost {
   // |account_info| is not null.
   // In case of an error, |status| must contain the reason of failure, other
   // than SUCCESS, and |account_info| is null.
+  // |persistent_error| flag set to true indicates that error cannot be
+  // eliminated by retrying the request.
   [MinVersion=15] RequestAccountInfo@13(string account_name)
-      => (ArcSignInStatus status, AccountInfo? account_info);
+      => (ArcSignInStatus status,
+          AccountInfo? account_info,
+          [MinVersion=24] bool persistent_error);
 
   // Returns |true| if Chrome OS Account Manager is available for this session.
   // Account Manager is not available in Guest Sessions and Public Sessions