phlux/mattermostest
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

[MM-52587] Clean up URL utils, use isInternalURL when possible (#2702)

Browse Source
This commit is contained in:
Devin Binnie
2023-05-03 08:48:41 -04:00
committed by GitHub
parent f3a4417464
commit e227c6bf1d
30 changed files with 481 additions and 634 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/main/app/app.test.js
View File

@@ -159,7 +159,7 @@ describe('main/app/app', () => {
await handleAppCertificateError(event, webContents, testURL, 'error-1', certificate, callback);
expect(callback).toHaveBeenCalledWith(true);
expect(certificateErrorCallbacks.has('http://server-1.com:error-1')).toBe(false);
expect(CertificateStore.add).toHaveBeenCalledWith('http://server-1.com', certificate);
expect(CertificateStore.add).toHaveBeenCalledWith(new URL('http://server-1.com'), certificate);
expect(CertificateStore.save).toHaveBeenCalled();
});
@@ -175,7 +175,7 @@ describe('main/app/app', () => {
await handleAppCertificateError(event, webContents, testURL, 'error-1', certificate, callback);
expect(callback).toHaveBeenCalledWith(false);
expect(certificateErrorCallbacks.has('http://server-1.com:error-1')).toBe(false);
expect(CertificateStore.add).toHaveBeenCalledWith('http://server-1.com', certificate, true);
expect(CertificateStore.add).toHaveBeenCalledWith(new URL('http://server-1.com'), certificate, true);
expect(CertificateStore.save).toHaveBeenCalled();
});
});

27
src/main/app/app.ts
View File

