Fix PushManager in ephemeral sessions

https://bugs.webkit.org/show_bug.cgi?id=241934 Reviewed by Geoffrey Garen. PushManager does not behave correctly when invoked in an ephemeral session. For instance: - PushManager.subscribe and PushManager.getSubscription fail with an AbortError. - PushManager.permissionState returns the persistent session's notification permissions for the given origin. This patch makes PushManager in ephemeral sessions behave as if a user denied notification permissions for all origins (which users can already do in persistent sessions via the appropriate checkbox). This also prevents these APIs from being used to detect private browsing mode. * LayoutTests/http/tests/push-api/permissions-ephemeral-expected.txt: Added. * LayoutTests/http/tests/push-api/permissions-ephemeral.html: Added. * LayoutTests/http/tests/push-api/resources/subscribe-tests.js * LayoutTests/http/tests/push-api/resources/subscribe-worker.js * LayoutTests/platform/gtk/TestExpectations: * LayoutTests/platform/ios/TestExpectations: * LayoutTests/platform/mac-wk1/TestExpectations: * LayoutTests/platform/win/TestExpectations: * Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.cpp: (WebKit::NetworkNotificationManager::getPushSubscription): (WebKit::NetworkNotificationManager::getPushPermissionState): * Source/WebKit/WebProcess/WebCoreSupport/WebNotificationClient.cpp: (WebKit::WebNotificationClient::requestPermission): (WebKit::WebNotificationClient::checkPermission): Canonical link: https://commits.webkit.org/251831@main
WebKit · Jun 24, 2022 · 0570dfb680f59ef121a7faa71d1e8e6297f3594f · 0570dfb
1 parent 5b9ecab
commit 0570dfb680f59ef121a7faa71d1e8e6297f3594f
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 30 deletions.
diff --git a/LayoutTests/http/tests/push-api/permissions-ephemeral-expected.txt b/LayoutTests/http/tests/push-api/permissions-ephemeral-expected.txt
@@ -0,0 +1,8 @@
+PASS: service worker permissionState was denied
+PASS: document permissionState was denied
+PASS: service worker subscribe was error: NotAllowedError
+PASS: document subscribe without user gesture was error: NotAllowedError
+PASS: document subscribe with user gesture was error: NotAllowedError
+PASS: document getSubscription was false
+PASS: service worker getSubscription was false
+
diff --git a/LayoutTests/http/tests/push-api/permissions-ephemeral.html b/LayoutTests/http/tests/push-api/permissions-ephemeral.html
@@ -0,0 +1,33 @@
+<!-- webkit-test-runner [ useEphemeralSession=true ] -->
+<html>
+<head>
+<script src="resources/push-api-test-pre.js"></script>
+<script src="resources/subscribe-tests.js"></script>
+</head>
+<body>
+<script>
+// Even if the UIProcess has granted permissions for notifications for a regular session,
+// they should not apply to this ephemeral session.
+if (window.testRunner)
+    testRunner.grantWebNotificationPermission(window.origin);
+
+navigator.serviceWorker.register("resources/subscribe-worker.js", { }).then(async (registration) => {
+    try {
+        await waitForState(registration.installing, "activated");
+        await testServiceWorkerPermissionState(registration, 'denied');
+        await testDocumentPermissionState(registration, 'denied');
+        await testServiceWorkerSubscribe(registration, 'NotAllowedError');
+        await testDocumentSubscribeWithoutUserGesture(registration, 'NotAllowedError');
+        await testDocumentSubscribeWithUserGesture(registration, 'NotAllowedError');
+        await testDocumentGetSubscription(registration, false);
+        await testServiceWorkerGetSubscription(registration, false);
+    } catch (e) {
+        log(`FAIL: unexpected exception ${e}`);
+    } finally {
+        await registration.unregister();
+        finishPushAPITest();
+    }
+});
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/push-api/resources/subscribe-tests.js b/LayoutTests/http/tests/push-api/resources/subscribe-tests.js
@@ -23,6 +23,36 @@ async function testDocumentPermissionState(registration, expected)
         log(`FAIL: document permissionState should be ${expected}, but was ${state}`);
 }
 
