electron · MarshallOfSound · Nov 26, 2019 · Nov 14, 2019 · Nov 15, 2019 · Nov 15, 2019
diff --git a/docs/api/app.md b/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.

diff --git a/docs/api/client-request.md b/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.
 

diff --git a/docs/api/web-contents.md b/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.
 

diff --git a/lib/browser/api/net.js b/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) {

@@ -85,3 +85,4 @@ build_win_fix_msstl_compatibility_for_pdf.patch
 remove_usage_of_incognito_apis_in_the_spellchecker.patch
 chore_use_electron_resources_not_chrome_for_spellchecker.patch
 fix_add_missing_include_algorithm_as_needed.patch
+add_trustedauthclient_to_urlloaderfactory.patch
@@ -0,0 +1,158 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <nornagon@nornagon.net>
+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 6b14d8354375377526e141ee499a7583be3f22b0..eeb9e19c0ecdf4631e596e7c0927693f2239f293 100644
+--- a/services/network/public/mojom/network_context.mojom
++++ b/services/network/public/mojom/network_context.mojom
+@@ -181,6 +181,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,
+@@ -559,6 +578,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 d4e13ffaed76847b00cf98b248ba17ad70a9884c..33ab3ea9c60e097d8525f1066f3890a5bccd754a 100644
+--- a/services/network/url_loader.cc
++++ b/services/network/url_loader.cc
+@@ -335,6 +335,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),
+@@ -391,6 +392,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;
+@@ -818,7 +824,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;
+   }
+@@ -834,10 +840,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 0a47148a52a46f8a6f12f503731623f87e15b173..db8ca018c7e99a1a1acea156b4d49a755b93cc09 100644
+--- a/services/network/url_loader.h
++++ b/services/network/url_loader.h
+@@ -85,6 +85,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;
+
+@@ -362,6 +363,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 7145e0e96550d554bb1df85bd79818ec9a45f7b1..53225eb1b0b7f1aa2498cecc8222f9f897ac364f 100644
+--- a/services/network/url_loader_factory.cc
++++ b/services/network/url_loader_factory.cc
+@@ -65,6 +65,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);
+@@ -209,6 +210,7 @@ void URLLoaderFactory::CreateLoaderAndStart(
+       resource_scheduler_client_, 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 7b143aa49be833ddf05b7b99bea19ee0b674b79c..6d1fbca87e3827c953fdac2cfb96806114d8aea9 100644
+--- a/services/network/url_loader_factory.h
++++ b/services/network/url_loader_factory.h
+@@ -71,6 +71,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_;
diff --git a/shell/browser/api/atom_api_url_request_ns.cc b/shell/browser/api/atom_api_url_request_ns.cc
@@ -6,13 +6,15 @@
 
 #include <utility>
 
+#include "base/containers/id_map.h"
 #include "gin/handle.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/system/string_data_source.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/gin_converters/callback_converter.h"
 #include "shell/common/gin_converters/gurl_converter.h"
 #include "shell/common/gin_converters/net_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
@@ -51,6 +53,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,
@@ -166,7 +173,9 @@ class ChunkedDataPipeGetter : public UploadDataPipeGetter,
   mojo::ReceiverSet<network::mojom::ChunkedDataPipeGetter> receiver_set_;
 };
 
-URLRequestNS::URLRequestNS(gin::Arguments* args) : weak_factory_(this) {
+URLRequestNS::URLRequestNS(gin::Arguments* args)
+    : id_(GetAllRequests().Add(this)), weak_factory_(this) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   request_ = std::make_unique<network::ResourceRequest>();
   gin_helper::Dictionary dict;
   if (args->GetNext(&dict)) {
@@ -176,6 +185,8 @@ URLRequestNS::URLRequestNS(gin::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)) {
@@ -192,6 +203,40 @@ URLRequestNS::URLRequestNS(gin::Arguments* args) : weak_factory_(this) {
 
 URLRequestNS::~URLRequestNS() = default;
 
+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,
+         gin::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));
+  v8::Local<v8::Value> cb_v8 = gin::ConvertToV8(isolate(), std::move(cb));
+  v8::Local<v8::Value> auth_info_v8 = gin::ConvertToV8(isolate(), auth_info);
+  Emit("login", auth_info_v8, cb_v8);
+}
+
 bool URLRequestNS::NotStarted() const {
   return request_state_ == 0;
 }
@@ -512,11 +557,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());
-  gin_helper::CustomEmit(isolate(), GetWrapper(), method, args...);
+  gin_helper::CustomEmit(isolate(), GetWrapper(), method,
+                         std::forward<Args>(args)...);
 }
 
 // static

diff --git a/shell/browser/api/atom_api_url_request_ns.h b/shell/browser/api/atom_api_url_request_ns.h
@@ -17,6 +17,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_deprecated.h"
 
 namespace electron {
@@ -32,6 +33,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(gin::Arguments* args);
@@ -89,7 +99,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_;
@@ -132,6 +142,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);