Don't evict username/password when handling "stale" digest authentication challenges.

Stale challenges indicate that the cached nonce (and associated nonce_count) are no longer valid.

BUG=53353
TEST=Use a digest authentication Squid proxy with default settings (which complain about stale nonce's). Ensure that there are no username/password prompts after getting a stale challenge. Also, net_unittests. 

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

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

7 files changed, 113 insertions, 11 deletions

diff --git a/net/http/http_auth.cc b/net/http/http_auth.cc
index aaa4de0..debcca7 100644
--- a/net/http/http_auth.cc
+++ b/net/http/http_auth.cc
@@ -56,7 +56,9 @@ HttpAuth::AuthorizationResult HttpAuth::HandleChallengeResponse(
     HttpAuthHandler* handler,
     const HttpResponseHeaders* headers,
     Target target,
-    const std::set<std::string>& disabled_schemes) {
+    const std::set<std::string>& disabled_schemes,
+    std::string* challenge_used) {
+  DCHECK(challenge_used);
   const std::string& current_scheme = handler->scheme();
   if (disabled_schemes.find(current_scheme) != disabled_schemes.end())
     return HttpAuth::AUTHORIZATION_RESULT_REJECT;
@@ -70,10 +72,12 @@ HttpAuth::AuthorizationResult HttpAuth::HandleChallengeResponse(
     if (!LowerCaseEqualsASCII(props.scheme(), current_scheme.c_str()))
       continue;
     authorization_result = handler->HandleAnotherChallenge(&props);
-    if (authorization_result != HttpAuth::AUTHORIZATION_RESULT_INVALID)
+    if (authorization_result != HttpAuth::AUTHORIZATION_RESULT_INVALID) {
+      *challenge_used = challenge;
       return authorization_result;
+    }
   }
-  // Finding no matches is equivalent to rejection
+  // Finding no matches is equivalent to rejection.
   return HttpAuth::AUTHORIZATION_RESULT_REJECT;
 }
 
diff --git a/net/http/http_auth.h b/net/http/http_auth.h
index d7d0347..dc78f0c 100644
--- a/net/http/http_auth.h
+++ b/net/http/http_auth.h
@@ -126,7 +126,8 @@ class HttpAuth {
       HttpAuthHandler* handler,
       const HttpResponseHeaders* headers,
       Target target,
-      const std::set<std::string>& disabled_schemes);
+      const std::set<std::string>& disabled_schemes,
+      std::string* challenge_used);
 
   // ChallengeTokenizer breaks up a challenge string into the the auth scheme
   // and parameter list, according to RFC 2617 Sec 1.2:
diff --git a/net/http/http_auth_cache.cc b/net/http/http_auth_cache.cc
index 5b1a275..7423115 100644
--- a/net/http/http_auth_cache.cc
+++ b/net/http/http_auth_cache.cc
@@ -166,6 +166,12 @@ bool HttpAuthCache::Entry::HasEnclosingPath(const std::string& dir) {
   return false;
 }
 
+void HttpAuthCache::Entry::UpdateStaleChallenge(
+    const std::string& auth_challenge) {
+  auth_challenge_ = auth_challenge;
+  nonce_count_ = 1;
+}
+
 bool HttpAuthCache::Remove(const GURL& origin,
                            const std::string& realm,
                            const std::string& scheme,
@@ -184,4 +190,15 @@ bool HttpAuthCache::Remove(const GURL& origin,
   return false;
 }
 
+bool HttpAuthCache::UpdateStaleChallenge(const GURL& origin,
+                                         const std::string& realm,
+                                         const std::string& scheme,
+                                         const std::string& auth_challenge) {
+  HttpAuthCache::Entry* entry = Lookup(origin, realm, scheme);
+  if (!entry)
+    return false;
+  entry->UpdateStaleChallenge(auth_challenge);
+  return true;
+}
+
 }  // namespace net
diff --git a/net/http/http_auth_cache.h b/net/http/http_auth_cache.h
index 3716646..764a563 100644
--- a/net/http/http_auth_cache.h
+++ b/net/http/http_auth_cache.h
@@ -80,6 +80,16 @@ class HttpAuthCache {
               const string16& username,
               const string16& password);
 
