feat: provide the frame URL with permission requests and checks (#18757)

* feat: provide the frame URL with permission requests and checks Also provides a handy isMainFrame property to determine if it is an iframe making the request * chore: refactor to use base::Value * chore: use Set<Type>Key over SetPath
electron · Jun 24, 2019 · a897301 · a897301
1 parent a9ada36
commit a897301
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 13 deletions.
diff --git a/atom/browser/atom_permission_manager.cc b/atom/browser/atom_permission_manager.cc
@@ -183,12 +183,14 @@ int AtomPermissionManager::RequestPermissionsWithDetails(
     const auto callback =
         base::Bind(&AtomPermissionManager::OnPermissionResponse,
                    base::Unretained(this), request_id, i);
-    if (details == nullptr) {
-      request_handler_.Run(web_contents, permission, callback,
-                           base::DictionaryValue());
-    } else {
-      request_handler_.Run(web_contents, permission, callback, *details);
-    }
+    auto mutable_details =
+        details == nullptr ? base::DictionaryValue() : details->Clone();
+    mutable_details.SetKey(
+        "requestingUrl",
+        base::Value(render_frame_host->GetLastCommittedURL().spec()));
+    mutable_details.SetKey(
+        "isMainFrame", base::Value(render_frame_host->GetParent() == nullptr));
+    request_handler_.Run(web_contents, permission, callback, mutable_details);
   }
 
   return request_id;
@@ -241,8 +243,15 @@ bool AtomPermissionManager::CheckPermissionWithDetails(
   }
   auto* web_contents =
       content::WebContents::FromRenderFrameHost(render_frame_host);
+  auto mutable_details =
+      details == nullptr ? base::DictionaryValue() : details->Clone();
+  mutable_details.SetKey(
+      "requestingUrl",
+      base::Value(render_frame_host->GetLastCommittedURL().spec()));
+  mutable_details.SetKey(
+      "isMainFrame", base::Value(render_frame_host->GetParent() == nullptr));
   return check_handler_.Run(web_contents, permission, requesting_origin,
-                            *details);
+                            mutable_details);
 }
 
 blink::mojom::PermissionStatus

diff --git a/atom/browser/atom_permission_manager.h b/atom/browser/atom_permission_manager.h
@@ -31,11 +31,11 @@ class AtomPermissionManager : public content::PermissionControllerDelegate {
   using RequestHandler = base::Callback<void(content::WebContents*,
                                              content::PermissionType,
                                              const StatusCallback&,
-                                             const base::DictionaryValue&)>;
+                                             const base::Value&)>;
   using CheckHandler = base::Callback<bool(content::WebContents*,
                                            content::PermissionType,
                                            const GURL& requesting_origin,
-                                           const base::DictionaryValue&)>;
+                                           const base::Value&)>;
 
   // Handler to dispatch permission requests in JS.
   void SetPermissionRequestHandler(const RequestHandler& handler);

diff --git a/docs/api/session.md b/docs/api/session.md
@@ -288,15 +288,17 @@ win.webContents.session.setCertificateVerifyProc((request, callback) => {
 #### `ses.setPermissionRequestHandler(handler)`
 
 * `handler` Function | null
-  * `webContents` [WebContents](web-contents.md) - WebContents requesting the permission.
+  * `webContents` [WebContents](web-contents.md) - WebContents requesting the permission.  Please note that if the request comes from a subframe you should use `requestingUrl` to check the request origin.
   * `permission` String - Enum of 'media', 'geolocation', 'notifications', 'midiSysex',
     'pointerLock', 'fullscreen', 'openExternal'.
   * `callback` Function
     * `permissionGranted` Boolean - Allow or deny the permission.
   * `details` Object - Some properties are only available on certain permission types.
-    * `externalURL` String - The url of the `openExternal` request.
-    * `mediaTypes` String[] - The types of media access being requested, elements can be `video`
+    * `externalURL` String (Optional) - The url of the `openExternal` request.
+    * `mediaTypes` String[] (Optional) - The types of media access being requested, elements can be `video`
       or `audio`
+    * `requestingUrl` String - The last URL the requesting frame loaded
+    * `isMainFrame` Boolean - Whether the frame making the request is the main frame
 
 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.
@@ -316,13 +318,15 @@ session.fromPartition('some-partition').setPermissionRequestHandler((webContents
 #### `ses.setPermissionCheckHandler(handler)`
 
 * `handler` Function<Boolean> | null
-  * `webContents` [WebContents](web-contents.md) - WebContents checking the permission.
+  * `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.
   * `permission` String - Enum of 'media'.
   * `requestingOrigin` String - The origin URL of the permission check
   * `details` Object - Some properties are only available on certain permission types.
     * `securityOrigin` String - The security orign of the `media` check.
     * `mediaType` String - The type of media access being requested, can be `video`,
       `audio` or `unknown`
+    * `requestingUrl` String - The last URL the requesting frame loaded
+    * `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.