The project ID isn't needed, since it's actually ok to expose the API key. We...

The project ID isn't needed, since it's actually ok to expose the API key. We should offer the option of putting it in the manifest though.

BUG=454887

Review URL: https://codereview.chromium.org/901213003

Cr-Commit-Position: refs/heads/master@{#315413}

chrome/browser/extensions/api/copresence/copresence_api.cc

@@ -185,19 +185,24 @@ const std::string CopresenceService::GetPlatformVersionString() const {
 
 const std::string
 CopresenceService::GetAPIKey(const std::string& app_id) const {
-  // This won't be const if we use map[]
+  // Check first if the app has set its key via the API.
   const auto& key = api_keys_by_app_.find(app_id);
-  return key == api_keys_by_app_.end() ? std::string() : key->second;
-}
+  if (key != api_keys_by_app_.end())
+    return key->second;
 
-const std::string
-CopresenceService::GetProjectId(const std::string& app_id) const {
+  // If no key was found, look in the manifest.
+  if (!app_id.empty()) {
     const Extension* extension = ExtensionRegistry::Get(browser_context_)
         ->GetExtensionById(app_id, ExtensionRegistry::ENABLED);
     DCHECK(extension) << "Invalid extension ID";
-  CopresenceManifestData* manifest_data = static_cast<CopresenceManifestData*>(
+    CopresenceManifestData* manifest_data =
+        static_cast<CopresenceManifestData*>(
             extension->GetManifestData(manifest_keys::kCopresence));
-  return manifest_data ? manifest_data->project_id : std::string();
+    if (manifest_data)
+      return manifest_data->api_key;
+  }
+
+  return std::string();
 }
 
 copresence::WhispernetClient* CopresenceService::GetWhispernetClient() {
@@ -282,6 +287,9 @@ ExtensionFunction::ResponseAction CopresenceSetApiKeyFunction::Run() {
       api::copresence::SetApiKey::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
 
+  LOG(WARNING) << "copresence.setApiKey() is deprecated. "
+               << "Put the key in the manifest at copresence.api_key instead.";
+
   // The api key may be set to empty, to clear it.
   CopresenceService::GetFactoryInstance()->Get(browser_context())
       ->set_api_key(extension_id(), params->api_key);

chrome/browser/extensions/api/copresence/copresence_api.h

@@ -86,7 +86,6 @@ class CopresenceService final : public BrowserContextKeyedAPI,
   net::URLRequestContextGetter* GetRequestContext() const override;
   const std::string GetPlatformVersionString() const override;
   const std::string GetAPIKey(const std::string& app_id) const override;
-  const std::string GetProjectId(const std::string& app_id) const override;
   copresence::WhispernetClient* GetWhispernetClient() override;
   gcm::GCMDriver* GetGCMDriver() override;
   const std::string GetDeviceId(bool authenticated) override;
@@ -127,6 +126,7 @@ class CopresenceExecuteFunction : public ChromeUIThreadExtensionFunction {
   void SendResult(copresence::CopresenceStatus status);
 };
 
+// TODO(ckehoe): Remove this function.
 class CopresenceSetApiKeyFunction : public ChromeUIThreadExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("copresence.setApiKey", COPRESENCE_SETAPIKEY);

chrome/common/extensions/manifest_handlers/copresence_manifest.cc

@@ -30,10 +30,10 @@ bool CopresenceManifestHandler::Parse(Extension* extension,
   }
 
   scoped_ptr<CopresenceManifestData> manifest_data(new CopresenceManifestData);
-  if (!copresence_config->GetString(manifest_values::kProjectId,
-                                    &manifest_data->project_id) ||
-      manifest_data->project_id.empty()) {
-    *error = base::ASCIIToUTF16(manifest_errors::kInvalidCopresenceProjectId);
+  if (!copresence_config->GetString(manifest_values::kApiKey,
+                                    &manifest_data->api_key) ||
+      manifest_data->api_key.empty()) {
+    *error = base::ASCIIToUTF16(manifest_errors::kInvalidCopresenceApiKey);
     return false;
   }
 

chrome/common/extensions/manifest_handlers/copresence_manifest.h

@@ -34,7 +34,7 @@ struct CopresenceManifestData final : public Extension::ManifestData {
   CopresenceManifestData();
   ~CopresenceManifestData() override;
 
-  std::string project_id;
+  std::string api_key;
 };
 
 }  // namespace extensions

components/copresence/proto/data.proto

@@ -9,7 +9,6 @@ message ClientVersion {
   optional string version_name = 2;
   optional int64 version_code = 3;
   optional string certificate_fingerprint = 4;
-  optional string project_id = 5;
 }
 message Status {
   optional StatusCode code = 1;

components/copresence/public/copresence_delegate.h

@@ -47,11 +47,8 @@ class CopresenceDelegate {
 
   virtual const std::string GetPlatformVersionString() const = 0;
 
-  // This is deprecated. Clients should pass in the project ID instead.
   virtual const std::string GetAPIKey(const std::string& app_id) const = 0;
 
-  virtual const std::string GetProjectId(const std::string& app_id) const = 0;
-
   // Thw WhispernetClient must outlive the CopresenceManager.
   virtual WhispernetClient* GetWhispernetClient() = 0;
 

components/copresence/rpc/rpc_handler.cc

@@ -139,18 +139,10 @@ scoped_ptr<DeviceState> GetDeviceCapabilities(const ReportRequest& request) {
 // an int64 version. We should probably change the version proto
 // to handle a more detailed version.
 ClientVersion* CreateVersion(const std::string& client,
-                             const std::string& version_name,
-                             const std::string& project_id) {
+                             const std::string& version_name) {
   ClientVersion* version = new ClientVersion;
-
   version->set_client(client);
   version->set_version_name(version_name);
-
-  if (!project_id.empty()) {
-    DVLOG(3) << "Using project ID " << project_id;
-    version->set_project_id(project_id);
-  }
-
   return version;
 }
 
@@ -213,6 +205,15 @@ void RpcHandler::SendReportRequest(scoped_ptr<ReportRequest> request,
                                    const StatusCallback& status_callback) {
   DCHECK(request.get());
 
+  // Check that the app, if any, has some kind of authentication token.
+  // Don't allow it to piggyback on Chrome's credentials.
+  if (!app_id.empty() && delegate_->GetAPIKey(app_id).empty() &&
+      auth_token.empty()) {
+    LOG(ERROR) << "App " << app_id << " has no API key or auth token";
+    status_callback.Run(FAIL);
+    return;
+  }
+
   // Store just one auth token since we should have only one account
   // per instance of the copresence component.
   // TODO(ckehoe): We may eventually need to support multiple auth tokens.
@@ -572,13 +573,9 @@ RequestHeader* RpcHandler::CreateRequestHeader(
   RequestHeader* header = new RequestHeader;
 
   header->set_allocated_framework_version(CreateVersion(
-      "Chrome", delegate_->GetPlatformVersionString(), std::string()));
-  if (!app_id.empty()) {
-    LOG_IF(WARNING, delegate_->GetProjectId(app_id).empty())
-        << "No copresence project ID available";
-    header->set_allocated_client_version(CreateVersion(
-        app_id, std::string(), delegate_->GetProjectId(app_id)));
-  }
+      "Chrome", delegate_->GetPlatformVersionString()));
+  if (!app_id.empty())
+    header->set_allocated_client_version(CreateVersion(app_id, std::string()));
   header->set_current_time_millis(base::Time::Now().ToJsTime());
   if (!device_id.empty())
     header->set_registered_device_id(device_id);
@@ -604,7 +601,7 @@ void RpcHandler::SendServerRequest(
     DCHECK(!auth_token_.empty());
   server_post_callback_.Run(delegate_->GetRequestContext(),
                             rpc_name,
-                            delegate_->GetAPIKey(app_id),  // Deprecated
+                            delegate_->GetAPIKey(app_id),
                             authenticated ? auth_token_ : std::string(),
                             make_scoped_ptr<MessageLite>(request.release()),
                             response_handler);
@@ -612,7 +609,7 @@ void RpcHandler::SendServerRequest(
 
 void RpcHandler::SendHttpPost(net::URLRequestContextGetter* url_context_getter,
                               const std::string& rpc_name,
-                              const std::string& api_key,  // Deprecated
+                              const std::string& api_key,
                               const std::string& auth_token,
                               scoped_ptr<MessageLite> request_proto,
                               const PostCleanupCallback& callback) {

components/copresence/rpc/rpc_handler.h

@@ -49,7 +49,7 @@ class RpcHandler {
   // Arguments:
   // URLRequestContextGetter: Context for the HTTP POST request.
   // string: Name of the rpc to invoke. URL format: server.google.com/rpc_name
-  // string: The API key to pass in the request. Deprecated.
+  // string: The API key to pass in the request.
   // string: The auth token to pass with the request.
   // MessageLite: Contents of POST request to be sent. This needs to be
   //     a (scoped) pointer to ease handling of the abstract MessageLite class.
@@ -157,7 +157,7 @@ class RpcHandler {
   // to contact the server, but it can be overridden for testing.
   void SendHttpPost(net::URLRequestContextGetter* url_context_getter,
                     const std::string& rpc_name,
-                    const std::string& api_key,  // Deprecated
+                    const std::string& api_key,
                     const std::string& auth_token,
                     scoped_ptr<google::protobuf::MessageLite> request_proto,
                     const PostCleanupCallback& callback);

components/copresence/rpc/rpc_handler_unittest.cc

@@ -81,10 +81,6 @@ class RpcHandlerTest : public testing::Test, public CopresenceDelegate {
     return app_id + " API Key";
   }
 
-  const std::string GetProjectId(const std::string& app_id) const override {
-    return app_id + " Project ID";
-  }
-
   WhispernetClient* GetWhispernetClient() override {
     return whispernet_client_.get();
   }
@@ -290,8 +286,6 @@ TEST_F(RpcHandlerTest, CreateRequestHeader) {
             report->header().framework_version().version_name());
   EXPECT_EQ("CreateRequestHeader App",
             report->header().client_version().client());
-  EXPECT_EQ("CreateRequestHeader App Project ID",
-            report->header().client_version().project_id());
   EXPECT_EQ("CreateRequestHeader Device ID",
             report->header().registered_device_id());
   EXPECT_EQ(CHROME_PLATFORM_TYPE,

extensions/common/manifest_constants.cc

@@ -185,6 +185,7 @@ const char kWhitelist[] = "whitelist";
 
 namespace manifest_values {
 
+const char kApiKey[] = "api_key";
 const char kBrowserActionCommandEvent[] = "_execute_browser_action";
 const char kIncognitoSplit[] = "split";
 const char kIncognitoSpanning[] = "spanning";
@@ -225,7 +226,6 @@ const char kRunAtDocumentIdle[] = "document_idle";
 const char kPageActionCommandEvent[] = "_execute_page_action";
 const char kPageActionTypeTab[] = "tab";
 const char kPageActionTypePermanent[] = "permanent";
-const char kProjectId[] = "project_id";
 const char kLaunchContainerPanel[] = "panel";
 const char kLaunchContainerTab[] = "tab";
 const char kLaunchContainerWindow[] = "window";
@@ -321,8 +321,8 @@ const char kInvalidContentScriptsList[] =
 const char kInvalidContentSecurityPolicy[] =
     "Invalid value for 'content_security_policy'.";
 const char kInvalidCopresenceConfig[] = "Invalid value for 'copresence'.";
-const char kInvalidCopresenceProjectId[] =
-    "copresence.project_id must not be empty.";
+const char kInvalidCopresenceApiKey[] =
+    "copresence.api_key must not be empty.";
 const char kInvalidCSPInsecureValue[] =
     "Ignored insecure CSP value \"*\" in directive '*'.";
 const char kInvalidCSPMissingSecureSrc[] =

extensions/common/manifest_constants.h

@@ -187,6 +187,7 @@ extern const char kWhitelist[];
 // Some values expected in manifests.
 namespace manifest_values {
 
+extern const char kApiKey[];
 extern const char kBrowserActionCommandEvent[];
 extern const char kIncognitoSplit[];
 extern const char kIncognitoSpanning[];
@@ -227,7 +228,6 @@ extern const char kLaunchContainerWindow[];
 extern const char kPageActionCommandEvent[];
 extern const char kPageActionTypePermanent[];
 extern const char kPageActionTypeTab[];
-extern const char kProjectId[];
 extern const char kRunAtDocumentEnd[];
 extern const char kRunAtDocumentIdle[];
 extern const char kRunAtDocumentStart[];
@@ -282,7 +282,7 @@ extern const char kInvalidContentScript[];
 extern const char kInvalidContentScriptsList[];
 extern const char kInvalidContentSecurityPolicy[];
 extern const char kInvalidCopresenceConfig[];
-extern const char kInvalidCopresenceProjectId[];
+extern const char kInvalidCopresenceApiKey[];
 extern const char kInvalidCSPInsecureValue[];
 extern const char kInvalidCSPMissingSecureSrc[];
 extern const char kInvalidCss[];