Skip to content

Commit

Permalink
Allow permission requests within the same eTLD+1
Browse files Browse the repository at this point in the history
  • Loading branch information
bbondy committed Jun 3, 2022
1 parent bb58896 commit 3e1a254
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 15 deletions.
9 changes: 9 additions & 0 deletions browser/brave_wallet/solana_provider_renderer_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1028,4 +1028,13 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, Block3PIframe) {
iframe_rfh = ChildFrameAt(main_frame, 0);
ASSERT_TRUE(iframe_rfh);
EXPECT_FALSE(content::EvalJs(iframe_rfh, kEvalSolana).ExtractBool());

// same eTLD+1
GURL iframe_url_etldp1(
embedded_test_server()->GetURL("dapps.please.a.com", "/"));
EXPECT_TRUE(
NavigateIframeToURL(web_contents(browser()), "test", iframe_url_etldp1));
iframe_rfh = ChildFrameAt(main_frame, 0);
ASSERT_TRUE(iframe_rfh);
EXPECT_FALSE(content::EvalJs(iframe_rfh, kEvalSolana).ExtractBool());
}
21 changes: 21 additions & 0 deletions browser/permissions/brave_wallet_permission_context_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,25 @@ IN_PROC_BROWSER_TEST_F(BraveWalletPermissionContextBrowserTest,
EXPECT_TRUE(was_called);
}

IN_PROC_BROWSER_TEST_F(BraveWalletPermissionContextBrowserTest,
GetAllowedAccountsAllowETLDP1Iframe) {
std::vector<std::string> addresses = {
"0xaf5Ad1E10926C0Ee4af4eDAC61DD60E853753f8A",
"0xaf5Ad1E10926C0Ee4af4eDAC61DD60E853753f8B"};

GURL top_url(https_server()->GetURL("a.com", "/iframe.html"));
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), top_url));
GURL iframe_url(https_server()->GetURL("c.b.a.com", "/"));
EXPECT_TRUE(NavigateIframeToURL(web_contents(), "test", iframe_url));

bool was_called = false;
auto* iframe_rfh = ChildFrameAt(web_contents()->GetMainFrame(), 0);
BraveWalletPermissionContext::GetAllowedAccounts(
ContentSettingsType::BRAVE_ETHEREUM, iframe_rfh, addresses,
base::BindOnce(&OnGetAllowedAccountsResult, &was_called, true,
std::vector<std::string>()));
content::RunAllTasksUntilIdle();
EXPECT_TRUE(was_called);
}

} // namespace permissions
81 changes: 75 additions & 6 deletions browser/permissions/permission_manager_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/command_line.h"
#include "base/feature_list.h"
#include "base/memory/raw_ptr.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "brave/components/brave_wallet/browser/permission_utils.h"
#include "brave/components/brave_wallet/common/features.h"
Expand All @@ -29,6 +30,7 @@
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_mock_cert_verifier.h"
#include "content/public/test/test_utils.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
Expand Down Expand Up @@ -81,7 +83,7 @@ class PermissionManagerBrowserTest : public InProcessBrowserTest {
void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();
host_resolver()->AddRule("*", "127.0.0.1");
https_server_.SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES);
mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK);
https_server()->ServeFilesFromSourceDirectory(GetChromeTestDataDir());
ASSERT_TRUE(https_server()->Start());

Expand All @@ -98,6 +100,21 @@ class PermissionManagerBrowserTest : public InProcessBrowserTest {
return HostContentSettingsMapFactory::GetForProfile(browser()->profile());
}

void SetUpCommandLine(base::CommandLine* command_line) override {
InProcessBrowserTest::SetUpCommandLine(command_line);
mock_cert_verifier_.SetUpCommandLine(command_line);
}

void SetUpInProcessBrowserTestFixture() override {
InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
mock_cert_verifier_.SetUpInProcessBrowserTestFixture();
}

void TearDownInProcessBrowserTestFixture() override {
mock_cert_verifier_.TearDownInProcessBrowserTestFixture();
InProcessBrowserTest::TearDownInProcessBrowserTestFixture();
}

