[net/auth] Don't abort network transaction over non-permanent auth errors.

net/http/http_network_transaction_unittest.cc

Index: net/http/http_network_transaction_unittest.cc
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 12b5f253267dd8f698cd7a6751f0a36a9e935754..b9dccb147438a4cdf34f1ccdfee34df77dbe1321 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -4134,6 +4134,127 @@ TEST_F(HttpNetworkTransactionTest,
   session->CloseAllConnections();
 }
 
+// A more nuanced test than GenerateAuthToken test which asserts that
+// ERR_INVALID_AUTH_CREDENTIALS does not cause the auth scheme to be
+// unnecessarily invalidated, and that if the server co-operates, the
+// authentication handshake can continue with the same scheme but with a
+// different identity.
+TEST_F(HttpNetworkTransactionTest, NonPermanentGenerateAuthTokenError) {
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("http://www.example.org/");
+
+  std::unique_ptr<HttpAuthHandlerMock::Factory> auth_handler_factory(
+      new HttpAuthHandlerMock::Factory());
+  auth_handler_factory->set_do_init_from_challenge(true);
+
+  // First handler. Uses default credentials, but barfs at generate auth token.
+  std::unique_ptr<HttpAuthHandlerMock> mock_handler(new HttpAuthHandlerMock());
+  mock_handler->set_allows_default_credentials(true);
+  mock_handler->set_allows_explicit_credentials(true);
+  mock_handler->set_connection_based(true);
+  mock_handler->SetGenerateExpectation(true, ERR_INVALID_AUTH_CREDENTIALS);
+  auth_handler_factory->AddMockHandler(mock_handler.release(),
+                                       HttpAuth::AUTH_SERVER);
+
+  // Add another handler for the second challenge. It supports default
+  // credentials, but they shouldn't be used, since they were already tried.

[mmenke, 2016/10/04 19:42:15]

So default credentials for all different handlers are considered the same, even
if the auth handlers are for different schemes?  Or are all auth handlers always
for the same scheme?

[asanka, 2016/10/06 14:16:35]

These are auth handlers for the same scheme. The state for which schemes and
identities have been tried is maintained by the HttpAuthController. A
HttpAuthHandler cannot be reused safely. So the controller throws out the
existing handler and creates a new one.

+  mock_handler.reset(new HttpAuthHandlerMock());
+  mock_handler->set_allows_default_credentials(true);
+  mock_handler->set_allows_explicit_credentials(true);
+  mock_handler->set_connection_based(true);
+  auth_handler_factory->AddMockHandler(mock_handler.release(),
+                                       HttpAuth::AUTH_SERVER);
+  session_deps_.http_auth_handler_factory = std::move(auth_handler_factory);
+
+  std::unique_ptr<HttpNetworkSession> session = CreateSession(&session_deps_);
+
+  MockWrite data_writes1[] = {
+      MockWrite("GET / HTTP/1.1\r\n"
+                "Host: www.example.org\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads1[] = {
+      MockRead("HTTP/1.1 401 Authentication Required\r\n"
+               "WWW-Authenticate: Mock\r\n"
+               "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockWrite data_writes2[] = {

[mmenke, 2016/10/04 19:42:15]

The first auth handler tries to generate credentials and fail, so we just retry?
 Maybe a comment about that?

[asanka, 2016/10/06 14:16:35]

Done.

+      MockWrite("GET / HTTP/1.1\r\n"
+                "Host: www.example.org\r\n"
+                "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads2[] = {
+      MockRead("HTTP/1.1 401 Authentication Required\r\n"
+               "WWW-Authenticate: Mock\r\n"
+               "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockWrite data_writes3[] = {
+      MockWrite("GET / HTTP/1.1\r\n"
+                "Host: www.example.org\r\n"
+                "Connection: keep-alive\r\n"
+                "Authorization: auth_token\r\n\r\n"),

[mmenke, 2016/10/04 19:42:15]

What would happen differently if it tried default credentials?

[asanka, 2016/10/06 14:16:35]

The type of credentials being used is checked by the following two assertions
that follow the first RestartWithAuth() call:

  EXPECT_FALSE(trans->IsReadyToRestartForAuth());
  EXPECT_TRUE(response->auth_challenge);

IsReadyToRestartForAuth() false if the chosen identity needs explicit
credentials (i.e. not using ambient / default credentials). auth_challenge would
be non-null if an auth challenge is available to prompt the user.

I added comments to point this out in the code below.

+  };
+
+  MockRead data_reads3[] = {
+      MockRead("HTTP/1.1 200 OK\r\n"
+               "Content-Length: 5\r\n"
+               "Content-Type: text/plain\r\n"
+               "Connection: keep-alive\r\n\r\n"
+               "Hello"),
+  };
+
+  StaticSocketDataProvider data1(data_reads1, arraysize(data_reads1),
+                                 data_writes1, arraysize(data_writes1));
+  session_deps_.socket_factory->AddSocketDataProvider(&data1);
+
+  StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
+                                 data_writes2, arraysize(data_writes2));
+  session_deps_.socket_factory->AddSocketDataProvider(&data2);
+
+  StaticSocketDataProvider data3(data_reads3, arraysize(data_reads3),
+                                 data_writes3, arraysize(data_writes3));
+  session_deps_.socket_factory->AddSocketDataProvider(&data3);
+
+  std::unique_ptr<HttpNetworkTransaction> trans(
+      new HttpNetworkTransaction(DEFAULT_PRIORITY, session.get()));
+
+  TestCompletionCallback callback;
+  int rv = trans->Start(&request, callback.callback(), NetLogWithSource());
+  EXPECT_THAT(callback.GetResult(rv), IsOk());
+
+  const HttpResponseInfo* response = trans->GetResponseInfo();
+  ASSERT_TRUE(response);
+  ASSERT_TRUE(response->headers);
+  EXPECT_EQ(401, response->headers->response_code());
+  EXPECT_TRUE(HttpVersion(1, 1) == response->headers->GetHttpVersion());
+  EXPECT_TRUE(trans->IsReadyToRestartForAuth());
+  EXPECT_FALSE(response->auth_challenge);
+
+  rv = trans->RestartWithAuth(AuthCredentials(), callback.callback());
+  EXPECT_THAT(callback.GetResult(rv), IsOk());
+  response = trans->GetResponseInfo();
+  ASSERT_TRUE(response);
+  ASSERT_TRUE(response->headers);
+  EXPECT_EQ(401, response->headers->response_code());
+  EXPECT_FALSE(trans->IsReadyToRestartForAuth());
+  EXPECT_TRUE(response->auth_challenge);
+
+  rv = trans->RestartWithAuth(AuthCredentials(), callback.callback());
+  EXPECT_THAT(callback.GetResult(rv), IsOk());
+  response = trans->GetResponseInfo();
+  ASSERT_TRUE(response);
+  ASSERT_TRUE(response->headers);
+  EXPECT_EQ(200, response->headers->response_code());
+
+  trans.reset();
+  session->CloseAllConnections();
+}
+
 // Test the load timing for HTTPS requests with an HTTP proxy.
 TEST_F(HttpNetworkTransactionTest, HttpProxyLoadTimingNoPacTwoRequests) {
   HttpRequestInfo request1;
@@ -11390,7 +11511,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
     int server_auth_rv;
     int num_auth_rounds;
     int first_ssl_round;
-    TestRound rounds[3];
+    TestRound rounds[4];
   } test_configs[] = {
       // Non-authenticating HTTP server with a direct connection.
       {NULL,
@@ -11421,8 +11542,25 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        ERR_INVALID_AUTH_CREDENTIALS,
        2,
        kNoSSL,
-       {TestRound(kGet, kServerChallenge, OK),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+       {TestRound(kGet, kServerChallenge, OK), TestRound(kGet, kSuccess, OK)}},
+      {NULL,
+       AUTH_NONE,
+       OK,
+       kServer,
+       AUTH_SYNC,
+       ERR_UNSUPPORTED_AUTH_SCHEME,
+       2,
+       kNoSSL,
+       {TestRound(kGet, kServerChallenge, OK), TestRound(kGet, kSuccess, OK)}},

[asanka, 2016/09/30 22:57:11]

The implication here is a bit subtle. What we are looking for is whether the
network transaction recovers from a permanent error by abandoning the auth
handler and sending a request with no authorization headers for the target. In
this case, this is verified by the second TestRound(kGet, [anything], OK) test
round.

This pattern is going to repeat in the test cases below.

+      {NULL,
+       AUTH_NONE,
+       OK,
+       kServer,
+       AUTH_SYNC,
+       ERR_UNDOCUMENTED_SECURITY_LIBRARY_STATUS,
+       2,
+       kNoSSL,
+       {TestRound(kGet, kServerChallenge, OK), TestRound(kGet, kSuccess, OK)}},
       {NULL,
        AUTH_NONE,
        OK,
@@ -11441,8 +11579,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        ERR_INVALID_AUTH_CREDENTIALS,
        2,
        kNoSSL,
-       {TestRound(kGet, kServerChallenge, OK),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+       {TestRound(kGet, kServerChallenge, OK), TestRound(kGet, kSuccess, OK)}},

[asanka, 2016/09/30 22:57:11]

Note that here and elsewhere, the old test round looks like :
TestRound(kGetAuth, ..., ERR_INVALID_AUTH_CREDENTIALS) and the new test round
looks like TestRound(kGet, ..., OK).

The kGetAuth mock write data *is not* relevant in the old code because the
transaction aborts before sending that request. The kGet mock write data *is*
relevant in the new code because the transaction doesn't abort, and instead
continues without authorization headers.

       // Non-authenticating HTTP server through a non-authenticating proxy.
       {kProxy,
        AUTH_NONE,
@@ -11473,8 +11610,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        kNoSSL,
        {TestRound(kGetProxy, kServerChallenge, OK),
-        TestRound(kGetAuthThroughProxy, kFailure,
-                  ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxy, kSuccess, OK)}},
       {kProxy,
        AUTH_NONE,
        OK,
@@ -11494,8 +11630,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        kNoSSL,
        {TestRound(kGetProxy, kServerChallenge, OK),
-        TestRound(kGetAuthThroughProxy, kFailure,
-                  ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxy, kSuccess, OK)}},
       // Non-authenticating HTTP server through an authenticating proxy.
       {kProxy,
        AUTH_SYNC,
@@ -11516,7 +11651,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        kNoSSL,
        {TestRound(kGetProxy, kProxyChallenge, OK),
-        TestRound(kGetProxyAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxy, kSuccess, OK)}},
       {kProxy,
        AUTH_ASYNC,
        OK,
@@ -11536,7 +11671,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        kNoSSL,
        {TestRound(kGetProxy, kProxyChallenge, OK),
-        TestRound(kGetProxyAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxy, kSuccess, OK)}},
       // Authenticating HTTP server through an authenticating proxy.
       {kProxy,
        AUTH_SYNC,
@@ -11559,8 +11694,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        kNoSSL,
        {TestRound(kGetProxy, kProxyChallenge, OK),
         TestRound(kGetProxyAuth, kServerChallenge, OK),
-        TestRound(kGetAuthWithProxyAuth, kFailure,
-                  ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxyAuth, kSuccess, OK)}},
       {kProxy,
        AUTH_ASYNC,
        OK,
@@ -11582,8 +11716,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        kNoSSL,
        {TestRound(kGetProxy, kProxyChallenge, OK),
         TestRound(kGetProxyAuth, kServerChallenge, OK),
-        TestRound(kGetAuthWithProxyAuth, kFailure,
-                  ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxyAuth, kSuccess, OK)}},
       {kProxy,
        AUTH_SYNC,
        OK,
@@ -11605,8 +11738,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        kNoSSL,
        {TestRound(kGetProxy, kProxyChallenge, OK),
         TestRound(kGetProxyAuth, kServerChallenge, OK),
-        TestRound(kGetAuthWithProxyAuth, kFailure,
-                  ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxyAuth, kSuccess, OK)}},
       {kProxy,
        AUTH_ASYNC,
        OK,
@@ -11628,8 +11760,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        kNoSSL,
        {TestRound(kGetProxy, kProxyChallenge, OK),
         TestRound(kGetProxyAuth, kServerChallenge, OK),
-        TestRound(kGetAuthWithProxyAuth, kFailure,
-                  ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGetProxyAuth, kSuccess, OK)}},
       // Non-authenticating HTTPS server with a direct connection.
       {NULL,
        AUTH_NONE,
@@ -11659,8 +11790,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        ERR_INVALID_AUTH_CREDENTIALS,
        2,
        0,
-       {TestRound(kGet, kServerChallenge, OK),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+       {TestRound(kGet, kServerChallenge, OK), TestRound(kGet, kSuccess, OK)}},
       {NULL,
        AUTH_NONE,
        OK,
@@ -11679,8 +11809,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        ERR_INVALID_AUTH_CREDENTIALS,
        2,
        0,
-       {TestRound(kGet, kServerChallenge, OK),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+       {TestRound(kGet, kServerChallenge, OK), TestRound(kGet, kSuccess, OK)}},
       // Non-authenticating HTTPS server with a non-authenticating proxy.
       {kProxy,
        AUTH_NONE,
@@ -11711,7 +11840,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        0,
        {TestRound(kConnect, kProxyConnected, OK, &kGet, &kServerChallenge),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGet, kSuccess, OK)}},
       {kProxy,
        AUTH_NONE,
        OK,
@@ -11731,7 +11860,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        0,
        {TestRound(kConnect, kProxyConnected, OK, &kGet, &kServerChallenge),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGet, kSuccess, OK)}},
       // Non-Authenticating HTTPS server through an authenticating proxy.
       {kProxy,
        AUTH_SYNC,
@@ -11752,7 +11881,27 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        kNoSSL,
        {TestRound(kConnect, kProxyChallenge, OK),
-        TestRound(kConnectProxyAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kConnect, kProxyConnected, OK, &kGet, &kSuccess)}},
+      {kProxy,
+       AUTH_SYNC,
+       ERR_UNSUPPORTED_AUTH_SCHEME,
+       kSecureServer,
+       AUTH_NONE,
+       OK,
+       2,
+       kNoSSL,
+       {TestRound(kConnect, kProxyChallenge, OK),
+        TestRound(kConnect, kProxyConnected, OK, &kGet, &kSuccess)}},
+      {kProxy,
+       AUTH_SYNC,
+       ERR_UNEXPECTED,
+       kSecureServer,
+       AUTH_NONE,
+       OK,
+       2,
+       kNoSSL,
+       {TestRound(kConnect, kProxyChallenge, OK),
+        TestRound(kConnect, kProxyConnected, ERR_UNEXPECTED)}},
       {kProxy,
        AUTH_ASYNC,
        OK,
@@ -11772,7 +11921,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        2,
        kNoSSL,
        {TestRound(kConnect, kProxyChallenge, OK),
-        TestRound(kConnectProxyAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kConnect, kProxyConnected, OK, &kGet, &kSuccess)}},
       // Authenticating HTTPS server through an authenticating proxy.
       {kProxy,
        AUTH_SYNC,
@@ -11797,7 +11946,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        {TestRound(kConnect, kProxyChallenge, OK),
         TestRound(kConnectProxyAuth, kProxyConnected, OK, &kGet,
                   &kServerChallenge),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGet, kSuccess, OK)}},
       {kProxy,
        AUTH_ASYNC,
        OK,
@@ -11821,7 +11970,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        {TestRound(kConnect, kProxyChallenge, OK),
         TestRound(kConnectProxyAuth, kProxyConnected, OK, &kGet,
                   &kServerChallenge),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGet, kSuccess, OK)}},
       {kProxy,
        AUTH_SYNC,
        OK,
@@ -11845,7 +11994,7 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        {TestRound(kConnect, kProxyChallenge, OK),
         TestRound(kConnectProxyAuth, kProxyConnected, OK, &kGet,
                   &kServerChallenge),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGet, kSuccess, OK)}},
       {kProxy,
        AUTH_ASYNC,
        OK,
@@ -11869,10 +12018,11 @@ TEST_F(HttpNetworkTransactionTest, GenerateAuthToken) {
        {TestRound(kConnect, kProxyChallenge, OK),
         TestRound(kConnectProxyAuth, kProxyConnected, OK, &kGet,
                   &kServerChallenge),
-        TestRound(kGetAuth, kFailure, ERR_INVALID_AUTH_CREDENTIALS)}},
+        TestRound(kGet, kSuccess, OK)}},
   };
 
   for (size_t i = 0; i < arraysize(test_configs); ++i) {
+    SCOPED_TRACE(::testing::Message() << "Test config " << i);
     HttpAuthHandlerMock::Factory* auth_factory(
         new HttpAuthHandlerMock::Factory());
     session_deps_.http_auth_handler_factory.reset(auth_factory);