+  // Updates a stale digest entry on server |origin| for realm |realm| and
+  // scheme |scheme|. The cached auth challenge is replaced with
+  // |auth_challenge| and the nonce count is reset.
+  // |UpdateStaleChallenge()| returns true if a matching entry exists in the
+  // cache, false otherwise.
+  bool UpdateStaleChallenge(const GURL& origin,
+                            const std::string& realm,
+                            const std::string& scheme,
+                            const std::string& auth_challenge);
+
   // Prevent unbounded memory growth. These are safeguards for abuse; it is
   // not expected that the limits will be reached in ordinary usage.
   // This also defines the worst-case lookup times (which grow linearly
@@ -128,6 +138,8 @@ class HttpAuthCache::Entry {
     return ++nonce_count_;
   }
 
+  void UpdateStaleChallenge(const std::string& auth_challenge);
+
  private:
   friend class HttpAuthCache;
   FRIEND_TEST_ALL_PREFIXES(HttpAuthCacheTest, AddPath);
diff --git a/net/http/http_auth_cache_unittest.cc b/net/http/http_auth_cache_unittest.cc
index 7b68ae3..bc4a0fb 100644
--- a/net/http/http_auth_cache_unittest.cc
+++ b/net/http/http_auth_cache_unittest.cc
@@ -323,6 +323,55 @@ TEST(HttpAuthCacheTest, Remove) {
   EXPECT_FALSE(NULL == entry);
 }
 
