Open captive portal login pages in popup windows in secure mode.

Secure DNS will be disabled in these windows, allowing users to
complete the captive portal resolution process.

Future cls will add additional conditions that will trigger captive
portal probes in secure mode.

Change-Id: I441d17465a9c9d11567775321ed3ddd8532598ab
Bug: 10161646
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984663Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Katharine Daly <dalyk@google.com>
Cr-Commit-Position: refs/heads/master@{#734218}

chrome/browser/captive_portal/captive_portal_browsertest.cc

@@ -28,17 +28,21 @@
 #include "base/test/bind_test_util.h"
 #include "base/values.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/captive_portal/captive_portal_service.h"
 #include "chrome/browser/captive_portal/captive_portal_service_factory.h"
 #include "chrome/browser/captive_portal/captive_portal_tab_helper.h"
 #include "chrome/browser/captive_portal/captive_portal_tab_reloader.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/net/dns_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/ssl_error_handler.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/navigation_correction_tab_observer.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_iterator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -84,6 +88,10 @@
 #include "net/url_request/url_request_status.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_WIN)
+#include "base/win/win_util.h"
+#endif
+
 using captive_portal::CaptivePortalResult;
 using content::BrowserThread;
 using content::WebContents;
@@ -162,6 +170,11 @@ std::string CreateServerRedirect(const std::string& dest_url) {
   return kServerRedirectBase + dest_url;
 }
 
+// Returns the total number of tabs across all Browsers, for all Profiles.
+int NumTabs() {
+  return std::distance(AllTabContentses().begin(), AllTabContentses().end());
+}
+
 // Returns the total number of loading tabs across all Browsers, for all
 // Profiles.
 int NumLoadingTabs() {
@@ -633,15 +646,18 @@ class CaptivePortalBrowserTest : public InProcessBrowserTest {
   // captive portal test is complete.
   //
   // If |expect_open_login_tab| is true, a login tab is then expected to be
-  // opened. It waits until both the login tab has finished loading, and two
-  // captive portal tests complete.  The second test is triggered by the load of
-  // the captive portal tab completing.
+  // opened. If |expect_new_login_browser| is true, this tab is expected to be
+  // opened in a new captive portal popup window.  It waits until both the
+  // login tab has finished loading, and two captive portal tests complete.
+  // The second test is triggered by the load of the captive portal tab
+  // completing.
   //
   // This function must not be called when the active tab is currently loading.
   // Waits for the hanging request to be issued, so other functions can rely
   // on WaitForJobs having been called.
   void SlowLoadBehindCaptivePortal(Browser* browser,
-                                   bool expect_open_login_tab);
+                                   bool expect_open_login_tab,
+                                   bool expect_new_login_browser);
 
   // Same as above, but takes extra parameters.
   //
@@ -652,6 +668,7 @@ class CaptivePortalBrowserTest : public InProcessBrowserTest {
   // ignored when |expect_open_login_tab| is false.
   void SlowLoadBehindCaptivePortal(Browser* browser,
                                    bool expect_open_login_tab,
+                                   bool expect_new_login_browser,
                                    const GURL& hanging_url,
                                    int expected_portal_checks,
                                    int expected_login_tab_navigations);
@@ -686,9 +703,12 @@ class CaptivePortalBrowserTest : public InProcessBrowserTest {
   // Simulates a login by updating the URLRequestMockCaptivePortalJob's
   // behind captive portal state, and navigating the login tab.  Waits for
   // all broken but not loading tabs to be reloaded.
+  // |captive-portal_browser| is the browser containing the login page.
   // |num_loading_tabs| and |num_timed_out_tabs| are used as extra checks
   // that nothing has gone wrong prior to the function call.
-  void Login(Browser* browser, int num_loading_tabs, int num_timed_out_tabs);
+  void Login(Browser* captive_portal_browser,
+             int num_loading_tabs,
+             int num_timed_out_tabs);
 
   // Simulates a login when the broken tab shows an SSL or captive portal
   // interstitial. Can't use Login() in those cases because the interstitial
@@ -876,6 +896,7 @@ class CaptivePortalBrowserTest : public InProcessBrowserTest {
   std::vector<content::URLLoaderInterceptor::RequestParams>
       ongoing_mock_requests_;
   std::atomic<bool> behind_captive_portal_;
+  const BrowserList* browser_list_;
   bool intercept_bad_cert_ = true;
 
  private:
@@ -883,7 +904,7 @@ class CaptivePortalBrowserTest : public InProcessBrowserTest {
 };
 
 CaptivePortalBrowserTest::CaptivePortalBrowserTest()
-    : behind_captive_portal_(true) {
+    : behind_captive_portal_(true), browser_list_(BrowserList::GetInstance()) {
   NavigationCorrectionTabObserver::SetAllowEnableCorrectionsForTesting(true);
 }
 
@@ -892,6 +913,12 @@ CaptivePortalBrowserTest::~CaptivePortalBrowserTest() {
 }
 
 void CaptivePortalBrowserTest::SetUpOnMainThread() {
+  // Mark as not enterprise managed to prevent the secure DNS mode from
+  // being downgraded to off.
+#if defined(OS_WIN)
+  base::win::ScopedDomainStateForTesting scoped_domain(false);
+#endif
+
   url_loader_interceptor_ =
       std::make_unique<content::URLLoaderInterceptor>(base::Bind(
           &CaptivePortalBrowserTest::OnIntercept, base::Unretained(this)));
@@ -1236,17 +1263,17 @@ void CaptivePortalBrowserTest::FastTimeoutNoCaptivePortal(
 
 void CaptivePortalBrowserTest::SlowLoadBehindCaptivePortal(
     Browser* browser,
-    bool expect_open_login_tab) {
-  SlowLoadBehindCaptivePortal(browser,
-                              expect_open_login_tab,
-                              GURL(kMockHttpsUrl),
-                              1,
+    bool expect_open_login_tab,
+    bool expect_new_login_browser = false) {
+  SlowLoadBehindCaptivePortal(browser, expect_open_login_tab,
+                              expect_new_login_browser, GURL(kMockHttpsUrl), 1,
                               1);
 }
 
 void CaptivePortalBrowserTest::SlowLoadBehindCaptivePortal(
     Browser* browser,
     bool expect_open_login_tab,
+    bool expect_new_login_browser,
     const GURL& hanging_url,
     int expected_portal_checks,
     int expected_login_tab_navigations) {
@@ -1268,6 +1295,7 @@ void CaptivePortalBrowserTest::SlowLoadBehindCaptivePortal(
   int initial_active_index = tab_strip_model->active_index();
   int initial_loading_tabs = NumLoadingTabs();
   int expected_broken_tabs = NumBrokenTabs();
+  size_t initial_browser_count = browser_list_->size();
   if (CaptivePortalTabReloader::STATE_BROKEN_BY_PORTAL !=
           GetStateOfTabReloader(tab_strip_model->GetActiveWebContents())) {
     ++expected_broken_tabs;
@@ -1282,19 +1310,39 @@ void CaptivePortalBrowserTest::SlowLoadBehindCaptivePortal(
 
   if (expect_open_login_tab) {
     ASSERT_GE(expected_login_tab_navigations, 1);
-
     navigation_observer.WaitForNavigations(expected_login_tab_navigations);
+    WebContents* login_tab;
+
+    if (expect_new_login_browser) {
+      ASSERT_EQ(initial_browser_count + 1, browser_list_->size());
 
+      // Check the original browser
+      ASSERT_EQ(initial_tab_count, tab_strip_model->count());
+      EXPECT_EQ(initial_tab_count - 1, tab_strip_model->active_index());
+      EXPECT_NE(browser_list_->get(initial_browser_count - 1),
+                browser_list_->GetLastActive());
+
+      // Check the new popup browser
+      Browser* popup_browser = browser_list_->get(initial_browser_count);
+      EXPECT_EQ(popup_browser, browser_list_->GetLastActive());
+      EXPECT_EQ(Browser::TYPE_POPUP, popup_browser->type());
+      login_tab = popup_browser->tab_strip_model()->GetWebContentsAt(0);
+      EXPECT_TRUE(CaptivePortalTabHelper::FromWebContents(login_tab)
+                      ->is_captive_portal_window());
+    } else {
+      ASSERT_EQ(initial_browser_count, browser_list_->size());
       ASSERT_EQ(initial_tab_count + 1, tab_strip_model->count());
       EXPECT_EQ(initial_tab_count, tab_strip_model->active_index());
+      login_tab = tab_strip_model->GetWebContentsAt(initial_tab_count);
+    }
 
     EXPECT_EQ(expected_login_tab_navigations,
-              navigation_observer.NumNavigationsForTab(
-                  tab_strip_model->GetWebContentsAt(initial_tab_count)));
+              navigation_observer.NumNavigationsForTab(login_tab));
     EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
-              GetStateOfTabReloaderAt(browser, 1));
-    EXPECT_TRUE(IsLoginTab(tab_strip_model->GetWebContentsAt(1)));
+              GetStateOfTabReloader(login_tab));
+    EXPECT_TRUE(IsLoginTab(login_tab));
   } else {
+    ASSERT_EQ(initial_browser_count, browser_list_->size());
     EXPECT_EQ(0, navigation_observer.num_navigations());
     EXPECT_EQ(initial_active_index, tab_strip_model->active_index());
     ASSERT_EQ(initial_tab_count, tab_strip_model->count());
@@ -1459,24 +1507,25 @@ void CaptivePortalBrowserTest::NavigateLoginTab(Browser* browser,
                    tab_strip_model->GetWebContentsAt(login_tab_index)));
 }
 
-void CaptivePortalBrowserTest::Login(Browser* browser,
+void CaptivePortalBrowserTest::Login(Browser* captive_portal_browser,
                                      int num_loading_tabs,
                                      int num_timed_out_tabs) {
   // Simulate logging in.
   SetBehindCaptivePortal(false);
 
   MultiNavigationObserver navigation_observer;
-  CaptivePortalObserver portal_observer(browser->profile());
+  CaptivePortalObserver portal_observer(captive_portal_browser->profile());
 
-  TabStripModel* tab_strip_model = browser->tab_strip_model();
-  int initial_tab_count = tab_strip_model->count();
+  TabStripModel* tab_strip_model = captive_portal_browser->tab_strip_model();
+  size_t initial_browser_count = browser_list_->size();
+  int initial_tab_count = NumTabs();
   ASSERT_EQ(num_loading_tabs, NumLoadingTabs());
   EXPECT_EQ(num_timed_out_tabs, NumBrokenTabs() - NumLoadingTabs());
 
   // Verify that the login page is on top.
   int login_tab_index = tab_strip_model->active_index();
   EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
-            GetStateOfTabReloaderAt(browser, login_tab_index));
+            GetStateOfTabReloaderAt(captive_portal_browser, login_tab_index));
   ASSERT_TRUE(IsLoginTab(tab_strip_model->GetWebContentsAt(login_tab_index)));
 
   // Trigger a navigation.
@@ -1496,9 +1545,10 @@ void CaptivePortalBrowserTest::Login(Browser* browser,
 
   // Make sure that the broken tabs have reloaded, and there's no more
   // captive portal tab.
-  EXPECT_EQ(initial_tab_count, tab_strip_model->count());
+  EXPECT_EQ(initial_browser_count, browser_list_->size());
+  EXPECT_EQ(initial_tab_count, NumTabs());
   EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
-            GetStateOfTabReloaderAt(browser, login_tab_index));
+            GetStateOfTabReloaderAt(captive_portal_browser, login_tab_index));
   EXPECT_FALSE(IsLoginTab(tab_strip_model->GetWebContentsAt(login_tab_index)));
 
   // Make sure there were no unexpected navigations of the login tab.
@@ -1588,8 +1638,9 @@ void CaptivePortalBrowserTest::FailLoadsWithoutLogin(Browser* browser,
   ASSERT_EQ(0, NumNeedReloadTabs());
   EXPECT_EQ(num_loading_tabs, NumBrokenTabs());
 
-  TabStripModel* tab_strip_model = browser->tab_strip_model();
-  int initial_num_tabs = tab_strip_model->count();
+  Browser* captive_portal_browser = browser_list_->GetLastActive();
+  TabStripModel* tab_strip_model = captive_portal_browser->tab_strip_model();
+  int initial_num_tabs = NumTabs();
   int login_tab = tab_strip_model->active_index();
   EXPECT_EQ(CaptivePortalTabReloader::STATE_NONE,
             GetStateOfTabReloader(tab_strip_model->GetActiveWebContents()));
@@ -1607,7 +1658,7 @@ void CaptivePortalBrowserTest::FailLoadsWithoutLogin(Browser* browser,
   // should be no new tabs.
   EXPECT_EQ(0, portal_observer.num_results_received());
   EXPECT_FALSE(CheckPending(browser));
-  EXPECT_EQ(initial_num_tabs, tab_strip_model->count());
+  EXPECT_EQ(initial_num_tabs, NumTabs());
 
   EXPECT_EQ(0, NumNeedReloadTabs());
   EXPECT_EQ(0, NumLoadingTabs());
@@ -1633,7 +1684,9 @@ void CaptivePortalBrowserTest::RunNavigateLoadingTabToTimeoutTest(
   SetBehindCaptivePortal(true);
 
   // Go to the first hanging url.
-  SlowLoadBehindCaptivePortal(browser, true, hanging_url, 1, 1);
+  SlowLoadBehindCaptivePortal(browser, true /* expect_open_login_tab */,
+                              false /* is-expect_new_login_browser */,
+                              hanging_url, 1, 1);
 
   // Abandon the request.
   WaitForJobs(1);
@@ -2649,8 +2702,9 @@ IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest, DISABLED_TwoWindows) {
 IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest, HttpToHttpsRedirectLogin) {
   ASSERT_TRUE(embedded_test_server()->Start());
   SlowLoadBehindCaptivePortal(
-      browser(), true, embedded_test_server()->GetURL(kRedirectToMockHttpsPath),
-      1, 1);
+      browser(), true /* expect_open_login_tab */,
+      false /* expect_new_login_browser */,
+      embedded_test_server()->GetURL(kRedirectToMockHttpsPath), 1, 1);
   Login(browser(), 1, 0);
   FailLoadsAfterLogin(browser(), 1);
 }
@@ -2677,7 +2731,9 @@ IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest, HttpsToHttpRedirect) {
 IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest, Status511) {
   SetUpCaptivePortalService(browser()->profile(),
                             GURL(kMockCaptivePortal511Url));
-  SlowLoadBehindCaptivePortal(browser(), true, GURL(kMockHttpsUrl), 2, 2);
+  SlowLoadBehindCaptivePortal(browser(), true /* expect_open_login_tab */,
+                              false /* expect_new_login_browser */,
+                              GURL(kMockHttpsUrl), 2, 2);
   Login(browser(), 1, 0);
   FailLoadsAfterLogin(browser(), 1);
 }
@@ -2704,7 +2760,9 @@ IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest,
   TabStripModel* tab_strip_model = browser()->tab_strip_model();
   int broken_tab_index = tab_strip_model->active_index();
   WebContents* broken_tab_contents = tab_strip_model->GetActiveWebContents();
-  SlowLoadBehindCaptivePortal(browser(), true, cert_error_url, 1, 1);
+  SlowLoadBehindCaptivePortal(browser(), true /* expect_open_login_tab */,
+                              false /* expect_new_login_browser */,
+                              cert_error_url, 1, 1);
 
   // No longer behind a captive portal. Committing the SSL page should trigger
   // an SSL interstitial which triggers a new captive portal check. Since there
@@ -2731,3 +2789,34 @@ IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest,
   EXPECT_EQ(SSLBlockingPage::kTypeForTesting,
             GetInterstitialType(broken_tab_contents));
 }
+
+IN_PROC_BROWSER_TEST_F(CaptivePortalBrowserTest, SecureDnsCaptivePortal) {
+  PrefService* local_state = g_browser_process->local_state();
+  local_state->SetString(prefs::kDnsOverHttpsMode,
+                         chrome_browser_net::kDnsOverHttpsModeSecure);
+  local_state->SetString(prefs::kDnsOverHttpsTemplates,
+                         "https://bar.test/dns-query{?dns}");
+
+  SlowLoadBehindCaptivePortal(browser(), true /* expect_open_login_tab */,
+                              true /* expect_new_login_browser */);
+  FailLoadsWithoutLogin(browser(), 1);
+
+  // A second navigation on the same tab should not cause a new captive portal
+  // window to open.
+  SlowLoadBehindCaptivePortal(browser(), false /* expect_open_login_tab */,
+                              false /* expect_new_login_browser */);
+
+  // A navigation in a new tab should not cause a new captive portal window to
+  // open.
+  Browser* second_user_browser = CreateBrowser(browser()->profile());
+  // Check that new window is visible.
+  EXPECT_TRUE(second_user_browser->window()->IsVisible());
+  SlowLoadBehindCaptivePortal(second_user_browser,
+                              false /* expect_open_login_tab */,
+                              false /* expect_new_login_browser */);
+  // Check that the existing captive portal popup is visible.
+  EXPECT_TRUE(browser_list_->get(1)->window()->IsVisible());
+
+  // Login to the captive portal.
+  Login(browser_list_->get(1), 2, 0);
+}

chrome/browser/captive_portal/captive_portal_tab_helper.cc

@@ -6,14 +6,18 @@
 
 #include "base/bind.h"
 #include "base/debug/dump_without_crashing.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/captive_portal/captive_portal_login_detector.h"
 #include "chrome/browser/captive_portal/captive_portal_service_factory.h"
 #include "chrome/browser/captive_portal/captive_portal_tab_reloader.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
@@ -26,6 +30,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/net_errors.h"
+#include "net/dns/dns_config.h"
 #include "net/ssl/ssl_info.h"
 
 using captive_portal::CaptivePortalResult;
@@ -161,6 +166,48 @@ void CaptivePortalTabHelper::OpenLoginTabForWebContents(
   if (!browser)
     return;
 
+  bool insecure_stub_resolver_enabled;
+  net::DnsConfig::SecureDnsMode secure_dns_mode;
+  base::Optional<std::vector<network::mojom::DnsOverHttpsServerPtr>>
+      dns_over_https_servers;
+  SystemNetworkContextManager::GetStubResolverConfig(
+      g_browser_process->local_state(), &insecure_stub_resolver_enabled,
+      &secure_dns_mode, &dns_over_https_servers);
+
+  // If the DNS mode is SECURE, captive portal login tabs should be opened in
+  // new popup windows where secure DNS will be disabled.
+  if (secure_dns_mode == net::DnsConfig::SecureDnsMode::SECURE) {
+    // If there is already a captive portal popup window, do not create another.
+    for (auto* contents : AllTabContentses()) {
+      CaptivePortalTabHelper* captive_portal_tab_helper =
+          CaptivePortalTabHelper::FromWebContents(contents);
+      if (captive_portal_tab_helper->IsLoginTab()) {
+        Browser* browser_with_login_tab =
+            chrome::FindBrowserWithWebContents(contents);
+        browser_with_login_tab->window()->Show();
+        browser_with_login_tab->tab_strip_model()->ActivateTabAt(
+            browser_with_login_tab->tab_strip_model()->GetIndexOfWebContents(
+                contents));
+        return;
+      }
+    }
+
+    // Otherwise, create a captive portal popup window.
+    NavigateParams params(
+        browser,
+        CaptivePortalServiceFactory::GetForProfile(browser->profile())
+            ->test_url(),
+        ui::PAGE_TRANSITION_TYPED);
+    params.disposition = WindowOpenDisposition::NEW_POPUP;
+    Navigate(&params);
+    content::WebContents* new_contents = params.navigated_or_inserted_contents;
+    CaptivePortalTabHelper* captive_portal_tab_helper =
+        CaptivePortalTabHelper::FromWebContents(new_contents);
+    captive_portal_tab_helper->set_is_captive_portal_window();
+    captive_portal_tab_helper->SetIsLoginTab();
+    return;
+  }
+
   // Check if the Profile's topmost browser window already has a login tab.
   // If so, do nothing.
   // TODO(mmenke):  Consider focusing that tab, at least if this is the tab