Fix Reporting API permission checks with network service.

Bug: 845559
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ibcaa434f5ffb1c43363190d4030ddb70f9449bf9
Reviewed-on: https://chromium-review.googlesource.com/1213275
Commit-Queue: John Abd-El-Malek <[email protected]>
Reviewed-by: Robert Sesek <[email protected]>
Cr-Commit-Position: refs/heads/master@{#589953}

content/browser/storage_partition_impl.cc

@@ -41,6 +41,7 @@
 #include "content/public/browser/indexed_db_context.h"
 #include "content/public/browser/local_storage_usage_info.h"
 #include "content/public/browser/network_service_instance.h"
+#include "content/public/browser/permission_controller.h"
 #include "content/public/browser/session_storage_usage_info.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
@@ -494,6 +495,7 @@ StoragePartitionImpl::StoragePartitionImpl(
     storage::SpecialStoragePolicy* special_storage_policy)
     : partition_path_(partition_path),
       special_storage_policy_(special_storage_policy),
+      network_context_client_binding_(this),
       browser_context_(browser_context),
       deletion_helpers_running_(0),
       weak_factory_(this) {}
@@ -875,6 +877,26 @@ void StoragePartitionImpl::OpenSessionStorage(
                                            std::move(request));
 }
 
+void StoragePartitionImpl::OnCanSendReportingReports(
+    const std::vector<url::Origin>& origins,
+    OnCanSendReportingReportsCallback callback) {
+  PermissionController* permission_controller =
+      BrowserContext::GetPermissionController(browser_context_);
+  DCHECK(permission_controller);
+
+  std::vector<url::Origin> origins_out;
+  for (auto& origin : origins) {
+    GURL origin_url = origin.GetURL();
+    bool allowed = permission_controller->GetPermissionStatus(
+                       PermissionType::BACKGROUND_SYNC, origin_url,
+                       origin_url) == blink::mojom::PermissionStatus::GRANTED;
+    if (allowed)
+      origins_out.push_back(origin);
+  }
+
+  std::move(callback).Run(origins_out);
+}
+
 void StoragePartitionImpl::ClearDataImpl(
     uint32_t remove_mask,
     uint32_t quota_storage_remove_mask,
@@ -1281,6 +1303,10 @@ void StoragePartitionImpl::InitNetworkContext() {
                        base::Unretained(network_context_owner_.get()),
                        MakeRequest(&network_context_), url_request_context_));
   }
+  network::mojom::NetworkContextClientPtr client_ptr;
+  network_context_client_binding_.Close();
+  network_context_client_binding_.Bind(mojo::MakeRequest(&client_ptr));
+  network_context_->SetClient(std::move(client_ptr));
   network_context_.set_connection_error_handler(base::BindOnce(
       &StoragePartitionImpl::InitNetworkContext, weak_factory_.GetWeakPtr()));
 }

content/browser/storage_partition_impl.h

@@ -34,6 +34,7 @@
 #include "content/public/browser/storage_partition.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "storage/browser/quota/special_storage_policy.h"
 #include "third_party/blink/public/mojom/dom_storage/storage_partition_service.mojom.h"
@@ -53,7 +54,8 @@ class GeneratedCodeCacheContext;
 
 class CONTENT_EXPORT StoragePartitionImpl
     : public StoragePartition,
-      public blink::mojom::StoragePartitionService {
+      public blink::mojom::StoragePartitionService,
+      public network::mojom::NetworkContextClient {
  public:
   // It is guaranteed that storage partitions are destructed before the
   // browser context starts shutting down its corresponding IO thread residents
@@ -152,6 +154,11 @@ class CONTENT_EXPORT StoragePartitionImpl
       const std::string& namespace_id,
       blink::mojom::SessionStorageNamespaceRequest request) override;
 
+  // network::mojom::NetworkContextClient interface.
+  void OnCanSendReportingReports(
+      const std::vector<url::Origin>& origins,
+      OnCanSendReportingReportsCallback callback) override;
+
   scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter() {
     return url_loader_factory_getter_;
   }
@@ -332,6 +339,9 @@ class CONTENT_EXPORT StoragePartitionImpl
   // by |network_context_owner_|.
   network::mojom::NetworkContextPtr network_context_;
 
+  mojo::Binding<network::mojom::NetworkContextClient>
+      network_context_client_binding_;
+
   scoped_refptr<URLLoaderFactoryForBrowserProcess>
       shared_url_loader_factory_for_browser_process_;
 

services/network/network_context.cc

@@ -524,6 +524,10 @@ void NetworkContext::CreateURLLoaderFactory(
       std::move(request), nullptr));
 }
 