@@ -4,7 +4,7 @@
import {app, BrowserWindow, Event, dialog, WebContents, Certificate, Details} from 'electron';
import {Logger} from 'common/log';
import urlUtils from 'common/utils/url';
import {parseURL} from 'common/utils/url';
import updateManager from 'main/autoUpdater';
import CertificateStore from 'main/certificateStore';
@@ -80,27 +80,26 @@ export function handleAppBeforeQuit() {
export async function handleAppCertificateError(event: Event, webContents: WebContents, url: string, error: string, certificate: Certificate, callback: (isTrusted: boolean) => void) {
log.verbose('handleAppCertificateError', {url, error, certificate});
const parsedURL = urlUtils.parseURL(url);
const parsedURL = parseURL(url);
if (!parsedURL) {
return;
}
const origin = parsedURL.origin;
if (CertificateStore.isExplicitlyUntrusted(origin)) {
if (CertificateStore.isExplicitlyUntrusted(parsedURL)) {
event.preventDefault();
log.warn(`Ignoring previously untrusted certificate for ${origin}`);
log.warn(`Ignoring previously untrusted certificate for ${parsedURL.origin}`);
callback(false);
} else if (CertificateStore.isTrusted(origin, certificate)) {
} else if (CertificateStore.isTrusted(parsedURL, certificate)) {
event.preventDefault();
callback(true);
} else {
// update the callback
const errorID = `${origin}:${error}`;
const errorID = `${parsedURL.origin}:${error}`;
const view = ViewManager.getViewByWebContentsId(webContents.id);
if (view?.tab.server) {
const serverURL = urlUtils.parseURL(view.tab.server.url);
if (serverURL && serverURL.origin !== origin) {
log.warn(`Ignoring certificate for unmatched origin ${origin}, will not trust`);
const serverURL = parseURL(view.tab.server.url);
if (serverURL && serverURL.origin !== parsedURL.origin) {
log.warn(`Ignoring certificate for unmatched origin ${parsedURL.origin}, will not trust`);
callback(false);
return;
}
@@ -112,8 +111,8 @@ export async function handleAppCertificateError(event: Event, webContents: WebCo
certificateErrorCallbacks.set(errorID, callback);
return;
}
const extraDetail = CertificateStore.isExisting(origin) ? localizeMessage('main.app.app.handleAppCertificateError.dialog.extraDetail', 'Certificate is different from previous one.\n\n') : '';
const detail = localizeMessage('main.app.app.handleAppCertificateError.certError.dialog.detail', '{extraDetail}origin: {origin}\nError: {error}', {extraDetail, origin, error});
const extraDetail = CertificateStore.isExisting(parsedURL) ? localizeMessage('main.app.app.handleAppCertificateError.dialog.extraDetail', 'Certificate is different from previous one.\n\n') : '';
const detail = localizeMessage('main.app.app.handleAppCertificateError.certError.dialog.detail', '{extraDetail}origin: {origin}\nError: {error}', {extraDetail, origin: parsedURL.origin, error});
certificateErrorCallbacks.set(errorID, callback);
@@ -154,7 +153,7 @@ export async function handleAppCertificateError(event: Event, webContents: WebCo
}
if (result.response === 0) {
CertificateStore.add(origin, certificate);
CertificateStore.add(parsedURL, certificate);
CertificateStore.save();
certificateErrorCallbacks.get(errorID)(true);
@@ -165,7 +164,7 @@ export async function handleAppCertificateError(event: Event, webContents: WebCo
}
} else {
if (result.checkboxChecked) {
CertificateStore.add(origin, certificate, true);
CertificateStore.add(parsedURL, certificate, true);
CertificateStore.save();
}
certificateErrorCallbacks.get(errorID)(false);

8
src/main/app/initialize.test.js
View File

@@ -6,7 +6,7 @@ import path from 'path';
import {app, session} from 'electron';
import Config from 'common/config';
import urlUtils from 'common/utils/url';
import {parseURL, isTrustedURL} from 'common/utils/url';
import parseArgs from 'main/ParseArgs';
import ViewManager from 'main/views/viewManager';
@@ -105,6 +105,7 @@ jest.mock('common/config', () => ({
}));
jest.mock('common/utils/url', () => ({
parseURL: jest.fn(),
isTrustedURL: jest.fn(),
}));
@@ -283,6 +284,9 @@ describe('main/app/initialize', () => {
},
},
});
parseURL.mockImplementation((url) => new URL(url));
isTrustedURL.mockImplementation((url) => url.toString() === 'http://server-1.com/');
let callback = jest.fn();
session.defaultSession.setPermissionRequestHandler.mockImplementation((cb) => {
cb({id: 1, getURL: () => 'http://server-1.com'}, 'bad-permission', callback);
@@ -298,8 +302,6 @@ describe('main/app/initialize', () => {
await initialize();
expect(callback).toHaveBeenCalledWith(true);
urlUtils.isTrustedURL.mockImplementation((url) => url === 'http://server-1.com');
callback = jest.fn();
session.defaultSession.setPermissionRequestHandler.mockImplementation((cb) => {
cb({id: 2, getURL: () => 'http://server-1.com'}, 'openExternal', callback);

10
src/main/app/initialize.ts
View File

@@ -43,8 +43,8 @@ import {
DOUBLE_CLICK_ON_WINDOW,
} from 'common/communication';
import Config from 'common/config';
import {isTrustedURL, parseURL} from 'common/utils/url';
import {Logger} from 'common/log';
import urlUtils from 'common/utils/url';
import AllowProtocolDialog from 'main/allowProtocolDialog';
import AppVersionManager from 'main/AppVersionManager';
@@ -460,8 +460,14 @@ async function initializeAfterAppReady() {
return;
}
const parsedURL = parseURL(requestingURL);
if (!parsedURL) {
callback(false);
return;
}
// is the requesting url trusted?
callback(urlUtils.isTrustedURL(requestingURL, serverURL));
callback(isTrustedURL(parsedURL, serverURL));
});
if (wasUpdated(AppVersionManager.lastAppVersion)) {

4
src/main/app/utils.ts
View File

@@ -16,8 +16,8 @@ import {Logger} from 'common/log';
import JsonFileManager from 'common/JsonFileManager';
import ServerManager from 'common/servers/serverManager';
import {MattermostServer} from 'common/servers/MattermostServer';
import urlUtils from 'common/utils/url';
import {APP_MENU_WILL_CLOSE} from 'common/communication';
import {isValidURI} from 'common/utils/url';
import updateManager from 'main/autoUpdater';
import {migrationInfoPath, updatePaths} from 'main/constants';
@@ -72,7 +72,7 @@ export function getDeeplinkingURL(args: string[]) {
if (Array.isArray(args) && args.length) {
// deeplink urls should always be the last argument, but may not be the first (i.e. Windows with the app already running)
const url = args[args.length - 1];
if (url && mainProtocol && url.startsWith(mainProtocol) && urlUtils.isValidURI(url)) {
if (url && mainProtocol && url.startsWith(mainProtocol) && isValidURI(url)) {
return url;
}
}