chore: refactor persisting permission granted to serial ports (#31418)

(cherry picked from commit 515785aabe757ae7fa8f7bee94b5e8cffc448031)

docs/api/session.md

@@ -297,6 +297,35 @@ app.whenReady().then(() => {
     width: 800,
     height: 600
   })
+
+  win.webContents.session.setPermissionCheckHandler((webContents, permission, requestingOrigin, details) => {
+    if (permission === 'serial') {
+      // Add logic here to determine if permission should be given to allow serial selection
+      return true
+    }
+    return false
+  })
+
+  // Optionally, retrieve previously persisted devices from a persistent store
+  const grantedDevices = fetchGrantedDevices()
+
+  win.webContents.session.setDevicePermissionHandler((details) => {
+    if (new URL(details.origin).hostname === 'some-host' && details.deviceType === 'serial') {
+      if (details.device.vendorId === 123 && details.device.productId === 345) {
+        // Always allow this type of device (this allows skipping the call to `navigator.serial.requestPort` first)
+        return true
+      }
+
+      // Search through the list of devices that have previously been granted permission
+      return grantedDevices.some((grantedDevice) => {
+        return grantedDevice.vendorId === details.device.vendorId &&
+              grantedDevice.productId === details.device.productId &&
+              grantedDevice.serialNumber && grantedDevice.serialNumber === details.device.serialNumber
+      })
+    }
+    return false
+  })
+
   win.webContents.session.on('select-serial-port', (event, portList, webContents, callback) => {
     event.preventDefault()
     const selectedPort = portList.find((device) => {
@@ -646,9 +675,9 @@ session.fromPartition('some-partition').setPermissionCheckHandler((webContents,
 
 * `handler` Function\<Boolean> | null
   * `details` Object
-    * `deviceType` String - The type of device that permission is being requested on, can be `hid`.
+    * `deviceType` String - The type of device that permission is being requested on, can be `hid` or `serial`.
     * `origin` String - The origin URL of the device permission check.
-    * `device` [HIDDevice](structures/hid-device.md) - the device that permission is being requested for.
+    * `device` [HIDDevice](structures/hid-device.md) | [SerialPort](structures/serial-port.md)- the device that permission is being requested for.
     * `frame` [WebFrameMain](web-frame-main.md) - WebFrameMain checking the device permission.
 
 Sets the handler which can be used to respond to device permission checks for the `session`.
@@ -673,6 +702,8 @@ app.whenReady().then(() => {
     if (permission === 'hid') {
       // Add logic here to determine if permission should be given to allow HID selection
       return true
+    } else if (permission === 'serial') {
+      // Add logic here to determine if permission should be given to allow serial port selection
     }
     return false
   })
@@ -693,6 +724,11 @@ app.whenReady().then(() => {
               grantedDevice.productId === details.device.productId &&
               grantedDevice.serialNumber && grantedDevice.serialNumber === details.device.serialNumber
       })
+    } else if (details.deviceType === 'serial') {
+      if (details.device.vendorId === 123 && details.device.productId === 345) {
+        // Always allow this type of device (this allows skipping the call to `navigator.hid.requestDevice` first)
+        return true
+      }
     }
     return false
   })

docs/tutorial/devices.md

@@ -84,13 +84,22 @@ There are several additional APIs for working with the Web Serial API:
   and [`serial-port-removed`](../api/session.md#event-serial-port-removed) events
   on the Session can be used to handle devices being plugged in or unplugged during the
   `navigator.serial.requestPort` process.
+* [`ses.setDevicePermissionHandler(handler)`](../api/session.md#sessetdevicepermissionhandlerhandler)
+  can be used to provide default permissioning to devices without first calling
+  for permission to devices via `navigator.serial.requestPort`.  Additionally,
+  the default behavior of Electron is to store granted device permision through
+  the lifetime of the corresponding WebContents.  If longer term storage is
+  needed, a developer can store granted device permissions (eg when handling
+  the `select-serial-port` event) and then read from that storage with
+  `setDevicePermissionHandler`.
 * [`ses.setPermissionCheckHandler(handler)`](../api/session.md#sessetpermissioncheckhandlerhandler)
   can be used to disable serial access for specific origins.
 
 ### Example
 
 This example demonstrates an Electron application that automatically selects
-the first available Arduino Uno serial device (if connected) through
+serial devices through [`ses.setDevicePermissionHandler(handler)`](../api/session.md#sessetdevicepermissionhandlerhandler)
+as well as demonstrating selecting the first available Arduino Uno serial device (if connected) through
 [`select-serial-port` event on the Session](../api/session.md#event-select-serial-port)
 when the `Test Web Serial` button is clicked.
 

shell/browser/electron_permission_manager.cc

@@ -22,6 +22,7 @@
 #include "shell/browser/electron_browser_client.h"
 #include "shell/browser/electron_browser_main_parts.h"
 #include "shell/browser/hid/hid_chooser_context.h"
+#include "shell/browser/serial/serial_chooser_context.h"
 #include "shell/browser/web_contents_permission_helper.h"
 #include "shell/browser/web_contents_preferences.h"
 #include "shell/common/gin_converters/content_converter.h"
@@ -328,6 +329,31 @@ bool ElectronPermissionManager::CheckDevicePermission(
           if (serial_number && device_serial_number &&
               *device_serial_number == *serial_number)
             return true;
+        } else if (permission ==
+                   static_cast<content::PermissionType>(
+                       WebContentsPermissionHelper::PermissionType::SERIAL)) {
+#if defined(OS_WIN)
+          if (device->FindStringKey(kDeviceInstanceIdKey) ==
+              granted_device.FindStringKey(kDeviceInstanceIdKey))
+            return true;
+#else
+          if (device->FindIntKey(kVendorIdKey) !=
+                  granted_device.FindIntKey(kVendorIdKey) ||
+              device->FindIntKey(kProductIdKey) !=
+                  granted_device.FindIntKey(kProductIdKey) ||
+              *device->FindStringKey(kSerialNumberKey) !=
+                  *granted_device.FindStringKey(kSerialNumberKey)) {
+            continue;
+          }
+
+#if defined(OS_MAC)
+          if (*device->FindStringKey(kUsbDriverKey) !=
+              *granted_device.FindStringKey(kUsbDriverKey)) {
+            continue;
+          }
+#endif  // defined(OS_MAC)
+          return true;
+#endif  // defined(OS_WIN)
         }
       }
     }

shell/browser/serial/electron_serial_delegate.cc

@@ -60,8 +60,7 @@ bool ElectronSerialDelegate::HasPortPermission(
   auto* chooser_context =
       SerialChooserContextFactory::GetForBrowserContext(browser_context);
   return chooser_context->HasPortPermission(
-      frame->GetLastCommittedOrigin(),
-      web_contents->GetMainFrame()->GetLastCommittedOrigin(), port);
+      web_contents->GetMainFrame()->GetLastCommittedOrigin(), port, frame);
 }
 
 device::mojom::SerialPortManager* ElectronSerialDelegate::GetPortManager(

shell/browser/serial/serial_chooser_context.cc

@@ -12,22 +12,25 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "content/public/browser/device_service.h"
+#include "content/public/browser/web_contents.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
+#include "shell/browser/web_contents_permission_helper.h"
 
 namespace electron {
 
 constexpr char kPortNameKey[] = "name";
 constexpr char kTokenKey[] = "token";
+
 #if defined(OS_WIN)
-constexpr char kDeviceInstanceIdKey[] = "device_instance_id";
+const char kDeviceInstanceIdKey[] = "device_instance_id";
 #else
-constexpr char kVendorIdKey[] = "vendor_id";
-constexpr char kProductIdKey[] = "product_id";
-constexpr char kSerialNumberKey[] = "serial_number";
+const char kVendorIdKey[] = "vendor_id";
+const char kProductIdKey[] = "product_id";
+const char kSerialNumberKey[] = "serial_number";
 #if defined(OS_MAC)
-constexpr char kUsbDriverKey[] = "usb_driver";
+const char kUsbDriverKey[] = "usb_driver";
 #endif  // defined(OS_MAC)
-#endif  // defined(OS_WIN)
+#endif  // defined(OS_WIN
 
 std::string EncodeToken(const base::UnguessableToken& token) {
   const uint64_t data[2] = {token.GetHighForSerialization(),
@@ -83,30 +86,51 @@ base::Value PortInfoToValue(const device::mojom::SerialPortInfo& port) {
 }
 
 SerialChooserContext::SerialChooserContext() = default;
+
 SerialChooserContext::~SerialChooserContext() = default;
 
 void SerialChooserContext::GrantPortPermission(
-    const url::Origin& requesting_origin,
-    const url::Origin& embedding_origin,
-    const device::mojom::SerialPortInfo& port) {
+    const url::Origin& origin,
+    const device::mojom::SerialPortInfo& port,
+    content::RenderFrameHost* render_frame_host) {
   base::Value value = PortInfoToValue(port);
   port_info_.insert({port.token, value.Clone()});
 
-  ephemeral_ports_[{requesting_origin, embedding_origin}].insert(port.token);
+  if (CanStorePersistentEntry(port)) {
+    auto* web_contents =
+        content::WebContents::FromRenderFrameHost(render_frame_host);
+    auto* permission_helper =
+        WebContentsPermissionHelper::FromWebContents(web_contents);
+    permission_helper->GrantSerialPortPermission(origin, std::move(value),
+                                                 render_frame_host);
+    return;
+  }
+
+  ephemeral_ports_[origin].insert(port.token);
 }
 
 bool SerialChooserContext::HasPortPermission(
-    const url::Origin& requesting_origin,
-    const url::Origin& embedding_origin,
-    const device::mojom::SerialPortInfo& port) {
-  auto it = ephemeral_ports_.find({requesting_origin, embedding_origin});
+    const url::Origin& origin,
+    const device::mojom::SerialPortInfo& port,
+    content::RenderFrameHost* render_frame_host) {
+  auto it = ephemeral_ports_.find(origin);
   if (it != ephemeral_ports_.end()) {
     const std::set<base::UnguessableToken> ports = it->second;
     if (base::Contains(ports, port.token))
       return true;
   }
 
-  return false;
+  if (!CanStorePersistentEntry(port)) {
+    return false;
+  }
+
+  auto* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  auto* permission_helper =
+      WebContentsPermissionHelper::FromWebContents(web_contents);
+  base::Value value = PortInfoToValue(port);
+  return permission_helper->CheckSerialPortPermission(origin, std::move(value),
+                                                      render_frame_host);
 }
 
 // static
@@ -170,14 +194,6 @@ void SerialChooserContext::OnPortRemoved(
   for (auto& observer : port_observer_list_)
     observer.OnPortRemoved(*port);
 
-  std::vector<std::pair<url::Origin, url::Origin>> revoked_url_pairs;
-  for (auto& map_entry : ephemeral_ports_) {
-    std::set<base::UnguessableToken>& ports = map_entry.second;
-    if (ports.erase(port->token) > 0) {
-      revoked_url_pairs.push_back(map_entry.first);
-    }
-  }
-
   port_info_.erase(port->token);
 }
 

shell/browser/serial/serial_chooser_context.h

@@ -28,6 +28,20 @@ class Value;
 
 namespace electron {
 
+extern const char kHidVendorIdKey[];
+extern const char kHidProductIdKey[];
+
+#if defined(OS_WIN)
+extern const char kDeviceInstanceIdKey[];
+#else
+extern const char kVendorIdKey[];
+extern const char kProductIdKey[];
+extern const char kSerialNumberKey[];
+#if defined(OS_MAC)
+extern const char kUsbDriverKey[];
+#endif  // defined(OS_MAC)
+#endif  // defined(OS_WIN)
+
 class SerialChooserContext : public KeyedService,
                              public device::mojom::SerialPortManagerClient {
  public:
@@ -37,12 +51,12 @@ class SerialChooserContext : public KeyedService,
   ~SerialChooserContext() override;
 
   // Serial-specific interface for granting and checking permissions.
-  void GrantPortPermission(const url::Origin& requesting_origin,
-                           const url::Origin& embedding_origin,
-                           const device::mojom::SerialPortInfo& port);
-  bool HasPortPermission(const url::Origin& requesting_origin,
-                         const url::Origin& embedding_origin,
-                         const device::mojom::SerialPortInfo& port);
+  void GrantPortPermission(const url::Origin& origin,
+                           const device::mojom::SerialPortInfo& port,
+                           content::RenderFrameHost* render_frame_host);
+  bool HasPortPermission(const url::Origin& origin,
+                         const device::mojom::SerialPortInfo& port,
+                         content::RenderFrameHost* render_frame_host);
   static bool CanStorePersistentEntry(
       const device::mojom::SerialPortInfo& port);
 
@@ -62,16 +76,9 @@ class SerialChooserContext : public KeyedService,
   void SetUpPortManagerConnection(
       mojo::PendingRemote<device::mojom::SerialPortManager> manager);
   void OnPortManagerConnectionError();
-  void OnGetPorts(const url::Origin& requesting_origin,
-                  const url::Origin& embedding_origin,
-                  blink::mojom::SerialService::GetPortsCallback callback,
-                  std::vector<device::mojom::SerialPortInfoPtr> ports);
-
-  // Tracks the set of ports to which an origin (potentially embedded in another
-  // origin) has access to. Key is (requesting_origin, embedding_origin).
-  std::map<std::pair<url::Origin, url::Origin>,
-           std::set<base::UnguessableToken>>
-      ephemeral_ports_;
+
+  // Tracks the set of ports to which an origin has access to.
+  std::map<url::Origin, std::set<base::UnguessableToken>> ephemeral_ports_;
 
   // Holds information about ports in |ephemeral_ports_|.
   std::map<base::UnguessableToken, base::Value> port_info_;

shell/browser/serial/serial_chooser_controller.cc

@@ -67,9 +67,9 @@ SerialChooserController::SerialChooserController(
     : WebContentsObserver(web_contents),
       filters_(std::move(filters)),
       callback_(std::move(callback)),
-      serial_delegate_(serial_delegate) {
-  requesting_origin_ = render_frame_host->GetLastCommittedOrigin();
-  embedding_origin_ = web_contents->GetMainFrame()->GetLastCommittedOrigin();
+      serial_delegate_(serial_delegate),
+      render_frame_host_id_(render_frame_host->GetGlobalId()) {
+  origin_ = web_contents->GetMainFrame()->GetLastCommittedOrigin();
 
   chooser_context_ = SerialChooserContextFactory::GetForBrowserContext(
                          web_contents->GetBrowserContext())
@@ -125,8 +125,8 @@ void SerialChooserController::OnDeviceChosen(const std::string& port_id) {
           return ptr->token.ToString() == port_id;
         });
     if (it != ports_.end()) {
-      chooser_context_->GrantPortPermission(requesting_origin_,
-                                            embedding_origin_, *it->get());
+      auto* rfh = content::RenderFrameHost::FromID(render_frame_host_id_);
+      chooser_context_->GrantPortPermission(origin_, *it->get(), rfh);
       RunCallback(it->Clone());
     } else {
       RunCallback(/*port=*/nullptr);

shell/browser/serial/serial_chooser_controller.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/serial_chooser.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -53,8 +54,7 @@ class SerialChooserController final : public SerialChooserContext::PortObserver,
 
   std::vector<blink::mojom::SerialPortFilterPtr> filters_;
   content::SerialChooser::Callback callback_;
-  url::Origin requesting_origin_;
-  url::Origin embedding_origin_;
+  url::Origin origin_;
 
   base::WeakPtr<SerialChooserContext> chooser_context_;
 
@@ -62,6 +62,8 @@ class SerialChooserController final : public SerialChooserContext::PortObserver,
 
   base::WeakPtr<ElectronSerialDelegate> serial_delegate_;
 
+  content::GlobalRenderFrameHostId render_frame_host_id_;
+
   base::WeakPtrFactory<SerialChooserController> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SerialChooserController);

shell/browser/web_contents_permission_helper.cc

@@ -190,6 +190,24 @@ bool WebContentsPermissionHelper::CheckSerialAccessPermission(
       static_cast<content::PermissionType>(PermissionType::SERIAL), &details);
 }
 
+bool WebContentsPermissionHelper::CheckSerialPortPermission(
+    const url::Origin& origin,
+    base::Value device,
+    content::RenderFrameHost* render_frame_host) const {
+  return CheckDevicePermission(
+      static_cast<content::PermissionType>(PermissionType::SERIAL), origin,
+      &device, render_frame_host);
+}
+
+void WebContentsPermissionHelper::GrantSerialPortPermission(
+    const url::Origin& origin,
+    base::Value device,
+    content::RenderFrameHost* render_frame_host) const {
+  return GrantDevicePermission(
+      static_cast<content::PermissionType>(PermissionType::SERIAL), origin,
+      &device, render_frame_host);
+}
+
 bool WebContentsPermissionHelper::CheckHIDAccessPermission(
     const url::Origin& embedding_origin) const {
   base::DictionaryValue details;

shell/browser/web_contents_permission_helper.h

@@ -42,6 +42,14 @@ class WebContentsPermissionHelper
   bool CheckMediaAccessPermission(const GURL& security_origin,
                                   blink::mojom::MediaStreamType type) const;
   bool CheckSerialAccessPermission(const url::Origin& embedding_origin) const;
+  bool CheckSerialPortPermission(
+      const url::Origin& origin,
+      base::Value device,
+      content::RenderFrameHost* render_frame_host) const;
+  void GrantSerialPortPermission(
+      const url::Origin& origin,
+      base::Value device,
+      content::RenderFrameHost* render_frame_host) const;
   bool CheckHIDAccessPermission(const url::Origin& embedding_origin) const;
   bool CheckHIDDevicePermission(
       const url::Origin& origin,