Disable HTTP auth schemes on permanent errors.

The underlying implementation for the Negotiate authentication
scheme might return error codes that indicate error conditions
that we are unlikely to recover from.  If we see those, then
treat these as permanent errors and disable the auth scheme for
the rest of the transaction.  We were already doing this partly
for cases where there are no default credentials.  This patch
extends the behavior to additional error codes.

BUG=49950
TEST=net_unittests --gtest_filter=HttpAuthControllerTest.PermanentErrors

Review URL: http://codereview.chromium.org/6745018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@79848 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 158 insertions, 13 deletions

diff --git a/net/http/http_auth_controller.cc b/net/http/http_auth_controller.cc
index 52fc3f1..f337c30 100644
--- a/net/http/http_auth_controller.cc
+++ b/net/http/http_auth_controller.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -192,13 +192,12 @@ int HttpAuthController::MaybeGenerateAuthToken(const HttpRequestInfo* request,
                                        request,
                                        &io_callback_,
                                        &auth_token_);
+  if (DisableOnAuthHandlerResult(rv))
+    rv = OK;
   if (rv == ERR_IO_PENDING)
     user_callback_ = callback;
   else
     OnIOComplete(rv);
-  // This error occurs with GSSAPI, if the user has not already logged in.
-  if (rv == ERR_MISSING_AUTH_CREDENTIALS)
-    rv = OK;
   return rv;
 }
 
