[Merge 116] Do not show SAA prompt when 3p cookie access is already a…

…llowed

(cherry picked from commit c43a0c6)

Bug: 1441133
Change-Id: Id318189ce744cc57de9fa555bd204e209a7e2aec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4650593
Reviewed-by: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Shuran Huang <shuuran@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1163921}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4670027
Commit-Queue: Avi Drissman <avi@chromium.org>
Auto-Submit: Shuran Huang <shuuran@chromium.org>
Cr-Commit-Position: refs/branch-heads/5845@{#350}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}

chrome/browser/storage_access_api/api_browsertest.cc

@@ -608,30 +608,6 @@ IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest, PermissionQueryCrossSite) {
   EXPECT_EQ(QueryPermission(GetFrame()), "prompt");
 }
 
-// When 3p cookie is allowed, check that in a A(B) frame tree, the embedded
-// B iframe can access cookie without requesting, but the prompt is still shown
-// if the iframe makes the request.
-IN_PROC_BROWSER_TEST_F(
-    StorageAccessAPIBrowserTest,
-    ThirdPartyCookiesAccess_CrossSiteIframe_AllowCrossSiteCookie) {
-  SetBlockThirdPartyCookies(false);
-
-  NavigateToPageWithFrame(kHostA);
-  NavigateFrameTo(EchoCookiesURL(kHostB));
-
-  // The cross-site iframe has cookie access since 3p cookies are allowed.
-  EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB),
-            CookieBundleWithContent("cross-site=b.test"));
-
-  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
-  prompt_factory()->set_response_type(
-      permissions::PermissionRequestManager::ACCEPT_ALL);
-  EXPECT_TRUE(storage::test::RequestAndCheckStorageAccessForFrame(GetFrame()));
-  // TODO(https://crbug.com/1441133): No prompt should be shown when 3p cookie
-  // is allowed.
-  EXPECT_EQ(1, prompt_factory()->TotalRequestCount());
-}
-
 // Validate that a cross-site iframe can bypass third-party cookie blocking via
 // the Storage Access API.
 IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
@@ -2105,8 +2081,9 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(ReadCookies(GetFrame(), kHostA), CookieBundle("cross-site=a.test"));
 }
 
-// Tests to verify that whether 3p cookie is already accessible is checked in
-// hasStorageAccess.
+// Tests to verify that when 3p cookie is allowed, the embedded iframe can
+// access cookie without requesting, and no prompt is shown if the iframe makes
+// the request.
 class StorageAccessAPIWith3PCEnabledBrowserTest
     : public StorageAccessAPIBaseBrowserTest {
  public:
@@ -2125,6 +2102,11 @@ IN_PROC_BROWSER_TEST_F(StorageAccessAPIWith3PCEnabledBrowserTest,
 
   EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB),
             CookieBundleWithContent("cross-site=b.test"));
+
+  prompt_factory()->set_response_type(
+      permissions::PermissionRequestManager::DISMISS);
+  EXPECT_TRUE(storage::test::RequestAndCheckStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(0, prompt_factory()->TotalRequestCount());
 }
 
 IN_PROC_BROWSER_TEST_F(StorageAccessAPIWith3PCEnabledBrowserTest,
@@ -2147,6 +2129,11 @@ IN_PROC_BROWSER_TEST_F(StorageAccessAPIWith3PCEnabledBrowserTest,
   NavigateFrameTo(EchoCookiesURL(kHostB));
   EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB),
             CookieBundleWithContent("cross-site=b.test"));
+
+  prompt_factory()->set_response_type(
+      permissions::PermissionRequestManager::DISMISS);
+  EXPECT_TRUE(storage::test::RequestAndCheckStorageAccessForFrame(GetFrame()));
+  EXPECT_EQ(0, prompt_factory()->TotalRequestCount());
 }
 
 // TODO(crbug.com/1448957): Add test cases of 3PC enabled by other mechanisms.

chrome/browser/storage_access_api/storage_access_grant_permission_context.cc

@@ -12,12 +12,14 @@
 #include "base/metrics/histogram_functions.h"
 #include "base/notreached.h"
 #include "base/ranges/algorithm.h"
+#include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/dips/dips_service.h"
 #include "chrome/browser/first_party_sets/first_party_sets_policy_service.h"
 #include "chrome/browser/first_party_sets/first_party_sets_policy_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/content_settings/browser/page_specific_content_settings.h"
+#include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_constraints.h"
@@ -31,6 +33,8 @@
 #include "content/public/common/content_features.h"
 #include "net/base/features.h"
 #include "net/base/schemeful_site.h"