+async function testDocumentGetSubscription(registration, expected)
+{
+    let result = false;
+    try {
+        let subscription = await registration.pushManager.getSubscription();
+        result = !!subscription;
+    } catch (e) {
+        log(`FAIL: document getSubscription failed with ${e}`);
+        return;
+    }
+
+    if (result == expected)
+        log(`PASS: document getSubscription was ${expected}`);
+    else
+        log(`FAIL: document getSubscription should be ${expected}, but was ${result}`);
+}
+
+async function testServiceWorkerGetSubscription(registration, expected)
+{
+    let promise = new Promise(resolve => navigator.serviceWorker.onmessage = resolve);
+    registration.active.postMessage(['getSubscription']);
+    let event = await promise;
+    let result = event.data;
+
+    if (result == expected)
+        log(`PASS: service worker getSubscription was ${expected}`);
+    else
+        log(`FAIL: service worker getSubscription should be ${expected}, but was ${result}`);
+}
+
 async function testServiceWorkerSubscribe(registration, domExceptionName)
 {
     let expected = domExceptionName ? `error: ${domExceptionName}` : "successful";
@@ -99,4 +129,4 @@ async function testDocumentSubscribeImpl(registration, domExceptionName, domMess
         log(`PASS: document subscribe ${gestureDescription} was ${expected}`);
     else
         log(`FAIL: document subscribe ${gestureDescription} should be ${expected}, but was ${result}`);
-}
+}
diff --git a/LayoutTests/http/tests/push-api/resources/subscribe-worker.js b/LayoutTests/http/tests/push-api/resources/subscribe-worker.js
@@ -10,6 +10,13 @@ self.addEventListener('message', async (event) => {
         } catch (e) {
             event.source.postMessage("error: " + e);
         }
+    } else if (op == 'getSubscription') {
+        try {
+            let subscription = await self.registration.pushManager.getSubscription();
+            event.source.postMessage(!!subscription);
+        } catch (e) {
+            event.source.postMessage("error: " + e);
+        }
     } else if (op == 'subscribe') {
         let subscription = null;
         let result = null;

diff --git a/LayoutTests/platform/gtk/TestExpectations b/LayoutTests/platform/gtk/TestExpectations
@@ -1868,12 +1868,7 @@ imported/w3c/web-platform-tests/html/canvas/element/manual/drawing-images-to-the
 imported/w3c/web-platform-tests/xhr/preserve-ua-header-on-redirect.htm [ Failure ]
 
 # Push subscription tests fail without platform-specific PushCrypto implementations.
-http/tests/push-api/subscribe-default-permissions-iframe-cross-origin.html [ Failure ]
-http/tests/push-api/subscribe-default-permissions-iframe-same-origin.html [ Failure ]
-http/tests/push-api/subscribe-default-permissions.html [ Failure ]
-http/tests/push-api/subscribe-deny-permissions-on-prompt.html [ Failure ]
-http/tests/push-api/subscribe-deny-permissions.html [ Failure ]
-http/tests/push-api/subscribe-grant-permissions.html [ Failure ]
+http/tests/push-api [ Skip ]
 
 # Ftp code is disabled in gtk port
 http/tests/misc/ftp-eplf-directory.py [ Skip ]

diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
@@ -3623,14 +3623,8 @@ webkit.org/b/240579 http/tests/workers/service/shownotification-denied.html [ Sk
 webkit.org/b/240579 http/tests/workers/service/shownotification-invalid-data.html [ Skip ]
 webkit.org/b/240579 http/tests/workers/service/getnotifications-stop.html [ Skip ]
 webkit.org/b/240579 http/tests/workers/service/getnotifications.html [ Skip ]
-webkit.org/b/240579 http/tests/push-api/subscribe-default-permissions-iframe-cross-origin.html [ Skip ]
-webkit.org/b/240579 http/tests/push-api/subscribe-default-permissions-iframe-same-origin.html [ Skip ]
-webkit.org/b/240579 http/tests/push-api/subscribe-default-permissions.html [ Skip ]
-webkit.org/b/240579 http/tests/push-api/subscribe-deny-permissions-on-prompt.html [ Skip ]
-webkit.org/b/240579 http/tests/push-api/subscribe-grant-permissions.html [ Skip ]
-webkit.org/b/240579 http/wpt/push-api/pushEvent.any.serviceworker.html [ Skip ]
-webkit.org/b/240579 http/wpt/push-api/pushManager.any.html [ Skip ]
-webkit.org/b/240579 http/wpt/push-api/pushManager.any.serviceworker.html [ Skip ]
+webkit.org/b/240579 http/tests/push-api [ Skip ]
+webkit.org/b/240579 http/wpt/push-api [ Skip ]
 
 # These tests finish with unfired UI script callbacks, causing crashes. See webkit.org/b/236794
 editing/spelling/editing-word-with-marker-1.html

diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -1618,12 +1618,7 @@ fast/scrolling/rtl-scrollbars-alternate-iframe-body-dir-attr-does-not-update-scr
 [ BigSur+ arm64 ] editing/execCommand/paste-as-quotation-disconnected-paragraph-ancestor-crash.html [ Skip ]
 
 # Push subscription tests don't work without service worker support.
-http/tests/push-api/subscribe-default-permissions-iframe-cross-origin.html [ Skip ]
-http/tests/push-api/subscribe-default-permissions-iframe-same-origin.html [ Skip ]
-http/tests/push-api/subscribe-default-permissions.html [ Skip ]
-http/tests/push-api/subscribe-deny-permissions-on-prompt.html [ Failure ]
-http/tests/push-api/subscribe-deny-permissions.html [ Skip ]
-http/tests/push-api/subscribe-grant-permissions.html [ Skip ]
+http/tests/push-api [ Skip ]
 
 webkit.org/b/236126 imported/w3c/web-platform-tests/html/user-activation/activation-trigger-pointerevent.html [ Pass Failure ]
 

diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
@@ -5038,12 +5038,7 @@ http/tests/security/contentSecurityPolicy/userAgentShadowDOM/default-src-object-
 streams/readable-stream-lock-after-worker-terminates-crash.html [ Timeout Pass ]
 
 # Push subscription tests fail without platform-specific PushCrypto implementations.
-http/tests/push-api/subscribe-default-permissions-iframe-cross-origin.html [ Failure ]
-http/tests/push-api/subscribe-default-permissions-iframe-same-origin.html [ Failure ]
-http/tests/push-api/subscribe-default-permissions.html [ Failure ]
-http/tests/push-api/subscribe-deny-permissions-on-prompt.html [ Failure ]
-http/tests/push-api/subscribe-deny-permissions.html [ Failure ]
-http/tests/push-api/subscribe-grant-permissions.html [ Failure ]
+http/tests/push-api [ Skip ]
 
 webkit.org/b/240617 webanimations/offset-anchor-animation-yields-compositing.html [ Pass Failure ]
 webkit.org/b/240617 webanimations/offset-distance-animation-yields-compositing.html [ Pass Failure ]

diff --git a/Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.cpp b/Source/WebKit/NetworkProcess/Notifications/NetworkNotificationManager.cpp
@@ -150,6 +150,11 @@ void NetworkNotificationManager::unsubscribeFromPushService(URL&& scopeURL, std:
 
 void NetworkNotificationManager::getPushSubscription(URL&& scopeURL, CompletionHandler<void(Expected<std::optional<WebCore::PushSubscriptionData>, WebCore::ExceptionData>&&)>&& completionHandler)
 {
+    if (m_networkSession.sessionID().isEphemeral()) {
+        completionHandler(std::optional<WebCore::PushSubscriptionData> { });
+        return;
+    }
+
     if (!m_connection) {
         completionHandler(makeUnexpected(ExceptionData { AbortError, "No connection to push daemon"_s }));
         return;
@@ -160,6 +165,11 @@ void NetworkNotificationManager::getPushSubscription(URL&& scopeURL, CompletionH
 
 void NetworkNotificationManager::getPushPermissionState(URL&& scopeURL, CompletionHandler<void(Expected<uint8_t, WebCore::ExceptionData>&&)>&& completionHandler)
 {
+    if (m_networkSession.sessionID().isEphemeral()) {
+        completionHandler(static_cast<uint8_t>(PushPermissionState::Denied));
+        return;
+    }
+
     if (!m_connection) {
         completionHandler(makeUnexpected(ExceptionData { AbortError, "No connection to push daemon"_s }));
         return;

diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebNotificationClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebNotificationClient.cpp
@@ -95,6 +95,9 @@ void WebNotificationClient::requestPermission(ScriptExecutionContext& context, P
     ASSERT(isMainRunLoop());
     ASSERT(m_page);
 
+    if (!context.isDocument() || WebProcess::singleton().sessionID().isEphemeral())
+        return permissionHandler(NotificationClient::Permission::Denied);
+
     auto* securityOrigin = context.securityOrigin();
     if (!securityOrigin)
         return permissionHandler(NotificationClient::Permission::Denied);
@@ -103,9 +106,9 @@ void WebNotificationClient::requestPermission(ScriptExecutionContext& context, P
 
 NotificationClient::Permission WebNotificationClient::checkPermission(ScriptExecutionContext* context)
 {
-    if (!context)
-        return NotificationClient::Permission::Denied;
-    if (!context->isDocument() && !context->isServiceWorkerGlobalScope())
+    if (!context
+        || (!context->isDocument() && !context->isServiceWorkerGlobalScope())
+        || WebProcess::singleton().sessionID().isEphemeral())
         return NotificationClient::Permission::Denied;
 
     auto* origin = context->securityOrigin();