Commit fc59bb21 authored by Yao Xiao's avatar Yao Xiao Committed by Commit Bot

Fill in the network isolation key for autofill request

Fill in the key through resource_request->trusted_params, as the
factory (StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcess())
is used for many other purposes. The test
AutofillInteractiveTestWithHistogramTester.BasicFormFill covers this path.

The tests All/AutofillQueryTest/* are using a separate testing factory and
autofill driver, so also fill in the key there and set |is_trusted| to
true in the test factory.

Bug: 1009600
Change-Id: I4465e6c3f26f27ee1c7416d57ee508d26e0068a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1836456Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Reviewed-by: default avatarShivani Sharma <shivanisha@chromium.org>
Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708362}
parent 5875f78c
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/bind_test_util.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/mock_entropy_provider.h" #include "base/test/mock_entropy_provider.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
...@@ -68,6 +69,7 @@ ...@@ -68,6 +69,7 @@
#include "content/public/test/content_mock_cert_verifier.h" #include "content/public/test/content_mock_cert_verifier.h"
#include "content/public/test/test_renderer_host.h" #include "content/public/test/test_renderer_host.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
#include "content/public/test/url_loader_interceptor.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/cert/mock_cert_verifier.h" #include "net/cert/mock_cert_verifier.h"
#include "net/dns/mock_host_resolver.h" #include "net/dns/mock_host_resolver.h"
...@@ -82,6 +84,7 @@ ...@@ -82,6 +84,7 @@
#include "ui/events/keycodes/keyboard_codes.h" #include "ui/events/keycodes/keyboard_codes.h"
using base::ASCIIToUTF16; using base::ASCIIToUTF16;
using content::URLLoaderInterceptor;
namespace autofill { namespace autofill {
...@@ -721,8 +724,43 @@ class AutofillInteractiveTest : public AutofillInteractiveTestBase { ...@@ -721,8 +724,43 @@ class AutofillInteractiveTest : public AutofillInteractiveTestBase {
} }
}; };
class AutofillInteractiveTestWithHistogramTester
: public AutofillInteractiveTest {
public:
void SetUp() override {
// Only allow requests to be loaded that are necessary for the test. This
// allows a histogram to test properties of some specific requests.
std::vector<std::string> allowlist = {
"/internal/test_url_path", "https://clients1.google.com/tbproxy"};
url_loader_interceptor_ =
std::make_unique<URLLoaderInterceptor>(base::BindLambdaForTesting(
[&](URLLoaderInterceptor::RequestParams* params) {
for (const auto& s : allowlist) {
const bool is_match =
params->url_request.url.spec().find(s) != std::string::npos;
if (is_match)
return false; // Do not intercept.
}
return true; // Intercept
}));
AutofillInteractiveTest::SetUp();
}
void TearDownOnMainThread() override {
url_loader_interceptor_.reset();
AutofillInteractiveTest::TearDownOnMainThread();
}
base::HistogramTester& histogram_tester() { return histogram_tester_; }
private:
base::HistogramTester histogram_tester_;
std::unique_ptr<URLLoaderInterceptor> url_loader_interceptor_;
};
// Test that basic form fill is working. // Test that basic form fill is working.
IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, BasicFormFill) { IN_PROC_BROWSER_TEST_F(AutofillInteractiveTestWithHistogramTester,
BasicFormFill) {
LOG(ERROR) << "crbug/967588: In case of flakes, report log statements to " LOG(ERROR) << "crbug/967588: In case of flakes, report log statements to "
"crbug.com/967588"; "crbug.com/967588";
CreateTestProfile(); CreateTestProfile();
...@@ -737,6 +775,13 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, BasicFormFill) { ...@@ -737,6 +775,13 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, BasicFormFill) {
// Invoke Autofill. // Invoke Autofill.
TryBasicFormFill(); TryBasicFormFill();
LOG(ERROR) << "crbug/967588: Basic form filling completed"; LOG(ERROR) << "crbug/967588: Basic form filling completed";
SubprocessMetricsProvider::MergeHistogramDeltasForTesting();
// Assert that the network isolation key is populated for 2 requests:
// - Navigation: /internal/test_url_path
// - Autofill query: https://clients1.google.com/tbproxy/af/query?...
histogram_tester().ExpectBucketCount("HttpCache.NetworkIsolationKeyPresent2",
2 /*kPresent*/, 2 /*count*/);
} }
IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, BasicClear) { IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, BasicClear) {
......
...@@ -202,6 +202,14 @@ gfx::RectF ContentAutofillDriver::TransformBoundingBoxToViewportCoordinates( ...@@ -202,6 +202,14 @@ gfx::RectF ContentAutofillDriver::TransformBoundingBoxToViewportCoordinates(
bounding_box.width(), bounding_box.height()); bounding_box.width(), bounding_box.height());
} }
net::NetworkIsolationKey ContentAutofillDriver::NetworkIsolationKey() {
content::RenderFrameHost* top_frame_host = render_frame_host_;
while (top_frame_host->GetParent())
top_frame_host = top_frame_host->GetParent();
return net::NetworkIsolationKey(top_frame_host->GetLastCommittedOrigin(),
render_frame_host_->GetLastCommittedOrigin());
}
void ContentAutofillDriver::FormsSeen(const std::vector<FormData>& forms, void ContentAutofillDriver::FormsSeen(const std::vector<FormData>& forms,
base::TimeTicks timestamp) { base::TimeTicks timestamp) {
autofill_handler_->OnFormsSeen(forms, timestamp); autofill_handler_->OnFormsSeen(forms, timestamp);
......
...@@ -83,6 +83,7 @@ class ContentAutofillDriver : public AutofillDriver, ...@@ -83,6 +83,7 @@ class ContentAutofillDriver : public AutofillDriver,
void PopupHidden() override; void PopupHidden() override;
gfx::RectF TransformBoundingBoxToViewportCoordinates( gfx::RectF TransformBoundingBoxToViewportCoordinates(
const gfx::RectF& bounding_box) override; const gfx::RectF& bounding_box) override;
net::NetworkIsolationKey NetworkIsolationKey() override;
// mojom::AutofillDriver: // mojom::AutofillDriver:
void FormsSeen(const std::vector<FormData>& forms, void FormsSeen(const std::vector<FormData>& forms,
......
...@@ -852,6 +852,9 @@ bool AutofillDownloadManager::StartRequest(FormRequestData request_data) { ...@@ -852,6 +852,9 @@ bool AutofillDownloadManager::StartRequest(FormRequestData request_data) {
resource_request->url = request_url; resource_request->url = request_url;
resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit; resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit;
resource_request->method = method; resource_request->method = method;
resource_request->trusted_params = network::ResourceRequest::TrustedParams();
resource_request->trusted_params->network_isolation_key =
driver_->NetworkIsolationKey();
// Add Chrome experiment state to the request headers. // Add Chrome experiment state to the request headers.
variations::AppendVariationsHeaderUnknownSignedIn( variations::AppendVariationsHeaderUnknownSignedIn(
......
...@@ -1353,9 +1353,13 @@ class AutofillServerCommunicationTest ...@@ -1353,9 +1353,13 @@ class AutofillServerCommunicationTest
// Intialize the autofill driver. // Intialize the autofill driver.
shared_url_loader_factory_ = shared_url_loader_factory_ =
base::MakeRefCounted<network::TestSharedURLLoaderFactory>(); base::MakeRefCounted<network::TestSharedURLLoaderFactory>(
nullptr /* network_service */, true /* is_trusted */);
driver_ = std::make_unique<TestAutofillDriver>(); driver_ = std::make_unique<TestAutofillDriver>();
driver_->SetSharedURLLoaderFactory(shared_url_loader_factory_); driver_->SetSharedURLLoaderFactory(shared_url_loader_factory_);
driver_->SetNetworkIsolationKey(
net::NetworkIsolationKey(url::Origin::Create(GURL("https://abc.com")),
url::Origin::Create(GURL("https://xyz.com"))));
// Configure the autofill server communications channel. // Configure the autofill server communications channel.
switch (GetParam()) { switch (GetParam()) {
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "components/autofill/core/common/form_data.h" #include "components/autofill/core/common/form_data.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "net/base/network_isolation_key.h"
#if !defined(OS_IOS) #if !defined(OS_IOS)
#include "third_party/blink/public/mojom/webauthn/internal_authenticator.mojom.h" #include "third_party/blink/public/mojom/webauthn/internal_authenticator.mojom.h"
...@@ -119,6 +120,8 @@ class AutofillDriver { ...@@ -119,6 +120,8 @@ class AutofillDriver {
// renderers cannot do this transformation themselves. // renderers cannot do this transformation themselves.
virtual gfx::RectF TransformBoundingBoxToViewportCoordinates( virtual gfx::RectF TransformBoundingBoxToViewportCoordinates(
const gfx::RectF& bounding_box) = 0; const gfx::RectF& bounding_box) = 0;
virtual net::NetworkIsolationKey NetworkIsolationKey() = 0;
}; };
} // namespace autofill } // namespace autofill
......
...@@ -89,6 +89,10 @@ gfx::RectF TestAutofillDriver::TransformBoundingBoxToViewportCoordinates( ...@@ -89,6 +89,10 @@ gfx::RectF TestAutofillDriver::TransformBoundingBoxToViewportCoordinates(
return bounding_box; return bounding_box;
} }
net::NetworkIsolationKey TestAutofillDriver::NetworkIsolationKey() {
return network_isolation_key_;
}
void TestAutofillDriver::SetIsIncognito(bool is_incognito) { void TestAutofillDriver::SetIsIncognito(bool is_incognito) {
is_incognito_ = is_incognito; is_incognito_ = is_incognito;
} }
...@@ -97,6 +101,11 @@ void TestAutofillDriver::SetIsInMainFrame(bool is_in_main_frame) { ...@@ -97,6 +101,11 @@ void TestAutofillDriver::SetIsInMainFrame(bool is_in_main_frame) {
is_in_main_frame_ = is_in_main_frame; is_in_main_frame_ = is_in_main_frame;
} }
void TestAutofillDriver::SetNetworkIsolationKey(
const net::NetworkIsolationKey& network_isolation_key) {
network_isolation_key_ = network_isolation_key;
}
void TestAutofillDriver::SetSharedURLLoaderFactory( void TestAutofillDriver::SetSharedURLLoaderFactory(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) { scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
test_shared_loader_factory_ = url_loader_factory; test_shared_loader_factory_ = url_loader_factory;
......
...@@ -51,12 +51,15 @@ class TestAutofillDriver : public AutofillDriver { ...@@ -51,12 +51,15 @@ class TestAutofillDriver : public AutofillDriver {
void PopupHidden() override; void PopupHidden() override;
gfx::RectF TransformBoundingBoxToViewportCoordinates( gfx::RectF TransformBoundingBoxToViewportCoordinates(
const gfx::RectF& bounding_box) override; const gfx::RectF& bounding_box) override;
net::NetworkIsolationKey NetworkIsolationKey() override;
// Methods unique to TestAutofillDriver that tests can use to specialize // Methods unique to TestAutofillDriver that tests can use to specialize
// functionality. // functionality.
void SetIsIncognito(bool is_incognito); void SetIsIncognito(bool is_incognito);
void SetIsInMainFrame(bool is_in_main_frame); void SetIsInMainFrame(bool is_in_main_frame);
void SetNetworkIsolationKey(
const net::NetworkIsolationKey& network_isolation_key);
void SetSharedURLLoaderFactory( void SetSharedURLLoaderFactory(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory); scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
...@@ -66,6 +69,7 @@ class TestAutofillDriver : public AutofillDriver { ...@@ -66,6 +69,7 @@ class TestAutofillDriver : public AutofillDriver {
scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_; scoped_refptr<network::SharedURLLoaderFactory> test_shared_loader_factory_;
bool is_incognito_ = false; bool is_incognito_ = false;
bool is_in_main_frame_ = false; bool is_in_main_frame_ = false;
net::NetworkIsolationKey network_isolation_key_;
DISALLOW_COPY_AND_ASSIGN(TestAutofillDriver); DISALLOW_COPY_AND_ASSIGN(TestAutofillDriver);
}; };
......
...@@ -65,6 +65,7 @@ class AutofillDriverIOS : public AutofillDriver { ...@@ -65,6 +65,7 @@ class AutofillDriverIOS : public AutofillDriver {
void PopupHidden() override; void PopupHidden() override;
gfx::RectF TransformBoundingBoxToViewportCoordinates( gfx::RectF TransformBoundingBoxToViewportCoordinates(
const gfx::RectF& bounding_box) override; const gfx::RectF& bounding_box) override;
net::NetworkIsolationKey NetworkIsolationKey() override;
bool is_processed() const { return processed_; } bool is_processed() const { return processed_; }
void set_processed(bool processed) { processed_ = processed; } void set_processed(bool processed) { processed_ = processed; }
......
...@@ -141,4 +141,20 @@ gfx::RectF AutofillDriverIOS::TransformBoundingBoxToViewportCoordinates( ...@@ -141,4 +141,20 @@ gfx::RectF AutofillDriverIOS::TransformBoundingBoxToViewportCoordinates(
return bounding_box; return bounding_box;
} }
net::NetworkIsolationKey AutofillDriverIOS::NetworkIsolationKey() {
std::string main_web_frame_id = web::GetMainWebFrameId(web_state_);
web::WebFrame* main_web_frame =
web::GetWebFrameWithId(web_state_, main_web_frame_id);
if (!main_web_frame)
return net::NetworkIsolationKey();
web::WebFrame* web_frame = web::GetWebFrameWithId(web_state_, web_frame_id_);
if (!web_frame)
return net::NetworkIsolationKey();
return net::NetworkIsolationKey(
url::Origin::Create(main_web_frame->GetSecurityOrigin()),
url::Origin::Create(web_frame->GetSecurityOrigin()));
}
} // namespace autofill } // namespace autofill
...@@ -12,7 +12,8 @@ ...@@ -12,7 +12,8 @@
namespace network { namespace network {
TestSharedURLLoaderFactory::TestSharedURLLoaderFactory( TestSharedURLLoaderFactory::TestSharedURLLoaderFactory(
NetworkService* network_service) { NetworkService* network_service,
bool is_trusted) {
url_request_context_ = std::make_unique<net::TestURLRequestContext>(); url_request_context_ = std::make_unique<net::TestURLRequestContext>();
mojo::Remote<mojom::NetworkContext> network_context; mojo::Remote<mojom::NetworkContext> network_context;
network_context_ = std::make_unique<NetworkContext>( network_context_ = std::make_unique<NetworkContext>(
...@@ -23,6 +24,7 @@ TestSharedURLLoaderFactory::TestSharedURLLoaderFactory( ...@@ -23,6 +24,7 @@ TestSharedURLLoaderFactory::TestSharedURLLoaderFactory(
mojom::URLLoaderFactoryParams::New(); mojom::URLLoaderFactoryParams::New();
params->process_id = mojom::kBrowserProcessId; params->process_id = mojom::kBrowserProcessId;
params->is_corb_enabled = false; params->is_corb_enabled = false;
params->is_trusted = is_trusted;
network_context_->CreateURLLoaderFactory( network_context_->CreateURLLoaderFactory(
url_loader_factory_.BindNewPipeAndPassReceiver(), std::move(params)); url_loader_factory_.BindNewPipeAndPassReceiver(), std::move(params));
} }
......
...@@ -26,8 +26,8 @@ class NetworkService; ...@@ -26,8 +26,8 @@ class NetworkService;
// across threads. // across threads.
class TestSharedURLLoaderFactory : public SharedURLLoaderFactory { class TestSharedURLLoaderFactory : public SharedURLLoaderFactory {
public: public:
explicit TestSharedURLLoaderFactory( explicit TestSharedURLLoaderFactory(NetworkService* network_service = nullptr,
NetworkService* network_service = nullptr); bool is_trusted = false);
// URLLoaderFactory implementation: // URLLoaderFactory implementation:
void CreateLoaderAndStart(mojom::URLLoaderRequest loader, void CreateLoaderAndStart(mojom::URLLoaderRequest loader,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment