Merge pull request #8134 from deepak1556/empty_client_certificate_patch

app: select-client-certificate event callback can accept certificate optionally

atom/browser/api/atom_api_app.cc

@@ -378,9 +378,21 @@ void OnClientCertificateSelected(
     v8::Isolate* isolate,
     std::shared_ptr<content::ClientCertificateDelegate> delegate,
     mate::Arguments* args) {
+  if (args->Length() == 2) {
+    delegate->ContinueWithCertificate(nullptr);
+    return;
+  }
+
+  v8::Local<v8::Value> val;
+  args->GetNext(&val);
+  if (val->IsNull()) {
+    delegate->ContinueWithCertificate(nullptr);
+    return;
+  }
+
   mate::Dictionary cert_data;
-  if (!args->GetNext(&cert_data)) {
+  if (!mate::ConvertFromV8(isolate, val, &cert_data)) {
-    args->ThrowError();
+    args->ThrowError("Must pass valid certificate object.");
     return;
   }
 

docs/api/app.md

@@ -113,7 +113,7 @@ Returns:
 * `url` String
 
 Emitted when the user wants to open a URL with the application. Your application's
-`Info.plist` file must define the url scheme within the `CFBundleURLTypes` key, and 
+`Info.plist` file must define the url scheme within the `CFBundleURLTypes` key, and
 set `NSPrincipalClass` to `AtomApplication`.
 
 You should call `event.preventDefault()` if you want to handle this event.
@@ -222,12 +222,12 @@ Returns:
 * `url` URL
 * `certificateList` [Certificate[]](structures/certificate.md)
 * `callback` Function
-  * `certificate` [Certificate](structures/certificate.md)
+  * `certificate` [Certificate](structures/certificate.md) (optional)
 
 Emitted when a client certificate is requested.
 
 The `url` corresponds to the navigation entry requesting the client certificate
-and `callback` needs to be called with an entry filtered from the list. Using
+and `callback` can be called with an entry filtered from the list. Using
 `event.preventDefault()` prevents the application from using the first
 certificate from the store.
 

spec/api-app-spec.js

@@ -4,7 +4,7 @@ const https = require('https')
 const net = require('net')
 const fs = require('fs')
 const path = require('path')
-const {remote} = require('electron')
+const {ipcRenderer, remote} = require('electron')
 const {closeWindow} = require('./window-helpers')
 
 const {app, BrowserWindow, ipcMain} = remote
@@ -41,6 +41,41 @@ describe('electron module', function () {
 })
 
 describe('app module', function () {
+  let server, secureUrl
+  const certPath = path.join(__dirname, 'fixtures', 'certificates')
+
+  before(function () {
+    const options = {
+      key: fs.readFileSync(path.join(certPath, 'server.key')),
+      cert: fs.readFileSync(path.join(certPath, 'server.pem')),
+      ca: [
+        fs.readFileSync(path.join(certPath, 'rootCA.pem')),
+        fs.readFileSync(path.join(certPath, 'intermediateCA.pem'))
+      ],
+      requestCert: true,
+      rejectUnauthorized: false
+    }
+
+    server = https.createServer(options, function (req, res) {
+      if (req.client.authorized) {
+        res.writeHead(200)
+        res.end('<title>authorized</title>')
+      } else {
+        res.writeHead(401)
+        res.end('<title>denied</title>')
+      }
+    })
+
+    server.listen(0, '127.0.0.1', function () {
+      const port = server.address().port
+      secureUrl = `https://127.0.0.1:${port}`
+    })
+  })
+
+  after(function () {
+    server.close()
+  })
+
   describe('app.getVersion()', function () {
     it('returns the version field of package.json', function () {
       assert.equal(app.getVersion(), '0.1.0')
@@ -165,24 +200,6 @@ describe('app module', function () {
     if (process.platform !== 'linux') return
 
     var w = null
-    var certPath = path.join(__dirname, 'fixtures', 'certificates')
-    var options = {
-      key: fs.readFileSync(path.join(certPath, 'server.key')),
-      cert: fs.readFileSync(path.join(certPath, 'server.pem')),
-      ca: [
-        fs.readFileSync(path.join(certPath, 'rootCA.pem')),
-        fs.readFileSync(path.join(certPath, 'intermediateCA.pem'))
-      ],
-      requestCert: true,
-      rejectUnauthorized: false
-    }
-
-    var server = https.createServer(options, function (req, res) {
-      if (req.client.authorized) {
-        res.writeHead(200)
-        res.end('authorized')
-      }
-    })
 
     afterEach(function () {
       return closeWindow(w).then(function () { w = null })
@@ -199,25 +216,24 @@ describe('app module', function () {
       })
 
       w.webContents.on('did-finish-load', function () {
-        server.close()
+        assert.equal(w.webContents.getTitle(), 'authorized')
         done()
       })
 
-      app.on('select-client-certificate', function (event, webContents, url, list, callback) {
+      ipcRenderer.once('select-client-certificate', function (event, webContentsId, list) {
+        assert.equal(webContentsId, w.webContents.id)
         assert.equal(list.length, 1)
         assert.equal(list[0].issuerName, 'Intermediate CA')
         assert.equal(list[0].subjectName, 'Client Cert')
         assert.equal(list[0].issuer.commonName, 'Intermediate CA')
         assert.equal(list[0].subject.commonName, 'Client Cert')
-        callback(list[0])
+        event.sender.send('client-certificate-response', list[0])
       })
 
       app.importCertificate(options, function (result) {
         assert(!result)
-        server.listen(0, '127.0.0.1', function () {
+        ipcRenderer.sendSync('set-client-certificate-option', false)
-          var port = server.address().port
+        w.loadURL(secureUrl)
-          w.loadURL(`https://127.0.0.1:${port}`)
-        })
       })
     })
   })
@@ -359,4 +375,32 @@ describe('app module', function () {
       assert.equal(app.getPath('music'), __dirname)
     })
   })
+
+  describe('select-client-certificate event', function () {
+    let w = null
+
+    beforeEach(function () {
+      w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          partition: 'empty-certificate'
+        }
+      })
+    })
+
+    afterEach(function () {
+      return closeWindow(w).then(function () { w = null })
+    })
+
+    it('can respond with empty certificate list', function (done) {
+      w.webContents.on('did-finish-load', function () {
+        assert.equal(w.webContents.getTitle(), 'denied')
+        server.close()
+        done()
+      })
+
+      ipcRenderer.sendSync('set-client-certificate-option', true)
+      w.webContents.loadURL(secureUrl)
+    })
+  })
 })

spec/static/main.js

@@ -206,3 +206,18 @@ app.on('ready', function () {
     }
   })
 })
+
+ipcMain.on('set-client-certificate-option', function (event, skip) {
+  app.once('select-client-certificate', function (event, webContents, url, list, callback) {
+    event.preventDefault()
+    if (skip) {
+      callback()
+    } else {
+      ipcMain.on('client-certificate-response', function (event, certificate) {
+        callback(certificate)
+      })
+      window.webContents.send('select-client-certificate', webContents.id, list)
+    }
+  })
+  event.returnValue = 'done'
+})