Refactor webContentsEvents and popup listeners (#2587)

This commit is contained in:
Devin Binnie 2023-03-07 08:50:39 -05:00 committed by GitHub
parent 2366214d9d
commit 12ec2748b4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 96 additions and 64 deletions

View file

@ -3,10 +3,12 @@
'use strict';
import {shell} from 'electron';
import {shell, BrowserWindow} from 'electron';
import urlUtils from 'common/utils/url';
import ContextMenu from 'main/contextMenu';
import * as WindowManager from '../windows/windowManager';
import allowProtocolDialog from '../allowProtocolDialog';
@ -17,22 +19,19 @@ jest.mock('electron', () => ({
shell: {
openExternal: jest.fn(),
},
BrowserWindow: jest.fn().mockImplementation(() => ({
once: jest.fn(),
show: jest.fn(),
loadURL: jest.fn(),
webContents: {
setWindowOpenHandler: jest.fn(),
},
})),
BrowserWindow: jest.fn(),
session: {},
}));
jest.mock('main/contextMenu', () => jest.fn());
jest.mock('../allowProtocolDialog', () => ({}));
jest.mock('../windows/windowManager', () => ({
getServerURLFromWebContentsId: jest.fn(),
showMainWindow: jest.fn(),
}));
jest.mock('../utils', () => ({
composeUserAgent: jest.fn(),
}));
jest.mock('common/config', () => ({
spellcheck: true,
@ -77,14 +76,14 @@ jest.mock('../allowProtocolDialog', () => ({
}));
describe('main/views/webContentsEvents', () => {
const event = {preventDefault: jest.fn(), sender: {id: 1}};
const event = {preventDefault: jest.fn()};
describe('willNavigate', () => {
const webContentsEventManager = new WebContentsEventManager();
const willNavigate = webContentsEventManager.generateWillNavigate(jest.fn());
const willNavigate = webContentsEventManager.generateWillNavigate(1);
beforeEach(() => {
WindowManager.getServerURLFromWebContentsId.mockImplementation(() => new URL('http://server-1.com'));
webContentsEventManager.getServerURLFromWebContentsId = jest.fn().mockImplementation(() => new URL('http://server-1.com'));
});
afterEach(() => {
@ -144,10 +143,10 @@ describe('main/views/webContentsEvents', () => {
describe('didStartNavigation', () => {
const webContentsEventManager = new WebContentsEventManager();
const didStartNavigation = webContentsEventManager.generateDidStartNavigation(jest.fn());
const didStartNavigation = webContentsEventManager.generateDidStartNavigation(1);
beforeEach(() => {
WindowManager.getServerURLFromWebContentsId.mockImplementation(() => new URL('http://server-1.com'));
webContentsEventManager.getServerURLFromWebContentsId = jest.fn().mockImplementation(() => new URL('http://server-1.com'));
urlUtils.isTrustedURL.mockReturnValue(true);
urlUtils.isInternalURL.mockImplementation((serverURL, parsedURL) => parsedURL.toString().startsWith(serverURL));
urlUtils.isCustomLoginURL.mockImplementation((parsedURL) => parsedURL.toString().startsWith('http://loginurl.com/login'));
@ -173,18 +172,32 @@ describe('main/views/webContentsEvents', () => {
describe('newWindow', () => {
const webContentsEventManager = new WebContentsEventManager();
const newWindow = webContentsEventManager.generateNewWindowListener(jest.fn());
const newWindow = webContentsEventManager.generateNewWindowListener(1, true);
beforeEach(() => {
urlUtils.isValidURI.mockReturnValue(true);
WindowManager.getServerURLFromWebContentsId.mockImplementation(() => new URL('http://server-1.com'));
webContentsEventManager.getServerURLFromWebContentsId = jest.fn().mockImplementation(() => new URL('http://server-1.com'));
urlUtils.isTeamUrl.mockImplementation((serverURL, parsedURL) => parsedURL.toString().startsWith(`${serverURL}myteam`));
urlUtils.isAdminUrl.mockImplementation((serverURL, parsedURL) => parsedURL.toString().startsWith(`${serverURL}admin_console`));
urlUtils.isPluginUrl.mockImplementation((serverURL, parsedURL) => parsedURL.toString().startsWith(`${serverURL}myplugin`));
urlUtils.isManagedResource.mockImplementation((serverURL, parsedURL) => parsedURL.toString().startsWith(`${serverURL}myplugin`));
urlUtils.isManagedResource.mockImplementation((serverURL, parsedURL) => parsedURL.toString().startsWith(`${serverURL}trusted`));
BrowserWindow.mockImplementation(() => ({
once: jest.fn(),
show: jest.fn(),
loadURL: jest.fn(),
webContents: {
on: jest.fn(),
setWindowOpenHandler: jest.fn(),
},
}));
ContextMenu.mockImplementation(() => ({
reload: jest.fn(),
}));
});
afterEach(() => {
webContentsEventManager.popupWindow = undefined;
jest.resetAllMocks();
});
it('should deny on bad URL', () => {
@ -237,7 +250,7 @@ describe('main/views/webContentsEvents', () => {
});
it('should prevent from opening a new window if popup already exists', () => {
webContentsEventManager.popupWindow = {webContents: {getURL: () => 'http://server-1.com/myplugin/login'}};
webContentsEventManager.popupWindow = {win: {webContents: {getURL: () => 'http://server-1.com/myplugin/login'}}};
expect(newWindow({url: 'http://server-1.com/myplugin/login'})).toStrictEqual({action: 'deny'});
});

View file

@ -28,32 +28,36 @@ const scheme = protocols && protocols[0] && protocols[0].schemes && protocols[0]
export class WebContentsEventManager {
customLogins: Record<number, CustomLogin>;
listeners: Record<number, () => void>;
popupWindow?: BrowserWindow;
popupWindow?: {win: BrowserWindow; serverURL?: URL};
constructor() {
this.customLogins = {};
this.listeners = {};
}
private isTrustedPopupWindow = (webContents: WebContents) => {
if (!webContents) {
return false;
}
private isTrustedPopupWindow = (webContentsId: number) => {
if (!this.popupWindow) {
return false;
}
return BrowserWindow.fromWebContents(webContents) === this.popupWindow;
return webContentsId === this.popupWindow.win.webContents.id;
}
generateWillNavigate = () => {
return (event: Event & {sender: WebContents}, url: string) => {
log.debug('webContentEvents.will-navigate', {webContentsId: event.sender.id, url});
private getServerURLFromWebContentsId = (webContentsId: number) => {
if (this.popupWindow && webContentsId === this.popupWindow.win.webContents.id) {
return this.popupWindow.serverURL;
}
return WindowManager.getServerURLFromWebContentsId(webContentsId);
}
generateWillNavigate = (webContentsId: number) => {
return (event: Event, url: string) => {
log.debug('webContentEvents.will-navigate', {webContentsId, url});
const contentID = event.sender.id;
const parsedURL = urlUtils.parseURL(url)!;
const serverURL = WindowManager.getServerURLFromWebContentsId(event.sender.id);
const serverURL = this.getServerURLFromWebContentsId(webContentsId);
if (serverURL && (urlUtils.isTeamUrl(serverURL, parsedURL) || urlUtils.isAdminUrl(serverURL, parsedURL) || this.isTrustedPopupWindow(event.sender))) {
if (serverURL && (urlUtils.isTeamUrl(serverURL, parsedURL) || urlUtils.isAdminUrl(serverURL, parsedURL) || this.isTrustedPopupWindow(webContentsId))) {
return;
}
@ -67,7 +71,7 @@ export class WebContentsEventManager {
if (parsedURL.protocol === 'mailto:') {
return;
}
if (this.customLogins[contentID]?.inProgress) {
if (this.customLogins[webContentsId]?.inProgress) {
flushCookiesStore(session.defaultSession);
return;
}
@ -82,22 +86,21 @@ export class WebContentsEventManager {
};
};
generateDidStartNavigation = () => {
return (event: Event & {sender: WebContents}, url: string) => {
log.debug('webContentEvents.did-start-navigation', {webContentsId: event.sender.id, url});
generateDidStartNavigation = (webContentsId: number) => {
return (event: Event, url: string) => {
log.debug('webContentEvents.did-start-navigation', {webContentsId, url});
const contentID = event.sender.id;
const parsedURL = urlUtils.parseURL(url)!;
const serverURL = WindowManager.getServerURLFromWebContentsId(event.sender.id);
const serverURL = this.getServerURLFromWebContentsId(webContentsId);
if (!serverURL || !urlUtils.isTrustedURL(parsedURL, serverURL)) {
return;
}
if (serverURL && urlUtils.isCustomLoginURL(parsedURL, serverURL)) {
this.customLogins[contentID].inProgress = true;
} else if (serverURL && this.customLogins[contentID].inProgress && urlUtils.isInternalURL(serverURL || new URL(''), parsedURL)) {
this.customLogins[contentID].inProgress = false;
this.customLogins[webContentsId].inProgress = true;
} else if (serverURL && this.customLogins[webContentsId].inProgress && urlUtils.isInternalURL(serverURL || new URL(''), parsedURL)) {
this.customLogins[webContentsId].inProgress = false;
}
};
};
@ -135,7 +138,7 @@ export class WebContentsEventManager {
return {action: 'deny'};
}
const serverURL = WindowManager.getServerURLFromWebContentsId(webContentsId);
const serverURL = this.getServerURLFromWebContentsId(webContentsId);
if (!serverURL) {
shell.openExternal(details.url);
return {action: 'deny'};
@ -170,24 +173,38 @@ export class WebContentsEventManager {
log.info(`${details.url} is an admin console page, preventing to open a new window`);
return {action: 'deny'};
}
if (this.popupWindow && this.popupWindow.webContents.getURL() === details.url) {
if (this.popupWindow && this.popupWindow.win.webContents.getURL() === details.url) {
log.info(`Popup window already open at provided url: ${details.url}`);
return {action: 'deny'};
}
// TODO: move popups to its own and have more than one.
if (urlUtils.isPluginUrl(serverURL, parsedURL) || urlUtils.isManagedResource(serverURL, parsedURL)) {
if (!this.popupWindow) {
this.popupWindow = new BrowserWindow({
backgroundColor: '#fff', // prevents blurry text: https://electronjs.org/docs/faq#the-font-looks-blurry-what-is-this-and-what-can-i-do
//parent: WindowManager.getMainWindow(),
show: false,
center: true,
webPreferences: {
spellcheck: (typeof spellcheck === 'undefined' ? true : spellcheck),
},
let popup: BrowserWindow;
if (this.popupWindow) {
this.popupWindow.win.once('ready-to-show', () => {
this.popupWindow?.win.show();
});
this.popupWindow.webContents.on('will-redirect', (event, url) => {
popup = this.popupWindow.win;
} else {
this.popupWindow = {
win: new BrowserWindow({
backgroundColor: '#fff', // prevents blurry text: https://electronjs.org/docs/faq#the-font-looks-blurry-what-is-this-and-what-can-i-do
//parent: WindowManager.getMainWindow(),
show: false,
center: true,
webPreferences: {
spellcheck: (typeof spellcheck === 'undefined' ? true : spellcheck),
},
}),
serverURL,
};
this.customLogins[this.popupWindow.win.webContents.id] = {
inProgress: false,
};
popup = this.popupWindow.win;
popup.webContents.on('will-redirect', (event, url) => {
const parsedURL = urlUtils.parseURL(url);
if (!parsedURL) {
event.preventDefault();
@ -198,23 +215,25 @@ export class WebContentsEventManager {
event.preventDefault();
}
});
this.popupWindow.webContents.setWindowOpenHandler(this.denyNewWindow);
this.popupWindow.once('closed', () => {
popup.webContents.on('will-navigate', this.generateWillNavigate(popup.webContents.id));
popup.webContents.on('did-start-navigation', this.generateDidStartNavigation(popup.webContents.id));
popup.webContents.setWindowOpenHandler(this.denyNewWindow);
popup.once('closed', () => {
this.popupWindow = undefined;
});
const contextMenu = new ContextMenu({}, this.popupWindow);
const contextMenu = new ContextMenu({}, popup);
contextMenu.reload();
}
const popupWindow = this.popupWindow;
popupWindow.once('ready-to-show', () => popupWindow.show());
popup.once('ready-to-show', () => popup.show());
if (urlUtils.isManagedResource(serverURL, parsedURL)) {
popupWindow.loadURL(details.url);
popup.loadURL(details.url);
} else {
// currently changing the userAgent for popup windows to allow plugins to go through google's oAuth
// should be removed once a proper oAuth2 implementation is setup.
popupWindow.loadURL(details.url, {
popup.loadURL(details.url, {
userAgent: composeUserAgent(),
});
}
@ -266,16 +285,16 @@ export class WebContentsEventManager {
this.removeWebContentsListeners(contents.id);
}
const willNavigate = this.generateWillNavigate();
contents.on('will-navigate', willNavigate as (e: Event, u: string) => void); // TODO: Electron types don't include sender for some reason
const willNavigate = this.generateWillNavigate(contents.id);
contents.on('will-navigate', willNavigate);
// handle custom login requests (oath, saml):
// 1. are we navigating to a supported local custom login path from the `/login` page?
// - indicate custom login is in progress
// 2. are we finished with the custom login process?
// - indicate custom login is NOT in progress
const didStartNavigation = this.generateDidStartNavigation();
contents.on('did-start-navigation', didStartNavigation as (e: Event, u: string) => void);
const didStartNavigation = this.generateDidStartNavigation(contents.id);
contents.on('did-start-navigation', didStartNavigation);
const spellcheck = Config.useSpellChecker;
const newWindow = this.generateNewWindowListener(contents.id, spellcheck);
@ -285,8 +304,8 @@ export class WebContentsEventManager {
const removeWebContentsListeners = () => {
try {
contents.removeListener('will-navigate', willNavigate as (e: Event, u: string) => void);
contents.removeListener('did-start-navigation', didStartNavigation as (e: Event, u: string) => void);
contents.removeListener('will-navigate', willNavigate);
contents.removeListener('did-start-navigation', didStartNavigation);
removeListeners?.(contents);
} catch (e) {
log.error(`Error while trying to detach listeners, this might be ok if the process crashed: ${e}`);