From 5fc25780f5e3c3822635bdbad6e769e91f2c49fc Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 15 May 2026 08:01:56 -0700 Subject: [PATCH 1/2] Handle 403 issues --- notification/method/webpush.php | 13 +- styles/all/template/webpush.js | 242 ++++++++++++++---- tests/controller/controller_webpush_test.php | 58 +++++ .../notification_method_webpush_test.php | 6 + ucp/controller/webpush.php | 13 +- 5 files changed, 270 insertions(+), 62 deletions(-) diff --git a/notification/method/webpush.php b/notification/method/webpush.php index 6f3988e..914cd78 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -282,7 +282,7 @@ protected function notify_using_webpush(): void if (!$report->isSuccess()) { // Fill array of endpoints to remove if subscription has expired or is permanently gone. - // Library checks for 404/410; we also check for 401 (Unauthorized) and endpoints + // Library checks for 404/410; we also check for 401/403 auth failures and endpoints // using the .invalid TLD (e.g. permanently-removed.invalid), which per RFC 6761 are // guaranteed to never resolve and are used as a sentinel for dead subscriptions. if ($report->isSubscriptionExpired() || $this->is_subscription_unauthorized($report) || $this->is_endpoint_permanently_removed($report->getEndpoint())) @@ -500,19 +500,20 @@ protected function set_endpoint_padding(\Minishlink\WebPush\WebPush $web_push, s } /** - * Check if subscription push failed with 401 Unauthorized status + * Check if subscription push failed with a permanent authorization error * - * 401 indicates the push service no longer accepts this subscription, - * typically due to revoked credentials or subscription no longer being valid. + * 401/403 indicate the push service no longer accepts this subscription, + * typically due to revoked credentials, rotated VAPID keys, or the + * subscription no longer being valid for the current credentials. * * @param \Minishlink\WebPush\MessageSentReport $report * - * @return bool True if subscription returned 401 Unauthorized + * @return bool True if subscription returned 401 Unauthorized or 403 Forbidden */ protected function is_subscription_unauthorized(\Minishlink\WebPush\MessageSentReport $report): bool { $response = $report->getResponse(); - return $response && $response->getStatusCode() === 401; + return $response && in_array($response->getStatusCode(), [401, 403], true); } /** diff --git a/styles/all/template/webpush.js b/styles/all/template/webpush.js index aa4de02..6601bfd 100644 --- a/styles/all/template/webpush.js +++ b/styles/all/template/webpush.js @@ -18,7 +18,7 @@ function PhpbbWebpush() { formToken: '', }; - /** @type {{endpoint: string, expiration: string}[]} Subscriptions */ + /** @type {{endpoint: string, expirationTime: number}[]} Subscriptions */ let subscriptions; /** @type {string} Title of error message */ @@ -73,11 +73,11 @@ function PhpbbWebpush() { if ('serviceWorker' in navigator && 'PushManager' in window) { navigator.serviceWorker.register(serviceWorkerUrl) - .then(() => { + .then(async () => { subscribeButton.addEventListener('click', subscribeButtonHandler); unsubscribeButton.addEventListener('click', unsubscribeButtonHandler); - updateButtonState(); + await updateButtonState(); initPopupPrompt(); }) .catch(error => { @@ -115,21 +115,32 @@ function PhpbbWebpush() { * * @return void */ - function updateButtonState() { - if (Notification.permission === 'granted') { - navigator.serviceWorker.getRegistration(serviceWorkerUrl) - .then(registration => { - if (typeof registration === 'undefined') { - return; - } + async function updateButtonState() { + setSubscriptionState(false); - registration.pushManager.getSubscription() - .then(subscribed => { - if (isValidSubscription(subscribed)) { - setSubscriptionState(true); - } - }); - }); + if (Notification.permission !== 'granted') { + return; + } + + try { + const registration = await navigator.serviceWorker.getRegistration(serviceWorkerUrl); + if (typeof registration === 'undefined') { + return; + } + + const subscription = await registration.pushManager.getSubscription(); + if (!subscription) { + return; + } + + if (shouldRefreshSubscription(subscription)) { + await refreshSubscription(registration, subscription); + return; + } + + setSubscriptionState(true); + } catch (error) { + console.error('Failed to update Web Push subscription state:', error); } } @@ -157,7 +168,7 @@ function PhpbbWebpush() { registration.pushManager.getSubscription() .then(subscription => { - if (!isValidSubscription(subscription)) { + if (shouldRefreshSubscription(subscription)) { showPopup(popup); } }); @@ -250,6 +261,72 @@ function PhpbbWebpush() { return false; }; + /** + * Check whether the current browser subscription uses the configured VAPID key + * + * @param {PushSubscription} subscription + * @returns {boolean} + */ + const hasCurrentVapidKey = subscription => { + if (!subscription || !subscription.options || !subscription.options.applicationServerKey) { + return true; + } + + return uint8ArrayToUrlB64(new Uint8Array(subscription.options.applicationServerKey)) === vapidPublicKey; + }; + + /** + * Check whether a subscription should be recreated in the browser and backend + * + * @param {PushSubscription} subscription + * @returns {boolean} + */ + const shouldRefreshSubscription = subscription => !isValidSubscription(subscription) || !hasCurrentVapidKey(subscription); + + /** + * Remove a cached subscription entry + * + * @param {string} endpoint + */ + function removeStoredSubscription(endpoint) { + if (!endpoint) { + return; + } + + subscriptions = subscriptions.filter(subscription => subscription.endpoint !== endpoint); + } + + /** + * Update cached subscriptions with the newest server state + * + * @param {PushSubscription} subscription + * @param {string} previousEndpoint + */ + function storeSubscription(subscription, previousEndpoint = '') { + removeStoredSubscription(previousEndpoint); + removeStoredSubscription(subscription.endpoint); + subscriptions.push({ + endpoint: subscription.endpoint, + expirationTime: subscription.expirationTime || 0, + }); + } + + /** + * Convert a PushSubscription to the payload expected by the backend + * + * @param {PushSubscription} subscription + * @param {string} previousEndpoint + * @returns {Object} + */ + function getSubscriptionPayload(subscription, previousEndpoint = '') { + const payload = subscription.toJSON(); + if (previousEndpoint) { + payload.previous_endpoint = previousEndpoint; + } + + return payload; + } + /** * Set subscription state for buttons * @@ -265,6 +342,66 @@ function PhpbbWebpush() { } } + /** + * Persist a browser subscription to the backend + * + * @param {PushSubscription} subscription + * @param {string} previousEndpoint + * @returns {Promise} + */ + async function persistSubscription(subscription, previousEndpoint = '') { + const loadingIndicator = phpbb.loadingIndicator(); + + try { + const response = await fetch(subscribeUrl, { + method: 'POST', + headers: { + 'X-Requested-With': 'XMLHttpRequest', + }, + body: getFormData(getSubscriptionPayload(subscription, previousEndpoint)), + }); + const data = await response.json(); + + if (!data.success) { + throw new Error(data.message || subscribeButton.getAttribute('data-l-unsupported')); + } + + handleSubscribe(data, subscription, previousEndpoint); + return data; + } finally { + loadingIndicator.fadeOut(phpbb.alertTime); + } + } + + /** + * Create a fresh browser subscription and store it in the backend + * + * @param {ServiceWorkerRegistration} registration + * @param {PushSubscription|null} previousSubscription + * @returns {Promise} + */ + async function refreshSubscription(registration, previousSubscription = null) { + const previousEndpoint = previousSubscription ? previousSubscription.endpoint : ''; + + if (previousSubscription) { + await previousSubscription.unsubscribe(); + removeStoredSubscription(previousEndpoint); + } + + const newSubscription = await registration.pushManager.subscribe({ + userVisibleOnly: true, + applicationServerKey: urlB64ToUint8Array(vapidPublicKey), + }); + + try { + await persistSubscription(newSubscription, previousEndpoint); + return newSubscription; + } catch (error) { + newSubscription.unsubscribe().catch(console.error); + throw error; + } + } + /** * Handler for pushing subscribe button * @@ -288,46 +425,16 @@ function PhpbbWebpush() { // We might already have a subscription that is unknown to this instance of phpBB. // Unsubscribe before trying to subscribe again. - if (typeof registration !== 'undefined') { - const subscribed = await registration.pushManager.getSubscription(); - if (subscribed) { - await subscribed.unsubscribe(); - } + if (typeof registration === 'undefined') { + throw new Error(subscribeButton.getAttribute('data-l-unsupported')); } - const newSubscription = await registration.pushManager.subscribe({ - userVisibleOnly: true, - applicationServerKey: urlB64ToUint8Array(vapidPublicKey), - }); - - const loadingIndicator = phpbb.loadingIndicator(); try { - const response = await fetch(subscribeUrl, { - method: 'POST', - headers: { - 'X-Requested-With': 'XMLHttpRequest', - }, - body: getFormData(newSubscription), - }); - const data = await response.json(); - loadingIndicator.fadeOut(phpbb.alertTime); - - if (data.success) { - handleSubscribe(data); - } else { - // Server rejected the subscription; clean up the browser-side subscription - // without awaiting, so a failure here can't bubble to the outer catch and - // incorrectly trigger promptDenied or show the wrong error message. - newSubscription.unsubscribe().catch(console.error); - promptDenied.set(); - hidePopup(document.getElementById('wpn_popup_prompt')); - phpbb.alert(ajaxErrorTitle, data.message || subscribeButton.getAttribute('data-l-unsupported')); - } + const subscribed = await registration.pushManager.getSubscription(); + await refreshSubscription(registration, subscribed); } catch (error) { - loadingIndicator.fadeOut(phpbb.alertTime); - // Clean up the browser-side subscription so it doesn't become orphaned - // when the server request fails (network error, bad JSON, etc.). - newSubscription.unsubscribe().catch(console.error); + promptDenied.set(); + hidePopup(document.getElementById('wpn_popup_prompt')); phpbb.alert(ajaxErrorTitle, error.message || subscribeButton.getAttribute('data-l-unsupported')); } } catch (error) { @@ -355,6 +462,11 @@ function PhpbbWebpush() { } const subscription = await registration.pushManager.getSubscription(); + if (!subscription) { + setSubscriptionState(false); + return; + } + const loadingIndicator = phpbb.loadingIndicator(); fetch(unsubscribeUrl, { method: 'POST', @@ -369,6 +481,7 @@ function PhpbbWebpush() { }) .then(unsubscribed => { if (unsubscribed) { + removeStoredSubscription(subscription.endpoint); setSubscriptionState(false); } }) @@ -423,8 +536,9 @@ function PhpbbWebpush() { * * @param {Object} response Response from subscription endpoint */ - function handleSubscribe(response) { + function handleSubscribe(response, subscription, previousEndpoint = '') { if (response.success) { + storeSubscription(subscription, previousEndpoint); setSubscriptionState(true); if ('form_tokens' in response) { updateFormTokens(response.form_tokens); @@ -477,6 +591,24 @@ function PhpbbWebpush() { return outputArray; } + /** + * Convert a Uint8Array to a URL-safe base64 string + * + * @param {Uint8Array} value + * @returns {string} + */ + function uint8ArrayToUrlB64(value) { + let stringValue = ''; + for (let i = 0; i < value.length; i++) { + stringValue += String.fromCharCode(value[i]); + } + + return window.btoa(stringValue) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/u, ''); + } + const promptDenied = { key: 'wpn_popup_denied', diff --git a/tests/controller/controller_webpush_test.php b/tests/controller/controller_webpush_test.php index e467b27..ec7fbe9 100644 --- a/tests/controller/controller_webpush_test.php +++ b/tests/controller/controller_webpush_test.php @@ -420,6 +420,48 @@ public function test_sub_unsubscribe_success() $this->assertEmpty($this->get_subscription_data()); } + public function test_subscribe_replaces_previous_subscription() + { + $this->form_helper->method('check_form_tokens')->willReturn(true); + $this->request->method('is_ajax')->willReturn(true); + $this->user->data['user_id'] = 2; + $this->user->data['is_bot'] = false; + $this->user->data['user_type'] = USER_NORMAL; + + $sql = 'INSERT INTO phpbb_wpn_push_subscriptions ' . $this->db->sql_build_array('INSERT', [ + 'user_id' => 2, + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/old_endpoint', + 'expiration_time' => 0, + 'p256dh' => 'old_p256dh', + 'auth' => 'old_auth', + ]); + $this->db->sql_query($sql); + + $symfony_request = $this->createMock(\phpbb\symfony_request::class); + $symfony_request->method('get')->willReturn(json_encode([ + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/new_endpoint', + 'previous_endpoint' => 'https://fcm.googleapis.com/fcm/send/old_endpoint', + 'expirationTime' => 42, + 'keys' => ['p256dh' => 'new_p256dh', 'auth' => 'new_auth'] + ])); + + $response = $this->controller->subscribe($symfony_request); + + $this->assertInstanceOf(JsonResponse::class, $response); + $this->assertEquals(['success' => true, 'form_tokens' => $this->form_helper->get_form_tokens(webpush::FORM_TOKEN_UCP)], json_decode($response->getContent(), true)); + + $subscriptions = $this->get_all_subscriptions(2); + $this->assertCount(1, $subscriptions); + $this->assertEquals([ + 'user_id' => '2', + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/new_endpoint', + 'p256dh' => 'new_p256dh', + 'auth' => 'new_auth', + 'expiration_time' => '42', + 'subscription_id' => '2', + ], $subscriptions[0]); + } + public function data_subscribe_invalid_endpoint(): array { return [ @@ -594,6 +636,22 @@ private function get_subscription_data() return $row; } + private function get_all_subscriptions(int $user_id): array + { + $sql = 'SELECT * + FROM phpbb_wpn_push_subscriptions + WHERE user_id = ' . $user_id . ' + ORDER BY subscription_id ASC'; + $result = $this->db->sql_query($sql); + $rows = []; + while ($row = $this->db->sql_fetchrow($result)) + { + $rows[] = $row; + } + $this->db->sql_freeresult($result); + return $rows; + } + private function get_user_popup_preference($user_id) { $sql = 'SELECT user_wpn_popup_disabled diff --git a/tests/notification/notification_method_webpush_test.php b/tests/notification/notification_method_webpush_test.php index 696aee0..d8d4ccd 100644 --- a/tests/notification/notification_method_webpush_test.php +++ b/tests/notification/notification_method_webpush_test.php @@ -662,6 +662,12 @@ public function test_is_subscription_unauthorized(): void $report_401 = new \Minishlink\WebPush\MessageSentReport($request_401, $response_401, false, 'Unauthorized'); $this->assertTrue($reflection->invoke($this->notification_method_webpush, $report_401), 'Expected 401 to be treated as unauthorized'); + // Test 403 status (should return true for invalid VAPID/subscription mismatches) + $response_403 = $this->createMockResponse(403); + $request_403 = $this->createMockRequest(); + $report_403 = new \Minishlink\WebPush\MessageSentReport($request_403, $response_403, false, 'Forbidden'); + $this->assertTrue($reflection->invoke($this->notification_method_webpush, $report_403), 'Expected 403 to be treated as a permanent authorization failure'); + // Test 404 status (should return false, handled by isSubscriptionExpired) $response_404 = $this->createMockResponse(404); $request_404 = $this->createMockRequest(); diff --git a/ucp/controller/webpush.php b/ucp/controller/webpush.php index 9eb23a7..84ef8e6 100644 --- a/ucp/controller/webpush.php +++ b/ucp/controller/webpush.php @@ -359,6 +359,8 @@ public function subscribe(symfony_request $symfony_request): JsonResponse $this->check_subscribe_requests(); $data = json_sanitizer::decode($symfony_request->get('data', '')); + $previous_endpoint = $data['previous_endpoint'] ?? ''; + $expiration_time = $data['expiration_time'] ?? $data['expirationTime'] ?? 0; $data['endpoint'] = $data['endpoint'] ?? ''; @@ -367,10 +369,19 @@ public function subscribe(symfony_request $symfony_request): JsonResponse throw new http_exception(Response::HTTP_BAD_REQUEST, 'WEBPUSH_INVALID_ENDPOINT'); } + $replace_endpoints = array_values(array_unique(array_filter([$data['endpoint'], $previous_endpoint]))); + if (!empty($replace_endpoints)) + { + $sql = 'DELETE FROM ' . $this->push_subscriptions_table . ' + WHERE user_id = ' . (int) $this->user->id() . ' + AND ' . $this->db->sql_in_set('endpoint', $replace_endpoints); + $this->db->sql_query($sql); + } + $sql = 'INSERT INTO ' . $this->push_subscriptions_table . ' ' . $this->db->sql_build_array('INSERT', [ 'user_id' => $this->user->id(), 'endpoint' => $data['endpoint'], - 'expiration_time' => $data['expiration_time'] ?? 0, + 'expiration_time' => $expiration_time, 'p256dh' => $data['keys']['p256dh'], 'auth' => $data['keys']['auth'], ]); From 7830f70f0b015b4c2ed48a20e111279c3ad9480a Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 15 May 2026 10:09:47 -0700 Subject: [PATCH 2/2] Fix issues with timestamps and improve update sub db handling --- notification/method/webpush.php | 2 +- tests/controller/controller_webpush_test.php | 49 +++++++++- tests/event/listener_test.php | 2 +- .../notification_method_webpush_test.php | 36 ++++++++ ucp/controller/webpush.php | 89 +++++++++++++++---- 5 files changed, 157 insertions(+), 21 deletions(-) diff --git a/notification/method/webpush.php b/notification/method/webpush.php index 914cd78..c538efe 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -384,7 +384,7 @@ public function get_ucp_template_data(helper $controller_helper, form_helper $fo { $subscriptions[] = [ 'endpoint' => $subscription['endpoint'], - 'expirationTime' => (int) $subscription['expiration_time'], + 'expirationTime' => max(0, (int) $subscription['expiration_time']) * 1000, ]; } } diff --git a/tests/controller/controller_webpush_test.php b/tests/controller/controller_webpush_test.php index ec7fbe9..2317f42 100644 --- a/tests/controller/controller_webpush_test.php +++ b/tests/controller/controller_webpush_test.php @@ -441,7 +441,7 @@ public function test_subscribe_replaces_previous_subscription() $symfony_request->method('get')->willReturn(json_encode([ 'endpoint' => 'https://fcm.googleapis.com/fcm/send/new_endpoint', 'previous_endpoint' => 'https://fcm.googleapis.com/fcm/send/old_endpoint', - 'expirationTime' => 42, + 'expirationTime' => 42000, 'keys' => ['p256dh' => 'new_p256dh', 'auth' => 'new_auth'] ])); @@ -462,6 +462,53 @@ public function test_subscribe_replaces_previous_subscription() ], $subscriptions[0]); } + public function test_subscribe_invalid_payload_preserves_existing_subscription() + { + $this->form_helper->method('check_form_tokens')->willReturn(true); + $this->request->method('is_ajax')->willReturn(true); + $this->user->data['user_id'] = 2; + $this->user->data['is_bot'] = false; + $this->user->data['user_type'] = USER_NORMAL; + + $sql = 'INSERT INTO phpbb_wpn_push_subscriptions ' . $this->db->sql_build_array('INSERT', [ + 'user_id' => 2, + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/old_endpoint', + 'expiration_time' => 10, + 'p256dh' => 'old_p256dh', + 'auth' => 'old_auth', + ]); + $this->db->sql_query($sql); + + $symfony_request = $this->createMock(\phpbb\symfony_request::class); + $symfony_request->method('get')->willReturn(json_encode([ + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/new_endpoint', + 'previous_endpoint' => 'https://fcm.googleapis.com/fcm/send/old_endpoint', + 'expirationTime' => 42000, + 'keys' => ['auth' => 'new_auth'] + ])); + + $this->expectException(http_exception::class); + $this->expectExceptionMessage('AJAX_ERROR_TEXT'); + + try + { + $this->controller->subscribe($symfony_request); + } + finally + { + $subscriptions = $this->get_all_subscriptions(2); + $this->assertCount(1, $subscriptions); + $this->assertEquals([ + 'user_id' => '2', + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/old_endpoint', + 'p256dh' => 'old_p256dh', + 'auth' => 'old_auth', + 'expiration_time' => '10', + 'subscription_id' => '1', + ], $subscriptions[0]); + } + } + public function data_subscribe_invalid_endpoint(): array { return [ diff --git a/tests/event/listener_test.php b/tests/event/listener_test.php index c380355..94f52b9 100644 --- a/tests/event/listener_test.php +++ b/tests/event/listener_test.php @@ -221,7 +221,7 @@ public function get_ucp_template_data_data() 'method_data' => [ 'id' => 'notification.method.phpbb.wpn.webpush', ], - [['endpoint' => 'https://web.push.test.localhost/foo3', 'expirationTime' => 1]], + [['endpoint' => 'https://web.push.test.localhost/foo3', 'expirationTime' => 1000]], true, ], [ // webpush method with an invalid webpush subscription, expect code execution but no subscription data diff --git a/tests/notification/notification_method_webpush_test.php b/tests/notification/notification_method_webpush_test.php index d8d4ccd..dbefe7a 100644 --- a/tests/notification/notification_method_webpush_test.php +++ b/tests/notification/notification_method_webpush_test.php @@ -648,6 +648,42 @@ public function test_get_type(): void $this->assertEquals('notification.method.phpbb.wpn.webpush', $this->notification_method_webpush->get_type()); } + public function test_get_ucp_template_data_uses_millisecond_expiration_time(): void + { + $this->user->data['user_id'] = 2; + $this->user->page['page'] = 'ucp.php?i=ucp_notifications'; + $this->config['load_notifications'] = true; + $this->config['allow_board_notifications'] = true; + $this->config['wpn_webpush_dropdown_subscribe'] = true; + + $sql = 'INSERT INTO phpbb_wpn_push_subscriptions ' . $this->db->sql_build_array('INSERT', [ + 'user_id' => 2, + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/test_endpoint', + 'expiration_time' => 42, + 'p256dh' => 'test_p256dh', + 'auth' => 'test_auth', + ]); + $this->db->sql_query($sql); + + $controller_helper = $this->createMock(\phpbb\controller\helper::class); + $controller_helper->method('route')->willReturnArgument(0); + + $form_helper = $this->createMock(\phpbb\webpushnotifications\form\form_helper::class); + $form_helper->method('get_form_tokens')->willReturn([ + 'creation_time' => 1, + 'form_token' => 'test', + ]); + + $template_data = $this->notification_method_webpush->get_ucp_template_data($controller_helper, $form_helper); + + $this->assertSame([ + [ + 'endpoint' => 'https://fcm.googleapis.com/fcm/send/test_endpoint', + 'expirationTime' => 42000, + ], + ], $template_data['SUBSCRIPTIONS']); + } + /** * Test is_subscription_unauthorized method with various HTTP status codes */ diff --git a/ucp/controller/webpush.php b/ucp/controller/webpush.php index 84ef8e6..e66a53f 100644 --- a/ucp/controller/webpush.php +++ b/ucp/controller/webpush.php @@ -359,33 +359,41 @@ public function subscribe(symfony_request $symfony_request): JsonResponse $this->check_subscribe_requests(); $data = json_sanitizer::decode($symfony_request->get('data', '')); - $previous_endpoint = $data['previous_endpoint'] ?? ''; - $expiration_time = $data['expiration_time'] ?? $data['expirationTime'] ?? 0; + $endpoint = is_string($data['endpoint'] ?? null) ? $data['endpoint'] : ''; + $previous_endpoint = is_string($data['previous_endpoint'] ?? null) ? $data['previous_endpoint'] : ''; - $data['endpoint'] = $data['endpoint'] ?? ''; - - if (!$this->is_valid_endpoint($data['endpoint'])) + if (!$this->is_valid_endpoint($endpoint)) { throw new http_exception(Response::HTTP_BAD_REQUEST, 'WEBPUSH_INVALID_ENDPOINT'); } - $replace_endpoints = array_values(array_unique(array_filter([$data['endpoint'], $previous_endpoint]))); - if (!empty($replace_endpoints)) + $subscription_data = $this->get_subscription_write_data($data); + $subscription_data['user_id'] = $this->user->id(); + $subscription_data['endpoint'] = $endpoint; + + $sql = 'UPDATE ' . $this->push_subscriptions_table . ' + SET ' . $this->db->sql_build_array('UPDATE', [ + 'expiration_time' => $subscription_data['expiration_time'], + 'p256dh' => $subscription_data['p256dh'], + 'auth' => $subscription_data['auth'], + ]) . " + WHERE user_id = " . (int) $this->user->id() . " + AND endpoint = '" . $this->db->sql_escape($endpoint) . "'"; + $this->db->sql_query($sql); + + if (!$this->db->sql_affectedrows()) { - $sql = 'DELETE FROM ' . $this->push_subscriptions_table . ' - WHERE user_id = ' . (int) $this->user->id() . ' - AND ' . $this->db->sql_in_set('endpoint', $replace_endpoints); + $sql = 'INSERT INTO ' . $this->push_subscriptions_table . ' ' . $this->db->sql_build_array('INSERT', $subscription_data); $this->db->sql_query($sql); } - $sql = 'INSERT INTO ' . $this->push_subscriptions_table . ' ' . $this->db->sql_build_array('INSERT', [ - 'user_id' => $this->user->id(), - 'endpoint' => $data['endpoint'], - 'expiration_time' => $expiration_time, - 'p256dh' => $data['keys']['p256dh'], - 'auth' => $data['keys']['auth'], - ]); - $this->db->sql_query($sql); + if ($previous_endpoint && $previous_endpoint !== $endpoint) + { + $sql = 'DELETE FROM ' . $this->push_subscriptions_table . ' + WHERE user_id = ' . (int) $this->user->id() . " + AND endpoint = '" . $this->db->sql_escape($previous_endpoint) . "'"; + $this->db->sql_query($sql); + } return new JsonResponse([ 'success' => true, @@ -393,6 +401,51 @@ public function subscribe(symfony_request $symfony_request): JsonResponse ]); } + /** + * Validate and normalize subscription data for database writes + * + * @param array $data + * @return array + */ + protected function get_subscription_write_data(array $data): array + { + $p256dh = is_string($data['keys']['p256dh'] ?? null) ? $data['keys']['p256dh'] : ''; + $auth = is_string($data['keys']['auth'] ?? null) ? $data['keys']['auth'] : ''; + + if ($p256dh === '' || $auth === '') + { + throw new http_exception(Response::HTTP_BAD_REQUEST, 'AJAX_ERROR_TEXT'); + } + + return [ + 'expiration_time' => $this->normalize_subscription_expiration_time($data), + 'p256dh' => $p256dh, + 'auth' => $auth, + ]; + } + + /** + * Normalize PushSubscription expiration timestamps to seconds for storage + * + * @param array $data + * @return int + */ + protected function normalize_subscription_expiration_time(array $data): int + { + if (isset($data['expiration_time'])) + { + return max(0, (int) $data['expiration_time']); + } + + $expiration_time = $data['expirationTime'] ?? 0; + if (empty($expiration_time)) + { + return 0; + } + + return max(0, (int) floor(((int) $expiration_time) / 1000)); + } + /** * Handle unsubscribe requests *