Adapt GaiaRemoteConsentFlow to support kWebAuthFlowInBrowserTab

Modification so that getAuthToken() can now use a tab instead of a app
window using kWebAuthFlowInBrowserTab flag.
Also connected GaiaRemoteConsentFlow to react to the injected JS in the
Gaia page for declined/aproved consent.
Adapted browser_tests to test with kWebAuthFlowInBrowserTab active as well.

Change-Id: I0b2b6dc6cffdf0e0cee99f89fdfd62c668edfc69
Bug: 1408402
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4120278
Commit-Queue: Ryan Sultanem <rsult@google.com>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096712}

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

@@ -12,6 +12,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/account_consistency_mode_manager.h"
 #include "chrome/browser/signin/account_reconcilor_factory.h"
+#include "chrome/browser/signin/google_accounts_private_api_host.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/common/chrome_features.h"
 #include "components/signin/core/browser/account_reconcilor.h"
@@ -72,19 +73,44 @@ void GaiaRemoteConsentFlow::Start() {
         WebAuthFlow::GET_AUTH_TOKEN, extension_name_);
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
     // `profile_` may be nullptr in tests.
-    if (profile_) {
+    if (profile_ &&
+        !base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab)) {
       AccountReconcilorFactory::GetForProfile(profile_)
           ->GetConsistencyCookieManager()
           ->AddExtraCookieManager(GetCookieManagerForPartition());
     }
 #endif
   }
 
+  if (base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab)) {
+    StartWebFlow();
+    return;
+  }
+
   SetAccountsInCookie();
 }
 
+void GaiaRemoteConsentFlow::StartWebFlow() {
+  network::mojom::CookieManager* cookie_manager =
+      GetCookieManagerForPartition();
+  net::CookieOptions options;
+  for (const auto& cookie : resolution_data_.cookies) {
+    cookie_manager->SetCanonicalCookie(
+        cookie,
+        net::cookie_util::SimulatedCookieSource(cookie, url::kHttpsScheme),
+        options, network::mojom::CookieManager::SetCanonicalCookieCallback());
+  }
+
+  web_flow_->Start();
+  web_flow_started_ = true;
+}
+
 void GaiaRemoteConsentFlow::OnSetAccountsComplete(
     signin::SetAccountsInCookieResult result) {
+  // No need to inject account cookies when the flow is displayed in a browser
+  // tab.
+  DCHECK(!base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab));
+
   set_accounts_in_cookie_task_.reset();
   if (web_flow_started_) {
     return;
@@ -96,16 +122,6 @@ void GaiaRemoteConsentFlow::OnSetAccountsComplete(
     return;
   }
 
-  network::mojom::CookieManager* cookie_manager =
-      GetCookieManagerForPartition();
-  net::CookieOptions options;
-  for (const auto& cookie : resolution_data_.cookies) {
-    cookie_manager->SetCanonicalCookie(
-        cookie,
-        net::cookie_util::SimulatedCookieSource(cookie, url::kHttpsScheme),
-        options, network::mojom::CookieManager::SetCanonicalCookieCallback());
-  }
-
   identity_api_set_consent_result_subscription_ =
       IdentityAPI::GetFactoryInstance()
           ->Get(profile_)
@@ -114,18 +130,11 @@ void GaiaRemoteConsentFlow::OnSetAccountsComplete(
                                   base::Unretained(this)));
 
   scoped_observation_.Observe(IdentityManagerFactory::GetForProfile(profile_));
-  web_flow_->Start();
-  web_flow_started_ = true;
+  StartWebFlow();
 }
 