+void NetworkContext::SetClient(mojom::NetworkContextClientPtr client) {
+  client_ = std::move(client);
+}
+
 void NetworkContext::CreateURLLoaderFactory(
     mojom::URLLoaderFactoryRequest request,
     mojom::URLLoaderFactoryParamsPtr params) {

services/network/network_context.h

@@ -128,6 +128,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
 
   NetworkService* network_service() { return network_service_; }
 
+  mojom::NetworkContextClient* client() { return client_.get(); }
+
   ResourceScheduler* resource_scheduler() { return resource_scheduler_.get(); }
 
   CookieManager* cookie_manager() { return cookie_manager_.get(); }
@@ -141,6 +143,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
       scoped_refptr<ResourceSchedulerClient> resource_scheduler_client);
 
   // mojom::NetworkContext implementation:
+  void SetClient(mojom::NetworkContextClientPtr client) override;
   void CreateURLLoaderFactory(mojom::URLLoaderFactoryRequest request,
                               mojom::URLLoaderFactoryParamsPtr params) override;
   void GetCookieManager(mojom::CookieManagerRequest request) override;
@@ -297,6 +300,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
 
   NetworkService* const network_service_;
 
+  mojom::NetworkContextClientPtr client_;
+
   std::unique_ptr<ResourceScheduler> resource_scheduler_;
 
   // Holds owning pointer to |url_request_context_|. Will contain a nullptr for

services/network/network_service_network_delegate.cc

@@ -4,6 +4,7 @@
 
 #include "services/network/network_service_network_delegate.h"
 
+#include "services/network/cookie_manager.h"
 #include "services/network/network_context.h"
 #include "services/network/network_service.h"
 #include "services/network/public/cpp/features.h"
