Storage Access API: Enable per-page storage access scope and align te…

…st cases https://bugs.webkit.org/show_bug.cgi?id=217077 <rdar://problem/69017878> Reviewed by Brent Fulgham. It was decided in privacycg/storage-access#3 that browsers should grant storage access for all same-site resources on the whole page, not just the requesting iframe. Source/WebCore: No new tests. Existing tests aligned, including LayoutTests/http/tests/storageAccess/request-and-grant-access-with-per-page-scope-access-from-another-frame.html which no longer calls internals.settings.setStorageAccessAPIPerPageScopeEnabled(true) to opt in to the setting that is now the default. * dom/DocumentStorageAccess.cpp: (WebCore::DocumentStorageAccess::requestStorageAccess): Changed the settings check to do the reverse, i.e. assume per-page access by default and per-frame access as the exception. * dom/DocumentStorageAccess.h: Changed the default setting for m_storageAccessScope to per-page. * page/Settings.yaml: Changed the default to true for storageAccessAPIPerPageScopeEnabled. Source/WebKit: * WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::hasStorageAccess): Added a call to WebPage::hasPageLevelStorageAccess() and an early return if it returns true. (WebKit::WebPage::clearPageLevelStorageAccess): New function to clear the webpage's m_domainsWithPageLevelStorageAccess map. * WebProcess/WebPage/WebPage.h: * WebProcess/WebProcess.cpp: (WebKit::WebProcess::clearResourceLoadStatistics): Now clears all webpages' page level storage access maps. LayoutTests: * http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-user-interaction-and-access-from-right-frame.html: * http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-user-interaction-but-access-from-wrong-frame.html: * http/tests/storageAccess/request-and-grant-access-then-detach-should-not-have-access.html: * http/tests/storageAccess/request-and-grant-access-then-navigate-cross-site-should-not-have-access.html: All of the above now explicitly call internals.settings.setStorageAccessAPIPerPageScopeEnabled(false) to opt out of the new default setting. * http/tests/storageAccess/request-and-grant-access-with-per-page-scope-access-from-another-frame.html: Changed to assume per-page is the default. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@267973 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima · Oct 5, 2020 · 28ffd8a · 28ffd8a
1 parent 20becca
commit 28ffd8a
Show file tree

Hide file tree

Showing 14 changed files with 109 additions and 6 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,25 @@
+2020-10-05  John Wilander  <[email protected]>
+
+        Storage Access API: Enable per-page storage access scope and align test cases
+        https://bugs.webkit.org/show_bug.cgi?id=217077
+        <rdar://problem/69017878>
+
+        Reviewed by Brent Fulgham.
+
+        It was decided in https://github.com/privacycg/storage-access/issues/3 that
+        browsers should grant storage access for all same-site resources on the whole
+        page, not just the requesting iframe.
+
+        * http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-user-interaction-and-access-from-right-frame.html:
+        * http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-user-interaction-but-access-from-wrong-frame.html:
+        * http/tests/storageAccess/request-and-grant-access-then-detach-should-not-have-access.html:
+        * http/tests/storageAccess/request-and-grant-access-then-navigate-cross-site-should-not-have-access.html:
+            All of the above now explicitly call
+            internals.settings.setStorageAccessAPIPerPageScopeEnabled(false)
+            to opt out of the new default setting.
+        * http/tests/storageAccess/request-and-grant-access-with-per-page-scope-access-from-another-frame.html:
+            Changed to assume per-page is the default.
+
 2020-10-05  Diego Pino Garcia  <[email protected]>
 
         [GLIB] Unreviewed test gardening. Merge common GTK and WPE baselines and emit new baselines for WPE tests after r267960.

diff --git a/...boxed-iframe-from-prevalent-domain-with-user-interaction-and-access-from-right-frame.html b/...boxed-iframe-from-prevalent-domain-with-user-interaction-and-access-from-right-frame.html
@@ -105,6 +105,8 @@
                     break;
                 case "#step6":
                     document.location.hash = "step7";
