aaron/mattermost-desktop
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

[MM-50973] Harden Calls windows URL navigation checks (#2573)

Browse source 
* Harden Calls windows URL navigation checks

* Update src/main/windows/callsWidgetWindow.ts

Co-authored-by: Eva Sarafianou <eva.sarafianou@gmail.com>

* Fix bad merge

* Extract team name to use isUrlType

* Simplify

---------

Co-authored-by: Eva Sarafianou <eva.sarafianou@gmail.com>
This commit is contained in:
Claudio Costa 2023-03-02 12:10:47 -06:00 committed by GitHub
parent 09debd6adb
commit a52e96694d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 148 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

54
src/common/utils/url.test.js
View file

@ -275,4 +275,58 @@ describe('common/utils/url', () => {
)).toBe(false);
});
});
describe('isCallsPopOutURL', () => {
it('should match correct URL', () => {
expect(urlUtils.isCallsPopOutURL(
'http://example.org',
'http://example.org/team/com.mattermost.calls/expanded/callid',
'callid',
)).toBe(true);
});
it('should match with subpath', () => {
expect(urlUtils.isCallsPopOutURL(
'http://example.org/subpath',
'http://example.org/subpath/team/com.mattermost.calls/expanded/callid',
'callid',
)).toBe(true);
});
it('should match with teamname with dash', () => {
expect(urlUtils.isCallsPopOutURL(
'http://example.org',
'http://example.org/team-name/com.mattermost.calls/expanded/callid',
'callid',
)).toBe(true);
});
it('should not match with invalid team name', () => {
expect(urlUtils.isCallsPopOutURL(
'http://example.org',
'http://example.org/invalid$team/com.mattermost.calls/expanded/othercallid',
'callid',
)).toBe(false);
});
it('should not match with incorrect callid', () => {
expect(urlUtils.isCallsPopOutURL(
'http://example.org',
'http://example.org/team/com.mattermost.calls/expanded/othercallid',
'callid',
)).toBe(false);
});
it('should not match with incorrect origin', () => {
expect(urlUtils.isCallsPopOutURL(
'http://example.com',
'http://example.org/team/com.mattermost.calls/expanded/callid',
'callid',
)).toBe(false);
});
it('should not match with missing arguments', () => {
expect(urlUtils.isCallsPopOutURL()).toBe(false);
});
});
});

31
src/common/utils/url.ts
View file

