Skip to content

GitLab

T
tangled
Commit 4e3ec245 authored by Emily Stark's avatar Emily Stark Committed by Commit Bot
Browse files
Options

Do not cancel http auth navigations for incorrect credentials 

Previously, with HTTP auth committed interstitials, LoginTabHelper
cancels navigations that receive 401 responses and commits a blank
page. The auth prompt is shown on top of the blank page
normally. However, in the case that the browser supplied credentials
and the server still sent a 401 response (i.e., credentials were
incorrect), LoginTabHelper would cancel the navigation but not show
the auth prompt (due to lack of auth challenge -- the challenge is
only populated when credentials aren't supplied in the request). This
led to a weird situation where neither the auth prompt nor the 401
response body from the server would be displayed to the user. To
rectify this, LoginTabHelper should not cancel a request with a 401
response unless there is an auth challenge present for which to show a
prompt. This maintains the same UX as we had before committed
interstitials.

Bug: 1047742
Change-Id: I83eba22e52676946d7f3a6cffc14f699265f75d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036970Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738298}
parent 7e2b687f
Hide whitespace changes
Inline Side-by-side
Showing with 79 additions and 0 deletions
chrome/browser/ui/login/login_handler_browsertest.cc
View file @ 4e3ec245
......@@ -2150,6 +2150,70 @@ IN_PROC_BROWSER_TEST_P(LoginPromptBrowserTest, FtpAuthWithCache) {
EXPECT_EQ(0u, observer.handlers().size());
}
namespace {
// A request handler that returns a 401 Unauthorized response on the
// /unauthorized path.
std::unique_ptr<net::test_server::HttpResponse> HandleUnauthorized(
const net::test_server::HttpRequest& request) {
if (request.relative_url != "/unauthorized") {
return nullptr;
}
std::unique_ptr<net::test_server::BasicHttpResponse> response =
std::make_unique<net::test_server::BasicHttpResponse>();
response->set_code(net::HTTP_UNAUTHORIZED);
response->set_content("<html><body>Unauthorized</body></html>");
return response;
}
} // namespace
// Tests that 401 responses are not cancelled and replaced with a blank page
// when incorrect credentials were supplied in the request. See
// https://crbug.com/1047742.
IN_PROC_BROWSER_TEST_P(LoginPromptBrowserTest,
ResponseNotCancelledWithIncorrectCredentials) {
// Register a custom handler that returns a 401 Unauthorized response
// regardless of what credentials were supplied in the request.
embedded_test_server()->RegisterRequestHandler(
base::BindRepeating(&HandleUnauthorized));
ASSERT_TRUE(embedded_test_server()->Start());
GURL test_page = embedded_test_server()->GetURL(kAuthBasicPage);
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
// Navigate to a page that prompts basic auth and fill in correct
// credentials. A subsequent navigation handled by HandleUnauthorized() will
// send the credentials cached from the navigation to |test_page|, but return
// a 401 Unauthorized response.
NavigationController* controller = &web_contents->GetController();
LoginPromptBrowserTestObserver observer;
observer.Register(content::Source<NavigationController>(controller));
WindowedAuthNeededObserver auth_needed_waiter(controller);
ui_test_utils::NavigateToURL(browser(), test_page);
auth_needed_waiter.Wait();
WindowedAuthSuppliedObserver auth_supplied_waiter(controller);
LoginHandler* handler = *observer.handlers().begin();
SetAuthFor(handler);
auth_supplied_waiter.Wait();
base::string16 expected_title = ExpectedTitleFromAuth(
base::ASCIIToUTF16("basicuser"), base::ASCIIToUTF16("secret"));
content::TitleWatcher title_watcher(web_contents, expected_title);
EXPECT_EQ(expected_title, title_watcher.WaitAndGetTitle());
// Now navigate to a page handled by HandleUnauthorized(), for which the
// cached credentials are incorrect.
ui_test_utils::NavigateToURL(browser(),
embedded_test_server()->GetURL("/unauthorized"));
// Test that the 401 response body is rendered, instead of the navigation
// being cancelled and a blank error page committing.
EXPECT_EQ(false,
content::EvalJs(
web_contents,
"document.body.innerHTML.indexOf('Unauthorized') === -1"));
}
class ProxyBrowserTestWithHttpAuthCommittedInterstitials
: public ProxyBrowserTest {
public:
......
chrome/browser/ui/login/login_handler_unittest.cc
View file @ 4e3ec245
......@@ -164,7 +164,9 @@ TEST(LoginHandlerTest, DialogStringsAndRealm) {
TEST_F(LoginHandlerWithWebContentsTest, NoPendingEntryDoesNotCrash) {
LoginTabHelper::CreateForWebContents(web_contents());
LoginTabHelper* helper = LoginTabHelper::FromWebContents(web_contents());
net::AuthChallengeInfo challenge;
content::MockNavigationHandle handle;
handle.SetAuthChallengeInfo(challenge);
content::NavigationThrottle::ThrottleCheckResult result =
helper->WillProcessMainFrameUnauthorizedResponse(&handle);
EXPECT_EQ(content::NavigationThrottle::CANCEL, result.action());
......
chrome/browser/ui/login/login_tab_helper.cc
View file @ 4e3ec245
......@@ -145,6 +145,13 @@ LoginTabHelper::WillProcessMainFrameUnauthorizedResponse(
return content::NavigationThrottle::PROCEED;
}
// Do not cancel the navigation if there is no auth challenge. We only want to
// cancel the navigation below to show a blank page if there is an auth
// challenge for which to show a login prompt.
if (!navigation_handle->GetAuthChallengeInfo()) {
return content::NavigationThrottle::PROCEED;
}
// Otherwise, rewrite the response to a blank page. DidFinishNavigation will
// show a login prompt on top of this blank page.
return {content::NavigationThrottle::CANCEL,
......
content/public/test/mock_navigation_handle.cc
View file @ 4e3ec245
......@@ -32,4 +32,9 @@ MockNavigationHandle::MockNavigationHandle(const GURL& url,
MockNavigationHandle::~MockNavigationHandle() = default;
void MockNavigationHandle::SetAuthChallengeInfo(
const net::AuthChallengeInfo& challenge) {
auth_challenge_info_ = challenge;
}
} // namespace content
content/public/test/mock_navigation_handle.h
View file @ 4e3ec245
......@@ -90,6 +90,7 @@ class MockNavigationHandle : public NavigationHandle {
override {
return auth_challenge_info_;
}
void SetAuthChallengeInfo(const net::AuthChallengeInfo& challenge);
net::ResolveErrorInfo GetResolveErrorInfo() override {
return resolve_error_info_;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment