fix: implement 'login' event for WebContents (electron#20954)

nornagon · web-flow · commit 034f4d5734bb · 2019-11-11T09:47:01.000-08:00
diff --git a/docs/api/app.md b/docs/api/app.md
@@ -314,10 +314,8 @@ Returns:
 
 * `event` Event
 * `webContents` [WebContents](web-contents.md)
-* `request` Object
-  * `method` String
+* `authenticationResponseDetails` Object
   * `url` URL
-  * `referrer` URL
 * `authInfo` Object
   * `isProxy` Boolean
   * `scheme` String
@@ -337,7 +335,7 @@ should prevent the default behavior with `event.preventDefault()` and call
 ```javascript
 const { app } = require('electron')
 
-app.on('login', (event, webContents, request, authInfo, callback) => {
+app.on('login', (event, webContents, details, authInfo, callback) => {
   event.preventDefault()
   callback('username', 'secret')
 })
diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md
@@ -454,10 +454,8 @@ The usage is the same with [the `select-client-certificate` event of
 Returns:
 
 * `event` Event
-* `request` Object
-  * `method` String
+* `authenticationResponseDetails` Object
   * `url` URL
-  * `referrer` URL
 * `authInfo` Object
   * `isProxy` Boolean
   * `scheme` String
diff --git a/lib/browser/api/app.ts b/lib/browser/api/app.ts
@@ -105,9 +105,9 @@ if (process.platform === 'linux') {
 }
 
 // Routes the events to webContents.
-const events = ['login', 'certificate-error', 'select-client-certificate']
+const events = ['certificate-error', 'select-client-certificate']
 for (const name of events) {
-  app.on(name as 'login', (event, webContents, ...args: any[]) => {
+  app.on(name as 'certificate-error', (event, webContents, ...args: any[]) => {
     webContents.emit(name, event, ...args)
   })
 }
diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js
@@ -423,6 +423,10 @@ WebContents.prototype._init = function () {
     })
   }
 
+  this.on('login', (event, ...args) => {
+    app.emit('login', event, this, ...args)
+  })
+
   const event = process.electronBinding('event').createEmpty()
   app.emit('web-contents-created', event, this)
 }
diff --git a/shell/browser/api/atom_api_app.cc b/shell/browser/api/atom_api_app.cc
@@ -495,15 +495,6 @@ void OnClientCertificateSelected(
   }
 }
 
-void PassLoginInformation(scoped_refptr<LoginHandler> login_handler,
-                          gin_helper::Arguments* args) {
-  base::string16 username, password;
-  if (args->GetNext(&username) && args->GetNext(&password))
-    login_handler->Login(username, password);
-  else
-    login_handler->CancelAuth();
-}
-
 #if defined(USE_NSS_CERTS)
 int ImportIntoCertStore(CertificateManagerModel* model,
                         const base::DictionaryValue& options) {
@@ -667,25 +658,6 @@ void App::OnNewWindowForTab() {
 }
 #endif
 
-void App::OnLogin(scoped_refptr<LoginHandler> login_handler,
-                  const base::DictionaryValue& request_details) {
-  v8::Locker locker(isolate());
-  v8::HandleScope handle_scope(isolate());
-  bool prevent_default = false;
-  content::WebContents* web_contents = login_handler->GetWebContents();
-  if (web_contents) {
-    prevent_default =
-        Emit("login", WebContents::FromOrCreate(isolate(), web_contents),
-             request_details, *login_handler->auth_info(),
-             base::BindOnce(&PassLoginInformation,
-                            base::RetainedRef(login_handler)));
-  }
-
-  // Default behavior is to always cancel the auth.
-  if (!prevent_default)
-    login_handler->CancelAuth();
-}
-
 bool App::CanCreateWindow(
     content::RenderFrameHost* opener,
     const GURL& opener_url,
diff --git a/shell/browser/api/atom_api_app.h b/shell/browser/api/atom_api_app.h
@@ -86,8 +86,6 @@ class App : public AtomBrowserClient::Delegate,
   void OnActivate(bool has_visible_windows) override;
   void OnWillFinishLaunching() override;
   void OnFinishLaunching(const base::DictionaryValue& launch_info) override;
-  void OnLogin(scoped_refptr<LoginHandler> login_handler,
-               const base::DictionaryValue& request_details) override;
   void OnAccessibilitySupportChanged() override;
   void OnPreMainMessageLoopRun() override;
 #if defined(OS_MACOSX)
diff --git a/shell/browser/atom_browser_client.cc b/shell/browser/atom_browser_client.cc
@@ -30,6 +30,7 @@
 #include "content/public/browser/browser_ppapi_host.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/client_certificate_delegate.h"
+#include "content/public/browser/login_delegate.h"
 #include "content/public/browser/overlay_window.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -1104,4 +1105,18 @@ void AtomBrowserClient::BindHostReceiverForRenderer(
 #endif
 }
 
+std::unique_ptr<content::LoginDelegate> AtomBrowserClient::CreateLoginDelegate(
+    const net::AuthChallengeInfo& auth_info,
+    content::WebContents* web_contents,
+    const content::GlobalRequestID& request_id,
+    bool is_main_frame,
+    const GURL& url,
+    scoped_refptr<net::HttpResponseHeaders> response_headers,
+    bool first_auth_attempt,
+    LoginAuthRequiredCallback auth_required_callback) {
+  return std::make_unique<LoginHandler>(
+      auth_info, web_contents, is_main_frame, url, response_headers,
+      first_auth_attempt, std::move(auth_required_callback));
+}
+
 }  // namespace electron
diff --git a/shell/browser/atom_browser_client.h b/shell/browser/atom_browser_client.h
@@ -211,6 +211,15 @@ class AtomBrowserClient : public content::ContentBrowserClient,
       const base::Optional<url::Origin>& initiating_origin,
       mojo::PendingRemote<network::mojom::URLLoaderFactory>* out_factory)
       override;
+  std::unique_ptr<content::LoginDelegate> CreateLoginDelegate(
+      const net::AuthChallengeInfo& auth_info,
+      content::WebContents* web_contents,
+      const content::GlobalRequestID& request_id,
+      bool is_main_frame,
+      const GURL& url,
+      scoped_refptr<net::HttpResponseHeaders> response_headers,
+      bool first_auth_attempt,
+      LoginAuthRequiredCallback auth_required_callback) override;
 
   // content::RenderProcessHostObserver:
   void RenderProcessHostDestroyed(content::RenderProcessHost* host) override;
diff --git a/shell/browser/browser.cc b/shell/browser/browser.cc
@@ -181,13 +181,6 @@ void Browser::OnAccessibilitySupportChanged() {
     observer.OnAccessibilitySupportChanged();
 }
 
-void Browser::RequestLogin(
-    scoped_refptr<LoginHandler> login_handler,
-    std::unique_ptr<base::DictionaryValue> request_details) {
-  for (BrowserObserver& observer : observers_)
-    observer.OnLogin(login_handler, *(request_details.get()));
-}
-
 void Browser::PreMainMessageLoopRun() {
   for (BrowserObserver& observer : observers_) {
     observer.OnPreMainMessageLoopRun();
diff --git a/shell/browser/browser.h b/shell/browser/browser.h
@@ -250,10 +250,6 @@ class Browser : public WindowListObserver {
 
   void OnAccessibilitySupportChanged();
 
-  // Request basic auth login.
-  void RequestLogin(scoped_refptr<LoginHandler> login_handler,
-                    std::unique_ptr<base::DictionaryValue> request_details);
-
   void PreMainMessageLoopRun();
 
   // Stores the supplied |quit_closure|, to be run when the last Browser
diff --git a/shell/browser/browser_observer.h b/shell/browser/browser_observer.h
@@ -49,10 +49,6 @@ class BrowserObserver : public base::CheckedObserver {
   virtual void OnWillFinishLaunching() {}
   virtual void OnFinishLaunching(const base::DictionaryValue& launch_info) {}
 
-  // The browser requests HTTP login.
-  virtual void OnLogin(scoped_refptr<LoginHandler> login_handler,
-                       const base::DictionaryValue& request_details) {}
-
   // The browser's accessibility suppport has changed.
   virtual void OnAccessibilitySupportChanged() {}
 
diff --git a/shell/browser/login_handler.cc b/shell/browser/login_handler.cc
@@ -9,157 +9,78 @@
 #include <utility>
 #include <vector>
 
-#include "base/task/post_task.h"
-#include "base/values.h"
-#include "content/public/browser/browser_task_traits.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/web_contents.h"
-#include "net/base/auth.h"
-#include "net/base/upload_bytes_element_reader.h"
-#include "net/base/upload_data_stream.h"
-#include "net/base/upload_element_reader.h"
-#include "net/base/upload_file_element_reader.h"
-#include "shell/browser/browser.h"
+#include "base/callback.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/net_converter.h"
+#include "shell/common/gin_converters/value_converter.h"
 
 using content::BrowserThread;
 
 namespace electron {
 
-namespace {
-
-void GetUploadData(base::ListValue* upload_data_list,
-                   const net::URLRequest* request) {
-  const net::UploadDataStream* upload_data = request->get_upload_for_testing();
-  if (!upload_data)
-    return;
-  const std::vector<std::unique_ptr<net::UploadElementReader>>* readers =
-      upload_data->GetElementReaders();
-  for (const auto& reader : *readers) {
-    auto upload_data_dict = std::make_unique<base::DictionaryValue>();
-    if (reader->AsBytesReader()) {
-      const net::UploadBytesElementReader* bytes_reader =
-          reader->AsBytesReader();
-      auto bytes = std::make_unique<base::Value>(
-          std::vector<char>(bytes_reader->bytes(),
-                            bytes_reader->bytes() + bytes_reader->length()));
-      upload_data_dict->Set("bytes", std::move(bytes));
-    } else if (reader->AsFileReader()) {
-      const net::UploadFileElementReader* file_reader = reader->AsFileReader();
-      auto file_path = file_reader->path().AsUTF8Unsafe();
-      upload_data_dict->SetKey("file", base::Value(file_path));
-    }
-    // else {
-    //   const storage::UploadBlobElementReader* blob_reader =
-    //       static_cast<storage::UploadBlobElementReader*>(reader.get());
-    //   upload_data_dict->SetString("blobUUID", blob_reader->uuid());
-    // }
-    upload_data_list->Append(std::move(upload_data_dict));
-  }
-}
-
-void FillRequestDetails(base::DictionaryValue* details,
-                        const net::URLRequest* request) {
-  details->SetString("method", request->method());
-  std::string url;
-  if (!request->url_chain().empty())
-    url = request->url().spec();
-  details->SetKey("url", base::Value(url));
-  details->SetString("referrer", request->referrer());
-  auto list = std::make_unique<base::ListValue>();
-  GetUploadData(list.get(), request);
-  if (!list->empty())
-    details->Set("uploadData", std::move(list));
-  auto headers_value = std::make_unique<base::DictionaryValue>();
-  for (net::HttpRequestHeaders::Iterator it(request->extra_request_headers());
-       it.GetNext();) {
-    headers_value->SetString(it.name(), it.value());
-  }
-  details->Set("headers", std::move(headers_value));
-}
-
-}  // namespace
-
-LoginHandler::LoginHandler(net::URLRequest* request,
-                           const net::AuthChallengeInfo& auth_info,
-                           // net::NetworkDelegate::AuthCallback callback,
-                           net::AuthCredentials* credentials)
-    : credentials_(credentials),
-      auth_info_(std::make_unique<net::AuthChallengeInfo>(auth_info)),
-      // auth_callback_(std::move(callback)),
-      weak_factory_(this) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  std::unique_ptr<base::DictionaryValue> request_details(
-      new base::DictionaryValue);
-  // TODO(zcbenz): Use the converters from net_converter.
-  FillRequestDetails(request_details.get(), request);
-
-  // TODO(deepak1556): fix with network service
-  // tracking issue: #19602
-  CHECK(false) << "fix with network service";
-  // web_contents_getter_ =
-  //     resource_request_info->GetWebContentsGetterForRequest();
-
-  base::PostTask(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(&Browser::RequestLogin, base::Unretained(Browser::Get()),
-                     base::RetainedRef(this), std::move(request_details)));
-}
-
-LoginHandler::~LoginHandler() = default;
-
-void LoginHandler::Login(const base::string16& username,
-                         const base::string16& password) {
+LoginHandler::LoginHandler(
+    const net::AuthChallengeInfo& auth_info,
+    content::WebContents* web_contents,
+    bool is_main_frame,
+    const GURL& url,
+    scoped_refptr<net::HttpResponseHeaders> response_headers,
+    bool first_auth_attempt,
+    LoginAuthRequiredCallback auth_required_callback)
+
+    : WebContentsObserver(web_contents),
+      auth_required_callback_(std::move(auth_required_callback)) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&LoginHandler::DoLogin, weak_factory_.GetWeakPtr(),
-                     username, password));
+      FROM_HERE, {base::CurrentThread()},
+      base::BindOnce(&LoginHandler::EmitEvent, weak_factory_.GetWeakPtr(),
+                     auth_info, is_main_frame, url, response_headers,
+                     first_auth_attempt));
 }
 
-void LoginHandler::CancelAuth() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+void LoginHandler::EmitEvent(
+    net::AuthChallengeInfo auth_info,
+    bool is_main_frame,
+    const GURL& url,
+    scoped_refptr<net::HttpResponseHeaders> response_headers,
+    bool first_auth_attempt) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+
+  auto api_web_contents = api::WebContents::From(isolate, web_contents());
+  if (api_web_contents.IsEmpty()) {
+    std::move(auth_required_callback_).Run(base::nullopt);
+    return;
+  }
 
-  base::PostTask(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&LoginHandler::DoCancelAuth, weak_factory_.GetWeakPtr()));
-}
+  v8::HandleScope scope(isolate);
 
-void LoginHandler::NotifyRequestDestroyed() {
-  // auth_callback_.Reset();
-  credentials_ = nullptr;
-  weak_factory_.InvalidateWeakPtrs();
-}
+  auto details = gin::Dictionary::CreateEmpty(isolate);
+  details.Set("url", url.spec());
 
-content::WebContents* LoginHandler::GetWebContents() const {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  // TODO(deepak1556): fix with network service
-  // tracking issue: #19602
-  CHECK(false) << "fix with network service";
-  return web_contents_getter_.Run();
-}
+  // 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
+  // are useful, we can add them to the docs :)
+  details.Set("isMainFrame", is_main_frame);
+  details.Set("firstAuthAttempt", first_auth_attempt);
+  details.Set("responseHeaders", response_headers.get());
 
-void LoginHandler::DoCancelAuth() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  /*
-  if (!auth_callback_.is_null())
-    std::move(auth_callback_)
-        .Run(net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_CANCEL_AUTH);
-        */
+  bool default_prevented =
+      api_web_contents->Emit("login", std::move(details), auth_info,
+                             base::BindOnce(&LoginHandler::CallbackFromJS,
+                                            weak_factory_.GetWeakPtr()));
+  if (!default_prevented) {
+    std::move(auth_required_callback_).Run(base::nullopt);
+  }
 }
 
-void LoginHandler::DoLogin(const base::string16& username,
-                           const base::string16& password) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  /*
-  if (!auth_callback_.is_null()) {
-    credentials_->Set(username, password);
-    std::move(auth_callback_)
-        .Run(net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_SET_AUTH);
-  }
-  */
+LoginHandler::~LoginHandler() = default;
+
+void LoginHandler::CallbackFromJS(base::string16 username,
+                                  base::string16 password) {
+  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
diff --git a/spec-main/api-app-spec.ts b/spec-main/api-app-spec.ts
diff --git a/spec-main/api-session-spec.ts b/spec-main/api-session-spec.ts
diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts

Original file line number	Diff line number	Diff line change
`@@ -105,9 +105,9 @@ if (process.platform === 'linux') {`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`// Routes the events to webContents.`
`108`		`-const events = ['login', 'certificate-error', 'select-client-certificate']`
	`108`	`+const events = ['certificate-error', 'select-client-certificate']`
`109`	`109`	`for (const name of events) {`
`110`		`- app.on(name as 'login', (event, webContents, ...args: any[]) => {`
	`110`	`+ app.on(name as 'certificate-error', (event, webContents, ...args: any[]) => {`
`111`	`111`	`webContents.emit(name, event, ...args)`
`112`	`112`	`})`
`113`	`113`	`}`
Original file line number	Diff line number	Diff line change
`@@ -423,6 +423,10 @@ WebContents.prototype._init = function () {`
`423`	`423`	`})`
`424`	`424`	`}`
`425`	`425`
	`426`	`+ this.on('login', (event, ...args) => {`
	`427`	`+ app.emit('login', event, this, ...args)`
	`428`	`+ })`
	`429`	`+`
`426`	`430`	`const event = process.electronBinding('event').createEmpty()`
`427`	`431`	`app.emit('web-contents-created', event, this)`
`428`	`432`	`}`