diff --git a/controller/acp_controller.php b/controller/acp_controller.php index 8fceae6..1f611d3 100644 --- a/controller/acp_controller.php +++ b/controller/acp_controller.php @@ -21,6 +21,7 @@ use phpbb\template\template; use phpbb\user; use phpbb\webpushnotifications\ext; +use phpbb\webpushnotifications\subscription\validator as subscription_validator; class acp_controller { @@ -223,6 +224,15 @@ protected function save_settings() { // Validate config values validate_config_vars($display_settings, $config_array, $this->errors); + + if (empty($this->errors) + && !subscription_validator::is_valid_vapid_keys( + $config_array['wpn_webpush_vapid_public'] ?? '', + $config_array['wpn_webpush_vapid_private'] ?? $this->config['wpn_webpush_vapid_private'] + )) + { + $this->errors[] = $this->lang->lang('WEBPUSH_VAPID_KEYS_INVALID'); + } } if ($this->display_errors()) diff --git a/language/en/info_mcp_webpushnotifications.php b/language/en/info_mcp_webpushnotifications.php new file mode 100644 index 0000000..c2c474c --- /dev/null +++ b/language/en/info_mcp_webpushnotifications.php @@ -0,0 +1,23 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +if (!defined('IN_PHPBB')) +{ + exit; +} + +if (empty($lang) || !is_array($lang)) +{ + $lang = []; +} + +$lang = array_merge($lang, [ + 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED' => 'Removed Web Push subscription:
» %s', +]); diff --git a/language/en/webpushnotifications_module_acp.php b/language/en/webpushnotifications_module_acp.php index 18c732b..7ef3224 100644 --- a/language/en/webpushnotifications_module_acp.php +++ b/language/en/webpushnotifications_module_acp.php @@ -53,6 +53,7 @@ 'WEBPUSH_ROTATE_VAPID_KEYS_CONFIRM'=> 'Saving new keys will invalidate all existing Web Push subscriptions. Users will not receive Web Push notifications until their next visit to the board, when their subscriptions will be recreated.', 'WEBPUSH_VAPID_KEYS_GENERATED'=> 'New key pair generated. Submit this form to activate it.', 'WEBPUSH_VAPID_KEYS_GENERATION_FAILED'=> 'Could not generate a key pair in this browser. Enter a valid pair manually or try a supported browser.', + 'WEBPUSH_VAPID_KEYS_INVALID' => 'The VAPID identification keys are invalid.', 'WEBPUSH_VAPID_ROTATION_WARNING'=> 'Important: Changing either key invalidates all existing Web Push subscriptions.', 'WEBPUSH_VAPID_PUBLIC' => 'Server identification public key', 'WEBPUSH_VAPID_PUBLIC_EXPLAIN' => 'Shared with push services to authenticate messages sent from your board.', diff --git a/language/ru/info_mcp_webpushnotifications.php b/language/ru/info_mcp_webpushnotifications.php new file mode 100644 index 0000000..b6e6cfe --- /dev/null +++ b/language/ru/info_mcp_webpushnotifications.php @@ -0,0 +1,23 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +if (!defined('IN_PHPBB')) +{ + exit; +} + +if (empty($lang) || !is_array($lang)) +{ + $lang = []; +} + +$lang = array_merge($lang, [ + 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED' => 'Удалена подписка на браузерные push—уведомления:
» %s', +]); diff --git a/language/ru/webpushnotifications_module_acp.php b/language/ru/webpushnotifications_module_acp.php index 734c8f5..a6f911f 100644 --- a/language/ru/webpushnotifications_module_acp.php +++ b/language/ru/webpushnotifications_module_acp.php @@ -53,6 +53,7 @@ 'WEBPUSH_ROTATE_VAPID_KEYS_CONFIRM'=> 'Сохранение новых ключей сделает недействительными все существующие подписки на браузерные push—уведомления. Пользователи не будут получать браузерные push—уведомления до следующего посещения конференции, при котором их подписки будут пересозданы.', 'WEBPUSH_VAPID_KEYS_GENERATED'=> 'Новая пара ключей сгенерирована. Отправьте эту форму, чтобы активировать её.', 'WEBPUSH_VAPID_KEYS_GENERATION_FAILED'=> 'Не удалось сгенерировать пару ключей в этом браузере. Введите корректную пару вручную или используйте поддерживаемый браузер.', + 'WEBPUSH_VAPID_KEYS_INVALID' => 'Ключи идентификации VAPID недействительны.', 'WEBPUSH_VAPID_ROTATION_WARNING'=> 'Важно: изменение любого ключа делает недействительными все существующие подписки на браузерные push—уведомления.', 'WEBPUSH_VAPID_PUBLIC' => 'Публичный ключ идентификации сервера', 'WEBPUSH_VAPID_PUBLIC_EXPLAIN' => 'Передаётся службам push—уведомлений для аутентификации сообщений, отправляемых конференцией.', diff --git a/notification/method/webpush.php b/notification/method/webpush.php index e99b535..264ef0d 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -211,15 +211,17 @@ protected function notify_using_webpush(): void $user_subscription_map = $this->get_user_subscription_map($notify_users); $invalid_subscriptions = []; - // Validate each stored subscription once so malformed keys cannot interrupt the delivery batch. + // Validate each stored subscription once so malformed data cannot interrupt the delivery batch. foreach ($user_subscription_map as $user_id => $subscriptions) { $valid_subscriptions = []; $user = $this->user_loader->get_user($user_id); foreach ($subscriptions as $subscription) { - if (!is_string($subscription['p256dh'] ?? null) + if (!is_string($subscription['endpoint'] ?? null) + || !is_string($subscription['p256dh'] ?? null) || !is_string($subscription['auth'] ?? null) + || !$this->is_valid_endpoint_url($subscription['endpoint']) || !subscription_validator::is_valid($subscription['p256dh'], $subscription['auth'])) { $invalid_subscriptions[$subscription['subscription_id']] = $user; @@ -239,7 +241,17 @@ protected function notify_using_webpush(): void ], ]; - $web_push = new \Minishlink\WebPush\WebPush($auth); + try + { + $web_push = new \Minishlink\WebPush\WebPush($auth); + } + catch (\Throwable $exception) + { + $this->log->add('critical', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$exception->getMessage()]); + $this->empty_queue(); + $this->remove_invalid_subscriptions($invalid_subscriptions); + return; + } $number_of_notifications = 0; @@ -300,10 +312,10 @@ 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/403 auth failures and endpoints + // Library checks for 404/410; we also check 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())) + if ($this->should_remove_subscription($report)) { $expired_endpoints[] = $report->getEndpoint(); } @@ -449,6 +461,18 @@ protected function get_user_subscription_map(array $notify_users): array return $user_subscription_map; } + /** + * Check whether a stored subscription endpoint is valid for delivery + * Wrapper used here exists only as test seam + * + * @param string $endpoint Push subscription endpoint URL + * @return bool + */ + protected function is_valid_endpoint_url(string $endpoint): bool + { + return subscription_validator::is_valid_endpoint_url($endpoint); + } + /** * Remove subscriptions * @@ -540,20 +564,19 @@ protected function set_endpoint_padding(\Minishlink\WebPush\WebPush $web_push, s } /** - * Check if subscription push failed with a permanent authorization error + * Check if a failed subscription should be removed * - * 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. + * Authentication failures are not proof that a subscription expired. In + * particular, a VAPID key rotation returns 401/403 until the browser can + * recreate its subscription, so those rows must remain as ownership records. * * @param \Minishlink\WebPush\MessageSentReport $report * - * @return bool True if subscription returned 401 Unauthorized or 403 Forbidden + * @return bool True if the subscription is expired or permanently gone */ - protected function is_subscription_unauthorized(\Minishlink\WebPush\MessageSentReport $report): bool + protected function should_remove_subscription(\Minishlink\WebPush\MessageSentReport $report): bool { - $response = $report->getResponse(); - return $response && in_array($response->getStatusCode(), [401, 403], true); + return $report->isSubscriptionExpired() || $this->is_endpoint_permanently_removed($report->getEndpoint()); } /** diff --git a/styles/all/template/push_worker.js.twig b/styles/all/template/push_worker.js.twig index 3af01a9..542d9bb 100644 --- a/styles/all/template/push_worker.js.twig +++ b/styles/all/template/push_worker.js.twig @@ -88,7 +88,7 @@ self.addEventListener('notificationclick', event => { } try { - const url = new URL(event.notification.data.url, '{{ U_BOARD_URL|e("js") }}/'); + const url = new URL(event.notification.data.url, self.location.href); if (url.protocol === 'http:' || url.protocol === 'https:') { event.waitUntil(self.clients.openWindow(url.href)); } diff --git a/styles/all/template/webpush.js b/styles/all/template/webpush.js index 9a4a185..6b5f32f 100644 --- a/styles/all/template/webpush.js +++ b/styles/all/template/webpush.js @@ -140,6 +140,11 @@ function PhpbbWebpush() { return; } + // A subscription this account does not own was made by another member on this browser. + if (!subscriptions.some(stored => stored.endpoint === subscription.endpoint)) { + return; + } + if (shouldRefreshSubscription(subscription)) { await refreshSubscription(registration, subscription); return; diff --git a/subscription/validator.php b/subscription/validator.php index 74f62fd..f57a4dd 100644 --- a/subscription/validator.php +++ b/subscription/validator.php @@ -12,14 +12,78 @@ use Base64Url\Base64Url; use Brick\Math\BigInteger; +use Jose\Component\Core\JWK; use Jose\Component\Core\Util\Ecc\NistCurve; +use Jose\Component\Signature\Algorithm\ES256; use Minishlink\WebPush\Utils; +use Minishlink\WebPush\VAPID; /** * Web Push subscription key validator */ class validator { + /** + * Validate VAPID identification keys + * + * @param string $public_key VAPID public key + * @param string $private_key VAPID private key + * @return bool + */ + public static function is_valid_vapid_keys(string $public_key, string $private_key): bool + { + try + { + $vapid = VAPID::validate([ + 'subject' => 'https://example.com', + 'publicKey' => $public_key, + 'privateKey' => $private_key, + ]); + + if (!self::is_valid($public_key, Base64Url::encode(substr($vapid['privateKey'], 0, 16)))) + { + return false; + } + + [$x, $y] = Utils::unserializePublicKey($vapid['publicKey']); + $key = new JWK([ + 'kty' => 'EC', + 'crv' => 'P-256', + 'x' => Base64Url::encode($x), + 'y' => Base64Url::encode($y), + 'd' => Base64Url::encode($vapid['privateKey']), + ]); + $algorithm = new ES256(); + $input = 'phpBB VAPID key pair validation'; + $signature = $algorithm->sign($key, $input); + + return $algorithm->verify($key, $input, $signature); + } + catch (\Throwable $exception) + { + return false; + } + } + + /** + * Check that an endpoint is a plain HTTPS URL with a well-formed hostname + * + * @param string $endpoint Push subscription endpoint URL + * @return bool + */ + public static function is_valid_endpoint_url(string $endpoint): bool + { + $parts = parse_url($endpoint); + + return is_array($parts) + && strtolower($parts['scheme'] ?? '') === 'https' + && !isset($parts['user']) + && !isset($parts['pass']) + && (!isset($parts['port']) || $parts['port'] === 443) + && is_string($parts['host'] ?? null) + && filter_var($parts['host'], FILTER_VALIDATE_DOMAIN, FILTER_FLAG_HOSTNAME) !== false; + } + /** * Validate Web Push subscription keys * diff --git a/tests/controller/controller_acp_test.php b/tests/controller/controller_acp_test.php index b8c478c..ff38c1f 100644 --- a/tests/controller/controller_acp_test.php +++ b/tests/controller/controller_acp_test.php @@ -16,6 +16,10 @@ class controller_acp_test extends \phpbb_test_case { + private const VALID_VAPID_PUBLIC_KEY = 'BIcGkq1Ncj3a2-J0UW-1A0NETLjvxZzNLiYBiPVMKNjgwmwPi5jyK87VfS4FZn9n7S9pLMQzjV3LmFuOnRSOvmI'; + private const VALID_VAPID_PRIVATE_KEY = 'SrlbBEVgibWmKHYbDPu4Y2XvDWPjeGcc9fC16jq01xU'; + private const OTHER_VAPID_PUBLIC_KEY = 'BDnYSJHVZBxq834LqDGr893IfazEez7q-jYH2QBNlT0ji2C9UwGosiqz8Dp_ZN23lqAngBZyRjXVWF4ZLA8X2zI'; + /** @var bool */ public static $valid_form = true; @@ -213,8 +217,8 @@ public function webpush_save_data(): array { $valid = [ 'wpn_webpush_enable' => 1, - 'wpn_webpush_vapid_public' => str_repeat('a', 25), - 'wpn_webpush_vapid_private' => str_repeat('b', 25), + 'wpn_webpush_vapid_public' => self::VALID_VAPID_PUBLIC_KEY, + 'wpn_webpush_vapid_private' => self::VALID_VAPID_PRIVATE_KEY, 'wpn_webpush_dropdown_subscribe' => 1, 'wpn_webpush_method_enabled' => 0, 'wpn_webpush_popup_prompt' => 1, @@ -228,8 +232,8 @@ public function webpush_save_data(): array 'ERROR_MSG' => '', ], [ - 'wpn_webpush_vapid_public' => str_repeat('a', 25), - 'wpn_webpush_vapid_private' => str_repeat('b', 25), + 'wpn_webpush_vapid_public' => self::VALID_VAPID_PUBLIC_KEY, + 'wpn_webpush_vapid_private' => self::VALID_VAPID_PRIVATE_KEY, 'wpn_webpush_method_enabled' => 0, ], true, @@ -243,11 +247,26 @@ public function webpush_save_data(): array 'ERROR_MSG' => '', ], [ - 'wpn_webpush_vapid_public' => str_repeat('a', 25), - 'wpn_webpush_vapid_private' => 'existing-private-key', + 'wpn_webpush_vapid_public' => self::VALID_VAPID_PUBLIC_KEY, + 'wpn_webpush_vapid_private' => self::VALID_VAPID_PRIVATE_KEY, ], true, ], + 'changed public key requires private key' => [ + array_merge($valid, [ + 'wpn_webpush_vapid_public' => self::OTHER_VAPID_PUBLIC_KEY, + 'wpn_webpush_vapid_private' => '', + ]), + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'WEBPUSH_VAPID_KEYS_INVALID', + ], + [ + 'wpn_webpush_vapid_public' => self::VALID_VAPID_PUBLIC_KEY, + 'wpn_webpush_vapid_private' => self::VALID_VAPID_PRIVATE_KEY, + ], + false, + ], 'invalid enabled VAPID keys are rejected' => [ array_merge($valid, [ 'wpn_webpush_vapid_public' => 'short', @@ -258,8 +277,23 @@ public function webpush_save_data(): array 'ERROR_MSG' => 'SETTING_TOO_SHORT
SETTING_TOO_SHORT', ], [ - 'wpn_webpush_vapid_public' => 'existing-public-key', - 'wpn_webpush_vapid_private' => 'existing-private-key', + 'wpn_webpush_vapid_public' => self::VALID_VAPID_PUBLIC_KEY, + 'wpn_webpush_vapid_private' => self::VALID_VAPID_PRIVATE_KEY, + ], + false, + ], + 'invalid VAPID key encoding is rejected' => [ + array_merge($valid, [ + 'wpn_webpush_vapid_public' => str_repeat('A', 30), + 'wpn_webpush_vapid_private' => str_repeat('B', 30), + ]), + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'WEBPUSH_VAPID_KEYS_INVALID', + ], + [ + 'wpn_webpush_vapid_public' => self::VALID_VAPID_PUBLIC_KEY, + 'wpn_webpush_vapid_private' => self::VALID_VAPID_PRIVATE_KEY, ], false, ], @@ -276,7 +310,7 @@ public function webpush_save_data(): array [ 'wpn_webpush_enable' => 0, 'wpn_webpush_vapid_public' => '', - 'wpn_webpush_vapid_private' => 'existing-private-key', + 'wpn_webpush_vapid_private' => self::VALID_VAPID_PRIVATE_KEY, ], true, ], @@ -288,8 +322,8 @@ public function webpush_save_data(): array */ public function test_save_settings_validates_and_persists_webpush_config(array $input, array $error_vars, array $expected_config, $expect_saved): void { - $this->config['wpn_webpush_vapid_public'] = 'existing-public-key'; - $this->config['wpn_webpush_vapid_private'] = 'existing-private-key'; + $this->config['wpn_webpush_vapid_public'] = self::VALID_VAPID_PUBLIC_KEY; + $this->config['wpn_webpush_vapid_private'] = self::VALID_VAPID_PRIVATE_KEY; $this->request->method('variable') ->with('config', ['' => ''], true) diff --git a/tests/controller/controller_webpush_test.php b/tests/controller/controller_webpush_test.php index ee36a3d..5d0b7a7 100644 --- a/tests/controller/controller_webpush_test.php +++ b/tests/controller/controller_webpush_test.php @@ -68,6 +68,12 @@ protected function setUp(): void $this->config = new \phpbb\config\config([ 'allow_nocensors' => false, 'sitename' => 'yourdomain.com', + 'force_server_vars' => true, + 'server_protocol' => 'http://', + 'server_name' => 'localhost', + 'server_port' => 80, + 'script_path' => '/phpBB', + 'cookie_secure' => false, ]); $config = $this->config; $this->controller_helper = $this->createMock(\phpbb\controller\helper::class); @@ -151,7 +157,7 @@ private function getExpectedResponse(): array 'heading' => 'yourdomain.com', 'title' => 'Quoted by Guest in:', 'text' => '"Welcome to phpBB"', - 'url' => 'phpBB/viewtopic.php?p=1#p1', + 'url' => 'http://localhost/phpBB/viewtopic.php?p=1#p1', 'avatar' => ['src' => ''], ]; } @@ -320,6 +326,32 @@ public function test_get_user_notification_legacy() $this->assertEquals($this->getExpectedResponse(), $response_data); } + public function legacy_notification_url_data(): array + { + return [ + 'board relative' => ['./viewtopic.php?p=1', 'http://localhost/phpBB/viewtopic.php?p=1'], + 'leading slash' => ['/viewtopic.php?p=1', 'http://localhost/phpBB/viewtopic.php?p=1'], + 'absolute HTTPS' => ['https://example.com/viewtopic.php?p=1', 'https://example.com/viewtopic.php?p=1'], + 'absolute HTTP' => ['http://example.com/viewtopic.php?p=1', 'http://example.com/viewtopic.php?p=1'], + 'unsafe scheme unchanged for worker rejection' => ['javascript:alert(1)', 'javascript:alert(1)'], + ]; + } + + /** + * @dataProvider legacy_notification_url_data + */ + public function test_legacy_notification_urls_are_normalized(string $url, string $expected): void + { + $method = new ReflectionMethod($this->controller, 'get_notification_data'); + $method->setAccessible(true); + $data = json_decode($method->invoke($this->controller, json_encode([ + 'heading' => 'Board', + 'url' => $url, + ])), true); + + self::assertSame($expected, $data['url']); + } + /** * @throws SyntaxError * @throws RuntimeError @@ -327,17 +359,10 @@ public function test_get_user_notification_legacy() */ public function test_worker() { - $this->config['force_server_vars'] = true; - $this->config['server_protocol'] = 'https://'; - $this->config['server_name'] = 'example.com'; - $this->config['server_port'] = 443; - $this->config['script_path'] = '/forum'; - $this->config['cookie_secure'] = true; $this->template->expects(self::once()) ->method('render') ->with('@phpbb_webpushnotifications/push_worker.js.twig', [ 'U_WEBPUSH_GET_NOTIFICATION' => 'test_route', - 'U_BOARD_URL' => 'https://example.com/forum', 'ASSETS_VERSION' => '1.0', ]) ->willReturn('rendered_content'); @@ -586,6 +611,7 @@ public function data_subscribe_invalid_endpoint(): array 'no_host' => ['not_a_url'], 'disallowed_host' => ['https://evil.example.com/push/endpoint'], 'disallowed_subdomain' => ['https://evil.fcm.googleapis.com.attacker.com/push'], + 'malformed_allowed_suffix' => ['https://evil.example.com\\.push.apple.com/x'], 'empty_string' => [''], ]; } @@ -678,6 +704,8 @@ public function data_is_valid_endpoint(): array ['https://evil.example.com/push/endpoint', false], // Invalid: subdomain spoofing ['https://evil.fcm.googleapis.com.attacker.com/push', false], + // Invalid: malformed host with an allowed suffix + ['https://evil.example.com\\.push.apple.com/x', false], // Invalid: not a URL ['not_a_url', false], // Invalid: bare wildcard domain without subdomain @@ -696,13 +724,26 @@ public function test_is_valid_endpoint(string $endpoint, bool $expected): void $this->assertEquals($expected, $this->controller->is_valid_endpoint($endpoint)); } - public function test_current_notification_data_is_rendered(): void + public function current_notification_url_data(): array + { + return [ + 'board relative' => ['viewtopic.php?p=1&x=2', 'http://localhost/phpBB/viewtopic.php?p=1&x=2'], + 'route relative' => ['./../../../ucp.php?i=pm&p=4', 'http://localhost/phpBB/ucp.php?i=pm&p=4'], + 'absolute HTTPS' => ['https://example.com/topic/1', 'https://example.com/topic/1'], + 'unsafe scheme unchanged for worker rejection' => ['javascript:alert(1)', 'javascript:alert(1)'], + ]; + } + + /** + * @dataProvider current_notification_url_data + */ + public function test_current_notification_data_is_rendered(string $url, string $expected_url): void { $notification = $this->createMock(\phpbb\notification\type\type_interface::class); $notification->method('users_to_query')->willReturn([]); - $notification->method('get_title')->willReturn('A & title'); - $notification->method('get_reference')->willReturn('A "reference"'); - $notification->method('get_url')->willReturn('viewtopic.php?p=1&x=2'); + $notification->method('get_title')->willReturn('Private Message from Alice & Bob:'); + $notification->method('get_reference')->willReturn('"She said "hello" & asked about a < b in <div> tags"'); + $notification->method('get_url')->willReturn($url); $notification->method('get_avatar')->willReturn(''); $this->notification_manager->expects(self::once()) ->method('get_item_type_class') @@ -716,9 +757,9 @@ public function test_current_notification_data_is_rendered(): void ])), true); self::assertSame('yourdomain.com', $data['heading']); - self::assertSame('A & title', $data['title']); - self::assertSame('A "reference"', $data['text']); - self::assertSame('viewtopic.php?p=1&x=2', $data['url']); + self::assertSame('Private Message from Alice & Bob:', $data['title']); + self::assertSame('"She said "hello" & asked about a < b in
tags"', $data['text']); + self::assertSame($expected_url, $data['url']); self::assertStringEndsWith('/images/avatar.png', $data['avatar']['src']); } diff --git a/tests/controller/fixtures/webpush.xml b/tests/controller/fixtures/webpush.xml index f9f67fe..7f025a6 100644 --- a/tests/controller/fixtures/webpush.xml +++ b/tests/controller/fixtures/webpush.xml @@ -25,7 +25,7 @@ 2 - + 488c17afe4f18714c235b395e21b78df1c3d78bf1e13d0633ed9425d2eebf967 2 4 diff --git a/tests/javascript/push_worker_test.js b/tests/javascript/push_worker_test.js new file mode 100644 index 0000000..3ada31e --- /dev/null +++ b/tests/javascript/push_worker_test.js @@ -0,0 +1,90 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const vm = require('node:vm'); + +const workerSource = fs.readFileSync(process.argv[2], 'utf8'); + +async function clickNotification(notificationUrl, workerUrl) { + const listeners = {}; + const openedUrls = []; + const self = { + location: new URL(workerUrl), + clients: { + claim() {}, + openWindow(url) { + openedUrls.push(url); + return Promise.resolve(null); + }, + }, + registration: { + showNotification() {}, + update() {}, + }, + skipWaiting() {}, + addEventListener(type, listener) { + listeners[type] = listener; + }, + }; + + vm.runInNewContext(workerSource, {console, self, URL}, {filename: process.argv[2]}); + + let pending; + listeners.notificationclick({ + notification: { + data: {url: notificationUrl}, + close() {}, + }, + waitUntil(promise) { + pending = promise; + }, + }); + + if (pending) { + await pending; + } + + return openedUrls; +} + +async function run() { + const subfolderWorker = 'http://localhost:8090/forum/app.php/user/push/worker'; + const notificationUrl = './../../../ucp.php?i=pm&mode=view&p=4'; + + assert.deepEqual( + await clickNotification(notificationUrl, subfolderWorker), + ['http://localhost:8090/forum/ucp.php?i=pm&mode=view&p=4'], + 'relative notification URLs must resolve from worker route on subfolder boards', + ); + assert.deepEqual( + await clickNotification(notificationUrl, 'http://localhost:8090/app.php/user/push/worker'), + ['http://localhost:8090/ucp.php?i=pm&mode=view&p=4'], + 'relative notification URLs must still work on root boards', + ); + assert.deepEqual( + await clickNotification('https://example.com/topic/1', subfolderWorker), + ['https://example.com/topic/1'], + 'absolute HTTPS notification URLs must remain unchanged', + ); + assert.deepEqual( + await clickNotification('http://example.com/topic/1', subfolderWorker), + ['http://example.com/topic/1'], + 'absolute HTTP notification URLs must remain unchanged', + ); + assert.deepEqual( + await clickNotification('javascript:alert(1)', subfolderWorker), + [], + 'non-HTTP notification URLs must be rejected', + ); + assert.deepEqual( + await clickNotification('http://[invalid', subfolderWorker), + [], + 'malformed notification URLs must be ignored', + ); +} + +run().catch(error => { + console.error(error); + process.exitCode = 1; +}); diff --git a/tests/javascript/push_worker_test.php b/tests/javascript/push_worker_test.php new file mode 100644 index 0000000..1678eb3 --- /dev/null +++ b/tests/javascript/push_worker_test.php @@ -0,0 +1,29 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\webpushnotifications\tests\javascript; + +use Symfony\Component\Process\Process; + +class push_worker_test extends \phpbb_test_case +{ + public function test_notification_click_url_resolution(): void + { + $process = new Process([ + 'node', + __DIR__ . '/push_worker_test.js', + __DIR__ . '/../../styles/all/template/push_worker.js.twig', + ]); + $process->setTimeout(10); + $process->run(); + + self::assertSame(0, $process->getExitCode(), $process->getErrorOutput() . $process->getOutput()); + } +} diff --git a/tests/javascript/webpush_test.js b/tests/javascript/webpush_test.js new file mode 100644 index 0000000..378853d --- /dev/null +++ b/tests/javascript/webpush_test.js @@ -0,0 +1,227 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const vm = require('node:vm'); + +const webpushSource = fs.readFileSync(process.argv[2], 'utf8'); +const endpoint = 'https://push.example.test/subscription/old'; +const currentVapidKey = 'AQID'; + +function createButton(id) { + const listeners = {}; + return { + id, + disabled: false, + hidden: false, + type: 'button', + classList: { + contains() { + return false; + }, + toggle(name, state) { + if (name === 'hidden') { + this.owner.hidden = state; + } + }, + }, + addEventListener(type, listener) { + listeners[type] = listener; + }, + removeEventListener(type) { + delete listeners[type]; + }, + getAttribute() { + return 'Web Push error'; + }, + listeners, + }; +} + +async function settle() { + await new Promise(resolve => setImmediate(resolve)); + await new Promise(resolve => setImmediate(resolve)); +} + +async function runScenario({stored = [], expirationTime = 0, applicationServerKey = [1, 2, 3], click = false}) { + const calls = {fetch: 0, subscribe: 0, unsubscribe: 0}; + const subscribeButton = createButton('subscribe_webpush'); + const unsubscribeButton = createButton('unsubscribe_webpush'); + subscribeButton.classList.owner = subscribeButton; + unsubscribeButton.classList.owner = unsubscribeButton; + + const popup = { + style: {display: 'none'}, + addEventListener() {}, + }; + const storage = new Map(); + let activeSubscription; + + function makeSubscription(subscriptionEndpoint, key, expires = 0) { + return { + endpoint: subscriptionEndpoint, + expirationTime: expires, + options: {applicationServerKey: Uint8Array.from(key).buffer}, + toJSON() { + return { + endpoint: this.endpoint, + expirationTime: this.expirationTime, + keys: {p256dh: 'public-key', auth: 'auth-token'}, + }; + }, + async unsubscribe() { + calls.unsubscribe++; + if (activeSubscription === this) { + activeSubscription = null; + } + return true; + }, + }; + } + + activeSubscription = makeSubscription(endpoint, applicationServerKey, expirationTime); + const registration = { + pushManager: { + async getSubscription() { + return activeSubscription; + }, + async subscribe() { + calls.subscribe++; + activeSubscription = makeSubscription('https://push.example.test/subscription/new', [1, 2, 3]); + return activeSubscription; + }, + }, + }; + + const document = { + readyState: 'complete', + querySelectorAll(selector) { + return selector.includes('unsubscribe') ? [unsubscribeButton] : [subscribeButton]; + }, + querySelector() { + return null; + }, + getElementById(id) { + return id === 'wpn_popup_prompt' ? popup : null; + }, + addEventListener() {}, + removeEventListener() {}, + }; + const window = { + isSecureContext: true, + PushManager: function PushManager() {}, + atob(value) { + return Buffer.from(value, 'base64').toString('binary'); + }, + btoa(value) { + return Buffer.from(value, 'binary').toString('base64'); + }, + }; + const phpbb = { + alert() {}, + alertTime: 0, + loadingIndicator() { + return {fadeOut() {}}; + }, + }; + const context = { + console, + document, + fetch: async() => { + calls.fetch++; + return { + async json() { + return {success: true, form_tokens: {creation_time: 2, form_token: 'new-token'}}; + }, + }; + }, + FormData: class FormData { + append() {} + }, + localStorage: { + getItem(key) { + return storage.get(key) || null; + }, + setItem(key, value) { + storage.set(key, value); + }, + removeItem(key) { + storage.delete(key); + }, + }, + navigator: { + serviceWorker: { + async register() { + return registration; + }, + async getRegistration() { + return registration; + }, + }, + }, + Notification: { + permission: 'granted', + async requestPermission() { + return 'granted'; + }, + }, + phpbb, + phpbbWebpushOptions: { + serviceWorkerUrl: '/push-worker', + subscribeUrl: '/subscribe', + unsubscribeUrl: '/unsubscribe', + togglePopupUrl: '/toggle-popup', + ajaxErrorTitle: 'Error', + unsubscribeError: 'Unsubscribe error', + vapidPublicKey: currentVapidKey, + formTokens: {creationTime: 1, formToken: 'token'}, + subscriptions: stored.map(storedEndpoint => ({endpoint: storedEndpoint, expirationTime: 0})), + }, + setTimeout(callback) { + callback(); + }, + Uint8Array, + window, + }; + + vm.runInNewContext(webpushSource, context, {filename: process.argv[2]}); + await settle(); + + if (click) { + await subscribeButton.listeners.click({ + currentTarget: subscribeButton, + preventDefault() {}, + stopPropagation() {}, + }); + await settle(); + } + + return {calls, popup, subscribeButton, unsubscribeButton}; +} + +async function run() { + const foreign = await runScenario({stored: []}); + assert.deepEqual(foreign.calls, {fetch: 0, subscribe: 0, unsubscribe: 0}); + assert.equal(foreign.subscribeButton.hidden, false, 'foreign subscription must leave subscribe control visible'); + assert.equal(foreign.unsubscribeButton.hidden, true, 'foreign subscription must not appear owned'); + assert.equal(foreign.popup.style.display, 'flex', 'foreign subscription must use normal opt-in prompt'); + + const owned = await runScenario({stored: [endpoint]}); + assert.deepEqual(owned.calls, {fetch: 0, subscribe: 0, unsubscribe: 0}); + assert.equal(owned.subscribeButton.hidden, true, 'owned valid subscription must appear subscribed'); + assert.equal(owned.unsubscribeButton.hidden, false, 'owned valid subscription must allow unsubscribe'); + + const staleKey = await runScenario({stored: [endpoint], applicationServerKey: [4, 5, 6]}); + assert.deepEqual(staleKey.calls, {fetch: 1, subscribe: 1, unsubscribe: 1}, 'owned stale VAPID subscription must refresh'); + + const expired = await runScenario({stored: [endpoint], expirationTime: Date.now() - 1000}); + assert.deepEqual(expired.calls, {fetch: 1, subscribe: 1, unsubscribe: 1}, 'owned expired subscription must refresh'); + + const explicit = await runScenario({stored: [], click: true}); + assert.deepEqual(explicit.calls, {fetch: 1, subscribe: 1, unsubscribe: 1}, 'explicit opt-in may replace foreign subscription'); +} + +run().catch(error => { + console.error(error); + process.exitCode = 1; +}); diff --git a/tests/javascript/webpush_test.php b/tests/javascript/webpush_test.php new file mode 100644 index 0000000..9a12584 --- /dev/null +++ b/tests/javascript/webpush_test.php @@ -0,0 +1,29 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\webpushnotifications\tests\javascript; + +use Symfony\Component\Process\Process; + +class webpush_test extends \phpbb_test_case +{ + public function test_subscription_ownership_and_refresh_behaviour(): void + { + $process = new Process([ + 'node', + __DIR__ . '/webpush_test.js', + __DIR__ . '/../../styles/all/template/webpush.js', + ]); + $process->setTimeout(10); + $process->run(); + + self::assertSame(0, $process->getExitCode(), $process->getErrorOutput() . $process->getOutput()); + } +} diff --git a/tests/notification/notification_method_webpush_test.php b/tests/notification/notification_method_webpush_test.php index 155d70b..501b04c 100644 --- a/tests/notification/notification_method_webpush_test.php +++ b/tests/notification/notification_method_webpush_test.php @@ -151,7 +151,7 @@ protected function setUp(): void $phpbb_container->setParameter('tables.phpbb.wpn.notification_push', 'phpbb_wpn_notification_push'); $phpbb_container->setParameter('tables.phpbb.wpn.push_subscriptions', 'phpbb_wpn_push_subscriptions'); - $this->notification_method_webpush = new webpush( + $this->notification_method_webpush = new integration_testable_webpush( $phpbb_container->get('config'), $phpbb_container->get('dbal.conn'), $phpbb_container->get('log'), @@ -787,6 +787,40 @@ public function test_invalid_subscription_does_not_interrupt_valid_delivery(): v $this->db->sql_freeresult($result); } + public function test_malformed_endpoint_does_not_interrupt_other_users_delivery(): void + { + $malformed_subscription = $this->create_subscription_for_user(2); + $valid_subscription = $this->create_subscription_for_user(3); + $push_subscriptions_table = $this->container->getParameter('tables.phpbb.wpn.push_subscriptions'); + $malformed_endpoint = 'https://evil.example.com\\.push.apple.com/x'; + + $sql = 'UPDATE ' . $push_subscriptions_table . " + SET endpoint = '" . $this->db->sql_escape($malformed_endpoint) . "' + WHERE endpoint = '" . $this->db->sql_escape($malformed_subscription['endpoint']) . "'"; + $this->db->sql_query($sql); + + $this->notifications->add_notifications('notification.type.post', [ + 'post_time' => 1349413322, + 'poster_id' => 1, + 'topic_title' => '', + 'post_subject' => '', + 'post_username' => '', + 'forum_name' => '', + 'forum_id' => '1', + 'post_id' => '2', + 'topic_id' => '1', + ]); + + $this->assertNotEmpty($this->get_messages_for_subscription($valid_subscription['clientHash'])); + + $sql = 'SELECT COUNT(*) AS subscription_count + FROM ' . $push_subscriptions_table . " + WHERE endpoint = '" . $this->db->sql_escape($malformed_endpoint) . "'"; + $result = $this->db->sql_query($sql); + $this->assertSame(0, (int) $this->db->sql_fetchfield('subscription_count')); + $this->db->sql_freeresult($result); + } + public function test_get_ucp_template_data_uses_millisecond_expiration_time(): void { $this->user->data['user_id'] = 2; @@ -845,36 +879,34 @@ public function test_get_ucp_template_data_separates_webpush_availability_from_d } /** - * Test is_subscription_unauthorized method with various HTTP status codes + * Authentication failures retain subscription ownership while expired subscriptions are removed */ - public function test_is_subscription_unauthorized(): void + public function test_should_remove_subscription(): void { - $reflection = new \ReflectionMethod($this->notification_method_webpush, 'is_subscription_unauthorized'); + $reflection = new \ReflectionMethod($this->notification_method_webpush, 'should_remove_subscription'); $reflection->setAccessible(true); - // Test 401 status (should return true) + // 401/403 identify VAPID authentication failures, not expired endpoints. $response_401 = $this->createMockResponse(401); $request_401 = $this->createMockRequest(); $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'); + $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_401), 'Expected 401 subscription ownership to be retained'); - // 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'); + $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_403), 'Expected 403 subscription ownership to be retained'); - // Test 404 status (should return false, handled by isSubscriptionExpired) + // 404/410 identify expired endpoints and remain eligible for cleanup. $response_404 = $this->createMockResponse(404); $request_404 = $this->createMockRequest(); $report_404 = new \Minishlink\WebPush\MessageSentReport($request_404, $response_404, false, 'Not Found'); - $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_404), 'Expected 404 to not be treated as unauthorized'); + $this->assertTrue($reflection->invoke($this->notification_method_webpush, $report_404), 'Expected 404 subscription to be removed'); - // Test 410 status (should return false, handled by isSubscriptionExpired) $response_410 = $this->createMockResponse(410); $request_410 = $this->createMockRequest(); $report_410 = new \Minishlink\WebPush\MessageSentReport($request_410, $response_410, false, 'Gone'); - $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_410), 'Expected 410 to not be treated as unauthorized'); + $this->assertTrue($reflection->invoke($this->notification_method_webpush, $report_410), 'Expected 410 subscription to be removed'); // Test 429 status (should return false, temporary error) $response_429 = $this->createMockResponse(429); @@ -1332,3 +1364,12 @@ public function test_export_data_with_migration(): void ); } } + +class integration_testable_webpush extends webpush +{ + protected function is_valid_endpoint_url(string $endpoint): bool + { + return strpos($endpoint, 'http://localhost:9012/notify/') === 0 + || parent::is_valid_endpoint_url($endpoint); + } +} diff --git a/tests/notification/webpush_branches_test.php b/tests/notification/webpush_branches_test.php index 034e2f4..372ab70 100644 --- a/tests/notification/webpush_branches_test.php +++ b/tests/notification/webpush_branches_test.php @@ -110,6 +110,31 @@ public function test_oversized_payload_logs_failure_without_removing_subscriptio self::assertSame([], $method->removed_subscriptions); } + public function test_invalid_vapid_config_logs_failure_without_throwing(): void + { + global $config; + + $GLOBALS['phpbb_wpn_test_banned_users'] = []; + $log = $this->createMock(\phpbb\log\log_interface::class); + $log->expects(self::once())->method('add')->with( + 'critical', + ANONYMOUS, + '', + 'LOG_WEBPUSH_MESSAGE_FAIL', + false, + self::callback(static function(array $arguments): bool { + return isset($arguments[0]) && strpos($arguments[0], 'Public key should be 65 bytes long') !== false; + }) + ); + $method = $this->create_method($log); + $config['wpn_webpush_vapid_public'] = str_repeat('A', 30); + $method->add_to_queue($this->create_notification(2)); + + $method->send_queued_notifications(); + + self::assertSame([], $method->get_queue()); + } + private function create_method($log = null, $user_loader = null, $assets_version = 1): testable_webpush { global $config, $phpbb_root_path, $phpEx, $user; diff --git a/tests/subscription/validator_test.php b/tests/subscription/validator_test.php index 65f798a..c6d570e 100644 --- a/tests/subscription/validator_test.php +++ b/tests/subscription/validator_test.php @@ -10,14 +10,44 @@ namespace phpbb\webpushnotifications\tests\subscription; +use Base64Url\Base64Url; use phpbb\webpushnotifications\subscription\validator; require_once __DIR__ . '/../../vendor/autoload.php'; class validator_test extends \phpbb_test_case { + private const VALID_VAPID_PRIVATE_KEY = 'SrlbBEVgibWmKHYbDPu4Y2XvDWPjeGcc9fC16jq01xU'; private const VALID_PUBLIC_KEY = 'BGsX0fLhLEJH-Lzm5WOkQPJ3A32BLeszoPShOUXYmMKWT-NC4v4af5uO5-tKfA-eFivOM1drMV7Oy7ZAaDe_UfU'; private const VALID_AUTH_TOKEN = 'AAAAAAAAAAAAAAAAAAAAAA'; + private const VALID_VAPID_PUBLIC_KEY = 'BIcGkq1Ncj3a2-J0UW-1A0NETLjvxZzNLiYBiPVMKNjgwmwPi5jyK87VfS4FZn9n7S9pLMQzjV3LmFuOnRSOvmI'; + + public function test_valid_vapid_keys(): void + { + self::assertTrue(validator::is_valid_vapid_keys(self::VALID_VAPID_PUBLIC_KEY, self::VALID_VAPID_PRIVATE_KEY)); + } + + public function test_invalid_vapid_keys(): void + { + self::assertFalse(validator::is_valid_vapid_keys(str_repeat('A', 30), str_repeat('B', 30))); + } + + public function invalid_vapid_key_data(): array + { + return [ + 'mismatched pair' => [self::VALID_VAPID_PUBLIC_KEY, 'IE5OYlmfWsMbBU1lzvr0bxrxVAXIteSkAnwGlZIhmRk'], + 'off-curve public point' => [Base64Url::encode("\x04" . str_repeat("\x00", 64)), self::VALID_VAPID_PRIVATE_KEY], + 'invalid public encoding' => [str_repeat('*', 87), self::VALID_VAPID_PRIVATE_KEY], + ]; + } + + /** + * @dataProvider invalid_vapid_key_data + */ + public function test_invalid_vapid_public_keys(string $public_key, string $private_key): void + { + self::assertFalse(validator::is_valid_vapid_keys($public_key, $private_key)); + } public function valid_key_data(): array { @@ -56,4 +86,27 @@ public function test_invalid_keys(string $public_key, string $auth_token): void { self::assertFalse(validator::is_valid($public_key, $auth_token)); } + + public function endpoint_url_data(): array + { + return [ + 'allowed exact host' => ['https://fcm.googleapis.com/fcm/send/test', true], + 'allowed wildcard host' => ['https://api.push.apple.com/3/device/test', true], + 'insecure scheme' => ['http://api.push.apple.com/3/device/test', false], + 'nonstandard port' => ['https://api.push.apple.com:8443/3/device/test', false], + 'username' => ['https://user@api.push.apple.com/3/device/test', false], + 'password' => ['https://user:pass@api.push.apple.com/3/device/test', false], + 'malformed wildcard host' => ['https://evil.example.com\\.push.apple.com/x', false], + 'percent-encoded wildcard host' => ['https://evil%2epush.apple.com/x', false], + 'invalid hostname label' => ['https://-evil.push.apple.com/x', false], + ]; + } + + /** + * @dataProvider endpoint_url_data + */ + public function test_valid_endpoint_url(string $endpoint, bool $expected): void + { + self::assertSame($expected, validator::is_valid_endpoint_url($endpoint)); + } } diff --git a/tests/system/simple_test.php b/tests/system/simple_test.php index 2baf7ea..9784731 100644 --- a/tests/system/simple_test.php +++ b/tests/system/simple_test.php @@ -70,6 +70,25 @@ public function test_decode_entities() { self::assertSame('A&B "quote"', \phpbb\webpushnotifications\ext::decode_entities('A&B "quote"', ENT_QUOTES)); } + + public function mcp_log_language_data(): array + { + return [ + 'English' => ['en', 'Removed Web Push subscription:
» %s'], + 'Russian' => ['ru', 'Удалена подписка на браузерные push—уведомления:
» %s'], + ]; + } + + /** + * @dataProvider mcp_log_language_data + */ + public function test_mcp_log_language(string $language, string $expected): void + { + $lang = []; + include __DIR__ . '/../../language/' . $language . '/info_mcp_webpushnotifications.php'; + + self::assertSame($expected, $lang['LOG_WEBPUSH_SUBSCRIPTION_REMOVED']); + } } class testable_ext extends \phpbb\webpushnotifications\ext diff --git a/ucp/controller/webpush.php b/ucp/controller/webpush.php index ecae851..ebb7bc8 100644 --- a/ucp/controller/webpush.php +++ b/ucp/controller/webpush.php @@ -41,6 +41,7 @@ class webpush public const PUSH_SERVICE_WHITELIST = [ 'android.googleapis.com', 'fcm.googleapis.com', + 'jmt17.google.com', 'updates.push.services.mozilla.com', 'updates-autopush.stage.mozaws.net', 'updates-autopush.dev.mozaws.net', @@ -247,10 +248,15 @@ private function get_notification_data(string $notification_data): string { $row_data = json_decode($notification_data, true); - // Old notification data is pre-parsed and just needs to be returned + // Old notification data is pre-parsed, but its URL is relative to the board root. if (isset($row_data['heading'])) { - return $notification_data; + if (is_string($row_data['url'] ?? null)) + { + $row_data['url'] = $this->normalize_notification_url($row_data['url']); + } + + return json_encode($row_data); } // Get notification from row_data @@ -261,13 +267,30 @@ private function get_notification_data(string $notification_data): string return json_encode([ 'heading' => ext::decode_entities($this->config['sitename'], ENT_QUOTES), - 'title' => strip_tags(ext::decode_entities($notification->get_title())), - 'text' => strip_tags(ext::decode_entities($notification->get_reference())), - 'url' => htmlspecialchars_decode($notification->get_url()), + 'title' => ext::decode_entities(strip_tags($notification->get_title()), ENT_QUOTES), + 'text' => ext::decode_entities(strip_tags($notification->get_reference()), ENT_QUOTES), + 'url' => $this->normalize_notification_url(htmlspecialchars_decode($notification->get_url())), 'avatar' => $this->prepare_avatar($notification->get_avatar()), ]); } + /** + * Convert a board-relative notification URL to an absolute URL + * + * @param string $url Notification URL + * @return string + */ + protected function normalize_notification_url(string $url): string + { + if (parse_url($url, PHP_URL_SCHEME) !== null) + { + return $url; + } + + $relative_url = ltrim($url, '/'); + return $this->get_board_url() . preg_replace('#^(?:(?:\.\.?)/)+#', '', $relative_url); + } + /** * Handle request to push worker javascript * @@ -280,7 +303,6 @@ public function worker(): Response { $content = $this->template->render('@phpbb_webpushnotifications/push_worker.js.twig', [ 'U_WEBPUSH_GET_NOTIFICATION' => $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_get_notification_controller'), - 'U_BOARD_URL' => generate_board_url(), 'ASSETS_VERSION' => $this->config['assets_version'], ]); @@ -306,17 +328,12 @@ public function worker(): Response */ public function is_valid_endpoint(string $endpoint): bool { - $parts = parse_url($endpoint); - - if (!is_array($parts) - || strtolower($parts['scheme'] ?? '') !== 'https' - || (isset($parts['port']) && $parts['port'] !== 443) - || !is_string($parts['host'] ?? null) - || empty($parts['host'])) + if (!subscription_validator::is_valid_endpoint_url($endpoint)) { return false; } + $parts = parse_url($endpoint); $host = strtolower($parts['host']); if (in_array($host, self::PUSH_SERVICE_WHITELIST, true)) @@ -536,7 +553,7 @@ protected function prepare_avatar($avatar): array } /** - * Returns the board url (and caches it in the function) + * Returns the board URL * * @return string the generated board url */