@ -4,7 +4,7 @@
import {isHttpsUri, isHttpUri, isUri} from 'valid-url';
import buildConfig from 'common/config/buildConfig';
import {customLoginRegexPaths, nonTeamUrlPaths} from 'common/utils/constants';
import {customLoginRegexPaths, nonTeamUrlPaths, CALLS_PLUGIN_ID} from 'common/utils/constants';
function isValidURL(testURL: string) {
return Boolean(isHttpUri(testURL) || isHttpsUri(testURL)) && Boolean(parseURL(testURL));
@ -98,12 +98,6 @@ function isAdminUrl(serverUrl: URL | string, inputURL: URL | string) {
}
function isTeamUrl(serverUrl: URL | string, inputURL: URL | string, withApi?: boolean) {
const parsedURL = parseURL(inputURL);
const server = getServerInfo(serverUrl);
if (!parsedURL || !server || (!equalUrlsIgnoringSubpath(server.url, parsedURL))) {
return false;
}
const paths = [...getManagedResources(), ...nonTeamUrlPaths];
if (withApi) {
@ -184,6 +178,28 @@ function cleanPathName(basePathName: string, pathName: string) {
return pathName;
}
function isCallsPopOutURL(serverURL: URL | string, inputURL: URL | string, callID: string) {
if (!serverURL || !inputURL || !callID) {
return false;
}
const parsedURL = parseURL(inputURL);
const server = getServerInfo(serverURL);
if (!server || !parsedURL) {
return false;
}
const matches = parsedURL.pathname.match(new RegExp(`^${server.subpath}([A-Za-z0-9-_]+)/`, 'i'));
if (matches?.length !== 2) {
return false;
}
const teamName = matches[1];
const subPath = `${teamName}/${CALLS_PLUGIN_ID}/expanded/${callID}`;
return isUrlType(subPath, serverURL, inputURL);
}
export default {
isValidURL,
isValidURI,
@ -201,4 +217,5 @@ export default {
isUrlType,
cleanPathName,
startsWithProtocol,
isCallsPopOutURL,
};

2
src/main/views/viewManager.ts
View file

@ -284,7 +284,7 @@ export class ViewManager {
log.error(`Couldn't find a view with the name ${viewName}`);
return;
}
WebContentsEventManager.addMattermostViewEventListeners(view, this.getServers);
WebContentsEventManager.addMattermostViewEventListeners(view);
}
finishLoading = (server: string) => {

11
src/main/views/webContentEvents.ts
View file

@ -4,8 +4,6 @@
import {BrowserWindow, session, shell, WebContents} from 'electron';
import log from 'electron-log';
import {TeamWithTabs} from 'types/config';
import Config from 'common/config';
import urlUtils from 'common/utils/url';
@ -74,6 +72,11 @@ export class WebContentsEventManager {
return;
}
const callID = WindowManager.callsWidgetWindow?.getCallID();
if (serverURL && callID && urlUtils.isCallsPopOutURL(serverURL, parsedURL, callID)) {
return;
}
log.info(`Prevented desktop from navigating to: ${url}`);
event.preventDefault();
};
@ -221,10 +224,9 @@ export class WebContentsEventManager {
}
};
addMattermostViewEventListeners = (mmview: MattermostView, getServersFunction: () => TeamWithTabs[]) => {
addMattermostViewEventListeners = (mmview: MattermostView) => {
this.addWebContentsEventListeners(
mmview.view.webContents,
getServersFunction,
(contents: WebContents) => {
contents.on('page-title-updated', mmview.handleTitleUpdate);
contents.on('page-favicon-updated', mmview.handleFaviconUpdate);
@ -242,7 +244,6 @@ export class WebContentsEventManager {
addWebContentsEventListeners = (
contents: WebContents,
getServersFunction: () => TeamWithTabs[],
addListeners?: (contents: WebContents) => void,
removeListeners?: (contents: WebContents) => void,
) => {

39
src/main/windows/callsWidgetWindow.test.js
View file

@ -27,7 +27,7 @@ jest.mock('electron', () => ({
}));
jest.mock('../views/webContentEvents', () => ({
generateNewWindowListener: jest.fn(),
addWebContentsEventListeners: jest.fn(),
}));
jest.mock('common/utils/url', () => {
@ -413,8 +413,15 @@ describe('main/windows/callsWidgetWindow', () => {
it('menubar disabled on popout', () => {
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
expect(widgetWindow.onPopOutOpen()).toHaveProperty('action', 'allow');
expect(widgetWindow.onPopOutOpen().overrideBrowserWindowOptions).toHaveProperty('autoHideMenuBar', true);
const popOutURL = 'http://localhost:8065/team/com.mattermost.calls/expanded/test-call-id';
expect(widgetWindow.onPopOutOpen({url: popOutURL})).toHaveProperty('action', 'allow');
expect(widgetWindow.onPopOutOpen({url: popOutURL}).overrideBrowserWindowOptions).toHaveProperty('autoHideMenuBar', true);
});
it('wrong popout url disabled', () => {
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
const popOutURL = 'http://localhost/team/com.mattermost.calls/expanded/test-call-id';
expect(widgetWindow.onPopOutOpen({url: popOutURL})).toHaveProperty('action', 'deny');
});
it('onPopOutFocus', () => {
@ -434,7 +441,6 @@ describe('main/windows/callsWidgetWindow', () => {
const popOut = new EventEmitter();
popOut.webContents = {
setWindowOpenHandler: jest.fn(),
on: jest.fn(),
id: 'webContentsId',
};
@ -448,8 +454,7 @@ describe('main/windows/callsWidgetWindow', () => {
widgetWindow.onPopOutCreate(popOut);
expect(widgetWindow.popOut).toBe(popOut);
expect(popOut.webContents.setWindowOpenHandler).toHaveBeenCalled();
expect(WebContentsEventManager.generateNewWindowListener).toHaveBeenCalledWith('webContentsId', true);
expect(WebContentsEventManager.addWebContentsEventListeners).toHaveBeenCalledWith(popOut.webContents);
widgetWindow.onPopOutFocus();
expect(popOut.focus).toHaveBeenCalled();
@ -508,5 +513,27 @@ describe('main/windows/callsWidgetWindow', () => {
sender: baseWindow.webContents,
})).toEqual(true);
});
it('getPopOutWebContentsId', () => {
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
widgetWindow.popOut = {
webContents: {
id: 'popOutID',
},
};
expect(widgetWindow.getPopOutWebContentsId()).toBe('popOutID');
});
it('onNavigate', () => {
const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
const ev = {preventDefault: jest.fn()};
widgetWindow.onNavigate(ev, widgetWindow.getWidgetURL());
expect(ev.preventDefault).not.toHaveBeenCalled();
widgetWindow.onNavigate(ev, 'http://localhost:8065/invalid/url');
expect(ev.preventDefault).toHaveBeenCalledTimes(1);
});
});
});

40
src/main/windows/callsWidgetWindow.ts
View file

@ -30,7 +30,6 @@ import {
CALLS_WIDGET_SHARE_SCREEN,
} from 'common/communication';
import webContentsEventManager from 'main/views/webContentEvents';
import Config from 'common/config';
type LoadURLOpts = {
extraHeaders: string;
@ -82,6 +81,10 @@ export default class CallsWidgetWindow extends EventEmitter {
this.win.webContents.setWindowOpenHandler(this.onPopOutOpen);
this.win.webContents.on('did-create-window', this.onPopOutCreate);
// Calls widget window is not supposed to navigate anywhere else.
this.win.webContents.on('will-navigate', this.onNavigate);
this.win.webContents.on('did-start-navigation', this.onNavigate);
this.load();
}
@ -102,6 +105,14 @@ export default class CallsWidgetWindow extends EventEmitter {
return this.config.callID;
}
private onNavigate = (ev: Event, url: string) => {
if (url === this.getWidgetURL()) {
return;
}
log.warn(`CallsWidgetWindow: prevented widget window from navigating to: ${url}`);
ev.preventDefault();
}
private load() {
const opts = {} as LoadURLOpts;
this.win.loadURL(this.getWidgetURL(), opts).catch((reason) => {
@ -209,22 +220,25 @@ export default class CallsWidgetWindow extends EventEmitter {
this.setBounds(initialBounds);
}
private onPopOutOpen = () => {
return {
action: 'allow' as const,
overrideBrowserWindowOptions: {
autoHideMenuBar: true,
},
};
private onPopOutOpen = ({url}: {url: string}) => {
if (urlUtils.isCallsPopOutURL(this.mainView.serverInfo.server.url, url, this.config.callID)) {
return {
action: 'allow' as const,
overrideBrowserWindowOptions: {
autoHideMenuBar: true,
},
};
}
log.warn(`CallsWidgetWindow.onPopOutOpen: prevented window open to ${url}`);
return {action: 'deny' as const};
}
private onPopOutCreate = (win: BrowserWindow) => {
this.popOut = win;
// Let the webContentsEventManager handle links that try to open a new window
const spellcheck = Config.useSpellChecker;
const newWindow = webContentsEventManager.generateNewWindowListener(this.popOut.webContents.id, spellcheck);
this.popOut.webContents.setWindowOpenHandler(newWindow);
webContentsEventManager.addWebContentsEventListeners(this.popOut.webContents);
}
private onPopOutFocus = () => {
@ -241,6 +255,10 @@ export default class CallsWidgetWindow extends EventEmitter {
return this.win.webContents.id;
}
public getPopOutWebContentsId() {
return this.popOut?.webContents.id;
}
public getURL() {
return urlUtils.parseURL(this.win.webContents.getURL());
}

2
src/main/windows/windowManager.ts
View file

@ -947,7 +947,7 @@ export class WindowManager {
}
getServerURLFromWebContentsId = (id: number) => {
if (this.callsWidgetWindow && id === this.callsWidgetWindow.getWebContentsId()) {
if (this.callsWidgetWindow && (id === this.callsWidgetWindow.getWebContentsId() || id === this.callsWidgetWindow.getPopOutWebContentsId())) {
return this.callsWidgetWindow.getURL();
}