Check and invalidate cached credentials if they were used for preemptive authentication and were rejected by the server.

BUG=72589
TEST=net_unittests --gtest_filter=HttpAuthHandler*.HandleAnotherChallenge

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

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

7 files changed, 84 insertions, 25 deletions

diff --git a/net/http/http_auth.h b/net/http/http_auth.h
index ef779a1..cc147a2 100644
--- a/net/http/http_auth.h
+++ b/net/http/http_auth.h
@@ -52,6 +52,13 @@ class HttpAuth {
     AUTHORIZATION_RESULT_INVALID,  // The authentication challenge headers are
                                    // poorly formed (the authorization attempt
                                    // itself may have been fine).
+
+    AUTHORIZATION_RESULT_DIFFERENT_REALM,  // The authorization
+                                           // attempt was rejected,
+                                           // but the realm associated
+                                           // with the new challenge
+                                           // is different from the
+                                           // previous attempt.
   };
 
   // Describes where the identity used for authentication came from.
diff --git a/net/http/http_auth_controller.cc b/net/http/http_auth_controller.cc
index f438ea4..8a342b8 100644
--- a/net/http/http_auth_controller.cc
+++ b/net/http/http_auth_controller.cc
@@ -273,26 +273,35 @@ int HttpAuthController::HandleAuthChallenge(
       case HttpAuth::AUTHORIZATION_RESULT_ACCEPT:
         break;
       case HttpAuth::AUTHORIZATION_RESULT_INVALID:
-        InvalidateCurrentHandler();
+        InvalidateCurrentHandler(INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS);
         break;
       case HttpAuth::AUTHORIZATION_RESULT_REJECT:
         HistogramAuthEvent(handler_.get(), AUTH_EVENT_REJECT);
-        InvalidateCurrentHandler();
+        InvalidateCurrentHandler(INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS);
         break;
       case HttpAuth::AUTHORIZATION_RESULT_STALE:
         if (http_auth_cache_->UpdateStaleChallenge(auth_origin_,
                                                    handler_->realm(),
                                                    handler_->auth_scheme(),
                                                    challenge_used)) {
-          handler_.reset();
-          identity_ = HttpAuth::Identity();
+          InvalidateCurrentHandler(INVALIDATE_HANDLER);
         } 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();
+          InvalidateCurrentHandler(INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS);
         }
         break;
+      case HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM:
+        // If the server asks for credentials for one realm and then
+        // rejects them, we remove the credentials from the cache
+        // unless it was in response to a preemptive authorization
+        // header.
+        InvalidateCurrentHandler(
+            (identity_.source == HttpAuth::IDENT_SRC_PATH_LOOKUP) ?
+            INVALIDATE_HANDLER :
+            INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS);
+        break;
       default:
         NOTREACHED();
         break;
@@ -403,10 +412,12 @@ bool HttpAuthController::HaveAuth() const {
   return handler_.get() && !identity_.invalid;
 }
 
