Fixes #13471 - Permissions should show the correct host for which the…

… permissions are requested Auditors - @bsclifton, @diracdeltas
brave · Mar 26, 2018 · 4dbc4fd · 4dbc4fd
1 parent 68ecb38
commit 4dbc4fd
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 20 deletions.
diff --git a/app/filtering.js b/app/filtering.js
@@ -394,7 +394,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, permissionTypes, muonCb) => {
+  session.setPermissionRequestHandler((mainFrameOrigin, requestingUrl, permissionTypes, muonCb) => {
     if (!permissions) {
       permissions = {
         media: {
@@ -427,35 +427,35 @@ function registerPermissionHandler (session, partition) {
     // 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()
-    const isBraveOrigin = origin.startsWith(`chrome-extension://${config.braveExtensionId}/`)
-    const isPDFOrigin = origin.startsWith(`${pdfjsOrigin}/`)
+    const isBraveOrigin = mainFrameOrigin.startsWith(`chrome-extension://${config.braveExtensionId}/`)
+    const isPDFOrigin = mainFrameOrigin.startsWith(`${pdfjsOrigin}/`)
     let settings
     let tempSettings
-    if (mainFrameUrl === appUrlUtil.getBraveExtIndexHTML() || isPDFOrigin || isBraveOrigin) {
+    if (requestingUrl === appUrlUtil.getBraveExtIndexHTML() || isPDFOrigin || isBraveOrigin) {
       // lookup, display and store site settings by the origin alias
-      origin = isPDFOrigin ? 'PDF Viewer' : 'Brave Browser'
+      requestingUrl = isPDFOrigin ? 'PDF Viewer' : 'Brave Browser'
       // display on all tabs
-      mainFrameUrl = null
+      mainFrameOrigin = null
       // Lookup by exact host pattern match since 'Brave Browser' is not
       // a parseable URL
-      settings = siteSettings.getSiteSettingsForHostPattern(appState.get('siteSettings'), origin)
-      tempSettings = siteSettings.getSiteSettingsForHostPattern(appState.get('temporarySiteSettings'), origin)
-    } else if (mainFrameUrl.startsWith('magnet:')) {
+      settings = siteSettings.getSiteSettingsForHostPattern(appState.get('siteSettings'), requestingUrl)
+      tempSettings = siteSettings.getSiteSettingsForHostPattern(appState.get('temporarySiteSettings'), requestingUrl)
+    } else if (requestingUrl.startsWith('magnet:')) {
       // Show "Allow magnet URL to open an external application?", instead of
       // "Allow null to open an external application?"
       // This covers an edge case where you open a magnet link tab, then disable Torrent Viewer
       // and restart Brave. I don't think it needs localization. See 'Brave Browser' above.
-      origin = 'Magnet URL'
+      requestingUrl = 'Magnet URL'
     } else {
       // Strip trailing slash
-      origin = getOrigin(origin)
-      settings = siteSettings.getSiteSettingsForURL(appState.get('siteSettings'), origin)
-      tempSettings = siteSettings.getSiteSettingsForURL(appState.get('temporarySiteSettings'), origin)
+      requestingUrl = getOrigin(requestingUrl)
+      settings = siteSettings.getSiteSettingsForURL(appState.get('siteSettings'), requestingUrl)
+      tempSettings = siteSettings.getSiteSettingsForURL(appState.get('temporarySiteSettings'), requestingUrl)
     }
 
     let response = []
 
-    if (origin == null) {
+    if (requestingUrl == null) {
       response = new Array(permissionTypes.length)
       response.fill(false, 0, permissionTypes.length)
       muonCb(response)
@@ -471,7 +471,7 @@ function registerPermissionHandler (session, partition) {
         response.push(false)
       } else if (permission === 'fullscreen' &&
         // The Torrent Viewer extension is always allowed to show fullscreen media
-        origin.startsWith('chrome-extension://' + config.torrentExtensionId)) {
+        mainFrameOrigin.startsWith('chrome-extension://' + config.torrentExtensionId)) {
         response.push(true)
       } else if (permission === 'fullscreen' && alwaysAllowFullscreen) {
         // Always allow fullscreen if setting is ON
@@ -497,7 +497,7 @@ function registerPermissionHandler (session, partition) {
 
       // 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)
+      const message = locale.translation('permissionMessage').replace(/{{\s*host\s*}}/, getOrigin(requestingUrl) || '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]) {
@@ -511,9 +511,9 @@ function registerPermissionHandler (session, partition) {
             {text: locale.translation('deny')},
             {text: locale.translation('allow')}
           ],
-          frameOrigin: getOrigin(mainFrameUrl),
+          frameOrigin: getOrigin(mainFrameOrigin),
           options: {
-            persist: !!origin,
+            persist: !!requestingUrl,
             index: i
           },
           message
@@ -530,7 +530,7 @@ function registerPermissionHandler (session, partition) {
           response[index] = result
           if (persist) {
             // remember site setting for this host
-            appActions.changeSiteSetting(origin, permission + 'Permission', result, isPrivate)
+            appActions.changeSiteSetting(requestingUrl, permission + 'Permission', result, isPrivate)
           }
           if (response.length === permissionTypes.length) {
             permissionCallbacks[message] = null

diff --git a/js/lib/urlutil.js b/js/lib/urlutil.js
@@ -470,7 +470,10 @@ const UrlUtil = {
       if (parsed.protocol === 'about:') {
         return [parsed.protocol, parsed.path].join('')
       }
-      return parsed.origin.replace(/\/+$/, '')
+      if (parsed.origin.length !== 0) {
+        return parsed.origin.replace(/\/+$/, '')
+      }
+      return parsed.href.replace(/\/+$/, '')
     }
     if (parsed.host && parsed.protocol) {
       return parsed.slashes ? [parsed.protocol, parsed.host].join('//') : [parsed.protocol, parsed.host].join('')