[Merge 116] Include unpartitioned cookie availability check in hasSto…

…rageAccess() This change is based on spec PR privacycg/storage-access#174. (cherry picked from commit 120b35b) Bug: 1433013 Change-Id: I6c29b2a2afddb288d40d946040dc73fbe76b6fcb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4611289 Reviewed-by: Mason Freed <[email protected]> Commit-Queue: Shuran Huang <[email protected]> Reviewed-by: Chris Fredrickson <[email protected]> Cr-Original-Commit-Position: refs/heads/main@{#1161766} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4665808 Auto-Submit: Shuran Huang <[email protected]> Reviewed-by: Dominic Farolino <[email protected]> Commit-Queue: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/branch-heads/5845@{#322} Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
qtmlabs · Jul 5, 2023 · c37d6ca · c37d6ca
1 parent 2c1cd95
commit c37d6ca
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 13 deletions.
diff --git a/chrome/browser/storage_access_api/api_browsertest.cc b/chrome/browser/storage_access_api/api_browsertest.cc
@@ -38,6 +38,7 @@
 #include "components/policy/policy_constants.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/content_paths.h"
 #include "content/public/test/browser_task_environment.h"
@@ -622,13 +623,12 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB),
             CookieBundleWithContent("cross-site=b.test"));
 
-  // TODO(https://crbug.com/1441133): We should either make sure there is way to
-  // let developer check whether they need to call rSA(), or no prompt is shown
-  // when 3p cookie is allowed.
-  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  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());
 }
 
@@ -835,7 +835,10 @@ IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
   prompt_factory()->set_response_type(
       permissions::PermissionRequestManager::ACCEPT_ALL);
-  EXPECT_TRUE(storage::test::RequestAndCheckStorageAccessForFrame(GetFrame()));
+  // TODO(https://crbug.com/1441133): requestStorageAccess() should be rejected
+  // when 3p cookie is blocked by user explicitly.
+  EXPECT_TRUE(content::ExecJs(GetFrame(), "document.requestStorageAccess()"));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
 
   EXPECT_EQ(ReadCookies(GetFrame(), kHostB), NoCookies());
 }
@@ -857,9 +860,11 @@ IN_PROC_BROWSER_TEST_F(
 
   prompt_factory()->set_response_type(
       permissions::PermissionRequestManager::ACCEPT_ALL);
-
+  // TODO(https://crbug.com/1441133): requestStorageAccess() should be rejected
+  // when 3p cookie is blocked by user explicitly.
   EXPECT_TRUE(
-      storage::test::RequestAndCheckStorageAccessForFrame(GetNestedFrame()));
+      content::ExecJs(GetNestedFrame(), "document.requestStorageAccess()"));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
 
   EXPECT_EQ(ReadCookies(GetNestedFrame(), kHostB), NoCookies());
 }
@@ -883,9 +888,11 @@ IN_PROC_BROWSER_TEST_F(
 
   prompt_factory()->set_response_type(
       permissions::PermissionRequestManager::ACCEPT_ALL);
-
+  // TODO(https://crbug.com/1441133): requestStorageAccess() should be rejected
+  // when 3p cookie is blocked by user explicitly.
   EXPECT_TRUE(
-      storage::test::RequestAndCheckStorageAccessForFrame(GetNestedFrame()));
+      content::ExecJs(GetNestedFrame(), "document.requestStorageAccess()"));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
 
   EXPECT_EQ(ReadCookies(GetNestedFrame(), kHostC), NoCookies());
 }
