diff options
author | cbentzel@chromium.org <cbentzel@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-20 11:09:24 +0000 |
---|---|---|
committer | cbentzel@chromium.org <cbentzel@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-20 11:09:24 +0000 |
commit | fa82f93da256dede111ee4143c340e55a195d7e3 (patch) | |
tree | 36082a958021799fd1de1e5e30d4a861cda4bba6 | |
parent | 6278df2e8f1e77409e499addf16fa4679f3dca0b (diff) | |
download | chromium_src-fa82f93da256dede111ee4143c340e55a195d7e3.zip chromium_src-fa82f93da256dede111ee4143c340e55a195d7e3.tar.gz chromium_src-fa82f93da256dede111ee4143c340e55a195d7e3.tar.bz2 |
Remove handler from HttpAuthCache.
This is part of a refactoring meant to simplify the connection phase of HttpNetworkTransaction.
BUG=None
TEST=net_unittests (which already includes unit tests for preemptive auth, as well as using values from cache).
Review URL: http://codereview.chromium.org/2056003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@47786 0039d316-1c4b-4281-b951-d872f2087c98
24 files changed, 239 insertions, 86 deletions
diff --git a/net/http/http_auth.h b/net/http/http_auth.h index 8d1ecc5..faa5e93 100644 --- a/net/http/http_auth.h +++ b/net/http/http_auth.h @@ -106,10 +106,16 @@ class HttpAuth { public: ChallengeTokenizer(std::string::const_iterator begin, std::string::const_iterator end) - : props_(begin, end, ','), valid_(true), expect_base64_token_(false) { + : props_(begin, end, ','), valid_(true), begin_(begin), end_(end), + expect_base64_token_(false) { Init(begin, end); } + // Get the original text. + std::string challenge_text() const { + return std::string(begin_, end_); + } + // Get the auth scheme of the challenge. std::string::const_iterator scheme_begin() const { return scheme_begin_; } std::string::const_iterator scheme_end() const { return scheme_end_; } @@ -164,6 +170,9 @@ class HttpAuth { HttpUtil::ValuesIterator props_; bool valid_; + std::string::const_iterator begin_; + std::string::const_iterator end_; + std::string::const_iterator scheme_begin_; std::string::const_iterator scheme_end_; diff --git a/net/http/http_auth_cache.cc b/net/http/http_auth_cache.cc index 2be2198..21db55d 100644 --- a/net/http/http_auth_cache.cc +++ b/net/http/http_auth_cache.cc @@ -97,7 +97,9 @@ HttpAuthCache::Entry* HttpAuthCache::LookupByPath(const GURL& origin, } HttpAuthCache::Entry* HttpAuthCache::Add(const GURL& origin, - HttpAuthHandler* handler, + const std::string& realm, + const std::string& scheme, + const std::string& auth_challenge, const std::wstring& username, const std::wstring& password, const std::string& path) { @@ -105,9 +107,7 @@ HttpAuthCache::Entry* HttpAuthCache::Add(const GURL& origin, CheckPathIsValid(path); // Check for existing entry (we will re-use it if present). - HttpAuthCache::Entry* entry = Lookup(origin, handler->realm(), - handler->scheme()); - + HttpAuthCache::Entry* entry = Lookup(origin, realm, scheme); if (!entry) { // Failsafe to prevent unbounded memory growth of the cache. if (entries_.size() >= kMaxNumRealmEntries) { @@ -118,11 +118,17 @@ HttpAuthCache::Entry* HttpAuthCache::Add(const GURL& origin, entries_.push_front(Entry()); entry = &entries_.front(); entry->origin_ = origin; + entry->realm_ = realm; + entry->scheme_ = scheme; } + DCHECK_EQ(origin, entry->origin_); + DCHECK_EQ(realm, entry->realm_); + DCHECK_EQ(scheme, entry->scheme_); + entry->auth_challenge_ = auth_challenge; entry->username_ = username; entry->password_ = password; - entry->handler_ = handler; + entry->nonce_count_ = 1; entry->AddPath(path); return entry; diff --git a/net/http/http_auth_cache.h b/net/http/http_auth_cache.h index 7062c76..1d238af1 100644 --- a/net/http/http_auth_cache.h +++ b/net/http/http_auth_cache.h @@ -10,7 +10,6 @@ #include "base/ref_counted.h" #include "googleurl/src/gurl.h" -#include "net/http/http_auth_handler.h" // This is needed for the FRIEND_TEST() macro. #include "testing/gtest/include/gtest/gtest_prod.h" @@ -51,14 +50,17 @@ class HttpAuthCache { // already exists, update it rather than replace it -- this preserves the // paths list. // |origin| - the {scheme, host, port} of the server. - // |handler| - handler for the challenge. + // |realm| - the auth realm for the challenge. + // |scheme| - the authentication scheme for the challenge. // |username| - login information for the realm. // |password| - login information for the realm. // |path| - absolute path for a resource contained in the protection // space; this will be added to the list of known paths. // returns - the entry that was just added/updated. Entry* Add(const GURL& origin, - HttpAuthHandler* handler, + const std::string& realm, + const std::string& scheme, + const std::string& auth_challenge, const std::wstring& username, const std::wstring& password, const std::string& path); @@ -98,29 +100,33 @@ class HttpAuthCache::Entry { // The case-sensitive realm string of the challenge. const std::string realm() const { - return handler_->realm(); + return realm_; } // The authentication scheme string of the challenge const std::string scheme() const { - return handler_->scheme(); + return scheme_; } - // The handler for the challenge. - HttpAuthHandler* handler() const { - return handler_.get(); + // The authentication challenge. + const std::string auth_challenge() const { + return auth_challenge_; } // The login username. - const std::wstring& username() const { + const std::wstring username() const { return username_; } // The login password. - const std::wstring& password() const { + const std::wstring password() const { return password_; } + int IncrementNonceCount() { + return ++nonce_count_; + } + private: friend class HttpAuthCache; FRIEND_TEST(HttpAuthCacheTest, AddPath); @@ -137,13 +143,15 @@ class HttpAuthCache::Entry { // |origin_| contains the {scheme, host, port} of the server. GURL origin_; + std::string realm_; + std::string scheme_; // Identity. + std::string auth_challenge_; std::wstring username_; std::wstring password_; - // Auth handler for the challenge. - scoped_refptr<HttpAuthHandler> handler_; + int nonce_count_; // List of paths that define the realm's protection space. typedef std::list<std::string> PathList; diff --git a/net/http/http_auth_cache_unittest.cc b/net/http/http_auth_cache_unittest.cc index 28cb138..46d3f93 100644 --- a/net/http/http_auth_cache_unittest.cc +++ b/net/http/http_auth_cache_unittest.cc @@ -5,6 +5,7 @@ #include "base/string_util.h" #include "net/base/net_errors.h" #include "net/http/http_auth_cache.h" +#include "net/http/http_auth_handler.h" #include "testing/gtest/include/gtest/gtest.h" @@ -61,23 +62,28 @@ TEST(HttpAuthCacheTest, Basic) { scoped_refptr<HttpAuthHandler> realm1_handler = new MockAuthHandler("basic", "Realm1", HttpAuth::AUTH_SERVER); - cache.Add(origin, realm1_handler, L"realm1-user", L"realm1-password", + cache.Add(origin, realm1_handler->realm(), realm1_handler->scheme(), + "Basic realm=Realm1", L"realm1-user", L"realm1-password", "/foo/bar/index.html"); scoped_refptr<HttpAuthHandler> realm2_handler = new MockAuthHandler("basic", "Realm2", HttpAuth::AUTH_SERVER); - cache.Add(origin, realm2_handler, L"realm2-user", L"realm2-password", + cache.Add(origin, realm2_handler->realm(), realm2_handler->scheme(), + "Basic realm=Realm2", L"realm2-user", L"realm2-password", "/foo2/index.html"); scoped_refptr<HttpAuthHandler> realm3_basic_handler = new MockAuthHandler("basic", "Realm3", HttpAuth::AUTH_PROXY); - cache.Add(origin, realm3_basic_handler, L"realm3-basic-user", - L"realm3-basic-password", ""); + cache.Add(origin, realm3_basic_handler->realm(), + realm3_basic_handler->scheme(), "Basic realm=Realm3", + L"realm3-basic-user", L"realm3-basic-password", ""); scoped_refptr<HttpAuthHandler> realm3_digest_handler = new MockAuthHandler("digest", "Realm3", HttpAuth::AUTH_PROXY); - cache.Add(origin, realm3_digest_handler, L"realm3-digest-user", - L"realm3-digest-password", "/baz/index.html"); + cache.Add(origin, realm3_digest_handler->realm(), + realm3_digest_handler->scheme(), "Digest realm=Realm3", + L"realm3-digest-user", L"realm3-digest-password", + "/baz/index.html"); // There is no Realm4 entry = cache.Lookup(origin, "Realm4", "basic"); @@ -94,23 +100,29 @@ TEST(HttpAuthCacheTest, Basic) { // Valid lookup by origin, realm, scheme. entry = cache.Lookup(GURL("http://www.google.com:80"), "Realm3", "basic"); - EXPECT_FALSE(NULL == entry); - EXPECT_TRUE(entry->handler() == realm3_basic_handler.get()); + ASSERT_FALSE(NULL == entry); + EXPECT_EQ("basic", entry->scheme()); + EXPECT_EQ("Realm3", entry->realm()); + EXPECT_EQ("Basic realm=Realm3", entry->auth_challenge()); EXPECT_EQ(L"realm3-basic-user", entry->username()); EXPECT_EQ(L"realm3-basic-password", entry->password()); // Valid lookup by origin, realm, scheme when there's a duplicate // origin, realm in the cache entry = cache.Lookup(GURL("http://www.google.com:80"), "Realm3", "digest"); - EXPECT_FALSE(NULL == entry); - EXPECT_TRUE(entry->handler() == realm3_digest_handler.get()); + ASSERT_FALSE(NULL == entry); + EXPECT_EQ("digest", entry->scheme()); + EXPECT_EQ("Realm3", entry->realm()); + EXPECT_EQ("Digest realm=Realm3", entry->auth_challenge()); EXPECT_EQ(L"realm3-digest-user", entry->username()); EXPECT_EQ(L"realm3-digest-password", entry->password()); // Valid lookup by realm. entry = cache.Lookup(origin, "Realm2", "basic"); - EXPECT_FALSE(NULL == entry); - EXPECT_TRUE(entry->handler() == realm2_handler.get()); + ASSERT_FALSE(NULL == entry); + EXPECT_EQ("basic", entry->scheme()); + EXPECT_EQ("Realm2", entry->realm()); + EXPECT_EQ("Basic realm=Realm2", entry->auth_challenge()); EXPECT_EQ(L"realm2-user", entry->username()); EXPECT_EQ(L"realm2-password", entry->password()); @@ -148,10 +160,21 @@ TEST(HttpAuthCacheTest, Basic) { entry = cache.LookupByPath(origin, "/baz"); EXPECT_FALSE(realm3_digest_entry == entry); + // Confirm we find the same realm, different auth scheme by path lookup + HttpAuthCache::Entry* realm3DigestEntry = + cache.Lookup(origin, "Realm3", "digest"); + EXPECT_FALSE(NULL == realm3DigestEntry); + entry = cache.LookupByPath(origin, "/baz/index.html"); + EXPECT_TRUE(realm3DigestEntry == entry); + entry = cache.LookupByPath(origin, "/baz/"); + EXPECT_TRUE(realm3DigestEntry == entry); + entry = cache.LookupByPath(origin, "/baz"); + EXPECT_FALSE(realm3DigestEntry == entry); + // Lookup using empty path (may be used for proxy). entry = cache.LookupByPath(origin, ""); EXPECT_FALSE(NULL == entry); - EXPECT_TRUE(entry->handler() == realm3_basic_handler.get()); + EXPECT_EQ("basic", entry->scheme()); EXPECT_EQ("Realm3", entry->realm()); } @@ -192,14 +215,18 @@ TEST(HttpAuthCacheTest, AddPath) { TEST(HttpAuthCacheTest, AddToExistingEntry) { HttpAuthCache cache; GURL origin("http://www.foobar.com:70"); + const std::string auth_challenge = "Basic realm=MyRealm"; scoped_refptr<HttpAuthHandler> handler = new MockAuthHandler("basic", "MyRealm", HttpAuth::AUTH_SERVER); HttpAuthCache::Entry* orig_entry = cache.Add( - origin, handler, L"user1", L"password1", "/x/y/z/"); - cache.Add(origin, handler, L"user2", L"password2", "/z/y/x/"); - cache.Add(origin, handler, L"user3", L"password3", "/z/y"); + origin, handler->realm(), handler->scheme(), auth_challenge, + L"user1", L"password1", "/x/y/z/"); + cache.Add(origin, handler->realm(), handler->scheme(), auth_challenge, + L"user2", L"password2", "/z/y/x/"); + cache.Add(origin, handler->realm(), handler->scheme(), auth_challenge, + L"user3", L"password3", "/z/y"); HttpAuthCache::Entry* entry = cache.Lookup(origin, "MyRealm", "basic"); @@ -228,10 +255,16 @@ TEST(HttpAuthCacheTest, Remove) { new MockAuthHandler("digest", "Realm3", HttpAuth::AUTH_SERVER); HttpAuthCache cache; - cache.Add(origin, realm1_handler, L"alice", L"123", "/"); - cache.Add(origin, realm2_handler, L"bob", L"princess", "/"); - cache.Add(origin, realm3_basic_handler, L"admin", L"password", "/"); - cache.Add(origin, realm3_digest_handler, L"root", L"wilecoyote", "/"); + cache.Add(origin, realm1_handler->realm(), realm1_handler->scheme(), + "basic realm=Realm1", L"alice", L"123", "/"); + cache.Add(origin, realm2_handler->realm(), realm2_handler->scheme(), + "basic realm=Realm2", L"bob", L"princess", "/"); + cache.Add(origin, realm3_basic_handler->realm(), + realm3_basic_handler->scheme(), "basic realm=Realm3", L"admin", + L"password", "/"); + cache.Add(origin, realm3_digest_handler->realm(), + realm3_digest_handler->scheme(), "digest realm=Realm3", L"root", + L"wilecoyote", "/"); // Fails, because there is no realm "Realm4". EXPECT_FALSE(cache.Remove(origin, "Realm4", "basic", L"alice", L"123")); @@ -259,11 +292,13 @@ TEST(HttpAuthCacheTest, Remove) { EXPECT_TRUE(cache.Remove(origin, "Realm3", "digest", L"root", L"wilecoyote")); // Succeed as above, but when entries were added in opposite order - cache.Add(origin, realm3_digest_handler, L"root", L"wilecoyote", "/"); + cache.Add(origin, realm3_digest_handler->realm(), + realm3_digest_handler->scheme(), "digest realm=Realm3", L"root", + L"wilecoyote", "/"); EXPECT_TRUE(cache.Remove(origin, "Realm3", "basic", L"admin", L"password")); - // Make sure that removing one entry still leaves the other available - // for lookup + // Make sure that removing one entry still leaves the other available for + // lookup. HttpAuthCache::Entry* entry = cache.Lookup(origin, "Realm3", "digest"); EXPECT_FALSE(NULL == entry); } @@ -291,12 +326,13 @@ class HttpAuthCacheEvictionTest : public testing::Test { "basic", GenerateRealm(realm_i), HttpAuth::AUTH_SERVER); std::string path = GeneratePath(realm_i, path_i); - cache_.Add(origin_, handler, L"username", L"password", path); + cache_.Add(origin_, handler->realm(), handler->scheme(), + handler->challenge(), L"username", L"password", path); } void CheckRealmExistence(int realm_i, bool exists) { const HttpAuthCache::Entry* entry = - cache_.Lookup(origin_, GenerateRealm(realm_i), "basic"); + cache_.Lookup(origin_, GenerateRealm(realm_i), "basic"); if (exists) { EXPECT_FALSE(entry == NULL); EXPECT_EQ(GenerateRealm(realm_i), entry->realm()); diff --git a/net/http/http_auth_handler.cc b/net/http/http_auth_handler.cc index 7aa114e..7c57cc7 100644 --- a/net/http/http_auth_handler.cc +++ b/net/http/http_auth_handler.cc @@ -18,6 +18,7 @@ bool HttpAuthHandler::InitFromChallenge( score_ = -1; properties_ = -1; + auth_challenge_ = challenge->challenge_text(); bool ok = Init(challenge); // Init() is expected to set the scheme, realm, score, and properties. The diff --git a/net/http/http_auth_handler.h b/net/http/http_auth_handler.h index 16e7822..6f2cb50 100644 --- a/net/http/http_auth_handler.h +++ b/net/http/http_auth_handler.h @@ -42,6 +42,11 @@ class HttpAuthHandler : public base::RefCounted<HttpAuthHandler> { return realm_; } + // The challenge which was issued when creating the handler. + const std::string challenge() const { + return auth_challenge_; + } + // Numeric rank based on the challenge's security level. Higher // numbers are better. Used by HttpAuth::ChooseBestChallenge(). int score() const { @@ -143,6 +148,9 @@ class HttpAuthHandler : public base::RefCounted<HttpAuthHandler> { // The realm. Used by "basic" and "digest". std::string realm_; + // The auth challenge. + std::string auth_challenge_; + // The {scheme, host, port} for the authentication target. Used by "ntlm" // and "negotiate" to construct the service principal name. GURL origin_; diff --git a/net/http/http_auth_handler_basic.cc b/net/http/http_auth_handler_basic.cc index 46b7bcd..e8b812a 100644 --- a/net/http/http_auth_handler_basic.cc +++ b/net/http/http_auth_handler_basic.cc @@ -77,6 +77,8 @@ int HttpAuthHandlerBasic::Factory::CreateAuthHandler( HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) { // TODO(cbentzel): Move towards model of parsing in the factory // method and only constructing when valid. diff --git a/net/http/http_auth_handler_basic.h b/net/http/http_auth_handler_basic.h index be63e21..5b4806d 100644 --- a/net/http/http_auth_handler_basic.h +++ b/net/http/http_auth_handler_basic.h @@ -21,6 +21,8 @@ class HttpAuthHandlerBasic : public HttpAuthHandler { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler); }; diff --git a/net/http/http_auth_handler_digest.cc b/net/http/http_auth_handler_digest.cc index 107cfcf..53edf06 100644 --- a/net/http/http_auth_handler_digest.cc +++ b/net/http/http_auth_handler_digest.cc @@ -94,12 +94,6 @@ int HttpAuthHandlerDigest::GenerateAuthToken( // Generate a random client nonce. std::string cnonce = GenerateNonce(); - // The nonce-count should be incremented after re-use per the spec. - // This may not be possible when there are multiple connections to the - // server though: - // https://bugzilla.mozilla.org/show_bug.cgi?id=114451 - int nonce_count = ++nonce_count_; - // Extract the request method and path -- the meaning of 'path' is overloaded // in certain cases, to be a hostname. std::string method; @@ -110,7 +104,7 @@ int HttpAuthHandlerDigest::GenerateAuthToken( // TODO(eroman): is this the right encoding? WideToUTF8(username), WideToUTF8(password), - cnonce, nonce_count); + cnonce, nonce_count_); return OK; } @@ -310,10 +304,13 @@ int HttpAuthHandlerDigest::Factory::CreateAuthHandler( HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) { // TODO(cbentzel): Move towards model of parsing in the factory // method and only constructing when valid. - scoped_refptr<HttpAuthHandler> tmp_handler(new HttpAuthHandlerDigest()); + scoped_refptr<HttpAuthHandler> tmp_handler( + new HttpAuthHandlerDigest(digest_nonce_count)); if (!tmp_handler->InitFromChallenge(challenge, target, origin)) return ERR_INVALID_RESPONSE; handler->swap(tmp_handler); diff --git a/net/http/http_auth_handler_digest.h b/net/http/http_auth_handler_digest.h index 876e346..d575a40 100644 --- a/net/http/http_auth_handler_digest.h +++ b/net/http/http_auth_handler_digest.h @@ -24,6 +24,8 @@ class HttpAuthHandlerDigest : public HttpAuthHandler { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler); }; @@ -39,7 +41,6 @@ class HttpAuthHandlerDigest : public HttpAuthHandler { protected: virtual bool Init(HttpAuth::ChallengeTokenizer* challenge) { - nonce_count_ = 0; return ParseChallenge(challenge); } @@ -70,6 +71,8 @@ class HttpAuthHandlerDigest : public HttpAuthHandler { QOP_AUTH_INT = 1 << 1, }; + explicit HttpAuthHandlerDigest(int nonce_count) : nonce_count_(nonce_count) {} + ~HttpAuthHandlerDigest() {} // Parse the challenge, saving the results into this instance. diff --git a/net/http/http_auth_handler_digest_unittest.cc b/net/http/http_auth_handler_digest_unittest.cc index 6af03fe..1b4130b 100644 --- a/net/http/http_auth_handler_digest_unittest.cc +++ b/net/http/http_auth_handler_digest_unittest.cc @@ -103,7 +103,7 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) { for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { std::string challenge(tests[i].challenge); - scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest; + scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest(1); HttpAuth::ChallengeTokenizer tok(challenge.begin(), challenge.end()); bool ok = digest->ParseChallenge(&tok); @@ -251,7 +251,7 @@ TEST(HttpAuthHandlerDigestTest, AssembleCredentials) { }; GURL origin("http://www.example.com"); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { - scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest; + scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest(1); std::string challenge = tests[i].challenge; HttpAuth::ChallengeTokenizer tok(challenge.begin(), challenge.end()); EXPECT_TRUE(digest->InitFromChallenge(&tok, HttpAuth::AUTH_SERVER, origin)); diff --git a/net/http/http_auth_handler_factory.cc b/net/http/http_auth_handler_factory.cc index 2921b66..6c09def 100644 --- a/net/http/http_auth_handler_factory.cc +++ b/net/http/http_auth_handler_factory.cc @@ -21,7 +21,19 @@ int HttpAuthHandlerFactory::CreateAuthHandlerFromString( const GURL& origin, scoped_refptr<HttpAuthHandler>* handler) { HttpAuth::ChallengeTokenizer props(challenge.begin(), challenge.end()); - return CreateAuthHandler(&props, target, origin, handler); + return CreateAuthHandler(&props, target, origin, CREATE_CHALLENGE, 1, + handler); +} + +int HttpAuthHandlerFactory::CreatePreemptiveAuthHandlerFromString( + const std::string& challenge, + HttpAuth::Target target, + const GURL& origin, + int digest_nonce_count, + scoped_refptr<HttpAuthHandler>* handler) { + HttpAuth::ChallengeTokenizer props(challenge.begin(), challenge.end()); + return CreateAuthHandler(&props, target, origin, CREATE_PREEMPTIVE, + digest_nonce_count, handler); } // static @@ -73,6 +85,8 @@ int HttpAuthHandlerRegistryFactory::CreateAuthHandler( HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) { if (!challenge->valid()) { *handler = NULL; @@ -85,7 +99,8 @@ int HttpAuthHandlerRegistryFactory::CreateAuthHandler( return ERR_UNSUPPORTED_AUTH_SCHEME; } DCHECK(it->second); - return it->second->CreateAuthHandler(challenge, target, origin, handler); + return it->second->CreateAuthHandler(challenge, target, origin, reason, + digest_nonce_count, handler); } HttpAuthHandlerFactory* HttpAuthHandlerRegistryFactory::GetSchemeFactory( diff --git a/net/http/http_auth_handler_factory.h b/net/http/http_auth_handler_factory.h index 0b65f1e..96213db 100644 --- a/net/http/http_auth_handler_factory.h +++ b/net/http/http_auth_handler_factory.h @@ -37,6 +37,11 @@ class HttpAuthHandlerFactory { return url_security_manager_; } + enum CreateReason { + CREATE_CHALLENGE, // Create a handler in response to a challenge. + CREATE_PREEMPTIVE, // Create a handler preemptively. + }; + // Creates an HttpAuthHandler object based on the authentication // challenge specified by |*challenge|. |challenge| must point to a valid // non-NULL tokenizer. @@ -50,6 +55,13 @@ class HttpAuthHandlerFactory { // If |*challenge| is improperly formed, |*handler| is set to NULL and // ERR_INVALID_RESPONSE is returned. // + // |create_reason| indicates why the handler is being created. This is used + // since NTLM and Negotiate schemes do not support preemptive creation. + // + // |digest_nonce_count| is specifically intended for the Digest authentication + // scheme, and indicates the number of handlers generated for a particular + // server nonce challenge. + // // For the NTLM and Negotiate handlers: // If |origin| does not match the authentication method's filters for // the specified |target|, ERR_INVALID_AUTH_CREDENTIALS is returned. @@ -59,6 +71,8 @@ class HttpAuthHandlerFactory { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason create_reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) = 0; // Creates an HTTP authentication handler based on the authentication @@ -71,6 +85,18 @@ class HttpAuthHandlerFactory { const GURL& origin, scoped_refptr<HttpAuthHandler>* handler); + // Creates an HTTP authentication handler based on the authentication + // challenge string |challenge|. + // This is a convenience function which creates a ChallengeTokenizer for + // |challenge| and calls |CreateAuthHandler|. See |CreateAuthHandler| for + // more details on return values. + int CreatePreemptiveAuthHandlerFromString( + const std::string& challenge, + HttpAuth::Target target, + const GURL& origin, + int digest_nonce_count, + scoped_refptr<HttpAuthHandler>* handler); + // Creates a standard HttpAuthHandlerRegistryFactory. The caller is // responsible for deleting the factory. // The default factory supports Basic, Digest, NTLM, and Negotiate schemes. @@ -117,6 +143,8 @@ class HttpAuthHandlerRegistryFactory : public HttpAuthHandlerFactory { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler); private: diff --git a/net/http/http_auth_handler_factory_unittest.cc b/net/http/http_auth_handler_factory_unittest.cc index da6e5b9..5293b89 100644 --- a/net/http/http_auth_handler_factory_unittest.cc +++ b/net/http/http_auth_handler_factory_unittest.cc @@ -19,6 +19,8 @@ class MockHttpAuthHandlerFactory : public HttpAuthHandlerFactory { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int nonce_count, scoped_refptr<HttpAuthHandler>* handler) { *handler = NULL; return return_code_; diff --git a/net/http/http_auth_handler_negotiate.h b/net/http/http_auth_handler_negotiate.h index 77add14..4e993f4 100644 --- a/net/http/http_auth_handler_negotiate.h +++ b/net/http/http_auth_handler_negotiate.h @@ -53,6 +53,8 @@ class HttpAuthHandlerNegotiate : public HttpAuthHandler { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler); #if defined(OS_WIN) diff --git a/net/http/http_auth_handler_negotiate_posix.cc b/net/http/http_auth_handler_negotiate_posix.cc index 50ff1c0..c4d2a0f 100644 --- a/net/http/http_auth_handler_negotiate_posix.cc +++ b/net/http/http_auth_handler_negotiate_posix.cc @@ -88,6 +88,8 @@ int HttpAuthHandlerNegotiate::Factory::CreateAuthHandler( HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) { return ERR_UNSUPPORTED_AUTH_SCHEME; } diff --git a/net/http/http_auth_handler_negotiate_win.cc b/net/http/http_auth_handler_negotiate_win.cc index 4be5923..b2a3569 100644 --- a/net/http/http_auth_handler_negotiate_win.cc +++ b/net/http/http_auth_handler_negotiate_win.cc @@ -195,8 +195,10 @@ int HttpAuthHandlerNegotiate::Factory::CreateAuthHandler( HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) { - if (is_unsupported_) + if (is_unsupported_ || reason == CREATE_PREEMPTIVE) return ERR_UNSUPPORTED_AUTH_SCHEME; if (max_token_length_ == 0) { int rv = DetermineMaxTokenLength(sspi_library_, NEGOSSP_NAME, diff --git a/net/http/http_auth_handler_ntlm.h b/net/http/http_auth_handler_ntlm.h index 1381b02..e778592 100644 --- a/net/http/http_auth_handler_ntlm.h +++ b/net/http/http_auth_handler_ntlm.h @@ -44,6 +44,8 @@ class HttpAuthHandlerNTLM : public HttpAuthHandler { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler); #if defined(NTLM_SSPI) // Set the SSPILibrary to use. Typically the only callers which need to diff --git a/net/http/http_auth_handler_ntlm_portable.cc b/net/http/http_auth_handler_ntlm_portable.cc index f65b104..e16f791 100644 --- a/net/http/http_auth_handler_ntlm_portable.cc +++ b/net/http/http_auth_handler_ntlm_portable.cc @@ -729,7 +729,11 @@ int HttpAuthHandlerNTLM::Factory::CreateAuthHandler( HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) { + if (reason == CREATE_PREEMPTIVE) + return ERR_UNSUPPORTED_AUTH_SCHEME; // TODO(cbentzel): Move towards model of parsing in the factory // method and only constructing when valid. // NOTE: Default credentials are not supported for the portable implementation diff --git a/net/http/http_auth_handler_ntlm_win.cc b/net/http/http_auth_handler_ntlm_win.cc index 7488110..c54520e 100644 --- a/net/http/http_auth_handler_ntlm_win.cc +++ b/net/http/http_auth_handler_ntlm_win.cc @@ -74,8 +74,10 @@ int HttpAuthHandlerNTLM::Factory::CreateAuthHandler( HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int digest_nonce_count, scoped_refptr<HttpAuthHandler>* handler) { - if (is_unsupported_) + if (is_unsupported_ || reason == CREATE_PREEMPTIVE) return ERR_UNSUPPORTED_AUTH_SCHEME; if (max_token_length_ == 0) { int rv = DetermineMaxTokenLength(sspi_library_, NTLMSP_NAME, diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc index d6624be..11dac6a 100644 --- a/net/http/http_cache.cc +++ b/net/http/http_cache.cc @@ -29,6 +29,7 @@ #include "net/http/http_request_info.h" #include "net/http/http_response_headers.h" #include "net/http/http_response_info.h" +#include "net/http/http_util.h" #include "net/spdy/spdy_session_pool.h" namespace net { diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 98effe3..d6c0d5f1 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -26,6 +26,7 @@ #include "net/base/upload_data_stream.h" #include "net/http/http_auth.h" #include "net/http/http_auth_handler.h" +#include "net/http/http_auth_handler_factory.h" #include "net/http/http_basic_stream.h" #include "net/http/http_chunked_decoder.h" #include "net/http/http_network_session.h" @@ -445,8 +446,12 @@ void HttpNetworkTransaction::PrepareForAuthRestart(HttpAuth::Target target) { break; default: session_->auth_cache()->Add( - AuthOrigin(target), auth_handler_[target], - auth_identity_[target].username, auth_identity_[target].password, + AuthOrigin(target), + auth_handler_[target]->realm(), + auth_handler_[target]->scheme(), + auth_handler_[target]->challenge(), + auth_identity_[target].username, + auth_identity_[target].password, AuthPath(target)); break; } @@ -1911,26 +1916,27 @@ bool HttpNetworkTransaction::SelectPreemptiveAuth(HttpAuth::Target target) { // is expected to be fast. LookupByPath() is fast in the common case, since // the number of http auth cache entries is expected to be very small. // (For most users in fact, it will be 0.) - HttpAuthCache::Entry* entry = session_->auth_cache()->LookupByPath( AuthOrigin(target), AuthPath(target)); + if (!entry) + return false; - // We don't support preemptive authentication for connection-based - // authentication schemes because they can't reuse entry->handler(). - // Hopefully we can remove this limitation in the future. - if (entry && !entry->handler()->is_connection_based()) { - auth_identity_[target].source = HttpAuth::IDENT_SRC_PATH_LOOKUP; - auth_identity_[target].invalid = false; - auth_identity_[target].username = entry->username(); - auth_identity_[target].password = entry->password(); - auth_handler_[target] = entry->handler(); - return true; - } - - // TODO(cbentzel): Preemptively use default credentials if they have worked - // for the origin/path in the past to save a round trip. + // Try to create a handler using the previous auth challenge. + scoped_refptr<HttpAuthHandler> handler_preemptive; + int rv_create = session_->http_auth_handler_factory()-> + CreatePreemptiveAuthHandlerFromString( + entry->auth_challenge(), target, AuthOrigin(target), + entry->IncrementNonceCount(), &handler_preemptive); + if (rv_create != OK) + return false; - return false; + // Set the state + auth_identity_[target].source = HttpAuth::IDENT_SRC_PATH_LOOKUP; + auth_identity_[target].invalid = false; + auth_identity_[target].username = entry->username(); + auth_identity_[target].password = entry->password(); + auth_handler_[target] = handler_preemptive; + return true; } bool HttpNetworkTransaction::SelectNextAuthIdentityToTry( diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index 6dac810..32b13ca 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -5488,6 +5488,8 @@ class MockAuthHandlerCanonical : public HttpAuthHandler { virtual int CreateAuthHandler(HttpAuth::ChallengeTokenizer* challenge, HttpAuth::Target target, const GURL& origin, + CreateReason reason, + int nonce_count, scoped_refptr<HttpAuthHandler>* handler) { *handler = mock_handler_; return OK; diff --git a/net/socket_stream/socket_stream.cc b/net/socket_stream/socket_stream.cc index 68fbbf3..b2f7a2c 100644 --- a/net/socket_stream/socket_stream.cc +++ b/net/socket_stream/socket_stream.cc @@ -18,6 +18,7 @@ #include "net/base/io_buffer.h" #include "net/base/net_errors.h" #include "net/base/net_util.h" +#include "net/http/http_auth_handler_factory.h" #include "net/http/http_response_headers.h" #include "net/http/http_util.h" #include "net/socket/client_socket_factory.h" @@ -540,15 +541,23 @@ int SocketStream::DoWriteTunnelHeaders() { std::string authorization_headers; if (!auth_handler_.get()) { - // First attempt. Find auth from the proxy address. + // Do preemptive authentication. HttpAuthCache::Entry* entry = auth_cache_.LookupByPath( ProxyAuthOrigin(), std::string()); - if (entry && !entry->handler()->is_connection_based()) { - auth_identity_.source = HttpAuth::IDENT_SRC_PATH_LOOKUP; - auth_identity_.invalid = false; - auth_identity_.username = entry->username(); - auth_identity_.password = entry->password(); - auth_handler_ = entry->handler(); + if (entry) { + scoped_refptr<HttpAuthHandler> handler_preemptive; + int rv_create = http_auth_handler_factory_-> + CreatePreemptiveAuthHandlerFromString( + entry->auth_challenge(), HttpAuth::AUTH_PROXY, + ProxyAuthOrigin(), entry->IncrementNonceCount(), + &handler_preemptive); + if (rv_create == OK) { + auth_identity_.source = HttpAuth::IDENT_SRC_PATH_LOOKUP; + auth_identity_.invalid = false; + auth_identity_.username = entry->username(); + auth_identity_.password = entry->password(); + auth_handler_ = handler_preemptive; + } } } @@ -881,7 +890,7 @@ int SocketStream::HandleAuthChallenge(const HttpResponseHeaders* headers) { // We only support basic authentication scheme now. // TODO(ukai): Support other authentication scheme. HttpAuthCache::Entry* entry = - auth_cache_.Lookup(auth_origin, auth_handler_->realm(), "basic"); + auth_cache_.Lookup(auth_origin, auth_handler_->realm(), "basic"); if (entry) { auth_identity_.source = HttpAuth::IDENT_SRC_REALM_LOOKUP; auth_identity_.invalid = false; @@ -905,8 +914,12 @@ void SocketStream::DoAuthRequired() { void SocketStream::DoRestartWithAuth() { DCHECK_EQ(next_state_, STATE_AUTH_REQUIRED); - auth_cache_.Add(ProxyAuthOrigin(), auth_handler_, - auth_identity_.username, auth_identity_.password, + auth_cache_.Add(ProxyAuthOrigin(), + auth_handler_->realm(), + auth_handler_->scheme(), + auth_handler_->challenge(), + auth_identity_.username, + auth_identity_.password, std::string()); tunnel_request_headers_ = NULL; |