[identity] Remote consent flow cookie update

On ChromeOS, new accounts are added through the system settings. Since
the remote consent flow uses a separate context for showing the consent
page, an account change in settings won't update cookies automatically.

This CL sets a listener for account updates in GaiaRemoteConsentFlow
that triggers a cookie update. There is no need to reload the consent
page because the Gaia OAuth page already handles cookie updates.

Bug: 1026237
Change-Id: Ie69a21e3a9715ba4993a16b222f1e07af0c4fdd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144201
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758819}

chrome/browser/extensions/api/identity/gaia_remote_consent_flow.cc

@@ -33,7 +33,9 @@ GaiaRemoteConsentFlow::GaiaRemoteConsentFlow(
     : delegate_(delegate),
       profile_(profile),
       account_id_(token_key.account_id),
-      resolution_data_(resolution_data) {}
+      resolution_data_(resolution_data),
+      web_flow_started_(false),
+      scoped_observer_(this) {}
 
 GaiaRemoteConsentFlow::~GaiaRemoteConsentFlow() {
   if (web_flow_)
@@ -46,40 +48,16 @@ void GaiaRemoteConsentFlow::Start() {
         this, profile_, resolution_data_.url, WebAuthFlow::INTERACTIVE);
   }
 
-  auto* identity_manager = IdentityManagerFactory::GetForProfile(profile_);
-  std::vector<CoreAccountId> accounts;
-  if (IdentityAPI::GetFactoryInstance()
-          ->Get(profile_)
-          ->AreExtensionsRestrictedToPrimaryAccount()) {
-    CoreAccountId primary_account_id = identity_manager->GetPrimaryAccountId();
-    accounts.push_back(primary_account_id);
-  } else {
-    auto chrome_accounts_with_refresh_tokens =
-        identity_manager->GetAccountsWithRefreshTokens();
-    for (const auto& chrome_account : chrome_accounts_with_refresh_tokens) {
-      // An account in persistent error state would make multilogin fail.
-      // Showing only a subset of accounts seems to be a better alternative than
-      // failing with an error.
-      if (identity_manager->HasAccountWithRefreshTokenInPersistentErrorState(
-              chrome_account.account_id)) {
-        continue;
-      }
-      accounts.push_back(chrome_account.account_id);
-    }
-  }
-
-  set_accounts_in_cookie_task_ =
-      identity_manager->GetAccountsCookieMutator()
-          ->SetAccountsInCookieForPartition(
-              this,
-              {gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER,
-               accounts},
-              base::BindOnce(&GaiaRemoteConsentFlow::OnSetAccountsComplete,
-                             base::Unretained(this)));
+  SetAccountsInCookie();
 }
 
 void GaiaRemoteConsentFlow::OnSetAccountsComplete(
     signin::SetAccountsInCookieResult result) {
+  set_accounts_in_cookie_task_.reset();
+  if (web_flow_started_) {
+    return;
+  }
+
   if (result != signin::SetAccountsInCookieResult::kSuccess) {
     delegate_->OnGaiaRemoteConsentFlowFailed(
         GaiaRemoteConsentFlow::Failure::SET_ACCOUNTS_IN_COOKIE_FAILED);
@@ -102,8 +80,9 @@ void GaiaRemoteConsentFlow::OnSetAccountsComplete(
               base::Bind(&GaiaRemoteConsentFlow::OnConsentResultSet,
                          base::Unretained(this)));
 
-  set_accounts_in_cookie_task_.reset();
+  scoped_observer_.Add(IdentityManagerFactory::GetForProfile(profile_));
   web_flow_->Start();
+  web_flow_started_ = true;
 }
 
 void GaiaRemoteConsentFlow::OnConsentResultSet(
@@ -163,6 +142,10 @@ GaiaRemoteConsentFlow::GetCookieManagerForPartition() {
   return web_flow_->GetGuestPartition()->GetCookieManagerForBrowserProcess();
 }
 
+void GaiaRemoteConsentFlow::OnEndBatchOfRefreshTokenStateChanges() {
+  SetAccountsInCookie();
+}
+
 void GaiaRemoteConsentFlow::SetWebAuthFlowForTesting(
     std::unique_ptr<WebAuthFlow> web_auth_flow) {
   if (web_flow_)
@@ -170,4 +153,44 @@ void GaiaRemoteConsentFlow::SetWebAuthFlowForTesting(
   web_flow_ = std::move(web_auth_flow);
 }
 
+void GaiaRemoteConsentFlow::SetAccountsInCookie() {
+  // Reset a task that is already in flight because it contains stale
+  // information.
+  if (set_accounts_in_cookie_task_)
+    set_accounts_in_cookie_task_.reset();
+
+  auto* identity_manager = IdentityManagerFactory::GetForProfile(profile_);
+  std::vector<CoreAccountId> accounts;
+  if (IdentityAPI::GetFactoryInstance()
+          ->Get(profile_)
+          ->AreExtensionsRestrictedToPrimaryAccount()) {
+    CoreAccountId primary_account_id = identity_manager->GetPrimaryAccountId();
+    accounts.push_back(primary_account_id);
+  } else {
+    auto chrome_accounts_with_refresh_tokens =
+        identity_manager->GetAccountsWithRefreshTokens();
+    for (const auto& chrome_account : chrome_accounts_with_refresh_tokens) {
+      // An account in persistent error state would make multilogin fail.
+      // Showing only a subset of accounts seems to be a better alternative than
+      // failing with an error.
+      if (identity_manager->HasAccountWithRefreshTokenInPersistentErrorState(
+              chrome_account.account_id)) {
+        continue;
+      }
+      accounts.push_back(chrome_account.account_id);
+    }
+  }
+
+  // base::Unretained() is safe here because this class owns
+  // |set_accounts_in_cookie_task_| that will eventually invoke this callback.
+  set_accounts_in_cookie_task_ =
+      identity_manager->GetAccountsCookieMutator()
+          ->SetAccountsInCookieForPartition(
+              this,
+              {gaia::MultiloginMode::MULTILOGIN_UPDATE_COOKIE_ACCOUNTS_ORDER,
+               accounts},
+              base::BindOnce(&GaiaRemoteConsentFlow::OnSetAccountsComplete,
+                             base::Unretained(this)));
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/identity/gaia_remote_consent_flow.h

@@ -8,9 +8,12 @@
 #include "base/callback_list.h"
 
 #include "base/macros.h"
+#include "base/scoped_observer.h"
 #include "chrome/browser/extensions/api/identity/extension_token_key.h"
 #include "chrome/browser/extensions/api/identity/web_auth_flow.h"
 #include "components/signin/public/identity_manager/accounts_cookie_mutator.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
+#include "components/signin/public/identity_manager/set_accounts_in_cookie_result.h"
 #include "google_apis/gaia/core_account_id.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "google_apis/gaia/oauth2_mint_token_flow.h"
@@ -19,7 +22,8 @@ namespace extensions {
 
 class GaiaRemoteConsentFlow
     : public WebAuthFlow::Delegate,
-      public signin::AccountsCookieMutator::PartitionDelegate {
+      public signin::AccountsCookieMutator::PartitionDelegate,
+      public signin::IdentityManager::Observer {
  public:
   enum Failure {
     WINDOW_CLOSED,
@@ -60,27 +64,37 @@ class GaiaRemoteConsentFlow
   void OnConsentResultSet(const std::string& consent_result,
                           const std::string& window_id);
 
-  // WebAuthFlow::Delegate implementation.
+  // WebAuthFlow::Delegate:
   void OnAuthFlowFailure(WebAuthFlow::Failure failure) override;
 
-  // AccountsCookieMutator::PartitionDelegate:
+  // signin::AccountsCookieMutator::PartitionDelegate:
   std::unique_ptr<GaiaAuthFetcher> CreateGaiaAuthFetcherForPartition(
       GaiaAuthConsumer* consumer) override;
   network::mojom::CookieManager* GetCookieManagerForPartition() override;
 
+  // signin::IdentityManager::Observer:
+  void OnEndBatchOfRefreshTokenStateChanges() override;
+
   void SetWebAuthFlowForTesting(std::unique_ptr<WebAuthFlow> web_auth_flow);
 
  private:
+  void SetAccountsInCookie();
+
   Delegate* delegate_;
   Profile* profile_;
   CoreAccountId account_id_;
   RemoteConsentResolutionData resolution_data_;
+
   std::unique_ptr<WebAuthFlow> web_flow_;
+  bool web_flow_started_;
+
   std::unique_ptr<signin::AccountsCookieMutator::SetAccountsInCookieTask>
       set_accounts_in_cookie_task_;
   std::unique_ptr<base::CallbackList<void(const std::string&,
                                           const std::string&)>::Subscription>
       identity_api_set_consent_result_subscription_;
+  ScopedObserver<signin::IdentityManager, signin::IdentityManager::Observer>
+      scoped_observer_;
 };
 
 }  // namespace extensions