From 2828774e6b285750441c4121bf27f40e1a28a97b Mon Sep 17 00:00:00 2001 From: oisupov Date: Mon, 24 Apr 2023 18:15:56 +0700 Subject: [PATCH 1/3] Disable brave wallet script injection if wallet is not created or MM is installed Resolves https://github.com/brave/brave-browser/issues/29853 --- ...brave_wallet_ethereum_chain_browsertest.cc | 3 + .../brave_wallet_event_emitter_browsertest.cc | 3 + .../brave_wallet_sign_message_browsertest.cc | 4 + .../brave_wallet_tab_helper_browsertest.cc | 4 + .../ethereum_provider_browsertest.cc | 4 + .../send_or_sign_transaction_browsertest.cc | 3 + .../solana_provider_browsertest.cc | 3 + .../solana_provider_renderer_browsertest.cc | 91 +++++++++++++++++++ .../wallet_watch_asset_browsertest.cc | 3 + browser/profiles/BUILD.gn | 10 ++ browser/profiles/brave_renderer_updater.cc | 39 +++++++- browser/profiles/brave_renderer_updater.h | 4 + renderer/test/BUILD.gn | 11 +++ .../test/js_ethereum_provider_browsertest.cc | 91 +++++++++++++++++++ 14 files changed, 269 insertions(+), 4 deletions(-) diff --git a/browser/brave_wallet/brave_wallet_ethereum_chain_browsertest.cc b/browser/brave_wallet/brave_wallet_ethereum_chain_browsertest.cc index 04066e0f3d73..239eff8f2abb 100644 --- a/browser/brave_wallet/brave_wallet_ethereum_chain_browsertest.cc +++ b/browser/brave_wallet/brave_wallet_ethereum_chain_browsertest.cc @@ -173,6 +173,9 @@ class BraveWalletEthereumChainTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); InProcessBrowserTest::SetUpOnMainThread(); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); diff --git a/browser/brave_wallet/brave_wallet_event_emitter_browsertest.cc b/browser/brave_wallet/brave_wallet_event_emitter_browsertest.cc index 042076422550..486575473a76 100644 --- a/browser/brave_wallet/brave_wallet_event_emitter_browsertest.cc +++ b/browser/brave_wallet/brave_wallet_event_emitter_browsertest.cc @@ -75,6 +75,9 @@ class BraveWalletEventEmitterTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); InProcessBrowserTest::SetUpOnMainThread(); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); diff --git a/browser/brave_wallet/brave_wallet_sign_message_browsertest.cc b/browser/brave_wallet/brave_wallet_sign_message_browsertest.cc index c1f674e5a238..9e26159232d7 100644 --- a/browser/brave_wallet/brave_wallet_sign_message_browsertest.cc +++ b/browser/brave_wallet/brave_wallet_sign_message_browsertest.cc @@ -13,6 +13,7 @@ #include "brave/browser/brave_wallet/brave_wallet_tab_helper.h" #include "brave/browser/brave_wallet/keyring_service_factory.h" #include "brave/components/brave_wallet/browser/brave_wallet_service.h" +#include "brave/components/brave_wallet/browser/brave_wallet_utils.h" #include "brave/components/brave_wallet/browser/keyring_service.h" #include "brave/components/brave_wallet/common/features.h" #include "brave/components/constants/brave_paths.h" @@ -77,6 +78,9 @@ class BraveWalletSignMessageBrowserTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); diff --git a/browser/brave_wallet/brave_wallet_tab_helper_browsertest.cc b/browser/brave_wallet/brave_wallet_tab_helper_browsertest.cc index 260e98506a79..dbe965d1ffeb 100644 --- a/browser/brave_wallet/brave_wallet_tab_helper_browsertest.cc +++ b/browser/brave_wallet/brave_wallet_tab_helper_browsertest.cc @@ -12,6 +12,7 @@ #include "brave/browser/brave_wallet/brave_wallet_tab_helper.h" #include "brave/browser/brave_wallet/json_rpc_service_factory.h" #include "brave/browser/ui/webui/brave_wallet/wallet_common_ui.h" +#include "brave/components/brave_wallet/browser/brave_wallet_utils.h" #include "brave/components/brave_wallet/common/features.h" #include "brave/components/constants/brave_paths.h" #include "brave/components/constants/webui_url_constants.h" @@ -104,6 +105,9 @@ class BraveWalletTabHelperBrowserTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); InProcessBrowserTest::SetUpOnMainThread(); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); diff --git a/browser/brave_wallet/ethereum_provider_browsertest.cc b/browser/brave_wallet/ethereum_provider_browsertest.cc index f6910075ab01..7569d85f1f2d 100644 --- a/browser/brave_wallet/ethereum_provider_browsertest.cc +++ b/browser/brave_wallet/ethereum_provider_browsertest.cc @@ -8,6 +8,7 @@ #include "base/path_service.h" #include "base/test/bind.h" #include "brave/browser/brave_wallet/keyring_service_factory.h" +#include "brave/components/brave_wallet/browser/brave_wallet_utils.h" #include "brave/components/brave_wallet/browser/keyring_service.h" #include "brave/components/constants/brave_paths.h" #include "chrome/browser/profiles/profile.h" @@ -64,6 +65,9 @@ class EthereumProviderBrowserTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); diff --git a/browser/brave_wallet/send_or_sign_transaction_browsertest.cc b/browser/brave_wallet/send_or_sign_transaction_browsertest.cc index 6d98eea2d59a..89cdb3463bd9 100644 --- a/browser/brave_wallet/send_or_sign_transaction_browsertest.cc +++ b/browser/brave_wallet/send_or_sign_transaction_browsertest.cc @@ -176,6 +176,9 @@ class SendOrSignTransactionBrowserTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); diff --git a/browser/brave_wallet/solana_provider_browsertest.cc b/browser/brave_wallet/solana_provider_browsertest.cc index 92bc18f9e967..9cfceec630c2 100644 --- a/browser/brave_wallet/solana_provider_browsertest.cc +++ b/browser/brave_wallet/solana_provider_browsertest.cc @@ -321,6 +321,9 @@ class SolanaProviderTest : public InProcessBrowserTest { ~SolanaProviderTest() override = default; void SetUpOnMainThread() override { + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); host_resolver()->AddRule("*", "127.0.0.1"); https_server_for_files_.SetSSLConfig( diff --git a/browser/brave_wallet/solana_provider_renderer_browsertest.cc b/browser/brave_wallet/solana_provider_renderer_browsertest.cc index 7c39232f7221..64290a0988f7 100644 --- a/browser/brave_wallet/solana_provider_renderer_browsertest.cc +++ b/browser/brave_wallet/solana_provider_renderer_browsertest.cc @@ -12,7 +12,11 @@ #include "base/strings/string_number_conversions.h" #include "base/test/scoped_feature_list.h" #include "brave/browser/brave_content_browser_client.h" +#include "brave/browser/brave_wallet/keyring_service_factory.h" +#include "brave/browser/profiles/brave_renderer_updater.h" +#include "brave/browser/profiles/brave_renderer_updater_factory.h" #include "brave/components/brave_wallet/browser/brave_wallet_utils.h" +#include "brave/components/brave_wallet/browser/keyring_service.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "brave/components/brave_wallet/common/brave_wallet_constants.h" #include "brave/components/brave_wallet/common/features.h" @@ -30,6 +34,7 @@ #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 "extensions/browser/disable_reason.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h" @@ -37,6 +42,14 @@ #include "net/test/embedded_test_server/embedded_test_server.h" #include "ui/base/l10n/l10n_util.h" +#if BUILDFLAG(ENABLE_EXTENSIONS) +#include "brave/browser/ethereum_remote_client/ethereum_remote_client_constants.h" +#include "chrome/browser/extensions/extension_service.h" +#include "extensions/browser/extension_system.h" +#include "extensions/common/extension.h" +#include "extensions/common/extension_builder.h" +#endif // BUILDFLAG(ENABLE_EXTENSIONS) + using brave_wallet::mojom::SolanaProviderError; namespace { @@ -532,6 +545,9 @@ class SolanaProviderRendererTest : public InProcessBrowserTest { void SetUpOnMainThread() override { InProcessBrowserTest::SetUpOnMainThread(); + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); content::SetBrowserClientForTesting(&test_content_browser_client_); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); @@ -637,6 +653,81 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, ExtensionOverwrite) { "test"); } +IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, + DoNotAttachIfNoWalletCreated) { + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + auto* keyring_service = + brave_wallet::KeyringServiceFactory::GetServiceForContext( + browser()->profile()); + keyring_service->Reset(false); + BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) + ->UpdateAllRenderersForTesting(); + + const GURL url = https_server_.GetURL("/simple.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + std::string command = "window.solana.isBraveWallet"; + EXPECT_TRUE(content::EvalJs(web_contents(browser()), command) + .error.find("Cannot read properties of undefined") != + std::string::npos); + EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); +} + +IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, AttachIfWalletCreated) { + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + auto* keyring_service = + brave_wallet::KeyringServiceFactory::GetServiceForContext( + browser()->profile()); + keyring_service->CreateWallet("password", base::DoNothing()); + BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) + ->UpdateAllRenderersForTesting(); + + const GURL url = https_server_.GetURL("/simple.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + std::string command = "window.solana.isBraveWallet"; + EXPECT_FALSE(content::EvalJs(web_contents(browser()), command) + .error.find("Cannot read properties of undefined") != + std::string::npos); + EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); +} + +#if BUILDFLAG(ENABLE_EXTENSIONS) +IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, + DoNotAttachIfExtensionIstalled) { + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + auto* keyring_service = + brave_wallet::KeyringServiceFactory::GetServiceForContext( + browser()->profile()); + keyring_service->CreateWallet("password", base::DoNothing()); + + scoped_refptr extension( + extensions::ExtensionBuilder("MetaMask") + .SetID(metamask_extension_id) + .Build()); + extensions::ExtensionSystem::Get(browser()->profile()) + ->extension_service() + ->AddExtension(extension.get()); + BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) + ->UpdateAllRenderersForTesting(); + + const GURL url = https_server_.GetURL("/simple.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + std::string command = "window.solana.isBraveWallet"; + EXPECT_TRUE(content::EvalJs(web_contents(browser()), command) + .error.find("Cannot read properties of undefined") != + std::string::npos); + EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); +} +#endif // BUILDFLAG(ENABLE_EXTENSIONS) + IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, NonWritable) { for (const std::string& provider : {"braveSolana", "solana"}) { // window.braveSolana.* and window.solana.* (methods) diff --git a/browser/brave_wallet/wallet_watch_asset_browsertest.cc b/browser/brave_wallet/wallet_watch_asset_browsertest.cc index 2065e91e0c86..f920fb122044 100644 --- a/browser/brave_wallet/wallet_watch_asset_browsertest.cc +++ b/browser/brave_wallet/wallet_watch_asset_browsertest.cc @@ -55,6 +55,9 @@ class WalletWatchAssetBrowserTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); host_resolver()->AddRule("*", "127.0.0.1"); diff --git a/browser/profiles/BUILD.gn b/browser/profiles/BUILD.gn index 23ffca857001..7b46583c4e19 100644 --- a/browser/profiles/BUILD.gn +++ b/browser/profiles/BUILD.gn @@ -1,5 +1,11 @@ +# Copyright (c) 2018 The Brave Authors. All rights reserved. +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this file, +# You can obtain one at https://mozilla.org/MPL/2.0/. + import("//brave/components/tor/buildflags/buildflags.gni") import("//components/gcm_driver/config.gni") +import("//extensions/buildflags/buildflags.gni") source_set("profiles") { # Remove when https://github.com/brave/brave-browser/issues/10648 is resolved @@ -42,6 +48,10 @@ source_set("profiles") { "//ui/base", ] + if (enable_extensions) { + deps += [ "//extensions/common" ] + } + if (use_gcm_from_platform) { deps += [ "//brave/browser/gcm_driver" ] } diff --git a/browser/profiles/brave_renderer_updater.cc b/browser/profiles/brave_renderer_updater.cc index 10633c9cf890..16cc45e80fad 100644 --- a/browser/profiles/brave_renderer_updater.cc +++ b/browser/profiles/brave_renderer_updater.cc @@ -9,8 +9,11 @@ #include "base/functional/bind.h" #include "brave/browser/brave_wallet/brave_wallet_context_utils.h" +#include "brave/browser/brave_wallet/keyring_service_factory.h" +#include "brave/browser/ethereum_remote_client/ethereum_remote_client_constants.h" #include "brave/common/brave_renderer_configuration.mojom.h" #include "brave/components/brave_wallet/browser/brave_wallet_utils.h" +#include "brave/components/brave_wallet/browser/keyring_service.h" #include "brave/components/brave_wallet/browser/pref_names.h" #include "brave/components/brave_wallet/common/brave_wallet.mojom.h" #include "brave/components/de_amp/browser/de_amp_util.h" @@ -20,9 +23,14 @@ #include "components/prefs/pref_service.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/render_process_host.h" +#include "extensions/buildflags/buildflags.h" #include "ipc/ipc_channel_proxy.h" #include "mojo/public/cpp/bindings/pending_receiver.h" +#if BUILDFLAG(ENABLE_EXTENSIONS) +#include "extensions/browser/extension_registry.h" +#endif + BraveRendererUpdater::BraveRendererUpdater(Profile* profile) : profile_(profile), is_wallet_allowed_for_context_(false) { PrefService* pref_service = profile->GetPrefs(); @@ -47,6 +55,10 @@ BraveRendererUpdater::BraveRendererUpdater(Profile* profile) BraveRendererUpdater::~BraveRendererUpdater() = default; +void BraveRendererUpdater::UpdateAllRenderersForTesting() { + UpdateAllRenderers(); +} + void BraveRendererUpdater::InitializeRenderer( content::RenderProcessHost* render_process_host) { auto renderer_configuration = GetRendererConfiguration(render_process_host); @@ -99,12 +111,30 @@ void BraveRendererUpdater::UpdateAllRenderers() { void BraveRendererUpdater::UpdateRenderer( mojo::AssociatedRemote* renderer_configuration) { +#if BUILDFLAG(ENABLE_EXTENSIONS) + extensions::ExtensionRegistry* registry = + extensions::ExtensionRegistry::Get(profile_); + bool has_installed_metamask = + registry && + registry->enabled_extensions().Contains(metamask_extension_id); +#else + bool has_installed_metamask = false; +#endif + + brave_wallet::KeyringService* keyring_service = + brave_wallet::KeyringServiceFactory::GetServiceForContext(profile_); + bool wallet_created = + keyring_service && + keyring_service->IsKeyringCreated(brave_wallet::mojom::kDefaultKeyringId); + bool should_ignore_brave_wallet = !wallet_created || has_installed_metamask; + auto default_ethereum_wallet = static_cast( brave_wallet_ethereum_provider_.GetValue()); bool brave_use_native_ethereum_wallet = - (default_ethereum_wallet == - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension || + ((default_ethereum_wallet == + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension && + !should_ignore_brave_wallet) || default_ethereum_wallet == brave_wallet::mojom::DefaultWallet::BraveWallet) && is_wallet_allowed_for_context_ && brave_wallet::IsDappsSupportEnabled(); @@ -115,8 +145,9 @@ void BraveRendererUpdater::UpdateRenderer( auto default_solana_wallet = static_cast( brave_wallet_solana_provider_.GetValue()); bool brave_use_native_solana_wallet = - (default_solana_wallet == - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension || + ((default_solana_wallet == + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension && + !should_ignore_brave_wallet) || default_solana_wallet == brave_wallet::mojom::DefaultWallet::BraveWallet) && is_wallet_allowed_for_context_ && brave_wallet::IsDappsSupportEnabled(); diff --git a/browser/profiles/brave_renderer_updater.h b/browser/profiles/brave_renderer_updater.h index c555899cb60f..1b2a43e5b91d 100644 --- a/browser/profiles/brave_renderer_updater.h +++ b/browser/profiles/brave_renderer_updater.h @@ -32,6 +32,10 @@ class BraveRendererUpdater : public KeyedService { // Initialize a newly-started renderer process. void InitializeRenderer(content::RenderProcessHost* render_process_host); + // TODO(cypt4): Remove this method after adding subscribtion on + // ExtensionRegistry and KeyringService + void UpdateAllRenderersForTesting(); + private: std::vector> GetRendererConfigurations(); diff --git a/renderer/test/BUILD.gn b/renderer/test/BUILD.gn index 29adcabb00aa..d1a9e43b549d 100644 --- a/renderer/test/BUILD.gn +++ b/renderer/test/BUILD.gn @@ -3,6 +3,7 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at http://mozilla.org/MPL/2.0/. +import("//extensions/buildflags/buildflags.gni") import("//testing/test.gni") source_set("browser_tests") { @@ -22,6 +23,7 @@ source_set("browser_tests") { deps = [ "//base/test:test_support", "//brave/browser/brave_wallet", + "//brave/browser/profiles:profiles", "//brave/common", "//brave/components/brave_wallet/browser", "//brave/components/brave_wallet/browser:utils", @@ -34,4 +36,13 @@ source_set("browser_tests") { "//content/test:test_support", "//net:test_support", ] + + if (enable_extensions) { + deps += [ + "//brave/browser/ethereum_remote_client:ethereum_remote_client", + "//chrome/browser/extensions", + "//extensions:test_support", + "//extensions/browser", + ] + } } diff --git a/renderer/test/js_ethereum_provider_browsertest.cc b/renderer/test/js_ethereum_provider_browsertest.cc index 989556b44fec..0bd86328e7c7 100644 --- a/renderer/test/js_ethereum_provider_browsertest.cc +++ b/renderer/test/js_ethereum_provider_browsertest.cc @@ -4,12 +4,17 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ #include + #include "base/path_service.h" #include "base/strings/stringprintf.h" #include "base/test/metrics/histogram_tester.h" #include "brave/browser/brave_wallet/json_rpc_service_factory.h" +#include "brave/browser/brave_wallet/keyring_service_factory.h" +#include "brave/browser/profiles/brave_renderer_updater.h" +#include "brave/browser/profiles/brave_renderer_updater_factory.h" #include "brave/components/brave_wallet/browser/brave_wallet_utils.h" #include "brave/components/brave_wallet/browser/json_rpc_service.h" +#include "brave/components/brave_wallet/browser/keyring_service.h" #include "brave/components/constants/brave_paths.h" #include "build/build_config.h" #include "chrome/browser/profiles/profile.h" @@ -27,6 +32,14 @@ #include "net/test/embedded_test_server/embedded_test_server.h" #include "url/gurl.h" +#if BUILDFLAG(ENABLE_EXTENSIONS) +#include "brave/browser/ethereum_remote_client/ethereum_remote_client_constants.h" +#include "chrome/browser/extensions/extension_service.h" +#include "extensions/browser/extension_system.h" +#include "extensions/common/extension.h" +#include "extensions/common/extension_builder.h" +#endif // BUILDFLAG(ENABLE_EXTENSIONS) + namespace { std::string NonWriteableScriptProperty(const std::string& property) { return base::StringPrintf( @@ -81,6 +94,9 @@ class JSEthereumProviderBrowserTest : public InProcessBrowserTest { } void SetUpOnMainThread() override { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWallet); InProcessBrowserTest::SetUpOnMainThread(); mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK); @@ -185,6 +201,81 @@ IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); } +IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, + DoNotAttachIfNoWalletCreated) { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + auto* keyring_service = + brave_wallet::KeyringServiceFactory::GetServiceForContext( + browser()->profile()); + keyring_service->Reset(false); + BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) + ->UpdateAllRenderersForTesting(); + + const GURL url = https_server_.GetURL("/simple.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + std::string command = "window.ethereum.isBraveWallet"; + EXPECT_TRUE(content::EvalJs(primary_main_frame(), command) + .error.find("Cannot read properties of undefined") != + std::string::npos); + EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); +} + +IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, AttachIfNoWalletCreated) { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + auto* keyring_service = + brave_wallet::KeyringServiceFactory::GetServiceForContext( + browser()->profile()); + keyring_service->CreateWallet("password", base::DoNothing()); + BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) + ->UpdateAllRenderersForTesting(); + + const GURL url = https_server_.GetURL("/simple.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + std::string command = "window.ethereum.isBraveWallet"; + EXPECT_FALSE(content::EvalJs(primary_main_frame(), command) + .error.find("Cannot read properties of undefined") != + std::string::npos); + EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); +} + +#if BUILDFLAG(ENABLE_EXTENSIONS) +IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, + DoNotAttachIfMetamaskInstalled) { + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + auto* keyring_service = + brave_wallet::KeyringServiceFactory::GetServiceForContext( + browser()->profile()); + keyring_service->CreateWallet("password", base::DoNothing()); + + scoped_refptr extension( + extensions::ExtensionBuilder("MetaMask") + .SetID(metamask_extension_id) + .Build()); + extensions::ExtensionSystem::Get(browser()->profile()) + ->extension_service() + ->AddExtension(extension.get()); + BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) + ->UpdateAllRenderersForTesting(); + + const GURL url = https_server_.GetURL("/simple.html"); + ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + + std::string command = "window.ethereum.isBraveWallet"; + EXPECT_TRUE(content::EvalJs(primary_main_frame(), command) + .error.find("Cannot read properties of undefined") != + std::string::npos); + EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); +} +#endif // BUILDFLAG(ENABLE_EXTENSIONS) + IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, NonWritable) { const GURL url = https_server_.GetURL("/simple.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); From c624e8e8f4f1864836783b869513569eb3c4a19d Mon Sep 17 00:00:00 2001 From: oisupov Date: Tue, 25 Apr 2023 01:29:57 +0700 Subject: [PATCH 2/3] Review fix --- .../solana_provider_renderer_browsertest.cc | 44 +++++++------------ browser/profiles/brave_renderer_updater.cc | 4 -- browser/profiles/brave_renderer_updater.h | 4 -- .../test/js_ethereum_provider_browsertest.cc | 39 ++++++++-------- 4 files changed, 35 insertions(+), 56 deletions(-) diff --git a/browser/brave_wallet/solana_provider_renderer_browsertest.cc b/browser/brave_wallet/solana_provider_renderer_browsertest.cc index 64290a0988f7..48c3694e3422 100644 --- a/browser/brave_wallet/solana_provider_renderer_browsertest.cc +++ b/browser/brave_wallet/solana_provider_renderer_browsertest.cc @@ -34,7 +34,6 @@ #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 "extensions/browser/disable_reason.h" #include "mojo/public/cpp/bindings/pending_remote.h" #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h" @@ -655,18 +654,15 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, ExtensionOverwrite) { IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, DoNotAttachIfNoWalletCreated) { - brave_wallet::SetDefaultSolanaWallet( - browser()->profile()->GetPrefs(), - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); auto* keyring_service = brave_wallet::KeyringServiceFactory::GetServiceForContext( browser()->profile()); keyring_service->Reset(false); - BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) - ->UpdateAllRenderersForTesting(); - const GURL url = https_server_.GetURL("/simple.html"); - ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + ReloadAndWaitForLoadStop(browser()); std::string command = "window.solana.isBraveWallet"; EXPECT_TRUE(content::EvalJs(web_contents(browser()), command) @@ -676,32 +672,26 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, } IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, AttachIfWalletCreated) { - brave_wallet::SetDefaultSolanaWallet( - browser()->profile()->GetPrefs(), - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); auto* keyring_service = brave_wallet::KeyringServiceFactory::GetServiceForContext( browser()->profile()); keyring_service->CreateWallet("password", base::DoNothing()); - BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) - ->UpdateAllRenderersForTesting(); - const GURL url = https_server_.GetURL("/simple.html"); - ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + ReloadAndWaitForLoadStop(browser()); - std::string command = "window.solana.isBraveWallet"; - EXPECT_FALSE(content::EvalJs(web_contents(browser()), command) - .error.find("Cannot read properties of undefined") != - std::string::npos); + constexpr char kEvalIsBraveWallet[] = "window.solana.isBraveWallet"; + EXPECT_TRUE(content::EvalJs(web_contents(browser())->GetPrimaryMainFrame(), + kEvalIsBraveWallet) + .ExtractBool()); EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); } #if BUILDFLAG(ENABLE_EXTENSIONS) IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, - DoNotAttachIfExtensionIstalled) { - brave_wallet::SetDefaultSolanaWallet( - browser()->profile()->GetPrefs(), - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + DoNotAttachIfMetaMaskInstalled) { auto* keyring_service = brave_wallet::KeyringServiceFactory::GetServiceForContext( browser()->profile()); @@ -714,11 +704,11 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, extensions::ExtensionSystem::Get(browser()->profile()) ->extension_service() ->AddExtension(extension.get()); - BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) - ->UpdateAllRenderersForTesting(); - const GURL url = https_server_.GetURL("/simple.html"); - ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); + brave_wallet::SetDefaultSolanaWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + ReloadAndWaitForLoadStop(browser()); std::string command = "window.solana.isBraveWallet"; EXPECT_TRUE(content::EvalJs(web_contents(browser()), command) diff --git a/browser/profiles/brave_renderer_updater.cc b/browser/profiles/brave_renderer_updater.cc index 16cc45e80fad..27bf6e86264a 100644 --- a/browser/profiles/brave_renderer_updater.cc +++ b/browser/profiles/brave_renderer_updater.cc @@ -55,10 +55,6 @@ BraveRendererUpdater::BraveRendererUpdater(Profile* profile) BraveRendererUpdater::~BraveRendererUpdater() = default; -void BraveRendererUpdater::UpdateAllRenderersForTesting() { - UpdateAllRenderers(); -} - void BraveRendererUpdater::InitializeRenderer( content::RenderProcessHost* render_process_host) { auto renderer_configuration = GetRendererConfiguration(render_process_host); diff --git a/browser/profiles/brave_renderer_updater.h b/browser/profiles/brave_renderer_updater.h index 1b2a43e5b91d..c555899cb60f 100644 --- a/browser/profiles/brave_renderer_updater.h +++ b/browser/profiles/brave_renderer_updater.h @@ -32,10 +32,6 @@ class BraveRendererUpdater : public KeyedService { // Initialize a newly-started renderer process. void InitializeRenderer(content::RenderProcessHost* render_process_host); - // TODO(cypt4): Remove this method after adding subscribtion on - // ExtensionRegistry and KeyringService - void UpdateAllRenderersForTesting(); - private: std::vector> GetRendererConfigurations(); diff --git a/renderer/test/js_ethereum_provider_browsertest.cc b/renderer/test/js_ethereum_provider_browsertest.cc index 0bd86328e7c7..02cc7b652dc4 100644 --- a/renderer/test/js_ethereum_provider_browsertest.cc +++ b/renderer/test/js_ethereum_provider_browsertest.cc @@ -203,15 +203,14 @@ IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, DoNotAttachIfNoWalletCreated) { - brave_wallet::SetDefaultEthereumWallet( - browser()->profile()->GetPrefs(), - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); auto* keyring_service = brave_wallet::KeyringServiceFactory::GetServiceForContext( browser()->profile()); keyring_service->Reset(false); - BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) - ->UpdateAllRenderersForTesting(); + + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); const GURL url = https_server_.GetURL("/simple.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); @@ -223,33 +222,29 @@ IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); } -IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, AttachIfNoWalletCreated) { - brave_wallet::SetDefaultEthereumWallet( - browser()->profile()->GetPrefs(), - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); +IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, AttachIfWalletCreated) { auto* keyring_service = brave_wallet::KeyringServiceFactory::GetServiceForContext( browser()->profile()); keyring_service->CreateWallet("password", base::DoNothing()); - BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) - ->UpdateAllRenderersForTesting(); + + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); const GURL url = https_server_.GetURL("/simple.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); - std::string command = "window.ethereum.isBraveWallet"; - EXPECT_FALSE(content::EvalJs(primary_main_frame(), command) - .error.find("Cannot read properties of undefined") != - std::string::npos); + constexpr char kEvalIsBraveWallet[] = "window.ethereum.isBraveWallet"; + EXPECT_TRUE( + content::EvalJs(primary_main_frame(), kEvalIsBraveWallet).ExtractBool()); + EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); } #if BUILDFLAG(ENABLE_EXTENSIONS) IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, - DoNotAttachIfMetamaskInstalled) { - brave_wallet::SetDefaultEthereumWallet( - browser()->profile()->GetPrefs(), - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); + DoNotAttachIfMetaMaskInstalled) { auto* keyring_service = brave_wallet::KeyringServiceFactory::GetServiceForContext( browser()->profile()); @@ -262,8 +257,10 @@ IN_PROC_BROWSER_TEST_F(JSEthereumProviderBrowserTest, extensions::ExtensionSystem::Get(browser()->profile()) ->extension_service() ->AddExtension(extension.get()); - BraveRendererUpdaterFactory::GetForProfile(browser()->profile()) - ->UpdateAllRenderersForTesting(); + + brave_wallet::SetDefaultEthereumWallet( + browser()->profile()->GetPrefs(), + brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); const GURL url = https_server_.GetURL("/simple.html"); ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url)); From 253b40c58c83f0ba39ecc0dc0759cce78d6fdf69 Mon Sep 17 00:00:00 2001 From: oisupov Date: Tue, 25 Apr 2023 02:51:13 +0700 Subject: [PATCH 3/3] Don't check metamask installation for Solana --- .../solana_provider_renderer_browsertest.cc | 38 ------------------- browser/profiles/brave_renderer_updater.cc | 8 ++-- 2 files changed, 5 insertions(+), 41 deletions(-) diff --git a/browser/brave_wallet/solana_provider_renderer_browsertest.cc b/browser/brave_wallet/solana_provider_renderer_browsertest.cc index 48c3694e3422..405c2c68ff21 100644 --- a/browser/brave_wallet/solana_provider_renderer_browsertest.cc +++ b/browser/brave_wallet/solana_provider_renderer_browsertest.cc @@ -41,14 +41,6 @@ #include "net/test/embedded_test_server/embedded_test_server.h" #include "ui/base/l10n/l10n_util.h" -#if BUILDFLAG(ENABLE_EXTENSIONS) -#include "brave/browser/ethereum_remote_client/ethereum_remote_client_constants.h" -#include "chrome/browser/extensions/extension_service.h" -#include "extensions/browser/extension_system.h" -#include "extensions/common/extension.h" -#include "extensions/common/extension_builder.h" -#endif // BUILDFLAG(ENABLE_EXTENSIONS) - using brave_wallet::mojom::SolanaProviderError; namespace { @@ -688,36 +680,6 @@ IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, AttachIfWalletCreated) { .ExtractBool()); EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); } - -#if BUILDFLAG(ENABLE_EXTENSIONS) -IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, - DoNotAttachIfMetaMaskInstalled) { - auto* keyring_service = - brave_wallet::KeyringServiceFactory::GetServiceForContext( - browser()->profile()); - keyring_service->CreateWallet("password", base::DoNothing()); - - scoped_refptr extension( - extensions::ExtensionBuilder("MetaMask") - .SetID(metamask_extension_id) - .Build()); - extensions::ExtensionSystem::Get(browser()->profile()) - ->extension_service() - ->AddExtension(extension.get()); - - brave_wallet::SetDefaultSolanaWallet( - browser()->profile()->GetPrefs(), - brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension); - ReloadAndWaitForLoadStop(browser()); - - std::string command = "window.solana.isBraveWallet"; - EXPECT_TRUE(content::EvalJs(web_contents(browser()), command) - .error.find("Cannot read properties of undefined") != - std::string::npos); - EXPECT_EQ(browser()->tab_strip_model()->GetTabCount(), 1); -} -#endif // BUILDFLAG(ENABLE_EXTENSIONS) - IN_PROC_BROWSER_TEST_F(SolanaProviderRendererTest, NonWritable) { for (const std::string& provider : {"braveSolana", "solana"}) { // window.braveSolana.* and window.solana.* (methods) diff --git a/browser/profiles/brave_renderer_updater.cc b/browser/profiles/brave_renderer_updater.cc index 27bf6e86264a..607c84674fb0 100644 --- a/browser/profiles/brave_renderer_updater.cc +++ b/browser/profiles/brave_renderer_updater.cc @@ -122,7 +122,9 @@ void BraveRendererUpdater::UpdateRenderer( bool wallet_created = keyring_service && keyring_service->IsKeyringCreated(brave_wallet::mojom::kDefaultKeyringId); - bool should_ignore_brave_wallet = !wallet_created || has_installed_metamask; + bool should_ignore_brave_wallet_for_eth = + !wallet_created || has_installed_metamask; + bool should_ignore_brave_wallet_for_sol = !wallet_created; auto default_ethereum_wallet = static_cast( @@ -130,7 +132,7 @@ void BraveRendererUpdater::UpdateRenderer( bool brave_use_native_ethereum_wallet = ((default_ethereum_wallet == brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension && - !should_ignore_brave_wallet) || + !should_ignore_brave_wallet_for_eth) || default_ethereum_wallet == brave_wallet::mojom::DefaultWallet::BraveWallet) && is_wallet_allowed_for_context_ && brave_wallet::IsDappsSupportEnabled(); @@ -143,7 +145,7 @@ void BraveRendererUpdater::UpdateRenderer( bool brave_use_native_solana_wallet = ((default_solana_wallet == brave_wallet::mojom::DefaultWallet::BraveWalletPreferExtension && - !should_ignore_brave_wallet) || + !should_ignore_brave_wallet_for_sol) || default_solana_wallet == brave_wallet::mojom::DefaultWallet::BraveWallet) && is_wallet_allowed_for_context_ && brave_wallet::IsDappsSupportEnabled();