Audit Fixes

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())

language/en/info_mcp_webpushnotifications.php

@@ -0,0 +1,23 @@
+<?php
+/**
+ *
+ * phpBB Browser Push Notifications. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2026, phpBB Limited <https://www.phpbb.com>
+ * @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' => '<strong>Removed Web Push subscription:</strong><br>» %s',
+]);

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'=> '<strong>Important:</strong> 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.',

language/ru/info_mcp_webpushnotifications.php

@@ -0,0 +1,23 @@
+<?php
+/**
+ *
+ * phpBB Browser Push Notifications. An extension for the phpBB Forum Software package.
+ *
+ * @copyright (c) 2026, phpBB Limited <https://www.phpbb.com>
+ * @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' => '<strong>Удалена подписка на браузерные push—уведомления:</strong><br>» %s',
+]);

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'=> '<strong>Важно:</strong> изменение любого ключа делает недействительными все существующие подписки на браузерные push—уведомления.',
 	'WEBPUSH_VAPID_PUBLIC'			=> 'Публичный ключ идентификации сервера',
 	'WEBPUSH_VAPID_PUBLIC_EXPLAIN'	=> 'Передаётся службам push—уведомлений для аутентификации сообщений, отправляемых конференцией.',

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());
 	}
 
 	/**

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));
 		}

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;

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
 	 *

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<br>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)