[Signin] Add support for unconsented account to AccountReconcilor

Adds AccountReconcilorDelegate::GetConsentLevelForPrimaryAccount() that
is used by the reconcilor to get the primary account. To preserve the
existing behavior, this method still returns ConsentLevel::kSync in all
delegates except MirrorAccountReconcilorDelegate, where it returns
ConsentLevel::kNotRequired if MobileIdentityConsistency feature is
enabled.

Bug: 1095128
Change-Id: I39f61e566ebdef3fdc506432c2f3f445ddc152ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366903
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807025}

components/signin/core/browser/account_reconcilor.cc

@@ -34,6 +34,7 @@
 #include "google_apis/gaia/google_service_auth_error.h"
 
 using signin::AccountReconcilorDelegate;
+using signin::ConsentLevel;
 using signin_metrics::AccountReconcilorState;
 
 namespace {
@@ -125,8 +126,8 @@ bool RevokeAllSecondaryTokens(
   return token_revoked;
 }
 
-// TODO(msalama): Move this code and |RevokeAllSecondaryTokens|
-// to |DiceAccountReconcilorDelegate|.
+// TODO(https://crbug.com/1122551): Move this code and
+// |RevokeAllSecondaryTokens| to |DiceAccountReconcilorDelegate|.
 signin::RevokeTokenAction RevokeTokensNotInCookies(
     signin::IdentityManager* identity_manager,
     const CoreAccountId& primary_account,
@@ -496,7 +497,8 @@ void AccountReconcilor::StartReconcile() {
                                  base::Unretained(this)));
   }
 
