fix: implement 'login' event for net.ClientRequest (#21135)

* fix: implement 'login' event for net.ClientRequest (#21096)

* fix patch

* lint

* include mojo header in atom_browser_context.h

* fix compile

* kick ci

* URLRequest -> URLRequestNS

* fix ts

* no ts in js

docs/api/app.md

@@ -323,8 +323,8 @@ Returns:
   * `port` Integer
   * `realm` String
 * `callback` Function
-  * `username` String
-  * `password` String
+  * `username` String (optional)
+  * `password` String (optional)
 
 Emitted when `webContents` wants to do basic auth.
 
@@ -341,6 +341,10 @@ app.on('login', (event, webContents, details, authInfo, callback) => {
 })
 ```
 
+If `callback` is called without a username or password, the authentication
+request will be cancelled and the authentication error will be returned to the
+page.
+
 ### Event: 'gpu-info-update'
 
 Emitted whenever there is a GPU info update.

docs/api/client-request.md

@@ -70,8 +70,8 @@ Returns:
   * `port` Integer
   * `realm` String
 * `callback` Function
-  * `username` String
-  * `password` String
+  * `username` String (optional)
+  * `password` String (optional)
 
 Emitted when an authenticating proxy is asking for user credentials.
 

docs/api/web-contents.md

@@ -463,8 +463,8 @@ Returns:
   * `port` Integer
   * `realm` String
 * `callback` Function
-  * `username` String
-  * `password` String
+  * `username` String (optional)
+  * `password` String (optional)
 
 Emitted when `webContents` wants to do basic auth.
 

lib/browser/api/net.js

@@ -247,22 +247,11 @@ class ClientRequest extends EventEmitter {
     })
 
     urlRequest.on('login', (event, authInfo, callback) => {
-      this.emit('login', authInfo, (username, password) => {
-        // If null or undefined username/password, force to empty string.
-        if (username === null || username === undefined) {
-          username = ''
-        }
-        if (typeof username !== 'string') {
-          throw new Error('username must be a string')
-        }
-        if (password === null || password === undefined) {
-          password = ''
-        }
-        if (typeof password !== 'string') {
-          throw new Error('password must be a string')
-        }
-        callback(username, password)
-      })
+      const handled = this.emit('login', authInfo, callback)
+      if (!handled) {
+        // If there were no listeners, cancel the authentication request.
+        callback()
+      }
     })
 
     if (callback) {

patches/chromium/.patches

@@ -81,5 +81,6 @@ net_avoid_vector_const_elements.patch
 feat_add_set_theme_source_to_allow_apps_to.patch
 build_fix_when_building_with_enable_plugins_false.patch
 x11_and_ozone_move_closing_logic_to_dwthplatform.patch
+add_trustedauthclient_to_urlloaderfactory.patch
 make_autocorrect_off_and_spellcheck_false_disable_touch_bar_typing.patch
 fix_focusowningwebcontents_to_handle_renderwidgethosts_for_oopifs.patch

patches/chromium/add_trustedauthclient_to_urlloaderfactory.patch

@@ -0,0 +1,158 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <[email protected]>
+Date: Tue, 12 Nov 2019 11:50:16 -0800
+Subject: add TrustedAuthClient to URLLoaderFactory
+
+This allows intercepting authentication requests for the 'net' module.
+Without this, the 'login' event for electron.net.ClientRequest can't be
+implemented, because the existing path checks for the presence of a
+WebContents, and cancels the authentication if there's no WebContents
+available, which there isn't in the case of the 'net' module.
+
+diff --git a/services/network/public/mojom/network_context.mojom b/services/network/public/mojom/network_context.mojom
+index 0671ed5ff69a53e6793c7e33db092150fc783601..b577333983ac1268d3a568699d7cde7a375e0877 100644
+--- a/services/network/public/mojom/network_context.mojom
++++ b/services/network/public/mojom/network_context.mojom
+@@ -168,6 +168,25 @@ interface TrustedURLLoaderHeaderClient {
+                   pending_receiver<TrustedHeaderClient> header_client);
+ };
+ 
++interface TrustedAuthClient {
++  OnAuthRequired(
++      mojo_base.mojom.UnguessableToken? window_id,
++      uint32 process_id,
++      uint32 routing_id,
++      uint32 request_id,
++      url.mojom.Url url,
++      bool first_auth_attempt,
++      AuthChallengeInfo auth_info,
++      URLResponseHead? head,
++      pending_remote<AuthChallengeResponder> auth_challenge_responder);
++};
++interface TrustedURLLoaderAuthClient {
++  // When a new URLLoader is created, this will be called to pass a
++  // corresponding |auth_client|.
++  OnLoaderCreated(int32 request_id,
++                  pending_receiver<TrustedAuthClient> auth_client);
++};
++
+ interface CertVerifierClient {
+   Verify(
+     int32 default_error,
+@@ -535,6 +554,8 @@ struct URLLoaderFactoryParams {
+   // impact because of the extra process hops, so use should be minimized.
+   pending_remote<TrustedURLLoaderHeaderClient>? header_client;
+ 
++  pending_remote<TrustedURLLoaderAuthClient>? auth_client;
++
+   // If non-empty array is given, |factory_bound_allow_patterns| is used for
+   // CORS checks in addition to the per-context allow patterns that is managed
+   // via NetworkContext interface. This still respects the per-context block
+diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
+index 341e1c78acf41b42b4ff210af575bbe870d543cc..5c2b111cf98ec6577e94da9bf5991d1396a367a8 100644
+--- a/services/network/url_loader.cc
++++ b/services/network/url_loader.cc
+@@ -336,6 +336,7 @@ URLLoader::URLLoader(
+     base::WeakPtr<KeepaliveStatisticsRecorder> keepalive_statistics_recorder,
+     base::WeakPtr<NetworkUsageAccumulator> network_usage_accumulator,
+     mojom::TrustedURLLoaderHeaderClient* url_loader_header_client,
++    mojom::TrustedURLLoaderAuthClient* url_loader_auth_client,
+     mojom::OriginPolicyManager* origin_policy_manager)
+     : url_request_context_(url_request_context),
+       network_service_client_(network_service_client),
+@@ -386,6 +387,11 @@ URLLoader::URLLoader(
+     header_client_.set_disconnect_handler(
+         base::BindOnce(&URLLoader::OnConnectionError, base::Unretained(this)));
+   }
++  if (url_loader_auth_client) {
++    url_loader_auth_client->OnLoaderCreated(request_id_, auth_client_.BindNewPipeAndPassReceiver());
++    auth_client_.set_disconnect_handler(
++        base::BindOnce(&URLLoader::OnConnectionError, base::Unretained(this)));
++  }
+   if (want_raw_headers_) {
+     options_ |= mojom::kURLLoadOptionSendSSLInfoWithResponse |
+                 mojom::kURLLoadOptionSendSSLInfoForCertificateError;
+@@ -819,7 +825,7 @@ void URLLoader::OnReceivedRedirect(net::URLRequest* url_request,
+ 
+ void URLLoader::OnAuthRequired(net::URLRequest* url_request,
+                                const net::AuthChallengeInfo& auth_info) {
+-  if (!network_context_client_) {
++  if (!network_context_client_ && !auth_client_) {
+     OnAuthCredentials(base::nullopt);
+     return;
+   }
+@@ -835,10 +841,18 @@ void URLLoader::OnAuthRequired(net::URLRequest* url_request,
+   if (url_request->response_headers())
+     head.headers = url_request->response_headers();
+   head.auth_challenge_info = auth_info;
+-  network_context_client_->OnAuthRequired(
+-      fetch_window_id_, factory_params_->process_id, render_frame_id_,
+-      request_id_, url_request_->url(), first_auth_attempt_, auth_info, head,
+-      auth_challenge_responder_receiver_.BindNewPipeAndPassRemote());
++
++  if (auth_client_) {
++    auth_client_->OnAuthRequired(
++        fetch_window_id_, factory_params_->process_id, render_frame_id_,
++        request_id_, url_request_->url(), first_auth_attempt_, auth_info, head,
++        auth_challenge_responder_receiver_.BindNewPipeAndPassRemote());
++  } else {
++    network_context_client_->OnAuthRequired(
++        fetch_window_id_, factory_params_->process_id, render_frame_id_,
++        request_id_, url_request_->url(), first_auth_attempt_, auth_info, head,
++        auth_challenge_responder_receiver_.BindNewPipeAndPassRemote());
++  }
+ 
+   auth_challenge_responder_receiver_.set_disconnect_handler(
+       base::BindOnce(&URLLoader::DeleteSelf, base::Unretained(this)));
+diff --git a/services/network/url_loader.h b/services/network/url_loader.h
+index 302e469ad63bdb7fe5b4dea775079962df89efd3..92706003d7a0d8905cf27ad52972044354f03841 100644
+--- a/services/network/url_loader.h
++++ b/services/network/url_loader.h
+@@ -84,6 +84,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
+       base::WeakPtr<KeepaliveStatisticsRecorder> keepalive_statistics_recorder,
+       base::WeakPtr<NetworkUsageAccumulator> network_usage_accumulator,
+       mojom::TrustedURLLoaderHeaderClient* url_loader_header_client,
++      mojom::TrustedURLLoaderAuthClient* url_loader_auth_client,
+       mojom::OriginPolicyManager* origin_policy_manager);
+   ~URLLoader() override;
+ 
+@@ -361,6 +362,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
+   base::Optional<base::UnguessableToken> fetch_window_id_;
+ 
+   mojo::Remote<mojom::TrustedHeaderClient> header_client_;
++  mojo::Remote<mojom::TrustedAuthClient> auth_client_;
+ 
+   std::unique_ptr<FileOpenerForUpload> file_opener_for_upload_;
+ 
+diff --git a/services/network/url_loader_factory.cc b/services/network/url_loader_factory.cc
+index e01d555e62568a506ed8093e2e4c3ff61ffe4ac2..bccc9c79b4653371567f4ee479dcbd8aca5c7438 100644
+--- a/services/network/url_loader_factory.cc
++++ b/services/network/url_loader_factory.cc
+@@ -37,6 +37,7 @@ URLLoaderFactory::URLLoaderFactory(
+       params_(std::move(params)),
+       resource_scheduler_client_(std::move(resource_scheduler_client)),
+       header_client_(std::move(params_->header_client)),
++      auth_client_(std::move(params_->auth_client)),
+       cors_url_loader_factory_(cors_url_loader_factory) {
+   DCHECK(context);
+   DCHECK_NE(mojom::kInvalidProcessId, params_->process_id);
+@@ -147,6 +148,7 @@ void URLLoaderFactory::CreateLoaderAndStart(
+       std::move(keepalive_statistics_recorder),
+       std::move(network_usage_accumulator),
+       header_client_.is_bound() ? header_client_.get() : nullptr,
++      auth_client_.is_bound() ? auth_client_.get() : nullptr,
+       context_->origin_policy_manager());
+   cors_url_loader_factory_->OnLoaderCreated(std::move(loader));
+ }
+diff --git a/services/network/url_loader_factory.h b/services/network/url_loader_factory.h
+index 30bdfafd8a951f35946c172e1bed0a8d7c367362..ec755f67f96422184d74d7b489f2887db12fb18c 100644
+--- a/services/network/url_loader_factory.h
++++ b/services/network/url_loader_factory.h
+@@ -70,6 +70,7 @@ class URLLoaderFactory : public mojom::URLLoaderFactory {
+   mojom::URLLoaderFactoryParamsPtr params_;
+   scoped_refptr<ResourceSchedulerClient> resource_scheduler_client_;
+   mojo::Remote<mojom::TrustedURLLoaderHeaderClient> header_client_;
++  mojo::Remote<mojom::TrustedURLLoaderAuthClient> auth_client_;
+ 
+   // |cors_url_loader_factory_| owns this.
+   cors::CorsURLLoaderFactory* cors_url_loader_factory_;

shell/browser/api/atom_api_url_request_ns.cc

@@ -6,13 +6,17 @@
 
 #include <utility>
 
+#include "base/containers/id_map.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/system/string_data_source.h"
+#include "native_mate/arguments.h"
 #include "native_mate/dictionary.h"
 #include "native_mate/object_template_builder.h"
 #include "net/http/http_util.h"
 #include "services/network/public/mojom/chunked_data_pipe_getter.mojom.h"
 #include "shell/browser/api/atom_api_session.h"
 #include "shell/browser/atom_browser_context.h"
+#include "shell/common/native_mate_converters/callback.h"
 #include "shell/common/native_mate_converters/gurl_converter.h"
 #include "shell/common/native_mate_converters/net_converter.h"
 
@@ -48,6 +52,11 @@ namespace api {
 
 namespace {
 
+base::IDMap<URLRequestNS*>& GetAllRequests() {
+  static base::NoDestructor<base::IDMap<URLRequestNS*>> s_all_requests;
+  return *s_all_requests;
+}
+
 // Network state for request and response.
 enum State {
   STATE_STARTED = 1 << 0,
@@ -159,7 +168,9 @@ class ChunkedDataPipeGetter : public UploadDataPipeGetter,
   mojo::BindingSet<network::mojom::ChunkedDataPipeGetter> binding_set_;
 };
 
-URLRequestNS::URLRequestNS(mate::Arguments* args) : weak_factory_(this) {
+URLRequestNS::URLRequestNS(mate::Arguments* args)
+    : id_(GetAllRequests().Add(this)), weak_factory_(this) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   request_ = std::make_unique<network::ResourceRequest>();
   mate::Dictionary dict;
   if (args->GetNext(&dict)) {
@@ -169,6 +180,8 @@ URLRequestNS::URLRequestNS(mate::Arguments* args) : weak_factory_(this) {
     request_->redirect_mode = redirect_mode_;
   }
 
+  request_->render_frame_id = id_;
+
   std::string partition;
   mate::Handle<api::Session> session;
   if (!dict.Get("session", &session)) {
@@ -185,6 +198,38 @@ URLRequestNS::URLRequestNS(mate::Arguments* args) : weak_factory_(this) {
 
 URLRequestNS::~URLRequestNS() {}
 
+URLRequestNS* URLRequestNS::FromID(uint32_t id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  return GetAllRequests().Lookup(id);
+}
+
+void URLRequestNS::OnAuthRequired(
+    const GURL& url,
+    bool first_auth_attempt,
+    net::AuthChallengeInfo auth_info,
+    network::mojom::URLResponseHeadPtr head,
+    mojo::PendingRemote<network::mojom::AuthChallengeResponder>
+        auth_challenge_responder) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  mojo::Remote<network::mojom::AuthChallengeResponder> auth_responder(
+      std::move(auth_challenge_responder));
+  auth_responder.set_disconnect_handler(
+      base::BindOnce(&URLRequestNS::Cancel, weak_factory_.GetWeakPtr()));
+  auto cb = base::BindOnce(
+      [](mojo::Remote<network::mojom::AuthChallengeResponder> auth_responder,
+         mate::Arguments* args) {
+        base::string16 username_str, password_str;
+        if (!args->GetNext(&username_str) || !args->GetNext(&password_str)) {
+          auth_responder->OnAuthCredentials(base::nullopt);
+          return;
+        }
+        auth_responder->OnAuthCredentials(
+            net::AuthCredentials(username_str, password_str));
+      },
+      std::move(auth_responder));
+  Emit("login", auth_info, base::AdaptCallbackForRepeating(std::move(cb)));
+}
+
 bool URLRequestNS::NotStarted() const {
   return request_state_ == 0;
 }
@@ -505,11 +550,12 @@ void URLRequestNS::EmitError(EventType type, base::StringPiece message) {
 }
 
 template <typename... Args>
-void URLRequestNS::EmitEvent(EventType type, Args... args) {
+void URLRequestNS::EmitEvent(EventType type, Args&&... args) {
   const char* method =
       type == EventType::kRequest ? "_emitRequestEvent" : "_emitResponseEvent";
   v8::HandleScope handle_scope(isolate());
-  mate::CustomEmit(isolate(), GetWrapper(), method, args...);
+  mate::CustomEmit(isolate(), GetWrapper(), method,
+                   std::forward<Args>(args)...);
 }
 
 // static

shell/browser/api/atom_api_url_request_ns.h

@@ -16,6 +16,7 @@
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/cpp/simple_url_loader_stream_consumer.h"
 #include "services/network/public/mojom/data_pipe_getter.mojom.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "shell/browser/api/event_emitter.h"
 
 namespace electron {
@@ -31,6 +32,15 @@ class URLRequestNS : public mate::EventEmitter<URLRequestNS>,
 
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::FunctionTemplate> prototype);
+  static URLRequestNS* FromID(uint32_t id);
+
+  void OnAuthRequired(
+      const GURL& url,
+      bool first_auth_attempt,
+      net::AuthChallengeInfo auth_info,
+      network::mojom::URLResponseHeadPtr head,
+      mojo::PendingRemote<network::mojom::AuthChallengeResponder>
+          auth_challenge_responder);
 
  protected:
   explicit URLRequestNS(mate::Arguments* args);
@@ -88,7 +98,7 @@ class URLRequestNS : public mate::EventEmitter<URLRequestNS>,
   };
   void EmitError(EventType type, base::StringPiece error);
   template <typename... Args>
-  void EmitEvent(EventType type, Args... args);
+  void EmitEvent(EventType type, Args&&... args);
 
   std::unique_ptr<network::ResourceRequest> request_;
   std::unique_ptr<network::SimpleURLLoader> loader_;
@@ -131,6 +141,8 @@ class URLRequestNS : public mate::EventEmitter<URLRequestNS>,
   // Used by pin/unpin to manage lifetime.
   v8::Global<v8::Object> wrapper_;
 
+  uint32_t id_;
+
   base::WeakPtrFactory<URLRequestNS> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestNS);

shell/browser/atom_browser_context.cc

@@ -26,9 +26,12 @@
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"  // nogncheck
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
+#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/base/escape.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
+#include "services/network/public/mojom/network_context.mojom.h"
+#include "shell/browser/api/atom_api_url_request_ns.h"
 #include "shell/browser/atom_browser_client.h"
 #include "shell/browser/atom_browser_main_parts.h"
 #include "shell/browser/atom_download_manager_delegate.h"
@@ -283,6 +286,7 @@ AtomBrowserContext::GetURLLoaderFactory() {
   network::mojom::URLLoaderFactoryParamsPtr params =
       network::mojom::URLLoaderFactoryParams::New();
   params->header_client = std::move(header_client);
+  params->auth_client = auth_client_.BindNewPipeAndPassRemote();
   params->process_id = network::mojom::kBrowserProcessId;
   params->is_corb_enabled = false;
   // The tests of net module would fail if this setting is true, it seems that
@@ -299,6 +303,39 @@ AtomBrowserContext::GetURLLoaderFactory() {
   return url_loader_factory_;
 }
 
+class AuthResponder : public network::mojom::TrustedAuthClient {
+ public:
+  AuthResponder() {}
+  ~AuthResponder() override = default;
+
+ private:
+  void OnAuthRequired(
+      const base::Optional<::base::UnguessableToken>& window_id,
+      uint32_t process_id,
+      uint32_t routing_id,
+      uint32_t request_id,
+      const ::GURL& url,
+      bool first_auth_attempt,
+      const ::net::AuthChallengeInfo& auth_info,
+      ::network::mojom::URLResponseHeadPtr head,
+      mojo::PendingRemote<network::mojom::AuthChallengeResponder>
+          auth_challenge_responder) override {
+    api::URLRequestNS* url_request = api::URLRequestNS::FromID(routing_id);
+    if (url_request) {
+      url_request->OnAuthRequired(url, first_auth_attempt, auth_info,
+                                  std::move(head),
+                                  std::move(auth_challenge_responder));
+    }
+  }
+};
+
+void AtomBrowserContext::OnLoaderCreated(
+    int32_t request_id,
+    mojo::PendingReceiver<network::mojom::TrustedAuthClient> auth_client) {
+  mojo::MakeSelfOwnedReceiver(std::make_unique<AuthResponder>(),
+                              std::move(auth_client));
+}
+
 content::PushMessagingService* AtomBrowserContext::GetPushMessagingService() {
   return nullptr;
 }

shell/browser/atom_browser_context.h

@@ -17,6 +17,8 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/resource_context.h"
 #include "electron/buildflags/buildflags.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "shell/browser/media/media_device_id_salt.h"
 
@@ -50,7 +52,8 @@ class WebViewManager;
 
 class AtomBrowserContext
     : public base::RefCountedDeleteOnSequence<AtomBrowserContext>,
-      public content::BrowserContext {
+      public content::BrowserContext,
+      public network::mojom::TrustedURLLoaderAuthClient {
  public:
   // partition_id => browser_context
   struct PartitionKey {
@@ -148,6 +151,10 @@ class AtomBrowserContext
   friend class base::RefCountedDeleteOnSequence<AtomBrowserContext>;
   friend class base::DeleteHelper<AtomBrowserContext>;
 
+  void OnLoaderCreated(int32_t request_id,
+                       mojo::PendingReceiver<network::mojom::TrustedAuthClient>
+                           header_client) override;
+
   // Initialize pref registry.
   void InitPrefs();
 
@@ -184,6 +191,7 @@ class AtomBrowserContext
 
   // Shared URLLoaderFactory.
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+  mojo::Receiver<network::mojom::TrustedURLLoaderAuthClient> auth_client_{this};
 
   base::WeakPtrFactory<AtomBrowserContext> weak_factory_;
 

shell/browser/login_handler.cc

@@ -10,11 +10,14 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/strings/string16.h"
+#include "native_mate/arguments.h"
 #include "native_mate/dictionary.h"
 #include "shell/browser/api/atom_api_web_contents.h"
-#include "shell/common/gin_converters/callback_converter_gin_adapter.h"
-#include "shell/common/gin_converters/net_converter_gin_adapter.h"
-#include "shell/common/gin_converters/value_converter_gin_adapter.h"
+#include "shell/common/native_mate_converters/callback.h"
+#include "shell/common/native_mate_converters/gurl_converter.h"
+#include "shell/common/native_mate_converters/net_converter.h"
+#include "shell/common/native_mate_converters/value_converter.h"
 
 using content::BrowserThread;
 
@@ -57,7 +60,7 @@ void LoginHandler::EmitEvent(
   v8::HandleScope scope(isolate);
 
   auto details = mate::Dictionary::CreateEmpty(isolate);
-  details.Set("url", url.spec());
+  details.Set("url", url);
 
   // These parameters aren't documented, and I'm not sure that they're useful,
   // but we might as well stick 'em on the details object. If it turns out they
@@ -77,9 +80,13 @@ void LoginHandler::EmitEvent(
 
 LoginHandler::~LoginHandler() = default;
 
-void LoginHandler::CallbackFromJS(base::string16 username,
-                                  base::string16 password) {
+void LoginHandler::CallbackFromJS(mate::Arguments* args) {
   if (auth_required_callback_) {
+    base::string16 username, password;
+    if (!args->GetNext(&username) || !args->GetNext(&password)) {
+      std::move(auth_required_callback_).Run(base::nullopt);
+      return;
+    }
     std::move(auth_required_callback_)
         .Run(net::AuthCredentials(username, password));
   }

shell/browser/login_handler.h

@@ -5,7 +5,6 @@
 #ifndef SHELL_BROWSER_LOGIN_HANDLER_H_
 #define SHELL_BROWSER_LOGIN_HANDLER_H_
 
-#include "base/strings/string16.h"
 #include "base/values.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/login_delegate.h"
@@ -15,6 +14,10 @@ namespace content {
 class WebContents;
 }
 
+namespace mate {
+class Arguments;
+}
+
 namespace electron {
 
 // Handles HTTP basic auth.
@@ -36,7 +39,7 @@ class LoginHandler : public content::LoginDelegate,
                  const GURL& url,
                  scoped_refptr<net::HttpResponseHeaders> response_headers,
                  bool first_auth_attempt);
-  void CallbackFromJS(base::string16 username, base::string16 password);
+  void CallbackFromJS(mate::Arguments* args);
 
   LoginAuthRequiredCallback auth_required_callback_;
 

spec-main/api-net-spec.ts

@@ -1,5 +1,5 @@
 import { expect } from 'chai'
-import { net, session, ClientRequest } from 'electron'
+import { net, session, ClientRequest, BrowserWindow } from 'electron'
 import * as http from 'http'
 import * as url from 'url'
 import { AddressInfo } from 'net'
@@ -193,6 +193,135 @@ describe('net module', () => {
         urlRequest.end()
       })
     })
+
+    it('should emit the login event when 401', async () => {
+      const [user, pass] = ['user', 'pass']
+      const serverUrl = await respondOnce.toSingleURL((request, response) => {
+        if (!request.headers.authorization) {
+          return response.writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' }).end()
+        }
+        response.writeHead(200).end('ok')
+      })
+      let loginAuthInfo: any
+      await new Promise((resolve, reject) => {
+        const request = net.request({ method: 'GET', url: serverUrl })
+        request.on('response', (response) => {
+          response.on('error', reject)
+          response.on('data', () => {})
+          response.on('end', () => resolve())
+        })
+        request.on('login', (authInfo, cb) => {
+          loginAuthInfo = authInfo
+          cb(user, pass)
+        })
+        request.on('error', reject)
+        request.end()
+      })
+      expect(loginAuthInfo.realm).to.equal('Foo')
+      expect(loginAuthInfo.scheme).to.equal('basic')
+    })
+
+    it('should produce an error on the response object when cancelling authentication', async () => {
+      const serverUrl = await respondOnce.toSingleURL((request, response) => {
+        if (!request.headers.authorization) {
+          return response.writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' }).end()
+        }
+        response.writeHead(200).end('ok')
+      })
+      await expect(new Promise((resolve, reject) => {
+        const request = net.request({ method: 'GET', url: serverUrl })
+        request.on('response', (response) => {
+          response.on('error', reject)
+          response.on('data', () => {})
+          response.on('end', () => resolve())
+        })
+        request.on('login', (authInfo, cb) => {
+          cb()
+        })
+        request.on('error', reject)
+        request.end()
+      })).to.eventually.be.rejectedWith('net::ERR_HTTP_RESPONSE_CODE_FAILURE')
+    })
+
+    it('should share credentials with WebContents', async () => {
+      const [user, pass] = ['user', 'pass']
+      const serverUrl = await new Promise<string>((resolve) => {
+        const server = http.createServer((request, response) => {
+          if (!request.headers.authorization) {
+            return response.writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' }).end()
+          }
+          return response.writeHead(200).end('ok')
+        })
+        server.listen(0, '127.0.0.1', () => {
+          resolve(`http://127.0.0.1:${(server.address() as AddressInfo).port}`)
+        })
+        after(() => { server.close() })
+      })
+      const bw = new BrowserWindow({ show: false })
+      const loaded = bw.loadURL(serverUrl)
+      bw.webContents.on('login', (event, details, authInfo, cb) => {
+        event.preventDefault()
+        cb(user, pass)
+      })
+      await loaded
+      bw.close()
+      await new Promise((resolve, reject) => {
+        const request = net.request({ method: 'GET', url: serverUrl })
+        request.on('response', (response) => {
+          response.on('error', reject)
+          response.on('data', () => {})
+          response.on('end', () => resolve())
+        })
+        request.on('login', () => {
+          // we shouldn't receive a login event, because the credentials should
+          // be cached.
+          reject(new Error('unexpected login event'))
+        })
+        request.on('error', reject)
+        request.end()
+      })
+    })
+
+    it('should share proxy credentials with WebContents', async () => {
+      const [user, pass] = ['user', 'pass']
+      const proxyPort = await new Promise<number>((resolve) => {
+        const server = http.createServer((request, response) => {
+          if (!request.headers['proxy-authorization']) {
+            return response.writeHead(407, { 'Proxy-Authenticate': 'Basic realm="Foo"' }).end()
+          }
+          return response.writeHead(200).end('ok')
+        })
+        server.listen(0, '127.0.0.1', () => {
+          resolve((server.address() as AddressInfo).port)
+        })
+        after(() => { server.close() })
+      })
+      const customSession = session.fromPartition(`net-proxy-test-${Math.random()}`)
+      await customSession.setProxy({ proxyRules: `127.0.0.1:${proxyPort}`, proxyBypassRules: '<-loopback>' } as any)
+      const bw = new BrowserWindow({ show: false, webPreferences: { session: customSession } })
+      const loaded = bw.loadURL('http://127.0.0.1:9999')
+      bw.webContents.on('login', (event, details, authInfo, cb) => {
+        event.preventDefault()
+        cb(user, pass)
+      })
+      await loaded
+      bw.close()
+      await new Promise((resolve, reject) => {
+        const request = net.request({ method: 'GET', url: 'http://127.0.0.1:9999', session: customSession })
+        request.on('response', (response) => {
+          response.on('error', reject)
+          response.on('data', () => {})
+          response.on('end', () => resolve())
+        })
+        request.on('login', () => {
+          // we shouldn't receive a login event, because the credentials should
+          // be cached.
+          reject(new Error('unexpected login event'))
+        })
+        request.on('error', reject)
+        request.end()
+      })
+    })
   })
 
   describe('ClientRequest API', () => {

spec-main/api-session-spec.js

@@ -526,6 +526,7 @@ describe('session module', () => {
           })
           response.on('error', (error) => { reject(new Error(error)) })
         });
