Add a UseCounter for cross origin redirects with Authorization header

To measure how often it happens.

Bug: 1393520
Change-Id: Idd8125250600381affc0f66349d77abdcb2dc9ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4235597
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1106130}

third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom

@@ -3808,6 +3808,7 @@ enum WebFeature {
   kCSSAnchorPositioning = 4467,
   kServiceWorkerAddHandlerAfterInitialization = 4468,
   kServiceWorkerSetAttributeHandlerAfterInitialization = 4469,
+  kAuthorizationCrossOrigin = 4470,
 
   // Add new features immediately above this line. Don't change assigned
   // numbers of any item, and don't reuse removed slots.

third_party/blink/renderer/platform/loader/fetch/resource_loader.cc

@@ -806,6 +806,14 @@ bool ResourceLoader::WillFollowRedirect(
         mojom::WebFeature::kAuthorizationCoveredByWildcard);
   }
 
+  if (resource_->GetResourceRequest().HttpHeaderFields().Contains(
+          net::HttpRequestHeaders::kAuthorization) &&
+      !SecurityOrigin::AreSameOrigin(resource_->LastResourceRequest().Url(),
+                                     new_url)) {
+    fetcher_->GetUseCounter().CountUse(
+        mojom::WebFeature::kAuthorizationCrossOrigin);
+  }
+
   if (removed_headers) {
     FindClientHintsToRemove(Context().GetPermissionsPolicy(),
                             GURL(new_url.GetString().Utf8()), removed_headers);
@@ -1318,6 +1326,10 @@ void ResourceLoader::DidFail(const WebURLError& error,
   HandleError(ResourceError(error));
 }
 
+void ResourceLoader::CountFeature(blink::mojom::WebFeature feature) {
+  fetcher_->GetUseCounter().CountUse(feature);
+}
+
 void ResourceLoader::HandleError(const ResourceError& error) {
   if (error.CorsErrorStatus() &&
       error.CorsErrorStatus()

third_party/blink/renderer/platform/loader/fetch/resource_loader.h

@@ -161,6 +161,7 @@ class PLATFORM_EXPORT ResourceLoader final
                int64_t encoded_data_length,
                uint64_t encoded_body_length,
                int64_t decoded_body_length) override;
+  void CountFeature(blink::mojom::WebFeature) override;
 
   mojom::blink::CodeCacheType GetCodeCacheType() const;
   void SendCachedCodeToResource(mojo_base::BigBuffer data);

third_party/blink/renderer/platform/loader/fetch/resource_loader_test.cc

@@ -11,13 +11,20 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "mojo/public/c/system/data_pipe.h"
+#include "net/http/http_response_headers.h"
 #include "services/network/public/mojom/fetch_api.mojom-blink.h"
+#include "services/network/public/mojom/url_response_head.mojom.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
+#include "third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom-blink.h"
 #include "third_party/blink/public/platform/resource_load_info_notifier_wrapper.h"
 #include "third_party/blink/public/platform/web_url_request_extra_data.h"
 #include "third_party/blink/renderer/platform/exported/wrapped_resource_response.h"
+#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/platform/instrumentation/use_counter.h"
+#include "third_party/blink/renderer/platform/loader/fetch/detachable_use_counter.h"
 #include "third_party/blink/renderer/platform/loader/fetch/raw_resource.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_load_scheduler.h"
@@ -27,6 +34,7 @@
 #include "third_party/blink/renderer/platform/loader/testing/bytes_consumer_test_reader.h"
 #include "third_party/blink/renderer/platform/loader/testing/mock_fetch_context.h"
 #include "third_party/blink/renderer/platform/loader/testing/test_resource_fetcher_properties.h"
+#include "third_party/blink/renderer/platform/network/http_names.h"
 #include "third_party/blink/renderer/platform/testing/code_cache_loader_mock.h"
 #include "third_party/blink/renderer/platform/testing/mock_context_lifecycle_notifier.h"
 #include "third_party/blink/renderer/platform/testing/noop_url_loader.h"
@@ -48,6 +56,19 @@ const char kCnameAliasWasAdTaggedHistogram[] =
 const char kCnameAliasWasBlockedHistogram[] =
     "SubresourceFilter.CnameAlias.Renderer.WasBlockedBasedOnAlias";
 
+namespace {
+
+using ::testing::_;
+
+class MockUseCounter : public GarbageCollected<MockUseCounter>,
+                       public UseCounter {
+ public:
+  MOCK_METHOD1(CountUse, void(mojom::WebFeature));
+  MOCK_METHOD1(CountDeprecation, void(mojom::WebFeature));
+};
+
+}  // namespace
+
 class ResourceLoaderTest : public testing::Test {
  public:
   enum class From {
@@ -100,15 +121,21 @@ class ResourceLoaderTest : public testing::Test {
   ResourceFetcher* MakeResourceFetcher(
       TestResourceFetcherProperties* properties,
       FetchContext* context) {
-    return MakeGarbageCollected<ResourceFetcher>(ResourceFetcherInit(
+    ResourceFetcherInit init(
         properties->MakeDetachable(), context, CreateTaskRunner(),
         CreateTaskRunner(), MakeGarbageCollected<NoopLoaderFactory>(),
         MakeGarbageCollected<MockContextLifecycleNotifier>(),
-        nullptr /* back_forward_cache_loader_helper */));
+        /*back_forward_cache_loader_helper=*/nullptr);
+    use_counter_ = MakeGarbageCollected<testing::StrictMock<MockUseCounter>>();
+    init.use_counter = MakeGarbageCollected<DetachableUseCounter>(use_counter_);
+    return MakeGarbageCollected<ResourceFetcher>(std::move(init));
   }
 
+  MockUseCounter* UseCounter() const { return use_counter_; }
+
  private:
   base::test::SingleThreadTaskEnvironment task_environment_;
+  Persistent<MockUseCounter> use_counter_;
 };
 
 std::ostream& operator<<(std::ostream& o, const ResourceLoaderTest::From& f) {
@@ -443,6 +470,77 @@ TEST_F(ResourceLoaderTest, LoadDataURL_DefersAsyncAndStream) {
   EXPECT_FALSE(resource->ResourceBuffer());
 }
 
+namespace {
+
+bool WillFollowRedirect(ResourceLoader* loader, KURL new_url) {
+  auto response_head = network::mojom::URLResponseHead::New();
+  auto response =
+      WebURLResponse::Create(new_url, *response_head,
+                             /*report_security_info=*/true, /*request_id=*/1);
+  bool has_devtools_request_id = false;
+  std::vector<std::string> removed_headers;
+  return loader->WillFollowRedirect(
+      new_url, net::SiteForCookies(), /*new_referrer=*/String(),
+      network::mojom::ReferrerPolicy::kAlways, "GET", response,
+      has_devtools_request_id, &removed_headers,
+      /*insecure_scheme_was_upgraded=*/false);
+}
+
+}  // namespace
+
+TEST_F(ResourceLoaderTest, AuthorizationCrossOriginRedirect) {
+  auto* properties = MakeGarbageCollected<TestResourceFetcherProperties>();
+  FetchContext* context = MakeGarbageCollected<MockFetchContext>();
+  auto* fetcher = MakeResourceFetcher(properties, context);
+
+  KURL url("https://a.test/");
+  ResourceRequest request(url);
+  request.SetRequestContext(mojom::blink::RequestContextType::FETCH);
+  request.SetHttpHeaderField(net::HttpRequestHeaders::kAuthorization,
+                             "Basic foo");
+
+  FetchParameters params = FetchParameters::CreateForTest(std::move(request));
+  Resource* resource = RawResource::Fetch(params, fetcher, nullptr);
+  ResourceLoader* loader = resource->Loader();
+
+  // Redirect to the same origin. Expect no UseCounter call.
+  {
+    KURL new_url("https://a.test/foo");
+    ASSERT_TRUE(WillFollowRedirect(loader, new_url));
+    ::testing::Mock::VerifyAndClear(UseCounter());
+  }
+
+  // Redirect to a cross origin. Expect a single UseCounter call.
+  {
+    EXPECT_CALL(*UseCounter(),
+                CountUse(mojom::WebFeature::kAuthorizationCrossOrigin))
+        .Times(1);
+    KURL new_url("https://b.test");
+    ASSERT_TRUE(WillFollowRedirect(loader, new_url));
+    ::testing::Mock::VerifyAndClear(UseCounter());
+  }
+}
+
+TEST_F(ResourceLoaderTest, CrossOriginRedirect_NoAuthorization) {
+  auto* properties = MakeGarbageCollected<TestResourceFetcherProperties>();
+  FetchContext* context = MakeGarbageCollected<MockFetchContext>();
+  auto* fetcher = MakeResourceFetcher(properties, context);
+
+  KURL url("https://a.test/");
+  ResourceRequest request(url);
+  request.SetRequestContext(mojom::blink::RequestContextType::FETCH);
+
+  FetchParameters params = FetchParameters::CreateForTest(std::move(request));
+  Resource* resource = RawResource::Fetch(params, fetcher, nullptr);
+  ResourceLoader* loader = resource->Loader();
+
+  // Redirect to a cross origin without Authorization header. Expect no
+  // UseCounter call.
+  KURL new_url("https://b.test");
+  ASSERT_TRUE(WillFollowRedirect(loader, new_url));
+  ::testing::Mock::VerifyAndClear(UseCounter());
+}
+
 class ResourceLoaderIsolatedCodeCacheTest : public ResourceLoaderTest {
  protected:
   bool LoadAndCheckIsolatedCodeCache(ResourceResponse response) {

third_party/blink/renderer/platform/loader/fetch/url_loader/sync_load_context.cc

@@ -13,6 +13,7 @@
 #include "base/task/single_thread_task_runner.h"
 #include "base/time/time.h"
 #include "mojo/public/cpp/bindings/associated_remote.h"
+#include "net/http/http_request_headers.h"
 #include "net/url_request/redirect_info.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/url_loader_completion_status.h"
@@ -23,6 +24,7 @@
 #include "third_party/blink/public/platform/resource_load_info_notifier_wrapper.h"
 #include "third_party/blink/renderer/platform/loader/fetch/url_loader/sync_load_response.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
+#include "url/origin.h"
 
 namespace blink {
 
@@ -151,6 +153,9 @@ SyncLoadContext::SyncLoadContext(
   // Initialize the final URL with the original request URL. It will be
   // overwritten on redirects.
   response_->url = request->url;
+
+  has_authorization_header_ =
+      request->headers.HasHeader(net::HttpRequestHeaders::kAuthorization);
 }
 
 SyncLoadContext::~SyncLoadContext() {}
@@ -162,6 +167,12 @@ bool SyncLoadContext::OnReceivedRedirect(
     network::mojom::URLResponseHeadPtr head,
     std::vector<std::string>* removed_headers) {
   DCHECK(!Completed());
+
+  if (has_authorization_header_ &&
+      !url::IsSameOriginWith(response_->url, redirect_info.new_url)) {
+    response_->has_authorization_header_between_cross_origin_redirect_ = true;
+  }
+
   if (removed_headers) {
     // TODO(yoav): Get the actual PermissionsPolicy here to support selective
     // removal for sync XHR.

third_party/blink/renderer/platform/loader/fetch/url_loader/sync_load_context.h

@@ -145,6 +145,11 @@ class BLINK_PLATFORM_EXPORT SyncLoadContext : public WebRequestPeer {
   bool blob_finished_ = false;
   bool request_completed_ = false;
 
+  // True when the request contains Authorization header.
+  // TODO(https://crbug.com/1393520): Remove this field once we get enough
+  // stats to make a decision.
+  bool has_authorization_header_ = false;
+
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   class SignalHelper;

third_party/blink/renderer/platform/loader/fetch/url_loader/sync_load_response.h

@@ -56,6 +56,11 @@ struct BLINK_PLATFORM_EXPORT SyncLoadResponse {
 
   // Used for blob response type XMLHttpRequest.
   scoped_refptr<BlobDataHandle> downloaded_blob;
+
+  // True when cross origin redirects happen with Authorization header.
+  // TODO(https://crbug.com/1393520): Remove this field once we get enough
+  // stats to make a decision.
+  bool has_authorization_header_between_cross_origin_redirect_ = false;
 };
 
 }  // namespace blink

third_party/blink/renderer/platform/loader/fetch/url_loader/url_loader.cc

@@ -566,6 +566,11 @@ void URLLoader::LoadSynchronously(
     return;
   }
 
+  if (sync_load_response
+          .has_authorization_header_between_cross_origin_redirect_) {
+    client->CountFeature(mojom::WebFeature::kAuthorizationCrossOrigin);
+  }
+
   response =
       WebURLResponse::Create(final_url, *sync_load_response.head,
                              has_devtools_request_id, context_->request_id());

third_party/blink/renderer/platform/loader/fetch/url_loader/url_loader_client.h

@@ -37,6 +37,7 @@
 #include "mojo/public/cpp/base/big_buffer.h"
 #include "mojo/public/cpp/system/data_pipe.h"
 #include "services/network/public/mojom/referrer_policy.mojom-shared.h"
+#include "third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom-shared.h"
 #include "third_party/blink/public/platform/web_common.h"
 #include "third_party/blink/public/platform/web_url_request.h"
 #include "third_party/blink/public/platform/web_vector.h"
@@ -124,6 +125,11 @@ class BLINK_PLATFORM_EXPORT URLLoaderClient {
                        uint64_t total_encoded_body_length,
                        int64_t total_decoded_body_length) {}
 
+  // Counts a WebFeature use.
+  // TODO(https://crbug.com/1393520): Remove this method once we get enough
+  // stats to make a decision.
+  virtual void CountFeature(blink::mojom::WebFeature) {}
+
   // Value passed to DidFinishLoading when total encoded data length isn't
   // known.
   static const int64_t kUnknownEncodedDataLength = -1;

tools/metrics/histograms/enums.xml

@@ -42240,6 +42240,7 @@ Called by update_use_counter_feature_enum.py.-->
   <int value="4468" label="ServiceWorkerAddHandlerAfterInitialization"/>
   <int value="4469"
       label="ServiceWorkerSetAttributeHandlerAfterInitialization"/>
+  <int value="4470" label="AuthorizationCrossOrigin"/>
 </enum>
 
 <enum name="FeaturePolicyAllowlistType">