electronegativity
electronegativity copied to clipboard
doyensec
Support the setWindowOpenHandler API in Electron 12+
Is your feature request related to a problem? Please describe.
Electron 12 has a new API to capture and prevent window creation, webContents.setWindowOpenHandler. See: https://www.electronjs.org/docs/tutorial/security#how-10.
The existing new-window event has been deprecated.
Describe the solution you'd like
LIMIT_NAVIGATION_JS_CHECK should detect the presence of the new handler API and treat it the same way as the new-window event, assuming it is an appropriate alternative (or addition).
Describe alternatives you've considered
It's possible to continue using the deprecated event. It is not entirely clear to me how these APIs interact when used together, or whether they are completely interchangeable .
Hello Mitch! I added to the LIMIT_NAVIGATION_JS_CHECK check the support for setWindowOpenHandler. The change is live from v1.9.1. Let me know if it works for you.
About your question, the webContents.on("new-window") et al works fine in v12 and previous electron version, for the moment it's just marked as deprecated. Note that setWindowOpenHandler was sporadically working on v12 (https://github.com/electron/electron/issues/27967):
As a temporary fix before #28498, for anyone looking to prevent navigations to untrusted origins it should still possible to use the web-contents-created event emitted when a new
webContentsis created and attach then yournew-window,will-navigate, andsetWindowOpenHandlerhandlers:
app.on('web-contents-created', (createEvent, contents) => {
contents.on('new-window', newEvent => {
console.log("Blocked by 'new-window'")
newEvent.preventDefault();
});
contents.on('will-navigate', newEvent => {
console.log("Blocked by 'will-navigate'")
newEvent.preventDefault()
});
contents.setWindowOpenHandler(({ url }) => {
if (url.startsWith("https://doyensec.com/")) {
setImmediate(() => {
shell.openExternal(url);
});
return { action: 'allow' }
} else {
console.log("Blocked by 'setWindowOpenHandler'")
return { action: 'deny' }
}
})
});
Link to the Fiddle gist used for testing. This works from 12.0.0 to the latest 12.0.2 as for now.
Originally posted by @phosphore in https://github.com/electron/electron/issues/27967#issuecomment-812840093
I'm still getting an error flagged in 1.9.1 from LIMIT_NAVIGATION_GLOBAL_CHECK when using setWindowOpenHandler as below:
this._window.webContents.setWindowOpenHandler((details) => {
if (this.isAllowedInternalNavigationUrl(details.url)) {
return { action: 'allow' };
} else {
this.tryExternalNavigation(details.url);
return { action: 'deny' };
}
});
This should now be resolved with v1.10. The issue was solved in ElectroNG since its early releases, but now I've ported it to electronegativity.
@phosphore This still seems to produce a warning (see the above PR), am I doing something wrong? Our usage of setWindowOpenHandler:
https://github.com/floating/frame/blob/develop/main/windows/window.ts#L41
https://github.com/floating/frame/blob/develop/main/windows/window.ts#L68
https://github.com/floating/frame/blob/develop/main/windows/index.ts#L383
