Fix cross origin check when deciding to show the HTTP auth interstitial.

HTTP auth prompts show a blank interstitial which causes the omnibox to show
 the correct URL and page contents to clear in order to prevent phishing. The
logic for this interstitial currently checks only for the visible URL. This misses
the case where the user navigates directly the URL: The URL of the resource
with HTTP Auth will be the same as the visible entry so no interstitial will be
shown. This patch adds a check to see if the last committed entry is also
different then the URL of the resource with HTTP Auth.

BUG=396961
TEST=LoginPromptBrowserTest.*

Review URL: https://codereview.chromium.org/410373003

Cr-Commit-Position: refs/heads/master@{#290090}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290090 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/net/websocket_browsertest.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/login/login_prompt.h"
+#include "chrome/browser/ui/login/login_prompt_test_utils.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -235,7 +236,8 @@ IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, WebSocketBasicAuthInHTTPSURL) {
 // This test verifies that login details entered by the user into the login
 // prompt to authenticate the main page are re-used for WebSockets from the same
 // origin.
-IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, WebSocketBasicAuthPrompt) {
+IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest,
+                       ReuseMainPageBasicAuthCredentialsForWebSocket) {
   // Launch a basic-auth-protected WebSocket server.
   ws_server_.set_websocket_basic_auth(true);
   ASSERT_TRUE(ws_server_.Start());
@@ -244,7 +246,9 @@ IN_PROC_BROWSER_TEST_F(WebSocketBrowserTest, WebSocketBasicAuthPrompt) {
       &browser()->tab_strip_model()->GetActiveWebContents()->GetController();
   AutoLogin auto_login("test", "test", navigation_controller);
 
+  WindowedAuthNeededObserver auth_needed_waiter(navigation_controller);
   NavigateToHTTP("connect_check.html");
+  auth_needed_waiter.Wait();
 
   EXPECT_TRUE(auto_login.logged_in());
   EXPECT_EQ("PASS", WaitAndGetTitle());

chrome/browser/ui/login/login_prompt.cc

@@ -510,8 +510,17 @@ void LoginDialogCallback(const GURL& request_url,
     return;
   }
 
+  // Check if the request is cross origin. There are two different ways the
+  // navigation can occur:
+  // 1- The user enters the resource URL in the omnibox.
+  // 2- The page redirects to the resource.
+  // In both cases, the last committed URL is different than the resource URL,
+  // so checking it is sufficient.
+  // Note that (1) will not be true once site isolation is enabled, as any
+  // navigation could cause a cross-process swap, including link clicks.
   if (is_main_frame &&
-      parent_contents->GetVisibleURL().GetOrigin() != request_url.GetOrigin()) {
+      parent_contents->GetLastCommittedURL().GetOrigin() !=
+          request_url.GetOrigin()) {
     // Show a blank interstitial for main-frame, cross origin requests
     // so that the correct URL is shown in the omnibox.
     base::Closure callback = base::Bind(&ShowLoginPrompt,

chrome/browser/ui/login/login_prompt_browsertest.cc

@@ -61,6 +61,9 @@ class LoginPromptBrowserTest : public InProcessBrowserTest {
 
   void SetAuthFor(LoginHandler* handler);
 
+  void TestCrossOriginPrompt(const GURL& visit_url,
+                             const std::string& landing_host) const;
+
   AuthMap auth_map_;
   std::string bad_password_;
   std::string bad_username_;
@@ -783,7 +786,7 @@ IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
       // When a cross origin iframe displays a login prompt, the blank
       // interstitial shouldn't be displayed and the omnibox should show the
       // main frame's url, not the iframe's.
-      EXPECT_EQ(new_host, contents->GetURL().host());
+      EXPECT_EQ(new_host, contents->GetVisibleURL().host());
 
       handler->CancelAuth();
       auth_cancelled_waiter.Wait();
@@ -791,7 +794,7 @@ IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
   }
 
   // Should stay on the main frame's url once the prompt the iframe is closed.
-  EXPECT_EQ("www.a.com", contents->GetURL().host());
+  EXPECT_EQ("www.a.com", contents->GetVisibleURL().host());
 
   EXPECT_EQ(1, observer.auth_needed_count());
   EXPECT_TRUE(test_server()->Stop());
@@ -1179,12 +1182,9 @@ IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
 
 // If a cross origin navigation triggers a login prompt, the destination URL
 // should be shown in the omnibox.
-IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
-                       ShowCorrectUrlForCrossOriginMainFrameRequests) {
-  const char* kTestPage = "files/login/cross_origin.html";
-  host_resolver()->AddRule("www.a.com", "127.0.0.1");
-  ASSERT_TRUE(test_server()->Start());
-
+void LoginPromptBrowserTest::TestCrossOriginPrompt(
+    const GURL& visit_url,
+    const std::string& auth_host) const {
   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   NavigationController* controller = &contents->GetController();
@@ -1192,23 +1192,20 @@ IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
 
   observer.Register(content::Source<NavigationController>(controller));
 
-  // Load a page which navigates to a cross origin page with a login prompt.
+  // Load a page which will trigger a login prompt.
   {
-    GURL test_page = test_server()->GetURL(kTestPage);
-    ASSERT_EQ("127.0.0.1", test_page.host());
-
     WindowedAuthNeededObserver auth_needed_waiter(controller);
     browser()->OpenURL(OpenURLParams(
-        test_page, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_TYPED,
+        visit_url, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_TYPED,
         false));
-    ASSERT_EQ("127.0.0.1", contents->GetURL().host());
+    ASSERT_EQ(visit_url.host(), contents->GetVisibleURL().host());
     auth_needed_waiter.Wait();
     ASSERT_EQ(1u, observer.handlers().size());
     WaitForInterstitialAttach(contents);
 
     // The omnibox should show the correct origin for the new page when the
     // login prompt is shown.
-    EXPECT_EQ("www.a.com", contents->GetURL().host());
+    EXPECT_EQ(auth_host, contents->GetVisibleURL().host());
     EXPECT_TRUE(contents->ShowingInterstitialPage());
 
     // Cancel and wait for the interstitial to detach.
@@ -1221,9 +1218,35 @@ IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
     handler->CancelAuth();
     if (content::InterstitialPage::GetInterstitialPage(contents))
       loop_runner->Run();
-    EXPECT_EQ("www.a.com", contents->GetURL().host());
+    EXPECT_EQ(auth_host, contents->GetVisibleURL().host());
     EXPECT_FALSE(contents->ShowingInterstitialPage());
   }
 }
 
+// If a cross origin direct navigation triggers a login prompt, the login
+// interstitial should be shown.
+IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
+                       ShowCorrectUrlForCrossOriginMainFrameRequests) {
+  ASSERT_TRUE(test_server()->Start());
+
+  GURL test_page = test_server()->GetURL(kAuthBasicPage);
+  ASSERT_EQ("127.0.0.1", test_page.host());
+  std::string auth_host("127.0.0.1");
+  TestCrossOriginPrompt(test_page, auth_host);
+}
+
+// If a cross origin redirect triggers a login prompt, the destination URL
+// should be shown in the omnibox when the auth dialog is displayed.
+IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
+                       ShowCorrectUrlForCrossOriginMainFrameRedirects) {
+  host_resolver()->AddRule("www.a.com", "127.0.0.1");
+  ASSERT_TRUE(test_server()->Start());
+
+  const char* kTestPage = "files/login/cross_origin.html";
+  GURL test_page = test_server()->GetURL(kTestPage);
+  ASSERT_EQ("127.0.0.1", test_page.host());
+  std::string auth_host("www.a.com");
+  TestCrossOriginPrompt(test_page, auth_host);
+}
+
 }  // namespace