NetworkDelegate::OnAuthRequired can set or cancel auth, in addition t…

…o taking no action.

BUG=32056
TEST=net_unittests


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

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

chrome/browser/net/chrome_network_delegate.cc

@@ -160,9 +160,13 @@ void ChromeNetworkDelegate::OnPACScriptError(int line_number,
       event_router_.get(), profile_, line_number, error);
 }
 
-void ChromeNetworkDelegate::OnAuthRequired(
+net::NetworkDelegate::AuthRequiredResponse
+ChromeNetworkDelegate::OnAuthRequired(
     net::URLRequest* request,
-    const net::AuthChallengeInfo& auth_info) {
+    const net::AuthChallengeInfo& auth_info,
+    const AuthCallback& callback,
+    net::AuthCredentials* credentials) {
   ExtensionWebRequestEventRouter::GetInstance()->OnAuthRequired(
       profile_, extension_info_map_.get(), request, auth_info);
+  return net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
 }

chrome/browser/net/chrome_network_delegate.h

@@ -63,8 +63,11 @@ class ChromeNetworkDelegate : public net::NetworkDelegate {
   virtual void OnURLRequestDestroyed(net::URLRequest* request) OVERRIDE;
   virtual void OnPACScriptError(int line_number,
                                 const string16& error) OVERRIDE;
-  virtual void OnAuthRequired(net::URLRequest* request,
-                              const net::AuthChallengeInfo& auth_info) OVERRIDE;
+  virtual net::NetworkDelegate::AuthRequiredResponse OnAuthRequired(
+      net::URLRequest* request,
+      const net::AuthChallengeInfo& auth_info,
+      const AuthCallback& callback,
+      net::AuthCredentials* credentials) OVERRIDE;
 
   scoped_refptr<ExtensionEventRouterForwarder> event_router_;
   void* profile_;

chrome_frame/test/net/fake_external_tab.cc

@@ -522,6 +522,15 @@ void FilterDisabledTests() {
 
     // Not supported in ChromeFrame as we use IE's network stack.
     "URLRequestTest.NetworkDelegateProxyError",
+
+    // URLRequestAutomationJob needs to support NeedsAuth.
+    // http://crbug.com/98446
+    "URLRequestTestHTTP.NetworkDelegateOnAuthRequiredSyncNoAction",
+    "URLRequestTestHTTP.NetworkDelegateOnAuthRequiredSyncSetAuth",
+    "URLRequestTestHTTP.NetworkDelegateOnAuthRequiredSyncCancel",
+    "URLRequestTestHTTP.NetworkDelegateOnAuthRequiredAsyncNoAction",
+    "URLRequestTestHTTP.NetworkDelegateOnAuthRequiredAsyncSetAuth",
+    "URLRequestTestHTTP.NetworkDelegateOnAuthRequiredAsyncCancel",
   };
 
   std::string filter("-");  // All following filters will be negative.

net/base/auth.cc

@@ -25,4 +25,10 @@ AuthData::AuthData() : state(AUTH_STATE_NEED_AUTH) {
 AuthData::~AuthData() {
 }
 
+AuthCredentials::AuthCredentials() {
+}
+
+AuthCredentials::~AuthCredentials() {
+}
+
 }  // namespace net

net/base/auth.h

@@ -43,6 +43,26 @@ class NET_EXPORT AuthChallengeInfo :
   ~AuthChallengeInfo();
 };
 
+// Authentication Credentials for an authentication credentials.
+class NET_EXPORT AuthCredentials {
+ public:
+  AuthCredentials();
+  ~AuthCredentials();
+
+  // The username to provide, possibly empty. This should be ASCII only to
+  // minimize compatibility problems, but arbitrary UTF-16 strings are allowed
+  // and will be attempted.
+  string16 username;
+
+  // The password to provide, possibly empty. This should be ASCII only to
+  // minimize compatibility problems, but arbitrary UTF-16 strings are allowed
+  // and will be attempted.
+  string16 password;
+
+  // Intentionally allowing the implicit copy constructor and assignment
+  // operators.
+};
+
 // Authentication structures
 enum AuthState {
   AUTH_STATE_DONT_NEED_AUTH,
@@ -54,8 +74,7 @@ enum AuthState {
 class AuthData : public base::RefCountedThreadSafe<AuthData> {
  public:
   AuthState state;  // whether we need, have, or gave up on authentication.
-  string16 username;  // the username supplied to us for auth.
-  string16 password;  // the password supplied to us for auth.
+  AuthCredentials credentials; // The credentials to use for auth.
 
   // We wouldn't instantiate this class if we didn't need authentication.
   AuthData();

net/base/network_delegate.cc

@@ -69,10 +69,13 @@ void NetworkDelegate::NotifyPACScriptError(int line_number,
   OnPACScriptError(line_number, error);
 }
 
-void NetworkDelegate::NotifyAuthRequired(URLRequest* request,
-                                         const AuthChallengeInfo& auth_info) {
+NetworkDelegate::AuthRequiredResponse NetworkDelegate::NotifyAuthRequired(
+    URLRequest* request,
+    const AuthChallengeInfo& auth_info,
+    const AuthCallback& callback,
+    AuthCredentials* credentials) {
   DCHECK(CalledOnValidThread());
-  OnAuthRequired(request, auth_info);
+  return OnAuthRequired(request, auth_info, callback, credentials);
 }
 
 }  // namespace net

net/base/network_delegate.h

@@ -6,8 +6,10 @@
 #define NET_BASE_NETWORK_DELEGATE_H_
 #pragma once
 
+#include "base/callback.h"
 #include "base/string16.h"
 #include "base/threading/non_thread_safe.h"
+#include "net/base/auth.h"
 #include "net/base/completion_callback.h"
 
 class GURL;
@@ -24,14 +26,24 @@ namespace net {
 // NOTE: It is not okay to add any compile-time dependencies on symbols outside
 // of net/base here, because we have a net_base library. Forward declarations
 // are ok.
-class AuthChallengeInfo;
 class HostPortPair;
 class HttpRequestHeaders;
 class URLRequest;
 class URLRequestJob;
 
 class NetworkDelegate : public base::NonThreadSafe {
  public:
+  // AuthRequiredResponse indicates how a NetworkDelegate handles an
+  // OnAuthRequired call. It's placed in this file to prevent url_request.h
+  // from having to include network_delegate.h.
+  enum AuthRequiredResponse {
+    AUTH_REQUIRED_RESPONSE_NO_ACTION,
+    AUTH_REQUIRED_RESPONSE_SET_AUTH,
+    AUTH_REQUIRED_RESPONSE_CANCEL_AUTH,
+    AUTH_REQUIRED_RESPONSE_IO_PENDING,
+  };
+  typedef base::Callback<void(AuthRequiredResponse)> AuthCallback;
+
   virtual ~NetworkDelegate() {}
 
   // Notification interface called by the network stack. Note that these
@@ -53,8 +65,10 @@ class NetworkDelegate : public base::NonThreadSafe {
   void NotifyCompleted(URLRequest* request);
   void NotifyURLRequestDestroyed(URLRequest* request);
   void NotifyPACScriptError(int line_number, const string16& error);
-  void NotifyAuthRequired(URLRequest* request,
-                          const AuthChallengeInfo& auth_info);
+  AuthRequiredResponse NotifyAuthRequired(URLRequest* request,
+                                          const AuthChallengeInfo& auth_info,
+                                          const AuthCallback& callback,
+                                          AuthCredentials* credentials);
 
  private:
   // This is the interface for subclasses of NetworkDelegate to implement. This
@@ -72,7 +86,7 @@ class NetworkDelegate : public base::NonThreadSafe {
 
   // Called right before the HTTP headers are sent. Allows the delegate to
   // read/write |headers| before they get sent out. |callback| and |headers| are
-  // valid only until OnHttpTransactionDestroyed is called for this request.
+  // valid only until OnURLRequestDestroyed is called for this request.
   // Returns a net status code.
   virtual int OnBeforeSendHeaders(URLRequest* request,
                                   OldCompletionCallback* callback,
@@ -103,9 +117,28 @@ class NetworkDelegate : public base::NonThreadSafe {
   // Corresponds to ProxyResolverJSBindings::OnError.
   virtual void OnPACScriptError(int line_number, const string16& error) = 0;
 
-  // Corresponds to URLRequest::Delegate::OnAuthRequired.
-  virtual void OnAuthRequired(URLRequest* reqest,
-                              const AuthChallengeInfo& auth_info) = 0;
+  // Called when a request receives an authentication challenge
+  // specified by |auth_info|, and is unable to respond using cached
+  // credentials. |callback| and |credentials| must be non-NULL, and must
+  // be valid until OnURLRequestDestroyed is called for |request|.
+  //
+  // The following return values are allowed:
+  //  - AUTH_REQUIRED_RESPONSE_NO_ACTION: |auth_info| is observed, but
+  //    no action is being taken on it.
+  //  - AUTH_REQUIRED_RESPONSE_SET_AUTH: |credentials| is filled in with
+  //    a username and password, which should be used in a response to
+  //    |auth_info|.
+  //  - AUTH_REQUIRED_RESPONSE_CANCEL_AUTH: The authentication challenge
+  //    should not be attempted.
+  //  - AUTH_REQUIRED_RESPONSE_IO_PENDING: The action will be decided
+  //    asynchronously. |callback| will be invoked when the decision is made,
+  //    and one of the other AuthRequiredResponse values will be passed in with
+  //    the same semantics as described above.
+  virtual AuthRequiredResponse OnAuthRequired(
+      URLRequest* request,
+      const AuthChallengeInfo& auth_info,
+      const AuthCallback& callback,
+      AuthCredentials* credentials) = 0;
 };
 
 }  // namespace net

net/proxy/network_delegate_error_observer_unittest.cc

@@ -28,12 +28,12 @@ class TestNetworkDelegate : public net::NetworkDelegate {
   virtual int OnBeforeURLRequest(URLRequest* request,
                                  OldCompletionCallback* callback,
                                  GURL* new_url) OVERRIDE {
-    return net::OK;
+    return OK;
   }
   virtual int OnBeforeSendHeaders(URLRequest* request,
                                   OldCompletionCallback* callback,
                                   HttpRequestHeaders* headers) OVERRIDE {
-    return net::OK;
+    return OK;
   }
   virtual void OnSendHeaders(URLRequest* request,
                              const HttpRequestHeaders& headers) OVERRIDE {}
@@ -49,8 +49,13 @@ class TestNetworkDelegate : public net::NetworkDelegate {
                                 const string16& error) OVERRIDE {
     got_pac_error_ = true;
   }
-  virtual void OnAuthRequired(URLRequest* request,
-                              const AuthChallengeInfo& auth_info) OVERRIDE {}
+  virtual AuthRequiredResponse OnAuthRequired(
+      URLRequest* request,
+      const AuthChallengeInfo& auth_info,
+      const AuthCallback& callback,
+      AuthCredentials* credentials) OVERRIDE {
+    return AUTH_REQUIRED_RESPONSE_NO_ACTION;
+  }
 
   bool got_pac_error_;
 };

net/url_request/url_request.cc

@@ -4,11 +4,14 @@
 
 #include "net/url_request/url_request.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop.h"
 #include "base/metrics/stats_counters.h"
 #include "base/synchronization/lock.h"
+#include "net/base/auth.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
@@ -430,8 +433,7 @@ void URLRequest::BeforeRequestComplete(int error) {
   DCHECK(!job_);
   DCHECK_NE(ERR_IO_PENDING, error);
 
-  if (blocked_on_delegate_)
-    SetUnblockedOnDelegate();
+  SetUnblockedOnDelegate();
   if (error != OK) {
     net_log_.AddEvent(NetLog::TYPE_CANCELLED,
         make_scoped_refptr(new NetLogStringParameter("source", "delegate")));
@@ -766,20 +768,57 @@ void URLRequest::SetUserData(const void* key, UserData* data) {
 }
 
 void URLRequest::NotifyAuthRequired(AuthChallengeInfo* auth_info) {
-  // TODO(battre): We could simulate a redirection there as follows:
-  // if (context_ && context_->network_delegate()) {
-  //  // We simulate a redirection.
-  //   context_->network_delegate()->NotifyBeforeRedirect(this, url());
-  //}
-  // This fixes URLRequestTestHTTP.BasicAuth but not
-  // URLRequestTestHTTP.BasicAuthWithCookies. In both cases we observe a
-  // call sequence of OnBeforeSendHeaders -> OnSendHeaders ->
-  // OnBeforeSendHeaders.
-  if (context_ && context_->network_delegate())
-    context_->network_delegate()->NotifyAuthRequired(this, *auth_info);
+  NetworkDelegate::AuthRequiredResponse rv =
+      NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
+  auth_info_ = auth_info;
+  if (context_ && context_->network_delegate()) {
+    rv = context_->network_delegate()->NotifyAuthRequired(
+        this,
+        *auth_info,
+        base::Bind(&URLRequest::NotifyAuthRequiredComplete,
+                   base::Unretained(this)),
+        &auth_credentials_);
+  }
 
-  if (delegate_)
-    delegate_->OnAuthRequired(this, auth_info);
+  if (rv == NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING) {
+    SetBlockedOnDelegate();
+  } else {
+    NotifyAuthRequiredComplete(rv);
+  }
+}
+
+void URLRequest::NotifyAuthRequiredComplete(
+    NetworkDelegate::AuthRequiredResponse result) {
+  SetUnblockedOnDelegate();
+
+  // NotifyAuthRequired may be called multiple times, such as
+  // when an authentication attempt fails. Clear out the data
+  // so it can be reset on another round.
+  AuthCredentials credentials = auth_credentials_;
+  auth_credentials_ = AuthCredentials();
+  scoped_refptr<AuthChallengeInfo> auth_info;
+  auth_info.swap(auth_info_);
+
+  switch (result) {
+    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION:
+      // Defer to the URLRequest::Delegate, since the NetworkDelegate
+      // didn't take an action.
+      if (delegate_)
+        delegate_->OnAuthRequired(this, auth_info.get());
+      break;
+
+    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH:
+      SetAuth(credentials.username, credentials.password);
+      break;
+
+    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH:
+      CancelAuth();
+      break;
+
+    case NetworkDelegate::AUTH_REQUIRED_RESPONSE_IO_PENDING:
+      NOTREACHED();
+      break;
+  }
 }
 
 void URLRequest::NotifyCertificateRequested(
@@ -837,6 +876,8 @@ void URLRequest::SetBlockedOnDelegate() {
 }
 
 void URLRequest::SetUnblockedOnDelegate() {
+  if (!blocked_on_delegate_)
+    return;
   blocked_on_delegate_ = false;
   load_state_param_.clear();
   net_log_.EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_DELEGATE, NULL);

net/url_request/url_request.h

@@ -17,10 +17,12 @@
 #include "base/string16.h"
 #include "base/threading/non_thread_safe.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/auth.h"
 #include "net/base/completion_callback.h"
 #include "net/base/load_states.h"
 #include "net/base/net_export.h"
 #include "net/base/net_log.h"
+#include "net/base/network_delegate.h"
 #include "net/base/request_priority.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_response_info.h"
@@ -716,6 +718,7 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) {
   // |delegate_| is not NULL. See URLRequest::Delegate for the meaning
   // of these functions.
   void NotifyAuthRequired(AuthChallengeInfo* auth_info);
+  void NotifyAuthRequiredComplete(NetworkDelegate::AuthRequiredResponse result);
   void NotifyCertificateRequested(SSLCertRequestInfo* cert_request_info);
   void NotifySSLCertificateError(const SSLInfo& ssl_info,
                                  bool is_hsts_host);
@@ -808,6 +811,13 @@ class NET_EXPORT URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) {
   // TODO(battre): Remove this. http://crbug.com/89049
   bool has_notified_completion_;
 
+  // Authentication data used by the NetworkDelegate for this request,
+  // if one is present. |auth_credentials_| may be filled in when calling
+  // |NotifyAuthRequired| on the NetworkDelegate. |auth_info_| holds
+  // the authentication challenge being handled by |NotifyAuthRequired|.
+  AuthCredentials auth_credentials_;
+  scoped_refptr<AuthChallengeInfo> auth_info_;
+
   DISALLOW_COPY_AND_ASSIGN(URLRequest);
 };