-void GaiaRemoteConsentFlow::OnConsentResultSet(
-    const std::string& consent_result,
-    const std::string& window_id) {
-  if (!web_flow_ || window_id != web_flow_->GetAppWindowKey())
-    return;
-
-  identity_api_set_consent_result_subscription_ = {};
-
+void GaiaRemoteConsentFlow::ReactToConsentResult(
+    const std::string& consent_result) {
   bool consent_approved = false;
   std::string gaia_id;
   if (!gaia::ParseOAuth2MintTokenConsentResult(consent_result,
@@ -143,6 +152,21 @@ void GaiaRemoteConsentFlow::OnConsentResultSet(
   delegate_->OnGaiaRemoteConsentFlowApproved(consent_result, gaia_id);
 }
 
+void GaiaRemoteConsentFlow::OnConsentResultSet(
+    const std::string& consent_result,
+    const std::string& window_id) {
+  // JS hook in a browser tab calls `ReactToConsentResult()` directly.
+  DCHECK(!base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab));
+
+  if (!web_flow_ || window_id != web_flow_->GetAppWindowKey()) {
+    return;
+  }
+
+  identity_api_set_consent_result_subscription_ = {};
+
+  ReactToConsentResult(consent_result);
+}
+
 void GaiaRemoteConsentFlow::OnAuthFlowFailure(WebAuthFlow::Failure failure) {
   GaiaRemoteConsentFlow::Failure gaia_failure;
 
@@ -192,6 +216,9 @@ GaiaRemoteConsentFlow::GetCookieManagerForPartition() {
 }
 
 void GaiaRemoteConsentFlow::OnEndBatchOfRefreshTokenStateChanges() {
+  // No need to copy added accounts when showing the flow in a browser tab.
+  DCHECK(!base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab));
+
 // On ChromeOS, new accounts are added through the account manager. They need to
 // be pushed to the partition used by this flow explicitly.
 // On Desktop, sign-in happens on the Web and a new account is directly added to
@@ -226,6 +253,10 @@ WebAuthFlow* GaiaRemoteConsentFlow::GetWebAuthFlowForTesting() const {
 }
 
 void GaiaRemoteConsentFlow::SetAccountsInCookie() {
+  // No need to inject account cookies when the flow is displayed in a browser
+  // tab.
+  DCHECK(!base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab));
+
   // Reset a task that is already in flight because it contains stale
   // information.
   if (set_accounts_in_cookie_task_)
@@ -278,7 +309,8 @@ void GaiaRemoteConsentFlow::DetachWebAuthFlow() {
 
 #if BUILDFLAG(IS_CHROMEOS_LACROS)
   // `profile_` may be nullptr in tests.
-  if (profile_) {
+  if (profile_ &&
+      !base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab)) {
     AccountReconcilorFactory::GetForProfile(profile_)
         ->GetConsistencyCookieManager()
         ->RemoveExtraCookieManager(GetCookieManagerForPartition());
@@ -287,4 +319,18 @@ void GaiaRemoteConsentFlow::DetachWebAuthFlow() {
   web_flow_.release()->DetachDelegateAndDelete();
 }
 
+void GaiaRemoteConsentFlow::OnNavigationFinished(
+    content::NavigationHandle* navigation_handle) {
+  // No need to create the receiver if we are not displaying the auth page
+  // through a Browser Tgab.
+  if (!base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab)) {
+    return;
+  }
+
+  GoogleAccountsPrivateApiHost::CreateReceiver(
+      base::BindRepeating(&GaiaRemoteConsentFlow::ReactToConsentResult,
+                          weak_factory.GetWeakPtr()),
+      navigation_handle);
+}
+
 }  // namespace extensions

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

@@ -7,6 +7,7 @@
 
 #include "base/callback_list.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/scoped_observation.h"
 #include "chrome/browser/extensions/api/identity/extension_token_key.h"
 #include "chrome/browser/extensions/api/identity/web_auth_flow.h"
@@ -66,12 +67,19 @@ class GaiaRemoteConsentFlow
   // Set accounts in cookie completion callback.
   void OnSetAccountsComplete(signin::SetAccountsInCookieResult result);
 
-  // setConsentResult() JavaScript callback.
+  // setConsentResult() JavaScript callback when using an App Window to display
+  // the Auth page.
   void OnConsentResultSet(const std::string& consent_result,
                           const std::string& window_id);
 
+  // Handles `consent_result` value when using either a Browser Tab or an App
+  // Window to display the Auth page.
+  void ReactToConsentResult(const std::string& consent_result);
+
   // WebAuthFlow::Delegate:
   void OnAuthFlowFailure(WebAuthFlow::Failure failure) override;
+  void OnNavigationFinished(
+      content::NavigationHandle* navigation_handle) override;
 
   // signin::AccountsCookieMutator::PartitionDelegate:
   std::unique_ptr<GaiaAuthFetcher> CreateGaiaAuthFetcherForPartition(
@@ -87,6 +95,8 @@ class GaiaRemoteConsentFlow
   WebAuthFlow* GetWebAuthFlowForTesting() const;
 
  private:
+  void StartWebFlow();
+
   void SetAccountsInCookie();
 
   void GaiaRemoteConsentFlowFailed(Failure failure);
@@ -110,6 +120,8 @@ class GaiaRemoteConsentFlow
   base::ScopedObservation<signin::IdentityManager,
                           signin::IdentityManager::Observer>
       scoped_observation_{this};
+
+  base::WeakPtrFactory<GaiaRemoteConsentFlow> weak_factory{this};
 };
 
 }  // namespace extensions

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

@@ -4,15 +4,18 @@
 
 #include "chrome/browser/extensions/api/identity/gaia_remote_consent_flow.h"
 
+#include "base/strings/strcat.h"
 #include "chrome/browser/extensions/api/identity/identity_private_api.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "components/signin/public/base/consent_level.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/signin/public/identity_manager/identity_test_utils.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/api_test_utils.h"
 #include "google_apis/gaia/core_account_id.h"
@@ -54,9 +57,11 @@ class MockGaiaRemoteConsentFlowDelegate
                     const std::string& gaia_id));
 };
 