+#include "net/cookies/cookie_setting_override.h"
+#include "net/cookies/site_for_cookies.h"
 #include "net/first_party_sets/first_party_set_entry.h"
 #include "net/first_party_sets/first_party_set_metadata.h"
 #include "net/first_party_sets/same_party_context.h"
@@ -99,6 +103,7 @@ content_settings::ContentSettingConstraints ComputeConstraints(
     case RequestOutcome::kDeniedByPrerequisites:
     case RequestOutcome::kReusedPreviousDecision:
     case RequestOutcome::kDeniedByTopLevelInteractionHeuristic:
+    case RequestOutcome::kAllowedByCookieSettings:
       NOTREACHED_NORETURN();
     case RequestOutcome::kGrantedByUser:
     case RequestOutcome::kDeniedByUser:
@@ -157,9 +162,30 @@ void StorageAccessGrantPermissionContext::DecidePermission(
     bool user_gesture,
     permissions::BrowserPermissionCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  CHECK(requesting_origin.is_valid());
+  CHECK(embedding_origin.is_valid());
+
+  content::RenderFrameHost* rfh =
+      content::RenderFrameHost::FromID(id.global_render_frame_host_id());
+  CHECK(rfh);
+
+  // Return early without prompting users if cookie access is already allowed.
+  // This does not take previously granted SAA permission into account.
+  scoped_refptr<content_settings::CookieSettings> cookie_settings =
+      CookieSettingsFactory::GetForProfile(
+          Profile::FromBrowserContext(browser_context()));
+  net::CookieSettingOverrides overrides = rfh->GetCookieSettingOverrides();
+  overrides.Remove(net::CookieSettingOverride::kStorageAccessGrantEligible);
+  if (cookie_settings->IsFullCookieAccessAllowed(
+          requesting_origin, net::SiteForCookies(),
+          url::Origin::Create(embedding_origin), overrides)) {
+    RecordOutcomeSample(RequestOutcome::kAllowedByCookieSettings);
+    std::move(callback).Run(CONTENT_SETTING_ALLOW);
+    return;
+  }
+
   if (!user_gesture ||
-      !base::FeatureList::IsEnabled(blink::features::kStorageAccessAPI) ||
-      !requesting_origin.is_valid() || !embedding_origin.is_valid()) {
+      !base::FeatureList::IsEnabled(blink::features::kStorageAccessAPI)) {
     RecordOutcomeSample(RequestOutcome::kDeniedByPrerequisites);
     std::move(callback).Run(CONTENT_SETTING_BLOCK);
     return;
@@ -175,10 +201,6 @@ void StorageAccessGrantPermissionContext::DecidePermission(
     return;
   }
 
-  content::RenderFrameHost* rfh =
-      content::RenderFrameHost::FromID(id.global_render_frame_host_id());
-  CHECK(rfh);
-
   net::SchemefulSite embedding_site(embedding_origin);
 
   first_party_sets::FirstPartySetsPolicyServiceFactory::GetForBrowserContext(

chrome/browser/storage_access_api/storage_access_grant_permission_context.h

@@ -45,7 +45,10 @@ enum class RequestOutcome {
   // The request was denied because the most recent top-level interaction on
   // the embedded site was too long ago, or there is no such interaction.
   kDeniedByTopLevelInteractionHeuristic = 8,
-  kMaxValue = kDeniedByTopLevelInteractionHeuristic,
+  // 3p cookies are already allowed by user agent, so there is no need to ask.
+  kAllowedByCookieSettings = 9,
+
+  kMaxValue = kAllowedByCookieSettings,
 };
 
 class StorageAccessGrantPermissionContext

chrome/browser/storage_access_api/storage_access_grant_permission_context_unittest.cc

@@ -17,13 +17,16 @@
 #include "chrome/browser/first_party_sets/scoped_mock_first_party_sets_handler.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "components/content_settings/browser/page_specific_content_settings.h"
+#include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/permissions/permission_request_id.h"
 #include "components/permissions/permission_request_manager.h"
 #include "components/permissions/permission_util.h"
 #include "components/permissions/test/mock_permission_prompt_factory.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
 #include "content/public/test/web_contents_tester.h"
@@ -120,6 +123,12 @@ class StorageAccessGrantPermissionContextTest
             permissions::PermissionRequestManager::FromWebContents(
                 web_contents()));
 
+    // Enable 3p cookie blocking.
+    profile()->GetPrefs()->SetInteger(
+        prefs::kCookieControlsMode,
+        static_cast<int>(
+            content_settings::CookieControlsMode::kBlockThirdParty));
+
     content_settings::PageSpecificContentSettings::CreateForWebContents(
         web_contents(),
         std::make_unique<chrome::PageSpecificContentSettingsDelegate>(
@@ -260,6 +269,28 @@ TEST_F(StorageAccessGrantPermissionContextAPIDisabledTest, PermissionBlocked) {
               IsEmpty());
 }
 
+// When 3p cookie access is already allowed by user-agent-specific cookie
+// settings, request should be allowed even when the Storage Access API feature
+// is disabled.
+TEST_F(StorageAccessGrantPermissionContextAPIDisabledTest,
+       AllowedByCookieSettings) {
+  base::HistogramTester histogram_tester;
+  // Allow 3p cookies.
+  profile()->GetPrefs()->SetInteger(
+      prefs::kCookieControlsMode,
+      static_cast<int>(content_settings::CookieControlsMode::kOff));
+
+  // User gesture is not needed.
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            DecidePermissionSync(/*user_gesture=*/false));
+  histogram_tester.ExpectUniqueSample(
+      kRequestOutcomeHistogram, RequestOutcome::kAllowedByCookieSettings, 1);
+
+  EXPECT_THAT(page_specific_content_settings()->GetTwoSiteRequests(
+                  ContentSettingsType::STORAGE_ACCESS),
+              IsEmpty());
+}
+
 class StorageAccessGrantPermissionContextAPIEnabledTest
     : public StorageAccessGrantPermissionContextTest {
  public:
@@ -359,6 +390,27 @@ TEST_F(StorageAccessGrantPermissionContextAPIEnabledTest,
                 .content_setting);
 }
 
+// When 3p cookie access is already allowed by user-agent-specific cookie
+// settings, request should be allowed without granting an explicit storage
+// access permission.
+TEST_F(StorageAccessGrantPermissionContextAPIEnabledTest,
+       AllowedByCookieSettings) {
+  // Allow 3p cookies.
+  profile()->GetPrefs()->SetInteger(
+      prefs::kCookieControlsMode,
+      static_cast<int>(content_settings::CookieControlsMode::kOff));
+
+  // User gesture is not needed.
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            DecidePermissionSync(/*user_gesture=*/false));
+  histogram_tester().ExpectUniqueSample(
+      kRequestOutcomeHistogram, RequestOutcome::kAllowedByCookieSettings, 1);
+
+  EXPECT_THAT(page_specific_content_settings()->GetTwoSiteRequests(
+                  ContentSettingsType::STORAGE_ACCESS),
+              IsEmpty());
+}
+
 class StorageAccessGrantPermissionContextAPIWithImplicitGrantsTest
     : public StorageAccessGrantPermissionContextAPIEnabledTest {
  public:

content/browser/renderer_host/render_frame_host_impl.h

@@ -2176,6 +2176,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
 
   bool IsLastCrossDocumentNavigationStartedByUser() const override;
 
+  net::CookieSettingOverrides GetCookieSettingOverrides() override;
+
   bool is_fenced_frame_root_originating_from_opaque_url() const {
     return is_fenced_frame_root_originating_from_opaque_url_;
   }
@@ -2913,11 +2915,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // used on primary main frames.
   bool IsPageReadyToBeClosed();
 
-  // Checks Blink runtime-enabled features (BREF) to create and return
-  // a CookieSettingOverrides pertaining to the last committed document in the
-  // frame. Can only be called on a frame with a committed navigation.
-  net::CookieSettingOverrides GetCookieSettingOverrides();
-
   // When this returns true, the user has specified that third-party cookies
   // should remain enabled for this frame.
   // It does so by checking the Blink runtime-enabled feature (BREF) for

content/public/browser/render_frame_host.h

@@ -21,6 +21,7 @@
 #include "content/public/common/isolated_world_ids.h"
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sender.h"
+#include "net/cookies/cookie_setting_override.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
 #include "services/network/public/mojom/web_sandbox_flags.mojom-forward.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -1026,6 +1027,11 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
   // transient user activation
   virtual bool IsLastCrossDocumentNavigationStartedByUser() const = 0;
 
+  // Checks Blink runtime-enabled features (BREF) to create and return
+  // a CookieSettingOverrides pertaining to the last committed document in the
+  // frame. Can only be called on a frame with a committed navigation.
+  virtual net::CookieSettingOverrides GetCookieSettingOverrides() = 0;
+
  private:
   // This interface should only be implemented inside content.
   friend class RenderFrameHostImpl;