From 3cfde182bb4f938b265f9f2dca72464c1f1d5b0b Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Sat, 7 Sep 2024 03:26:07 -0700 Subject: [PATCH] Create group policy for sync URL Fixes https://github.com/brave/brave-browser/issues/20431 --- browser/policy/brave_simple_policy_map.h | 5 ++ .../policy/tools/generate_policy_source.py | 20 ++++++++ .../sync/service/sync_service_impl.cc | 40 ++++++++++++++++ components/brave_sync/brave_sync_prefs.cc | 1 + components/brave_sync/brave_sync_prefs.h | 2 + .../brave_sync_service_impl_unittest.cc | 48 ++++++++++++++++++- 6 files changed, 114 insertions(+), 2 deletions(-) diff --git a/browser/policy/brave_simple_policy_map.h b/browser/policy/brave_simple_policy_map.h index 662df3a1da09..19ef7c89834d 100644 --- a/browser/policy/brave_simple_policy_map.h +++ b/browser/policy/brave_simple_policy_map.h @@ -7,6 +7,7 @@ #define BRAVE_BROWSER_POLICY_BRAVE_SIMPLE_POLICY_MAP_H_ #include "brave/components/ai_chat/core/common/buildflags/buildflags.h" +#include "brave/components/brave_sync/brave_sync_prefs.h" #include "brave/components/brave_vpn/common/buildflags/buildflags.h" #include "brave/components/constants/pref_names.h" #include "brave/components/tor/buildflags/buildflags.h" @@ -44,6 +45,10 @@ inline constexpr PolicyToPreferenceMapEntry kBraveSimplePolicyMap[] = { {policy::key::kBraveShieldsEnabledForUrls, kManagedBraveShieldsEnabledForUrls, base::Value::Type::LIST}, #endif + + {policy::key::kBraveSyncUrl, brave_sync::kManagedBraveSyncUrl, + base::Value::Type::STRING}, + #if BUILDFLAG(ENABLE_TOR) {policy::key::kTorDisabled, tor::prefs::kTorDisabled, base::Value::Type::BOOLEAN}, diff --git a/chromium_src/components/policy/tools/generate_policy_source.py b/chromium_src/components/policy/tools/generate_policy_source.py index 4e1479e60989..be0745ebc0d9 100644 --- a/chromium_src/components/policy/tools/generate_policy_source.py +++ b/chromium_src/components/policy/tools/generate_policy_source.py @@ -166,6 +166,26 @@ def AddBravePolicies(template_file_contents): 'desc': ('''This policy allows an admin to specify that Brave ''' '''AI Chat feature will be enabled.'''), }, + { + 'name': 'BraveSyncUrl', + 'type': 'main', + 'schema': { + 'type': 'string' + }, + 'supported_on': ['chrome.*:128-'], + 'features': { + 'dynamic_refresh': False, + 'per_profile': True, + 'can_be_recommended': False, + 'can_be_mandatory': True + }, + 'example_value': ['https://sync-v2.brave.com/v2'], + 'id': 8, + 'caption': '''Custom sync server URL.''', + 'tags': [], + 'desc': ('''This policy allows an admin to specify a custom ''' + '''sync server URL for Brave.'''), + }, ] # Our new polices are added with highest id diff --git a/chromium_src/components/sync/service/sync_service_impl.cc b/chromium_src/components/sync/service/sync_service_impl.cc index 47f9d87cf568..f0573fa8ca44 100644 --- a/chromium_src/components/sync/service/sync_service_impl.cc +++ b/chromium_src/components/sync/service/sync_service_impl.cc @@ -3,13 +3,53 @@ * 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/. */ +#include "brave/components/brave_sync/brave_sync_prefs.h" #include "brave/components/sync/service/brave_sync_auth_manager.h" #include "brave/components/sync/service/brave_sync_stopped_reporter.h" +#include "components/prefs/pref_service.h" +#include "components/sync/base/sync_util.h" + +namespace syncer { + +GURL BraveGetSyncServiceURL(const base::CommandLine& command_line, + version_info::Channel channel, + PrefService* prefs) { + // Allow group policy to override sync service URL. + // This has a higher priority than the --sync-url command-line param. + // https://github.com/brave/brave-browser/issues/20431 + if (prefs) { + if (prefs->IsManagedPreference(brave_sync::kManagedBraveSyncUrl)) { + std::string value(prefs->GetString(brave_sync::kManagedBraveSyncUrl)); + if (!value.empty()) { + GURL custom_sync_url(value); + // Provided URL must be HTTPS. + if (custom_sync_url.is_valid() && + custom_sync_url.SchemeIs(url::kHttpsScheme)) { + DVLOG(2) << "Sync URL specified via GPO: " + << prefs->GetString(brave_sync::kManagedBraveSyncUrl); + return custom_sync_url; + } else { + LOG(WARNING) << "The following sync URL specified via GPO " + << "is invalid: " << value; + } + } + } + } + + // Default logic. + // See `GetSyncServiceURL` in `components/sync/base/sync_util.cc` + return GetSyncServiceURL(command_line, channel); +} + +} // namespace syncer #define SyncAuthManager BraveSyncAuthManager #define SyncStoppedReporter BraveSyncStoppedReporter +#define GetSyncServiceURL(...) \ + BraveGetSyncServiceURL(__VA_ARGS__, sync_client_->GetPrefService()) #include "src/components/sync/service/sync_service_impl.cc" #undef SyncAuthManager #undef SyncStoppedReporter +#undef GetSyncServiceURL diff --git a/components/brave_sync/brave_sync_prefs.cc b/components/brave_sync/brave_sync_prefs.cc index a3b8630ebfe9..3ba167c03771 100644 --- a/components/brave_sync/brave_sync_prefs.cc +++ b/components/brave_sync/brave_sync_prefs.cc @@ -83,6 +83,7 @@ void Prefs::RegisterProfilePrefs(PrefRegistrySimple* registry) { registry->RegisterBooleanPref(kSyncFailedDecryptSeedNoticeDismissed, false); registry->RegisterBooleanPref(kSyncAccountDeletedNoticePending, false); registry->RegisterStringPref(kSyncLeaveChainDetails, std::string()); + registry->RegisterStringPref(kManagedBraveSyncUrl, std::string()); } // static diff --git a/components/brave_sync/brave_sync_prefs.h b/components/brave_sync/brave_sync_prefs.h index 4d7f2a2f0acc..323fa1522fbf 100644 --- a/components/brave_sync/brave_sync_prefs.h +++ b/components/brave_sync/brave_sync_prefs.h @@ -23,6 +23,8 @@ class Time; namespace brave_sync { +inline constexpr char kManagedBraveSyncUrl[] = "brave_sync.sync_url"; + class Prefs { public: explicit Prefs(PrefService* pref_service); diff --git a/components/sync/service/brave_sync_service_impl_unittest.cc b/components/sync/service/brave_sync_service_impl_unittest.cc index a06ed9e812b9..bba8f0de891f 100644 --- a/components/sync/service/brave_sync_service_impl_unittest.cc +++ b/components/sync/service/brave_sync_service_impl_unittest.cc @@ -32,7 +32,7 @@ #include "components/sync/test/fake_sync_engine_factory.h" #include "components/sync/test/fake_sync_manager.h" #include "components/sync/test/sync_service_impl_bundle.h" -#include "components/sync/test/test_data_type_store_service.h" +#include "components/sync_preferences/testing_pref_service_syncable.h" #include "content/public/test/browser_task_environment.h" #include "testing/gtest/include/gtest/gtest.h" @@ -126,7 +126,7 @@ class BraveSyncServiceImplTest : public testing::Test { SyncPrefs* sync_prefs() { return &sync_prefs_; } - PrefService* pref_service() { + sync_preferences::TestingPrefServiceSyncable* pref_service() { return sync_service_impl_bundle_.pref_service(); } @@ -155,6 +155,50 @@ class BraveSyncServiceImplTest : public testing::Test { std::unique_ptr sync_service_impl_; }; +TEST_F(BraveSyncServiceImplTest, GroupPolicyOverride) { + pref_service()->SetManagedPref(brave_sync::kManagedBraveSyncUrl, + base::Value("https://sync.example.com/v2")); + + OSCryptMocker::SetUp(); + + CreateSyncService(); + + brave_sync_service_impl()->Initialize(); + EXPECT_FALSE(engine()); + + GURL expected_service_url = GURL("https://sync.example.com/v2"); + GURL actual_service_url = + brave_sync_service_impl()->GetSyncServiceUrlForDebugging(); + EXPECT_EQ(expected_service_url, actual_service_url); + + OSCryptMocker::TearDown(); + + pref_service()->SetManagedPref(brave_sync::kManagedBraveSyncUrl, + base::Value("")); +} + +TEST_F(BraveSyncServiceImplTest, GroupPolicyNonHttpsOverride) { + pref_service()->SetManagedPref(brave_sync::kManagedBraveSyncUrl, + base::Value("http://sync.example.com/v2")); + + OSCryptMocker::SetUp(); + + CreateSyncService(); + + brave_sync_service_impl()->Initialize(); + EXPECT_FALSE(engine()); + + GURL expected_service_url = GURL("http://sync.example.com/v2"); + GURL actual_service_url = + brave_sync_service_impl()->GetSyncServiceUrlForDebugging(); + EXPECT_NE(expected_service_url, actual_service_url); + + OSCryptMocker::TearDown(); + + pref_service()->SetManagedPref(brave_sync::kManagedBraveSyncUrl, + base::Value("")); +} + TEST_F(BraveSyncServiceImplTest, ValidPassphrase) { OSCryptMocker::SetUp();