content::WebContents* web_contents() {
return browser()->tab_strip_model()->GetActiveWebContents();
}
Expand All @@ -119,11 +136,12 @@ class PermissionManagerBrowserTest : public InProcessBrowserTest {
raw_ptr<PermissionManager> permission_manager_ = nullptr;

private:
content::ContentMockCertVerifier mock_cert_verifier_;
base::test::ScopedFeatureList scoped_feature_list_;
};

IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest, RequestPermissions) {
const GURL& url = https_server()->GetURL("a.test", "/empty.html");
const GURL& url = https_server()->GetURL("a.com", "/empty.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));

auto* permission_request_manager = GetPermissionRequestManager();
Expand Down Expand Up @@ -238,7 +256,7 @@ IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest, RequestPermissions) {

IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest,
RequestPermissionsTabClosed) {
const GURL& url = https_server()->GetURL("a.test", "/empty.html");
const GURL& url = https_server()->GetURL("a.com", "/empty.html");

struct {
std::vector<std::string> addresses;
Expand Down Expand Up @@ -321,9 +339,9 @@ IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest,
"JDqrvDz8d8tFCADashbUKQDKfJZFobNy13ugN65t1wvV"},
ContentSettingsType::BRAVE_SOLANA}};

GURL top_url(https_server()->GetURL("a.test", "/iframe.html"));
GURL top_url(https_server()->GetURL("a.com", "/iframe.html"));
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), top_url));
GURL iframe_url(https_server()->GetURL("b.test", "/"));
GURL iframe_url(https_server()->GetURL("b.com", "/"));
EXPECT_TRUE(NavigateIframeToURL(web_contents(), "test", iframe_url));

auto* iframe_rfh = ChildFrameAt(web_contents()->GetMainFrame(), 0);
Expand All @@ -341,8 +359,59 @@ IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest,
}
}

IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest,
RequestPermissionsAllowETLDP1) {
struct {
std::vector<std::string> addresses;
ContentSettingsType type;
} cases[] = {{{"0xaf5Ad1E10926C0Ee4af4eDAC61DD60E853753f8A",
"0xaf5Ad1E10926C0Ee4af4eDAC61DD60E853753f8B"},
ContentSettingsType::BRAVE_ETHEREUM},
{{"BrG44HdsEhzapvs8bEqzvkq4egwevS3fRE6ze2ENo6S8",
"JDqrvDz8d8tFCADashbUKQDKfJZFobNy13ugN65t1wvV"},
ContentSettingsType::BRAVE_SOLANA}};

GURL top_url(https_server()->GetURL("a.com", "/iframe.html"));
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), top_url));
GURL iframe_url(https_server()->GetURL("c.b.a.com", "/"));
EXPECT_TRUE(NavigateIframeToURL(web_contents(), "test", iframe_url));
WaitForLoadStop(web_contents());

auto* iframe_rfh = ChildFrameAt(web_contents()->GetMainFrame(), 0);
auto* permission_request_manager = GetPermissionRequestManager();

for (const auto& current_case : cases) {
SCOPED_TRACE(testing::PrintToString(current_case.addresses));
auto observer = std::make_unique<PermissionRequestManagerObserver>(
permission_request_manager);

base::RunLoop run_loop;
BraveWalletPermissionContext::RequestPermissions(
current_case.type, iframe_rfh, current_case.addresses,
// This callback is not called until after a response is given
base::BindLambdaForTesting(
[&](const std::vector<ContentSetting>& responses) {
EXPECT_FALSE(responses.empty());
run_loop.Quit();
}));

RequestType request_type =
ContentSettingsTypeToRequestType(current_case.type);
// Needed for the request to be considered to be in progress
content::RunAllTasksUntilIdle();
EXPECT_TRUE(BraveWalletPermissionContext::HasRequestsInProgress(
iframe_rfh, request_type));
EXPECT_TRUE(observer->IsShowingBubble());
EXPECT_FALSE(IsPendingGroupedRequestsEmpty(current_case.type));
permissions::BraveWalletPermissionContext::Cancel(web_contents());
run_loop.Run();
EXPECT_FALSE(BraveWalletPermissionContext::HasRequestsInProgress(
iframe_rfh, request_type));
}
}