@@ -1533,7 +1540,7 @@ IN_PROC_BROWSER_TEST_P(StorageAccessAPIStorageBrowserTest, MultiTabTest) {
   storage::test::ExpectCrossTabInfoForFrame(GetFrame(), false);
   storage::test::SetCrossTabInfoForFrame(GetFrame());
   storage::test::ExpectCrossTabInfoForFrame(GetFrame(), true);
-  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
 
   // Create a second tab to test communication between tabs.
   NavigateToNewTabWithFrame(kHostA);
@@ -1545,7 +1552,7 @@ IN_PROC_BROWSER_TEST_P(StorageAccessAPIStorageBrowserTest, MultiTabTest) {
           permissions::PermissionRequestManager::ACCEPT_ALL);
 
   storage::test::ExpectCrossTabInfoForFrame(GetFrame(), true);
-  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
 
   SetBlockThirdPartyCookies(true);
 
@@ -1879,6 +1886,7 @@ IN_PROC_BROWSER_TEST_P(StorageAccessAPIEnterprisePolicyBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
                        EnsureOnePromptDenialSuffices) {
+  SetBlockThirdPartyCookies(true);
   NavigateToPageWithFrame(kHostA);
   NavigateFrameTo(EchoCookiesURL(kHostB));
 
@@ -1912,6 +1920,7 @@ IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
                        DismissalAllowsFuturePrompts) {
+  SetBlockThirdPartyCookies(true);
   NavigateToPageWithFrame(kHostA);
   NavigateFrameTo(EchoCookiesURL(kHostB));
 
@@ -2096,4 +2105,50 @@ 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.
+class StorageAccessAPIWith3PCEnabledBrowserTest
+    : public StorageAccessAPIBaseBrowserTest {
+ public:
+  StorageAccessAPIWith3PCEnabledBrowserTest()
+      : StorageAccessAPIBaseBrowserTest(/*is_storage_partitioned=*/false) {}
+};
+
+IN_PROC_BROWSER_TEST_F(StorageAccessAPIWith3PCEnabledBrowserTest,
+                       AllowedWhenUnblocked) {
+  SetBlockThirdPartyCookies(false);
+
+  NavigateToPageWithFrame(kHostA);
+  NavigateFrameTo(EchoCookiesURL(kHostB));
+
+  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+
+  EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB),
+            CookieBundleWithContent("cross-site=b.test"));
+}
+
+IN_PROC_BROWSER_TEST_F(StorageAccessAPIWith3PCEnabledBrowserTest,
+                       AllowedByUserBypass) {
+  SetBlockThirdPartyCookies(true);
+
+  NavigateToPageWithFrame(kHostA);
+  NavigateFrameTo(EchoCookiesURL(kHostB));
+  EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB), NoCookiesWithContent());
+
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+
+  // Enable UserBypass on hostA as top-level.
+  CookieSettingsFactory::GetForProfile(browser()->profile())
+      ->SetCookieSettingForUserBypass(GetURL(kHostA));
+
+  EXPECT_TRUE(storage::test::HasStorageAccessForFrame(GetFrame()));
+
+  NavigateToPageWithFrame(kHostA);
+  NavigateFrameTo(EchoCookiesURL(kHostB));
+  EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB),
+            CookieBundleWithContent("cross-site=b.test"));
+}
+
+// TODO(crbug.com/1448957): Add test cases of 3PC enabled by other mechanisms.
+
 }  // namespace
diff --git a/third_party/blink/renderer/core/dom/document.cc b/third_party/blink/renderer/core/dom/document.cc
@@ -6272,8 +6272,9 @@ ScriptPromise Document::hasStorageAccess(ScriptState* script_state) {
       return true;
     }
 
-    // #9: return global's `has storage access`.
-    return dom_window_->HasStorageAccess();
+    // #9 & #10: checks unpartitioned cookie availability with global's `has
+    // storage access`.
+    return CookiesEnabled();
   }());
   return promise;
 }

diff --git a/...arty/blink/web_tests/external/wpt/storage-access-api/hasStorageAccess.sub.https.window.js b/...arty/blink/web_tests/external/wpt/storage-access-api/hasStorageAccess.sub.https.window.js
@@ -1,4 +1,6 @@
 // META: script=helpers.js
+// META: script=/resources/testdriver.js
+// META: script=/resources/testdriver-vendor.js
 'use strict';
 
 const {testPrefix, topLevelDocument} = processQueryParams();
@@ -9,6 +11,7 @@ promise_test(async () => {
 }, "[" + testPrefix + "] document.hasStorageAccess() should exist on the document interface");
 
 promise_test(async () => {
+  await MaybeSetStorageAccess("*", "*", "blocked");
   const hasAccess = await document.hasStorageAccess();
   if (topLevelDocument || testPrefix.includes('same-origin')) {
     assert_true(hasAccess, "Access should be granted in top-level frame or iframe that is in first-party context by default.");

diff --git a/...eb_tests/external/wpt/storage-access-api/resources/hasStorageAccess-ABA-iframe.https.html b/...eb_tests/external/wpt/storage-access-api/resources/hasStorageAccess-ABA-iframe.https.html
@@ -2,6 +2,8 @@
 <meta charset=utf-8>
 
 <script src="/resources/testharness.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
 <script src="/storage-access-api/helpers.js"></script>
 <body>
 <script src="/storage-access-api/resources/hasStorageAccess-ABA-iframe.sub.https.window.js"></script>
diff --git a/...external/wpt/storage-access-api/resources/hasStorageAccess-ABA-iframe.sub.https.window.js b/...external/wpt/storage-access-api/resources/hasStorageAccess-ABA-iframe.sub.https.window.js
@@ -1,4 +1,6 @@
 // META: script=../helpers.js
+// META: script=/resources/testdriver.js
+// META: script=/resources/testdriver-vendor.js
 'use strict';
 
 // This expects to be run in an iframe that is cross-site to the top-level frame.

diff --git a/...nk/web_tests/external/wpt/storage-access-api/resources/hasStorageAccess-iframe.https.html b/...nk/web_tests/external/wpt/storage-access-api/resources/hasStorageAccess-iframe.https.html
@@ -2,6 +2,8 @@
 <meta charset=utf-8>
 
 <script src="/resources/testharness.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
 <!-- no testharnessreport.js -->
 <script src="../helpers.js"></script>
 <div id=log></div>