diff --git a/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.cc b/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.cc index f5c9e3885fb204..f4485bfaf3f378 100644 --- a/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.cc +++ b/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.cc @@ -109,6 +109,14 @@ void WilcoDtcSupportdNetworkContextImpl::OnAuthRequired( auth_challenge_responder_remote->OnAuthCredentials(base::nullopt); } +void WilcoDtcSupportdNetworkContextImpl::OnClearSiteData( + const GURL& url, + const std::string& header_value, + int32_t load_flags, + OnClearSiteDataCallback callback) { + std::move(callback).Run(); +} + void WilcoDtcSupportdNetworkContextImpl::Clone( mojo::PendingReceiver observer) { diff --git a/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.h b/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.h index fe7efd9bc2fbb8..83ba614a83fcc3 100644 --- a/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.h +++ b/chrome/browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_network_context.h @@ -70,6 +70,10 @@ class WilcoDtcSupportdNetworkContextImpl const scoped_refptr& head_headers, mojo::PendingRemote auth_challenge_responder) override; + void OnClearSiteData(const GURL& url, + const std::string& header_value, + int32_t load_flags, + OnClearSiteDataCallback callback) override; void Clone(mojo::PendingReceiver< network::mojom::AuthenticationAndCertificateObserver> listener) override; diff --git a/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.cc b/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.cc index 75105b3a8e86e0..ee65355246c8a7 100644 --- a/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.cc +++ b/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.cc @@ -34,16 +34,6 @@ void PrefetchProxyNetworkContextClient::OnCanSendDomainReliabilityUpload( std::move(callback).Run(false); } -void PrefetchProxyNetworkContextClient::OnClearSiteData( - int32_t process_id, - int32_t routing_id, - const GURL& url, - const std::string& header_value, - int load_flags, - OnClearSiteDataCallback callback) { - std::move(callback).Run(); -} - #if defined(OS_ANDROID) void PrefetchProxyNetworkContextClient::OnGenerateHttpNegotiateAuthToken( const std::string& server_auth_token, diff --git a/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.h b/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.h index a44b1c6a6c8717..aa344a3563b6ec 100644 --- a/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.h +++ b/chrome/browser/prefetch/prefetch_proxy/prefetch_proxy_network_context_client.h @@ -30,12 +30,6 @@ class PrefetchProxyNetworkContextClient void OnCanSendDomainReliabilityUpload( const GURL& origin, OnCanSendDomainReliabilityUploadCallback callback) override; - void OnClearSiteData(int32_t process_id, - int32_t routing_id, - const GURL& url, - const std::string& header_value, - int load_flags, - OnClearSiteDataCallback callback) override; #if defined(OS_ANDROID) void OnGenerateHttpNegotiateAuthToken( const std::string& server_auth_token, diff --git a/content/browser/network_context_client_base_impl.cc b/content/browser/network_context_client_base_impl.cc index 9625d66b5e3d80..f33b81244679fd 100644 --- a/content/browser/network_context_client_base_impl.cc +++ b/content/browser/network_context_client_base_impl.cc @@ -103,16 +103,6 @@ void NetworkContextClientBase::OnCanSendDomainReliabilityUpload( std::move(callback).Run(false); } -void NetworkContextClientBase::OnClearSiteData( - int32_t process_id, - int32_t routing_id, - const GURL& url, - const std::string& header_value, - int load_flags, - OnClearSiteDataCallback callback) { - std::move(callback).Run(); -} - #if defined(OS_ANDROID) void NetworkContextClientBase::OnGenerateHttpNegotiateAuthToken( const std::string& server_auth_token, diff --git a/content/browser/storage_partition_impl.cc b/content/browser/storage_partition_impl.cc index a3e6d130d1fef5..4cef106fdf6ae5 100644 --- a/content/browser/storage_partition_impl.cc +++ b/content/browser/storage_partition_impl.cc @@ -1836,13 +1836,13 @@ void StoragePartitionImpl::OnCanSendDomainReliabilityUpload( blink::mojom::PermissionStatus::GRANTED); } -void StoragePartitionImpl::OnClearSiteData(int32_t process_id, - int32_t routing_id, - const GURL& url, +void StoragePartitionImpl::OnClearSiteData(const GURL& url, const std::string& header_value, int load_flags, OnClearSiteDataCallback callback) { DCHECK(initialized_); + int process_id = auth_cert_observers_.current_context().process_id; + int routing_id = auth_cert_observers_.current_context().routing_id; auto browser_context_getter = base::BindRepeating( GetBrowserContextFromStoragePartition, weak_factory_.GetWeakPtr()); auto web_contents_getter = base::BindRepeating( @@ -2551,6 +2551,7 @@ StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal( // Corb requests are likely made on behalf of untrusted renderers. if (!corb_enabled) params->is_trusted = true; + params->auth_cert_observer = CreateAuthCertObserverForServiceWorker(); params->disable_web_security = base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kDisableWebSecurity); diff --git a/content/browser/storage_partition_impl.h b/content/browser/storage_partition_impl.h index 1f2aea2e5dfa0a..853ef468b48452 100644 --- a/content/browser/storage_partition_impl.h +++ b/content/browser/storage_partition_impl.h @@ -250,12 +250,6 @@ class CONTENT_EXPORT StoragePartitionImpl void OnCanSendDomainReliabilityUpload( const GURL& origin, OnCanSendDomainReliabilityUploadCallback callback) override; - void OnClearSiteData(int32_t process_id, - int32_t routing_id, - const GURL& url, - const std::string& header_value, - int load_flags, - OnClearSiteDataCallback callback) override; #if defined(OS_ANDROID) void OnGenerateHttpNegotiateAuthToken( const std::string& server_auth_token, @@ -294,6 +288,10 @@ class CONTENT_EXPORT StoragePartitionImpl const scoped_refptr& head_headers, mojo::PendingRemote auth_challenge_responder) override; + void OnClearSiteData(const GURL& url, + const std::string& header_value, + int load_flags, + OnClearSiteDataCallback callback) override; scoped_refptr url_loader_factory_getter() { return url_loader_factory_getter_; diff --git a/content/public/browser/network_context_client_base.h b/content/public/browser/network_context_client_base.h index b16281aa0e2904..5d4909ba0b3cb3 100644 --- a/content/public/browser/network_context_client_base.h +++ b/content/public/browser/network_context_client_base.h @@ -33,12 +33,6 @@ class CONTENT_EXPORT NetworkContextClientBase void OnCanSendDomainReliabilityUpload( const GURL& origin, OnCanSendDomainReliabilityUploadCallback callback) override; - void OnClearSiteData(int32_t process_id, - int32_t routing_id, - const GURL& url, - const std::string& header_value, - int load_flags, - OnClearSiteDataCallback callback) override; #if defined(OS_ANDROID) void OnGenerateHttpNegotiateAuthToken( const std::string& server_auth_token, diff --git a/services/network/network_service_network_delegate.cc b/services/network/network_service_network_delegate.cc index 25e64eb7c66c30..72219c95f7c17d 100644 --- a/services/network/network_service_network_delegate.cc +++ b/services/network/network_service_network_delegate.cc @@ -321,21 +321,24 @@ int NetworkServiceNetworkDelegate::HandleClearSiteDataHeader( net::CompletionOnceCallback callback, const net::HttpResponseHeaders* original_response_headers) { DCHECK(request); - if (!original_response_headers || !network_context_->client()) + if (!original_response_headers) return net::OK; URLLoader* url_loader = URLLoader::ForRequest(*request); if (!url_loader) return net::OK; + auto* auth_cert_observer = url_loader->GetAuthCertObserver(); + if (!auth_cert_observer) + return net::OK; + std::string header_value; if (!original_response_headers->GetNormalizedHeader(kClearSiteDataHeader, &header_value)) { return net::OK; } - network_context_->client()->OnClearSiteData( - url_loader->GetProcessId(), url_loader->GetRenderFrameId(), + auth_cert_observer->OnClearSiteData( request->url(), header_value, request->load_flags(), base::BindOnce(&NetworkServiceNetworkDelegate::FinishedClearSiteData, weak_ptr_factory_.GetWeakPtr(), request->GetWeakPtr(), diff --git a/services/network/network_service_unittest.cc b/services/network/network_service_unittest.cc index b1bf2d33eef513..66db279a416e03 100644 --- a/services/network/network_service_unittest.cc +++ b/services/network/network_service_unittest.cc @@ -58,6 +58,7 @@ #include "services/network/public/mojom/network_context.mojom.h" #include "services/network/public/mojom/network_service.mojom.h" #include "services/network/test/fake_test_cert_verifier_params_factory.h" +#include "services/network/test/test_auth_cert_observer.h" #include "services/network/test/test_network_context_client.h" #include "services/network/test/test_network_service_client.h" #include "services/network/test/test_url_loader_client.h" @@ -1408,24 +1409,32 @@ class NetworkServiceNetworkDelegateTest : public NetworkServiceTest { std::move(context_params)); } - void LoadURL(const GURL& url, int options = mojom::kURLLoadOptionNone) { + void LoadURL(const GURL& url, + int options = mojom::kURLLoadOptionNone, + mojo::PendingRemote + auth_cert_observer = mojo::NullRemote()) { ResourceRequest request; request.url = url; request.method = "GET"; request.request_initiator = url::Origin(); - StartLoadingURL(request, 0 /* process_id */, options); + StartLoadingURL(request, 0 /* process_id */, options, + std::move(auth_cert_observer)); client_->RunUntilComplete(); } - void StartLoadingURL(const ResourceRequest& request, - uint32_t process_id, - int options = mojom::kURLLoadOptionNone) { + void StartLoadingURL( + const ResourceRequest& request, + uint32_t process_id, + int options = mojom::kURLLoadOptionNone, + mojo::PendingRemote + auth_cert_observer = mojo::NullRemote()) { client_.reset(new TestURLLoaderClient()); mojo::Remote loader_factory; mojom::URLLoaderFactoryParamsPtr params = mojom::URLLoaderFactoryParams::New(); params->process_id = process_id; params->is_corb_enabled = false; + params->auth_cert_observer = std::move(auth_cert_observer); network_context_->CreateURLLoaderFactory( loader_factory.BindNewPipeAndPassReceiver(), std::move(params)); @@ -1483,16 +1492,12 @@ class NetworkServiceNetworkDelegateTest : public NetworkServiceTest { DISALLOW_COPY_AND_ASSIGN(NetworkServiceNetworkDelegateTest); }; -class ClearSiteDataNetworkContextClient : public TestNetworkContextClient { +class ClearSiteDataAuthCertObserver : public TestAuthCertObserver { public: - explicit ClearSiteDataNetworkContextClient( - mojo::PendingReceiver receiver) - : receiver_(this, std::move(receiver)) {} - ~ClearSiteDataNetworkContextClient() override = default; - - void OnClearSiteData(int32_t process_id, - int32_t routing_id, - const GURL& url, + explicit ClearSiteDataAuthCertObserver() = default; + ~ClearSiteDataAuthCertObserver() override = default; + + void OnClearSiteData(const GURL& url, const std::string& header_value, int load_flags, OnClearSiteDataCallback callback) override { @@ -1515,34 +1520,30 @@ class ClearSiteDataNetworkContextClient : public TestNetworkContextClient { private: int on_clear_site_data_counter_ = 0; std::string last_on_clear_site_data_header_value_; - mojo::Receiver receiver_; }; // Check that |NetworkServiceNetworkDelegate| handles Clear-Site-Data header // w/ and w/o |NetworkContextCient|. -TEST_F(NetworkServiceNetworkDelegateTest, ClearSiteDataNetworkContextCient) { +TEST_F(NetworkServiceNetworkDelegateTest, ClearSiteDataObserver) { const char kClearCookiesHeader[] = "Clear-Site-Data: \"cookies\""; CreateNetworkContext(); - // Null |NetworkContextCient|. The request should complete without being + // Null |auth_cert_observer|. The request should complete without being // deferred. GURL url = https_server()->GetURL("/foo"); url = AddQuery(url, "header", kClearCookiesHeader); LoadURL(url); EXPECT_EQ(net::OK, client()->completion_status().error_code); - // With |NetworkContextCient|. The request should go through - // |ClearSiteDataNetworkContextClient| and complete. - mojo::PendingRemote client_remote; - auto client_impl = std::make_unique( - client_remote.InitWithNewPipeAndPassReceiver()); - network_context_->SetClient(std::move(client_remote)); + // With |auth_cert_observer|. The request should go through + // |ClearSiteDataAuthCertObserver| and complete. + ClearSiteDataAuthCertObserver clear_site_observer; url = https_server()->GetURL("/bar"); url = AddQuery(url, "header", kClearCookiesHeader); - EXPECT_EQ(0, client_impl->on_clear_site_data_counter()); - LoadURL(url); + EXPECT_EQ(0, clear_site_observer.on_clear_site_data_counter()); + LoadURL(url, mojom::kURLLoadOptionNone, clear_site_observer.Bind()); EXPECT_EQ(net::OK, client()->completion_status().error_code); - EXPECT_EQ(1, client_impl->on_clear_site_data_counter()); + EXPECT_EQ(1, clear_site_observer.on_clear_site_data_counter()); } // Check that headers are handled and passed to the client correctly. @@ -1551,10 +1552,7 @@ TEST_F(NetworkServiceNetworkDelegateTest, HandleClearSiteDataHeaders) { const char kClearCookiesHeader[] = "Clear-Site-Data: \"cookies\""; CreateNetworkContext(); - mojo::PendingRemote client_remote; - auto client_impl = std::make_unique( - client_remote.InitWithNewPipeAndPassReceiver()); - network_context_->SetClient(std::move(client_remote)); + ClearSiteDataAuthCertObserver clear_site_observer; // |passed_header_value| are only checked if |should_call_client| is true. const struct TestCase { @@ -1592,18 +1590,18 @@ TEST_F(NetworkServiceNetworkDelegateTest, HandleClearSiteDataHeaders) { GURL url = https_server()->GetURL("/foo"); url = AddQuery(url, "header", test_case.response_headers); - EXPECT_EQ(0, client_impl->on_clear_site_data_counter()); - LoadURL(url); + EXPECT_EQ(0, clear_site_observer.on_clear_site_data_counter()); + LoadURL(url, mojom::kURLLoadOptionNone, clear_site_observer.Bind()); EXPECT_EQ(net::OK, client()->completion_status().error_code); if (test_case.should_call_client) { - EXPECT_EQ(1, client_impl->on_clear_site_data_counter()); + EXPECT_EQ(1, clear_site_observer.on_clear_site_data_counter()); EXPECT_EQ(test_case.passed_header_value, - client_impl->last_on_clear_site_data_header_value()); + clear_site_observer.last_on_clear_site_data_header_value()); } else { - EXPECT_EQ(0, client_impl->on_clear_site_data_counter()); + EXPECT_EQ(0, clear_site_observer.on_clear_site_data_counter()); } - client_impl->ClearOnClearSiteDataCounter(); + clear_site_observer.ClearOnClearSiteDataCounter(); } } diff --git a/services/network/public/mojom/auth_and_certificate_observer.mojom b/services/network/public/mojom/auth_and_certificate_observer.mojom index 0192f6b3cf632f..ece09be5a439a2 100644 --- a/services/network/public/mojom/auth_and_certificate_observer.mojom +++ b/services/network/public/mojom/auth_and_certificate_observer.mojom @@ -67,6 +67,8 @@ interface AuthChallengeResponder { // This interface is used by an URLLoaderFactory to provide notification // of authentication and certificate events. This is typically implemented // by a trusted process such as the browser process. +// TODO(dtapuska): Rename this interface. It is used for more than +// authentication and certificate now. interface AuthenticationAndCertificateObserver { // Called when an SSL certificate is encountered. // The callback argument is a net::ERROR value. If it's net::OK, then the @@ -113,6 +115,16 @@ interface AuthenticationAndCertificateObserver { HttpResponseHeaders? head_headers, pending_remote auth_challenge_responder); + // Called when the Clear-Site-Data header has been received. The callback + // should be run after the data is deleted. + // https://www.w3.org/TR/clear-site-data/ + // TODO(crbug.com/876931): We might want to move header parsing work to + // Network Service for security concerns (e.g. |header_value| => booleans). + OnClearSiteData(url.mojom.Url url, + string header_value, + int32 load_flags) => (); + + // Used by the NetworkService to create a copy of this observer. // (e.g. when creating an observer for URLLoader from URLLoaderFactory's // observer). diff --git a/services/network/public/mojom/network_context.mojom b/services/network/public/mojom/network_context.mojom index f8ea033c73dfeb..7d708f26098739 100644 --- a/services/network/public/mojom/network_context.mojom +++ b/services/network/public/mojom/network_context.mojom @@ -713,17 +713,6 @@ interface NetworkContextClient { // Checks if a Domain Reliability report can be uploaded for the given origin. OnCanSendDomainReliabilityUpload(url.mojom.Url origin) => (bool allowed); - // Called when the Clear-Site-Data header has been received. The callback - // should be run after the data is deleted. - // https://www.w3.org/TR/clear-site-data/ - // TODO(crbug.com/876931): We might want to move header parsing work to - // Network Service for security concerns (e.g. |header_value| => booleans). - OnClearSiteData(int32 process_id, - int32 routing_id, - url.mojom.Url url, - string header_value, - int32 load_flags) => (); - // Called to generate an auth token for SPNEGO authentication on Android. [EnableIf=is_android] OnGenerateHttpNegotiateAuthToken(string server_auth_token, bool can_delegate, diff --git a/services/network/test/test_auth_cert_observer.cc b/services/network/test/test_auth_cert_observer.cc index 5e0d506da79f5d..02093391be4565 100644 --- a/services/network/test/test_auth_cert_observer.cc +++ b/services/network/test/test_auth_cert_observer.cc @@ -43,6 +43,13 @@ void TestAuthCertObserver::OnAuthRequired( mojo::PendingRemote auth_challenge_responder) {} +void TestAuthCertObserver::OnClearSiteData(const GURL& url, + const std::string& header_value, + int32_t load_flags, + OnClearSiteDataCallback callback) { + std::move(callback).Run(); +} + void TestAuthCertObserver::Clone( mojo::PendingReceiver observer) { receivers_.Add(this, std::move(observer)); diff --git a/services/network/test/test_auth_cert_observer.h b/services/network/test/test_auth_cert_observer.h index 80f9a08bfc8704..0a6b3397f17ee9 100644 --- a/services/network/test/test_auth_cert_observer.h +++ b/services/network/test/test_auth_cert_observer.h @@ -46,6 +46,10 @@ class TestAuthCertObserver const scoped_refptr& head_headers, mojo::PendingRemote auth_challenge_responder) override; + void OnClearSiteData(const GURL& url, + const std::string& header_value, + int32_t load_flags, + OnClearSiteDataCallback callback) override; void Clone(mojo::PendingReceiver observer) override; diff --git a/services/network/test/test_network_context_client.h b/services/network/test/test_network_context_client.h index 6586494c26c953..53fd3f6ee5033e 100644 --- a/services/network/test/test_network_context_client.h +++ b/services/network/test/test_network_context_client.h @@ -40,12 +40,6 @@ class TestNetworkContextClient : public network::mojom::NetworkContextClient { void OnCanSendDomainReliabilityUpload( const GURL& origin, OnCanSendDomainReliabilityUploadCallback callback) override {} - void OnClearSiteData(int32_t process_id, - int32_t routing_id, - const GURL& url, - const std::string& header_value, - int32_t load_flags, - OnClearSiteDataCallback callback) override {} #if defined(OS_ANDROID) void OnGenerateHttpNegotiateAuthToken( const std::string& server_auth_token, diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc index 56bb13b10d7997..ce483753dff013 100644 --- a/services/network/url_loader.cc +++ b/services/network/url_loader.cc @@ -1645,6 +1645,12 @@ int URLLoader::OnHeadersReceived( return net::OK; } +mojom::AuthenticationAndCertificateObserver* URLLoader::GetAuthCertObserver() { + if (!auth_cert_observer_) + return nullptr; + return auth_cert_observer_.get(); +} + net::LoadState URLLoader::GetLoadStateForTesting() const { return url_request_->GetLoadState().state; } diff --git a/services/network/url_loader.h b/services/network/url_loader.h index c93bb090502349..21ec3b43cd58c7 100644 --- a/services/network/url_loader.h +++ b/services/network/url_loader.h @@ -179,6 +179,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader const net::IPEndPoint& endpoint, base::Optional* preserve_fragment_on_redirect_url); + mojom::AuthenticationAndCertificateObserver* GetAuthCertObserver(); + // mojom::AuthChallengeResponder: void OnAuthCredentials( const base::Optional& credentials) override;