@@ -244,9 +243,13 @@ void HttpAuthController::AddAuthorizationHeader(
     HttpRequestHeaders* authorization_headers) {
   DCHECK(CalledOnValidThread());
   DCHECK(HaveAuth());
-  authorization_headers->SetHeader(
-      HttpAuth::GetAuthorizationHeaderName(target_), auth_token_);
-  auth_token_.clear();
+  // auth_token_ can be empty if we encountered a permanent error with
+  // the auth scheme and want to retry.
+  if (!auth_token_.empty()) {
+    authorization_headers->SetHeader(
+        HttpAuth::GetAuthorizationHeaderName(target_), auth_token_);
+    auth_token_.clear();
+  }
 }
 
 int HttpAuthController::HandleAuthChallenge(
@@ -498,15 +501,40 @@ void HttpAuthController::PopulateAuthChallenge() {
   auth_info_->realm = ASCIIToWide(handler_->realm());
 }
 
+bool HttpAuthController::DisableOnAuthHandlerResult(int result) {
+  DCHECK(CalledOnValidThread());
+
+  switch (result) {
+    // Occurs with GSSAPI, if the user has not already logged in.
+    case ERR_MISSING_AUTH_CREDENTIALS:
+
+    // Can occur with GSSAPI or SSPI if the underlying library reports
+    // a permanent error.
+    case ERR_UNSUPPORTED_AUTH_SCHEME:
+
+    // These two error codes represent failures we aren't handling.
+    case ERR_UNEXPECTED_SECURITY_LIBRARY_STATUS:
+    case ERR_UNDOCUMENTED_SECURITY_LIBRARY_STATUS:
+
+    // Can be returned by SSPI if the authenticating authority or
+    // target is not known.
+    case ERR_MISCONFIGURED_AUTH_ENVIRONMENT:
+
+      // In these cases, disable the current scheme as it cannot
+      // succeed.
+      DisableAuthScheme(handler_->auth_scheme());
+      auth_token_.clear();
+      return true;
+
+    default:
+      return false;
+  }
+}
+
 void HttpAuthController::OnIOComplete(int result) {
   DCHECK(CalledOnValidThread());
-  // This error occurs with GSSAPI, if the user has not already logged in.
-  // In that case, disable the current scheme as it cannot succeed.
-  if (result == ERR_MISSING_AUTH_CREDENTIALS) {
-    DisableAuthScheme(handler_->auth_scheme());
-    auth_token_.clear();
+  if (DisableOnAuthHandlerResult(result))
     result = OK;
-  }
   if (user_callback_) {
     CompletionCallback* c = user_callback_;
     user_callback_ = NULL;
diff --git a/net/http/http_auth_controller.h b/net/http/http_auth_controller.h
index 93eae5b..8fed8c8 100644
--- a/net/http/http_auth_controller.h
+++ b/net/http/http_auth_controller.h
@@ -107,6 +107,11 @@ class HttpAuthController : public base::RefCounted<HttpAuthController>,
   // URLRequestHttpJob can prompt for a username/password.
   void PopulateAuthChallenge();
 
+  // If |result| indicates a permanent failure, disables the current
+  // auth scheme for this controller and returns true.  Returns false
+  // otherwise.
+  bool DisableOnAuthHandlerResult(int result);
+
   void OnIOComplete(int result);
 
   // Indicates if this handler is for Proxy auth or Server auth.
diff --git a/net/http/http_auth_controller_unittest.cc b/net/http/http_auth_controller_unittest.cc
new file mode 100644
index 0000000..f324e66
--- /dev/null
+++ b/net/http/http_auth_controller_unittest.cc
@@ -0,0 +1,112 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/http/http_auth_controller.h"
+
+#include "net/base/net_errors.h"
+#include "net/base/net_log.h"
+#include "net/base/test_completion_callback.h"
+#include "net/http/http_auth_cache.h"
+#include "net/http/http_auth_handler_mock.h"
+#include "net/http/http_request_info.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+enum HandlerRunMode {
+  RUN_HANDLER_SYNC,
+  RUN_HANDLER_ASYNC
+};
+
+enum SchemeState {
+  SCHEME_IS_DISABLED,
+  SCHEME_IS_ENABLED
+};
+
+// Runs an HttpAuthController with a single round mock auth handler
+// that returns |handler_rv| on token generation.  The handler runs in
+// async if |run_mode| is RUN_HANDLER_ASYNC.  Upon completion, the
+// return value of the controller is tested against
+// |expected_controller_rv|.  |scheme_state| indicates whether the
+// auth scheme used should be disabled after this run.
+void RunSingleRoundAuthTest(HandlerRunMode run_mode,
+                            int handler_rv,
+                            int expected_controller_rv,
+                            SchemeState scheme_state) {
+  BoundNetLog dummy_log;
+  HttpAuthCache dummy_auth_cache;
+
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("http://example.com");
+
+  const std::string headers_raw_string =
+      "HTTP/1.1 407\r\n"
+      "Proxy-Authenticate: MOCK foo\r\n"
+      "\r\n";
+  std::string headers_string = HttpUtil::AssembleRawHeaders(
+      headers_raw_string.c_str(), headers_raw_string.length());
+  scoped_refptr<HttpResponseHeaders> headers(
+      new HttpResponseHeaders(headers_string));
+
+  HttpAuthHandlerMock::Factory auth_handler_factory;
+  HttpAuthHandlerMock* auth_handler = new HttpAuthHandlerMock();
+  auth_handler->SetGenerateExpectation((run_mode == RUN_HANDLER_ASYNC),
+                                       handler_rv);
+  auth_handler_factory.set_mock_handler(auth_handler, HttpAuth::AUTH_PROXY);
+  auth_handler_factory.set_do_init_from_challenge(true);
+
+  scoped_refptr<HttpAuthController> controller(
+      new HttpAuthController(HttpAuth::AUTH_PROXY,
+                             GURL("http://example.com"),
+                             &dummy_auth_cache, &auth_handler_factory));
+  ASSERT_EQ(OK,
+            controller->HandleAuthChallenge(headers, false, false, dummy_log));
+  EXPECT_TRUE(controller->HaveAuthHandler());
+  controller->ResetAuth(string16(), string16());
+  EXPECT_TRUE(controller->HaveAuth());
+
+  TestCompletionCallback callback;
+  EXPECT_EQ((run_mode == RUN_HANDLER_ASYNC)? ERR_IO_PENDING:
+            expected_controller_rv,
+            controller->MaybeGenerateAuthToken(&request, &callback,
+                                               dummy_log));
+  if (run_mode == RUN_HANDLER_ASYNC)
+    EXPECT_EQ(expected_controller_rv, callback.WaitForResult());
+  EXPECT_EQ((scheme_state == SCHEME_IS_DISABLED),
+            controller->IsAuthSchemeDisabled(HttpAuth::AUTH_SCHEME_MOCK));
+}
+
+}  // namespace
+
+// If an HttpAuthHandler returns an error code that indicates a
+// permanent error, the HttpAuthController should disable the scheme
+// used and retry the request.
+TEST(HttpAuthControllerTest, PermanentErrors) {
+
+  // Run a synchronous handler that returns
+  // ERR_UNEXPECTED_SECURITY_LIBRARY_STATUS.  We expect a return value
+  // of OK from the controller so we can retry the request.
+  RunSingleRoundAuthTest(RUN_HANDLER_SYNC,
+                         ERR_UNEXPECTED_SECURITY_LIBRARY_STATUS,
+                         OK, SCHEME_IS_DISABLED);
+
+  // Now try an async handler that returns
+  // ERR_MISSING_AUTH_CREDENTIALS.  Async and sync handlers invoke
+  // different code paths in HttpAuthController when generating
+  // tokens.
+  RunSingleRoundAuthTest(RUN_HANDLER_ASYNC, ERR_MISSING_AUTH_CREDENTIALS, OK,
+                         SCHEME_IS_DISABLED);
+
+  // If a non-permanent error is returned by the handler, then the
+  // controller should report it unchanged.
+  RunSingleRoundAuthTest(RUN_HANDLER_ASYNC, ERR_INVALID_AUTH_CREDENTIALS,
+                         ERR_INVALID_AUTH_CREDENTIALS, SCHEME_IS_ENABLED);
+}
+
+}  // namespace net