@@ -77,34 +78,44 @@ bool NetworkServiceNetworkDelegate::OnCanAccessFile(
 
 bool NetworkServiceNetworkDelegate::OnCanQueueReportingReport(
     const url::Origin& origin) const {
-  // TODO(crbug.com/845559): Disable all Reporting uploads until we can perform
-  // a BACKGROUND_SYNC permissions check across service boundaries.
-  return false;
+  return network_context_->cookie_manager()
+      ->cookie_settings()
+      .IsCookieAccessAllowed(origin.GetURL(), origin.GetURL());
 }
 
 void NetworkServiceNetworkDelegate::OnCanSendReportingReports(
     std::set<url::Origin> origins,
     base::OnceCallback<void(std::set<url::Origin>)> result_callback) const {
-  // TODO(crbug.com/845559): Disable all Reporting uploads until we can perform
-  // a BACKGROUND_SYNC permissions check across service boundaries.
-  origins.clear();
-  std::move(result_callback).Run(std::move(origins));
+  auto* client = network_context_->client();
+  if (!client) {
+    origins.clear();
+    std::move(result_callback).Run(std::move(origins));
+    return;
+  }
+
+  std::vector<url::Origin> origin_vector;
+  std::copy(origins.begin(), origins.end(), std::back_inserter(origin_vector));
+  client->OnCanSendReportingReports(
+      origin_vector,
+      base::BindOnce(
+          &NetworkServiceNetworkDelegate::FinishedCanSendReportingReports,
+          weak_ptr_factory_.GetWeakPtr(), std::move(result_callback)));
 }
 
 bool NetworkServiceNetworkDelegate::OnCanSetReportingClient(
     const url::Origin& origin,
     const GURL& endpoint) const {
-  // TODO(crbug.com/845559): Disable all Reporting uploads until we can perform
-  // a BACKGROUND_SYNC permissions check across service boundaries.
-  return false;
+  return network_context_->cookie_manager()
+      ->cookie_settings()
+      .IsCookieAccessAllowed(origin.GetURL(), origin.GetURL());
 }
 
 bool NetworkServiceNetworkDelegate::OnCanUseReportingClient(
     const url::Origin& origin,
     const GURL& endpoint) const {
-  // TODO(crbug.com/845559): Disable all Reporting uploads until we can perform
-  // a BACKGROUND_SYNC permissions check across service boundaries.
-  return false;
+  return network_context_->cookie_manager()
+      ->cookie_settings()
+      .IsCookieAccessAllowed(origin.GetURL(), origin.GetURL());
 }
 
 int NetworkServiceNetworkDelegate::HandleClearSiteDataHeader(
@@ -143,4 +154,11 @@ void NetworkServiceNetworkDelegate::FinishedClearSiteData(
     std::move(callback).Run(net::OK);
 }
 
+void NetworkServiceNetworkDelegate::FinishedCanSendReportingReports(
+    base::OnceCallback<void(std::set<url::Origin>)> result_callback,
+    const std::vector<url::Origin>& origins) {
+  std::set<url::Origin> origin_set(origins.begin(), origins.end());
+  std::move(result_callback).Run(origin_set);
+}
+
 }  // namespace network

services/network/network_service_network_delegate.h

@@ -55,10 +55,13 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkServiceNetworkDelegate
 
   void FinishedClearSiteData(base::WeakPtr<net::URLRequest> request,
                              net::CompletionOnceCallback callback);
+  void FinishedCanSendReportingReports(
+      base::OnceCallback<void(std::set<url::Origin>)> result_callback,
+      const std::vector<url::Origin>& origins);
 
   NetworkContext* network_context_;
 
-  base::WeakPtrFactory<NetworkServiceNetworkDelegate> weak_ptr_factory_;
+  mutable base::WeakPtrFactory<NetworkServiceNetworkDelegate> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkServiceNetworkDelegate);
 };

services/network/public/mojom/network_context.mojom

@@ -330,9 +330,22 @@ struct URLLoaderFactoryParams {
   bool disable_web_security = false;
 };
 
+// Callback interface for NetworkContext when routing identifiers aren't
+// available. Otherwise generally callbacks from the network service go on
+// NetworkServiceClient.
+interface NetworkContextClient {
+  // Checks if network error reports could be sent for the given origins.
+  // Replies with the origins that are allowed.
+  OnCanSendReportingReports(array<url.mojom.Origin> origins) =>
+      (array<url.mojom.Origin> origins);
+};
+
 // Represents a distinct context for making network requests, with its own
 // storage (e.g. cookies and cache).
 interface NetworkContext {
+  // Sets a client for this network context.
+  SetClient(NetworkContextClient client);
+
   // Creates a new URLLoaderFactory with the given |params|.
   CreateURLLoaderFactory(URLLoaderFactory& url_loader_factory,
                          URLLoaderFactoryParams params);

services/network/test/test_network_context.h

@@ -35,6 +35,7 @@ class TestNetworkContext : public mojom::NetworkContext {
   TestNetworkContext() = default;
   ~TestNetworkContext() override = default;
 
+  void SetClient(mojom::NetworkContextClientPtr client) override {}
   void CreateURLLoaderFactory(
       mojom::URLLoaderFactoryRequest request,
       mojom::URLLoaderFactoryParamsPtr params) override {}

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -107,11 +107,6 @@
 -NativeBindings/ExternallyConnectableMessagingTest.WebConnectableWithNonEmptyTlsChannelId/0
 -NativeBindings/MessagingApiTest.DifferentStoragePartitionTLSChannelID/0
 
-# https://crbug.com/845559
-# Reporting needs to check BACKGROUND_SYNC permission before uploading reports
-# about an origin.
--ReportingBrowserTest.TestReportingHeadersProcessed
-
 # https://crbug.com/721403
 -ContextMenuBrowserTest.DataSaverOpenOrigImageInNewTab