Add tests for UniversalAccessFromFileUrls.
This CL verifies that permissions requests for file:/// with enabled UniversalAccessFromFileUrls works as intended. CL uses web_contents->GetMainFrame()->GetLastCommittedURL() instead of web_contents->GetLastCommittedURL() in most cases in //content. Bug: 698985 Change-Id: I7605c2d01cbb17e299d00b9f3b37c132367e5c1b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3016192 Reviewed-by: Mike West <mkwst@chromium.org> Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Andy Paicu <andypaicu@chromium.org> Reviewed-by: Richard Coles <torne@chromium.org> Commit-Queue: Illia Klimov <elklm@google.com> Cr-Commit-Position: refs/heads/master@{#905244}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
f2754d1c20
commit
594a4ce05c
android_webview/browser
chrome/browser/permissions
components/permissions
content/browser/permissions
@ -232,6 +232,7 @@ source_set("browser") {
|
||||
"//components/minidump_uploader",
|
||||
"//components/navigation_interception",
|
||||
"//components/page_load_metrics/browser",
|
||||
"//components/permissions",
|
||||
"//components/policy:generated",
|
||||
"//components/policy/content/",
|
||||
"//components/policy/core/browser",
|
||||
|
@ -28,6 +28,7 @@ include_rules = [
|
||||
"+components/navigation_interception",
|
||||
"+components/optimization_guide/core/bloom_filter.h",
|
||||
"+components/page_load_metrics/browser",
|
||||
"+components/permissions",
|
||||
"+components/policy/core/browser",
|
||||
"+components/policy/core/common",
|
||||
"+components/policy/content",
|
||||
|
@ -13,6 +13,7 @@
|
||||
#include "base/bind.h"
|
||||
#include "base/callback.h"
|
||||
#include "base/logging.h"
|
||||
#include "components/permissions/permission_util.h"
|
||||
#include "content/public/browser/child_process_security_policy.h"
|
||||
#include "content/public/browser/permission_controller.h"
|
||||
#include "content/public/browser/permission_type.h"
|
||||
@ -462,9 +463,8 @@ PermissionStatus AwPermissionManager::GetPermissionStatusForFrame(
|
||||
const GURL& requesting_origin) {
|
||||
return GetPermissionStatus(
|
||||
permission, requesting_origin,
|
||||
content::WebContents::FromRenderFrameHost(render_frame_host)
|
||||
->GetLastCommittedURL()
|
||||
.GetOrigin());
|
||||
permissions::PermissionUtil::GetLastCommittedOriginAsURL(
|
||||
render_frame_host));
|
||||
}
|
||||
|
||||
AwPermissionManager::SubscriptionId
|
||||
@ -595,12 +595,13 @@ int AwPermissionManager::GetRenderFrameID(
|
||||
|
||||
GURL AwPermissionManager::LastCommittedOrigin(
|
||||
content::RenderFrameHost* render_frame_host) {
|
||||
return content::WebContents::FromRenderFrameHost(render_frame_host)
|
||||
->GetLastCommittedURL().GetOrigin();
|
||||
return permissions::PermissionUtil::GetLastCommittedOriginAsURL(
|
||||
render_frame_host);
|
||||
}
|
||||
|
||||
AwBrowserPermissionRequestDelegate* AwPermissionManager::GetDelegate(
|
||||
int render_process_id, int render_frame_id) {
|
||||
int render_process_id,
|
||||
int render_frame_id) {
|
||||
return AwBrowserPermissionRequestDelegate::FromID(render_process_id,
|
||||
render_frame_id);
|
||||
}
|
||||
|
@ -14,7 +14,10 @@
|
||||
#include "components/permissions/features.h"
|
||||
#include "components/permissions/test/mock_permission_prompt_factory.h"
|
||||
#include "content/public/test/browser_test.h"
|
||||
#include "content/public/test/browser_test_utils.h"
|
||||
#include "content/public/test/content_browser_test_utils.h"
|
||||
#include "services/device/public/cpp/test/scoped_geolocation_overrider.h"
|
||||
#include "third_party/blink/public/common/web_preferences/web_preferences.h"
|
||||
#include "url/gurl.h"
|
||||
|
||||
namespace {
|
||||
@ -556,4 +559,72 @@ IN_PROC_BROWSER_TEST_P(PermissionsSecurityModelBrowserTest,
|
||||
// Permission is failed because it is another file.
|
||||
VerifyPermissionsForFile(main_rfh, /*expect_granted*/ false);
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_P(PermissionsSecurityModelBrowserTest,
|
||||
UniversalAccessFromFileUrls) {
|
||||
ASSERT_TRUE(embedded_test_server()->Start());
|
||||
|
||||
content::WebContents* embedder_contents =
|
||||
browser()->tab_strip_model()->GetActiveWebContents();
|
||||
ASSERT_TRUE(embedder_contents);
|
||||
|
||||
// Activate the preference to allow universal access from file URLs.
|
||||
blink::web_pref::WebPreferences prefs =
|
||||
embedder_contents->GetOrCreateWebPreferences();
|
||||
prefs.allow_universal_access_from_file_urls = true;
|
||||
embedder_contents->SetWebPreferences(prefs);
|
||||
|
||||
const GURL url(embedded_test_server()->GetURL("/empty.html"));
|
||||
content::RenderFrameHost* main_rfh =
|
||||
ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(browser(), url,
|
||||
1);
|
||||
ASSERT_TRUE(main_rfh);
|
||||
EXPECT_FALSE(main_rfh->GetLastCommittedURL().SchemeIsFile());
|
||||
|
||||
main_rfh = ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(
|
||||
browser(), CreateFileURL(), 1);
|
||||
EXPECT_TRUE(main_rfh->GetLastCommittedURL().SchemeIsFile());
|
||||
EXPECT_TRUE(main_rfh->GetLastCommittedOrigin().GetURL().SchemeIsFile());
|
||||
|
||||
permissions::PermissionRequestManager* manager =
|
||||
permissions::PermissionRequestManager::FromWebContents(embedder_contents);
|
||||
std::unique_ptr<permissions::MockPermissionPromptFactory> bubble_factory =
|
||||
std::make_unique<permissions::MockPermissionPromptFactory>(manager);
|
||||
|
||||
bubble_factory->set_response_type(
|
||||
permissions::PermissionRequestManager::AutoResponseType::ACCEPT_ALL);
|
||||
|
||||
VerifyPermissionsForFile(main_rfh, /*expect_granted*/ true);
|
||||
|
||||
content::EvalJsResult result = content::EvalJs(
|
||||
embedder_contents, "history.pushState({}, {}, 'https://chromium.org');");
|
||||
EXPECT_EQ(std::string(), result.error);
|
||||
EXPECT_EQ("https://chromium.org/", main_rfh->GetLastCommittedURL().spec());
|
||||
EXPECT_TRUE(main_rfh->GetLastCommittedOrigin().GetURL().SchemeIsFile());
|
||||
|
||||
const struct {
|
||||
std::string check_permission;
|
||||
std::string request_permission;
|
||||
} kTests[] = {
|
||||
{kCheckCamera, kRequestCamera},
|
||||
{kCheckGeolocation, kRequestGeolocation},
|
||||
};
|
||||
|
||||
for (const auto& test : kTests) {
|
||||
ASSERT_FALSE(
|
||||
content::EvalJs(main_rfh, test.check_permission).value.GetBool());
|
||||
EXPECT_EQ("granted",
|
||||
content::EvalJs(main_rfh, test.request_permission,
|
||||
content::EXECUTE_SCRIPT_USE_MANUAL_REPLY)
|
||||
.ExtractString());
|
||||
ASSERT_TRUE(
|
||||
content::EvalJs(main_rfh, test.check_permission).value.GetBool());
|
||||
}
|
||||
|
||||
// Notifications is not supported for file:/// with changed URL.
|
||||
ASSERT_FALSE(content::EvalJs(main_rfh, kCheckNotifications).value.GetBool());
|
||||
EXPECT_EQ("denied", content::EvalJs(main_rfh, kRequestNotifications,
|
||||
content::EXECUTE_SCRIPT_DEFAULT_OPTIONS)
|
||||
.ExtractString());
|
||||
}
|
||||
} // anonymous namespace
|
||||
|
@ -20,6 +20,7 @@ include_rules = [
|
||||
"+sql",
|
||||
"+services/network/public/cpp/is_potentially_trustworthy.h",
|
||||
"+third_party/blink/public/common/bluetooth/web_bluetooth_device_id.h",
|
||||
"+third_party/blink/public/common/web_preferences",
|
||||
"+third_party/blink/public/mojom/bluetooth",
|
||||
"+third_party/blink/public/mojom/permissions_policy",
|
||||
"+third_party/blink/public/mojom/quota",
|
||||
|
@ -11,6 +11,7 @@
|
||||
#include "components/permissions/features.h"
|
||||
#include "content/public/browser/permission_type.h"
|
||||
#include "content/public/browser/web_contents.h"
|
||||
#include "third_party/blink/public/common/web_preferences/web_preferences.h"
|
||||
#include "url/gurl.h"
|
||||
|
||||
using content::PermissionType;
|
||||
@ -258,7 +259,24 @@ GURL PermissionUtil::GetLastCommittedOriginAsURL(
|
||||
GURL PermissionUtil::GetLastCommittedOriginAsURL(
|
||||
content::RenderFrameHost* render_frame_host) {
|
||||
DCHECK(render_frame_host);
|
||||
|
||||
if (base::FeatureList::IsEnabled(features::kRevisedOriginHandling)) {
|
||||
content::WebContents* web_contents =
|
||||
content::WebContents::FromRenderFrameHost(render_frame_host);
|
||||
// If `allow_universal_access_from_file_urls` flag is enabled, a file can
|
||||
// introduce discrepancy between GetLastCommittedURL and
|
||||
// GetLastCommittedOrigin. In that case GetLastCommittedURL should be used
|
||||
// for requesting and verifying permissions.
|
||||
// Disabling `kRevisedOriginHandling` feature introduces no side effects,
|
||||
// because in both cases we rely on GetLastCommittedURL().GetOrigin().
|
||||
if (web_contents->GetOrCreateWebPreferences()
|
||||
.allow_universal_access_from_file_urls &&
|
||||
render_frame_host->GetLastCommittedOrigin()
|
||||
.GetURL()
|
||||
.SchemeIsFileSystem()) {
|
||||
return render_frame_host->GetLastCommittedURL().GetOrigin();
|
||||
}
|
||||
|
||||
if (render_frame_host->GetLastCommittedURL().IsAboutBlank()) {
|
||||
return render_frame_host->GetLastCommittedOrigin().GetURL();
|
||||
}
|
||||
|
@ -6,6 +6,7 @@
|
||||
|
||||
#include "components/permissions/features.h"
|
||||
#include "content/public/browser/web_contents.h"
|
||||
#include "third_party/blink/public/common/web_preferences/web_preferences.h"
|
||||
#include "url/gurl.h"
|
||||
|
||||
namespace content {
|
||||
@ -19,14 +20,37 @@ namespace content {
|
||||
GURL PermissionUtil::GetLastCommittedOriginAsURL(
|
||||
content::WebContents* web_contents) {
|
||||
DCHECK(web_contents);
|
||||
return GetLastCommittedOriginAsURL(web_contents->GetMainFrame());
|
||||
}
|
||||
|
||||
GURL PermissionUtil::GetLastCommittedOriginAsURL(
|
||||
content::RenderFrameHost* render_frame_host) {
|
||||
DCHECK(render_frame_host);
|
||||
|
||||
if (base::FeatureList::IsEnabled(
|
||||
permissions::features::kRevisedOriginHandling)) {
|
||||
if (web_contents->GetLastCommittedURL().IsAboutBlank()) {
|
||||
return web_contents->GetMainFrame()->GetLastCommittedOrigin().GetURL();
|
||||
content::WebContents* web_contents =
|
||||
content::WebContents::FromRenderFrameHost(render_frame_host);
|
||||
// If `allow_universal_access_from_file_urls` flag is enabled, a file can
|
||||
// introduce discrepancy between GetLastCommittedURL and
|
||||
// GetLastCommittedOrigin. In that case GetLastCommittedURL should be used
|
||||
// for requesting and verifying permissions.
|
||||
// Disabling `kRevisedOriginHandling` feature introduces no side effects,
|
||||
// because in both cases we rely on GetLastCommittedURL().GetOrigin().
|
||||
if (web_contents->GetOrCreateWebPreferences()
|
||||
.allow_universal_access_from_file_urls &&
|
||||
render_frame_host->GetLastCommittedOrigin()
|
||||
.GetURL()
|
||||
.SchemeIsFileSystem()) {
|
||||
return render_frame_host->GetLastCommittedURL().GetOrigin();
|
||||
}
|
||||
|
||||
if (render_frame_host->GetLastCommittedURL().IsAboutBlank()) {
|
||||
return render_frame_host->GetLastCommittedOrigin().GetURL();
|
||||
}
|
||||
}
|
||||
|
||||
return web_contents->GetLastCommittedURL().GetOrigin();
|
||||
return render_frame_host->GetLastCommittedURL().GetOrigin();
|
||||
}
|
||||
|
||||
} // namespace content
|
||||
|
@ -9,6 +9,7 @@ class GURL;
|
||||
|
||||
namespace content {
|
||||
class WebContents;
|
||||
class RenderFrameHost;
|
||||
|
||||
class PermissionUtil {
|
||||
public:
|
||||
@ -18,6 +19,8 @@ class PermissionUtil {
|
||||
// ultimately all call sites should be migrated to determine the authoritative
|
||||
// security origin based on the requesting RenderFrameHost.
|
||||
static GURL GetLastCommittedOriginAsURL(content::WebContents* web_contents);
|
||||
static GURL GetLastCommittedOriginAsURL(
|
||||
content::RenderFrameHost* render_frame_host);
|
||||
};
|
||||
|
||||
} // namespace content
|
||||
|
Reference in New Issue
Block a user