diff options
author | ericroman@google.com <ericroman@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-11-08 06:46:23 +0000 |
---|---|---|
committer | ericroman@google.com <ericroman@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-11-08 06:46:23 +0000 |
commit | f9ee6b5a5925d8496f05309963c42bfdd3ec1a8b (patch) | |
tree | 7867cc64559bf86408da5a744e918d2861bf3889 | |
parent | f6028ee8661996ba41763a6601469ebd599480f5 (diff) | |
download | chromium_src-f9ee6b5a5925d8496f05309963c42bfdd3ec1a8b.zip chromium_src-f9ee6b5a5925d8496f05309963c42bfdd3ec1a8b.tar.gz chromium_src-f9ee6b5a5925d8496f05309963c42bfdd3ec1a8b.tar.bz2 |
- Add preemptive authorization (new http stack only)
- Check for auth identity in URL (new http stack only)
- Move auth cache logic out of url request job, and hide it in the url request ftp job and http transaction classes.
Note: Somehow the original codereview thread got corrupted so it was recreated.
The real review comments should be under (http://codereview.chromium.org/6481)
Review URL: http://codereview.chromium.org/8231
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5064 0039d316-1c4b-4281-b951-d872f2087c98
34 files changed, 1418 insertions, 209 deletions
diff --git a/net/build/net.vcproj b/net/build/net.vcproj index 33b5d77..85b387d 100644 --- a/net/build/net.vcproj +++ b/net/build/net.vcproj @@ -769,6 +769,14 @@ > </File> <File + RelativePath="..\http\http_auth_cache.cc" + > + </File> + <File + RelativePath="..\http\http_auth_cache.h" + > + </File> + <File RelativePath="..\http\http_auth_handler.h" > </File> diff --git a/net/build/net_unittests.vcproj b/net/build/net_unittests.vcproj index 51fe42c..f92d227 100644 --- a/net/build/net_unittests.vcproj +++ b/net/build/net_unittests.vcproj @@ -227,6 +227,10 @@ Name="http" > <File + RelativePath="..\http\http_auth_cache_unittest.cc" + > + </File> + <File RelativePath="..\http\http_auth_handler_basic_unittest.cc" > </File> diff --git a/net/http/http_auth.cc b/net/http/http_auth.cc index 324f901..c3764cc 100644 --- a/net/http/http_auth.cc +++ b/net/http/http_auth.cc @@ -16,42 +16,44 @@ namespace net { // static -HttpAuthHandler* HttpAuth::ChooseBestChallenge( - const HttpResponseHeaders* headers, Target target) { +void HttpAuth::ChooseBestChallenge(const HttpResponseHeaders* headers, + Target target, + scoped_refptr<HttpAuthHandler>* handler) { // Choose the challenge whose authentication handler gives the maximum score. - scoped_ptr<HttpAuthHandler> best; + scoped_refptr<HttpAuthHandler> best; const std::string header_name = GetChallengeHeaderName(target); std::string cur_challenge; void* iter = NULL; while (headers->EnumerateHeader(&iter, header_name, &cur_challenge)) { - scoped_ptr<HttpAuthHandler> cur( - CreateAuthHandler(cur_challenge, target)); - if (cur.get() && (!best.get() || best->score() < cur->score())) - best.reset(cur.release()); + scoped_refptr<HttpAuthHandler> cur; + CreateAuthHandler(cur_challenge, target, &cur); + if (cur && (!best || best->score() < cur->score())) + best.swap(cur); } - return best.release(); + handler->swap(best); } // static -HttpAuthHandler* HttpAuth::CreateAuthHandler(const std::string& challenge, - Target target) { +void HttpAuth::CreateAuthHandler(const std::string& challenge, + Target target, + scoped_refptr<HttpAuthHandler>* handler) { // Find the right auth handler for the challenge's scheme. ChallengeTokenizer props(challenge.begin(), challenge.end()); - scoped_ptr<HttpAuthHandler> handler; + scoped_refptr<HttpAuthHandler> tmp_handler; if (LowerCaseEqualsASCII(props.scheme(), "basic")) { - handler.reset(new HttpAuthHandlerBasic()); + tmp_handler = new HttpAuthHandlerBasic(); } else if (LowerCaseEqualsASCII(props.scheme(), "digest")) { - handler.reset(new HttpAuthHandlerDigest()); + tmp_handler = new HttpAuthHandlerDigest(); } - if (handler.get()) { - if (!handler->InitFromChallenge(challenge.begin(), challenge.end(), - target)) { + if (tmp_handler) { + if (!tmp_handler->InitFromChallenge(challenge.begin(), challenge.end(), + target)) { // Invalid/unsupported challenge. - return NULL; + tmp_handler = NULL; } } - return handler.release(); + handler->swap(tmp_handler); } void HttpAuth::ChallengeTokenizer::Init(std::string::const_iterator begin, diff --git a/net/http/http_auth.h b/net/http/http_auth.h index 34d10f0..3bb2a86 100644 --- a/net/http/http_auth.h +++ b/net/http/http_auth.h @@ -5,6 +5,7 @@ #ifndef NET_HTTP_HTTP_AUTH_H_ #define NET_HTTP_HTTP_AUTH_H_ +#include "base/ref_counted.h" #include "net/http/http_util.h" namespace net { @@ -23,6 +24,39 @@ class HttpAuth { AUTH_SERVER = 1, }; + // Describes where the identity used for authentication came from. + enum IdentitySource { + // Came from nowhere -- the identity is not initialized. + IDENT_SRC_NONE, + + // The identity came from the auth cache, by doing a path-based + // lookup (premptive authorization). + IDENT_SRC_PATH_LOOKUP, + + // The identity was extracted from a URL of the form: + // http://<username>:<password>@host:port + IDENT_SRC_URL, + + // The identity was retrieved from the auth cache, by doing a + // realm lookup. + IDENT_SRC_REALM_LOOKUP, + + // The identity was provided by RestartWithAuth -- it likely + // came from a prompt (or maybe the password manager). + IDENT_SRC_EXTERNAL, + }; + + // Helper structure used by HttpNetworkTransaction to track + // the current identity being used for authorization. + struct Identity { + Identity() : source(IDENT_SRC_NONE), invalid(true) { } + + IdentitySource source; + bool invalid; + std::wstring username; + std::wstring password; + }; + // Get the name of the header containing the auth challenge // (either WWW-Authenticate or Proxy-Authenticate). static std::string GetChallengeHeaderName(Target target); @@ -31,18 +65,20 @@ class HttpAuth { // (either Authorization or Proxy-Authorization). static std::string GetAuthorizationHeaderName(Target target); - // Create a handler to generate credentials for the challenge. If the - // challenge is unsupported or invalid, returns NULL. - // The caller owns the returned pointer. - static HttpAuthHandler* CreateAuthHandler(const std::string& challenge, - Target target); + // Create a handler to generate credentials for the challenge, and pass + // it back in |*handler|. If the challenge is unsupported or invalid + // |*handler| is set to NULL. + static void CreateAuthHandler(const std::string& challenge, + Target target, + scoped_refptr<HttpAuthHandler>* handler); // Iterate through the challenge headers, and pick the best one that - // we support. Returns the implementation class for handling the challenge. - // If no supported challenge was found, returns NULL. - // The caller owns the returned pointer. - static HttpAuthHandler* ChooseBestChallenge( - const HttpResponseHeaders* headers, Target target); + // we support. Obtains the implementation class for handling the challenge, + // and passes it back in |*handler|. If no supported challenge was found + // |*handler| is set to NULL. + static void ChooseBestChallenge(const HttpResponseHeaders* headers, + Target target, + scoped_refptr<HttpAuthHandler>* handler); // 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 new file mode 100644 index 0000000..bced3fa --- /dev/null +++ b/net/http/http_auth_cache.cc @@ -0,0 +1,179 @@ +// Copyright (c) 2008 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_cache.h" + +#include "base/logging.h" +#include "base/string_util.h" + +namespace { + +// Helper to find the containing directory of path. In RFC 2617 this is what +// they call the "last symbolic element in the absolute path". +// Examples: +// "/foo/bar.txt" --> "/foo/" +// "/foo/" --> "/foo/" +std::string GetParentDirectory(const std::string& path) { + std::string::size_type last_slash = path.rfind("/"); + if (last_slash == std::string::npos) { + // No slash (absolute paths always start with slash, so this must be + // the proxy case which uses empty string). + DCHECK(path.empty()); + return path; + } + return path.substr(0, last_slash + 1); +} + +// Debug helper to check that |path| arguments are properly formed. +// (should be absolute path, or empty string). +void CheckPathIsValid(const std::string& path) { + DCHECK(path.empty() || path[0] == '/'); +} + +// Return true if |path| is a subpath of |container|. In other words, is +// |container| an ancestor of |path|? +bool IsEnclosingPath(const std::string& container, const std::string& path) { + DCHECK(container.empty() || *(container.end() - 1) == '/'); + return (container.empty() && path.empty()) || + (!container.empty() && StartsWithASCII(path, container, true)); +} + +// Debug helper to check that |origin| arguments are properly formed. +void CheckOriginIsValid(const GURL& origin) { + DCHECK(origin.is_valid()); + DCHECK(origin.SchemeIs("http") || origin.SchemeIs("https")); + DCHECK(origin.GetOrigin() == origin); +} + +// Functor used by remove_if. +struct IsEnclosedBy { + IsEnclosedBy(const std::string& path) : path(path) { } + bool operator() (const std::string& x) { + return IsEnclosingPath(path, x); + } + const std::string& path; +}; + +// 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 +// with number of elements in the cache). +const size_t kMaxNumPathsPerRealmEntry = 10; +const size_t kMaxNumRealmEntries = 10; + +} // namespace + +namespace net { + +// Performance: O(n), where n is the number of realm entries. +HttpAuthCache::Entry* HttpAuthCache::LookupByRealm(const GURL& origin, + const std::string& realm) { + CheckOriginIsValid(origin); + + // Linear scan through the realm entries. + for (EntryList::iterator it = entries_.begin(); it != entries_.end(); ++it) { + if (it->origin() == origin && it->realm() == realm) + return &(*it); + } + return NULL; // No realm entry found. +} + +// Performance: O(n*m), where n is the number of realm entries, m is the number +// of path entries per realm. Both n amd m are expected to be small; m is +// kept small because AddPath() only keeps the shallowest entry. +HttpAuthCache::Entry* HttpAuthCache::LookupByPath(const GURL& origin, + const std::string& path) { + CheckOriginIsValid(origin); + CheckPathIsValid(path); + + // RFC 2617 section 2: + // A client SHOULD assume that all paths at or deeper than the depth of + // the last symbolic element in the path field of the Request-URI also are + // within the protection space ... + std::string parent_dir = GetParentDirectory(path); + + // Linear scan through the realm entries. + for (EntryList::iterator it = entries_.begin(); it != entries_.end(); ++it) { + if (it->origin() == origin && it->HasEnclosingPath(parent_dir)) + return &(*it); + } + return NULL; // No entry found. +} + +HttpAuthCache::Entry* HttpAuthCache::Add(const GURL& origin, + HttpAuthHandler* handler, + const std::wstring& username, + const std::wstring& password, + const std::string& path) { + CheckOriginIsValid(origin); + CheckPathIsValid(path); + + // Check for existing entry (we will re-use it if present). + HttpAuthCache::Entry* entry = LookupByRealm(origin, handler->realm()); + + if (!entry) { + // Failsafe to prevent unbounded memory growth of the cache. + if (entries_.size() >= kMaxNumRealmEntries) { + LOG(WARNING) << "Num auth cache entries reached limit -- evicting"; + entries_.pop_back(); + } + + entries_.push_front(Entry()); + entry = &entries_.front(); + entry->origin_ = origin; + } + + entry->username_ = username; + entry->password_ = password; + entry->handler_ = handler; + entry->AddPath(path); + + return entry; +} + +void HttpAuthCache::Entry::AddPath(const std::string& path) { + std::string parent_dir = GetParentDirectory(path); + if (!HasEnclosingPath(parent_dir)) { + // Remove any entries that have been subsumed by the new entry. + paths_.remove_if(IsEnclosedBy(parent_dir)); + + // Failsafe to prevent unbounded memory growth of the cache. + if (paths_.size() >= kMaxNumPathsPerRealmEntry) { + LOG(WARNING) << "Num path entries for " << origin() + << " has grown too large -- evicting"; + paths_.pop_back(); + } + + // Add new path. + paths_.push_front(parent_dir); + } +} + +bool HttpAuthCache::Entry::HasEnclosingPath(const std::string& dir) { + DCHECK(GetParentDirectory(dir) == dir); + for (PathList::const_iterator it = paths_.begin(); it != paths_.end(); + ++it) { + if (IsEnclosingPath(*it, dir)) + return true; + } + return false; +} + +bool HttpAuthCache::Remove(const GURL& origin, + const std::string& realm, + const std::wstring& username, + const std::wstring& password) { + for (EntryList::iterator it = entries_.begin(); it != entries_.end(); ++it) { + if (it->origin() == origin && it->realm() == realm) { + if (username == it->username() && password == it->password()) { + entries_.erase(it); + return true; + } + return false; + } + } + return false; +} + +} // namespace net diff --git a/net/http/http_auth_cache.h b/net/http/http_auth_cache.h new file mode 100644 index 0000000..ab43f9b --- /dev/null +++ b/net/http/http_auth_cache.h @@ -0,0 +1,140 @@ +// Copyright (c) 2008 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. + +#ifndef NET_HTTP_HTTP_AUTH_CACHE_H_ +#define NET_HTTP_HTTP_AUTH_CACHE_H_ + +#include <list> +#include <string> + +#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" + +namespace net { + +// TODO(eroman): Can we change the key from (origin, realm) to +// (origin, realm, auth_scheme)? + +// HttpAuthCache stores HTTP authentication identities and challenge info. +// For each realm the cache stores a HttpAuthCache::Entry, which holds: +// - the realm name +// - the origin server {scheme, host, port} +// - the last identity used (username/password) +// - the last auth handler used +// - the list of paths which used this realm +// Entries can be looked up by either (origin, realm) or (origin, path). +class HttpAuthCache { + public: + class Entry; + + // Find the realm entry on server |origin| for realm |realm|. + // |origin| - the {scheme, host, port} of the server. + // |realm| - case sensitive realm string. + // returns - the matched entry or NULL. + Entry* LookupByRealm(const GURL& origin, const std::string& realm); + + // Find the realm entry on server |origin| whose protection space includes + // |path|. This uses the assumption in RFC 2617 section 2 that deeper + // paths lie in the same protection space. + // |origin| - the {scheme, host, port} of the server. + // |path| - absolute path of the resource, or empty string in case of + // proxy auth (which does not use the concept of paths). + // returns - the matched entry or NULL. + Entry* LookupByPath(const GURL& origin, const std::string& path); + + // Add a realm entry on server |origin| for realm |handler->realm()|, If an + // entry for this realm already exists, update it rather than replace it -- + // this preserves the realm's paths list. + // |origin| - the {scheme, host, port} of the server. + // |handler| - handler 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::wstring& username, + const std::wstring& password, + const std::string& path); + + // Remove realm entry on server |origin| for realm |realm| if one exists + // AND if the cached identity matches (|username|, |password|). + // |origin| - the {scheme, host, port} of the server. + // |realm| - case sensitive realm string. + // |username| - condition to match. + // |password| - condition to match. + // returns - true if an entry was removed. + bool Remove(const GURL& origin, + const std::string& realm, + const std::wstring& username, + const std::wstring& password); + + private: + typedef std::list<Entry> EntryList; + EntryList entries_; +}; + +// An authentication realm entry. +class HttpAuthCache::Entry { + public: + const GURL& origin() const { + return origin_; + } + + // The case-sensitive realm string of the challenge. + const std::string realm() const { + return handler_->realm(); + } + + // The handler for the challenge. + HttpAuthHandler* handler() const { + return handler_.get(); + } + + // The login username. + const std::wstring& username() const { + return username_; + } + + // The login password. + const std::wstring& password() const { + return password_; + } + + private: + friend class HttpAuthCache; + FRIEND_TEST(HttpAuthCacheTest, AddPath); + FRIEND_TEST(HttpAuthCacheTest, AddToExistingEntry); + + Entry() {} + + // Adds a path defining the realm's protection space. If the path is + // already contained in the protection space, is a no-op. + void AddPath(const std::string& path); + + // Returns true if |dir| is contained within the realm's protection space. + bool HasEnclosingPath(const std::string& dir); + + // |origin_| contains the {scheme, host, port} of the server. + GURL origin_; + + // Identity. + std::wstring username_; + std::wstring password_; + + // Auth handler for the challenge. + scoped_refptr<HttpAuthHandler> handler_; + + // List of paths that define the realm's protection space. + typedef std::list<std::string> PathList; + PathList paths_; +}; + +} // namespace net + +#endif // NET_HTTP_HTTP_AUTH_CACHE_H_ diff --git a/net/http/http_auth_cache_unittest.cc b/net/http/http_auth_cache_unittest.cc new file mode 100644 index 0000000..44429fe --- /dev/null +++ b/net/http/http_auth_cache_unittest.cc @@ -0,0 +1,206 @@ +// Copyright (c) 2008 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_cache.h" + +#include "testing/gtest/include/gtest/gtest.h" + +namespace net { + +namespace { + +class MockAuthHandler : public HttpAuthHandler { + public: + MockAuthHandler(const char* scheme, const char* realm, + HttpAuth::Target target) { + // Can't use initializer list since these are members of the base class. + scheme_ = scheme; + realm_ = realm; + score_ = 1; + target_ = target; + } + + virtual std::string GenerateCredentials(const std::wstring&, + const std::wstring&, + const HttpRequestInfo*, + const ProxyInfo*) { + return "mock-credentials"; // Unused. + } + + protected: + virtual bool Init(std::string::const_iterator, std::string::const_iterator) { + return false; // Unused. + } +}; + +} // namespace + +// Test adding and looking-up cache entries (both by realm and by path). +TEST(HttpAuthCacheTest, Basic) { + GURL origin("http://www.google.com"); + HttpAuthCache cache; + HttpAuthCache::Entry* entry; + + // Add cache entries for 3 realms: "Realm1", "Realm2", "Realm3" + + scoped_refptr<HttpAuthHandler> realm1_handler = + new MockAuthHandler("basic", "Realm1", HttpAuth::AUTH_SERVER); + cache.Add(origin, realm1_handler, 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", + "/foo2/index.html"); + + scoped_refptr<HttpAuthHandler> realm3_handler = + new MockAuthHandler("basic", "Realm3", HttpAuth::AUTH_PROXY); + cache.Add(origin, realm3_handler, L"realm3-user", L"realm3-password", ""); + + // There is no Realm4 + entry = cache.LookupByRealm(origin, "Realm4"); + EXPECT_TRUE(NULL == entry); + + // While Realm3 does exist, the origin scheme is wrong. + entry = cache.LookupByRealm(GURL("https://www.google.com"), "Realm3"); + EXPECT_TRUE(NULL == entry); + + // Valid lookup by realm. + entry = cache.LookupByRealm(GURL("http://www.google.com:80"), "Realm3"); + EXPECT_FALSE(NULL == entry); + EXPECT_TRUE(entry->handler() == realm3_handler.get()); + EXPECT_EQ(L"realm3-user", entry->username()); + EXPECT_EQ(L"realm3-password", entry->password()); + + // Valid lookup by realm. + entry = cache.LookupByRealm(origin, "Realm2"); + EXPECT_FALSE(NULL == entry); + EXPECT_TRUE(entry->handler() == realm2_handler.get()); + EXPECT_EQ(L"realm2-user", entry->username()); + EXPECT_EQ(L"realm2-password", entry->password()); + + // Check that subpaths are recognized. + HttpAuthCache::Entry* realm2Entry = cache.LookupByRealm(origin, "Realm2"); + EXPECT_FALSE(NULL == realm2Entry); + // Positive tests: + entry = cache.LookupByPath(origin, "/foo2/index.html"); + EXPECT_TRUE(realm2Entry == entry); + entry = cache.LookupByPath(origin, "/foo2/foobar.html"); + EXPECT_TRUE(realm2Entry == entry); + entry = cache.LookupByPath(origin, "/foo2/bar/index.html"); + EXPECT_TRUE(realm2Entry == entry); + entry = cache.LookupByPath(origin, "/foo2/"); + EXPECT_TRUE(realm2Entry == entry); + // Negative tests: + entry = cache.LookupByPath(origin, "/foo2"); + EXPECT_FALSE(realm2Entry == entry); + entry = cache.LookupByPath(origin, "/foo3/index.html"); + EXPECT_FALSE(realm2Entry == entry); + entry = cache.LookupByPath(origin, ""); + EXPECT_FALSE(realm2Entry == entry); + entry = cache.LookupByPath(origin, "/"); + EXPECT_FALSE(realm2Entry == entry); + + // Lookup using empty path (may be used for proxy). + entry = cache.LookupByPath(origin, ""); + EXPECT_FALSE(NULL == entry); + EXPECT_TRUE(entry->handler() == realm3_handler.get()); + EXPECT_EQ("Realm3", entry->realm()); +} + +TEST(HttpAuthCacheTest, AddPath) { + HttpAuthCache::Entry entry; + + // All of these paths have a common root /1/2/2/4/5/ + entry.AddPath("/1/2/3/4/5/x.txt"); + entry.AddPath("/1/2/3/4/5/y.txt"); + entry.AddPath("/1/2/3/4/5/z.txt"); + + EXPECT_EQ(1U, entry.paths_.size()); + EXPECT_EQ("/1/2/3/4/5/", entry.paths_.front()); + + // Add a new entry (not a subpath). + entry.AddPath("/1/XXX/q"); + EXPECT_EQ(2U, entry.paths_.size()); + EXPECT_EQ("/1/XXX/", entry.paths_.front()); + EXPECT_EQ("/1/2/3/4/5/", entry.paths_.back()); + + // Add containing paths of /1/2/3/4/5/ -- should swallow up the deeper paths. + entry.AddPath("/1/2/3/4/x.txt"); + EXPECT_EQ(2U, entry.paths_.size()); + EXPECT_EQ("/1/2/3/4/", entry.paths_.front()); + EXPECT_EQ("/1/XXX/", entry.paths_.back()); + entry.AddPath("/1/2/3/x"); + EXPECT_EQ(2U, entry.paths_.size()); + EXPECT_EQ("/1/2/3/", entry.paths_.front()); + EXPECT_EQ("/1/XXX/", entry.paths_.back()); + + entry.AddPath("/index.html"); + EXPECT_EQ(1U, entry.paths_.size()); + EXPECT_EQ("/", entry.paths_.front()); +} + +// Calling Add when the realm entry already exists, should append that +// path. +TEST(HttpAuthCacheTest, AddToExistingEntry) { + HttpAuthCache cache; + GURL origin("http://www.foobar.com:70"); + + 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"); + + HttpAuthCache::Entry* entry = cache.LookupByRealm(origin, "MyRealm"); + + EXPECT_TRUE(entry == orig_entry); + EXPECT_EQ(L"user3", entry->username()); + EXPECT_EQ(L"password3", entry->password()); + + EXPECT_EQ(2U, entry->paths_.size()); + EXPECT_EQ("/z/", entry->paths_.front()); + EXPECT_EQ("/x/y/z/", entry->paths_.back()); +} + +TEST(HttpAuthCacheTest, Remove) { + GURL origin("http://foobar2.com"); + + scoped_refptr<HttpAuthHandler> realm1_handler = + new MockAuthHandler("basic", "Realm1", HttpAuth::AUTH_SERVER); + + scoped_refptr<HttpAuthHandler> realm2_handler = + new MockAuthHandler("basic", "Realm2", HttpAuth::AUTH_SERVER); + + scoped_refptr<HttpAuthHandler> realm3_handler = + new MockAuthHandler("basic", "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_handler, L"admin", L"password", "/"); + + // Fails, because there is no realm "Realm4". + EXPECT_FALSE(cache.Remove(origin, "Realm4", L"alice", L"123")); + + // Fails because the origin is wrong. + EXPECT_FALSE(cache.Remove( + GURL("http://foobar2.com:100"), "Realm1", L"alice", L"123")); + + // Fails because the username is wrong. + EXPECT_FALSE(cache.Remove(origin, "Realm1", L"alice2", L"123")); + + // Fails because the password is wrong. + EXPECT_FALSE(cache.Remove(origin, "Realm1", L"alice", L"1234")); + + // Succeeds. + EXPECT_TRUE(cache.Remove(origin, "Realm1", L"alice", L"123")); + + // Fails because we just deleted the entry! + EXPECT_FALSE(cache.Remove(origin, "Realm1", L"alice", L"123")); +} + +} // namespace net diff --git a/net/http/http_auth_handler.h b/net/http/http_auth_handler.h index 0608f7b..3e74420 100644 --- a/net/http/http_auth_handler.h +++ b/net/http/http_auth_handler.h @@ -7,6 +7,7 @@ #include <string> +#include "base/ref_counted.h" #include "net/http/http_auth.h" namespace net { @@ -18,15 +19,17 @@ class ProxyInfo; // (basic, digest, ...) // The registry mapping auth-schemes to implementations is hardcoded in // HttpAuth::CreateAuthHandler(). -class HttpAuthHandler { +class HttpAuthHandler : public base::RefCounted<HttpAuthHandler> { public: + virtual ~HttpAuthHandler() { } + // Initialize the handler by parsing a challenge string. bool InitFromChallenge(std::string::const_iterator begin, std::string::const_iterator end, HttpAuth::Target target); // Lowercase name of the auth scheme - virtual std::string scheme() const { + std::string scheme() const { return scheme_; } diff --git a/net/http/http_auth_handler_basic_unittest.cc b/net/http/http_auth_handler_basic_unittest.cc index d78dd3d..a6e4e3d 100644 --- a/net/http/http_auth_handler_basic_unittest.cc +++ b/net/http/http_auth_handler_basic_unittest.cc @@ -25,12 +25,12 @@ TEST(HttpAuthHandlerBasicTest, GenerateCredentials) { }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { std::string challenge = "Basic realm=\"Atlantis\""; - HttpAuthHandlerBasic basic; - basic.InitFromChallenge(challenge.begin(), challenge.end(), - HttpAuth::AUTH_SERVER); - std::string credentials = basic.GenerateCredentials(tests[i].username, - tests[i].password, - NULL, NULL); + scoped_refptr<HttpAuthHandlerBasic> basic = new HttpAuthHandlerBasic; + basic->InitFromChallenge(challenge.begin(), challenge.end(), + HttpAuth::AUTH_SERVER); + std::string credentials = basic->GenerateCredentials(tests[i].username, + tests[i].password, + NULL, NULL); EXPECT_STREQ(tests[i].expected_credentials, credentials.c_str()); } } diff --git a/net/http/http_auth_handler_digest.cc b/net/http/http_auth_handler_digest.cc index 18f166b..be46494 100644 --- a/net/http/http_auth_handler_digest.cc +++ b/net/http/http_auth_handler_digest.cc @@ -89,8 +89,7 @@ std::string HttpAuthHandlerDigest::GenerateCredentials( // This may not be possible when there are multiple connections to the // server though: // https://bugzilla.mozilla.org/show_bug.cgi?id=114451 - // TODO(eroman): leave as 1 for now, and possibly permanently. - int nonce_count = 1; + int nonce_count = nonce_count_++; // Extract the request method and path -- the meaning of 'path' is overloaded // in certain cases, to be a hostname. diff --git a/net/http/http_auth_handler_digest.h b/net/http/http_auth_handler_digest.h index 5289cc5..85d3f3b 100644 --- a/net/http/http_auth_handler_digest.h +++ b/net/http/http_auth_handler_digest.h @@ -23,6 +23,7 @@ class HttpAuthHandlerDigest : public HttpAuthHandler { protected: virtual bool Init(std::string::const_iterator challenge_begin, std::string::const_iterator challenge_end) { + nonce_count_ = 1; return ParseChallenge(challenge_begin, challenge_end); } @@ -98,6 +99,8 @@ class HttpAuthHandlerDigest : public HttpAuthHandler { bool stale_; DigestAlgorithm algorithm_; int qop_; // Bitfield of QualityOfProtection + + int nonce_count_; }; } // namespace net diff --git a/net/http/http_auth_handler_digest_unittest.cc b/net/http/http_auth_handler_digest_unittest.cc index f15ecd0..bb21d8b 100644 --- a/net/http/http_auth_handler_digest_unittest.cc +++ b/net/http/http_auth_handler_digest_unittest.cc @@ -90,17 +90,17 @@ TEST(HttpAuthHandlerDigestTest, ParseChallenge) { for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { std::string challenge(tests[i].challenge); - HttpAuthHandlerDigest auth; - bool ok = auth.ParseChallenge(challenge.begin(), challenge.end()); + scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest; + bool ok = digest->ParseChallenge(challenge.begin(), challenge.end()); EXPECT_EQ(tests[i].parsed_success, ok); - EXPECT_STREQ(tests[i].parsed_realm, auth.realm_.c_str()); - EXPECT_STREQ(tests[i].parsed_nonce, auth.nonce_.c_str()); - EXPECT_STREQ(tests[i].parsed_domain, auth.domain_.c_str()); - EXPECT_STREQ(tests[i].parsed_opaque, auth.opaque_.c_str()); - EXPECT_EQ(tests[i].parsed_stale, auth.stale_); - EXPECT_EQ(tests[i].parsed_algorithm, auth.algorithm_); - EXPECT_EQ(tests[i].parsed_qop, auth.qop_); + EXPECT_STREQ(tests[i].parsed_realm, digest->realm_.c_str()); + EXPECT_STREQ(tests[i].parsed_nonce, digest->nonce_.c_str()); + EXPECT_STREQ(tests[i].parsed_domain, digest->domain_.c_str()); + EXPECT_STREQ(tests[i].parsed_opaque, digest->opaque_.c_str()); + EXPECT_EQ(tests[i].parsed_stale, digest->stale_); + EXPECT_EQ(tests[i].parsed_algorithm, digest->algorithm_); + EXPECT_EQ(tests[i].parsed_qop, digest->qop_); } } @@ -236,12 +236,12 @@ TEST(HttpAuthHandlerDigestTest, AssembleCredentials) { } }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) { - HttpAuthHandlerDigest digest; + scoped_refptr<HttpAuthHandlerDigest> digest = new HttpAuthHandlerDigest; std::string challenge = tests[i].challenge; - EXPECT_TRUE(digest.InitFromChallenge( + EXPECT_TRUE(digest->InitFromChallenge( challenge.begin(), challenge.end(), HttpAuth::AUTH_SERVER)); - std::string creds = digest.AssembleCredentials(tests[i].req_method, + std::string creds = digest->AssembleCredentials(tests[i].req_method, tests[i].req_path, tests[i].username, tests[i].password, tests[i].cnonce, tests[i].nonce_count); diff --git a/net/http/http_auth_unittest.cc b/net/http/http_auth_unittest.cc index d0a2ace..d591501 100644 --- a/net/http/http_auth_unittest.cc +++ b/net/http/http_auth_unittest.cc @@ -4,7 +4,7 @@ #include "testing/gtest/include/gtest/gtest.h" -#include "base/scoped_ptr.h" +#include "base/ref_counted.h" #include "net/http/http_auth.h" #include "net/http/http_auth_handler.h" #include "net/http/http_response_headers.h" @@ -52,10 +52,12 @@ TEST(HttpAuthTest, ChooseBestChallenge) { headers_with_status_line.c_str(), headers_with_status_line.length()))); - scoped_ptr<HttpAuthHandler> handler(HttpAuth::ChooseBestChallenge( - headers.get(), HttpAuth::AUTH_SERVER)); + scoped_refptr<HttpAuthHandler> handler; + HttpAuth::ChooseBestChallenge(headers.get(), + HttpAuth::AUTH_SERVER, + &handler); - if (handler.get()) { + if (handler) { EXPECT_STREQ(tests[i].challenge_realm, handler->realm().c_str()); } else { EXPECT_STREQ("", tests[i].challenge_realm); @@ -157,24 +159,27 @@ TEST(HttpAuthTest, GetAuthorizationHeaderName) { TEST(HttpAuthTest, CreateAuthHandler) { { - scoped_ptr<HttpAuthHandler> handler( - HttpAuth::CreateAuthHandler("Basic realm=\"FooBar\"", - HttpAuth::AUTH_SERVER)); + scoped_refptr<HttpAuthHandler> handler; + HttpAuth::CreateAuthHandler("Basic realm=\"FooBar\"", + HttpAuth::AUTH_SERVER, + &handler); EXPECT_FALSE(handler.get() == NULL); EXPECT_STREQ("basic", handler->scheme().c_str()); EXPECT_STREQ("FooBar", handler->realm().c_str()); EXPECT_EQ(HttpAuth::AUTH_SERVER, handler->target()); } { - scoped_ptr<HttpAuthHandler> handler( - HttpAuth::CreateAuthHandler("UNSUPPORTED realm=\"FooBar\"", - HttpAuth::AUTH_SERVER)); + scoped_refptr<HttpAuthHandler> handler; + HttpAuth::CreateAuthHandler("UNSUPPORTED realm=\"FooBar\"", + HttpAuth::AUTH_SERVER, + &handler); EXPECT_TRUE(handler.get() == NULL); } { - scoped_ptr<HttpAuthHandler> handler(HttpAuth::CreateAuthHandler( - "Digest realm=\"FooBar\", nonce=\"xyz\"", - HttpAuth::AUTH_PROXY)); + scoped_refptr<HttpAuthHandler> handler; + HttpAuth::CreateAuthHandler("Digest realm=\"FooBar\", nonce=\"xyz\"", + HttpAuth::AUTH_PROXY, + &handler); EXPECT_FALSE(handler.get() == NULL); EXPECT_STREQ("digest", handler->scheme().c_str()); EXPECT_STREQ("FooBar", handler->realm().c_str()); diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc index 4c00738..e3aa533 100644 --- a/net/http/http_cache.cc +++ b/net/http/http_cache.cc @@ -983,10 +983,6 @@ HttpCache* HttpCache::GetCache() { return this; } -AuthCache* HttpCache::GetAuthCache() { - return network_layer_->GetAuthCache(); -} - void HttpCache::Suspend(bool suspend) { network_layer_->Suspend(suspend); } diff --git a/net/http/http_cache.h b/net/http/http_cache.h index 60be215..a2683f4 100644 --- a/net/http/http_cache.h +++ b/net/http/http_cache.h @@ -75,7 +75,6 @@ class HttpCache : public HttpTransactionFactory { // HttpTransactionFactory implementation: virtual HttpTransaction* CreateTransaction(); virtual HttpCache* GetCache(); - virtual AuthCache* GetAuthCache(); virtual void Suspend(bool suspend); // Helper function for reading response info from the disk cache. diff --git a/net/http/http_network_layer.cc b/net/http/http_network_layer.cc index 9822998..0cd2fc4 100644 --- a/net/http/http_network_layer.cc +++ b/net/http/http_network_layer.cc @@ -80,10 +80,6 @@ HttpCache* HttpNetworkLayer::GetCache() { return NULL; } -AuthCache* HttpNetworkLayer::GetAuthCache() { - return session_->auth_cache(); -} - void HttpNetworkLayer::Suspend(bool suspend) { suspended_ = suspend; diff --git a/net/http/http_network_layer.h b/net/http/http_network_layer.h index 7597736..93e7eb4 100644 --- a/net/http/http_network_layer.h +++ b/net/http/http_network_layer.h @@ -32,7 +32,6 @@ class HttpNetworkLayer : public HttpTransactionFactory { // HttpTransactionFactory methods: virtual HttpTransaction* CreateTransaction(); virtual HttpCache* GetCache(); - virtual AuthCache* GetAuthCache(); virtual void Suspend(bool suspend); private: diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h index e808554..11ccc26c 100644 --- a/net/http/http_network_session.h +++ b/net/http/http_network_session.h @@ -6,9 +6,9 @@ #define NET_HTTP_HTTP_NETWORK_SESSION_H_ #include "base/ref_counted.h" -#include "net/base/auth_cache.h" #include "net/base/client_socket_pool.h" #include "net/base/ssl_config_service.h" +#include "net/http/http_auth_cache.h" #include "net/proxy/proxy_service.h" namespace net { @@ -27,7 +27,7 @@ class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> { proxy_service_(proxy_resolver) { } - AuthCache* auth_cache() { return &auth_cache_; } + HttpAuthCache* auth_cache() { return &auth_cache_; } ClientSocketPool* connection_pool() { return connection_pool_; } ProxyService* proxy_service() { return &proxy_service_; } #if defined(OS_WIN) @@ -35,7 +35,7 @@ class HttpNetworkSession : public base::RefCounted<HttpNetworkSession> { #endif private: - AuthCache auth_cache_; + HttpAuthCache auth_cache_; scoped_refptr<ClientSocketPool> connection_pool_; scoped_ptr<ProxyResolver> proxy_resolver_; ProxyService proxy_service_; diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 0c87b54..9783025 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -23,11 +23,6 @@ #include "net/http/http_request_info.h" #include "net/http/http_util.h" -// TODO(darin): -// - authentication -// + pre-emptive authorization -// + use the username/password encoded in the URL. - using base::Time; namespace net { @@ -101,16 +96,12 @@ int HttpNetworkTransaction::RestartWithAuth( HttpAuth::AUTH_PROXY : HttpAuth::AUTH_SERVER; // Update the username/password. - auth_data_[target]->state = AUTH_STATE_HAVE_AUTH; - auth_data_[target]->username = username; - auth_data_[target]->password = password; - - next_state_ = STATE_INIT_CONNECTION; - connection_.set_socket(NULL); - connection_.Reset(); + auth_identity_[target].source = HttpAuth::IDENT_SRC_EXTERNAL; + auth_identity_[target].invalid = false; + auth_identity_[target].username = username; + auth_identity_[target].password = password; - // Reset the other member variables. - ResetStateForRestart(); + PrepareForAuthRestart(target); DCHECK(user_callback_ == NULL); int rv = DoLoop(OK); @@ -120,6 +111,26 @@ int HttpNetworkTransaction::RestartWithAuth( return rv; } +void HttpNetworkTransaction::PrepareForAuthRestart(HttpAuth::Target target) { + DCHECK(HaveAuth(target)); + DCHECK(auth_identity_[target].source != HttpAuth::IDENT_SRC_PATH_LOOKUP); + + // Add the auth entry to the cache before restarting. We don't know whether + // the identity is valid yet, but if it is valid we want other transactions + // to know about it. If an entry for (origin, handler->realm()) already + // exists, we update it. + session_->auth_cache()->Add(AuthOrigin(target), auth_handler_[target], + auth_identity_[target].username, auth_identity_[target].password, + AuthPath(target)); + + next_state_ = STATE_INIT_CONNECTION; + connection_.set_socket(NULL); + connection_.Reset(); + + // Reset the other member variables. + ResetStateForRestart(); +} + int HttpNetworkTransaction::Read(char* buf, int buf_len, CompletionCallback* callback) { DCHECK(response_.headers); @@ -840,7 +851,11 @@ int HttpNetworkTransaction::DidReadResponseHeaders() { response_.headers = headers; response_.vary_data.Init(*request_, *response_.headers); - int rv = PopulateAuthChallenge(); + int rv = HandleAuthChallenge(); + if (rv == WILL_RESTART_TRANSACTION) { + DCHECK(next_state_ == STATE_INIT_CONNECTION); + return OK; + } if (rv != OK) return rv; @@ -1028,16 +1043,17 @@ int HttpNetworkTransaction::ReconsiderProxyAfterError(int error) { } void HttpNetworkTransaction::AddAuthorizationHeader(HttpAuth::Target target) { - DCHECK(HaveAuth(target)); - DCHECK(!auth_cache_key_[target].empty()); + // If we have no authentication information, check if we can select + // a cache entry preemptively (based on the path). + if(!HaveAuth(target) && !SelectPreemptiveAuth(target)) + return; - // Add auth data to cache - session_->auth_cache()->Add(auth_cache_key_[target], auth_data_[target]); + DCHECK(HaveAuth(target)); // Add a Authorization/Proxy-Authorization header line. std::string credentials = auth_handler_[target]->GenerateCredentials( - auth_data_[target]->username, - auth_data_[target]->password, + auth_identity_[target].username, + auth_identity_[target].password, request_, &proxy_info_); request_headers_ += HttpAuth::GetAuthorizationHeaderName(target) + @@ -1054,15 +1070,124 @@ void HttpNetworkTransaction::ApplyAuth() { // Don't send origin server auth while establishing tunnel. bool should_apply_server_auth = !establishing_tunnel_; - if (should_apply_proxy_auth && HaveAuth(HttpAuth::AUTH_PROXY)) + if (should_apply_proxy_auth) AddAuthorizationHeader(HttpAuth::AUTH_PROXY); - if (should_apply_server_auth && HaveAuth(HttpAuth::AUTH_SERVER)) + if (should_apply_server_auth) AddAuthorizationHeader(HttpAuth::AUTH_SERVER); } -// Populates response_.auth_challenge with the authentication challenge info. -// This info is consumed by URLRequestHttpJob::GetAuthChallengeInfo(). -int HttpNetworkTransaction::PopulateAuthChallenge() { +GURL HttpNetworkTransaction::AuthOrigin(HttpAuth::Target target) const { + return target == HttpAuth::AUTH_PROXY ? + GURL("http://" + proxy_info_.proxy_server()) : + request_->url.GetOrigin(); +} + +std::string HttpNetworkTransaction::AuthPath(HttpAuth::Target target) + const { + // Proxy authentication realms apply to all paths. So we will use + // empty string in place of an absolute path. + return target == HttpAuth::AUTH_PROXY ? + std::string() : request_->url.path(); +} + +void HttpNetworkTransaction::InvalidateRejectedAuthFromCache( + HttpAuth::Target target) { + DCHECK(HaveAuth(target)); + + // 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 (auth_identity_[target].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. + session_->auth_cache()->Remove(AuthOrigin(target), + auth_handler_[target]->realm(), + auth_identity_[target].username, + auth_identity_[target].password); +} + +bool HttpNetworkTransaction::SelectPreemptiveAuth(HttpAuth::Target target) { + DCHECK(!HaveAuth(target)); + + // Don't do preemptive authorization if the URL contains a username/password, + // since we must first be challenged in order to use the URL's identity. + if (request_->url.has_username()) + return false; + + // SelectPreemptiveAuth() is on the critical path for each request, so it + // 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) { + 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; + } + return false; +} + +bool HttpNetworkTransaction::SelectNextAuthIdentityToTry( + HttpAuth::Target target) { + DCHECK(auth_handler_[target]); + DCHECK(auth_identity_[target].invalid); + + // Try to use the username/password encoded into the URL first. + // (By checking source == IDENT_SRC_NONE, we make sure that this + // is only done once for the transaction.) + if (target == HttpAuth::AUTH_SERVER && request_->url.has_username() && + auth_identity_[target].source == HttpAuth::IDENT_SRC_NONE) { + auth_identity_[target].source = HttpAuth::IDENT_SRC_URL; + auth_identity_[target].invalid = false; + auth_identity_[target].username = UTF8ToWide(request_->url.username()); + auth_identity_[target].password = UTF8ToWide(request_->url.password()); + // TODO(eroman): If the password is blank, should we also try combining + // with a password from the cache? + return true; + } + + // Check the auth cache for a realm entry. + HttpAuthCache::Entry* entry = session_->auth_cache()->LookupByRealm( + AuthOrigin(target), auth_handler_[target]->realm()); + + if (entry) { + // Disallow re-using of identity if the scheme of the originating challenge + // does not match. This protects against the following situation: + // 1. Browser prompts user to sign into DIGEST realm="Foo". + // 2. Since the auth-scheme is not BASIC, the user is reasured that it + // will not be sent over the wire in clear text. So they use their + // most trusted password. + // 3. Next, the browser receives a challenge for BASIC realm="Foo". This + // is the same realm that we have a cached identity for. However if + // we use that identity, it would get sent over the wire in + // clear text (which isn't what the user agreed to when entering it). + if (entry->handler()->scheme() != auth_handler_[target]->scheme()) { + LOG(WARNING) << "The scheme of realm " << auth_handler_[target]->realm() + << " has changed from " << entry->handler()->scheme() + << " to " << auth_handler_[target]->scheme(); + return false; + } + + auth_identity_[target].source = HttpAuth::IDENT_SRC_REALM_LOOKUP; + auth_identity_[target].invalid = false; + auth_identity_[target].username = entry->username(); + auth_identity_[target].password = entry->password(); + return true; + } + return false; +} + +int HttpNetworkTransaction::HandleAuthChallenge() { DCHECK(response_.headers); int status = response_.headers->response_code(); @@ -1074,46 +1199,57 @@ int HttpNetworkTransaction::PopulateAuthChallenge() { if (target == HttpAuth::AUTH_PROXY && proxy_info_.is_direct()) return ERR_UNEXPECTED_PROXY_AUTH; + // The auth we tried just failed, hence it can't be valid. Remove it from + // the cache so it won't be used again. + if (HaveAuth(target)) + InvalidateRejectedAuthFromCache(target); + + auth_identity_[target].invalid = true; + // Find the best authentication challenge that we support. - scoped_ptr<HttpAuthHandler> auth_handler( - HttpAuth::ChooseBestChallenge(response_.headers.get(), target)); + HttpAuth::ChooseBestChallenge(response_.headers.get(), + target, + &auth_handler_[target]); // We found no supported challenge -- let the transaction continue // so we end up displaying the error page. - if (!auth_handler.get()) + if (!auth_handler_[target]) return OK; - // Construct an AuthChallengeInfo. - scoped_refptr<AuthChallengeInfo> auth_info = new AuthChallengeInfo; + // Pick a new auth identity to try, by looking to the URL and auth cache. + // If an identity to try is found, it is saved to auth_identity_[target]. + bool has_identity_to_try = SelectNextAuthIdentityToTry(target); + DCHECK(has_identity_to_try == !auth_identity_[target].invalid); + + if (has_identity_to_try) { + DCHECK(user_callback_); + PrepareForAuthRestart(target); + return WILL_RESTART_TRANSACTION; + } else { + // We have exhausted all identity possibilities, all we can do now is + // pass the challenge information back to the client. + PopulateAuthChallenge(target); + } + + return OK; +} + +void HttpNetworkTransaction::PopulateAuthChallenge(HttpAuth::Target target) { + // Populates response_.auth_challenge with the authentication challenge info. + // This info is consumed by URLRequestHttpJob::GetAuthChallengeInfo(). + + AuthChallengeInfo* auth_info = new AuthChallengeInfo; auth_info->is_proxy = target == HttpAuth::AUTH_PROXY; - auth_info->scheme = ASCIIToWide(auth_handler->scheme()); + auth_info->scheme = ASCIIToWide(auth_handler_[target]->scheme()); // TODO(eroman): decode realm according to RFC 2047. - auth_info->realm = ASCIIToWide(auth_handler->realm()); + auth_info->realm = ASCIIToWide(auth_handler_[target]->realm()); if (target == HttpAuth::AUTH_PROXY) { auth_info->host = ASCIIToWide(proxy_info_.proxy_server()); } else { DCHECK(target == HttpAuth::AUTH_SERVER); auth_info->host = ASCIIToWide(request_->url.host()); } - - // Update the auth cache key and remove any data in the auth cache. - if (!auth_data_[target]) - auth_data_[target] = new AuthData; - auth_cache_key_[target] = AuthCache::HttpKey(request_->url, *auth_info); - DCHECK(!auth_cache_key_[target].empty()); - auth_data_[target]->scheme = auth_info->scheme; - if (auth_data_[target]->state == AUTH_STATE_HAVE_AUTH) { - // The cached identity probably isn't valid so remove it. - // The assumption here is that the cached auth data is what we - // just used. - session_->auth_cache()->Remove(auth_cache_key_[target]); - auth_data_[target]->state = AUTH_STATE_NEED_AUTH; - } - - response_.auth_challenge.swap(auth_info); - auth_handler_[target].reset(auth_handler.release()); - - return OK; + response_.auth_challenge = auth_info; } } // namespace net diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h index 2f733389..9ac684f 100644 --- a/net/http/http_network_transaction.h +++ b/net/http/http_network_transaction.h @@ -124,7 +124,10 @@ class HttpNetworkTransaction : public HttpTransaction { return header_buf_http_offset_ != -1; } - // Resets the members of the transaction, to rewinding next_state_. + // Sets up the state machine to restart the transaction with auth. + void PrepareForAuthRestart(HttpAuth::Target target); + + // Resets the members of the transaction so it can be restarted. void ResetStateForRestart(); // Attach any credentials needed for the proxy server or origin server. @@ -134,21 +137,47 @@ class HttpNetworkTransaction : public HttpTransaction { // origin server auth header, as specified by |target| void AddAuthorizationHeader(HttpAuth::Target target); - // Handles HTTP status code 401 or 407. Populates response_.auth_challenge - // with the required information so that URLRequestHttpJob can prompt - // for a username/password. - int PopulateAuthChallenge(); + // Handles HTTP status code 401 or 407. + // HandleAuthChallenge() returns a network error code, or OK, or + // WILL_RESTART_TRANSACTION. The latter indicates that the state machine has + // been updated to restart the transaction with a new auth attempt. + enum { WILL_RESTART_TRANSACTION = 1 }; + int HandleAuthChallenge(); + + // Populates response_.auth_challenge with the challenge information, so that + // URLRequestHttpJob can prompt for a username/password. + void PopulateAuthChallenge(HttpAuth::Target target); + + // Invalidates any auth cache entries after authentication has failed. + // The identity that was rejected is auth_identity_[target]. + void InvalidateRejectedAuthFromCache(HttpAuth::Target target); + + // Sets auth_identity_[target] to the next identity that the transaction + // should try. It chooses candidates by searching the auth cache + // and the URL for a username:password. Returns true if an identity + // was found. + bool SelectNextAuthIdentityToTry(HttpAuth::Target target); + + // Searches the auth cache for an entry that encompasses the request's path. + // If such an entry is found, updates auth_identity_[target] and + // auth_handler_[target] with the cache entry's data and returns true. + bool SelectPreemptiveAuth(HttpAuth::Target target); bool NeedAuth(HttpAuth::Target target) const { - return auth_data_[target] && - auth_data_[target]->state == AUTH_STATE_NEED_AUTH; + return auth_handler_[target].get() && auth_identity_[target].invalid; } bool HaveAuth(HttpAuth::Target target) const { - return auth_data_[target] && - auth_data_[target]->state == AUTH_STATE_HAVE_AUTH; + return auth_handler_[target].get() && !auth_identity_[target].invalid; } + // Get the {scheme, host, port} for the authentication target + GURL AuthOrigin(HttpAuth::Target target) const; + + // Get the absolute path of the resource needing authentication. + // For proxy authentication the path is always empty string. + std::string AuthPath(HttpAuth::Target target) const; + // The following three auth members are arrays of size two -- index 0 is // for the proxy server, and index 1 is for the origin server. // Use the enum HttpAuth::Target to index into them. @@ -156,16 +185,12 @@ class HttpNetworkTransaction : public HttpTransaction { // auth_handler encapsulates the logic for the particular auth-scheme. // This includes the challenge's parameters. If NULL, then there is no // associated auth handler. - scoped_ptr<HttpAuthHandler> auth_handler_[2]; - - // auth_data tracks the identity (username/password) that is to be - // applied to the proxy/origin server. The identify may have come - // from a login prompt, or from the auth cache. It is fed to us - // by URLRequestHttpJob, via RestartWithAuth(). - scoped_refptr<AuthData> auth_data_[2]; + scoped_refptr<HttpAuthHandler> auth_handler_[2]; - // The key in the auth cache, for auth_data_. - std::string auth_cache_key_[2]; + // auth_identity_ holds the (username/password) that should be used by + // the auth_handler_ to generate credentials. This identity can come from + // a number of places (url, cache, prompt). + HttpAuth::Identity auth_identity_[2]; CompletionCallbackImpl<HttpNetworkTransaction> io_callback_; CompletionCallback* user_callback_; diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index dc53f4f..507b90b 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -618,6 +618,12 @@ TEST_F(HttpNetworkTransactionTest, BasicAuth) { request.url = GURL("http://www.google.com/"); request.load_flags = 0; + MockWrite data_writes1[] = { + MockWrite("GET / HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + MockRead data_reads1[] = { MockRead("HTTP/1.0 401 Unauthorized\r\n"), // Give a couple authenticate options (only the middle one is actually @@ -650,6 +656,7 @@ TEST_F(HttpNetworkTransactionTest, BasicAuth) { MockSocket data1; data1.reads = data_reads1; + data1.writes = data_writes1; MockSocket data2; data2.reads = data_reads2; data2.writes = data_writes2; @@ -707,6 +714,12 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyThenServer) { request.url = GURL("http://www.google.com/"); request.load_flags = 0; + MockWrite data_writes1[] = { + MockWrite("GET http://www.google.com/ HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n\r\n"), + }; + MockRead data_reads1[] = { MockRead("HTTP/1.0 407 Unauthorized\r\n"), // Give a couple authenticate options (only the middle one is actually @@ -762,6 +775,7 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyThenServer) { MockSocket data1; data1.reads = data_reads1; + data1.writes = data_writes1; MockSocket data2; data2.reads = data_reads2; data2.writes = data_writes2; @@ -1017,3 +1031,477 @@ TEST_F(HttpNetworkTransactionTest, ResendRequestOnWriteBodyError) { EXPECT_EQ(kExpectedResponseData[i], response_data); } } + +// Test the request-challenge-retry sequence for basic auth when there is +// an identity in the URL. The request should be sent as normal, but when +// it fails the identity from the URL is used to answer the challenge. +TEST_F(HttpNetworkTransactionTest, AuthIdentityInUrl) { + scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction( + CreateSession(), &mock_socket_factory)); + + net::HttpRequestInfo request; + request.method = "GET"; + // Note: the URL has a username:password in it. + request.url = GURL("http://foo:bar@www.google.com/"); + request.load_flags = 0; + + MockWrite data_writes1[] = { + MockWrite("GET / HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + + MockRead data_reads1[] = { + MockRead("HTTP/1.0 401 Unauthorized\r\n"), + MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"), + MockRead("Content-Length: 10\r\n\r\n"), + MockRead(false, net::ERR_FAILED), + }; + + // After the challenge above, the transaction will be restarted using the + // identity from the url (foo, bar) to answer the challenge. + MockWrite data_writes2[] = { + MockWrite("GET / HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + }; + + MockRead data_reads2[] = { + MockRead("HTTP/1.0 200 OK\r\n"), + MockRead("Content-Length: 100\r\n\r\n"), + MockRead(false, net::OK), + }; + + MockSocket data1; + data1.reads = data_reads1; + data1.writes = data_writes1; + MockSocket data2; + data2.reads = data_reads2; + data2.writes = data_writes2; + mock_sockets[0] = &data1; + mock_sockets[1] = &data2; + mock_sockets[2] = NULL; + + TestCompletionCallback callback1; + + int rv = trans->Start(&request, &callback1); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback1.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + const net::HttpResponseInfo* response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + + // There is no challenge info, since the identity in URL worked. + EXPECT_TRUE(response->auth_challenge.get() == NULL); + + EXPECT_EQ(100, response->headers->GetContentLength()); + + // Empty the current queue. + MessageLoop::current()->RunAllPending(); +} + +// Test that previously tried username/passwords for a realm get re-used. +TEST_F(HttpNetworkTransactionTest, BasicAuthCacheAndPreauth) { + scoped_refptr<net::HttpNetworkSession> session = CreateSession(); + + // Transaction 1: authenticate (foo, bar) on MyRealm1 + { + scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction( + session, &mock_socket_factory)); + + net::HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("http://www.google.com/x/y/z"); + request.load_flags = 0; + + MockWrite data_writes1[] = { + MockWrite("GET /x/y/z HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + + MockRead data_reads1[] = { + MockRead("HTTP/1.0 401 Unauthorized\r\n"), + MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"), + MockRead("Content-Length: 10000\r\n\r\n"), + MockRead(false, net::ERR_FAILED), + }; + + // Resend with authorization (username=foo, password=bar) + MockWrite data_writes2[] = { + MockWrite("GET /x/y/z HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + }; + + // Sever accepts the authorization. + MockRead data_reads2[] = { + MockRead("HTTP/1.0 200 OK\r\n"), + MockRead("Content-Length: 100\r\n\r\n"), + MockRead(false, net::OK), + }; + + MockSocket data1; + data1.reads = data_reads1; + data1.writes = data_writes1; + MockSocket data2; + data2.reads = data_reads2; + data2.writes = data_writes2; + mock_sockets_index = 0; + mock_sockets[0] = &data1; + mock_sockets[1] = &data2; + mock_sockets[2] = NULL; + + TestCompletionCallback callback1; + + int rv = trans->Start(&request, &callback1); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback1.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + const net::HttpResponseInfo* response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + + // The password prompt info should have been set in + // response->auth_challenge. + EXPECT_FALSE(response->auth_challenge.get() == NULL); + + // TODO(eroman): this should really include the effective port (80) + EXPECT_EQ(L"www.google.com", response->auth_challenge->host); + EXPECT_EQ(L"MyRealm1", response->auth_challenge->realm); + EXPECT_EQ(L"basic", response->auth_challenge->scheme); + + TestCompletionCallback callback2; + + rv = trans->RestartWithAuth(L"foo", L"bar", &callback2); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback2.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + EXPECT_TRUE(response->auth_challenge.get() == NULL); + EXPECT_EQ(100, response->headers->GetContentLength()); + } + + // ------------------------------------------------------------------------ + + // Transaction 2: authenticate (foo2, bar2) on MyRealm2 + { + scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction( + session, &mock_socket_factory)); + + net::HttpRequestInfo request; + request.method = "GET"; + // Note that Transaction 1 was at /x/y/z, so this is in the same + // protection space as MyRealm1. + request.url = GURL("http://www.google.com/x/y/a/b"); + request.load_flags = 0; + + MockWrite data_writes1[] = { + MockWrite("GET /x/y/a/b HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + // Send preemptive authorization for MyRealm1 + "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + }; + + // The server didn't like the preemptive authorization, and + // challenges us for a different realm (MyRealm2). + MockRead data_reads1[] = { + MockRead("HTTP/1.0 401 Unauthorized\r\n"), + MockRead("WWW-Authenticate: Basic realm=\"MyRealm2\"\r\n"), + MockRead("Content-Length: 10000\r\n\r\n"), + MockRead(false, net::ERR_FAILED), + }; + + // Resend with authorization for MyRealm2 (username=foo2, password=bar2) + MockWrite data_writes2[] = { + MockWrite("GET /x/y/a/b HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + "Authorization: Basic Zm9vMjpiYXIy\r\n\r\n"), + }; + + // Sever accepts the authorization. + MockRead data_reads2[] = { + MockRead("HTTP/1.0 200 OK\r\n"), + MockRead("Content-Length: 100\r\n\r\n"), + MockRead(false, net::OK), + }; + + MockSocket data1; + data1.reads = data_reads1; + data1.writes = data_writes1; + MockSocket data2; + data2.reads = data_reads2; + data2.writes = data_writes2; + mock_sockets_index = 0; + mock_sockets[0] = &data1; + mock_sockets[1] = &data2; + mock_sockets[2] = NULL; + + TestCompletionCallback callback1; + + int rv = trans->Start(&request, &callback1); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback1.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + const net::HttpResponseInfo* response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + + // The password prompt info should have been set in + // response->auth_challenge. + EXPECT_FALSE(response->auth_challenge.get() == NULL); + + // TODO(eroman): this should really include the effective port (80) + EXPECT_EQ(L"www.google.com", response->auth_challenge->host); + EXPECT_EQ(L"MyRealm2", response->auth_challenge->realm); + EXPECT_EQ(L"basic", response->auth_challenge->scheme); + + TestCompletionCallback callback2; + + rv = trans->RestartWithAuth(L"foo2", L"bar2", &callback2); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback2.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + EXPECT_TRUE(response->auth_challenge.get() == NULL); + EXPECT_EQ(100, response->headers->GetContentLength()); + } + + // ------------------------------------------------------------------------ + + // Transaction 3: Resend a request in MyRealm's protection space -- + // succeed with preemptive authorization. + { + scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction( + session, &mock_socket_factory)); + + net::HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("http://www.google.com/x/y/z2"); + request.load_flags = 0; + + MockWrite data_writes1[] = { + MockWrite("GET /x/y/z2 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + // The authorization for MyRealm1 gets sent preemptively + // (since the url is in the same protection space) + "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + }; + + // Sever accepts the preemptive authorization + MockRead data_reads1[] = { + MockRead("HTTP/1.0 200 OK\r\n"), + MockRead("Content-Length: 100\r\n\r\n"), + MockRead(false, net::OK), + }; + + MockSocket data1; + data1.reads = data_reads1; + data1.writes = data_writes1; + mock_sockets_index = 0; + mock_sockets[0] = &data1; + mock_sockets[1] = NULL; + + TestCompletionCallback callback1; + + int rv = trans->Start(&request, &callback1); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback1.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + const net::HttpResponseInfo* response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + + EXPECT_TRUE(response->auth_challenge.get() == NULL); + EXPECT_EQ(100, response->headers->GetContentLength()); + } + + // ------------------------------------------------------------------------ + + // Transaction 4: request another URL in MyRealm (however the + // url is not known to belong to the protection space, so no pre-auth). + { + scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction( + session, &mock_socket_factory)); + + net::HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("http://www.google.com/x/1"); + request.load_flags = 0; + + MockWrite data_writes1[] = { + MockWrite("GET /x/1 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + + MockRead data_reads1[] = { + MockRead("HTTP/1.0 401 Unauthorized\r\n"), + MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"), + MockRead("Content-Length: 10000\r\n\r\n"), + MockRead(false, net::ERR_FAILED), + }; + + // Resend with authorization from MyRealm's cache. + MockWrite data_writes2[] = { + MockWrite("GET /x/1 HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + }; + + // Sever accepts the authorization. + MockRead data_reads2[] = { + MockRead("HTTP/1.0 200 OK\r\n"), + MockRead("Content-Length: 100\r\n\r\n"), + MockRead(false, net::OK), + }; + + MockSocket data1; + data1.reads = data_reads1; + data1.writes = data_writes1; + MockSocket data2; + data2.reads = data_reads2; + data2.writes = data_writes2; + mock_sockets_index = 0; + mock_sockets[0] = &data1; + mock_sockets[1] = &data2; + mock_sockets[2] = NULL; + + TestCompletionCallback callback1; + + int rv = trans->Start(&request, &callback1); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback1.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + const net::HttpResponseInfo* response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + EXPECT_TRUE(response->auth_challenge.get() == NULL); + EXPECT_EQ(100, response->headers->GetContentLength()); + } + + // ------------------------------------------------------------------------ + + // Transaction 5: request a URL in MyRealm, but the server rejects the + // cached identity. Should invalidate and re-prompt. + { + scoped_ptr<net::HttpTransaction> trans(new net::HttpNetworkTransaction( + session, &mock_socket_factory)); + + net::HttpRequestInfo request; + request.method = "GET"; + request.url = GURL("http://www.google.com/p/q/t"); + request.load_flags = 0; + + MockWrite data_writes1[] = { + MockWrite("GET /p/q/t HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + + MockRead data_reads1[] = { + MockRead("HTTP/1.0 401 Unauthorized\r\n"), + MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"), + MockRead("Content-Length: 10000\r\n\r\n"), + MockRead(false, net::ERR_FAILED), + }; + + // Resend with authorization from cache for MyRealm. + MockWrite data_writes2[] = { + MockWrite("GET /p/q/t HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + "Authorization: Basic Zm9vOmJhcg==\r\n\r\n"), + }; + + // Sever rejects the authorization. + MockRead data_reads2[] = { + MockRead("HTTP/1.0 401 Unauthorized\r\n"), + MockRead("WWW-Authenticate: Basic realm=\"MyRealm1\"\r\n"), + MockRead("Content-Length: 10000\r\n\r\n"), + MockRead(false, net::ERR_FAILED), + }; + + // At this point we should prompt for new credentials for MyRealm. + // Restart with username=foo3, password=foo4. + MockWrite data_writes3[] = { + MockWrite("GET /p/q/t HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n" + "Authorization: Basic Zm9vMzpiYXIz\r\n\r\n"), + }; + + // Sever accepts the authorization. + MockRead data_reads3[] = { + MockRead("HTTP/1.0 200 OK\r\n"), + MockRead("Content-Length: 100\r\n\r\n"), + MockRead(false, net::OK), + }; + + MockSocket data1; + data1.reads = data_reads1; + data1.writes = data_writes1; + MockSocket data2; + data2.reads = data_reads2; + data2.writes = data_writes2; + MockSocket data3; + data3.reads = data_reads3; + data3.writes = data_writes3; + mock_sockets_index = 0; + mock_sockets[0] = &data1; + mock_sockets[1] = &data2; + mock_sockets[2] = &data3; + mock_sockets[3] = NULL; + + TestCompletionCallback callback1; + + int rv = trans->Start(&request, &callback1); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback1.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + const net::HttpResponseInfo* response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + + // The password prompt info should have been set in + // response->auth_challenge. + EXPECT_FALSE(response->auth_challenge.get() == NULL); + + // TODO(eroman): this should really include the effective port (80) + EXPECT_EQ(L"www.google.com", response->auth_challenge->host); + EXPECT_EQ(L"MyRealm1", response->auth_challenge->realm); + EXPECT_EQ(L"basic", response->auth_challenge->scheme); + + TestCompletionCallback callback2; + + rv = trans->RestartWithAuth(L"foo3", L"bar3", &callback2); + EXPECT_EQ(net::ERR_IO_PENDING, rv); + + rv = callback2.WaitForResult(); + EXPECT_EQ(net::OK, rv); + + response = trans->GetResponseInfo(); + EXPECT_FALSE(response == NULL); + EXPECT_TRUE(response->auth_challenge.get() == NULL); + EXPECT_EQ(100, response->headers->GetContentLength()); + } +} diff --git a/net/http/http_transaction_factory.h b/net/http/http_transaction_factory.h index b7f67c4..e363c21 100644 --- a/net/http/http_transaction_factory.h +++ b/net/http/http_transaction_factory.h @@ -7,7 +7,6 @@ namespace net { -class AuthCache; class HttpCache; class HttpTransaction; @@ -22,9 +21,6 @@ class HttpTransactionFactory { // Returns the associated cache if any (may be NULL). virtual HttpCache* GetCache() = 0; - // Returns the associated HTTP auth cache if any (may be NULL). - virtual AuthCache* GetAuthCache() = 0; - // Suspends the creation of new transactions. If |suspend| is false, creation // of new transactions is resumed. virtual void Suspend(bool suspend) = 0; diff --git a/net/http/http_transaction_unittest.h b/net/http/http_transaction_unittest.h index 93f76c7..eb2d7bb 100644 --- a/net/http/http_transaction_unittest.h +++ b/net/http/http_transaction_unittest.h @@ -292,10 +292,6 @@ class MockNetworkLayer : public net::HttpTransactionFactory { return NULL; } - virtual net::AuthCache* GetAuthCache() { - return NULL; - } - virtual void Suspend(bool suspend) {} int transaction_count() const { return transaction_count_; } diff --git a/net/http/http_transaction_winhttp.cc b/net/http/http_transaction_winhttp.cc index bb5f610..4429511 100644 --- a/net/http/http_transaction_winhttp.cc +++ b/net/http/http_transaction_winhttp.cc @@ -755,10 +755,6 @@ HttpCache* HttpTransactionWinHttp::Factory::GetCache() { return NULL; } -AuthCache* HttpTransactionWinHttp::Factory::GetAuthCache() { - return session_->auth_cache(); -} - void HttpTransactionWinHttp::Factory::Suspend(bool suspend) { is_suspended_ = suspend; @@ -1490,7 +1486,9 @@ int HttpTransactionWinHttp::DidReceiveHeaders() { return ERR_FILE_TOO_BIG; response_.vary_data.Init(*request_, *response_.headers); - PopulateAuthChallenge(); + int rv = PopulateAuthChallenge(); + if (rv != OK) + return rv; // Unfortunately, WinHttp does not close the connection when a non-keepalive // response is _not_ followed by the server closing the connection. So, we @@ -1502,12 +1500,12 @@ int HttpTransactionWinHttp::DidReceiveHeaders() { } // Populates response_.auth_challenge with the authentication challenge info. -void HttpTransactionWinHttp::PopulateAuthChallenge() { +int HttpTransactionWinHttp::PopulateAuthChallenge() { DCHECK(response_.headers); int status = response_.headers->response_code(); if (status != 401 && status != 407) - return; + return OK; scoped_refptr<AuthChallengeInfo> auth_info = new AuthChallengeInfo; @@ -1529,7 +1527,7 @@ void HttpTransactionWinHttp::PopulateAuthChallenge() { std::string header_name = auth_info->is_proxy ? "Proxy-Authenticate" : "WWW-Authenticate"; if (!response_.headers->EnumerateHeader(NULL, header_name, &header_value)) - return; + return OK; // TODO(darin): Need to support RFC 2047 encoded realm strings. For now, we // limit our support to ASCII and "native code page" realm strings. @@ -1564,9 +1562,19 @@ void HttpTransactionWinHttp::PopulateAuthChallenge() { if (auth->state == AUTH_STATE_HAVE_AUTH) { session_->auth_cache()->Remove(*auth_cache_key); auth->state = AUTH_STATE_NEED_AUTH; + } else { + AuthData* cached_auth = session_->auth_cache()->Lookup(*auth_cache_key); + if (cached_auth) { + CompletionCallback* callback = callback_; + callback_ = NULL; + return RestartWithAuth(cached_auth->username, + cached_auth->password, + callback); + } } response_.auth_challenge.swap(auth_info); + return OK; } static DWORD StringToAuthScheme(const std::wstring& scheme) { diff --git a/net/http/http_transaction_winhttp.h b/net/http/http_transaction_winhttp.h index 122aea3..430a288 100644 --- a/net/http/http_transaction_winhttp.h +++ b/net/http/http_transaction_winhttp.h @@ -40,7 +40,6 @@ class HttpTransactionWinHttp : public HttpTransaction { virtual HttpTransaction* CreateTransaction(); virtual HttpCache* GetCache(); - virtual AuthCache* GetAuthCache(); virtual void Suspend(bool suspend); private: @@ -95,7 +94,7 @@ class HttpTransactionWinHttp : public HttpTransaction { int DidReadData(DWORD num_bytes); int DidReceiveHeaders(); - void PopulateAuthChallenge(); + int PopulateAuthChallenge(); void ApplyAuth(); std::string GetRequestHeaders() const; diff --git a/net/net.xcodeproj/project.pbxproj b/net/net.xcodeproj/project.pbxproj index 068fc37..f0669a0 100644 --- a/net/net.xcodeproj/project.pbxproj +++ b/net/net.xcodeproj/project.pbxproj @@ -34,6 +34,7 @@ /* End PBXAggregateTarget section */ /* Begin PBXBuildFile section */ + 042A4D480EC4F4500083281F /* http_auth_cache_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 042A4D470EC4F4500083281F /* http_auth_cache_unittest.cc */; }; 0435A4660E8DD69C00E4DF08 /* http_auth.cc in Sources */ = {isa = PBXBuildFile; fileRef = 0435A4650E8DD69C00E4DF08 /* http_auth.cc */; }; 0435A47A0E8DD6F300E4DF08 /* http_auth_handler.cc in Sources */ = {isa = PBXBuildFile; fileRef = 0435A4790E8DD6F300E4DF08 /* http_auth_handler.cc */; }; 0435A4800E8DD73600E4DF08 /* http_auth_handler_basic.cc in Sources */ = {isa = PBXBuildFile; fileRef = 0435A47F0E8DD73600E4DF08 /* http_auth_handler_basic.cc */; }; @@ -45,6 +46,7 @@ 04C626D60E8DE39E0067E92A /* http_auth_handler_digest_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 04C626D50E8DE39E0067E92A /* http_auth_handler_digest_unittest.cc */; }; 04C626D80E8DE3AA0067E92A /* http_auth_handler_basic_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 04C626D70E8DE3AA0067E92A /* http_auth_handler_basic_unittest.cc */; }; 04C626DA0E8DE3BA0067E92A /* http_auth_unittest.cc in Sources */ = {isa = PBXBuildFile; fileRef = 04C626D90E8DE3BA0067E92A /* http_auth_unittest.cc */; }; + 04E7BD550EC4ECF60078FE58 /* http_auth_cache.cc in Sources */ = {isa = PBXBuildFile; fileRef = 04E7BD540EC4ECF60078FE58 /* http_auth_cache.cc */; }; 4DB04D3F0EB24EDF00A5633C /* dns_resolution_observer.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7BED32590E5A181C00A747DB /* dns_resolution_observer.cc */; }; 533102E70E5E3EBF00FF8E32 /* net_util_posix.cc in Sources */ = {isa = PBXBuildFile; fileRef = 533102E60E5E3EBF00FF8E32 /* net_util_posix.cc */; }; 7B2630680E82F2A1001CE27F /* libevent.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 7B2630650E82F282001CE27F /* libevent.a */; }; @@ -405,6 +407,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 042A4D470EC4F4500083281F /* http_auth_cache_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_auth_cache_unittest.cc; sourceTree = "<group>"; }; 0435A4650E8DD69C00E4DF08 /* http_auth.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_auth.cc; sourceTree = "<group>"; }; 0435A4670E8DD6B300E4DF08 /* http_auth.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = http_auth.h; sourceTree = "<group>"; }; 0435A4790E8DD6F300E4DF08 /* http_auth_handler.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_auth_handler.cc; sourceTree = "<group>"; }; @@ -417,6 +420,8 @@ 04C626D50E8DE39E0067E92A /* http_auth_handler_digest_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_auth_handler_digest_unittest.cc; sourceTree = "<group>"; }; 04C626D70E8DE3AA0067E92A /* http_auth_handler_basic_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_auth_handler_basic_unittest.cc; sourceTree = "<group>"; }; 04C626D90E8DE3BA0067E92A /* http_auth_unittest.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_auth_unittest.cc; sourceTree = "<group>"; }; + 04E7BD540EC4ECF60078FE58 /* http_auth_cache.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = http_auth_cache.cc; sourceTree = "<group>"; }; + 04E7BD560EC4ED020078FE58 /* http_auth_cache.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = http_auth_cache.h; sourceTree = "<group>"; }; 533102E60E5E3EBF00FF8E32 /* net_util_posix.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = net_util_posix.cc; sourceTree = "<group>"; }; 7B2630600E82F282001CE27F /* libevent.xcodeproj */ = {isa = PBXFileReference; lastKnownFileType = "wrapper.pb-project"; name = libevent.xcodeproj; path = third_party/libevent/libevent.xcodeproj; sourceTree = "<group>"; }; 7B466C460E5E732900C91F63 /* platform_test_mac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = platform_test_mac.mm; sourceTree = "<group>"; }; @@ -1050,6 +1055,9 @@ 7BED33600E5A194700A747DB /* http_atom_list.h */, 0435A4650E8DD69C00E4DF08 /* http_auth.cc */, 0435A4670E8DD6B300E4DF08 /* http_auth.h */, + 04E7BD540EC4ECF60078FE58 /* http_auth_cache.cc */, + 04E7BD560EC4ED020078FE58 /* http_auth_cache.h */, + 042A4D470EC4F4500083281F /* http_auth_cache_unittest.cc */, 04C626D90E8DE3BA0067E92A /* http_auth_unittest.cc */, 0435A4790E8DD6F300E4DF08 /* http_auth_handler.cc */, 0435A47B0E8DD6FA00E4DF08 /* http_auth_handler.h */, @@ -1517,6 +1525,7 @@ 821F20A50E5CD414003C7E38 /* url_request_view_cache_job.cc in Sources */, 82113BBD0E892E5800E3848F /* x509_certificate.cc in Sources */, 827E139D0E81611D00183614 /* x509_certificate_mac.cc in Sources */, + 04E7BD550EC4ECF60078FE58 /* http_auth_cache.cc in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1564,6 +1573,7 @@ 7BA361450E8C341F0023C8B9 /* test_completion_callback_unittest.cc in Sources */, 82113A1D0E8434EE00E3848F /* x509_certificate_unittest.cc in Sources */, A5AB7BFC0EB7DBA10070A7D3 /* file_stream_unittest.cc in Sources */, + 042A4D480EC4F4500083281F /* http_auth_cache_unittest.cc in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/net/net_lib.scons b/net/net_lib.scons index 86ae6b0..82c1af2 100644 --- a/net/net_lib.scons +++ b/net/net_lib.scons @@ -76,6 +76,7 @@ input_files = [ 'disk_cache/trace.cc', 'http/cert_status_cache.cc', 'http/http_auth.cc', + 'http/http_auth_cache.cc', 'http/http_auth_handler.cc', 'http/http_auth_handler_basic.cc', 'http/http_auth_handler_digest.cc', diff --git a/net/net_unittests.scons b/net/net_unittests.scons index 3966f70..6079392 100644 --- a/net/net_unittests.scons +++ b/net/net_unittests.scons @@ -88,6 +88,7 @@ input_files = [ 'disk_cache/entry_unittest.cc', 'disk_cache/mapped_file_unittest.cc', 'disk_cache/storage_block_unittest.cc', + 'http/http_auth_cache_unittest.cc', 'http/http_auth_unittest.cc', 'http/http_auth_handler_basic_unittest.cc', 'http/http_auth_handler_digest_unittest.cc', diff --git a/net/url_request/url_request_ftp_job.cc b/net/url_request/url_request_ftp_job.cc index fab4ff3..9cd7ca9 100644 --- a/net/url_request/url_request_ftp_job.cc +++ b/net/url_request/url_request_ftp_job.cc @@ -167,19 +167,32 @@ void URLRequestFtpJob::OnIOComplete(const AsyncResult& result) { // fall through case ERROR_INTERNET_INCORRECT_USER_NAME: // fall through - case ERROR_INTERNET_INCORRECT_PASSWORD: + case ERROR_INTERNET_INCORRECT_PASSWORD: { + // TODO(eroman): shouldn't the port be part of the key? + std::string cache_key = request_->url().host(); if (server_auth_ != NULL && server_auth_->state == net::AUTH_STATE_HAVE_AUTH) { - request_->context()->ftp_auth_cache()->Remove(request_->url().host()); + request_->context()->ftp_auth_cache()->Remove(cache_key); } else { server_auth_ = new net::AuthData(); } - // Try again, prompting for authentication. server_auth_->state = net::AUTH_STATE_NEED_AUTH; - // The io completed fine, the error was due to invalid auth. - SetStatus(URLRequestStatus()); - NotifyHeadersComplete(); + + scoped_refptr<net::AuthData> cached_auth = + request_->context()->ftp_auth_cache()->Lookup(cache_key); + + if (cached_auth) { + // Retry using cached auth data. + SetAuth(cached_auth->username, cached_auth->password); + } else { + // The io completed fine, the error was due to invalid auth. + SetStatus(URLRequestStatus()); + + // Prompt for a username/password. + NotifyHeadersComplete(); + } return; + } case ERROR_SUCCESS: connection_handle_ = (HINTERNET)result.dwResult; OnConnect(); @@ -261,13 +274,6 @@ void URLRequestFtpJob::GetAuthChallengeInfo( result->swap(auth_info); } -void URLRequestFtpJob::GetCachedAuthData( - const net::AuthChallengeInfo& auth_info, - scoped_refptr<net::AuthData>* auth_data) { - *auth_data = request_->context()->ftp_auth_cache()-> - Lookup(WideToUTF8(auth_info.host)); -} - void URLRequestFtpJob::OnConnect() { DCHECK_EQ(state_, CONNECTING); diff --git a/net/url_request/url_request_ftp_job.h b/net/url_request/url_request_ftp_job.h index 3b766b4..44e845c 100644 --- a/net/url_request/url_request_ftp_job.h +++ b/net/url_request/url_request_ftp_job.h @@ -33,8 +33,6 @@ class URLRequestFtpJob : public URLRequestInetJob { virtual void OnSetAuth(); virtual bool NeedsAuth(); virtual void GetAuthChallengeInfo(scoped_refptr<net::AuthChallengeInfo>*); - virtual void GetCachedAuthData(const net::AuthChallengeInfo& auth_info, - scoped_refptr<net::AuthData>* auth_data); virtual bool IsRedirectResponse(GURL* location, int* http_status_code); private: diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index 49a8925..69f7bb5 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -261,20 +261,6 @@ void URLRequestHttpJob::GetAuthChallengeInfo( *result = response_info_->auth_challenge; } -void URLRequestHttpJob::GetCachedAuthData( - const net::AuthChallengeInfo& auth_info, - scoped_refptr<net::AuthData>* auth_data) { - net::AuthCache* auth_cache = - request_->context()->http_transaction_factory()->GetAuthCache(); - if (!auth_cache) { - *auth_data = NULL; - return; - } - std::string auth_cache_key = - net::AuthCache::HttpKey(request_->url(), auth_info); - *auth_data = auth_cache->Lookup(auth_cache_key); -} - void URLRequestHttpJob::SetAuth(const std::wstring& username, const std::wstring& password) { DCHECK(transaction_.get()); diff --git a/net/url_request/url_request_http_job.h b/net/url_request/url_request_http_job.h index f36a072..eda4b4b 100644 --- a/net/url_request/url_request_http_job.h +++ b/net/url_request/url_request_http_job.h @@ -48,8 +48,6 @@ class URLRequestHttpJob : public URLRequestJob { virtual bool IsSafeRedirect(const GURL& location); virtual bool NeedsAuth(); virtual void GetAuthChallengeInfo(scoped_refptr<net::AuthChallengeInfo>*); - virtual void GetCachedAuthData(const net::AuthChallengeInfo& auth_info, - scoped_refptr<net::AuthData>* auth_data); virtual void SetAuth(const std::wstring& username, const std::wstring& password); virtual void CancelAuth(); diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc index ea5ccb9..6e969b2 100644 --- a/net/url_request/url_request_job.cc +++ b/net/url_request/url_request_job.cc @@ -314,12 +314,6 @@ void URLRequestJob::NotifyHeadersComplete() { // Need to check for a NULL auth_info because the server may have failed // to send a challenge with the 401 response. if (auth_info) { - scoped_refptr<net::AuthData> auth_data; - GetCachedAuthData(*auth_info, &auth_data); - if (auth_data) { - SetAuth(auth_data->username, auth_data->password); - return; - } request_->delegate()->OnAuthRequired(request_, auth_info); // Wait for SetAuth or CancelAuth to be called. return; diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h index 6b309be..43fa866 100644 --- a/net/url_request/url_request_job.h +++ b/net/url_request/url_request_job.h @@ -164,14 +164,6 @@ class URLRequestJob : public base::RefCountedThreadSafe<URLRequestJob> { virtual void GetAuthChallengeInfo( scoped_refptr<net::AuthChallengeInfo>* auth_info); - // Returns cached auth data for the auth challenge. Returns NULL if there - // is no auth cache or if the auth cache doesn't have the auth data for - // the auth challenge. - virtual void GetCachedAuthData(const net::AuthChallengeInfo& auth_info, - scoped_refptr<net::AuthData>* auth_data) { - *auth_data = NULL; - } - // Resend the request with authentication credentials. virtual void SetAuth(const std::wstring& username, const std::wstring& password); |