fix: persist permission granted to serial ports (#31190)

shell/browser/api/electron_api_web_contents.cc

@@ -911,6 +911,12 @@ void WebContents::InitWithWebContents(content::WebContents* web_contents,
 }
 
 WebContents::~WebContents() {
+  // clear out objects that have been granted permissions so that when
+  // WebContents::RenderFrameDeleted is called as a result of WebContents
+  // destruction it doesn't try to clear out a granted_devices_
+  // on a destructed object.
+  granted_devices_.clear();
+
   MarkDestroyed();
   // The destroy() is called.
   if (inspectable_web_contents_) {
@@ -1415,6 +1421,12 @@ void WebContents::RenderFrameDeleted(
   // - Cross-origin navigation creates a new RFH in a separate process which
   //   is swapped by content::RenderFrameHostManager.
   //
+
+  // clear out objects that have been granted permissions
+  if (!granted_devices_.empty()) {
+    granted_devices_.erase(render_frame_host->GetFrameTreeNodeId());
+  }
+
   // WebFrameMain::FromRenderFrameHost(rfh) will use the RFH's FrameTreeNode ID
   // to find an existing instance of WebFrameMain. During a cross-origin
   // navigation, the deleted RFH will be the old host which was swapped out. In
@@ -3258,6 +3270,42 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
   return handle;
 }
 
+void WebContents::GrantDevicePermission(
+    const url::Origin& origin,
+    const base::Value* device,
+    content::PermissionType permissionType,
+    content::RenderFrameHost* render_frame_host) {
+  granted_devices_[render_frame_host->GetFrameTreeNodeId()][permissionType]
+                  [origin]
+                      .push_back(
+                          std::make_unique<base::Value>(device->Clone()));
+}
+
+std::vector<base::Value> WebContents::GetGrantedDevices(
+    const url::Origin& origin,
+    content::PermissionType permissionType,
+    content::RenderFrameHost* render_frame_host) {
+  const auto& devices_for_frame_host_it =
+      granted_devices_.find(render_frame_host->GetFrameTreeNodeId());
+  if (devices_for_frame_host_it == granted_devices_.end())
+    return {};
+
+  const auto& current_devices_it =
+      devices_for_frame_host_it->second.find(permissionType);
+  if (current_devices_it == devices_for_frame_host_it->second.end())
+    return {};
+
+  const auto& origin_devices_it = current_devices_it->second.find(origin);
+  if (origin_devices_it == current_devices_it->second.end())
+    return {};
+
+  std::vector<base::Value> results;
+  for (const auto& object : origin_devices_it->second)
+    results.push_back(object->Clone());
+
+  return results;
+}
+
 void WebContents::UpdatePreferredSize(content::WebContents* web_contents,
                                       const gfx::Size& pref_size) {
   Emit("preferred-size-changed", pref_size);

shell/browser/api/electron_api_web_contents.h

@@ -20,6 +20,7 @@
 #include "content/common/frame.mojom.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/keyboard_event_processing_result.h"
+#include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
@@ -90,6 +91,11 @@ class OffScreenWebContentsView;
 
 namespace api {
 
+using DevicePermissionMap = std::map<
+    int,
+    std::map<content::PermissionType,
+             std::map<url::Origin, std::vector<std::unique_ptr<base::Value>>>>>;
+
 // Wrapper around the content::WebContents.
 class WebContents : public gin::Wrappable<WebContents>,
                     public gin_helper::EventEmitterMixin<WebContents>,
@@ -428,6 +434,21 @@ class WebContents : public gin::Wrappable<WebContents>,
   void DoGetZoomLevel(
       electron::mojom::ElectronBrowser::DoGetZoomLevelCallback callback);
 
+  // Grants |origin| access to |device|.
+  // To be used in place of ObjectPermissionContextBase::GrantObjectPermission.
+  void GrantDevicePermission(const url::Origin& origin,
+                             const base::Value* device,
+                             content::PermissionType permissionType,
+                             content::RenderFrameHost* render_frame_host);
+
+  // Returns the list of devices that |origin| has been granted permission to
+  // access. To be used in place of
+  // ObjectPermissionContextBase::GetGrantedObjects.
+  std::vector<base::Value> GetGrantedDevices(
+      const url::Origin& origin,
+      content::PermissionType permissionType,
+      content::RenderFrameHost* render_frame_host);
+
  private:
   // Does not manage lifetime of |web_contents|.
   WebContents(v8::Isolate* isolate, content::WebContents* web_contents);
@@ -786,6 +807,9 @@ class WebContents : public gin::Wrappable<WebContents>,
 
   service_manager::BinderRegistryWithArgs<content::RenderFrameHost*> registry_;
 
+  // In-memory cache that holds objects that have been granted permissions.
+  DevicePermissionMap granted_devices_;
+
   base::WeakPtrFactory<WebContents> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(WebContents);

shell/browser/electron_permission_manager.cc

@@ -15,9 +15,17 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "gin/data_object_builder.h"
+#include "shell/browser/api/electron_api_web_contents.h"
 #include "shell/browser/electron_browser_client.h"
 #include "shell/browser/electron_browser_main_parts.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"
+#include "shell/common/gin_converters/frame_converter.h"
+#include "shell/common/gin_converters/value_converter.h"
+#include "shell/common/gin_helper/event_emitter_caller.h"
 
 namespace electron {
 
@@ -277,6 +285,65 @@ bool ElectronPermissionManager::CheckPermissionWithDetails(
                             mutable_details);
 }
 
+bool ElectronPermissionManager::CheckDevicePermission(
+    content::PermissionType permission,
+    const url::Origin& origin,
+    const base::Value* device,
+    content::RenderFrameHost* render_frame_host) const {
+  auto* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  api::WebContents* api_web_contents = api::WebContents::From(web_contents);
+
+  if (api_web_contents) {
+    std::vector<base::Value> granted_devices =
+        api_web_contents->GetGrantedDevices(origin, permission,
+                                            render_frame_host);
+
+    for (const auto& granted_device : granted_devices) {
+      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)
+      }
+    }
+  }
+  return false;
+}
+
+void ElectronPermissionManager::GrantDevicePermission(
+    content::PermissionType permission,
+    const url::Origin& origin,
+    const base::Value* device,
+    content::RenderFrameHost* render_frame_host) const {
+  auto* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  api::WebContents* api_web_contents = api::WebContents::From(web_contents);
+  if (api_web_contents)
+    api_web_contents->GrantDevicePermission(origin, device, permission,
+                                            render_frame_host);
+}
+
 blink::mojom::PermissionStatus
 ElectronPermissionManager::GetPermissionStatusForFrame(
     content::PermissionType permission,

shell/browser/electron_permission_manager.h

@@ -13,6 +13,7 @@
 #include "base/containers/id_map.h"
 #include "base/values.h"
 #include "content/public/browser/permission_controller_delegate.h"
+#include "gin/dictionary.h"
 
 namespace content {
 class WebContents;
@@ -77,6 +78,16 @@ class ElectronPermissionManager : public content::PermissionControllerDelegate {
                                   const GURL& requesting_origin,
                                   const base::DictionaryValue* details) const;
 
+  bool CheckDevicePermission(content::PermissionType permission,
+                             const url::Origin& origin,
+                             const base::Value* object,
+                             content::RenderFrameHost* render_frame_host) const;
+
+  void GrantDevicePermission(content::PermissionType permission,
+                             const url::Origin& origin,
+                             const base::Value* object,
+                             content::RenderFrameHost* render_frame_host) const;
+
  protected:
   void OnPermissionResponse(int request_id,
                             int permission_id,

shell/browser/serial/electron_serial_delegate.cc

@@ -71,8 +71,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

@@ -10,22 +10,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(),
@@ -81,30 +84,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
@@ -168,14 +192,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

@@ -30,6 +30,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:
@@ -39,12 +53,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);
 
@@ -69,11 +83,8 @@ class SerialChooserContext : public KeyedService,
                   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->GetGlobalFrameRoutingId()) {
+  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::GlobalFrameRoutingId render_frame_host_id_;
+
   base::WeakPtrFactory<SerialChooserController> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SerialChooserController);

shell/browser/web_contents_permission_helper.cc

@@ -94,6 +94,28 @@ bool WebContentsPermissionHelper::CheckPermission(
                                                         details);
 }
 
+bool WebContentsPermissionHelper::CheckDevicePermission(
+    content::PermissionType permission,
+    const url::Origin& origin,
+    const base::Value* device,
+    content::RenderFrameHost* render_frame_host) const {
+  auto* permission_manager = static_cast<ElectronPermissionManager*>(
+      web_contents_->GetBrowserContext()->GetPermissionControllerDelegate());
+  return permission_manager->CheckDevicePermission(permission, origin, device,
+                                                   render_frame_host);
+}
+
+void WebContentsPermissionHelper::GrantDevicePermission(
+    content::PermissionType permission,
+    const url::Origin& origin,
+    const base::Value* device,
+    content::RenderFrameHost* render_frame_host) const {
+  auto* permission_manager = static_cast<ElectronPermissionManager*>(
+      web_contents_->GetBrowserContext()->GetPermissionControllerDelegate());
+  permission_manager->GrantDevicePermission(permission, origin, device,
+                                            render_frame_host);
+}
+
 void WebContentsPermissionHelper::RequestFullscreenPermission(
     base::OnceCallback<void(bool)> callback) {
   RequestPermission(
@@ -168,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);
+}
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(WebContentsPermissionHelper)
 
 }  // namespace electron

shell/browser/web_contents_permission_helper.h

@@ -40,6 +40,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;
 
  private:
   explicit WebContentsPermissionHelper(content::WebContents* web_contents);
@@ -53,6 +61,16 @@ class WebContentsPermissionHelper
   bool CheckPermission(content::PermissionType permission,
                        const base::DictionaryValue* details) const;
 
+  bool CheckDevicePermission(content::PermissionType permission,
+                             const url::Origin& origin,
+                             const base::Value* device,
+                             content::RenderFrameHost* render_frame_host) const;
+
+  void GrantDevicePermission(content::PermissionType permission,
+                             const url::Origin& origin,
+                             const base::Value* device,
+                             content::RenderFrameHost* render_frame_host) const;
+
   content::WebContents* web_contents_;
 
   WEB_CONTENTS_USER_DATA_KEY_DECL();