From 16a6c86fe4a6c5383d6cbb183efb4a6ccde4f163 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 20 Sep 2017 09:52:52 -0400 Subject: [PATCH] Merge pull request #10896 from brave/issue-10864 handle an array of permission types --- app/filtering.js | 147 ++++++++++-------- .../components/main/notificationItem.js | 9 +- 2 files changed, 84 insertions(+), 72 deletions(-) diff --git a/app/filtering.js b/app/filtering.js index 9a7c0fb0e60..04db46eccec 100644 --- a/app/filtering.js +++ b/app/filtering.js @@ -388,7 +388,7 @@ function registerPermissionHandler (session, partition) { const isPrivate = module.exports.isPrivate(partition) // Keep track of per-site permissions granted for this session. let permissions = null - session.setPermissionRequestHandler((origin, mainFrameUrl, permission, cb) => { + session.setPermissionRequestHandler((origin, mainFrameUrl, permissionTypes, cb) => { if (!permissions) { permissions = { media: { @@ -418,34 +418,7 @@ function registerPermissionHandler (session, partition) { } } - if (!permissions[permission]) { - console.warn('WARNING: got unregistered permission request', permission) - cb(false) - return - } - - // The Torrent Viewer extension is always allowed to show fullscreen media - if (permission === 'fullscreen' && - origin.startsWith('chrome-extension://' + config.torrentExtensionId)) { - cb(true) - return - } - - // Always allow fullscreen if setting is ON - const alwaysAllowFullscreen = module.exports.alwaysAllowFullscreen() === fullscreenOption.ALWAYS_ALLOW - if (permission === 'fullscreen' && alwaysAllowFullscreen) { - cb(true) - return - } - - // The Brave extension and PDFJS are always allowed to open files in an external app - if (permission === 'openExternal' && ( - origin.startsWith('chrome-extension://' + config.PDFJSExtensionId) || - origin.startsWith('chrome-extension://' + config.braveExtensionId))) { - cb(true) - return - } - + // TODO(bridiver) - the permission handling should be converted to an action because we should never call `appStore.getState()` // Check whether there is a persistent site setting for this host const appState = appStore.getState() let settings @@ -473,52 +446,88 @@ function registerPermissionHandler (session, partition) { tempSettings = siteSettings.getSiteSettingsForURL(appState.get('temporarySiteSettings'), origin) } - const permissionName = permission + 'Permission' - let isAllowed - if (settings) { - isAllowed = settings.get(permissionName) - } - // Private tabs inherit settings from normal tabs, but not vice versa. - if (isPrivate && tempSettings) { - isAllowed = tempSettings.get(permissionName) - } - if (typeof isAllowed === 'boolean') { - cb(isAllowed) - return - } + let response = [] + for (let i = 0; i < permissionTypes.length; i++) { + const responseSizeThisIteration = response.length + const permission = permissionTypes[i] + const alwaysAllowFullscreen = module.exports.alwaysAllowFullscreen() === fullscreenOption.ALWAYS_ALLOW + if (!permissions[permission]) { + console.warn('WARNING: got unregistered permission request', permission) + response.push(false) + } else if (permission === 'fullscreen' && + // The Torrent Viewer extension is always allowed to show fullscreen media + origin.startsWith('chrome-extension://' + config.torrentExtensionId)) { + response.push(true) + } else if (permission === 'fullscreen' && alwaysAllowFullscreen) { + // Always allow fullscreen if setting is ON + response.push(true) + } else if (permission === 'openExternal' && ( + // The Brave extension and PDFJS are always allowed to open files in an external app + origin.startsWith('chrome-extension://' + config.PDFJSExtensionId) || + origin.startsWith('chrome-extension://' + config.braveExtensionId))) { + response.push(true) + } else { + const permissionName = permission + 'Permission' + let isAllowed + if (settings) { + isAllowed = settings.get(permissionName) + } + // Private tabs inherit settings from normal tabs, but not vice versa. + if (isPrivate && tempSettings) { + isAllowed = tempSettings.get(permissionName) + } + if (typeof isAllowed === 'boolean') { + response.push(isAllowed) + } + } - // Display 'Brave Browser' if the origin is null; ex: when a mailto: link - // is opened in a new tab via right-click - const message = locale.translation('permissionMessage').replace(/{{\s*host\s*}}/, origin || 'Brave Browser').replace(/{{\s*permission\s*}}/, permissions[permission].action) + // Display 'Brave Browser' if the origin is null; ex: when a mailto: link + // is opened in a new tab via right-click + const message = locale.translation('permissionMessage').replace(/{{\s*host\s*}}/, origin || 'Brave Browser').replace(/{{\s*permission\s*}}/, permissions[permission].action) - // If this is a duplicate, clear the previous callback and use the new one - if (permissionCallbacks[message]) { - permissionCallbacks[message](0, false) - } + // If this is a duplicate, clear the previous callback and use the new one + if (permissionCallbacks[message]) { + permissionCallbacks[message](0, false) + } - appActions.showNotification({ - buttons: [ - {text: locale.translation('deny')}, - {text: locale.translation('allow')} - ], - frameOrigin: getOrigin(mainFrameUrl), - options: { - persist: !!origin - }, - message - }) + const responseAutoAdded = responseSizeThisIteration !== response.length + if (!responseAutoAdded) { + appActions.showNotification({ + buttons: [ + {text: locale.translation('deny')}, + {text: locale.translation('allow')} + ], + frameOrigin: getOrigin(mainFrameUrl), + options: { + persist: !!origin, + index: i + }, + message + }) - permissionCallbacks[message] = (buttonIndex, persist) => { - permissionCallbacks[message] = null - // hide the notification if this was triggered automatically - appActions.hideNotification(message) - const result = !!(buttonIndex) - cb(result) - if (persist) { - // remember site setting for this host - appActions.changeSiteSetting(origin, permission + 'Permission', result, isPrivate) + // Use a closure here for the index instead of passing an index to the + // function because ipcMain.on(messages.NOTIFICATION_RESPONSE above + // calls into the callback without knowing an index. + const index = i + permissionCallbacks[message] = (buttonIndex, persist) => { + // hide the notification if this was triggered automatically + appActions.hideNotification(message) + const result = !!(buttonIndex) + response[index] = result + if (persist) { + // remember site setting for this host + appActions.changeSiteSetting(origin, permission + 'Permission', result, isPrivate) + } + if (response.length === permissionTypes.length) { + permissionCallbacks[message] = null + cb(response) + } + } } } + if (response.length === permissionTypes.length) { + cb(response) + } }) } diff --git a/app/renderer/components/main/notificationItem.js b/app/renderer/components/main/notificationItem.js index 3ba54190f8b..847f948cbe9 100644 --- a/app/renderer/components/main/notificationItem.js +++ b/app/renderer/components/main/notificationItem.js @@ -32,19 +32,21 @@ class NotificationItem extends React.Component { clickHandler (buttonIndex) { if (this.props.nonce) { - ipc.emit( + ipc.send( messages.NOTIFICATION_RESPONSE + this.props.nonce, {}, this.props.message, buttonIndex, - this.checkbox ? this.checkbox.checked : false + this.checkbox ? this.checkbox.checked : false, + this.props.index ) } else { ipc.send( messages.NOTIFICATION_RESPONSE, this.props.message, buttonIndex, - this.checkbox ? this.checkbox.checked : false + this.checkbox ? this.checkbox.checked : false, + this.props.index ) } } @@ -74,6 +76,7 @@ class NotificationItem extends React.Component { props.advancedLink = notification.getIn(['options', 'advancedLink']) props.persist = notification.getIn(['options', 'persist']) props.nonce = notification.getIn(['options', 'nonce']) + props.index = notification.getIn(['options', 'index']) return props }