Use the origin/name of the main frame in chooser dialogs

Permissions dialogs (for WebUSB, WebHID, and Web Serial) should request permission for the main origin (the origin embedding the iframe) and not the site hosting the iframe as per crbug.com/802945. Update the UI code to accurately display the main origin (or extension name) and add tests. Bug: 1280233 Change-Id: Ia3bf9274eb49c1d842e204a518635cc1187f3d3d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3353956 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Andy Paicu <andypaicu@chromium.org> Commit-Queue: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/main@{#963184}
chromium · Jan 25, 2022 · b13ddd2 · b13ddd2
1 parent 734f4e0
commit b13ddd2
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 5 deletions.
diff --git a/chrome/browser/chooser_controller/title_util.cc b/chrome/browser/chooser_controller/title_util.cc
@@ -22,6 +22,8 @@ std::u16string CreateExtensionAwareChooserTitle(
     int title_string_id_extension) {
   if (!render_frame_host)
     return u"";
+  // Ensure the permission request is attributed to the main frame.
+  render_frame_host = render_frame_host->GetMainFrame();
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   url::Origin origin = render_frame_host->GetLastCommittedOrigin();

diff --git a/chrome/browser/chooser_controller/title_util.h b/chrome/browser/chooser_controller/title_util.h
@@ -12,7 +12,8 @@ class RenderFrameHost;
 }
 
 // Creates a title for a chooser. For extensions the extension name is used if
-// possible. In all other cases the origin is used.
+// possible. In all other cases the origin of the main frame for
+// `render_frame_host` is used.
 std::u16string CreateExtensionAwareChooserTitle(
     content::RenderFrameHost* render_frame_host,
     int title_string_id_origin,

diff --git a/chrome/browser/chooser_controller/title_util_unittest.cc b/chrome/browser/chooser_controller/title_util_unittest.cc
@@ -0,0 +1,78 @@
+// Copyright 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chooser_controller/title_util.h"
+
+#include "chrome/grit/generated_resources.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "components/strings/grit/components_strings.h"
+
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/test_extension_system.h"
+#include "content/public/test/navigation_simulator.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_builder.h"
+#include "extensions/common/value_builder.h"
+#include "url/gurl.h"
+#include "url/origin.h"
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
+
+namespace {
+
+constexpr int kNonExtensionTitleResourceId =
+    IDS_USB_DEVICE_CHOOSER_PROMPT_ORIGIN;
+constexpr int kExtensionTitleResourceId =
+    IDS_USB_DEVICE_CHOOSER_PROMPT_EXTENSION_NAME;
+
+using ExtensionsAwareChooserTitleTest = ChromeRenderViewHostTestHarness;
+
+TEST_F(ExtensionsAwareChooserTitleTest, NoFrame) {
+  EXPECT_EQ(u"", CreateExtensionAwareChooserTitle(nullptr,
+                                                  kNonExtensionTitleResourceId,
+                                                  kExtensionTitleResourceId));
+}
+
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+TEST_F(ExtensionsAwareChooserTitleTest, FrameTree) {
+  extensions::DictionaryBuilder manifest;
+  manifest.Set("name", "Chooser Title Subframe Test")
+      .Set("version", "0.1")
+      .Set("manifest_version", 2)
+      .Set("web_accessible_resources",
+           extensions::ListBuilder().Append("index.html").Build());
+  scoped_refptr<const extensions::Extension> extension =
+      extensions::ExtensionBuilder().SetManifest(manifest.Build()).Build();
+  ASSERT_TRUE(extension);
+
+  extensions::TestExtensionSystem* extension_system =
+      static_cast<extensions::TestExtensionSystem*>(
+          extensions::ExtensionSystem::Get(profile()));
+  extensions::ExtensionService* extension_service =
+      extension_system->CreateExtensionService(
+          base::CommandLine::ForCurrentProcess(), base::FilePath(), false);
+  extension_service->AddExtension(extension.get());
+
+  NavigateAndCommit(extension->GetResourceURL("index.html"));
+  content::RenderFrameHost* subframe =
+      content::NavigationSimulator::NavigateAndCommitFromDocument(
+          GURL("data:text/html,"),
+          content::RenderFrameHostTester::For(main_rfh())
+              ->AppendChild("subframe"));
+
+  EXPECT_EQ(extension->id(), main_rfh()->GetLastCommittedOrigin().host());
+  EXPECT_EQ(
+      u"\"Chooser Title Subframe Test\" wants to connect",
+      CreateExtensionAwareChooserTitle(main_rfh(), kNonExtensionTitleResourceId,
+                                       kExtensionTitleResourceId));
+  EXPECT_NE(extension->id(), subframe->GetLastCommittedOrigin().host());
+  EXPECT_EQ(
+      u"\"Chooser Title Subframe Test\" wants to connect",
+      CreateExtensionAwareChooserTitle(subframe, kNonExtensionTitleResourceId,
+                                       kExtensionTitleResourceId));
+}
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
+
+}  // namespace
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
@@ -4526,6 +4526,7 @@ test("unit_tests") {
     "../browser/browsing_data/counters/site_data_counting_helper_unittest.cc",
     "../browser/browsing_data/counters/site_settings_counter_unittest.cc",
     "../browser/browsing_data/site_data_size_collector_unittest.cc",
+    "../browser/chooser_controller/title_util_unittest.cc",
     "../browser/chrome_content_browser_client_unittest.cc",
     "../browser/command_updater_impl_unittest.cc",
     "../browser/complex_tasks/task_tab_helper_unittest.cc",

diff --git a/components/permissions/BUILD.gn b/components/permissions/BUILD.gn
@@ -234,6 +234,7 @@ source_set("test_support") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "chooser_title_util_unittest.cc",
     "contexts/camera_pan_tilt_zoom_permission_context_unittest.cc",
     "contexts/geolocation_permission_context_unittest.cc",
     "contexts/midi_permission_context_unittest.cc",
@@ -270,6 +271,7 @@ source_set("unit_tests") {
     "//components/content_settings/core/browser",
     "//components/keyed_service/content",
     "//components/prefs:test_support",
+    "//components/strings:components_strings_grit",
     "//components/ukm:test_support",
     "//components/ukm/content",
     "//components/variations",

diff --git a/components/permissions/chooser_title_util.cc b/components/permissions/chooser_title_util.cc
@@ -15,11 +15,10 @@ std::u16string CreateChooserTitle(content::RenderFrameHost* render_frame_host,
                                   int title_string_id_origin) {
   if (!render_frame_host)
     return u"";
-
   return l10n_util::GetStringFUTF16(
       title_string_id_origin,
       url_formatter::FormatOriginForSecurityDisplay(
-          render_frame_host->GetLastCommittedOrigin(),
+          render_frame_host->GetMainFrame()->GetLastCommittedOrigin(),
           url_formatter::SchemeDisplay::OMIT_CRYPTOGRAPHIC));
 }
 

diff --git a/components/permissions/chooser_title_util.h b/components/permissions/chooser_title_util.h
@@ -13,8 +13,9 @@ class RenderFrameHost;
 
 namespace permissions {
 
-// Creates a title for a chooser using the origin of the frame. Returns the
-// empty string if |render_frame_host| is null.
+// Creates a title for a chooser using the origin of the main frame
+// containing `render_frame_host`. Returns the empty string if
+// `render_frame_host` is null.
 std::u16string CreateChooserTitle(content::RenderFrameHost* render_frame_host,
                                   int title_string_id_origin);
 

diff --git a/components/permissions/chooser_title_util_unittest.cc b/components/permissions/chooser_title_util_unittest.cc
@@ -0,0 +1,41 @@
+// Copyright 2022 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/permissions/chooser_title_util.h"
+
+#include "components/strings/grit/components_strings.h"
+#include "content/public/test/navigation_simulator.h"
+#include "content/public/test/test_renderer_host.h"
+#include "url/gurl.h"
+#include "url/origin.h"
+
+namespace permissions {
+namespace {
+
+constexpr int kTitleResourceId = IDS_USB_DEVICE_CHOOSER_PROMPT_ORIGIN;
+
+using ChooserTitleTest = content::RenderViewHostTestHarness;
+
+TEST_F(ChooserTitleTest, NoFrame) {
+  EXPECT_EQ(u"", CreateChooserTitle(nullptr, kTitleResourceId));
+}
+
+TEST_F(ChooserTitleTest, FrameTree) {
+  NavigateAndCommit(GURL("https://main-frame.com"));
+  content::RenderFrameHost* subframe =
+      content::NavigationSimulator::NavigateAndCommitFromDocument(
+          GURL("https://sub-frame.com"),
+          content::RenderFrameHostTester::For(main_rfh())
+              ->AppendChild("subframe"));
+
+  EXPECT_EQ("main-frame.com", main_rfh()->GetLastCommittedOrigin().host());
+  EXPECT_EQ(u"main-frame.com wants to connect",
+            CreateChooserTitle(main_rfh(), kTitleResourceId));
+  EXPECT_EQ("sub-frame.com", subframe->GetLastCommittedOrigin().host());
+  EXPECT_EQ(u"main-frame.com wants to connect",
+            CreateChooserTitle(subframe, kTitleResourceId));
+}
+
+}  // namespace
+}  // namespace permissions