diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index 44766b908c371..5b8b1bf1bc86e 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -3776,6 +3776,11 @@ ChromeContentBrowserClient::GetSystemNetworkContext() { } std::string ChromeContentBrowserClient::GetGeolocationApiKey() { +#if BUILDFLAG(IS_CHROMEOS) + if (ash::features::IsCrosSeparateGeoApiKeyEnabled()) { + return google_apis::GetCrosChromeGeoAPIKey(); + } +#endif return google_apis::GetAPIKey(); } diff --git a/chrome/browser/chrome_content_browser_client_unittest.cc b/chrome/browser/chrome_content_browser_client_unittest.cc index 44341f3bec94a..5304141edc018 100644 --- a/chrome/browser/chrome_content_browser_client_unittest.cc +++ b/chrome/browser/chrome_content_browser_client_unittest.cc @@ -147,6 +147,9 @@ #include "chromeos/components/kiosk/kiosk_utils.h" #include "components/user_manager/scoped_user_manager.h" #include "content/public/test/scoped_web_ui_controller_factory_registration.h" +#include "google_apis/api_key_cache.h" +#include "google_apis/default_api_keys.h" +#include "google_apis/google_api_keys.h" #endif // BUILDFLAG(IS_CHROMEOS) #if BUILDFLAG(IS_WIN) @@ -1378,6 +1381,50 @@ TEST_F(ChromeContentBrowserClientTest, RequestFileAccessDeny) { continuation_callback.GetCallback()); EXPECT_FALSE(continuation_callback.Take().is_allowed()); } + +namespace override_geo_api_keys { + +// We start every test by creating a clean environment for the +// preprocessor defines used in define_baked_in_api_keys-inc.cc +#undef GOOGLE_API_KEY +#undef GOOGLE_API_KEY_CROS_SYSTEM_GEO +#undef GOOGLE_API_KEY_CROS_CHROME_GEO + +// Set Geolocation-specific keys. +#define GOOGLE_API_KEY "bogus_api_key" +#define GOOGLE_API_KEY_CROS_SYSTEM_GEO "bogus_cros_system_geo_api_key" +#define GOOGLE_API_KEY_CROS_CHROME_GEO "bogus_cros_chrome_geo_api_key" + +// This file must be included after the internal files defining official keys. +#include "google_apis/default_api_keys-inc.cc" + +} // namespace override_geo_api_keys + +// Test that when `kCrosSeparateGeoApiKey` feature is enabled, +// Chrome-on-ChromeOS switches to using a separate (ChromeOS-specific) API Key +// for the location requests. +TEST_F(ChromeContentBrowserClientTest, UseCorrectGeoAPIKey) { + auto default_key_values = + override_geo_api_keys::GetDefaultApiKeysFromDefinedValues(); + default_key_values.allow_unset_values = true; + google_apis::ApiKeyCache api_key_cache(default_key_values); + auto scoped_override = + google_apis::SetScopedApiKeyCacheForTesting(&api_key_cache); + + // Check that by default Chrome-on-ChromeOS uses shared API key for + // geolocation requests. + ChromeContentBrowserClient client; + EXPECT_EQ(client.GetGeolocationApiKey(), google_apis::GetAPIKey()); + + // Check that when the `kCrosSeparateGeoApiKey` feature is enabled, + // Chrome-on-ChromeOS uses ChromeOS-specific API key for geolocation. + base::test::ScopedFeatureList scoped_feature_list; + scoped_feature_list.InitAndEnableFeature( + ash::features::kCrosSeparateGeoApiKey); + EXPECT_EQ(client.GetGeolocationApiKey(), + google_apis::GetCrosChromeGeoAPIKey()); +} + #endif // BUILDFLAG(IS_CHROMEOS) class ChromeContentBrowserClientSwitchTest diff --git a/chromeos/ash/components/geolocation/BUILD.gn b/chromeos/ash/components/geolocation/BUILD.gn index 0329928134440..2a8ade785e916 100644 --- a/chromeos/ash/components/geolocation/BUILD.gn +++ b/chromeos/ash/components/geolocation/BUILD.gn @@ -36,6 +36,7 @@ source_set("unit_tests") { "//base/test:test_support", "//chromeos/ash/components/dbus/shill", "//chromeos/ash/components/network:test_support", + "//google_apis", "//net", "//services/network:test_support", "//services/network/public/cpp", diff --git a/chromeos/ash/components/geolocation/simple_geolocation_request.cc b/chromeos/ash/components/geolocation/simple_geolocation_request.cc index 59ab2753497e6..96914803546cd 100644 --- a/chromeos/ash/components/geolocation/simple_geolocation_request.cc +++ b/chromeos/ash/components/geolocation/simple_geolocation_request.cc @@ -11,6 +11,7 @@ #include <string> #include <utility> +#include "ash/constants/ash_features.h" #include "base/functional/bind.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" @@ -153,7 +154,12 @@ GURL GeolocationRequestURL(const GURL& url) { if (url != SimpleGeolocationProvider::DefaultGeolocationProviderURL()) return url; - std::string api_key = google_apis::GetAPIKey(); + std::string api_key; + if (features::IsCrosSeparateGeoApiKeyEnabled()) { + api_key = google_apis::GetCrosSystemGeoAPIKey(); + } else { + api_key = google_apis::GetAPIKey(); + } if (api_key.empty()) return url; @@ -478,6 +484,10 @@ std::string SimpleGeolocationRequest::FormatRequestBodyForTesting() const { return FormatRequestBody(); } +GURL SimpleGeolocationRequest::GetServiceURLForTesting() const { + return request_url_; +} + void SimpleGeolocationRequest::Retry(bool server_error) { base::TimeDelta delay(server_error ? retry_sleep_on_server_error_ : retry_sleep_on_bad_response_); diff --git a/chromeos/ash/components/geolocation/simple_geolocation_request.h b/chromeos/ash/components/geolocation/simple_geolocation_request.h index d36406c273a02..4c1d53972b43a 100644 --- a/chromeos/ash/components/geolocation/simple_geolocation_request.h +++ b/chromeos/ash/components/geolocation/simple_geolocation_request.h @@ -85,6 +85,7 @@ class COMPONENT_EXPORT(CHROMEOS_ASH_COMPONENTS_GEOLOCATION) static void SetTestMonitor(SimpleGeolocationRequestTestMonitor* monitor); std::string FormatRequestBodyForTesting() const; + GURL GetServiceURLForTesting() const; private: void OnSimpleURLLoaderComplete(std::unique_ptr<std::string> response_body); diff --git a/chromeos/ash/components/geolocation/simple_geolocation_unittest.cc b/chromeos/ash/components/geolocation/simple_geolocation_unittest.cc index e51195495d419..3902aae517690 100644 --- a/chromeos/ash/components/geolocation/simple_geolocation_unittest.cc +++ b/chromeos/ash/components/geolocation/simple_geolocation_unittest.cc @@ -6,6 +6,7 @@ #include <memory> +#include "ash/constants/ash_features.h" #include "ash/constants/geolocation_access_level.h" #include "base/containers/contains.h" #include "base/functional/bind.h" @@ -14,6 +15,7 @@ #include "base/run_loop.h" #include "base/strings/string_number_conversions.h" #include "base/strings/stringprintf.h" +#include "base/test/scoped_feature_list.h" #include "base/test/task_environment.h" #include "base/time/time.h" #include "chromeos/ash/components/dbus/shill/shill_manager_client.h" @@ -21,6 +23,9 @@ #include "chromeos/ash/components/geolocation/simple_geolocation_request_test_monitor.h" #include "chromeos/ash/components/network/geolocation_handler.h" #include "chromeos/ash/components/network/network_handler_test_helper.h" +#include "google_apis/api_key_cache.h" +#include "google_apis/default_api_keys.h" +#include "google_apis/google_api_keys.h" #include "net/http/http_response_headers.h" #include "net/http/http_status_code.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h" @@ -117,7 +122,12 @@ class TestGeolocationAPILoaderFactory : public network::TestURLLoaderFactory { } void Intercept(const network::ResourceRequest& request) { - EXPECT_EQ(url_, request.url); + // Drop the query component potentially appended by the + // `SimpleGeolocationRequest` class. + GURL::Replacements replacements; + replacements.ClearQuery(); + EXPECT_EQ(url_.ReplaceComponents(replacements), + request.url.ReplaceComponents(replacements)); SimpleGeolocationProvider* provider = SimpleGeolocationProvider::GetInstance(); @@ -188,14 +198,17 @@ class WirelessTestMonitor : public SimpleGeolocationRequestTestMonitor { void OnRequestCreated(SimpleGeolocationRequest* request) override {} void OnStart(SimpleGeolocationRequest* request) override { last_request_body_ = request->FormatRequestBodyForTesting(); + last_request_url_ = request->GetServiceURLForTesting(); ++requests_count_; } const std::string& last_request_body() const { return last_request_body_; } + GURL last_request_url() const { return last_request_url_; } unsigned int requests_count() const { return requests_count_; } private: std::string last_request_body_; + GURL last_request_url_; unsigned int requests_count_ = 0; }; @@ -399,6 +412,93 @@ TEST_F(SimpleGeolocationTest, SystemGeolocationPermissionDenied) { } } +namespace override_geo_api_keys { + +// We start every test by creating a clean environment for the +// preprocessor defines used in define_baked_in_api_keys-inc.cc +#undef GOOGLE_API_KEY +#undef GOOGLE_API_KEY_CROS_SYSTEM_GEO +#undef GOOGLE_API_KEY_CROS_CHROME_GEO + +// Set Geolocation-specific keys. +#define GOOGLE_API_KEY "bogus_api_key" +#define GOOGLE_API_KEY_CROS_SYSTEM_GEO "bogus_cros_system_geo_api_key" +#define GOOGLE_API_KEY_CROS_CHROME_GEO "bogus_cros_chrome_geo_api_key" + +// This file must be included after the internal files defining official keys. +#include "google_apis/default_api_keys-inc.cc" + +} // namespace override_geo_api_keys + +class SimpleGeolocationAPIKeyTest : public SimpleGeolocationTest, + public testing::WithParamInterface<bool> { + public: + SimpleGeolocationAPIKeyTest() : is_separate_api_keys_enabled_(GetParam()) { + if (is_separate_api_keys_enabled_) { + feature_list_.InitAndEnableFeature(features::kCrosSeparateGeoApiKey); + } else { + feature_list_.InitAndDisableFeature(features::kCrosSeparateGeoApiKey); + } + } + + void SetUp() override { + SimpleGeolocationTest::SetUp(); + + // Set URL to the production value to let the `SimpleGeolocationRequest` + // extend it with the API keys. + SimpleGeolocationProvider::GetInstance() + ->SetGeolocationProviderUrlForTesting( + SimpleGeolocationProvider::DefaultGeolocationProviderURL() + .spec() + .c_str()); + url_factory_.Configure( + SimpleGeolocationProvider::DefaultGeolocationProviderURL(), + net::HTTP_OK, kSimpleResponseBody, 0); + } + + const bool is_separate_api_keys_enabled_; + + private: + base::test::ScopedFeatureList feature_list_; +}; + +TEST_P(SimpleGeolocationAPIKeyTest, TestCorrectAPIKeysAreUsed) { + GeolocationReceiver receiver; + WirelessTestMonitor requests_monitor; + SimpleGeolocationRequest::SetTestMonitor(&requests_monitor); + + // Override the `ApiKeyCache` with the bogus values from above. + auto default_key_values = + override_geo_api_keys::GetDefaultApiKeysFromDefinedValues(); + default_key_values.allow_unset_values = true; + google_apis::ApiKeyCache api_key_cache(default_key_values); + auto scoped_key_cache_override = + google_apis::SetScopedApiKeyCacheForTesting(&api_key_cache); + + // Request geolocation and wait for the response. + EnableGeolocationUsage(); + SimpleGeolocationProvider::GetInstance()->RequestGeolocation( + base::Seconds(1), false, false, + base::BindOnce(&GeolocationReceiver::OnRequestDone, + base::Unretained(&receiver)), + SimpleGeolocationProvider::ClientId::kForTesting); + receiver.WaitUntilRequestDone(); + + // Check that the appropriate API key was used depending on the + // `CrosSeparateGeoApiKey` feature status. + const GURL request_url = requests_monitor.last_request_url(); + ASSERT_TRUE(request_url.has_query()); + EXPECT_EQ(is_separate_api_keys_enabled_, + request_url.query().find(GOOGLE_API_KEY_CROS_SYSTEM_GEO) != + std::string::npos); + EXPECT_EQ(is_separate_api_keys_enabled_, + request_url.query().find(GOOGLE_API_KEY) == std::string::npos); + EXPECT_EQ(1U, url_factory_.attempts()); +} + +// GetParam() - `ash::features::kCrosSeparateGeoApiKey` feature state. +INSTANTIATE_TEST_SUITE_P(All, SimpleGeolocationAPIKeyTest, testing::Bool()); + // Test sending of WiFi Access points and Cell Towers. // (This is mostly derived from GeolocationHandlerTest.) class SimpleGeolocationWirelessTest : public SimpleGeolocationTestBase, diff --git a/google_apis/api_key_cache.cc b/google_apis/api_key_cache.cc index 55c6028248b49..9f5a5a13b3f9f 100644 --- a/google_apis/api_key_cache.cc +++ b/google_apis/api_key_cache.cc @@ -180,6 +180,21 @@ ApiKeyCache::ApiKeyCache(const DefaultApiKeys& default_api_keys) { environment.get(), command_line, gaia_config, default_api_keys.allow_override_via_environment, default_api_keys.allow_unset_values); + + api_key_cros_system_geo_ = CalculateKeyValue( + default_api_keys.google_api_key_cros_system_geo_, + STRINGIZE_NO_EXPANSION(GOOGLE_API_KEY_CROS_SYSTEM_GEO), nullptr, + std::string(), environment.get(), command_line, gaia_config, + default_api_keys.allow_override_via_environment, + default_api_keys.allow_unset_values); + + api_key_cros_chrome_geo_ = CalculateKeyValue( + default_api_keys.google_api_key_cros_chrome_geo_, + STRINGIZE_NO_EXPANSION(GOOGLE_API_KEY_CROS_CHROME_GEO), nullptr, + std::string(), environment.get(), command_line, gaia_config, + default_api_keys.allow_override_via_environment, + default_api_keys.allow_unset_values); + #endif metrics_key_ = CalculateKeyValue( diff --git a/google_apis/api_key_cache.h b/google_apis/api_key_cache.h index a3a64745e4c7b..cb7a864de21f4 100644 --- a/google_apis/api_key_cache.h +++ b/google_apis/api_key_cache.h @@ -38,6 +38,12 @@ class COMPONENT_EXPORT(GOOGLE_APIS) ApiKeyCache { const std::string& api_key_read_aloud() const { return api_key_read_aloud_; } const std::string& api_key_fresnel() const { return api_key_fresnel_; } const std::string& api_key_boca() const { return api_key_boca_; } + const std::string& api_key_cros_system_geo() const { + return api_key_cros_system_geo_; + } + const std::string& api_key_cros_chrome_geo() const { + return api_key_cros_chrome_geo_; + } #endif const std::string& metrics_key() const { return metrics_key_; } @@ -67,6 +73,8 @@ class COMPONENT_EXPORT(GOOGLE_APIS) ApiKeyCache { std::string api_key_read_aloud_; std::string api_key_fresnel_; std::string api_key_boca_; + std::string api_key_cros_system_geo_; + std::string api_key_cros_chrome_geo_; #endif std::string metrics_key_; diff --git a/google_apis/default_api_keys-inc.cc b/google_apis/default_api_keys-inc.cc index c4630ac6cf388..6b8b027ee401d 100644 --- a/google_apis/default_api_keys-inc.cc +++ b/google_apis/default_api_keys-inc.cc @@ -92,6 +92,19 @@ #if !defined(GOOGLE_API_KEY_BOCA) #define GOOGLE_API_KEY_BOCA google_apis::DefaultApiKeys::kUnsetApiToken #endif + +// API key for ChromeOS system services to resolve location. +#if !defined(GOOGLE_API_KEY_CROS_SYSTEM_GEO) +#define GOOGLE_API_KEY_CROS_SYSTEM_GEO \ + google_apis::DefaultApiKeys::kUnsetApiToken +#endif + +// API key for ChromeOS Chrome to resolve location. +#if !defined(GOOGLE_API_KEY_CROS_CHROME_GEO) +#define GOOGLE_API_KEY_CROS_CHROME_GEO \ + google_apis::DefaultApiKeys::kUnsetApiToken +#endif + #endif // BUILDFLAG(IS_CHROMEOS) // These are used as shortcuts for developers and users providing @@ -127,6 +140,8 @@ constexpr ::google_apis::DefaultApiKeys GetDefaultApiKeysFromDefinedValues() { .google_api_key_read_aloud = GOOGLE_API_KEY_READ_ALOUD, .google_api_key_fresnel = GOOGLE_API_KEY_FRESNEL, .google_api_key_boca = GOOGLE_API_KEY_BOCA, + .google_api_key_cros_system_geo_ = GOOGLE_API_KEY_CROS_SYSTEM_GEO, + .google_api_key_cros_chrome_geo_ = GOOGLE_API_KEY_CROS_CHROME_GEO, #endif .google_client_id_main = GOOGLE_CLIENT_ID_MAIN, .google_client_secret_main = GOOGLE_CLIENT_SECRET_MAIN, diff --git a/google_apis/default_api_keys.h b/google_apis/default_api_keys.h index 5010913dc8447..5f9e8cc19bf84 100644 --- a/google_apis/default_api_keys.h +++ b/google_apis/default_api_keys.h @@ -34,6 +34,8 @@ struct DefaultApiKeys { const char* google_api_key_read_aloud; const char* google_api_key_fresnel; const char* google_api_key_boca; + const char* google_api_key_cros_system_geo_; + const char* google_api_key_cros_chrome_geo_; #endif const char* google_client_id_main; diff --git a/google_apis/google_api_keys.cc b/google_apis/google_api_keys.cc index 84cc1ca534093..6e98c9aed842b 100644 --- a/google_apis/google_api_keys.cc +++ b/google_apis/google_api_keys.cc @@ -114,6 +114,13 @@ const std::string& GetFresnelAPIKey() { const std::string& GetBocaAPIKey() { return GetApiKeyCacheInstance().api_key_boca(); } + +const std::string& GetCrosSystemGeoAPIKey() { + return GetApiKeyCacheInstance().api_key_cros_system_geo(); +} +const std::string& GetCrosChromeGeoAPIKey() { + return GetApiKeyCacheInstance().api_key_cros_chrome_geo(); +} #endif const std::string& GetMetricsKey() { diff --git a/google_apis/google_api_keys.h b/google_apis/google_api_keys.h index ada1e52cdf2e3..0f10fd01c9b7c 100644 --- a/google_apis/google_api_keys.h +++ b/google_apis/google_api_keys.h @@ -113,6 +113,14 @@ COMPONENT_EXPORT(GOOGLE_APIS) const std::string& GetFresnelAPIKey(); // Retrieves the Boca API Key. COMPONENT_EXPORT(GOOGLE_APIS) const std::string& GetBocaAPIKey(); + +// Retrieves the ChromeOS-specific Geolocation API Key used by the System +// Services. +COMPONENT_EXPORT(GOOGLE_APIS) const std::string& GetCrosSystemGeoAPIKey(); +// Retrieves the ChromeOS-specific Geolocation API Key used by the Chrome +// browser. +COMPONENT_EXPORT(GOOGLE_APIS) const std::string& GetCrosChromeGeoAPIKey(); + #endif // Retrieves the key used to sign metrics (UMA/UKM) uploads.