Consistent interface to add the Google API key to a request

As part of crrev/c/5734315 I notice there is a lot of inconsistency
and duplication in how we set API keys on requests:
* Some places set a URL parameter, some set the HTTP header (both are
valid per https://cloud.google.com/apis/docs/system-parameters)
* The header name is hard-coded in many places
* Call sites don't all handle the different stable/non-stable channel
keys

This CL adds an API to improve on the issues above.

Bug: 355544759
Change-Id: Iea4616bcbdcb10ba7d9e2dcd8ffefddd2df813be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5730337
Auto-Submit: James Lee <ljjlee@google.com>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Nohemi Fernandez <fernandex@chromium.org>
Commit-Queue: James Lee <ljjlee@google.com>
Cr-Commit-Position: refs/heads/main@{#1334202}

google_apis/BUILD.gn

@@ -179,6 +179,7 @@ component("google_apis") {
 
   deps = [
     ":proto",
+    "//base/version_info:channel",
     "//build:branding_buildflags",
     "//build:chromeos_buildflags",
     "//crypto",
@@ -276,6 +277,7 @@ test("google_apis_unittests") {
     ":test_support",
     "//base",
     "//base/test:test_support",
+    "//base/version_info:channel",
     "//build:branding_buildflags",
     "//build:chromeos_buildflags",
     "//google_apis/calendar:calendar_unittests",

google_apis/common/BUILD.gn

@@ -9,6 +9,8 @@ static_library("common") {
   sources = [
     "api_error_codes.cc",
     "api_error_codes.h",
+    "api_key_request_util.cc",
+    "api_key_request_util.h",
     "auth_service.cc",
     "auth_service.h",
     "auth_service_interface.h",
@@ -59,6 +61,7 @@ source_set("common_unittests") {
   testonly = true
 
   sources = [
+    "api_key_request_util_unittest.cc",
     "base_requests_unittest.cc",
     "request_sender_unittest.cc",
     "time_util_unittest.cc",
@@ -69,6 +72,7 @@ source_set("common_unittests") {
     ":test_support",
     "//base",
     "//base/test:test_support",
+    "//base/version_info:channel",
     "//services/network:test_support",
     "//testing/gmock",
     "//testing/gtest",

google_apis/common/api_key_request_util.cc

@@ -0,0 +1,40 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "google_apis/common/api_key_request_util.h"
+
+#include "base/version_info/channel.h"
+#include "google_apis/google_api_keys.h"
+#include "services/network/public/cpp/resource_request.h"
+
+namespace {
+
+// See https://cloud.google.com/apis/docs/system-parameters
+constexpr char kApiKeyHeaderName[] = "X-Goog-Api-Key";
+
+}  // namespace
+
+namespace google_apis {
+
+void AddDefaultAPIKeyToRequest(network::ResourceRequest& request,
+                               version_info::Channel channel) {
+  AddAPIKeyToRequest(request, GetAPIKey(channel));
+}
+
+void AddAPIKeyToRequest(network::ResourceRequest& request,
+                        const std::string& api_key) {
+  // TODO(b/355544759): check that no Authorization header is present.
+  // TODO(b/355544759): check that the API isn't present as a URL query param.
+  // Don't use CHECK for validation, to make migrating to this API less risky.
+  if (api_key.empty()) {
+    DLOG(FATAL) << "API key cannot be empty.";
+    return;
+  }
+  DLOG_ASSERT(!request.headers.HasHeader(kApiKeyHeaderName))
+      << "API key already present on the request.";
+
+  request.headers.SetHeader(kApiKeyHeaderName, api_key);
+}
+
+}  // namespace google_apis

google_apis/common/api_key_request_util.h

@@ -0,0 +1,35 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef GOOGLE_APIS_COMMON_API_KEY_REQUEST_UTIL_H_
+#define GOOGLE_APIS_COMMON_API_KEY_REQUEST_UTIL_H_
+
+#include <string>
+
+namespace network {
+struct ResourceRequest;
+}
+
+namespace version_info {
+enum class Channel;
+}
+
+namespace google_apis {
+
+// Adds the default API key to the request.
+//
+// This uses the default API key as returned by `GetAPIKey(channel)`. Use this
+// method if your request does not have separate per-service or overridable key.
+void AddDefaultAPIKeyToRequest(network::ResourceRequest& request,
+                               version_info::Channel channel);
+
+// Adds a specific API key to the request.
+//
+// Use this method if your request has separate per-service or overridable key.
+void AddAPIKeyToRequest(network::ResourceRequest& request,
+                        const std::string& api_key);
+
+}  // namespace google_apis
+
+#endif  // GOOGLE_APIS_COMMON_API_KEY_REQUEST_UTIL_H_

google_apis/common/api_key_request_util_unittest.cc

@@ -0,0 +1,63 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "google_apis/common/api_key_request_util.h"
+
+#include "base/test/gtest_util.h"
+#include "base/version_info/channel.h"
+#include "google_apis/google_api_keys.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace google_apis {
+namespace {
+
+constexpr char kApiKeyHeaderName[] = "X-Goog-Api-Key";
+
+class APIKeyRequestUtilTest : public testing::Test {
+ public:
+  APIKeyRequestUtilTest() = default;
+  ~APIKeyRequestUtilTest() override = default;
+};
+
+TEST_F(APIKeyRequestUtilTest, AddDefaultAPIKeyToRequest) {
+  network::ResourceRequest request;
+
+  AddDefaultAPIKeyToRequest(request, version_info::Channel::STABLE);
+
+  ASSERT_TRUE(request.headers.HasHeader(kApiKeyHeaderName));
+  ASSERT_EQ(request.headers.GetHeader(kApiKeyHeaderName),
+            GetAPIKey(version_info::Channel::STABLE));
+}
+
+TEST_F(APIKeyRequestUtilTest, AddAPIKeyToRequest) {
+  network::ResourceRequest request;
+
+  AddAPIKeyToRequest(request, "test_api_key");
+
+  ASSERT_TRUE(request.headers.HasHeader(kApiKeyHeaderName));
+  ASSERT_EQ(request.headers.GetHeader(kApiKeyHeaderName), "test_api_key");
+}
+
+#if DCHECK_IS_ON()
+TEST_F(APIKeyRequestUtilTest, AddAPIKeyToRequest_EmptyKey) {
+  network::ResourceRequest request;
+
+  // This is a misuse of the API, enforced by a DLOG(FATAL) in tests.
+  EXPECT_DEATH_IF_SUPPORTED(AddAPIKeyToRequest(request, ""),
+                            "API key cannot be empty.");
+}
+
+TEST_F(APIKeyRequestUtilTest, AddAPIKeyToRequest_ExistingHeader) {
+  network::ResourceRequest request;
+  request.headers.SetHeader(kApiKeyHeaderName, "existing_api_key");
+
+  // This is a misuse of the API, enforced by a DLOG(FATAL) in tests.
+  EXPECT_DEATH_IF_SUPPORTED(AddAPIKeyToRequest(request, "test_api_key"),
+                            "API key already present on the request.");
+}
+#endif  // DCHECK_IS_ON()
+
+}  // namespace
+}  // namespace google_apis

google_apis/google_api_keys-inc.cc

@@ -22,6 +22,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/strings/stringize_macros.h"
+#include "base/version_info/channel.h"
 #include "build/branding_buildflags.h"
 #include "build/chromeos_buildflags.h"
 #include "google_apis/buildflags.h"
@@ -391,6 +392,11 @@ bool HasAPIKeyConfigured() {
   return GetAPIKey() != DUMMY_API_TOKEN;
 }
 
+std::string GetAPIKey(::version_info::Channel channel) {
+  return channel == ::version_info::Channel::STABLE ? GetAPIKey()
+                                                    : GetNonStableAPIKey();
+}
+
 std::string GetAPIKey() {
   return g_api_key_cache.Get().api_key();
 }

google_apis/google_api_keys.h

@@ -14,6 +14,10 @@
 #include "build/chromeos_buildflags.h"
 #include "google_apis/buildflags.h"
 
+namespace version_info {
+enum class Channel;
+}
+
 // These functions enable you to retrieve keys to use for Google APIs
 // such as Translate and Safe Browsing.
 //
@@ -72,9 +76,23 @@ COMPONENT_EXPORT(GOOGLE_APIS) bool HasAPIKeyConfigured();
 //
 // Note that the key should be escaped for the context you use it in,
 // e.g. URL-escaped if you use it in a URL.
+//
+// If you want to attach the key to a network request, consider using
+// `AddDefaultAPIKeyToRequest()` rather than calling this method and manually
+// adding the key.
+COMPONENT_EXPORT(GOOGLE_APIS)
+std::string GetAPIKey(version_info::Channel channel);
+
+// Retrieves the API key, for the stable channel.
+//
+// DEPRECATED: Use `GetAPIKey(channel)` to get the right key for your
+// distribution channel instead of calling this function directly.
 COMPONENT_EXPORT(GOOGLE_APIS) std::string GetAPIKey();
 
 // Non-stable channels may have a different Google API key.
+//
+// DEPRECATED: Use `GetAPIKey(channel)` to get the right key for your
+// distribution channel instead of calling this function directly.
 COMPONENT_EXPORT(GOOGLE_APIS) std::string GetNonStableAPIKey();
 
 // Retrieves the Chrome Remote Desktop API key.

google_apis/google_api_keys_mac_unittest.mm

@@ -38,6 +38,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/strings/stringize_macros.h"
+#include "base/version_info/channel.h"
 #include "google_apis/gaia/gaia_config.h"
 #include "google_apis/google_api_keys_mac.h"
 #include "google_apis/google_api_keys_utils.h"

google_apis/google_api_keys_unittest.cc

@@ -46,10 +46,12 @@
 #include <stddef.h>
 
 #include <string>
+
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/strings/stringize_macros.h"
+#include "base/version_info/channel.h"
 
 #if BUILDFLAG(IS_APPLE)
 #include "google_apis/google_api_keys_mac.h"
@@ -609,6 +611,7 @@ TEST_F(GoogleAPIKeysTest, OverrideAllKeysUsingSetters) {
   EXPECT_TRUE(testcase::HasAPIKeyConfigured());
   EXPECT_TRUE(testcase::HasOAuthClientConfigured());
 
+  EXPECT_EQ(api_key, testcase::GetAPIKey(::version_info::Channel::STABLE));
   EXPECT_EQ(api_key, testcase::GetAPIKey());
 
   EXPECT_EQ(id_main, testcase::GetOAuth2ClientID(testcase::CLIENT_MAIN));
@@ -670,6 +673,8 @@ TEST_F(GoogleAPIKeysTest, OverrideAllKeysUsingConfig) {
   EXPECT_TRUE(testcase::HasAPIKeyConfigured());
   EXPECT_TRUE(testcase::HasOAuthClientConfigured());
 
+  EXPECT_EQ("config-API_KEY",
+            testcase::GetAPIKey(::version_info::Channel::STABLE));
   EXPECT_EQ("config-API_KEY", testcase::GetAPIKey());
   EXPECT_EQ("config-ID_MAIN",
             testcase::GetOAuth2ClientID(testcase::CLIENT_MAIN));

google_apis/google_api_keys_utils.h

@@ -12,7 +12,7 @@
 namespace google_apis {
 
 // Returns an API key that can be used to override the API key and that is
-// configured via and experimental feature.
+// configured via an experimental feature.
 COMPONENT_EXPORT(GOOGLE_APIS)
 std::string GetAPIKeyOverrideViaFeature();