fix: implement login event for WebContents (#21098)

* fix: implement login event for WebContents

* fix gin header path

* use mate

* correct path to native_mate/dictionary.h

* use BindRepeating bc apparently no BindOnce converter...?

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')
 })

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

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)
   })
 }

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)
 }

shell/browser/api/atom_api_app.cc

@@ -498,15 +498,6 @@ void OnClientCertificateSelected(
   }
 }
 
-void PassLoginInformation(scoped_refptr<LoginHandler> login_handler,
-                          mate::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) {
@@ -669,25 +660,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,

shell/browser/api/atom_api_app.h

@@ -90,8 +90,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)

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

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;

shell/browser/browser.cc

@@ -180,13 +180,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();

shell/browser/browser.h

@@ -247,10 +247,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

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() {}
 

shell/browser/login_handler.cc

@@ -5,100 +5,84 @@
 #include "shell/browser/login_handler.h"
 
 #include <memory>
+#include <string>
 #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 "shell/browser/browser.h"
+#include "base/callback.h"
+#include "native_mate/dictionary.h"
+#include "shell/browser/api/atom_api_web_contents.h"
+#include "shell/common/native_mate_converters/callback_converter_deprecated.h"
 #include "shell/common/native_mate_converters/net_converter.h"
+#include "shell/common/native_mate_converters/value_converter.h"
 
 using content::BrowserThread;
 
 namespace electron {
 
-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);
-  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 = mate::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::BindRepeating(&LoginHandler::CallbackFromJS,
+                                                 weak_factory_.GetWeakPtr()));
+  if (!default_prevented && auth_required_callback_) {
+    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) {
+  if (auth_required_callback_) {
+    std::move(auth_required_callback_)
+        .Run(net::AuthCredentials(username, password));
   }
-  */
 }
 
 }  // namespace electron

shell/browser/login_handler.h

@@ -5,15 +5,11 @@
 #ifndef SHELL_BROWSER_LOGIN_HANDLER_H_
 #define SHELL_BROWSER_LOGIN_HANDLER_H_
 
-#include <memory>
-
-#include "base/callback.h"
-#include "base/memory/ref_counted.h"
-#include "base/memory/weak_ptr.h"
-#include "base/sequenced_task_runner_helpers.h"
 #include "base/strings/string16.h"
-#include "content/public/browser/web_contents.h"
-#include "net/base/network_delegate.h"
+#include "base/values.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/login_delegate.h"
+#include "content/public/browser/web_contents_observer.h"
 
 namespace content {
 class WebContents;
@@ -21,52 +17,30 @@ class WebContents;
 
 namespace electron {
 
-// Handles the HTTP basic auth, must be created on IO thread.
-class LoginHandler : public base::RefCountedThreadSafe<LoginHandler> {
+// Handles HTTP basic auth.
+class LoginHandler : public content::LoginDelegate,
+                     public content::WebContentsObserver {
  public:
-  LoginHandler(net::URLRequest* request,
-               const net::AuthChallengeInfo& auth_info,
-               // net::NetworkDelegate::AuthCallback callback,
-               net::AuthCredentials* credentials);
-
-  // The auth is cancelled, must be called on UI thread.
-  void CancelAuth();
-
-  // The URLRequest associated with the auth is destroyed.
-  void NotifyRequestDestroyed();
-
-  // Login with |username| and |password|, must be called on UI thread.
-  void Login(const base::string16& username, const base::string16& password);
-
-  // Returns the WebContents associated with the request, must be called on UI
-  // thread.
-  content::WebContents* GetWebContents() const;
-
-  const net::AuthChallengeInfo* auth_info() const { return auth_info_.get(); }
+  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);
+  ~LoginHandler() override;
 
  private:
-  friend class base::RefCountedThreadSafe<LoginHandler>;
-  friend class base::DeleteHelper<LoginHandler>;
-
-  ~LoginHandler();
-
-  // Must be called on IO thread.
-  void DoCancelAuth();
-  void DoLogin(const base::string16& username, const base::string16& password);
-
-  // Credentials to be used for the auth.
-  net::AuthCredentials* credentials_;
-
-  // Who/where/what asked for the authentication.
-  std::unique_ptr<const net::AuthChallengeInfo> auth_info_;
-
-  // WebContents associated with the login request.
-  content::WebContents::Getter web_contents_getter_;
+  void EmitEvent(net::AuthChallengeInfo auth_info,
+                 bool is_main_frame,
+                 const GURL& url,
+                 scoped_refptr<net::HttpResponseHeaders> response_headers,
+                 bool first_auth_attempt);
+  void CallbackFromJS(base::string16 username, base::string16 password);
 
-  // Called with preferred value of net::NetworkDelegate::AuthRequiredResponse.
-  // net::NetworkDelegate::AuthCallback auth_callback_;
+  LoginAuthRequiredCallback auth_required_callback_;
 
-  base::WeakPtrFactory<LoginHandler> weak_factory_;
+  base::WeakPtrFactory<LoginHandler> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(LoginHandler);
 };

spec-main/api-app-spec.ts

@@ -2,6 +2,7 @@ import * as chai from 'chai'
 import * as chaiAsPromised from 'chai-as-promised'
 import * as cp from 'child_process'
 import * as https from 'https'
+import * as http from 'http'
 import * as net from 'net'
 import * as fs from 'fs'
 import * as path from 'path'
@@ -9,7 +10,7 @@ import { homedir } from 'os'
 import split = require('split')
 import { app, BrowserWindow, Menu } from 'electron'
 import { emittedOnce } from './events-helpers';
-import { closeWindow } from './window-helpers';
+import { closeWindow, closeAllWindows } from './window-helpers';
 import { ifdescribe } from './spec-helpers';
 
 const features = process.electronBinding('features')
@@ -1460,6 +1461,33 @@ describe('default behavior', () => {
       expect(output[0]).to.equal(output[1])
     })
   })
