[serial] Finish wiring up serial guard permission

This change finishes wiring up the CONTENT_SETTINGS_TYPES_SERIAL_GUARD
permission which can be used to entirely block the chooser UI shown by
navigator.serial.requestPort(). As with the USB guard permission this
needs special casing in the page info and site settings UIs because it
can't be set to "allow," only "ask" or "block."

Bug: 908836
Change-Id: I085c9e60e8a3cd6fa85ca0b18a58f359ef35e7f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538681Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652697}

chrome/browser/resources/settings/site_settings/site_details_permission.js

@@ -227,7 +227,9 @@ Polymer({
    * @private
    */
   showAllowedSetting_: function(category) {
-    return category != settings.ContentSettingsTypes.USB_DEVICES;
+    return !(
+        category == settings.ContentSettingsTypes.SERIAL_PORTS ||
+        category == settings.ContentSettingsTypes.USB_DEVICES);
   },
 
   /**
@@ -240,7 +242,8 @@ Polymer({
    */
   showAskSetting_: function(category, setting, source) {
     // For chooser-based permissions 'ask' takes the place of 'allow'.
-    if (category == settings.ContentSettingsTypes.USB_DEVICES) {
+    if (category == settings.ContentSettingsTypes.SERIAL_PORTS ||
+        category == settings.ContentSettingsTypes.USB_DEVICES) {
       return true;
     }
 

chrome/browser/serial/chrome_serial_delegate.cc

@@ -40,6 +40,17 @@ std::unique_ptr<content::SerialChooser> ChromeSerialDelegate::RunChooser(
   return std::make_unique<SerialChooser>(std::move(bubble_reference));
 }
 
+bool ChromeSerialDelegate::CanRequestPortPermission(
+    content::RenderFrameHost* frame) {
+  auto* web_contents = content::WebContents::FromRenderFrameHost(frame);
+  auto* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  auto* chooser_context = SerialChooserContextFactory::GetForProfile(profile);
+  return chooser_context->CanRequestObjectPermission(
+      frame->GetLastCommittedURL().GetOrigin(),
+      web_contents->GetMainFrame()->GetLastCommittedURL().GetOrigin());
+}
+
 bool ChromeSerialDelegate::HasPortPermission(
     content::RenderFrameHost* frame,
     const device::mojom::SerialPortInfo& port) {

chrome/browser/serial/chrome_serial_delegate.h

@@ -19,6 +19,7 @@ class ChromeSerialDelegate : public content::SerialDelegate {
       content::RenderFrameHost* frame,
       std::vector<blink::mojom::SerialPortFilterPtr> filters,
       content::SerialChooser::Callback callback) override;
+  bool CanRequestPortPermission(content::RenderFrameHost* frame) override;
   bool HasPortPermission(content::RenderFrameHost* frame,
                          const device::mojom::SerialPortInfo& port) override;
   device::mojom::SerialPortManager* GetPortManager(

chrome/browser/serial/serial_chooser_context.cc

@@ -74,14 +74,17 @@ std::string SerialChooserContext::GetObjectName(
 std::vector<std::unique_ptr<ChooserContextBase::Object>>
 SerialChooserContext::GetGrantedObjects(const GURL& requesting_origin,
                                         const GURL& embedding_origin) {
-  std::vector<std::unique_ptr<Object>> objects;
+  if (!CanRequestObjectPermission(requesting_origin, embedding_origin))
+    return {};
+
   auto origin_it = ephemeral_ports_.find(
       std::make_pair(url::Origin::Create(requesting_origin),
                      url::Origin::Create(embedding_origin)));
   if (origin_it == ephemeral_ports_.end())
-    return objects;
+    return {};
   const std::set<base::UnguessableToken> ports = origin_it->second;
 
+  std::vector<std::unique_ptr<Object>> objects;
   for (const auto& token : ports) {
     auto it = port_info_.find(token);
     if (it == port_info_.end())

chrome/browser/serial/serial_chooser_context_unittest.cc

@@ -5,9 +5,11 @@
 #include "chrome/browser/serial/serial_chooser_context.h"
 
 #include "base/run_loop.h"
+#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/chooser_context_base_mock_permission_observer.h"
 #include "chrome/browser/serial/serial_chooser_context_factory.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "services/device/public/mojom/serial.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -80,3 +82,28 @@ TEST_F(SerialChooserContextTest, GrantAndRevokeEphemeralPermission) {
   objects = context->GetAllGrantedObjects();
   EXPECT_EQ(0u, objects.size());
 }
+
+TEST_F(SerialChooserContextTest, GuardPermission) {
+  const auto origin = url::Origin::Create(GURL("https://google.com"));
+
+  auto port = device::mojom::SerialPortInfo::New();
+  port->token = base::UnguessableToken::Create();
+
+  SerialChooserContext* context = GetContext(profile());
+  context->GrantPortPermission(origin, origin, *port);
+  EXPECT_TRUE(context->HasPortPermission(origin, origin, *port));
+
+  auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
+  map->SetContentSettingDefaultScope(origin.GetURL(), origin.GetURL(),
+                                     CONTENT_SETTINGS_TYPE_SERIAL_GUARD,
+                                     std::string(), CONTENT_SETTING_BLOCK);
+  EXPECT_FALSE(context->HasPortPermission(origin, origin, *port));
+
+  std::vector<std::unique_ptr<ChooserContextBase::Object>> objects =
+      context->GetGrantedObjects(origin.GetURL(), origin.GetURL());
+  EXPECT_EQ(0u, objects.size());
+
+  std::vector<std::unique_ptr<ChooserContextBase::Object>> all_origin_objects =
+      context->GetAllGrantedObjects();
+  EXPECT_EQ(0u, all_origin_objects.size());
+}

chrome/browser/ui/page_info/page_info.cc

@@ -127,6 +127,9 @@ ContentSettingsType kPermissionType[] = {
     CONTENT_SETTINGS_TYPE_MIDI_SYSEX,
     CONTENT_SETTINGS_TYPE_CLIPBOARD_READ,
     CONTENT_SETTINGS_TYPE_USB_GUARD,
+#if !defined(OS_ANDROID)
+    CONTENT_SETTINGS_TYPE_SERIAL_GUARD,
+#endif
 };
 
 // Checks whether this permission is currently the factory default, as set by

chrome/browser/ui/page_info/page_info_ui.cc

@@ -162,6 +162,9 @@ const PermissionsUIInfo kPermissionsUIInfo[] = {
     {CONTENT_SETTINGS_TYPE_CLIPBOARD_READ, IDS_PAGE_INFO_TYPE_CLIPBOARD},
     {CONTENT_SETTINGS_TYPE_SENSORS, IDS_PAGE_INFO_TYPE_SENSORS},
     {CONTENT_SETTINGS_TYPE_USB_GUARD, IDS_PAGE_INFO_TYPE_USB},
+#if !defined(OS_ANDROID)
+    {CONTENT_SETTINGS_TYPE_SERIAL_GUARD, IDS_PAGE_INFO_TYPE_SERIAL},
+#endif
 };
 
 std::unique_ptr<PageInfoUI::SecurityDescription> CreateSecurityDescription(
@@ -541,6 +544,11 @@ const gfx::ImageSkia PageInfoUI::GetPermissionIcon(const PermissionInfo& info,
     case CONTENT_SETTINGS_TYPE_USB_GUARD:
       icon = &vector_icons::kUsbIcon;
       break;
+#if !defined(OS_ANDROID)
+    case CONTENT_SETTINGS_TYPE_SERIAL_GUARD:
+      icon = &vector_icons::kSerialPortIcon;
+      break;
+#endif
     default:
       // All other |ContentSettingsType|s do not have icons on desktop or are
       // not shown in the Page Info bubble.

chrome/browser/ui/page_info/permission_menu_model.cc

@@ -88,12 +88,15 @@ bool PermissionMenuModel::ShouldShowAllow(const GURL& url) {
   }
 
   // Chooser permissions do not support CONTENT_SETTING_ALLOW.
-  if (permission_.type == CONTENT_SETTINGS_TYPE_USB_GUARD)
+  if (permission_.type == CONTENT_SETTINGS_TYPE_SERIAL_GUARD ||
+      permission_.type == CONTENT_SETTINGS_TYPE_USB_GUARD) {
     return false;
+  }
 
   return true;
 }
 
 bool PermissionMenuModel::ShouldShowAsk(const GURL& url) {
-  return permission_.type == CONTENT_SETTINGS_TYPE_USB_GUARD;
+  return permission_.type == CONTENT_SETTINGS_TYPE_USB_GUARD ||
+         permission_.type == CONTENT_SETTINGS_TYPE_SERIAL_GUARD;
 }

chrome/browser/ui/page_info/permission_menu_model_unittest.cc

@@ -99,3 +99,31 @@ TEST_F(PermissionMenuModelTest, TestUsbGuard) {
                             permission, callback.callback());
   EXPECT_EQ(3, model.GetItemCount());
 }
+
+TEST_F(PermissionMenuModelTest, TestSerialGuard) {
+  const GURL kUrl("http://www.google.com");
+  TestCallback callback;
+  PageInfoUI::PermissionInfo permission;
+  permission.type = CONTENT_SETTINGS_TYPE_SERIAL_GUARD;
+  permission.setting = CONTENT_SETTING_ASK;
+  permission.source = content_settings::SETTING_SOURCE_USER;
+  permission.is_incognito = false;
+
+  permission.default_setting = CONTENT_SETTING_ASK;
+  PermissionMenuModel default_ask_model(profile(), kUrl, permission,
+                                        callback.callback());
+  ASSERT_EQ(3, default_ask_model.GetItemCount());
+  EXPECT_EQ(base::ASCIIToUTF16("Ask (default)"),
+            default_ask_model.GetLabelAt(0));
+  EXPECT_EQ(base::ASCIIToUTF16("Block"), default_ask_model.GetLabelAt(1));
+  EXPECT_EQ(base::ASCIIToUTF16("Ask"), default_ask_model.GetLabelAt(2));
+
+  permission.default_setting = CONTENT_SETTING_BLOCK;
+  PermissionMenuModel default_block_model(profile(), kUrl, permission,
+                                          callback.callback());
+  ASSERT_EQ(3, default_block_model.GetItemCount());
+  EXPECT_EQ(base::ASCIIToUTF16("Block (default)"),
+            default_block_model.GetLabelAt(0));
+  EXPECT_EQ(base::ASCIIToUTF16("Block"), default_block_model.GetLabelAt(1));
+  EXPECT_EQ(base::ASCIIToUTF16("Ask"), default_block_model.GetLabelAt(2));
+}

components/page_info_strings.grdp

@@ -247,6 +247,9 @@
     <message name="IDS_PAGE_INFO_TYPE_USB" desc="The label used for the USB permission controls in the Page Info popup.">
       USB devices
     </message>
+    <message name="IDS_PAGE_INFO_TYPE_SERIAL" desc="The label used for the serial port permission controls in the Page Info popup.">
+      Serial ports
+    </message>
     <!-- TODO(crbug.com/716303): A few permissions are missing here. -->
 
     <!-- Permission values -->

content/browser/serial/serial_browsertest.cc

@@ -21,6 +21,7 @@
 
 using testing::_;
 using testing::ByMove;
+using testing::Exactly;
 using testing::Return;
 
 namespace content {
@@ -83,6 +84,8 @@ IN_PROC_BROWSER_TEST_F(SerialTest, GetPorts) {
 IN_PROC_BROWSER_TEST_F(SerialTest, RequestPort) {
   NavigateToURL(shell(), GetTestUrl(nullptr, "simple_page.html"));
 
+  EXPECT_CALL(delegate(), CanRequestPortPermission).WillOnce(Return(true));
+
   auto port = device::mojom::SerialPortInfo::New();
   port->token = base::UnguessableToken::Create();
   EXPECT_CALL(delegate(), RunChooserInternal)
@@ -95,4 +98,21 @@ IN_PROC_BROWSER_TEST_F(SerialTest, RequestPort) {
                          })())"));
 }
 
+IN_PROC_BROWSER_TEST_F(SerialTest, DisallowRequestPort) {
+  NavigateToURL(shell(), GetTestUrl(nullptr, "simple_page.html"));
+
+  EXPECT_CALL(delegate(), CanRequestPortPermission(_)).WillOnce(Return(false));
+  EXPECT_CALL(delegate(), RunChooserInternal).Times(Exactly(0));
+
+  EXPECT_EQ(false, EvalJs(shell(),
+                          R"((async () => {
+                            try {
+                              await navigator.serial.requestPort({});
+                              return true;
+                            } catch (e) {
+                              return false;
+                            }
+                          })())"));
+}
+
 }  // namespace content

content/browser/serial/serial_service.cc

@@ -76,6 +76,11 @@ void SerialService::RequestPort(
     return;
   }
 
+  if (!delegate->CanRequestPortPermission(render_frame_host_)) {
+    std::move(callback).Run(nullptr);
+    return;
+  }
+
   chooser_ = delegate->RunChooser(
       render_frame_host_, std::move(filters),
       base::BindOnce(&SerialService::FinishRequestPort,

content/browser/serial/serial_test_utils.h

@@ -22,6 +22,7 @@ class MockSerialDelegate : public SerialDelegate {
       SerialChooser::Callback callback) override;
 
   MOCK_METHOD0(RunChooserInternal, device::mojom::SerialPortInfoPtr());
+  MOCK_METHOD1(CanRequestPortPermission, bool(RenderFrameHost* frame));
   MOCK_METHOD2(HasPortPermission,
                bool(content::RenderFrameHost* frame,
                     const device::mojom::SerialPortInfo& port));

content/public/browser/serial_delegate.h

@@ -23,12 +23,16 @@ class CONTENT_EXPORT SerialDelegate {
 
   // Shows a chooser for the user to select a serial port.  |callback| will be
   // run when the prompt is closed. Deleting the returned object will cancel the
-  // prompt.
+  // prompt. This method should not be called if CanRequestPortPermission()
+  // below returned false.
   virtual std::unique_ptr<SerialChooser> RunChooser(
       RenderFrameHost* frame,
       std::vector<blink::mojom::SerialPortFilterPtr> filters,
       SerialChooser::Callback callback) = 0;
 
+  // Returns whether |frame| has permission to request access to a port.
+  virtual bool CanRequestPortPermission(RenderFrameHost* frame) = 0;
+
   // Returns whether |frame| has permission to access |port|.
   virtual bool HasPortPermission(RenderFrameHost* frame,
                                  const device::mojom::SerialPortInfo& port) = 0;