Fix Mirror cookie validity mismatch

In certain cases, Gaia's /ListAccounts APIs claims accounts to be in a
valid state when they are actually invalid. This causes issues with
account reconciliation because |AccountReconcilor| uses /ListAccounts as
the source of truth and is unable to detect this state.
In this state, certain Google properties like Gmail for example send a
Mirror re-authentication header for the offending account. However, even
after the user re-authenticates the account, account reconciliation ends
up being a no-op because the reconcilor thinks that the user's cookies
are valid and there is nothing to reconcile.

Fix this by storing the accounts for which we receive Mirror re-
authentication headers from Gaia and calling LogOutAllAccounts to
remint cookie when tokens for these accounts are updated.
Please check the attached bug for more context.

Note: This is a temporary client-side fix until Gaia fixes its
/ListAccounts API.

Bug: 1012649
Change-Id: I797a049d59fe32ea406cf9ddbbbaa94bc2b57b6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1871591
Commit-Queue: Anastasiia Nikolaienko <anastasiian@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713402}

chrome/browser/BUILD.gn

@@ -1625,6 +1625,8 @@ jumbo_static_library("browser") {
     "signin/chrome_signin_proxying_url_loader_factory.h",
     "signin/chrome_signin_url_loader_throttle.cc",
     "signin/chrome_signin_url_loader_throttle.h",
+    "signin/cookie_reminter_factory.cc",
+    "signin/cookie_reminter_factory.h",
     "signin/header_modification_delegate.h",
     "signin/header_modification_delegate_impl.cc",
     "signin/header_modification_delegate_impl.h",

chrome/browser/signin/chrome_signin_helper.cc

@@ -24,6 +24,7 @@
 #include "chrome/browser/signin/account_reconcilor_factory.h"
 #include "chrome/browser/signin/chrome_signin_client.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
+#include "chrome/browser/signin/cookie_reminter_factory.h"
 #include "chrome/browser/signin/dice_response_handler.h"
 #include "chrome/browser/signin/dice_tab_helper.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
@@ -34,6 +35,7 @@
 #include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
 #include "chrome/common/url_constants.h"
 #include "components/signin/core/browser/account_reconcilor.h"
+#include "components/signin/core/browser/cookie_reminter.h"
 #include "components/signin/public/base/account_consistency_method.h"
 #include "components/signin/public/base/signin_buildflags.h"
 #include "content/public/browser/browser_task_traits.h"
@@ -197,7 +199,8 @@ void ProcessMirrorHeader(
   //    have been forced through an online in-browser sign-in for sensitive
   //    webpages, thereby decreasing their session validity. After their session
   //    expires, they will receive a "Mirror" re-authentication request for all
-  //    Google web properties.
+  //    Google web properties. Another case when this can be triggered is
+  //    https://crbug.com/1012649.
   // 3. Displaying the Account Manager for managing accounts.
 
   // 1. Going incognito.
@@ -217,6 +220,27 @@ void ProcessMirrorHeader(
       return;
     }
 
+    // The account's cookie is invalid but the cookie has not been removed by
+    // |AccountReconcilor|. Ideally, this should not happen. At this point,
+    // |AccountReconcilor| cannot detect this state because its source of truth
+    // (/ListAccounts) is giving us false positives (claiming an invalid account
+    // to be valid). We need to store that this account's cookie is actually
+    // invalid, so that if/when this account is re-authenticated, we can force a
+    // reconciliation for this account instead of treating it as a no-op.
+    // See https://crbug.com/1012649 for details.
+    signin::IdentityManager* const identity_manager =
+        IdentityManagerFactory::GetForProfile(profile);
+    base::Optional<AccountInfo> maybe_account_info =
+        identity_manager
+            ->FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
+                manage_accounts_params.email);
+    if (maybe_account_info.has_value()) {
+      CookieReminter* const cookie_reminter =
+          CookieReminterFactory::GetForProfile(profile);
+      cookie_reminter->ForceCookieRemintingOnNextTokenUpdate(
+          maybe_account_info.value());
+    }
+
     // Display a re-authentication dialog.
     chromeos::InlineLoginHandlerDialogChromeOS::Show(
         manage_accounts_params.email);

chrome/browser/signin/cookie_reminter_factory.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/signin/cookie_reminter_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/signin/core/browser/cookie_reminter.h"
+
+CookieReminterFactory::CookieReminterFactory()
+    : BrowserContextKeyedServiceFactory(
+          "CookieReminter",
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(IdentityManagerFactory::GetInstance());
+}
+
+CookieReminterFactory::~CookieReminterFactory() {}
+
+// static
+CookieReminter* CookieReminterFactory::GetForProfile(Profile* profile) {
+  return static_cast<CookieReminter*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+// static
+CookieReminterFactory* CookieReminterFactory::GetInstance() {
+  return base::Singleton<CookieReminterFactory>::get();
+}
+
+KeyedService* CookieReminterFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  Profile* profile = Profile::FromBrowserContext(context);
+  signin::IdentityManager* identity_manager =
+      IdentityManagerFactory::GetForProfile(profile);
+  return new CookieReminter(identity_manager);
+}

chrome/browser/signin/cookie_reminter_factory.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SIGNIN_COOKIE_REMINTER_FACTORY_H_
+#define CHROME_BROWSER_SIGNIN_COOKIE_REMINTER_FACTORY_H_
+
+#include <memory>
+
+#include "base/memory/singleton.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+class CookieReminter;
+class Profile;
+
+class CookieReminterFactory : public BrowserContextKeyedServiceFactory {
+ public:
+  static CookieReminter* GetForProfile(Profile* profile);
+  static CookieReminterFactory* GetInstance();
+
+ private:
+  friend struct base::DefaultSingletonTraits<CookieReminterFactory>;
+
+  CookieReminterFactory();
+  ~CookieReminterFactory() override;
+
+  // BrowserContextKeyedServiceFactory:
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* profile) const override;
+};
+
+#endif  // CHROME_BROWSER_SIGNIN_COOKIE_REMINTER_FACTORY_H_

