Add Digital Asset Links verification for postMessage API

chrome/browser/android/digital_asset_links/digital_asset_links_handler.cc

+// Copyright (c) 2017 The Chromium Authors. All rights reserved.

[nyquist, 2017/04/19 06:35:56]

Nit: Remove (c) for all these files

[Yusuf, 2017/04/26 00:51:36]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/android/digital_asset_links/digital_asset_links_handler.h"
+
+#include "base/json/json_reader.h"
+#include "base/logging.h"
+#include "base/strings/string_number_conversions.h"

[nyquist, 2017/04/19 06:35:57]

Nit: Which conversion is this needed for again?

[Yusuf, 2017/04/26 00:51:36]

You know... That one..

Removed.

+#include "base/strings/stringprintf.h"
+#include "base/values.h"
+#include "net/base/load_flags.h"
+#include "net/base/url_util.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/http/http_util.h"
+#include "net/url_request/url_request_status.h"
+
+namespace {
+const char kDigitalAssetLinksBaseURL[] =
+    "https://digitalassetlinks.googleapis.com";
+const char kDigitalAssetLinksCheckAPI[] = "/v1/assetlinks:check?";
+const char kTargetOriginParam[] = "source.web.site";
+const char kSourcePackageNameParam[] = "target.androidApp.packageName";
+const char kSourceFingerprintParam[] =
+    "target.androidApp.certificate.sha256Fingerprint";
+const char kRelationshipParam[] = "relation";
+
+GURL GetUrlForCheckingRelationship(const std::string& web_domain,
+                                   const std::string& package_name,
+                                   const std::string& fingerprint,
+                                   const std::string& relationship) {
+  GURL request_url =
+      GURL(kDigitalAssetLinksBaseURL).Resolve(kDigitalAssetLinksCheckAPI);
+  request_url =
+      net::AppendQueryParameter(request_url, kTargetOriginParam, web_domain);
+  request_url = net::AppendQueryParameter(request_url, kSourcePackageNameParam,
+                                          package_name);
+  request_url = net::AppendQueryParameter(request_url, kSourceFingerprintParam,
+                                          fingerprint);
+  request_url =
+      net::AppendQueryParameter(request_url, kRelationshipParam, relationship);
+  DCHECK(request_url.is_valid());
+  return request_url;
+}
+}  // namespace
+
+namespace digital_asset_links {
+
+const char kDigitalAssetLinksCheckResponseKeyLinked[] = "linked";
+
+DigitalAssetLinksHandler::DigitalAssetLinksHandler(
+    const scoped_refptr<net::URLRequestContextGetter>& request_context)
+    : request_context_(request_context) {}
+
+DigitalAssetLinksHandler::~DigitalAssetLinksHandler() {}

[nyquist, 2017/04/19 06:35:57]

Nit: = default?

[Yusuf, 2017/04/26 00:51:36]

Done.

[nyquist, 2017/04/27 04:38:25]

Done. You keep using that word. I don't think it means what you think it means.
;-)

+
+void DigitalAssetLinksHandler::OnURLFetchComplete(
+    const net::URLFetcher* source) {
+  if (!source->GetStatus().is_success() ||
+      source->GetResponseCode() != net::HTTP_OK) {
+    LOG(WARNING) << base::StringPrintf(
+        "Digital Asset Links endpoint responded with code %d.",
+        source->GetResponseCode());
+    callback_.Run(nullptr);
+    return;
+  }
+
+  std::string response_body;
+  source->GetResponseAsString(&response_body);
+
+  callback_.Run(
+      base::DictionaryValue::From(base::JSONReader::Read(response_body)));

[nyquist, 2017/04/19 06:35:57]

You're running in the main thread of the browser process and parsing external
JSON data. Should you instead use //components/safe_json/safe_json_parser.h ?

[Yusuf, 2017/04/26 00:51:36]

Done.

+
+  url_fetcher_.reset(nullptr);
+}
+
+bool DigitalAssetLinksHandler::CheckDigitalAssetLinkRelationship(
+    RelationshipCheckResultCallback listener,

[nyquist, 2017/04/19 06:35:57]

Nit: Should this be called |callback|?

[Yusuf, 2017/04/26 00:51:36]

Done.

+    const std::string& web_domain,
+    const std::string& package_name,
+    const std::string& fingerprint,
+    const std::string& relationship) {
+  GURL request_url = GetUrlForCheckingRelationship(web_domain, package_name,

[nyquist, 2017/04/19 06:35:56]

Nit: Should we verify that this happens on a particular thread?

[Yusuf, 2017/04/26 00:51:36]

Added a DCHECK to OriginVerifier side of the caller. Cant check it here because
we dont know what BrowserThread is here.

[nyquist, 2017/04/27 04:38:25]

Acknowledged.

+                                                   fingerprint, relationship);
+  if (!request_url.is_valid())
+    return false;
+
+  callback_ = listener;

[nyquist, 2017/04/19 06:35:57]

What if I call this method multiple times, particularly while the request is
outstanding? Wouldn't that lead to the wrong callback being invoked in
OnURLFetchComplete()?
Or is the plan that since the url_fetcher_ on the next line resets the
url_fetcher_ the OnURLFetchComplete will never happen since they both happen on
the same thread? If so, could you clarify why this is safe with a comment?

[Yusuf, 2017/04/26 00:51:36]

The latter. And I added a comment to this effect.

+
+  url_fetcher_ =
+      net::URLFetcher::Create(0, request_url, net::URLFetcher::GET, this);

[nyquist, 2017/04/19 06:35:56]

Should this have a traffic annotation attached to it? It would probably be
helpful for figuring out where the traffic is from when doing attribution of
power and network ,etc. See https://codereview.chromium.org/2703283004 as an
example.

[Yusuf, 2017/04/26 00:51:36]

Done.

[nyquist, 2017/04/27 04:38:25]

Awesome! Super duper helpful!

+  url_fetcher_->SetAutomaticallyRetryOn5xx(false);
+  url_fetcher_->SetRequestContext(request_context_.get());
+  url_fetcher_->Start();
+  return true;
+}
+
+}  // namespace digital_asset_links