+                    // Set per-frame access since clearing access on cross-site navigation of the iframe requires per-frame access.
+                    internals.settings.setStorageAccessAPIPerPageScopeEnabled(false);
                     // Create iframe that will request storage access.
                     let iframeElement = document.createElement("iframe");
                     iframeElement.setAttribute("sandbox", "allow-storage-access-by-user-activation allow-scripts allow-same-origin allow-modals");
@@ -143,6 +145,8 @@
                     existingIframe3.src = thirdPartyBaseUrl + subPathToGetCookies + "&message=After the top frame navigates the sub frame cross-site and back, the sub frame should no longer have access to cookies.";
                     break;
                 case "#step10":
+                    // Reset access scope.
+                    internals.settings.setStorageAccessAPIPerPageScopeEnabled(true);
                     setEnableFeature(false, finishJSTest);
                     break;
             }

diff --git a/...boxed-iframe-from-prevalent-domain-with-user-interaction-but-access-from-wrong-frame.html b/...boxed-iframe-from-prevalent-domain-with-user-interaction-but-access-from-wrong-frame.html
@@ -105,6 +105,8 @@
                     break;
                 case "#step6":
                     document.location.hash = "step7";
+                    // Set per-frame access since otherwise the sibling iframe will get access.
+                    internals.settings.setStorageAccessAPIPerPageScopeEnabled(false);
                     let iframeElement = document.createElement("iframe");
                     iframeElement.setAttribute("sandbox", "allow-storage-access-by-user-activation allow-scripts allow-same-origin allow-modals");
                     iframeElement.onload = function() {
@@ -122,6 +124,8 @@
                     openIframe(thirdPartyBaseUrl + subPathToGetCookies + "&message=Should not receive cookies.", runTest);
                     break;
                 case "#step8":
+                    // Reset access scope.
+                    internals.settings.setStorageAccessAPIPerPageScopeEnabled(true);
                     setEnableFeature(false, finishJSTest);
                     break;
             }

diff --git a/...http/tests/storageAccess/request-and-grant-access-then-detach-should-not-have-access.html b/...http/tests/storageAccess/request-and-grant-access-then-detach-should-not-have-access.html
@@ -11,6 +11,8 @@
         window.addEventListener("message", receiveMessage, false);
 
         function finishTest() {
+            // Reset access scope.
+            internals.settings.setStorageAccessAPIPerPageScopeEnabled(true);
             setEnableFeature(false, finishJSTest);
         }
 