+TEST(HttpAuthCacheTest, UpdateStaleChallenge) {
+  HttpAuthCache cache;
+  GURL origin("http://foobar2.com");
+  scoped_ptr<HttpAuthHandler> digest_handler(
+      new MockAuthHandler(kDigest, kRealm1, HttpAuth::AUTH_PROXY));
+  HttpAuthCache::Entry* entry_pre = cache.Add(
+      origin,
+      digest_handler->realm(),
+      digest_handler->scheme(),
+      "Digest realm=Realm1,"
+      "nonce=\"s3MzvFhaBAA=4c520af5acd9d8d7ae26947529d18c8eae1e98f4\"",
+      ASCIIToUTF16("realm-digest-user"),
+      ASCIIToUTF16("realm-digest-password"),
+      "/baz/index.html");
+  ASSERT_TRUE(entry_pre != NULL);
+
+  EXPECT_EQ(2, entry_pre->IncrementNonceCount());
+  EXPECT_EQ(3, entry_pre->IncrementNonceCount());
+  EXPECT_EQ(4, entry_pre->IncrementNonceCount());
+
+  bool update_success = cache.UpdateStaleChallenge(
+      origin,
+      digest_handler->realm(),
+      digest_handler->scheme(),
+      "Digest realm=Realm1,"
+      "nonce=\"claGgoRXBAA=7583377687842fdb7b56ba0555d175baa0b800e3\","
+      "stale=\"true\"");
+  EXPECT_TRUE(update_success);
+
+  // After the stale update, the entry should still exist in the cache and
+  // the nonce count should be reset to 0.
+  HttpAuthCache::Entry* entry_post = cache.Lookup(
+      origin,
+      digest_handler->realm(),
+      digest_handler->scheme());
+  ASSERT_TRUE(entry_post != NULL);
+  EXPECT_EQ(2, entry_post->IncrementNonceCount());
+
+  // UpdateStaleChallenge will fail if an entry doesn't exist in the cache.
+  bool update_failure = cache.UpdateStaleChallenge(
+      origin,
+      kRealm2,
+      digest_handler->scheme(),
+      "Digest realm=Realm2,"
+      "nonce=\"claGgoRXBAA=7583377687842fdb7b56ba0555d175baa0b800e3\","
+      "stale=\"true\"");
+  EXPECT_FALSE(update_failure);
+}
+
 // Test fixture class for eviction tests (contains helpers for bulk
 // insertion and existence testing).
 class HttpAuthCacheEvictionTest : public testing::Test {
diff --git a/net/http/http_auth_controller.cc b/net/http/http_auth_controller.cc
index 5cdb1c6..5226b02 100644
--- a/net/http/http_auth_controller.cc
+++ b/net/http/http_auth_controller.cc
@@ -161,8 +161,9 @@ int HttpAuthController::HandleAuthChallenge(
   // challenge appeared to be rejected, or is using a stale nonce in the Digest
   // case.
   if (HaveAuth()) {
+    std::string challenge_used;
     HttpAuth::AuthorizationResult result = HttpAuth::HandleChallengeResponse(
-        handler_.get(), headers, target_, disabled_schemes_);
+        handler_.get(), headers, target_, disabled_schemes_, &challenge_used);
     switch (result) {
       case HttpAuth::AUTHORIZATION_RESULT_ACCEPT:
         break;
@@ -171,9 +172,18 @@ int HttpAuthController::HandleAuthChallenge(
         InvalidateCurrentHandler();
         break;
       case HttpAuth::AUTHORIZATION_RESULT_STALE:
-        // TODO(cbentzel): Support "stale" invalidation in HttpAuthCache.
-        // Right now this does the same invalidation as before.
-        InvalidateCurrentHandler();
+        if (session_->auth_cache()->UpdateStaleChallenge(auth_origin_,
+                                                         handler_->realm(),
+                                                         handler_->scheme(),
+                                                         challenge_used)) {
+          handler_.reset();
+          identity_ = HttpAuth::Identity();
+        } else {
+          // It's possible that a server could incorrectly issue a stale
+          // response when the entry is not in the cache. Just evict the
+          // current value from the cache.
+          InvalidateCurrentHandler();
+        }
         break;
       default:
         NOTREACHED();
diff --git a/net/http/http_auth_unittest.cc b/net/http/http_auth_unittest.cc
index 4d8ddee..a3f2b28 100644
--- a/net/http/http_auth_unittest.cc
+++ b/net/http/http_auth_unittest.cc
@@ -135,12 +135,15 @@ TEST(HttpAuthTest, HandleChallengeResponse_RequestBased) {
       HeadersFromResponseText(
           "HTTP/1.1 401 Unauthorized\n"
           "WWW-Authenticate: Mock token_here\n"));
+  std::string challenge_used;
   EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
             HttpAuth::HandleChallengeResponse(
                 mock_handler.get(),
                 headers.get(),
                 HttpAuth::AUTH_SERVER,
-                disabled_schemes));
+                disabled_schemes,
+                &challenge_used));
+  EXPECT_EQ("Mock token_here", challenge_used);
 }
 
 TEST(HttpAuthTest, HandleChallengeResponse_ConnectionBased) {
@@ -150,12 +153,15 @@ TEST(HttpAuthTest, HandleChallengeResponse_ConnectionBased) {
       HeadersFromResponseText(
           "HTTP/1.1 401 Unauthorized\n"
           "WWW-Authenticate: Mock token_here\n"));
+  std::string challenge_used;
   EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_ACCEPT,
             HttpAuth::HandleChallengeResponse(
                 mock_handler.get(),
                 headers.get(),
                 HttpAuth::AUTH_SERVER,
-                disabled_schemes));
+                disabled_schemes,
+                &challenge_used));
+  EXPECT_EQ("Mock token_here", challenge_used);
 }
 
 TEST(HttpAuthTest, HandleChallengeResponse_ConnectionBasedNoMatch) {
@@ -165,12 +171,15 @@ TEST(HttpAuthTest, HandleChallengeResponse_ConnectionBasedNoMatch) {
       HeadersFromResponseText(
           "HTTP/1.1 401 Unauthorized\n"
           "WWW-Authenticate: Basic realm=\"happy\"\n"));
+  std::string challenge_used;
   EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
             HttpAuth::HandleChallengeResponse(
                 mock_handler.get(),
                 headers.get(),
                 HttpAuth::AUTH_SERVER,
-                disabled_schemes));
+                disabled_schemes,
+                &challenge_used));
+  EXPECT_TRUE(challenge_used.empty());
 }
 
 TEST(HttpAuthTest, ChallengeTokenizer) {