+        request.on('error', (error) => { reject(new Error(error)) })
         request.end()
       })
       // the first time should throw due to unauthenticated

spec-main/api-web-contents-spec.ts

@@ -244,7 +244,7 @@ describe('webContents module', () => {
         }
         response
           .writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' })
-          .end()
+          .end('401')
       }).listen(0, '127.0.0.1', () => {
         serverPort = (server.address() as AddressInfo).port
         serverUrl = `http://127.0.0.1:${serverPort}`
@@ -267,6 +267,10 @@ describe('webContents module', () => {
       })
     })
 
+    afterEach(async () => {
+      await session.defaultSession.clearAuthCache({ type: 'password' })
+    })
+
     after(() => {
       server.close()
       proxyServer.close()
@@ -317,5 +321,16 @@ describe('webContents module', () => {
       expect(eventAuthInfo.port).to.equal(proxyServerPort)
       expect(eventAuthInfo.realm).to.equal('Foo')
     })
+
+    it('cancels authentication when callback is called with no arguments', async () => {
+      const w = new BrowserWindow({ show: false })
+      w.webContents.on('login', (event, request, authInfo, cb) => {
+        event.preventDefault()
+        cb()
+      })
+      await w.loadURL(serverUrl)
+      const body = await w.webContents.executeJavaScript(`document.documentElement.textContent`)
+      expect(body).to.equal('401')
+    })
   })
 })