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

[MM-58089] Disallow redirects to untrusted URLs without a permission prompt (#3024)

Browse Source 
* [MM-58089] Disallow redirects to untrusted URLs without a permission prompt

* Fix types

* Add test
This commit is contained in:
Devin Binnie
2024-05-08 11:12:09 -04:00
committed by GitHub
parent e623fd1536
commit b411437a15
4 changed files with 29 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/notifications/index.ts
View File

@@ -55,7 +55,7 @@ class NotificationManager {
soundName,
};
if (!await PermissionsManager.doPermissionRequest(webcontents.id, 'notifications', view.view.server.url.toString())) {
if (!await PermissionsManager.doPermissionRequest(webcontents.id, 'notifications', {requestingUrl: view.view.server.url.toString(), isMainFrame: false})) {
return {status: 'not_sent', reason: 'notifications_permission_disallowed'};
}

17
src/main/permissionsManager.test.js
View File

@@ -109,7 +109,7 @@ describe('main/PermissionsManager', () => {
it('should allow if dialog is not required', async () => {
const permissionsManager = new PermissionsManager('anyfile.json');
const cb = jest.fn();
await permissionsManager.handlePermissionRequest({id: 2}, 'fullscreen', cb, {securityOrigin: 'http://anyurl.com'});
await permissionsManager.handlePermissionRequest({id: 2}, 'fullscreen', cb, {requestingUrl: 'http://anyurl.com'});
expect(cb).toHaveBeenCalledWith(true);
});
@@ -181,9 +181,9 @@ describe('main/PermissionsManager', () => {
const cb = jest.fn();
dialog.showMessageBox.mockReturnValue(Promise.resolve({response: 0}));
await Promise.all([
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {securityOrigin: 'http://anyurl.com'}),
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {securityOrigin: 'http://anyurl.com'}),
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {securityOrigin: 'http://anyurl.com'}),
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}),
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}),
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}),
]);
expect(dialog.showMessageBox).toHaveBeenCalledTimes(1);
});
@@ -203,4 +203,13 @@ describe('main/PermissionsManager', () => {
await permissionsManager.handlePermissionRequest({id: 2}, 'media', cb, {securityOrigin: 'http://anyurl.com'});
expect(dialog.showMessageBox).toHaveBeenCalled();
});
it('should pop dialog for external applications', async () => {
const permissionsManager = new PermissionsManager('anyfile.json');
permissionsManager.writeToFile = jest.fn();
const cb = jest.fn();
dialog.showMessageBox.mockReturnValue(Promise.resolve({response: 0}));
await permissionsManager.handlePermissionRequest({id: 2}, 'openExternal', cb, {requestingUrl: 'http://anyurl.com', externalURL: 'ms-excel://differenturl.com'});
expect(dialog.showMessageBox).toHaveBeenCalled();
});
});

18
src/main/permissionsManager.ts
View File

@@ -38,6 +38,7 @@ const authorizablePermissionTypes = [
'media',
'geolocation',
'notifications',
'openExternal',
];
type Permissions = {
@@ -67,16 +68,16 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
callback(await this.doPermissionRequest(
webContents.id,
permission,
details.securityOrigin ?? details.requestingUrl,
details,
));
};
doPermissionRequest = async (
webContentsId: number,
permission: string,
requestingURL: string,
details: PermissionRequestHandlerHandlerDetails,
) => {
log.debug('doPermissionRequest', requestingURL, permission);
log.debug('doPermissionRequest', permission, details);
// is the requested permission type supported?
if (!supportedPermissionTypes.includes(permission)) {
@@ -89,7 +90,12 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
return true;
}
const parsedURL = parseURL(requestingURL);
let url = details.requestingUrl;
if (permission === 'media' && details.securityOrigin) {
url = details.securityOrigin;
}
const parsedURL = parseURL(url);
if (!parsedURL) {
return false;
}
@@ -142,7 +148,7 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
// Show the dialog to ask the user
const {response} = await dialog.showMessageBox(mainWindow, {
title: localizeMessage('main.permissionsManager.checkPermission.dialog.title', 'Permission Requested'),
message: localizeMessage(`main.permissionsManager.checkPermission.dialog.message.${permission}`, '{appName} ({url}) is requesting the "{permission}" permission.', {appName: app.name, url: parsedURL.origin, permission}),
message: localizeMessage(`main.permissionsManager.checkPermission.dialog.message.${permission}`, '{appName} ({url}) is requesting the "{permission}" permission.', {appName: app.name, url: parsedURL.origin, permission, externalURL: details.externalURL}),
detail: localizeMessage(`main.permissionsManager.checkPermission.dialog.detail.${permission}`, 'Would you like to grant {appName} this permission?', {appName: app.name}),
type: 'question',
buttons: [
@@ -178,9 +184,11 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
t('main.permissionsManager.checkPermission.dialog.message.media');
t('main.permissionsManager.checkPermission.dialog.message.geolocation');
t('main.permissionsManager.checkPermission.dialog.message.notifications');
t('main.permissionsManager.checkPermission.dialog.message.openExternal');
t('main.permissionsManager.checkPermission.dialog.detail.media');
t('main.permissionsManager.checkPermission.dialog.detail.geolocation');
t('main.permissionsManager.checkPermission.dialog.detail.notifications');
t('main.permissionsManager.checkPermission.dialog.detail.openExternal');
let permissionsManager = new PermissionsManager(permissionsJson);