feat: route frame based permission checks through our permission chec…

…k handler (#19903)

* feat: route frame based permission checks through our permission check handler

* docs: add change to setPermissionCheckHandler to breaking changes doc

docs/api/session.md

@@ -500,6 +500,7 @@ win.webContents.session.setCertificateVerifyProc((request, callback) => {
     * `pointerLock` - Request to directly interpret mouse movements as an input method. Click [here](https://developer.mozilla.org/en-US/docs/Web/API/Pointer_Lock_API) to know more.
     * `fullscreen` - Request for the app to enter fullscreen mode.
     * `openExternal` - Request to open links in external applications.
+    * `unknown` - An unrecognized permission request
   * `callback` Function
     * `permissionGranted` Boolean - Allow or deny the permission.
   * `details` Object - Some properties are only available on certain permission types.
@@ -511,7 +512,9 @@ win.webContents.session.setCertificateVerifyProc((request, callback) => {
 
 Sets the handler which can be used to respond to permission requests for the `session`.
 Calling `callback(true)` will allow the permission and `callback(false)` will reject it.
-To clear the handler, call `setPermissionRequestHandler(null)`.
+To clear the handler, call `setPermissionRequestHandler(null)`.  Please note that
+you must also implement `setPermissionCheckHandler` to get complete permission handling.
+Most web APIs do a permission check and then make a permission request if the check is denied.
 
 ```javascript
 const { session } = require('electron')
@@ -527,28 +530,32 @@ session.fromPartition('some-partition').setPermissionRequestHandler((webContents
 #### `ses.setPermissionCheckHandler(handler)`
 
 * `handler` Function\<Boolean> | null
-  * `webContents` [WebContents](web-contents.md) - WebContents checking the permission.  Please note that if the request comes from a subframe you should use `requestingUrl` to check the request origin.
+  * `webContents` ([WebContents](web-contents.md) | null) - WebContents checking the permission.  Please note that if the request comes from a subframe you should use `requestingUrl` to check the request origin.  Cross origin sub frames making permission checks will pass a `null` webContents to this handler.  You should use `embeddingOrigin` and `requestingOrigin` to determine what origin the owning frame and the requesting frame are on respectively.
   * `permission` String - Type of permission check.  Valid values are `midiSysex`, `notifications`, `geolocation`, `media`,`mediaKeySystem`,`midi`, `pointerLock`, `fullscreen`, `openExternal`, or `serial`.
   * `requestingOrigin` String - The origin URL of the permission check
   * `details` Object - Some properties are only available on certain permission types.
-    * `securityOrigin` String - The security origin of the `media` check.
-    * `mediaType` String - The type of media access being requested, can be `video`,
+    * `embeddingOrigin` String (optional) - The origin of the frame embedding the frame that made the permission check.  Only set for cross-origin sub frames making permission checks.
+    * `securityOrigin` String (optional) - The security origin of the `media` check.
+    * `mediaType` String (optional) - The type of media access being requested, can be `video`,
       `audio` or `unknown`
-    * `requestingUrl` String - The last URL the requesting frame loaded
+    * `requestingUrl` String (optional) - The last URL the requesting frame loaded.  This is not provided for cross-origin sub frames making permission checks.
     * `isMainFrame` Boolean - Whether the frame making the request is the main frame
 
 Sets the handler which can be used to respond to permission checks for the `session`.
-Returning `true` will allow the permission and `false` will reject it.
+Returning `true` will allow the permission and `false` will reject it.  Please note that
+you must also implement `setPermissionRequestHandler` to get complete permission handling.
+Most web APIs do a permission check and then make a permission request if the check is denied.
 To clear the handler, call `setPermissionCheckHandler(null)`.
 
 ```javascript
 const { session } = require('electron')
-session.fromPartition('some-partition').setPermissionCheckHandler((webContents, permission) => {
-  if (webContents.getURL() === 'some-host' && permission === 'notifications') {
-    return false // denied
+const url = require('url')
+session.fromPartition('some-partition').setPermissionCheckHandler((webContents, permission, requestingOrigin) => {
+  if (new URL(requestingOrigin).hostname === 'some-host' && permission === 'notifications') {
+    return true // granted
   }
 
-  return true
+  return false // denied
 })
 ```
 

docs/breaking-changes.md

@@ -14,6 +14,28 @@ This document uses the following convention to categorize breaking changes:
 
 ## Planned Breaking API Changes (13.0)
 
+### API Changed: `session.setPermissionCheckHandler(handler)`
+
+The `handler` methods first parameter was previously always a `webContents`, it can now sometimes be `null`.  You should use the `requestingOrigin`, `embeddingOrigin` and `securityOrigin` properties to respond to the permission check correctly.  As the `webContents` can be `null` it can no longer be relied on.
+
+```js
+// Old code
+session.setPermissionCheckHandler((webContents, permission) => {
+  if (webContents.getURL().startsWith('https://google.com/') && permission === 'notification') {
+    return true
+  }
+  return false
+})
+
+// Replace with
+session.setPermissionCheckHandler((webContents, permission, requestingOrigin) => {
+  if (new URL(requestingOrigin).hostname === 'google.com' && permission === 'notification') {
+    return true
+  }
+  return false
+})
+```
+
 ### Removed: `shell.moveItemToTrash()`
 
 The deprecated synchronous `shell.moveItemToTrash()` API has been removed. Use

shell/browser/electron_permission_manager.cc

@@ -224,7 +224,12 @@ blink::mojom::PermissionStatus ElectronPermissionManager::GetPermissionStatus(
     content::PermissionType permission,
     const GURL& requesting_origin,
     const GURL& embedding_origin) {
-  return blink::mojom::PermissionStatus::GRANTED;
+  base::DictionaryValue details;
+  details.SetString("embeddingOrigin", embedding_origin.spec());
+  bool granted = CheckPermissionWithDetails(permission, nullptr,
+                                            requesting_origin, &details);
+  return granted ? blink::mojom::PermissionStatus::GRANTED
+                 : blink::mojom::PermissionStatus::DENIED;
 }
 
 int ElectronPermissionManager::SubscribePermissionStatusChange(
@@ -247,13 +252,28 @@ bool ElectronPermissionManager::CheckPermissionWithDetails(
     return true;
   }
   auto* web_contents =
-      content::WebContents::FromRenderFrameHost(render_frame_host);
+      render_frame_host
+          ? content::WebContents::FromRenderFrameHost(render_frame_host)
+          : nullptr;
   auto mutable_details =
       details == nullptr ? base::DictionaryValue() : details->Clone();
-  mutable_details.SetStringKey("requestingUrl",
-                               render_frame_host->GetLastCommittedURL().spec());
-  mutable_details.SetBoolKey("isMainFrame",
-                             render_frame_host->GetParent() == nullptr);
+  if (render_frame_host) {
+    mutable_details.SetStringKey(
+        "requestingUrl", render_frame_host->GetLastCommittedURL().spec());
+  }
+  mutable_details.SetBoolKey(
+      "isMainFrame",
+      render_frame_host && render_frame_host->GetParent() == nullptr);
+  switch (permission) {
+    case content::PermissionType::AUDIO_CAPTURE:
+      mutable_details.SetStringKey("mediaType", "audio");
+      break;
+    case content::PermissionType::VIDEO_CAPTURE:
+      mutable_details.SetStringKey("mediaType", "video");
+      break;
+    default:
+      break;
+  }
   return check_handler_.Run(web_contents, permission, requesting_origin,
                             mutable_details);
 }
@@ -263,7 +283,11 @@ ElectronPermissionManager::GetPermissionStatusForFrame(
     content::PermissionType permission,
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin) {
-  return blink::mojom::PermissionStatus::GRANTED;
+  base::DictionaryValue details;
+  bool granted = CheckPermissionWithDetails(permission, render_frame_host,
+                                            requesting_origin, &details);
+  return granted ? blink::mojom::PermissionStatus::GRANTED
+                 : blink::mojom::PermissionStatus::DENIED;
 }
 
 }  // namespace electron