-  const CoreAccountId& account_id = identity_manager_->GetPrimaryAccountId();
+  CoreAccountId account_id = identity_manager_->GetPrimaryAccountId(
+      delegate_->GetConsentLevelForPrimaryAccount());
   if (identity_manager_->HasAccountWithRefreshTokenInPersistentErrorState(
           account_id) &&
       delegate_->ShouldAbortReconcileIfPrimaryHasError()) {
@@ -625,8 +627,14 @@ void AccountReconcilor::OnAccountsInCookieUpdated(
       << "Ignore " << accounts.size() - verified_gaia_accounts.size()
       << " unverified account(s).";
 
-  CoreAccountId primary_account = identity_manager_->GetPrimaryAccountId();
+  ConsentLevel consent_level = delegate_->GetConsentLevelForPrimaryAccount();
+  CoreAccountId primary_account =
+      identity_manager_->GetPrimaryAccountId(consent_level);
   if (delegate_->ShouldRevokeTokensNotInCookies()) {
+    // This code is only used with DiceAccountReconcilorDelegate and should
+    // thus use sync account.
+    // TODO(https://crbug.com/1122551): Move to |DiceAccountReconcilorDelegate|.
+    DCHECK_EQ(consent_level, ConsentLevel::kSync);
     signin::RevokeTokenAction revoke_token_action = RevokeTokensNotInCookies(
         identity_manager_, primary_account, verified_gaia_accounts);
     delegate_->OnRevokeTokensNotInCookiesCompleted(revoke_token_action);
@@ -636,6 +644,7 @@ void AccountReconcilor::OnAccountsInCookieUpdated(
   // completely remove them from Chrome.
   // Revoking the token for the primary account is not supported (it should be
   // signed out or put to auth error state instead).
+  // TODO(https://crbug.com/1122551): Move to |DiceAccountReconcilorDelegate|.
   AccountReconcilorDelegate::RevokeTokenOption revoke_option =
       delegate_->ShouldRevokeSecondaryTokensBeforeReconcile(
           verified_gaia_accounts);
@@ -669,8 +678,13 @@ void AccountReconcilor::OnAccountsCookieDeletedByUserAction() {
   if (!delegate_->ShouldRevokeTokensOnCookieDeleted())
     return;
 
-  const CoreAccountId& primary_account =
-      identity_manager_->GetPrimaryAccountId();
+  // This code is only used with DiceAccountReconcilorDelegate and should thus
+  // use sync account.
+  // TODO(https://crbug.com/1122551): Move to |DiceAccountReconcilorDelegate|.
+  DCHECK_EQ(delegate_->GetConsentLevelForPrimaryAccount(), ConsentLevel::kSync);
+
+  CoreAccountId primary_account =
+      identity_manager_->GetPrimaryAccountId(ConsentLevel::kSync);
   // Revoke secondary tokens.
   RevokeAllSecondaryTokens(
       identity_manager_, AccountReconcilorDelegate::RevokeTokenOption::kRevoke,

components/signin/core/browser/account_reconcilor_delegate.cc

@@ -31,6 +31,11 @@ bool AccountReconcilorDelegate::ShouldAbortReconcileIfPrimaryHasError() const {
   return false;
 }
 
+ConsentLevel AccountReconcilorDelegate::GetConsentLevelForPrimaryAccount()
+    const {
+  return ConsentLevel::kSync;
+}
+
 CoreAccountId AccountReconcilorDelegate::GetFirstGaiaAccountForReconcile(
     const std::vector<CoreAccountId>& chrome_accounts,
     const std::vector<gaia::ListedAccount>& gaia_accounts,

components/signin/core/browser/account_reconcilor_delegate.h

@@ -10,6 +10,7 @@
 
 #include "base/time/time.h"
 #include "components/signin/public/base/multilogin_parameters.h"
+#include "components/signin/public/identity_manager/consent_level.h"
 #include "google_apis/gaia/gaia_auth_fetcher.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/google_service_auth_error.h"
@@ -64,6 +65,10 @@ class AccountReconcilorDelegate {
   // error state. Defaults to false.
   virtual bool ShouldAbortReconcileIfPrimaryHasError() const;
 
+  // Returns the consent level that should be used for obtaining the primary
+  // account. Defaults to ConsentLevel::kSync.
+  virtual ConsentLevel GetConsentLevelForPrimaryAccount() const;
+
   // Returns the first account to add in the Gaia cookie.
   // If this returns an empty string, the user must be logged out of all
   // accounts.

components/signin/core/browser/account_reconcilor_unittest.cc

@@ -562,6 +562,10 @@ class BaseAccountReconcilorTestTable : public AccountReconcilorTest {
     EXPECT_EQ(identity_manager->GetAccountsWithRefreshTokens().size(),
               tokens.size());
 
+    signin::ConsentLevel consent_level =
+        GetMockReconcilor()->delegate_->GetConsentLevelForPrimaryAccount();
+    CoreAccountId primary_account_id =
+        identity_manager->GetPrimaryAccountId(consent_level);
     bool authenticated_account_found = false;
     for (const Token& token : tokens) {
       CoreAccountId account_id =
@@ -572,12 +576,12 @@ class BaseAccountReconcilorTestTable : public AccountReconcilorTest {
           identity_manager->HasAccountWithRefreshTokenInPersistentErrorState(
               account_id));
       if (token.is_authenticated) {
-        EXPECT_EQ(account_id, identity_manager->GetPrimaryAccountId());
+        EXPECT_EQ(account_id, primary_account_id);
         authenticated_account_found = true;
       }
     }
     if (!authenticated_account_found)
-      EXPECT_EQ(CoreAccountId(), identity_manager->GetPrimaryAccountId());
+      EXPECT_EQ(CoreAccountId(), primary_account_id);
   }
 
   void SetupTokens(const char* tokens_string) {
@@ -632,14 +636,14 @@ class BaseAccountReconcilorTestTable : public AccountReconcilorTest {
     std::vector<Cookie> cookies = ParseCookieString(cookies_);
     ConfigureCookieManagerService(cookies);
 
-    // Setup tokens. This triggers listing cookies so we need to setup cookies
-    // before that.
-    SetupTokens(tokens_);
-
     // Call list accounts now so that the next call completes synchronously.
     identity_test_env()->identity_manager()->GetAccountsInCookieJar();
     base::RunLoop().RunUntilIdle();
 
+    // Setup tokens. This triggers listing cookies so we need to setup cookies
+    // before that.
+    SetupTokens(tokens_);
+
     // Setup expectations.
     testing::InSequence mock_sequence;
     bool logout_action = false;
@@ -1121,14 +1125,14 @@ TEST_P(AccountReconcilorTestForceDiceMigration, TableRowTestCheckNoOp) {
   std::vector<Cookie> cookies = ParseCookieString(cookies_after_reconcile_);
   ConfigureCookieManagerService(cookies);
 
-  // Setup tokens. This triggers listing cookies so we need to setup cookies
-  // before that.
-  SetupTokens(tokens_after_reconcile_);
-
   // Call list accounts now so that the next call completes synchronously.
   identity_test_env()->identity_manager()->GetAccountsInCookieJar();
   base::RunLoop().RunUntilIdle();
 
+  // Setup tokens. This triggers listing cookies so we need to setup cookies
+  // before that.
+  SetupTokens(tokens_after_reconcile_);
+
   EXPECT_CALL(*GetMockReconcilor(), PerformMergeAction(testing::_)).Times(0);
   EXPECT_CALL(*GetMockReconcilor(), PerformLogoutAllAccountsAction()).Times(0);
   EXPECT_CALL(*GetMockReconcilor(), PerformSetCookiesAction(testing::_))
@@ -1171,14 +1175,14 @@ TEST_P(AccountReconcilorTestDiceMultilogin, TableRowTest) {
   ConfigureCookieManagerService(cookies);
   std::vector<Cookie> cookies_after_reconcile = cookies;
 
-  // Setup tokens. This triggers listing cookies so we need to setup cookies
-  // before that.
-  SetupTokens(GetParam().tokens);
-
   // Call list accounts now so that the next call completes synchronously.
   identity_test_env()->identity_manager()->GetAccountsInCookieJar();
   base::RunLoop().RunUntilIdle();
 
+  // Setup tokens. This triggers listing cookies so we need to setup cookies
+  // before that.
+  SetupTokens(GetParam().tokens);
+
   // Setup expectations.
   testing::InSequence mock_sequence;
   if (GetParam().gaia_api_calls_multilogin[0] != '\0') {
@@ -1669,9 +1673,6 @@ TEST_P(AccountReconcilorTestMirrorMultilogin, TableRowTest) {
   // Enable Mirror.
   SetAccountConsistency(signin::AccountConsistencyMethod::kMirror);
 
-  // Setup tokens.
-  SetupTokens(GetParam().tokens);
-
   // Setup cookies.
   std::vector<Cookie> cookies = ParseCookieString(GetParam().cookies);
   ConfigureCookieManagerService(cookies);
@@ -1680,6 +1681,9 @@ TEST_P(AccountReconcilorTestMirrorMultilogin, TableRowTest) {
   identity_test_env()->identity_manager()->GetAccountsInCookieJar();
   base::RunLoop().RunUntilIdle();
 
+  // Setup tokens.
+  SetupTokens(GetParam().tokens);
+
   // Setup expectations.
   testing::InSequence mock_sequence;
   bool logout_action = false;
@@ -1790,10 +1794,6 @@ const std::vector<AccountReconcilorTestTableParam> kActiveDirectoryParams = {
 // clang-format on
 
 TEST_P(AccountReconcilorTestActiveDirectory, TableRowTestMultilogin) {
-  // Setup tokens.
-  std::vector<Token> tokens = ParseTokenString(GetParam().tokens);
-  SetupTokens(GetParam().tokens);
-
   // Setup cookies.
   std::vector<Cookie> cookies = ParseCookieString(GetParam().cookies);
   ConfigureCookieManagerService(cookies);
@@ -1802,6 +1802,10 @@ TEST_P(AccountReconcilorTestActiveDirectory, TableRowTestMultilogin) {
   identity_test_env()->identity_manager()->GetAccountsInCookieJar();
   base::RunLoop().RunUntilIdle();
 
+  // Setup tokens.
+  std::vector<Token> tokens = ParseTokenString(GetParam().tokens);
+  SetupTokens(GetParam().tokens);
+
   testing::InSequence mock_sequence;
   MockAccountReconcilor* reconcilor = GetMockReconcilor(
       std::make_unique<signin::ActiveDirectoryAccountReconcilorDelegate>());

components/signin/core/browser/mirror_account_reconcilor_delegate.cc

@@ -5,7 +5,9 @@
 #include "components/signin/core/browser/mirror_account_reconcilor_delegate.h"
 
 #include "base/logging.h"
+#include "build/build_config.h"
 #include "components/signin/core/browser/account_reconcilor.h"
+#include "components/signin/public/base/account_consistency_method.h"
 
 namespace signin {
 
@@ -14,6 +16,8 @@ MirrorAccountReconcilorDelegate::MirrorAccountReconcilorDelegate(
     : identity_manager_(identity_manager) {
   DCHECK(identity_manager_);
   identity_manager_->AddObserver(this);
+  reconcile_enabled_ =
+      identity_manager_->HasPrimaryAccount(GetConsentLevelForPrimaryAccount());
 }
 
 MirrorAccountReconcilorDelegate::~MirrorAccountReconcilorDelegate() {
@@ -21,7 +25,7 @@ MirrorAccountReconcilorDelegate::~MirrorAccountReconcilorDelegate() {
 }
 
 bool MirrorAccountReconcilorDelegate::IsReconcileEnabled() const {
-  return identity_manager_->HasPrimaryAccount();
+  return reconcile_enabled_;
 }
 
 bool MirrorAccountReconcilorDelegate::IsAccountConsistencyEnforced() const {
@@ -37,6 +41,16 @@ bool MirrorAccountReconcilorDelegate::ShouldAbortReconcileIfPrimaryHasError()
   return true;
 }
 
+ConsentLevel MirrorAccountReconcilorDelegate::GetConsentLevelForPrimaryAccount()
+    const {
+#if defined(OS_ANDROID)
+  if (base::FeatureList::IsEnabled(kMobileIdentityConsistency)) {
+    return ConsentLevel::kNotRequired;
+  }
+#endif
+  return ConsentLevel::kSync;
+}
+
 CoreAccountId MirrorAccountReconcilorDelegate::GetFirstGaiaAccountForReconcile(
     const std::vector<CoreAccountId>& chrome_accounts,
     const std::vector<gaia::ListedAccount>& gaia_accounts,
@@ -61,14 +75,38 @@ MirrorAccountReconcilorDelegate::GetChromeAccountsForReconcile(
                                            gaia_accounts);
 }
 
+// TODO(https://crbug.com/1046746): Replace separate IdentityManager::Observer
+// method overrides below with a single OnPrimaryAccountChanged method and
+// inline UpdateReconcilorStatus.
 void MirrorAccountReconcilorDelegate::OnPrimaryAccountSet(
     const CoreAccountInfo& primary_account_info) {
-  reconcilor()->EnableReconcile();
+  UpdateReconcilorStatus();
 }
 
 void MirrorAccountReconcilorDelegate::OnPrimaryAccountCleared(
     const CoreAccountInfo& previous_primary_account_info) {
+  UpdateReconcilorStatus();
+}
+
+void MirrorAccountReconcilorDelegate::OnUnconsentedPrimaryAccountChanged(
+    const CoreAccountInfo& unconsented_primary_account_info) {
+  UpdateReconcilorStatus();
+}
+
+void MirrorAccountReconcilorDelegate::UpdateReconcilorStatus() {
+  // Have to check whether the state has actually changed, as calling
+  // DisableReconcile logs out all accounts even if it was already disabled.
+  bool should_enable_reconcile =
+      identity_manager_->HasPrimaryAccount(GetConsentLevelForPrimaryAccount());
+  if (reconcile_enabled_ == should_enable_reconcile)
+    return;
+
+  reconcile_enabled_ = should_enable_reconcile;
+  if (should_enable_reconcile) {
+    reconcilor()->EnableReconcile();
+  } else {
     reconcilor()->DisableReconcile(true /* logout_all_gaia_accounts */);
+  }
 }
 
 }  // namespace signin

components/signin/core/browser/mirror_account_reconcilor_delegate.h

@@ -34,6 +34,7 @@ class MirrorAccountReconcilorDelegate : public AccountReconcilorDelegate,
   bool IsAccountConsistencyEnforced() const override;
   gaia::GaiaSource GetGaiaApiSource() const override;
   bool ShouldAbortReconcileIfPrimaryHasError() const override;
+  ConsentLevel GetConsentLevelForPrimaryAccount() const override;
   CoreAccountId GetFirstGaiaAccountForReconcile(
       const std::vector<CoreAccountId>& chrome_accounts,
       const std::vector<gaia::ListedAccount>& gaia_accounts,
@@ -51,8 +52,13 @@ class MirrorAccountReconcilorDelegate : public AccountReconcilorDelegate,
       const CoreAccountInfo& primary_account_info) override;
   void OnPrimaryAccountCleared(
       const CoreAccountInfo& previous_primary_account_info) override;
+  void OnUnconsentedPrimaryAccountChanged(
+      const CoreAccountInfo& unconsented_primary_account_info) override;
+
+  void UpdateReconcilorStatus();
 
   IdentityManager* identity_manager_;
+  bool reconcile_enabled_;
 
   DISALLOW_COPY_AND_ASSIGN(MirrorAccountReconcilorDelegate);
 };