+
+  describe('login event', () => {
+    afterEach(closeAllWindows)
+    let server: http.Server
+    let serverUrl: string
+
+    before((done) => {
+      server = http.createServer((request, response) => {
+        if (request.headers.authorization) {
+          return response.end('ok')
+        }
+        response
+          .writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' })
+          .end()
+      }).listen(0, '127.0.0.1', () => {
+        serverUrl = 'http://127.0.0.1:' + (server.address() as net.AddressInfo).port
+        done()
+      })
+    })
+
+    it('should emit a login event on app when a WebContents hits a 401', async () => {
+      const w = new BrowserWindow({ show: false })
+      w.loadURL(serverUrl)
+      const [, webContents] = await emittedOnce(app, 'login')
+      expect(webContents).to.equal(w.webContents)
+    })
+  })
 })
 
 async function runTestApp (name: string, ...args: any[]) {

spec-main/api-session-spec.ts

@@ -313,11 +313,6 @@ describe('session module', () => {
 
     beforeEach(async () => {
       customSession = session.fromPartition('proxyconfig')
-      // FIXME(deepak1556): This is just a hack to force
-      // creation of request context which in turn initializes
-      // the network context, can be removed with network
-      // service enabled.
-      await customSession.clearHostResolverCache()
     })
 
     afterEach(() => {

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

@@ -1520,4 +1520,99 @@ describe('webContents module', () => {
       await devtoolsClosed
     })
   })
+
+  describe('login event', () => {
+    afterEach(closeAllWindows)
+
+    let server: http.Server
+    let serverUrl: string
+    let serverPort: number
+    let proxyServer: http.Server
+    let proxyServerPort: number
+
+    before((done) => {
+      server = http.createServer((request, response) => {
+        if (request.url === '/no-auth') {
+          return response.end('ok')
+        }
+        if (request.headers.authorization) {
+          response.writeHead(200, { 'Content-type': 'text/plain' })
+          return response.end(request.headers.authorization)
+        }
+        response
+          .writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' })
+          .end()
+      }).listen(0, '127.0.0.1', () => {
+        serverPort = (server.address() as AddressInfo).port
+        serverUrl = `http://127.0.0.1:${serverPort}`
+        done()
+      })
+    })
+
+    before((done) => {
+      proxyServer = http.createServer((request, response) => {
+        if (request.headers['proxy-authorization']) {
+          response.writeHead(200, { 'Content-type': 'text/plain' })
+          return response.end(request.headers['proxy-authorization'])
+        }
+        response
+          .writeHead(407, { 'Proxy-Authenticate': 'Basic realm="Foo"' })
+          .end()
+      }).listen(0, '127.0.0.1', () => {
+        proxyServerPort = (proxyServer.address() as AddressInfo).port
+        done()
+      })
+    })
+
+    after(() => {
+      server.close()
+      proxyServer.close()
+    })
+
+    it('is emitted when navigating', async () => {
+      const [user, pass] = ['user', 'pass']
+      const w = new BrowserWindow({ show: false })
+      let eventRequest: any
+      let eventAuthInfo: any
+      w.webContents.on('login', (event, request, authInfo, cb) => {
+        eventRequest = request
+        eventAuthInfo = authInfo
+        event.preventDefault()
+        cb(user, pass)
+      })
+      await w.loadURL(serverUrl)
+      const body = await w.webContents.executeJavaScript(`document.documentElement.textContent`)
+      expect(body).to.equal(`Basic ${Buffer.from(`${user}:${pass}`).toString('base64')}`)
+      expect(eventRequest.url).to.equal(serverUrl + '/')
+      expect(eventAuthInfo.isProxy).to.be.false()
+      expect(eventAuthInfo.scheme).to.equal('basic')
+      expect(eventAuthInfo.host).to.equal('127.0.0.1')
+      expect(eventAuthInfo.port).to.equal(serverPort)
+      expect(eventAuthInfo.realm).to.equal('Foo')
+    })
+
+    it('is emitted when a proxy requests authorization', async () => {
+      const customSession = session.fromPartition(`${Math.random()}`)
+      await customSession.setProxy({ proxyRules: `127.0.0.1:${proxyServerPort}`, proxyBypassRules: '<-loopback>' })
+      const [user, pass] = ['user', 'pass']
+      const w = new BrowserWindow({ show: false, webPreferences: { session: customSession } })
+      let eventRequest: any
+      let eventAuthInfo: any
+      w.webContents.on('login', (event, request, authInfo, cb) => {
+        eventRequest = request
+        eventAuthInfo = authInfo
+        event.preventDefault()
+        cb(user, pass)
+      })
+      await w.loadURL(`${serverUrl}/no-auth`)
+      const body = await w.webContents.executeJavaScript(`document.documentElement.textContent`)
+      expect(body).to.equal(`Basic ${Buffer.from(`${user}:${pass}`).toString('base64')}`)
+      expect(eventRequest.url).to.equal(`${serverUrl}/no-auth`)
+      expect(eventAuthInfo.isProxy).to.be.true()
+      expect(eventAuthInfo.scheme).to.equal('basic')
+      expect(eventAuthInfo.host).to.equal('127.0.0.1')
+      expect(eventAuthInfo.port).to.equal(proxyServerPort)
+      expect(eventAuthInfo.realm).to.equal('Foo')
+    })
+  })
 })