@@ -93,6 +95,9 @@
                 document.location.href = statisticsUrl + "/storageAccess/resources/set-cookie.php?name=firstPartyCookie&value=value#http://127.0.0.1:8000/storageAccess/request-and-grant-access-then-detach-should-not-have-access.html#firstPartyCookieSet";
             });
         } else {
+            // Set per-frame access since clearing access on detaching the iframe requires per-frame access.
+            internals.settings.setStorageAccessAPIPerPageScopeEnabled(false);
+
             testRunner.setStatisticsPrevalentResource(statisticsUrl, true, function() {
                 if (!testRunner.isStatisticsPrevalentResource(statisticsUrl))
                     testFailed("Host did not get set as prevalent resource.");

diff --git a/...orageAccess/request-and-grant-access-then-navigate-cross-site-should-not-have-access.html b/...orageAccess/request-and-grant-access-then-navigate-cross-site-should-not-have-access.html
@@ -13,6 +13,8 @@
         window.addEventListener("message", receiveMessage, false);
 
         function finishTest() {
+            // Reset access scope.
+            internals.settings.setStorageAccessAPIPerPageScopeEnabled(true);
             setEnableFeature(false, finishJSTest);
         }
 
@@ -68,6 +70,8 @@
                 document.location.href = statisticsUrl + "/storageAccess/resources/set-cookie.php?name=firstPartyCookie&value=value#http://127.0.0.1:8000/storageAccess/request-and-grant-access-then-navigate-cross-site-should-not-have-access.html#firstPartyCookieSet";
             });
         } else {
+            // Set per-frame access since clearing access on cross-site navigation of the iframe requires per-frame access.
+            internals.settings.setStorageAccessAPIPerPageScopeEnabled(false);
             testRunner.setStatisticsPrevalentResource(statisticsUrl, true, function() {
                 if (!testRunner.isStatisticsPrevalentResource(statisticsUrl))
                     testFailed("Host did not get set as prevalent resource.");

diff --git a/...storageAccess/request-and-grant-access-with-per-page-scope-access-from-another-frame.html b/...storageAccess/request-and-grant-access-with-per-page-scope-access-from-another-frame.html
@@ -83,7 +83,6 @@
                     break;
                 case "#step4":
                     document.location.hash = "step5";
-                    internals.settings.setStorageAccessAPIPerPageScopeEnabled(true);
                     let iframeElement = document.createElement("iframe");
                     iframeElement.onload = function() {
                         activateElement("TheIframeThatRequestsStorageAccess");
@@ -99,7 +98,6 @@
                     break;
                 case "#step6":
                     testRunner.setStatisticsShouldBlockThirdPartyCookies(false, function() {
-                        internals.settings.setStorageAccessAPIPerPageScopeEnabled(false);
                         setEnableFeature(false, finishJSTest);
                     });
                     break;

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2020-10-05  John Wilander  <[email protected]>
+
+        Storage Access API: Enable per-page storage access scope and align test cases
+        https://bugs.webkit.org/show_bug.cgi?id=217077
+        <rdar://problem/69017878>
+
+        Reviewed by Brent Fulgham.
+
+        It was decided in https://github.com/privacycg/storage-access/issues/3 that
+        browsers should grant storage access for all same-site resources on the whole
+        page, not just the requesting iframe.
+
+        No new tests. Existing tests aligned, including
+        LayoutTests/http/tests/storageAccess/request-and-grant-access-with-per-page-scope-access-from-another-frame.html
+        which no longer calls
+        internals.settings.setStorageAccessAPIPerPageScopeEnabled(true)
+        to opt in to the setting that is now the default.
+
+        * dom/DocumentStorageAccess.cpp:
+        (WebCore::DocumentStorageAccess::requestStorageAccess):
+            Changed the settings check to do the reverse, i.e. assume
+            per-page access by default and per-frame access as the
+            exception.
+        * dom/DocumentStorageAccess.h:
+            Changed the default setting for m_storageAccessScope to per-page.
+        * page/Settings.yaml:
+            Changed the default to true for
+            storageAccessAPIPerPageScopeEnabled.
+
 2020-10-05  Sam Weinig  <[email protected]>
 
         Remove support for enabling subpixel CSSOM values, it's off by default everywhere and known to be not-compatible with the web

diff --git a/Source/WebCore/dom/DocumentStorageAccess.cpp b/Source/WebCore/dom/DocumentStorageAccess.cpp
@@ -197,8 +197,8 @@ void DocumentStorageAccess::requestStorageAccess(Ref<DeferredPromise>&& promise)
         return;
     }
 
-    if (page->settings().storageAccessAPIPerPageScopeEnabled())
-        m_storageAccessScope = StorageAccessScope::PerPage;
+    if (!page->settings().storageAccessAPIPerPageScopeEnabled())
+        m_storageAccessScope = StorageAccessScope::PerFrame;
 
     page->chrome().client().requestStorageAccess(RegistrableDomain::uncheckedCreateFromHost(m_document.securityOrigin().host()), RegistrableDomain::uncheckedCreateFromHost(m_document.topDocument().securityOrigin().host()), *frame, m_storageAccessScope, [this, weakThis = makeWeakPtr(*this), promise = WTFMove(promise)] (RequestStorageAccessResult result) mutable {
         if (!weakThis)

diff --git a/Source/WebCore/dom/DocumentStorageAccess.h b/Source/WebCore/dom/DocumentStorageAccess.h
@@ -109,7 +109,7 @@ class DocumentStorageAccess final : public Supplement<Document>, public CanMakeW
 
     uint8_t m_numberOfTimesExplicitlyDeniedFrameSpecificStorageAccess = 0;
 
-    StorageAccessScope m_storageAccessScope = StorageAccessScope::PerFrame;
+    StorageAccessScope m_storageAccessScope = StorageAccessScope::PerPage;
 };
 
 template<class Encoder>

diff --git a/Source/WebCore/page/Settings.yaml b/Source/WebCore/page/Settings.yaml
@@ -701,7 +701,7 @@ storageAccessAPIEnabled:
   initial: true
 
 storageAccessAPIPerPageScopeEnabled:
-  initial: false
+  initial: true
 
 timeWithoutMouseMovementBeforeHidingControls:
   type: Seconds

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,27 @@
+2020-10-05  John Wilander  <[email protected]>
+
+        Storage Access API: Enable per-page storage access scope and align test cases
+        https://bugs.webkit.org/show_bug.cgi?id=217077
+        <rdar://problem/69017878>
+
+        Reviewed by Brent Fulgham.
+
+        It was decided in https://github.com/privacycg/storage-access/issues/3 that
+        browsers should grant storage access for all same-site resources on the whole
+        page, not just the requesting iframe.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::hasStorageAccess):
+            Added a call to WebPage::hasPageLevelStorageAccess()
+            and an early return if it returns true.
+        (WebKit::WebPage::clearPageLevelStorageAccess):
+            New function to clear the webpage's m_domainsWithPageLevelStorageAccess
+            map.
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::clearResourceLoadStatistics):
+            Now clears all webpages' page level storage access maps.
+
 2020-10-05  Sam Weinig  <[email protected]>
 
         Remove support for enabling subpixel CSSOM values, it's off by default everywhere and known to be not-compatible with the web

diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
@@ -6813,6 +6813,11 @@ void WebPage::frameBecameRemote(FrameIdentifier frameID, GlobalFrameIdentifier&&
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
 void WebPage::hasStorageAccess(RegistrableDomain&& subFrameDomain, RegistrableDomain&& topFrameDomain, WebFrame& frame, CompletionHandler<void(bool)>&& completionHandler)
 {
+    if (hasPageLevelStorageAccess(topFrameDomain, subFrameDomain)) {
+        completionHandler(true);
+        return;
+    }
+
     WebProcess::singleton().ensureNetworkProcessConnection().connection().sendWithAsyncReply(Messages::NetworkConnectionToWebProcess::HasStorageAccess(WTFMove(subFrameDomain), WTFMove(topFrameDomain), frame.frameID(), m_identifier), WTFMove(completionHandler));
 }
 
@@ -6844,6 +6849,11 @@ bool WebPage::hasPageLevelStorageAccess(const RegistrableDomain& topLevelDomain,
     return it != m_domainsWithPageLevelStorageAccess.end() && it->value == resourceDomain;
 }
 
+void WebPage::clearPageLevelStorageAccess()
+{
+    m_domainsWithPageLevelStorageAccess.clear();
+}
+
 void WebPage::wasLoadedWithDataTransferFromPrevalentResource()
 {
     auto* frame = mainFrame();

diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
@@ -1221,6 +1221,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
     void requestStorageAccess(WebCore::RegistrableDomain&& subFrameDomain, WebCore::RegistrableDomain&& topFrameDomain, WebFrame&, WebCore::StorageAccessScope, CompletionHandler<void(WebCore::RequestStorageAccessResult)>&&);
     bool hasPageLevelStorageAccess(const WebCore::RegistrableDomain& topLevelDomain, const WebCore::RegistrableDomain& resourceDomain) const;
     void addDomainWithPageLevelStorageAccess(const WebCore::RegistrableDomain& topLevelDomain, const WebCore::RegistrableDomain& resourceDomain);
+    void clearPageLevelStorageAccess();
     void wasLoadedWithDataTransferFromPrevalentResource();
     void didLoadFromRegistrableDomain(WebCore::RegistrableDomain&&);
     void clearLoadedSubresourceDomains();

diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp
@@ -1596,6 +1596,8 @@ void WebProcess::clearResourceLoadStatistics()
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
     if (auto* observer = ResourceLoadObserver::sharedIfExists())
         observer->clearState();
+    for (auto& page : m_pageMap.values())
+        page->clearPageLevelStorageAccess();
 #endif
 }