-class GaiaRemoteConsentFlowBrowserTest : public InProcessBrowserTest {
+class GaiaRemoteConsentFlowParamBrowserTest
+    : public InProcessBrowserTest,
+      public testing::WithParamInterface<bool> {
  public:
-  GaiaRemoteConsentFlowBrowserTest()
+  GaiaRemoteConsentFlowParamBrowserTest()
       : fake_gaia_test_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
 #if BUILDFLAG(IS_CHROMEOS_ASH)
     std::unique_ptr<ash::MockNetworkPortalDetector>
@@ -74,6 +79,9 @@ class GaiaRemoteConsentFlowBrowserTest : public InProcessBrowserTest {
     fake_gaia_test_server()->AddDefaultHandlers(GetChromeTestDataDir());
     fake_gaia_test_server_.RegisterRequestHandler(base::BindRepeating(
         &FakeGaia::HandleRequest, base::Unretained(&fake_gaia_)));
+
+    scoped_feature_list_.InitWithFeatureState(
+        features::kWebAuthFlowInBrowserTab, GetParam());
   }
 
   void SetUp() override {
@@ -148,13 +156,31 @@ class GaiaRemoteConsentFlowBrowserTest : public InProcessBrowserTest {
     navigation_observer.Wait();
   }
 
-  void SimulateConsentResult(const std::string& consent_value,
-                             const std::string& window_id) {
+  void SimulateConsentResult(const std::string& consent_value) {
+    // When the auth flow is using the browser tab, we are able to properly test
+    // the JS injected script since we rely on the Gaia Origin to filter out
+    // unwanted urls, and in the test we are overriding the value of Gaia
+    // Origin, so we can bypass the filter for testing.
+    if (base::FeatureList::IsEnabled(features::kWebAuthFlowInBrowserTab)) {
+      // JS function is properly called but returns nullptr.
+      ASSERT_EQ(nullptr, content::EvalJs(
+                             flow()->GetWebAuthFlowForTesting()->web_contents(),
+                             "window.OAuthConsent.setConsentResult(\"" +
+                                 consent_value + "\")"));
+      return;
+    }
+
+    // Since we cannot bypass the filter that is added in the internal extension
+    // (in it's manifest) we do not directly test the JS function but instead
+    // the layer right above in the API through
+    // `IdentityPrivateSetConsentResultFunction`.
+    std::string consent_result =
+        "[\"" + consent_value + "\", \"" +
+        flow()->GetWebAuthFlowForTesting()->GetAppWindowKey() + "\"]";
     scoped_refptr<ExtensionFunction> func =
         base::MakeRefCounted<IdentityPrivateSetConsentResultFunction>();
-    ASSERT_TRUE(api_test_utils::RunFunction(
-        func.get(), "[\"" + consent_value + "\", \"" + window_id + "\"]",
-        profile()));
+    ASSERT_TRUE(
+        api_test_utils::RunFunction(func.get(), consent_result, profile()));
   }
 
   MockGaiaRemoteConsentFlowDelegate& mock() {
@@ -176,41 +202,50 @@ class GaiaRemoteConsentFlowBrowserTest : public InProcessBrowserTest {
 
   net::EmbeddedTestServer fake_gaia_test_server_;
   FakeGaia fake_gaia_;
+
+  base::test::ScopedFeatureList scoped_feature_list_;
 };
 
-IN_PROC_BROWSER_TEST_F(GaiaRemoteConsentFlowBrowserTest,
+IN_PROC_BROWSER_TEST_P(GaiaRemoteConsentFlowParamBrowserTest,
                        SimulateInvalidConsent) {
   LaunchAndWaitGaiaRemoteConsentFlow();
 
   EXPECT_CALL(mock(),
               OnGaiaRemoteConsentFlowFailed(
                   GaiaRemoteConsentFlow::Failure::INVALID_CONSENT_RESULT));
-  SimulateConsentResult("invalid_consent",
-                        flow()->GetWebAuthFlowForTesting()->GetAppWindowKey());
+  SimulateConsentResult("invalid_consent");
 }
 
-IN_PROC_BROWSER_TEST_F(GaiaRemoteConsentFlowBrowserTest, SimulateNoGrant) {
+IN_PROC_BROWSER_TEST_P(GaiaRemoteConsentFlowParamBrowserTest, SimulateNoGrant) {
   LaunchAndWaitGaiaRemoteConsentFlow();
 
   EXPECT_CALL(mock(), OnGaiaRemoteConsentFlowFailed(
                           GaiaRemoteConsentFlow::Failure::NO_GRANT));
   std::string declined_consent = gaia::GenerateOAuth2MintTokenConsentResult(
       /*approved=*/false, "consent_not_granted", kGaiaId);
-  SimulateConsentResult(declined_consent,
-                        flow()->GetWebAuthFlowForTesting()->GetAppWindowKey());
+  SimulateConsentResult(declined_consent);
 }
 
-IN_PROC_BROWSER_TEST_F(GaiaRemoteConsentFlowBrowserTest,
+IN_PROC_BROWSER_TEST_P(GaiaRemoteConsentFlowParamBrowserTest,
                        SimulateAccessGranted) {
   LaunchAndWaitGaiaRemoteConsentFlow();
 
   std::string approved_consent = gaia::GenerateOAuth2MintTokenConsentResult(
       /*approved=*/true, "consent_granted", kGaiaId);
   EXPECT_CALL(mock(),
               OnGaiaRemoteConsentFlowApproved(approved_consent, kGaiaId));
-  SimulateConsentResult(approved_consent,
-                        flow()->GetWebAuthFlowForTesting()->GetAppWindowKey());
+  SimulateConsentResult(approved_consent);
 }
 
+INSTANTIATE_TEST_SUITE_P(
+    ,
+    GaiaRemoteConsentFlowParamBrowserTest,
+    testing::Bool(),
+    [](const testing::TestParamInfo<
+        GaiaRemoteConsentFlowParamBrowserTest::ParamType>& info) {
+      return base::StrCat({"WebAuthFlowInBrowserTab_",
+                           info.param ? "FeatureOn" : "FeatureOff"});
+    });
+
 }  // namespace extensions
 #endif  // !BUILDFLAG(IS_CHROMEOS_LACROS)