Remove auto-resolve to true cases from hasStorageAccess()

Since we should always take user settings into account. This change is based on spec PR privacycg/storage-access#174. Bug: 1433013 Change-Id: I76a4fe5841c6ac1c566cbb2fa34298832a6f7da0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4705048 Reviewed-by: Johann Hofmann <[email protected]> Reviewed-by: Chris Fredrickson <[email protected]> Commit-Queue: Shuran Huang <[email protected]> Reviewed-by: Dominic Farolino <[email protected]> Cr-Commit-Position: refs/heads/main@{#1174378}
chromium · Jul 24, 2023 · 65370e4 · 65370e4
1 parent 884299b
commit 65370e4
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 15 deletions.
diff --git a/chrome/browser/storage_access_api/api_browsertest.cc b/chrome/browser/storage_access_api/api_browsertest.cc
@@ -869,6 +869,58 @@ IN_PROC_BROWSER_TEST_F(
   EXPECT_EQ(ReadCookies(GetNestedFrame(), kHostC), NoCookies());
 }
 
+// Validate that user settings take precedence for the leaf in a A(B(A)) frame
+// tree.
+IN_PROC_BROWSER_TEST_F(
+    StorageAccessAPIBrowserTest,
+    ThirdPartyCookiesIFrameThirdPartyExceptions_CrossSiteAncestorChain) {
+  EnsureUserInteractionOn(kHostA);
+  SetBlockThirdPartyCookies(true);
+  BlockAllCookiesOnHost(kHostA);
+
+  NavigateToPageWithFrame(kHostA);
+  NavigateFrameTo(kHostB, "/iframe.html");
+  NavigateNestedFrameTo(EchoCookiesURL(kHostA));
+
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetPrimaryMainFrame()));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetNestedFrame()));
+
+  prompt_factory()->set_response_type(
+      permissions::PermissionRequestManager::ACCEPT_ALL);
+  // TODO(https://crbug.com/1441133): should EXPECT_FALSE here since 3p cookies
+  // are blocked by user explicitly.
+  EXPECT_TRUE(
+      content::ExecJs(GetNestedFrame(), "document.requestStorageAccess()"));
+
+  EXPECT_EQ(ReadCookiesAndContent(GetNestedFrame(), kHostA),
+            NoCookiesWithContent());
+}
+
+// Validate that user settings take precedence for the leaf in a A(A) frame
+// tree.
+IN_PROC_BROWSER_TEST_F(
+    StorageAccessAPIBrowserTest,
+    ThirdPartyCookiesIFrameThirdPartyExceptions_SameSiteAncestorChain) {
+  EnsureUserInteractionOn(kHostA);
+  SetBlockThirdPartyCookies(true);
+  BlockAllCookiesOnHost(kHostA);
+
+  NavigateToPageWithFrame(kHostA);
+  NavigateFrameTo(EchoCookiesURL(kHostA));
+
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetPrimaryMainFrame()));
+  EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
+  prompt_factory()->set_response_type(
+      permissions::PermissionRequestManager::ACCEPT_ALL);
+  // TODO(https://crbug.com/1441133): should EXPECT_FALSE here since user
+  // explicitly blocks cookies for hostA.
+  EXPECT_TRUE(content::ExecJs(GetFrame(), "document.requestStorageAccess()"));
+
+  EXPECT_EQ(0, prompt_factory()->TotalRequestCount());
+
+  EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostA), NoCookiesWithContent());
+}
+
 // Validates that once a grant is removed access is also removed.
 IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
                        ThirdPartyGrantsDeletedAccess) {
@@ -891,7 +943,7 @@ IN_PROC_BROWSER_TEST_F(StorageAccessAPIBrowserTest,
   // Verify cookie cannot be accessed.
   EXPECT_EQ(ReadCookies(GetFrame(), kHostB), NoCookies());
 
-  NavigateFrameTo(EchoCookiesURL(kHostB));
+  // Access change should be reflected immediately.
   EXPECT_EQ(ReadCookiesAndContent(GetFrame(), kHostB), NoCookiesWithContent());
   EXPECT_FALSE(storage::test::HasStorageAccessForFrame(GetFrame()));
 }

diff --git a/third_party/blink/renderer/core/dom/document.cc b/third_party/blink/renderer/core/dom/document.cc
@@ -6255,25 +6255,13 @@ ScriptPromise Document::hasStorageAccess(ScriptState* script_state) {
       return false;
     }
 
-    // #6: if doc's browsing context is a top-level browsing context, return
-    // true.
-    if (IsInOutermostMainFrame()) {
-      return true;
-    }
-
-    // #7: if the top-level origin of doc's relevant settings object is an
+    // #6: if the top-level origin of doc's relevant settings object is an
     // opaque origin, return false.
     if (TopFrameOrigin()->IsOpaque()) {
       return false;
     }
 
-    // #8: if doc's origin is in the first-party context with the top-level
-    // origin of doc's relevant settings object, return true.
-    if (!SiteForCookies().IsNull()) {
-      return true;
-    }
-
-    // #9 & #10: checks unpartitioned cookie availability with global's `has
+    // #7 - #10: checks unpartitioned cookie availability with global's `has
     // storage access`.
     return CookiesEnabled();
   }());