-void HttpAuthController::InvalidateCurrentHandler() {
+void HttpAuthController::InvalidateCurrentHandler(
+    InvalidateHandlerAction action) {
   DCHECK(CalledOnValidThread());
 
-  InvalidateRejectedAuthFromCache();
+  if (action == INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS)
+    InvalidateRejectedAuthFromCache();
   handler_.reset();
   identity_ = HttpAuth::Identity();
 }
@@ -415,13 +426,6 @@ void HttpAuthController::InvalidateRejectedAuthFromCache() {
   DCHECK(CalledOnValidThread());
   DCHECK(HaveAuth());
 
-  // TODO(eroman): this short-circuit can be relaxed. If the realm of
-  // the preemptively used auth entry matches the realm of the subsequent
-  // challenge, then we can invalidate the preemptively used entry.
-  // Otherwise as-is we may send the failed credentials one extra time.
-  if (identity_.source == HttpAuth::IDENT_SRC_PATH_LOOKUP)
-    return;
-
   // Clear the cache entry for the identity we just failed on.
   // Note: we require the username/password to match before invalidating
   // since the entry in the cache may be newer than what we used last time.
diff --git a/net/http/http_auth_controller.h b/net/http/http_auth_controller.h
index 0b7f430..c4fc15e 100644
--- a/net/http/http_auth_controller.h
+++ b/net/http/http_auth_controller.h
@@ -73,6 +73,12 @@ class HttpAuthController : public base::RefCounted<HttpAuthController>,
   virtual void DisableAuthScheme(HttpAuth::Scheme scheme);
 
  private:
+  // Actions for InvalidateCurrentHandler()
+  enum InvalidateHandlerAction {
+    INVALIDATE_HANDLER_AND_CACHED_CREDENTIALS,
+    INVALIDATE_HANDLER
+  };
+
   // So that we can mock this object.
   friend class base::RefCounted<HttpAuthController>;
 
@@ -84,7 +90,7 @@ class HttpAuthController : public base::RefCounted<HttpAuthController>,
   bool SelectPreemptiveAuth(const BoundNetLog& net_log);
 
   // Invalidates the current handler, including cache.
-  void InvalidateCurrentHandler();
+  void InvalidateCurrentHandler(InvalidateHandlerAction action);
 
   // Invalidates any auth cache entries after authentication has failed.
   // The identity that was rejected is |identity_|.
diff --git a/net/http/http_auth_handler_basic.cc b/net/http/http_auth_handler_basic.cc
index e48aa67..9ed28e2 100644
--- a/net/http/http_auth_handler_basic.cc
+++ b/net/http/http_auth_handler_basic.cc
@@ -53,9 +53,20 @@ bool HttpAuthHandlerBasic::ParseChallenge(
 
 HttpAuth::AuthorizationResult HttpAuthHandlerBasic::HandleAnotherChallenge(
     HttpAuth::ChallengeTokenizer* challenge) {
-  // Basic authentication is always a single round, so any responses should
-  // be treated as a rejection.
-  return HttpAuth::AUTHORIZATION_RESULT_REJECT;
+  // Basic authentication is always a single round, so any responses
+  // should be treated as a rejection.  However, if the new challenge
+  // is for a different realm, then indicate the realm change.
+  HttpUtil::NameValuePairsIterator parameters = challenge->param_pairs();
+  std::string realm;
+  while (parameters.GetNext()) {
+    if (LowerCaseEqualsASCII(parameters.name(), "realm")) {
+      realm = parameters.value();
+      break;
+    }
+  }
+  return (realm_ != realm)?
+      HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM:
+      HttpAuth::AUTHORIZATION_RESULT_REJECT;
 }
 
 int HttpAuthHandlerBasic::GenerateAuthTokenImpl(
diff --git a/net/http/http_auth_handler_basic_unittest.cc b/net/http/http_auth_handler_basic_unittest.cc
index f2ddbebc..0150579 100644
--- a/net/http/http_auth_handler_basic_unittest.cc
+++ b/net/http/http_auth_handler_basic_unittest.cc
@@ -46,6 +46,25 @@ TEST(HttpAuthHandlerBasicTest, GenerateAuthToken) {
   }
 }
 
+TEST(HttpAuthHandlerBasicTest, HandleAnotherChallenge) {
+  GURL origin("http://www.example.com");
+  std::string challenge1 = "Basic realm=\"First\"";
+  std::string challenge2 = "Basic realm=\"Second\"";
+  HttpAuthHandlerBasic::Factory factory;
+  scoped_ptr<HttpAuthHandler> basic;
+  EXPECT_EQ(OK, factory.CreateAuthHandlerFromString(
+      challenge1, HttpAuth::AUTH_SERVER, origin, BoundNetLog(), &basic));
+  HttpAuth::ChallengeTokenizer tok_first(challenge1.begin(),
+                                         challenge1.end());
+  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
+            basic->HandleAnotherChallenge(&tok_first));
+
+  HttpAuth::ChallengeTokenizer tok_second(challenge2.begin(),
+                                          challenge2.end());
+  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM,
+            basic->HandleAnotherChallenge(&tok_second));
+}
+
 TEST(HttpAuthHandlerBasicTest, InitFromChallenge) {
   static const struct {
     const char* challenge;
diff --git a/net/http/http_auth_handler_digest.cc b/net/http/http_auth_handler_digest.cc
index e8cb819..28d2f58 100644
--- a/net/http/http_auth_handler_digest.cc
+++ b/net/http/http_auth_handler_digest.cc
@@ -114,16 +114,21 @@ HttpAuth::AuthorizationResult HttpAuthHandlerDigest::HandleAnotherChallenge(
     return HttpAuth::AUTHORIZATION_RESULT_INVALID;
 
   HttpUtil::NameValuePairsIterator parameters = challenge->param_pairs();
+  std::string realm;
 
-  // Try to find the "stale" value.
+  // Try to find the "stale" value, and also keep track of the realm
+  // for the new challenge.
   while (parameters.GetNext()) {
-    if (!LowerCaseEqualsASCII(parameters.name(), "stale"))
-      continue;
-    if (LowerCaseEqualsASCII(parameters.value(), "true"))
-      return HttpAuth::AUTHORIZATION_RESULT_STALE;
+    if (LowerCaseEqualsASCII(parameters.name(), "stale")) {
+      if (LowerCaseEqualsASCII(parameters.value(), "true"))
+        return HttpAuth::AUTHORIZATION_RESULT_STALE;
+    } else if (LowerCaseEqualsASCII(parameters.name(), "realm")) {
+      realm = parameters.value();
+    }
   }
-
-  return HttpAuth::AUTHORIZATION_RESULT_REJECT;
+  return (realm_ != realm) ?
+      HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM :
+      HttpAuth::AUTHORIZATION_RESULT_REJECT;
 }
 
 bool HttpAuthHandlerDigest::Init(HttpAuth::ChallengeTokenizer* challenge) {
diff --git a/net/http/http_auth_handler_digest_unittest.cc b/net/http/http_auth_handler_digest_unittest.cc
index 9b57c16e..8464a53 100644
--- a/net/http/http_auth_handler_digest_unittest.cc
+++ b/net/http/http_auth_handler_digest_unittest.cc
@@ -553,6 +553,13 @@ TEST(HttpAuthHandlerDigest, HandleAnotherChallenge) {
                                                stale_false_challenge.end());
   EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_REJECT,
             handler->HandleAnotherChallenge(&tok_stale_false));
+
+  std::string realm_change_challenge =
+      "Digest realm=\"SomethingElse\", nonce=\"nonce-value2\"";
+  HttpAuth::ChallengeTokenizer tok_realm_change(realm_change_challenge.begin(),
+                                                realm_change_challenge.end());
+  EXPECT_EQ(HttpAuth::AUTHORIZATION_RESULT_DIFFERENT_REALM,
+            handler->HandleAnotherChallenge(&tok_realm_change));
 }
 
 TEST(HttpAuthHandlerDigest, RespondToServerChallenge) {