MM-51535 - Calls: Fix unrestricted redirect from calls widget (#2635)

* add will-redirect handler to prevent unrestricted redirect

* import ordering

* simplify onWillRedirect handler; tests

* Adding punctuation to force tests to run again.
This commit is contained in:
Christopher Poile
2023-03-28 09:53:30 -04:00
committed by GitHub
parent cc706f7a97
commit d18e3e2251
2 changed files with 52 additions and 11 deletions

View File

@@ -454,6 +454,16 @@ describe('main/windows/callsWidgetWindow', () => {
expect(widgetWindow.onPopOutOpen({url: popOutURL})).toHaveProperty('action', 'deny'); expect(widgetWindow.onPopOutOpen({url: popOutURL})).toHaveProperty('action', 'deny');
}); });
it('popout redirects are disabled', () => {
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
const ev = {preventDefault: jest.fn()};
const redirectURL = 'http://localhost:8065/login/sso/saml?redirect_to=https://google.com';
widgetWindow.onWillRedirect(ev, redirectURL);
expect(ev.preventDefault).toHaveBeenCalled();
});
it('onPopOutFocus', () => { it('onPopOutFocus', () => {
baseWindow.webContents = { baseWindow.webContents = {
...baseWindow.webContents, ...baseWindow.webContents,
@@ -484,7 +494,6 @@ describe('main/windows/callsWidgetWindow', () => {
widgetWindow.onPopOutCreate(popOut); widgetWindow.onPopOutCreate(popOut);
expect(widgetWindow.popOut).toBe(popOut); expect(widgetWindow.popOut).toBe(popOut);
expect(WebContentsEventManager.addWebContentsEventListeners).toHaveBeenCalledWith(popOut.webContents);
widgetWindow.onPopOutFocus(); widgetWindow.onPopOutFocus();
expect(popOut.focus).toHaveBeenCalled(); expect(popOut.focus).toHaveBeenCalled();
@@ -496,6 +505,32 @@ describe('main/windows/callsWidgetWindow', () => {
expect(popOut.restore).toHaveBeenCalled(); expect(popOut.restore).toHaveBeenCalled();
}); });
it('onPopOutCreate', () => {
baseWindow.webContents = {
...baseWindow.webContents,
send: jest.fn(),
};
baseWindow.restore = jest.fn();
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
expect(baseWindow.webContents.setWindowOpenHandler).toHaveBeenCalledWith(widgetWindow.onPopOutOpen);
expect(baseWindow.webContents.on).toHaveBeenCalledWith('did-create-window', widgetWindow.onPopOutCreate);
expect(widgetWindow.popOut).toBeNull();
const popOut = new EventEmitter();
popOut.webContents = {
on: jest.fn(),
id: 'webContentsId',
};
widgetWindow.onPopOutCreate(popOut);
expect(widgetWindow.popOut).toBe(popOut);
expect(WebContentsEventManager.addWebContentsEventListeners).toHaveBeenCalledWith(popOut.webContents);
expect(popOut.webContents.on).toHaveBeenCalledWith('will-redirect', widgetWindow.onWillRedirect);
});
it('getWebContentsId', () => { it('getWebContentsId', () => {
baseWindow.webContents = { baseWindow.webContents = {
...baseWindow.webContents, ...baseWindow.webContents,

View File

@@ -2,25 +2,21 @@
// See LICENSE.txt for license information. // See LICENSE.txt for license information.
import {EventEmitter} from 'events'; import {EventEmitter} from 'events';
import {BrowserWindow, Rectangle, ipcMain, IpcMainEvent} from 'electron'; import {BrowserWindow, ipcMain, IpcMainEvent, Rectangle} from 'electron';
import log from 'electron-log'; import log from 'electron-log';
import { import {
CallsWidgetWindowConfig, CallsJoinedCallMessage,
CallsWidgetResizeMessage, CallsWidgetResizeMessage,
CallsWidgetShareScreenMessage, CallsWidgetShareScreenMessage,
CallsJoinedCallMessage, CallsWidgetWindowConfig,
} from 'types/calls'; } from 'types/calls';
import {MattermostView} from 'main/views/MattermostView'; import {MattermostView} from 'main/views/MattermostView';
import {getLocalPreload} from 'main/utils'; import {getLocalPreload} from 'main/utils';
import { import {CALLS_PLUGIN_ID, MINIMUM_CALLS_WIDGET_HEIGHT, MINIMUM_CALLS_WIDGET_WIDTH} from 'common/utils/constants';
MINIMUM_CALLS_WIDGET_WIDTH,
MINIMUM_CALLS_WIDGET_HEIGHT,
CALLS_PLUGIN_ID,
} from 'common/utils/constants';
import Utils from 'common/utils/util'; import Utils from 'common/utils/util';
import urlUtils, {getFormattedPathName} from 'common/utils/url'; import urlUtils, {getFormattedPathName} from 'common/utils/url';
import { import {
@@ -231,7 +227,7 @@ export default class CallsWidgetWindow extends EventEmitter {
this.setBounds(initialBounds); this.setBounds(initialBounds);
} }
private onPopOutOpen = ({url}: {url: string}) => { private onPopOutOpen = ({url}: { url: string }) => {
if (urlUtils.isCallsPopOutURL(this.mainView.serverInfo.server.url, url, this.config.callID)) { if (urlUtils.isCallsPopOutURL(this.mainView.serverInfo.server.url, url, this.config.callID)) {
return { return {
action: 'allow' as const, action: 'allow' as const,
@@ -248,8 +244,18 @@ export default class CallsWidgetWindow extends EventEmitter {
private onPopOutCreate = (win: BrowserWindow) => { private onPopOutCreate = (win: BrowserWindow) => {
this.popOut = win; this.popOut = win;
// Let the webContentsEventManager handle links that try to open a new window // Let the webContentsEventManager handle links that try to open a new window.
webContentsEventManager.addWebContentsEventListeners(this.popOut.webContents); webContentsEventManager.addWebContentsEventListeners(this.popOut.webContents);
// Need to capture and handle redirects for security.
this.popOut.webContents.on('will-redirect', this.onWillRedirect);
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
private onWillRedirect = (event: Event, url: string) => {
// There's no reason we would allow a redirect from the call's popout. Eventually we may, so revise then.
// Note for the future: the code from https://github.com/mattermost/desktop/pull/2580 will not work for us.
event.preventDefault();
} }
private onPopOutFocus = () => { private onPopOutFocus = () => {