IN_PROC_BROWSER_TEST_F(PermissionManagerBrowserTest, GetCanonicalOrigin) {
const GURL& url = https_server()->GetURL("a.test", "/empty.html");
const GURL& url = https_server()->GetURL("a.com", "/empty.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));

struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "build/build_config.h"
#include "components/permissions/permission_request.h"
#include "components/permissions/request_type.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "url/origin.h"

#if BUILDFLAG(IS_ANDROID)
Expand All @@ -39,7 +40,9 @@ bool ChromePermissionsClient::BraveCanBypassEmbeddingOriginCheck(
permissions::ContentSettingsTypeToRequestType(type),
url::Origin::Create(requesting_origin), &original_requesting_origin,
nullptr) &&
original_requesting_origin == url::Origin::Create(embedding_origin)) {
net::registry_controlled_domains::SameDomainOrHost(
original_requesting_origin, url::Origin::Create(embedding_origin),
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
return true;
}

Expand Down
20 changes: 14 additions & 6 deletions components/permissions/contexts/brave_wallet_permission_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "components/permissions/permissions_client.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom.h"
#include "url/origin.h"

Expand Down Expand Up @@ -163,9 +164,12 @@ void BraveWalletPermissionContext::RequestPermissions(
}

auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
// Fail the request came from 3p origin.
if (web_contents->GetMainFrame()->GetLastCommittedOrigin() !=
rfh->GetLastCommittedOrigin()) {
// Fail the request came from a different eTLD+1 origin.
if (rfh->GetParent() &&
!net::registry_controlled_domains::SameDomainOrHost(
web_contents->GetMainFrame()->GetLastCommittedOrigin(),
rfh->GetLastCommittedOrigin(),
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
std::move(callback).Run(std::vector<ContentSetting>());
return;
}
Expand Down Expand Up @@ -211,9 +215,13 @@ void BraveWalletPermissionContext::GetAllowedAccounts(
}

auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
// Fail the request came from 3p origin.
if (web_contents->GetMainFrame()->GetLastCommittedOrigin() !=
rfh->GetLastCommittedOrigin()) {
// Fail the request came from a different eTLD+1 origin. is missing for this
// function
if (rfh->GetParent() &&
!net::registry_controlled_domains::SameDomainOrHost(
web_contents->GetMainFrame()->GetLastCommittedOrigin(),
rfh->GetLastCommittedOrigin(),
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
std::move(callback).Run(false, std::vector<std::string>());
return;
}
Expand Down
10 changes: 8 additions & 2 deletions renderer/brave_wallet/brave_wallet_render_frame_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "brave/components/brave_wallet/common/features.h"
#include "content/public/common/isolated_world_ids.h"
#include "content/public/renderer/render_frame.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/web/web_local_frame.h"

Expand Down Expand Up @@ -45,9 +46,14 @@ void BraveWalletRenderFrameObserver::DidCreateScriptContext(
js_ethereum_provider_.reset();
return;
}
// Wallet provider objects won't be generated for third party iframe

// Wallet provider objects won't be generated for different eTLD+1s
if (!render_frame()->IsMainFrame() &&
render_frame()->GetWebFrame()->IsCrossOriginToOutermostMainFrame()) {
!net::registry_controlled_domains::SameDomainOrHost(
url::Origin(
render_frame()->GetWebFrame()->Top()->GetSecurityOrigin()),
url::Origin(render_frame()->GetWebFrame()->GetSecurityOrigin()),
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
return;
}

Expand Down
7 changes: 7 additions & 0 deletions renderer/test/js_ethereum_provider_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,4 +227,11 @@ IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, Block3PIframe) {
iframe_rfh = ChildFrameAt(main_frame(), 0);
ASSERT_TRUE(iframe_rfh);
EXPECT_FALSE(content::EvalJs(iframe_rfh, kEvalEthereum).ExtractBool());

// same eTLD+1
GURL iframe_url_etldp1(https_server_.GetURL("c.b.a.com", "/"));
EXPECT_TRUE(NavigateIframeToURL(web_contents(), "test", iframe_url_etldp1));
iframe_rfh = ChildFrameAt(main_frame(), 0);
ASSERT_TRUE(iframe_rfh);
EXPECT_FALSE(content::EvalJs(iframe_rfh, kEvalEthereum).ExtractBool());
}

0 comments on commit 3e1a254

Please sign in to comment.