components/signin/core/browser/BUILD.gn

@@ -23,6 +23,8 @@ static_library("browser") {
     "chrome_connected_header_helper.h",
     "consistency_cookie_manager_base.cc",
     "consistency_cookie_manager_base.h",
+    "cookie_reminter.cc",
+    "cookie_reminter.h",
     "cookie_settings_util.cc",
     "cookie_settings_util.h",
     "dice_account_reconcilor_delegate.cc",

components/signin/core/browser/cookie_reminter.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/signin/core/browser/cookie_reminter.h"
+
+#include "base/syslog_logging.h"
+#include "components/signin/public/identity_manager/accounts_cookie_mutator.h"
+
+namespace {
+
+bool DoesAccountRequireCookieReminting(
+    const std::vector<CoreAccountInfo>& accounts_requiring_cookie_remint,
+    const CoreAccountInfo& account_info) {
+  for (const CoreAccountInfo& account_requiring_remint :
+       accounts_requiring_cookie_remint) {
+    if (account_info.gaia == account_requiring_remint.gaia) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+}  // namespace
+
+CookieReminter::CookieReminter(signin::IdentityManager* identity_manager)
+    : identity_manager_(identity_manager) {
+  identity_manager_->AddObserver(this);
+}
+
+CookieReminter::~CookieReminter() {
+  identity_manager_->RemoveObserver(this);
+}
+
+void CookieReminter::ForceCookieRemintingOnNextTokenUpdate(
+    const CoreAccountInfo& account_info) {
+  if (DoesAccountRequireCookieReminting(accounts_requiring_cookie_remint_,
+                                        account_info)) {
+    return;
+  }
+
+  accounts_requiring_cookie_remint_.emplace_back(account_info);
+}
+
+void CookieReminter::OnRefreshTokenUpdatedForAccount(
+    const CoreAccountInfo& account_info) {
+  if (DoesAccountRequireCookieReminting(accounts_requiring_cookie_remint_,
+                                        account_info)) {
+    // Cookies are going to be reminted for all accounts.
+    accounts_requiring_cookie_remint_.clear();
+    identity_manager_->GetAccountsCookieMutator()->LogOutAllAccounts(
+        gaia::GaiaSource::kChromeOS);
+  }
+}

components/signin/core/browser/cookie_reminter.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+#ifndef COMPONENTS_SIGNIN_CORE_BROWSER_COOKIE_REMINTER_H_
+#define COMPONENTS_SIGNIN_CORE_BROWSER_COOKIE_REMINTER_H_
+
+#include <vector>
+
+#include "components/keyed_service/core/keyed_service.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
+
+// Stores accounts with invalid cookies, which cannot be detected by
+// /ListAccounts, so we can force cookie reminting when account is
+// reauthenticated.
+//
+// Subscribed to |OnRefreshTokenUpdatedForAccount| of IdentityManager, calls
+// |AccountsCookieMutator::LogOutAllAccounts| after refresh token update of
+// any of the accounts that have been added to
+// |ForceCookieRemintingOnNextTokenUpdate|.
+class CookieReminter : public KeyedService,
+                       public signin::IdentityManager::Observer {
+ public:
+  explicit CookieReminter(signin::IdentityManager* identity_manager);
+  ~CookieReminter() override;
+
+  // Forces a cookie reminting if/when the refresh token for |account_info| is
+  // updated.
+  void ForceCookieRemintingOnNextTokenUpdate(
+      const CoreAccountInfo& account_info);
+
+ private:
+  // Overridden from signin::IdentityManager::Observer.
+  void OnRefreshTokenUpdatedForAccount(
+      const CoreAccountInfo& account_info) override;
+
+  signin::IdentityManager* identity_manager_;
+  std::vector<CoreAccountInfo> accounts_requiring_cookie_remint_;
+};
+
+#endif  // COMPONENTS_SIGNIN_CORE_BROWSER_COOKIE_REMINTER_H_