* [MM-58089] Disallow redirects to untrusted URLs without a permission prompt
* Fix types
* Add test
(cherry picked from commit b411437a15
)
Co-authored-by: Devin Binnie <52460000+devinbinnie@users.noreply.github.com>
This commit is contained in:
parent
a1244be5ac
commit
1c9fc719dc
|
@ -113,9 +113,11 @@
|
||||||
"main.permissionsManager.checkPermission.dialog.detail.geolocation": "{appName} will use the location for setting up your timezone. You can always change this later in your computer's settings.",
|
"main.permissionsManager.checkPermission.dialog.detail.geolocation": "{appName} will use the location for setting up your timezone. You can always change this later in your computer's settings.",
|
||||||
"main.permissionsManager.checkPermission.dialog.detail.media": "{appName} will use the microphone and camera for calls and voice messages. You can always change this later in your computer's settings.",
|
"main.permissionsManager.checkPermission.dialog.detail.media": "{appName} will use the microphone and camera for calls and voice messages. You can always change this later in your computer's settings.",
|
||||||
"main.permissionsManager.checkPermission.dialog.detail.notifications": "{appName} will send you notifications for messages and calls. You can configure your notification preferences in Settings.",
|
"main.permissionsManager.checkPermission.dialog.detail.notifications": "{appName} will send you notifications for messages and calls. You can configure your notification preferences in Settings.",
|
||||||
|
"main.permissionsManager.checkPermission.dialog.detail.openExternal": "{appName} will open the requested link in an external application. If you do not trust this link, or do not recognize it, click Deny. You can always change this later in your computer's settings.",
|
||||||
"main.permissionsManager.checkPermission.dialog.message.geolocation": "{appName} ({url}) would like to access your location.",
|
"main.permissionsManager.checkPermission.dialog.message.geolocation": "{appName} ({url}) would like to access your location.",
|
||||||
"main.permissionsManager.checkPermission.dialog.message.media": "{appName} ({url}) would like to access the microphone and camera.",
|
"main.permissionsManager.checkPermission.dialog.message.media": "{appName} ({url}) would like to access the microphone and camera.",
|
||||||
"main.permissionsManager.checkPermission.dialog.message.notifications": "{appName} ({url}) would like to send you notifications.",
|
"main.permissionsManager.checkPermission.dialog.message.notifications": "{appName} ({url}) would like to send you notifications.",
|
||||||
|
"main.permissionsManager.checkPermission.dialog.message.openExternal": "{appName} ({url}) would like permission to open the following URL: {externalURL}",
|
||||||
"main.permissionsManager.checkPermission.dialog.title": "Permission Requested",
|
"main.permissionsManager.checkPermission.dialog.title": "Permission Requested",
|
||||||
"main.tray.tray.expired": "Session Expired: Please sign in to continue receiving notifications.",
|
"main.tray.tray.expired": "Session Expired: Please sign in to continue receiving notifications.",
|
||||||
"main.tray.tray.mention": "You have been mentioned",
|
"main.tray.tray.mention": "You have been mentioned",
|
||||||
|
|
|
@ -55,7 +55,7 @@ class NotificationManager {
|
||||||
soundName,
|
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'};
|
return {status: 'not_sent', reason: 'notifications_permission_disallowed'};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -109,7 +109,7 @@ describe('main/PermissionsManager', () => {
|
||||||
it('should allow if dialog is not required', async () => {
|
it('should allow if dialog is not required', async () => {
|
||||||
const permissionsManager = new PermissionsManager('anyfile.json');
|
const permissionsManager = new PermissionsManager('anyfile.json');
|
||||||
const cb = jest.fn();
|
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);
|
expect(cb).toHaveBeenCalledWith(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -181,9 +181,9 @@ describe('main/PermissionsManager', () => {
|
||||||
const cb = jest.fn();
|
const cb = jest.fn();
|
||||||
dialog.showMessageBox.mockReturnValue(Promise.resolve({response: 0}));
|
dialog.showMessageBox.mockReturnValue(Promise.resolve({response: 0}));
|
||||||
await Promise.all([
|
await Promise.all([
|
||||||
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, {securityOrigin: 'http://anyurl.com'}),
|
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}),
|
||||||
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {securityOrigin: 'http://anyurl.com'}),
|
permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}),
|
||||||
]);
|
]);
|
||||||
expect(dialog.showMessageBox).toHaveBeenCalledTimes(1);
|
expect(dialog.showMessageBox).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
@ -203,4 +203,13 @@ describe('main/PermissionsManager', () => {
|
||||||
await permissionsManager.handlePermissionRequest({id: 2}, 'media', cb, {securityOrigin: 'http://anyurl.com'});
|
await permissionsManager.handlePermissionRequest({id: 2}, 'media', cb, {securityOrigin: 'http://anyurl.com'});
|
||||||
expect(dialog.showMessageBox).toHaveBeenCalled();
|
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();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -38,6 +38,7 @@ const authorizablePermissionTypes = [
|
||||||
'media',
|
'media',
|
||||||
'geolocation',
|
'geolocation',
|
||||||
'notifications',
|
'notifications',
|
||||||
|
'openExternal',
|
||||||
];
|
];
|
||||||
|
|
||||||
type Permissions = {
|
type Permissions = {
|
||||||
|
@ -67,16 +68,16 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
|
||||||
callback(await this.doPermissionRequest(
|
callback(await this.doPermissionRequest(
|
||||||
webContents.id,
|
webContents.id,
|
||||||
permission,
|
permission,
|
||||||
details.securityOrigin ?? details.requestingUrl,
|
details,
|
||||||
));
|
));
|
||||||
};
|
};
|
||||||
|
|
||||||
doPermissionRequest = async (
|
doPermissionRequest = async (
|
||||||
webContentsId: number,
|
webContentsId: number,
|
||||||
permission: string,
|
permission: string,
|
||||||
requestingURL: string,
|
details: PermissionRequestHandlerHandlerDetails,
|
||||||
) => {
|
) => {
|
||||||
log.debug('doPermissionRequest', requestingURL, permission);
|
log.debug('doPermissionRequest', permission, details);
|
||||||
|
|
||||||
// is the requested permission type supported?
|
// is the requested permission type supported?
|
||||||
if (!supportedPermissionTypes.includes(permission)) {
|
if (!supportedPermissionTypes.includes(permission)) {
|
||||||
|
@ -89,7 +90,12 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
const parsedURL = parseURL(requestingURL);
|
let url = details.requestingUrl;
|
||||||
|
if (permission === 'media' && details.securityOrigin) {
|
||||||
|
url = details.securityOrigin;
|
||||||
|
}
|
||||||
|
|
||||||
|
const parsedURL = parseURL(url);
|
||||||
if (!parsedURL) {
|
if (!parsedURL) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
@ -142,7 +148,7 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
|
||||||
// Show the dialog to ask the user
|
// Show the dialog to ask the user
|
||||||
const {response} = await dialog.showMessageBox(mainWindow, {
|
const {response} = await dialog.showMessageBox(mainWindow, {
|
||||||
title: localizeMessage('main.permissionsManager.checkPermission.dialog.title', 'Permission Requested'),
|
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}),
|
detail: localizeMessage(`main.permissionsManager.checkPermission.dialog.detail.${permission}`, 'Would you like to grant {appName} this permission?', {appName: app.name}),
|
||||||
type: 'question',
|
type: 'question',
|
||||||
buttons: [
|
buttons: [
|
||||||
|
@ -178,9 +184,11 @@ export class PermissionsManager extends JsonFileManager<Permissions> {
|
||||||
t('main.permissionsManager.checkPermission.dialog.message.media');
|
t('main.permissionsManager.checkPermission.dialog.message.media');
|
||||||
t('main.permissionsManager.checkPermission.dialog.message.geolocation');
|
t('main.permissionsManager.checkPermission.dialog.message.geolocation');
|
||||||
t('main.permissionsManager.checkPermission.dialog.message.notifications');
|
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.media');
|
||||||
t('main.permissionsManager.checkPermission.dialog.detail.geolocation');
|
t('main.permissionsManager.checkPermission.dialog.detail.geolocation');
|
||||||
t('main.permissionsManager.checkPermission.dialog.detail.notifications');
|
t('main.permissionsManager.checkPermission.dialog.detail.notifications');
|
||||||
|
t('main.permissionsManager.checkPermission.dialog.detail.openExternal');
|
||||||
|
|
||||||
let permissionsManager = new PermissionsManager(permissionsJson);
|
let permissionsManager = new PermissionsManager(permissionsJson);
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue