Fix ForceBrowserSignIn not working with networking service.

The problem was that the old sign-in flow didn't work with networking service. This hasn't been converted to use DICE, which already works.

There were two causes:
-webRequest interception wasn't enabled for the few internal webui pages that are whitelisted for webRequest API
-webRequest users don't see Set-Cookie headers for responses (since these were sent to the interceptor from the network process, and the IPC serialization removes Cookie headers)

Bug: 887381
Change-Id: I4f0b598c1610cd3b11dcf01cdd4d43425f218a4f
Reviewed-on: https://chromium-review.googlesource.com/1238720Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593703}

chrome/browser/ui/webui/signin/inline_login_handler.cc

@@ -153,7 +153,50 @@ void InlineLoginHandler::ContinueHandleInitializeMessage() {
 
 void InlineLoginHandler::HandleCompleteLoginMessage(
     const base::ListValue* args) {
-  CompleteLogin(args);
+  // When the network service is enabled, the webRequest API doesn't expose
+  // cookie headers. So manually fetch the cookies for the GAIA URL from the
+  // CookieManager.
+  content::WebContents* contents = web_ui()->GetWebContents();
+  content::StoragePartition* partition =
+      content::BrowserContext::GetStoragePartitionForSite(
+          contents->GetBrowserContext(), signin::GetSigninPartitionURL());
+
+  net::CookieOptions cookie_options;
+  cookie_options.set_include_httponly();
+
+  partition->GetCookieManagerForBrowserProcess()->GetCookieList(
+      GaiaUrls::GetInstance()->gaia_url(), cookie_options,
+      base::BindOnce(&InlineLoginHandler::HandleCompleteLoginMessageWithCookies,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     base::ListValue(args->GetList())));
+}
+
+void InlineLoginHandler::HandleCompleteLoginMessageWithCookies(
+    const base::ListValue& args,
+    const std::vector<net::CanonicalCookie>& cookies) {
+  const base::DictionaryValue* dict = nullptr;
+  args.GetDictionary(0, &dict);
+
+  const std::string& email = dict->FindKey("email")->GetString();
+  const std::string& password = dict->FindKey("password")->GetString();
+  const std::string& gaia_id = dict->FindKey("gaiaId")->GetString();
+
+  std::string auth_code;
+  for (const auto& cookie : cookies) {
+    if (cookie.Name() == "oauth_code")
+      auth_code = cookie.Value();
+  }
+
+  bool skip_for_now = false;
+  dict->GetBoolean("skipForNow", &skip_for_now);
+  bool trusted = false;
+  bool trusted_found = dict->GetBoolean("trusted", &trusted);
+
+  bool choose_what_to_sync = false;
+  dict->GetBoolean("chooseWhatToSync", &choose_what_to_sync);
+
+  CompleteLogin(email, password, gaia_id, auth_code, skip_for_now, trusted,
+                trusted_found, choose_what_to_sync);
 }
 
 void InlineLoginHandler::HandleSwitchToFullTabMessage(

chrome/browser/ui/webui/signin/inline_login_handler.h

@@ -13,6 +13,10 @@ namespace base {
 class DictionaryValue;
 }
 
+namespace net {
+class CanonicalCookie;
+}
+
 namespace signin_metrics {
 enum class AccessPoint;
 }
@@ -54,6 +58,12 @@ class InlineLoginHandler : public content::WebUIMessageHandler {
   // work.
   void HandleCompleteLoginMessage(const base::ListValue* args);
 
+  // Called by HandleCompleteLoginMessage after it gets the GAIA URL's cookies
+  // from the CookieManager.
+  void HandleCompleteLoginMessageWithCookies(
+      const base::ListValue& args,
+      const std::vector<net::CanonicalCookie>& cookies);
+
   // JS callback to switch the UI from a constrainted dialog to a full tab.
   void HandleSwitchToFullTabMessage(const base::ListValue* args);
 
@@ -65,7 +75,14 @@ class InlineLoginHandler : public content::WebUIMessageHandler {
   void HandleDialogClose(const base::ListValue* args);
 
   virtual void SetExtraInitParams(base::DictionaryValue& params) {}
-  virtual void CompleteLogin(const base::ListValue* args) = 0;
+  virtual void CompleteLogin(const std::string& email,
+                             const std::string& password,
+                             const std::string& gaia_id,
+                             const std::string& auth_code,
+                             bool skip_for_now,
+                             bool trusted,
+                             bool trusted_found,
+                             bool choose_what_to_sync) = 0;
 
   base::WeakPtrFactory<InlineLoginHandler> weak_ptr_factory_;
 

chrome/browser/ui/webui/signin/inline_login_handler_chromeos.cc

@@ -118,15 +118,16 @@ void InlineLoginHandlerChromeOS::SetExtraInitParams(
   params.SetKey("flow", base::Value("addaccount"));
 }
 
-void InlineLoginHandlerChromeOS::CompleteLogin(const base::ListValue* args) {
-  const base::DictionaryValue* auth_data = nullptr;
-  CHECK(args->GetDictionary(0, &auth_data));
-
-  const std::string& auth_code = auth_data->FindKey("authCode")->GetString();
+void InlineLoginHandlerChromeOS::CompleteLogin(const std::string& email,
+                                               const std::string& password,
+                                               const std::string& gaia_id,
+                                               const std::string& auth_code,
+                                               bool skip_for_now,
+                                               bool trusted,
+                                               bool trusted_found,
+                                               bool choose_what_to_sync) {
   CHECK(!auth_code.empty());
-  const std::string& gaia_id = auth_data->FindKey("gaiaId")->GetString();
   CHECK(!gaia_id.empty());
-  const std::string& email = auth_data->FindKey("email")->GetString();
   CHECK(!email.empty());
 
   // TODO(sinhak): Do not depend on Profile unnecessarily.

chrome/browser/ui/webui/signin/inline_login_handler_chromeos.h

@@ -21,7 +21,14 @@ class InlineLoginHandlerChromeOS : public InlineLoginHandler {
 
   // InlineLoginHandler overrides.
   void SetExtraInitParams(base::DictionaryValue& params) override;
-  void CompleteLogin(const base::ListValue* args) override;
+  void CompleteLogin(const std::string& email,
+                     const std::string& password,
+                     const std::string& gaia_id,
+                     const std::string& auth_code,
+                     bool skip_for_now,
+                     bool trusted,
+                     bool trusted_found,
+                     bool choose_what_to_sync) override;
 
  private:
   base::RepeatingClosure close_dialog_closure_;

chrome/browser/ui/webui/signin/inline_login_handler_impl.cc

@@ -484,15 +484,17 @@ void InlineLoginHandlerImpl::SetExtraInitParams(base::DictionaryValue& params) {
   LogHistogramValue(signin_metrics::HISTOGRAM_SHOWN);
 }
 
-void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
+void InlineLoginHandlerImpl::CompleteLogin(const std::string& email,
+                                           const std::string& password,
+                                           const std::string& gaia_id,
+                                           const std::string& auth_code,
+                                           bool skip_for_now,
+                                           bool trusted,
+                                           bool trusted_found,
+                                           bool choose_what_to_sync) {
   content::WebContents* contents = web_ui()->GetWebContents();
   const GURL& current_url = contents->GetURL();
 
-  const base::DictionaryValue* dict = NULL;
-  args->GetDictionary(0, &dict);
-
-  bool skip_for_now = false;
-  dict->GetBoolean("skipForNow", &skip_for_now);
   if (skip_for_now) {
     signin::SetUserSkippedPromo(Profile::FromWebUI(web_ui()));
     SyncStarterCallback(OneClickSigninSyncStarter::SYNC_SETUP_FAILURE);
@@ -500,32 +502,13 @@ void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
   }
 
   // This value exists only for webview sign in.
-  bool trusted = false;
-  if (dict->GetBoolean("trusted", &trusted))
+  if (trusted_found)
     confirm_untrusted_signin_ = !trusted;
 
-  base::string16 email_string16;
-  dict->GetString("email", &email_string16);
-  DCHECK(!email_string16.empty());
-  std::string email(base::UTF16ToASCII(email_string16));
-
-  base::string16 password_string16;
-  dict->GetString("password", &password_string16);
-  std::string password(base::UTF16ToASCII(password_string16));
-
-  base::string16 gaia_id_string16;
-  dict->GetString("gaiaId", &gaia_id_string16);
-  DCHECK(!gaia_id_string16.empty());
-  std::string gaia_id = base::UTF16ToASCII(gaia_id_string16);
-
-  base::string16 auth_code_string16;
-  dict->GetString("authCode", &auth_code_string16);
-  std::string auth_code = base::UTF16ToASCII(auth_code_string16);
+  DCHECK(!email.empty());
+  DCHECK(!gaia_id.empty());
   DCHECK(!auth_code.empty());
 
-  bool choose_what_to_sync = false;
-  dict->GetBoolean("chooseWhatToSync", &choose_what_to_sync);
-
   content::StoragePartition* partition =
       content::BrowserContext::GetStoragePartitionForSite(
           contents->GetBrowserContext(), signin::GetSigninPartitionURL());

chrome/browser/ui/webui/signin/inline_login_handler_impl.h

@@ -50,7 +50,14 @@ class InlineLoginHandlerImpl : public InlineLoginHandler,
  private:
   // InlineLoginHandler overrides:
   void SetExtraInitParams(base::DictionaryValue& params) override;
-  void CompleteLogin(const base::ListValue* args) override;
+  void CompleteLogin(const std::string& email,
+                     const std::string& password,
+                     const std::string& gaia_id,
+                     const std::string& auth_code,
+                     bool skip_for_now,
+                     bool trusted,
+                     bool trusted_found,
+                     bool choose_what_to_sync) override;
 
   // This struct exists to pass paramters to the FinishCompleteLogin() method,
   // since the base::Bind() call does not support this many template args.

extensions/browser/api/web_request/web_request_api.cc

@@ -39,6 +39,7 @@
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/resource_type.h"
+#include "content/public/common/url_constants.h"
 #include "extensions/browser/api/activity_log/web_request_constants.h"
 #include "extensions/browser/api/declarative/rules_registry_service.h"
 #include "extensions/browser/api/declarative_net_request/ruleset_manager.h"
@@ -65,6 +66,7 @@
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/guest_view/guest_view_events.h"
 #include "extensions/browser/guest_view/web_view/web_view_constants.h"
+#include "extensions/browser/guest_view/web_view/web_view_guest.h"
 #include "extensions/browser/info_map.h"
 #include "extensions/browser/io_thread_extension_message_filter.h"
 #include "extensions/browser/runtime_data.h"
@@ -75,6 +77,7 @@
 #include "extensions/common/event_filtering_info.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/features/feature.h"
+#include "extensions/common/features/feature_provider.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/common/url_pattern.h"
 #include "extensions/strings/grit/extensions_strings.h"
@@ -594,8 +597,31 @@ bool WebRequestAPI::MaybeProxyURLLoaderFactory(
     bool is_navigation,
     network::mojom::URLLoaderFactoryRequest* factory_request) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (!MayHaveProxies())
-    return false;
+  if (!MayHaveProxies()) {
+    bool skip_proxy = true;
+    // There are a few internal WebUIs that use WebView tag that are whitelisted
+    // for webRequest.
+    if (frame) {
+      auto* web_contents = content::WebContents::FromRenderFrameHost(frame);
+      if (web_contents && WebViewGuest::IsGuest(web_contents)) {
+        auto* guest_web_contents =
+            WebViewGuest::GetTopLevelWebContents(web_contents);
+        auto& guest_url = guest_web_contents->GetURL();
+        if (guest_url.SchemeIs(content::kChromeUIScheme)) {
+          auto* feature = FeatureProvider::GetAPIFeature("webRequestInternal");
+          if (feature
+                  ->IsAvailableToContext(nullptr, Feature::WEBUI_CONTEXT,
+                                         guest_url)
+                  .is_available()) {
+            skip_proxy = false;
+          }
+        }
+      }
+    }
+
+    if (skip_proxy)
+      return false;
+  }
 
   auto proxied_request = std::move(*factory_request);
   network::mojom::URLLoaderFactoryPtrInfo target_factory_info;