twolfson / karma-electron

Karma launcher and preprocessor for Electron
The Unlicense
59 stars 21 forks source link

Electron 5 nodeIntegration #38

Closed fxha closed 5 years ago

fxha commented 5 years ago

nodeIntegration is disable by default since Electron 5 because of security reasons. Could you please add a option to enable node in electron-launcher.js or maybe allow nodeIntegration by default?

webPreferences: {
  nodeIntegration: true
}
qazbnm456 commented 5 years ago

For anyone bumping into this issue, here is a temporary workaround:

karma.conf.js

...
    customLaunchers: {
      'visibleElectron': {
        base: 'Electron',
        flags: ['--show'],
        require: path.join('test', 'unit', 'override.js')
      }
    },
...

test/unit/override.js

const { app, BrowserWindow } = require('electron');

app.once('browser-window-created', (event, window) => {
  const preferences = window.webContents.getLastWebPreferences();
  preferences.contextIsolation = false;
  preferences.nodeIntegration = true;
  preferences.webviewTag = true;

  window.webContents.on('did-start-navigation', (event, url) => {
    event.preventDefault();
    const browserWindow = new BrowserWindow({
      show: !!window.webContents.browserWindowOptions.show,
      webPreferences: preferences
    });
    browserWindow.loadURL(url, {
      userAgent: 'Electron ' + process.versions.electron + ' (Node ' + process.versions.node + ')'
    });
    window.destroy();
  });
});
twolfson commented 5 years ago

Sorry for the slow reply. I'm coming back online at the end of a very long vacation (still pending full completion)

I'll look into this over the next 2 weeks

On Sat, May 11, 2019, 3:15 PM Boik notifications@github.com wrote:

For anyone bumping into this issue, here is a temporary workaround: karma.conf.js

... customLaunchers: { 'visibleElectron': { base: 'Electron', flags: ['--show'], require: path.join('test', 'unit', 'override.js') } },...

test/unit/override.js

const { app, BrowserWindow } = require('electron'); app.once('browser-window-created', (event, window) => { const preferences = window.webContents.getLastWebPreferences(); preferences.contextIsolation = false; preferences.nodeIntegration = true; preferences.webviewTag = true;

window.webContents.on('did-start-navigation', (event, url) => { event.preventDefault(); const browserWindow = new BrowserWindow({ show: !!window.webContents.browserWindowOptions.show, webPreferences: preferences }); browserWindow.loadURL(url, { userAgent: 'Electron ' + process.versions.electron + ' (Node ' + process.versions.node + ')' }); window.destroy(); }); });

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/twolfson/karma-electron/issues/38#issuecomment-491510277, or mute the thread https://github.com/notifications/unsubscribe-auth/AAG4KWBCRONS3XFACKB7A43PU3BI7ANCNFSM4HI4I7UA .

gabrielstuff commented 5 years ago

Thanks @twolfson, actually the idea would be to let the user passes webPreferences to the browser window. This way, we can configure the Karma Electron the way we want.

Thanks !

gabrielstuff commented 5 years ago

Thanks @qazbnm456 your quick patch works for now. https://github.com/twolfson/karma-electron/issues/38#issuecomment-491510277

twolfson commented 5 years ago

I remember seeing a similar idea when I first built this repo in karma-electron-launcher (didn't use that repo due to it not handling __filename and such properly). Their execution was overkill so I skipped using it (wrote to a file to pass through) and short circuited to the only option I needed (hiding/showing window):

https://github.com/lele85/karma-electron-launcher/blob/eeb8712785c4c7a0739923db19ef8b2cf73cc971/index.js#L37-L41

All that being said, I'm definitely open to supporting arbitrary BrowserWindow and loadURL parameters. Would either of you be open to discussing options here and writing a PR?

twolfson commented 5 years ago

Also happy to accept a PR for --node-integration since we'd only be at 2 wrapper parameters at the moment

gabrielstuff commented 5 years ago

Hello,

I think, this will be soon mandatory. Not everybody is moving to Electron 5.0, but with weeks passing, people will.

When reading your code, I was wondering why not directly allowing to override the webPreferences key when you launch : https://github.com/twolfson/karma-electron/blob/master/lib/electron-launcher.js#L48

I did not check where you get the params, I guess it is through prompt or something else. We could pass it in the karma.conf.js ?

twolfson commented 5 years ago

I explained in https://github.com/twolfson/karma-electron/issues/38#issuecomment-496036339 why we don't yet have the functionality; because there wasn't a need for it

Electron is executed as a separate process (just as a browser is). Parameters are loaded/passed via Karma's browser launcher tooling as CLI flags:

https://github.com/twolfson/karma-electron/blob/6.1.1/lib/karma-electron-launcher.js#L20-L47

I'd be open to loading preferences via karma.conf.js. Upon contemplation though, I fear it's going to lead to more issues:

The simplest route is to use a CLI flag with serialized JSON but then we run into potential length issues. I've definitely had problems in the past on supporting Windows =/

The next level from that is using stdin for communication to Electron (e.g. pass a --stdin-options flag to let our child process to know to read stdin for options, then parse JSON from that). It looks like we have some decent control over child_process.spawn so we should be able to do it

https://github.com/karma-runner/karma/blob/v4.1.0/lib/launcher.js#L14-L27

https://github.com/karma-runner/karma/blob/v4.1.0/lib/launcher.js#L52-L53

https://github.com/karma-runner/karma/blob/v4.1.0/lib/launchers/process.js#L62-L75

twolfson commented 5 years ago

After receiving both #40 and #41, I checked the Electron@5 release dates. It looks like it's been out for 1.5 months so we should get support moving along. Going to start looking into this

https://electronjs.org/releases/stable?version=5#5.0.0

twolfson commented 5 years ago

Okay, we've got a working proof of concept for passing data to stdin

https://github.com/twolfson/karma-electron/tree/c90454e328d4f01881d933a1eb10139686d85658

However, I'm realizing that this isn't as easily accessible as I'd like though -- I want people to opt-in to nodeIntegration without needing to define a custom launcher. As a result, I'm going to explore defining a second launcher (e.g. ElectronWithNode) to make setup more accessible

twolfson commented 5 years ago

After reading through Electron's security document, it's clear that there's no catch-all setup for an Electron app (e.g. everyone has their own security concerns and trade-offs). As a result, it's foolish for me to try to build out ElectronWithNode -- instead, allowing custom webPreferences is indeed the best path:

https://github.com/electron/electron/blob/v7.0.0-nightly.20190612/docs/tutorial/security.md

twolfson commented 5 years ago

We've added support for browserWindowOptions and loadURLOptions in 6.2.0. Thanks for the bug report

Here's the corresponding documentation and example usage:

https://github.com/twolfson/karma-electron/tree/6.2.0#launcher-configuration

https://github.com/twolfson/karma-electron/tree/6.2.0#examples