From 4f1536479e115b506d3ce5ee3083f98131f9ba1d Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Thu, 14 Nov 2019 10:01:18 -0800 Subject: [PATCH] fix: implement 'login' event for net.ClientRequest (#21096) --- docs/api/app.md | 8 +- docs/api/client-request.md | 4 +- docs/api/web-contents.md | 4 +- lib/browser/api/net.js | 21 +-- patches/chromium/.patches | 1 + ...rustedauthclient_to_urlloaderfactory.patch | 158 ++++++++++++++++++ shell/browser/api/atom_api_url_request.cc | 50 +++++- shell/browser/api/atom_api_url_request.h | 14 +- shell/browser/atom_browser_context.cc | 36 ++++ shell/browser/atom_browser_context.h | 9 +- shell/browser/login_handler.cc | 21 ++- shell/browser/login_handler.h | 7 +- shell/browser/net/asar/asar_url_loader.cc | 1 - spec-main/api-net-spec.ts | 131 ++++++++++++++- spec-main/api-session-spec.ts | 1 + spec-main/api-web-contents-spec.ts | 17 +- 16 files changed, 445 insertions(+), 38 deletions(-) create mode 100644 patches/chromium/add_trustedauthclient_to_urlloaderfactory.patch diff --git a/docs/api/app.md b/docs/api/app.md index eccb8a9b748a..42a5b19e7436 100644 --- 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 index 382fd04d3817..f5247883082d 100644 --- 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 index bd8c3568a4b8..163a0f9ac462 100644 --- 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 index 379245b4c9bf..7168c1b512f0 100644 --- 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) { diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 11250b07cadb..dc07521857ec 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -86,3 +86,4 @@ fix_ambiguous_reference_to_data.patch backport_fix_msstl_compat_in_ui_events.patch build_win_fix_msstl_compatibility_for_pdf.patch fix_missing_algorithm_include.patch +add_trustedauthclient_to_urlloaderfactory.patch diff --git a/patches/chromium/add_trustedauthclient_to_urlloaderfactory.patch b/patches/chromium/add_trustedauthclient_to_urlloaderfactory.patch new file mode 100644 index 000000000000..8cc66b7e4f42 --- /dev/null +++ b/patches/chromium/add_trustedauthclient_to_urlloaderfactory.patch @@ -0,0 +1,158 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Jeremy Apthorp +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 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 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 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? header_client; + ++ pending_remote? 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 keepalive_statistics_recorder, + base::WeakPtr 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 keepalive_statistics_recorder, + base::WeakPtr 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 fetch_window_id_; + + mojo::Remote header_client_; ++ mojo::Remote auth_client_; + + std::unique_ptr 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 resource_scheduler_client_; + mojo::Remote header_client_; ++ mojo::Remote auth_client_; + + // |cors_url_loader_factory_| owns this. + cors::CorsURLLoaderFactory* cors_url_loader_factory_; diff --git a/shell/browser/api/atom_api_url_request.cc b/shell/browser/api/atom_api_url_request.cc index 4fb2d91bfc56..6c5b6d191b57 100644 --- a/shell/browser/api/atom_api_url_request.cc +++ b/shell/browser/api/atom_api_url_request.cc @@ -6,6 +6,7 @@ #include +#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" @@ -13,6 +14,7 @@ #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& GetAllRequests() { + static base::NoDestructor> 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 receiver_set_; }; -URLRequest::URLRequest(gin::Arguments* args) : weak_factory_(this) { +URLRequest::URLRequest(gin::Arguments* args) + : id_(GetAllRequests().Add(this)), weak_factory_(this) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); request_ = std::make_unique(); gin_helper::Dictionary dict; if (args->GetNext(&dict)) { @@ -176,6 +185,8 @@ URLRequest::URLRequest(gin::Arguments* args) : weak_factory_(this) { request_->redirect_mode = redirect_mode_; } + request_->render_frame_id = id_; + std::string partition; gin::Handle session; if (!dict.Get("session", &session)) { @@ -192,6 +203,38 @@ URLRequest::URLRequest(gin::Arguments* args) : weak_factory_(this) { URLRequest::~URLRequest() = default; +URLRequest* URLRequest::FromID(uint32_t id) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + return GetAllRequests().Lookup(id); +} + +void URLRequest::OnAuthRequired( + const GURL& url, + bool first_auth_attempt, + net::AuthChallengeInfo auth_info, + network::mojom::URLResponseHeadPtr head, + mojo::PendingRemote + auth_challenge_responder) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + mojo::Remote auth_responder( + std::move(auth_challenge_responder)); + auth_responder.set_disconnect_handler( + base::BindOnce(&URLRequest::Cancel, weak_factory_.GetWeakPtr())); + auto cb = base::BindOnce( + [](mojo::Remote 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)); + Emit("login", auth_info, base::AdaptCallbackForRepeating(std::move(cb))); +} + bool URLRequest::NotStarted() const { return request_state_ == 0; } @@ -512,11 +555,12 @@ void URLRequest::EmitError(EventType type, base::StringPiece message) { } template -void URLRequest::EmitEvent(EventType type, Args... args) { +void URLRequest::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)...); } // static diff --git a/shell/browser/api/atom_api_url_request.h b/shell/browser/api/atom_api_url_request.h index f8a276ea7d60..36dd349685e0 100644 --- a/shell/browser/api/atom_api_url_request.h +++ b/shell/browser/api/atom_api_url_request.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/common/gin_helper/event_emitter.h" namespace electron { @@ -32,6 +33,15 @@ class URLRequest : public gin_helper::EventEmitter, static void BuildPrototype(v8::Isolate* isolate, v8::Local prototype); + static URLRequest* FromID(uint32_t id); + + void OnAuthRequired( + const GURL& url, + bool first_auth_attempt, + net::AuthChallengeInfo auth_info, + network::mojom::URLResponseHeadPtr head, + mojo::PendingRemote + auth_challenge_responder); protected: explicit URLRequest(gin::Arguments* args); @@ -89,7 +99,7 @@ class URLRequest : public gin_helper::EventEmitter, }; void EmitError(EventType type, base::StringPiece error); template - void EmitEvent(EventType type, Args... args); + void EmitEvent(EventType type, Args&&... args); std::unique_ptr request_; std::unique_ptr loader_; @@ -132,6 +142,8 @@ class URLRequest : public gin_helper::EventEmitter, // Used by pin/unpin to manage lifetime. v8::Global wrapper_; + uint32_t id_; + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(URLRequest); diff --git a/shell/browser/atom_browser_context.cc b/shell/browser/atom_browser_context.cc index aa83f66ed592..faa5fdf9c84a 100644 --- a/shell/browser/atom_browser_context.cc +++ b/shell/browser/atom_browser_context.cc @@ -28,10 +28,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.h" #include "shell/browser/atom_browser_client.h" #include "shell/browser/atom_browser_main_parts.h" #include "shell/browser/atom_download_manager_delegate.h" @@ -325,6 +327,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_trusted = true; params->is_corb_enabled = false; @@ -342,6 +345,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 + auth_challenge_responder) override { + api::URLRequest* url_request = api::URLRequest::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 auth_client) { + mojo::MakeSelfOwnedReceiver(std::make_unique(), + std::move(auth_client)); +} + content::PushMessagingService* AtomBrowserContext::GetPushMessagingService() { return nullptr; } diff --git a/shell/browser/atom_browser_context.h b/shell/browser/atom_browser_context.h index 76ec59b6a0cf..0ea9efdedcf2 100644 --- a/shell/browser/atom_browser_context.h +++ b/shell/browser/atom_browser_context.h @@ -17,6 +17,7 @@ #include "content/public/browser/browser_context.h" #include "content/public/browser/resource_context.h" #include "electron/buildflags/buildflags.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 +51,8 @@ class WebViewManager; class AtomBrowserContext : public base::RefCountedDeleteOnSequence, - public content::BrowserContext { + public content::BrowserContext, + public network::mojom::TrustedURLLoaderAuthClient { public: // partition_id => browser_context struct PartitionKey { @@ -149,6 +151,10 @@ class AtomBrowserContext friend class base::RefCountedDeleteOnSequence; friend class base::DeleteHelper; + void OnLoaderCreated(int32_t request_id, + mojo::PendingReceiver + header_client) override; + // Initialize pref registry. void InitPrefs(); @@ -185,6 +191,7 @@ class AtomBrowserContext // Shared URLLoaderFactory. scoped_refptr url_loader_factory_; + mojo::Receiver auth_client_{this}; base::WeakPtrFactory weak_factory_; diff --git a/shell/browser/login_handler.cc b/shell/browser/login_handler.cc index 872ea5ef32e2..9a84324630a0 100644 --- a/shell/browser/login_handler.cc +++ b/shell/browser/login_handler.cc @@ -10,9 +10,12 @@ #include #include "base/callback.h" +#include "base/strings/string16.h" +#include "gin/arguments.h" #include "gin/dictionary.h" #include "shell/browser/api/atom_api_web_contents.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_converters/value_converter.h" @@ -57,7 +60,7 @@ void LoginHandler::EmitEvent( v8::HandleScope scope(isolate); auto details = gin::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 @@ -70,17 +73,23 @@ void LoginHandler::EmitEvent( api_web_contents->Emit("login", std::move(details), auth_info, base::BindOnce(&LoginHandler::CallbackFromJS, weak_factory_.GetWeakPtr())); - if (!default_prevented) { + if (!default_prevented && auth_required_callback_) { std::move(auth_required_callback_).Run(base::nullopt); } } LoginHandler::~LoginHandler() = default; -void LoginHandler::CallbackFromJS(base::string16 username, - base::string16 password) { - std::move(auth_required_callback_) - .Run(net::AuthCredentials(username, password)); +void LoginHandler::CallbackFromJS(gin::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)); + } } } // namespace electron diff --git a/shell/browser/login_handler.h b/shell/browser/login_handler.h index 6277adae52fd..3164b46198f8 100644 --- a/shell/browser/login_handler.h +++ b/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 gin { +class Arguments; +} + namespace electron { // Handles HTTP basic auth. @@ -36,7 +39,7 @@ class LoginHandler : public content::LoginDelegate, const GURL& url, scoped_refptr response_headers, bool first_auth_attempt); - void CallbackFromJS(base::string16 username, base::string16 password); + void CallbackFromJS(gin::Arguments* args); LoginAuthRequiredCallback auth_required_callback_; diff --git a/shell/browser/net/asar/asar_url_loader.cc b/shell/browser/net/asar/asar_url_loader.cc index 32aeaa3ecec8..f7c22b153c8b 100644 --- a/shell/browser/net/asar/asar_url_loader.cc +++ b/shell/browser/net/asar/asar_url_loader.cc @@ -13,7 +13,6 @@ #include "base/task/post_task.h" #include "content/public/browser/file_url_loader.h" #include "mojo/public/cpp/bindings/receiver.h" -#include "mojo/public/cpp/bindings/strong_binding.h" #include "mojo/public/cpp/system/data_pipe_producer.h" #include "mojo/public/cpp/system/file_data_source.h" #include "net/base/filename_util.h" diff --git a/spec-main/api-net-spec.ts b/spec-main/api-net-spec.ts index 8aeb212c4737..85f8cea0e016 100644 --- a/spec-main/api-net-spec.ts +++ b/spec-main/api-net-spec.ts @@ -1,5 +1,5 @@ import { expect } from 'chai' -import { net as originalNet, session, ClientRequest } from 'electron' +import { net as originalNet, session, ClientRequest, BrowserWindow } from 'electron' import * as http from 'http' import * as url from 'url' import { AddressInfo } from 'net' @@ -206,6 +206,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((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((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>' }) + 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', () => { diff --git a/spec-main/api-session-spec.ts b/spec-main/api-session-spec.ts index ce338c415958..899b158f80c0 100644 --- a/spec-main/api-session-spec.ts +++ b/spec-main/api-session-spec.ts @@ -520,6 +520,7 @@ describe('session module', () => { }) response.on('error', (error: any) => { reject(new Error(error)) }) }) + request.on('error', (error: any) => { reject(new Error(error)) }) request.end() }) // the first time should throw due to unauthenticated diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts index 466ef6d9cedc..eb77f337d31e 100644 --- a/spec-main/api-web-contents-spec.ts +++ b/spec-main/api-web-contents-spec.ts @@ -1534,7 +1534,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}` @@ -1557,6 +1557,10 @@ describe('webContents module', () => { }) }) + afterEach(async () => { + await session.defaultSession.clearAuthCache({ type: 'password' }) + }) + after(() => { server.close() proxyServer.close() @@ -1607,5 +1611,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') + }) }) })