From a26047c819670d2240930321ebb19790c8ca2b30 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 16 Oct 2026 14:50:07 -0700 Subject: [PATCH 01/18] ACP saves now whitelist Web Push settings only ACP --- acp/wpn_acp_module.php | 9 ++++++--- tests/acp/acp_module_test.php | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/acp/wpn_acp_module.php b/acp/wpn_acp_module.php index 3b7e106..1c0979d 100644 --- a/acp/wpn_acp_module.php +++ b/acp/wpn_acp_module.php @@ -178,7 +178,7 @@ public function save_settings() unset($display_settings['wpn_webpush_vapid_private'], $config_array['wpn_webpush_vapid_private']); } - if ($config_array['wpn_webpush_enable']) + if (!empty($config_array['wpn_webpush_enable'])) { // Validate config values validate_config_vars($display_settings, $config_array, $this->errors); @@ -191,9 +191,12 @@ public function save_settings() $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_CONFIG_WEBPUSH'); - foreach ($config_array as $config_name => $config_value) + foreach (array_keys($display_settings) as $config_name) { - $this->config->set($config_name, $config_value); + if (array_key_exists($config_name, $config_array)) + { + $this->config->set($config_name, $config_array[$config_name]); + } } trigger_error($this->lang->lang('CONFIG_UPDATED') . adm_back_link($this->u_action), E_USER_NOTICE); diff --git a/tests/acp/acp_module_test.php b/tests/acp/acp_module_test.php index 4abc8ad..aa93cb2 100644 --- a/tests/acp/acp_module_test.php +++ b/tests/acp/acp_module_test.php @@ -468,6 +468,28 @@ public function test_save_settings_requires_private_key_when_none_is_stored(): v self::assertSame('', $this->config['wpn_webpush_vapid_private']); } + public function test_save_settings_ignores_unrelated_config(): void + { + $this->config['allow_privmsg'] = 1; + $this->request->method('variable') + ->with('config', ['' => ''], true) + ->willReturn([ + 'wpn_webpush_enable' => 0, + 'wpn_webpush_vapid_public' => 'existing-public-key', + 'wpn_webpush_vapid_private' => '', + 'wpn_webpush_dropdown_subscribe' => 1, + 'wpn_webpush_method_enabled' => 1, + 'wpn_webpush_popup_prompt' => 1, + 'allow_privmsg' => 0, + ]); + + $this->assertTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED', function() { + $this->create_module()->save_settings(); + }); + + self::assertSame(1, $this->config['allow_privmsg']); + } + public function pwa_save_data(): array { $valid = [ From 7b23185d4d8ee7582b1578ef4a67414615391642 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 16 Oct 2026 14:55:59 -0700 Subject: [PATCH 02/18] Scalar/malformed JSON returns controlled HTTP 400 responses Scalar --- json/sanitizer.php | 2 +- tests/controller/controller_webpush_test.php | 34 +++++++++++++++++ tests/json/sanitizer_test.php | 40 ++++++++++++++++++++ ucp/controller/webpush.php | 4 ++ 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 tests/json/sanitizer_test.php diff --git a/json/sanitizer.php b/json/sanitizer.php index b8cd3b0..a8c7a6c 100644 --- a/json/sanitizer.php +++ b/json/sanitizer.php @@ -49,6 +49,6 @@ public static function sanitize(array $data) : array public static function decode(string $json) : array { $data = json_decode($json, true); - return !empty($data) ? self::sanitize($data) : []; + return is_array($data) && !empty($data) ? self::sanitize($data) : []; } } diff --git a/tests/controller/controller_webpush_test.php b/tests/controller/controller_webpush_test.php index 648c27c..7f353e4 100644 --- a/tests/controller/controller_webpush_test.php +++ b/tests/controller/controller_webpush_test.php @@ -437,6 +437,40 @@ public function test_sub_unsubscribe_success() $this->assertEmpty($this->get_subscription_data()); } + public function scalar_json_data(): array + { + return [ + 'boolean' => ['true'], + 'number' => ['42'], + 'string' => ['"subscription"'], + 'null' => ['null'], + ]; + } + + /** + * @dataProvider scalar_json_data + */ + public function test_subscribe_rejects_scalar_json(string $payload): void + { + $symfony_request = $this->create_subscription_request($payload); + + $this->expectException(http_exception::class); + $this->expectExceptionMessage('WEBPUSH_INVALID_ENDPOINT'); + $this->controller->subscribe($symfony_request); + } + + /** + * @dataProvider scalar_json_data + */ + public function test_unsubscribe_rejects_scalar_json(string $payload): void + { + $symfony_request = $this->create_subscription_request($payload); + + $this->expectException(http_exception::class); + $this->expectExceptionMessage('WEBPUSH_INVALID_ENDPOINT'); + $this->controller->unsubscribe($symfony_request); + } + public function test_subscribe_replaces_previous_subscription() { $this->form_helper->method('check_form_tokens')->willReturn(true); diff --git a/tests/json/sanitizer_test.php b/tests/json/sanitizer_test.php new file mode 100644 index 0000000..21d2a9f --- /dev/null +++ b/tests/json/sanitizer_test.php @@ -0,0 +1,40 @@ + ['true'], + 'number' => ['42'], + 'string' => ['"value"'], + 'null' => ['null'], + 'invalid' => ['not-json'], + ]; + } + + /** + * @dataProvider non_array_json_data + */ + public function test_decode_returns_empty_array_for_non_array_json(string $json): void + { + self::assertSame([], sanitizer::decode($json)); + } + + public function test_decode_sanitizes_array_json(): void + { + self::assertSame(['value' => '<script>'], sanitizer::decode('{"value":" {% endif %} +{% if NOTIFICATIONS_WEBPUSH_ENABLE %} + {% include '@phpbb_webpushnotifications/webpush_popup.html' %} +{% endif %} + {% if S_PWA_SHOW_BANNER %} {% INCLUDEJS '@phpbb_webpushnotifications/pwa_install_banner.js' %} {% INCLUDECSS '@phpbb_webpushnotifications/phpbb_pwa.css' %} diff --git a/styles/all/template/event/overall_header_head_append.html b/styles/all/template/event/overall_header_head_append.html index a7bd27f..504b8e6 100644 --- a/styles/all/template/event/overall_header_head_append.html +++ b/styles/all/template/event/overall_header_head_append.html @@ -13,5 +13,4 @@ {% if NOTIFICATIONS_WEBPUSH_ENABLE %} {% include '@phpbb_webpushnotifications/ucp_notifications_webpush.html' %} - {% include '@phpbb_webpushnotifications/webpush_popup.html' %} {% endif %} From 9a89fb905d2ffe5fcf9164133d92cc49040594ea Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 16 Oct 2026 14:58:54 -0700 Subject: [PATCH 04/18] Unsubscribe fallback translated in English/Russian --- language/en/webpushnotifications_module_ucp.php | 1 + language/ru/webpushnotifications_module_ucp.php | 1 + styles/all/template/ucp_notifications_webpush.html | 1 + styles/all/template/webpush.js | 6 +++++- 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/language/en/webpushnotifications_module_ucp.php b/language/en/webpushnotifications_module_ucp.php index 3bc83c1..99bceba 100644 --- a/language/en/webpushnotifications_module_ucp.php +++ b/language/en/webpushnotifications_module_ucp.php @@ -46,6 +46,7 @@ 'NOTIFY_WEBPUSH_ENABLE_EXPLAIN' => 'Enabling push notifications will activate them on this device only. You can turn off notifications at any time through your browser settings or by clicking the button above. Additionally, if no web push notification types are selected below, you will not receive any web push notifications.', 'NOTIFY_WEBPUSH_SUBSCRIBE' => 'Enable to subscribe', 'NOTIFY_WEBPUSH_UNSUBSCRIBE' => 'Disable to unsubscribe', + 'NOTIFY_WEBPUSH_UNSUBSCRIBE_ERROR' => 'Unable to unsubscribe from Web Push notifications.', 'NOTIFY_WEBPUSH_DROPDOWN_TITLE' => 'Visit notifications settings to set your preferred push notifications.', 'NOTIFY_WEBPUSH_DENIED' => 'You have denied notifications from this site. To enable push notifications, allow notifications from this site in your browser settings.', 'NOTIFY_WEBPUSH_NOT_SUPPORTED' => 'Push notifications not supported', diff --git a/language/ru/webpushnotifications_module_ucp.php b/language/ru/webpushnotifications_module_ucp.php index 575dbf8..08bbfc5 100644 --- a/language/ru/webpushnotifications_module_ucp.php +++ b/language/ru/webpushnotifications_module_ucp.php @@ -46,6 +46,7 @@ 'NOTIFY_WEBPUSH_ENABLE_EXPLAIN' => 'Включение браузерных push—уведомлений активирует их только на данном устройстве. Вы сможете отключить их в любое время через настройки браузера или с помощью кнопки «Отключить» выше, которая появится после включения браузерных push—уведомлений. Если не выбран ни один из типов уведомлений в категории «Браузерные» ниже, вы также не будете получать браузерные push—уведомления.', 'NOTIFY_WEBPUSH_SUBSCRIBE' => 'Подписаться', 'NOTIFY_WEBPUSH_UNSUBSCRIBE' => 'Отписаться', + 'NOTIFY_WEBPUSH_UNSUBSCRIBE_ERROR' => 'Не удалось отписаться от Web Push-уведомлений.', 'NOTIFY_WEBPUSH_DROPDOWN_TITLE' => 'Посетите настройки уведомлений, чтобы установить предпочтительные типы браузерных уведомлений.', 'NOTIFY_WEBPUSH_DENIED' => 'Вы запретили браузерные уведомления для данного сайта. Для того, чтобы подписаться, необходимо их разрешить в настройках браузера.', 'NOTIFY_WEBPUSH_NOT_SUPPORTED' => 'Не поддерживается', diff --git a/styles/all/template/ucp_notifications_webpush.html b/styles/all/template/ucp_notifications_webpush.html index 38929ef..2b896a4 100644 --- a/styles/all/template/ucp_notifications_webpush.html +++ b/styles/all/template/ucp_notifications_webpush.html @@ -5,6 +5,7 @@ unsubscribeUrl: '{{ U_WEBPUSH_UNSUBSCRIBE|e('js') }}', togglePopupUrl: '{{ U_WEBPUSH_TOGGLE_POPUP|e('js') }}', ajaxErrorTitle: '{{ lang_js('AJAX_ERROR_TITLE') }}', + unsubscribeError: '{{ lang_js('NOTIFY_WEBPUSH_UNSUBSCRIBE_ERROR') }}', vapidPublicKey: '{{ VAPID_PUBLIC_KEY|e('js') }}', formTokens: { creationTime: {{ WEBPUSH_FORM_TOKENS.creation_time|int }}, diff --git a/styles/all/template/webpush.js b/styles/all/template/webpush.js index eb1edde..88c0e7e 100644 --- a/styles/all/template/webpush.js +++ b/styles/all/template/webpush.js @@ -24,6 +24,9 @@ function PhpbbWebpush() { /** @type {string} Title of an error message */ let ajaxErrorTitle = ''; + /** @type {string} Fallback unsubscribe error message */ + let unsubscribeError = ''; + /** @type {string} VAPID public key */ let vapidPublicKey = ''; @@ -55,6 +58,7 @@ function PhpbbWebpush() { this.formTokens = options.formTokens; subscriptions = options.subscriptions; ajaxErrorTitle = options.ajaxErrorTitle; + unsubscribeError = options.unsubscribeError; vapidPublicKey = options.vapidPublicKey; subscribeButton = document.querySelector('#subscribe_webpush'); @@ -487,7 +491,7 @@ function PhpbbWebpush() { // Ignore JSON parsing failures and fall back below. } if (!response.ok || !data || !data.success) { - throw new Error(data && data.message ? data.message : 'Unsubscribe failed.'); + throw new Error(data && data.message ? data.message : unsubscribeError); } const unsubscribed = await subscription.unsubscribe(); From 2adc9cf92e80de4ecf0afacbc9c1c06512ac6047 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 16 Oct 2026 15:07:05 -0700 Subject: [PATCH 05/18] Subscription hardening subs subs Subs subs --- notification/method/webpush.php | 38 +++++++-- subscription/validator.php | 80 +++++++++++++++++++ tests/controller/controller_webpush_test.php | 66 ++++++++++++--- .../notification_method_webpush_test.php | 35 ++++++++ tests/notification/webpush_branches_test.php | 44 +++++++++- tests/subscription/validator_test.php | 59 ++++++++++++++ ucp/controller/webpush.php | 15 ++-- 7 files changed, 311 insertions(+), 26 deletions(-) create mode 100644 subscription/validator.php create mode 100644 tests/subscription/validator_test.php diff --git a/notification/method/webpush.php b/notification/method/webpush.php index 42e2f69..11c2a30 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -21,6 +21,7 @@ use phpbb\user_loader; use phpbb\webpushnotifications\form\form_helper; use phpbb\webpushnotifications\json\sanitizer as json_sanitizer; +use phpbb\webpushnotifications\subscription\validator as subscription_validator; /** * Web Push notification method class @@ -208,6 +209,31 @@ protected function notify_using_webpush(): void // Get subscriptions for users $user_subscription_map = $this->get_user_subscription_map($notify_users); + $remove_subscriptions = []; + + // Validate each stored subscription once so malformed keys 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) + || !is_string($subscription['auth'] ?? null) + || !subscription_validator::is_valid($subscription['p256dh'], $subscription['auth'])) + { + $remove_subscriptions[$subscription['subscription_id']] = $subscription['subscription_id']; + $this->log->add('user', $user['user_id'], $user['user_ip'] ?? '', 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED', false, [ + 'reportee_id' => $user['user_id'], + $user['username'], + ]); + continue; + } + + $valid_subscriptions[] = $subscription; + } + $user_subscription_map[$user_id] = $valid_subscriptions; + } $auth = [ 'VAPID' => [ @@ -220,7 +246,6 @@ protected function notify_using_webpush(): void $web_push = new \Minishlink\WebPush\WebPush($auth); $number_of_notifications = 0; - $remove_subscriptions = []; // Time to go through the queue and send notifications /** @var type_interface $notification */ @@ -263,17 +288,14 @@ protected function notify_using_webpush(): void } catch (\ErrorException $exception) { - $remove_subscriptions[] = $subscription['subscription_id']; - $this->log->add('user', $user['user_id'], $user['user_ip'] ?? '', 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED', false, [ - 'reportee_id' => $user['user_id'], - $user['username'], - ]); + $this->log->add('critical', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$exception->getMessage()]); + break; } } } // Remove any subscriptions that couldn't be queued, i.e. that have invalid data - $this->remove_subscriptions($remove_subscriptions); + $this->remove_subscriptions(array_values($remove_subscriptions)); // List to fill with expired subscriptions based on return $expired_endpoints = []; @@ -300,7 +322,7 @@ protected function notify_using_webpush(): void } } } - catch (\ErrorException $exception) + catch (\Throwable $exception) { $this->log->add('critical', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$exception->getMessage()]); } diff --git a/subscription/validator.php b/subscription/validator.php new file mode 100644 index 0000000..e99a095 --- /dev/null +++ b/subscription/validator.php @@ -0,0 +1,80 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\webpushnotifications\subscription; + +use Base64Url\Base64Url; +use Brick\Math\BigInteger; +use Jose\Component\Core\Util\Ecc\NistCurve; +use Minishlink\WebPush\Utils; + +/** + * Web Push subscription key validator + */ +class validator +{ + /** + * Validate Web Push subscription keys + * + * @param string $public_key P-256 public key encoded with Base64URL + * @param string $auth_token Authentication secret encoded with Base64URL + * @return bool + */ + public static function is_valid(string $public_key, string $auth_token): bool + { + try + { + $decoded_public_key = self::decode_base64url($public_key); + $decoded_auth_token = self::decode_base64url($auth_token); + + if (Utils::safeStrlen($decoded_public_key) !== 65 + || $decoded_public_key[0] !== "\x04" + || Utils::safeStrlen($decoded_auth_token) !== 16) + { + return false; + } + + [$x, $y] = Utils::unserializePublicKey($decoded_public_key); + NistCurve::curve256()->getPublicKeyFrom( + BigInteger::fromBase(bin2hex($x), 16), + BigInteger::fromBase(bin2hex($y), 16) + ); + + return true; + } + catch (\InvalidArgumentException|\RuntimeException $exception) + { + return false; + } + } + + /** + * Strictly decode a Base64URL value + * + * @param string $value Base64URL encoded value + * @return string + * @throws \InvalidArgumentException + */ + private static function decode_base64url(string $value): string + { + if (!preg_match('/^[A-Za-z0-9_-]+={0,2}$/D', $value)) + { + throw new \InvalidArgumentException('Invalid Base64URL value'); + } + + $decoded = Base64Url::decode($value); + if (Base64Url::encode($decoded) !== rtrim($value, '=')) + { + throw new \InvalidArgumentException('Non-canonical Base64URL value'); + } + + return $decoded; + } +} diff --git a/tests/controller/controller_webpush_test.php b/tests/controller/controller_webpush_test.php index 7f353e4..ee36a3d 100644 --- a/tests/controller/controller_webpush_test.php +++ b/tests/controller/controller_webpush_test.php @@ -20,8 +20,13 @@ use Twig\Error\RuntimeError; use Twig\Error\SyntaxError; +require_once __DIR__ . '/../../vendor/autoload.php'; + class controller_webpush_test extends \phpbb_database_test_case { + private const VALID_PUBLIC_KEY = 'BGsX0fLhLEJH-Lzm5WOkQPJ3A32BLeszoPShOUXYmMKWT-NC4v4af5uO5-tKfA-eFivOM1drMV7Oy7ZAaDe_UfU'; + private const VALID_AUTH_TOKEN = 'AAAAAAAAAAAAAAAAAAAAAA'; + protected $auth; protected $avatar_helper; protected $config; @@ -411,7 +416,7 @@ public function test_sub_unsubscribe_success() $symfony_request->method('get')->willReturn(json_encode([ 'endpoint' => 'https://fcm.googleapis.com/fcm/send/test_endpoint', 'expiration_time' => 0, - 'keys' => ['p256dh' => 'test_p256dh', 'auth' => 'test_auth'] + 'keys' => ['p256dh' => self::VALID_PUBLIC_KEY, 'auth' => self::VALID_AUTH_TOKEN] ])); $response = $this->controller->subscribe($symfony_request); @@ -422,8 +427,8 @@ public function test_sub_unsubscribe_success() $this->assertEquals([ 'user_id' => '2', 'endpoint' => 'https://fcm.googleapis.com/fcm/send/test_endpoint', - 'p256dh' => 'test_p256dh', - 'auth' => 'test_auth', + 'p256dh' => self::VALID_PUBLIC_KEY, + 'auth' => self::VALID_AUTH_TOKEN, 'expiration_time' => '0', 'subscription_id' => '1', ], $this->get_subscription_data()); @@ -437,6 +442,21 @@ public function test_sub_unsubscribe_success() $this->assertEmpty($this->get_subscription_data()); } + public function test_unsubscribe_removes_legacy_endpoint(): void + { + $endpoint = 'http://fcm.googleapis.com/fcm/send/legacy_endpoint'; + $sql = 'INSERT INTO phpbb_wpn_push_subscriptions ' . $this->db->sql_build_array('INSERT', [ + 'user_id' => 2, + 'endpoint' => $endpoint, + ]); + $this->db->sql_query($sql); + + $response = $this->controller->unsubscribe($this->create_subscription_request(json_encode(['endpoint' => $endpoint]))); + + self::assertInstanceOf(JsonResponse::class, $response); + self::assertEmpty($this->get_subscription_data()); + } + public function scalar_json_data(): array { return [ @@ -493,7 +513,7 @@ public function test_subscribe_replaces_previous_subscription() 'endpoint' => 'https://fcm.googleapis.com/fcm/send/new_endpoint', 'previous_endpoint' => 'https://fcm.googleapis.com/fcm/send/old_endpoint', 'expirationTime' => 42000, - 'keys' => ['p256dh' => 'new_p256dh', 'auth' => 'new_auth'] + 'keys' => ['p256dh' => self::VALID_PUBLIC_KEY, 'auth' => self::VALID_AUTH_TOKEN] ])); $response = $this->controller->subscribe($symfony_request); @@ -506,8 +526,8 @@ public function test_subscribe_replaces_previous_subscription() $this->assertEquals([ 'user_id' => '2', 'endpoint' => 'https://fcm.googleapis.com/fcm/send/new_endpoint', - 'p256dh' => 'new_p256dh', - 'auth' => 'new_auth', + 'p256dh' => self::VALID_PUBLIC_KEY, + 'auth' => self::VALID_AUTH_TOKEN, 'expiration_time' => '42', 'subscription_id' => '2', ], $subscriptions[0]); @@ -649,6 +669,11 @@ public function data_is_valid_endpoint(): array // Wildcard *.push.apple.com ['https://api.push.apple.com/3/device/test', true], ['https://api.development.push.apple.com/3/device/test', true], + ['https://fcm.googleapis.com:443/fcm/send/test', true], + // Invalid: insecure transport or nonstandard port + ['http://fcm.googleapis.com/fcm/send/test', false], + ['https://fcm.googleapis.com:80/fcm/send/test', false], + ['https://fcm.googleapis.com:8443/fcm/send/test', false], // Invalid: disallowed host ['https://evil.example.com/push/endpoint', false], // Invalid: subdomain spoofing @@ -700,9 +725,9 @@ public function test_current_notification_data_is_rendered(): void public function subscription_write_data(): array { return [ - 'legacy seconds' => [['expiration_time' => 42, 'keys' => ['p256dh' => 'key', 'auth' => 'auth']], 42], - 'empty browser timestamp' => [['expirationTime' => null, 'keys' => ['p256dh' => 'key', 'auth' => 'auth']], 0], - 'browser milliseconds' => [['expirationTime' => 42000, 'keys' => ['p256dh' => 'key', 'auth' => 'auth']], 42], + 'legacy seconds' => [['expiration_time' => 42, 'keys' => ['p256dh' => self::VALID_PUBLIC_KEY, 'auth' => self::VALID_AUTH_TOKEN]], 42], + 'empty browser timestamp' => [['expirationTime' => null, 'keys' => ['p256dh' => self::VALID_PUBLIC_KEY, 'auth' => self::VALID_AUTH_TOKEN]], 0], + 'browser milliseconds' => [['expirationTime' => 42000, 'keys' => ['p256dh' => self::VALID_PUBLIC_KEY, 'auth' => self::VALID_AUTH_TOKEN]], 42], ]; } @@ -727,6 +752,16 @@ public function test_subscription_write_data_rejects_non_array_keys(): void $method->invoke($this->controller, ['keys' => 'invalid']); } + public function test_subscription_write_data_rejects_invalid_keys(): void + { + $method = new ReflectionMethod($this->controller, 'get_subscription_write_data'); + $method->setAccessible(true); + + $this->expectException(http_exception::class); + $this->expectExceptionMessage('AJAX_ERROR_TEXT'); + $method->invoke($this->controller, ['keys' => ['p256dh' => 'a', 'auth' => 'a']]); + } + public function avatar_data(): array { return [ @@ -840,6 +875,19 @@ public function test_toggle_popup_anonymous_user($user_id, $user_type, $is_bot) $this->controller->toggle_popup(); } + private function create_subscription_request(string $payload): \phpbb\symfony_request + { + $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; + + $symfony_request = $this->createMock(\phpbb\symfony_request::class); + $symfony_request->method('get')->willReturn($payload); + return $symfony_request; + } + private function get_subscription_data() { $sql = 'SELECT * diff --git a/tests/notification/notification_method_webpush_test.php b/tests/notification/notification_method_webpush_test.php index 4d79d2b..44ab25d 100644 --- a/tests/notification/notification_method_webpush_test.php +++ b/tests/notification/notification_method_webpush_test.php @@ -752,6 +752,41 @@ public function test_push_token_map_is_per_user(): void } } + public function test_invalid_subscription_does_not_interrupt_valid_delivery(): void + { + $valid_subscription = $this->create_subscription_for_user(2); + $push_subscriptions_table = $this->container->getParameter('tables.phpbb.wpn.push_subscriptions'); + $invalid_endpoint = 'https://fcm.googleapis.com/fcm/send/invalid-key-test'; + $sql = 'INSERT INTO ' . $push_subscriptions_table . ' ' . $this->db->sql_build_array('INSERT', [ + 'user_id' => 2, + 'endpoint' => $invalid_endpoint, + 'p256dh' => 'a', + 'auth' => 'a', + ]); + $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($invalid_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; diff --git a/tests/notification/webpush_branches_test.php b/tests/notification/webpush_branches_test.php index d917cf0..9e3a732 100644 --- a/tests/notification/webpush_branches_test.php +++ b/tests/notification/webpush_branches_test.php @@ -57,7 +57,7 @@ public function test_invalid_subscription_is_removed_and_logged(): void 'user_ip' => '127.0.0.1', 'username' => 'Tester', ]); - $method = $this->create_method($log, $user_loader, str_repeat('x', 5000)); + $method = $this->create_method($log, $user_loader); $method->subscriptions = [ 2 => [[ 'subscription_id' => 9, @@ -67,13 +67,49 @@ public function test_invalid_subscription_is_removed_and_logged(): void ]], ]; $method->add_to_queue($this->create_notification(2)); - $method->set_push_token(1, 10, 2, 'token'); + $method->add_to_queue($this->create_notification(2, 11)); $method->send_queued_notifications(); self::assertSame([9], $method->removed_subscriptions); } + public function test_oversized_payload_logs_failure_without_removing_subscription(): void + { + $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' + ); + $user_loader = $this->createMock(\phpbb\user_loader::class); + $user_loader->method('get_user')->with(2)->willReturn([ + 'user_id' => 2, + 'user_type' => USER_NORMAL, + 'user_inactive_reason' => 0, + 'user_form_salt' => 'salt', + 'user_ip' => '127.0.0.1', + 'username' => 'Tester', + ]); + $method = $this->create_method($log, $user_loader, str_repeat('x', 5000)); + $method->subscriptions = [ + 2 => [[ + 'subscription_id' => 9, + 'endpoint' => 'https://updates.push.services.mozilla.com/test', + 'p256dh' => notification_method_webpush_test::VAPID_KEYS['publicKey'], + 'auth' => 'AAAAAAAAAAAAAAAAAAAAAA', + ]], + ]; + $method->add_to_queue($this->create_notification(2)); + $method->set_push_token(1, 10, 2, 'token'); + + $method->send_queued_notifications(); + + self::assertSame([], $method->removed_subscriptions); + } + private function create_method($log = null, $user_loader = null, $assets_version = 1): testable_webpush { global $config, $phpbb_root_path, $phpEx, $user; @@ -104,12 +140,12 @@ private function create_method($log = null, $user_loader = null, $assets_version ); } - private function create_notification($user_id) + private function create_notification($user_id, $item_id = 10) { $notification = $this->createMock(\phpbb\notification\type\type_interface::class); $notification->user_id = $user_id; $notification->notification_type_id = 1; - $notification->item_id = 10; + $notification->item_id = $item_id; return $notification; } } diff --git a/tests/subscription/validator_test.php b/tests/subscription/validator_test.php new file mode 100644 index 0000000..92671c9 --- /dev/null +++ b/tests/subscription/validator_test.php @@ -0,0 +1,59 @@ + [self::VALID_PUBLIC_KEY, self::VALID_AUTH_TOKEN], + 'padded' => [self::VALID_PUBLIC_KEY . '=', self::VALID_AUTH_TOKEN . '=='], + ]; + } + + /** + * @dataProvider valid_key_data + */ + public function test_valid_keys(string $public_key, string $auth_token): void + { + self::assertTrue(validator::is_valid($public_key, $auth_token)); + } + + public function invalid_key_data(): array + { + $off_curve_key = rtrim(strtr(base64_encode("\x04" . str_repeat("\x00", 64)), '+/', '-_'), '='); + + return [ + 'invalid alphabet' => ['***', self::VALID_AUTH_TOKEN], + 'non-canonical encoding' => [self::VALID_PUBLIC_KEY . '==', self::VALID_AUTH_TOKEN], + 'short public key' => ['YQ', self::VALID_AUTH_TOKEN], + 'compressed public key' => [rtrim(strtr(base64_encode("\x02" . str_repeat("\x00", 32)), '+/', '-_'), '='), self::VALID_AUTH_TOKEN], + 'off-curve public key' => [$off_curve_key, self::VALID_AUTH_TOKEN], + 'short auth token' => [self::VALID_PUBLIC_KEY, 'YQ'], + ]; + } + + /** + * @dataProvider invalid_key_data + */ + public function test_invalid_keys(string $public_key, string $auth_token): void + { + self::assertFalse(validator::is_valid($public_key, $auth_token)); + } +} diff --git a/ucp/controller/webpush.php b/ucp/controller/webpush.php index 342361e..ecae851 100644 --- a/ucp/controller/webpush.php +++ b/ucp/controller/webpush.php @@ -19,6 +19,7 @@ use phpbb\webpushnotifications\ext; use phpbb\webpushnotifications\form\form_helper; use phpbb\webpushnotifications\json\sanitizer as json_sanitizer; +use phpbb\webpushnotifications\subscription\validator as subscription_validator; use phpbb\path_helper; use phpbb\request\request_interface; use phpbb\symfony_request; @@ -305,14 +306,18 @@ public function worker(): Response */ public function is_valid_endpoint(string $endpoint): bool { - $host = parse_url($endpoint, PHP_URL_HOST); + $parts = parse_url($endpoint); - if (!is_string($host) || empty($host)) + if (!is_array($parts) + || strtolower($parts['scheme'] ?? '') !== 'https' + || (isset($parts['port']) && $parts['port'] !== 443) + || !is_string($parts['host'] ?? null) + || empty($parts['host'])) { return false; } - $host = strtolower($host); + $host = strtolower($parts['host']); if (in_array($host, self::PUSH_SERVICE_WHITELIST, true)) { @@ -420,7 +425,7 @@ protected function get_subscription_write_data(array $data): array $p256dh = is_string($keys['p256dh'] ?? null) ? $keys['p256dh'] : ''; $auth = is_string($keys['auth'] ?? null) ? $keys['auth'] : ''; - if ($p256dh === '' || $auth === '') + if (!subscription_validator::is_valid($p256dh, $auth)) { throw new http_exception(Response::HTTP_BAD_REQUEST, 'AJAX_ERROR_TEXT'); } @@ -467,7 +472,7 @@ public function unsubscribe(symfony_request $symfony_request): JsonResponse $data = json_sanitizer::decode($symfony_request->get('data', '')); $endpoint = is_string($data['endpoint'] ?? null) ? $data['endpoint'] : ''; - if (!$this->is_valid_endpoint($endpoint)) + if ($endpoint === '') { throw new http_exception(Response::HTTP_BAD_REQUEST, 'WEBPUSH_INVALID_ENDPOINT'); } From 432e1c16fd6c634a6d034080101c227c004101c4 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 16 Oct 2026 15:11:32 -0700 Subject: [PATCH 06/18] Missing notification dropdown handled safely --- styles/all/template/webpush.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/styles/all/template/webpush.js b/styles/all/template/webpush.js index 88c0e7e..8f98a6a 100644 --- a/styles/all/template/webpush.js +++ b/styles/all/template/webpush.js @@ -104,7 +104,9 @@ function PhpbbWebpush() { subscribeButton.disabled = true; const notificationList = document.getElementById('notification_list'); - const subscribeToggle = notificationList.querySelector('.wpn-notification-dropdown-footer'); + const subscribeToggle = notificationList + ? notificationList.querySelector('.wpn-notification-dropdown-footer') + : null; if (subscribeToggle) { subscribeToggle.style.display = 'none'; From 0fdde4c501d7d1130443c6239b56398a1ce9bd14 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 16 Oct 2026 15:55:55 -0700 Subject: [PATCH 07/18] Additional fixes --- acp/wpn_acp_module.php | 8 ++++---- json/sanitizer.php | 2 +- styles/all/template/webpush.js | 4 +--- subscription/validator.php | 2 +- tests/json/sanitizer_test.php | 2 +- tests/subscription/validator_test.php | 2 +- 6 files changed, 9 insertions(+), 11 deletions(-) diff --git a/acp/wpn_acp_module.php b/acp/wpn_acp_module.php index 1c0979d..9cd4a50 100644 --- a/acp/wpn_acp_module.php +++ b/acp/wpn_acp_module.php @@ -139,11 +139,11 @@ public function display_settings() $this->template->assign_vars([ 'S_WEBPUSH_ENABLE' => $this->config['wpn_webpush_enable'], 'WEBPUSH_VAPID_PUBLIC' => $this->config['wpn_webpush_vapid_public'], - 'S_VAPID_KEYS_CONFIGURED' => (bool) ($this->config['wpn_webpush_vapid_public'] && $this->config['wpn_webpush_vapid_private']), + 'S_VAPID_KEYS_CONFIGURED' => $this->config['wpn_webpush_vapid_public'] && $this->config['wpn_webpush_vapid_private'], 'S_VAPID_PRIVATE_CONFIGURED' => (bool) $this->config['wpn_webpush_vapid_private'], - 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => $this->config['wpn_webpush_dropdown_subscribe'], - 'S_WEBPUSH_METHOD_ENABLED' => $this->config['wpn_webpush_method_enabled'], - 'S_WEBPUSH_POPUP_PROMPT' => $this->config['wpn_webpush_popup_prompt'], + 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => (bool) $this->config['wpn_webpush_dropdown_subscribe'], + 'S_WEBPUSH_METHOD_ENABLED' => (bool) $this->config['wpn_webpush_method_enabled'], + 'S_WEBPUSH_POPUP_PROMPT' => (bool) $this->config['wpn_webpush_popup_prompt'], 'U_ACTION' => $this->u_action, ]); diff --git a/json/sanitizer.php b/json/sanitizer.php index a8c7a6c..c68b7be 100644 --- a/json/sanitizer.php +++ b/json/sanitizer.php @@ -28,7 +28,7 @@ public static function sanitize(array $data) : array { if (!empty($data)) { - $json_sanitizer = function (&$value) + $json_sanitizer = static function (&$value) { $type_cast_helper = new type_cast_helper(); $type_cast_helper->set_var($value, $value, gettype($value), true); diff --git a/styles/all/template/webpush.js b/styles/all/template/webpush.js index 8f98a6a..0f9a42b 100644 --- a/styles/all/template/webpush.js +++ b/styles/all/template/webpush.js @@ -104,9 +104,7 @@ function PhpbbWebpush() { subscribeButton.disabled = true; const notificationList = document.getElementById('notification_list'); - const subscribeToggle = notificationList - ? notificationList.querySelector('.wpn-notification-dropdown-footer') - : null; + const subscribeToggle = notificationList ? notificationList.querySelector('.wpn-notification-dropdown-footer') : null; if (subscribeToggle) { subscribeToggle.style.display = 'none'; diff --git a/subscription/validator.php b/subscription/validator.php index e99a095..74f62fd 100644 --- a/subscription/validator.php +++ b/subscription/validator.php @@ -3,7 +3,7 @@ * * phpBB Browser Push Notifications. An extension for the phpBB Forum Software package. * - * @copyright (c) 2023, phpBB Limited + * @copyright (c) 2026, phpBB Limited * @license GNU General Public License, version 2 (GPL-2.0) * */ diff --git a/tests/json/sanitizer_test.php b/tests/json/sanitizer_test.php index 21d2a9f..fc5999e 100644 --- a/tests/json/sanitizer_test.php +++ b/tests/json/sanitizer_test.php @@ -3,7 +3,7 @@ * * phpBB Browser Push Notifications. An extension for the phpBB Forum Software package. * - * @copyright (c) 2026 + * @copyright (c) 2026, phpBB Limited * @license GNU General Public License, version 2 (GPL-2.0) * */ diff --git a/tests/subscription/validator_test.php b/tests/subscription/validator_test.php index 92671c9..65f798a 100644 --- a/tests/subscription/validator_test.php +++ b/tests/subscription/validator_test.php @@ -3,7 +3,7 @@ * * phpBB Browser Push Notifications. An extension for the phpBB Forum Software package. * - * @copyright (c) 2026 + * @copyright (c) 2026, phpBB Limited * @license GNU General Public License, version 2 (GPL-2.0) * */ From 6afeaf1e6412a98984eb61abc3689ab87ec1ea9f Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Fri, 16 Oct 2026 16:20:54 -0700 Subject: [PATCH 08/18] Re-state vapid key rotation effects --- language/en/webpushnotifications_module_acp.php | 2 +- language/ru/webpushnotifications_module_acp.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/language/en/webpushnotifications_module_acp.php b/language/en/webpushnotifications_module_acp.php index 0abce04..14b6360 100644 --- a/language/en/webpushnotifications_module_acp.php +++ b/language/en/webpushnotifications_module_acp.php @@ -50,7 +50,7 @@ 'WEBPUSH_GENERATE_VAPID_KEYS' => 'Generate key pair', 'WEBPUSH_REGENERATE_VAPID_KEYS'=> 'Generate new key pair', 'WEBPUSH_ROTATE_VAPID_KEYS_TITLE'=> 'Generate a new VAPID key pair?', - 'WEBPUSH_ROTATE_VAPID_KEYS_CONFIRM'=> 'Saving the new keys will invalidate all existing Web Push subscriptions. Users will need to subscribe again.', + '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_ROTATION_WARNING'=> 'Important: Changing either key invalidates all existing Web Push subscriptions.', diff --git a/language/ru/webpushnotifications_module_acp.php b/language/ru/webpushnotifications_module_acp.php index 9fa204b..4d53b52 100644 --- a/language/ru/webpushnotifications_module_acp.php +++ b/language/ru/webpushnotifications_module_acp.php @@ -50,7 +50,7 @@ 'WEBPUSH_GENERATE_VAPID_KEYS' => 'Сгенерировать пару ключей', 'WEBPUSH_REGENERATE_VAPID_KEYS'=> 'Сгенерировать новую пару', 'WEBPUSH_ROTATE_VAPID_KEYS_TITLE'=> 'Сгенерировать новую пару ключей VAPID?', - 'WEBPUSH_ROTATE_VAPID_KEYS_CONFIRM'=> 'После сохранения новых ключей все существующие подписки на браузерные push—уведомления станут недействительными. Пользователям потребуется подписаться снова.', + 'WEBPUSH_ROTATE_VAPID_KEYS_CONFIRM'=> 'Сохранение новых ключей сделает недействительными все существующие подписки на браузерные push—уведомления. Пользователи не будут получать браузерные push—уведомления до следующего посещения конференции, при котором их подписки будут пересозданы.', 'WEBPUSH_VAPID_KEYS_GENERATED'=> 'Новая пара ключей сгенерирована. Отправьте эту форму, чтобы активировать её.', 'WEBPUSH_VAPID_KEYS_GENERATION_FAILED'=> 'Не удалось сгенерировать пару ключей в этом браузере. Введите корректную пару вручную или используйте поддерживаемый браузер.', 'WEBPUSH_VAPID_ROTATION_WARNING'=> 'Важно: изменение любого ключа делает недействительными все существующие подписки на браузерные push—уведомления.', From 4124389177857404bb43a66994eac572b0a516e1 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 05:16:02 -0700 Subject: [PATCH 09/18] Update webpushnotifications_module_ucp.php Co-authored-by: rxu --- language/ru/webpushnotifications_module_ucp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/language/ru/webpushnotifications_module_ucp.php b/language/ru/webpushnotifications_module_ucp.php index 08bbfc5..d807a06 100644 --- a/language/ru/webpushnotifications_module_ucp.php +++ b/language/ru/webpushnotifications_module_ucp.php @@ -46,7 +46,7 @@ 'NOTIFY_WEBPUSH_ENABLE_EXPLAIN' => 'Включение браузерных push—уведомлений активирует их только на данном устройстве. Вы сможете отключить их в любое время через настройки браузера или с помощью кнопки «Отключить» выше, которая появится после включения браузерных push—уведомлений. Если не выбран ни один из типов уведомлений в категории «Браузерные» ниже, вы также не будете получать браузерные push—уведомления.', 'NOTIFY_WEBPUSH_SUBSCRIBE' => 'Подписаться', 'NOTIFY_WEBPUSH_UNSUBSCRIBE' => 'Отписаться', - 'NOTIFY_WEBPUSH_UNSUBSCRIBE_ERROR' => 'Не удалось отписаться от Web Push-уведомлений.', + 'NOTIFY_WEBPUSH_UNSUBSCRIBE_ERROR' => 'Не удалось отписаться от браузерных push—уведомлений.', 'NOTIFY_WEBPUSH_DROPDOWN_TITLE' => 'Посетите настройки уведомлений, чтобы установить предпочтительные типы браузерных уведомлений.', 'NOTIFY_WEBPUSH_DENIED' => 'Вы запретили браузерные уведомления для данного сайта. Для того, чтобы подписаться, необходимо их разрешить в настройках браузера.', 'NOTIFY_WEBPUSH_NOT_SUPPORTED' => 'Не поддерживается', From d83912e69ed4241a3b14cec45a70040f62a908cf Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 06:55:38 -0700 Subject: [PATCH 10/18] Move logging of removed user subscriptions to better flow --- notification/method/webpush.php | 36 +++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/notification/method/webpush.php b/notification/method/webpush.php index 11c2a30..a8aeece 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -209,7 +209,7 @@ protected function notify_using_webpush(): void // Get subscriptions for users $user_subscription_map = $this->get_user_subscription_map($notify_users); - $remove_subscriptions = []; + $invalid_subscriptions = []; // Validate each stored subscription once so malformed keys cannot interrupt the delivery batch. foreach ($user_subscription_map as $user_id => $subscriptions) @@ -222,11 +222,7 @@ protected function notify_using_webpush(): void || !is_string($subscription['auth'] ?? null) || !subscription_validator::is_valid($subscription['p256dh'], $subscription['auth'])) { - $remove_subscriptions[$subscription['subscription_id']] = $subscription['subscription_id']; - $this->log->add('user', $user['user_id'], $user['user_ip'] ?? '', 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED', false, [ - 'reportee_id' => $user['user_id'], - $user['username'], - ]); + $invalid_subscriptions[$subscription['subscription_id']] = $user; continue; } @@ -294,9 +290,6 @@ protected function notify_using_webpush(): void } } - // Remove any subscriptions that couldn't be queued, i.e. that have invalid data - $this->remove_subscriptions(array_values($remove_subscriptions)); - // List to fill with expired subscriptions based on return $expired_endpoints = []; @@ -327,10 +320,12 @@ protected function notify_using_webpush(): void $this->log->add('critical', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$exception->getMessage()]); } - $this->clean_expired_subscriptions($user_subscription_map, $expired_endpoints); - // We're done, empty the queue $this->empty_queue(); + + // Perform subscription cleanup only after valid notifications have been delivered. + $this->remove_invalid_subscriptions($invalid_subscriptions); + $this->clean_expired_subscriptions($user_subscription_map, $expired_endpoints); } /** @@ -469,6 +464,25 @@ public function remove_subscriptions(array $subscription_ids): void } } + /** + * Remove invalid subscriptions and record their removal + * + * @param array $invalid_subscriptions User data keyed by subscription id + * @return void + */ + protected function remove_invalid_subscriptions(array $invalid_subscriptions): void + { + $this->remove_subscriptions(array_keys($invalid_subscriptions)); + + foreach ($invalid_subscriptions as $user) + { + $this->log->add('user', $user['user_id'], $user['user_ip'] ?? '', 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED', false, [ + 'reportee_id' => $user['user_id'], + $user['username'], + ]); + } + } + /** * Clean expired subscriptions from the database * From 7fd6549b4866c05703ed62c02ae0ca60e98874ff Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 08:52:07 -0700 Subject: [PATCH 11/18] Background/system log should be owned by ANONYMOUS --- notification/method/webpush.php | 2 +- tests/notification/webpush_branches_test.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/notification/method/webpush.php b/notification/method/webpush.php index a8aeece..eeb43aa 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -476,7 +476,7 @@ protected function remove_invalid_subscriptions(array $invalid_subscriptions): v foreach ($invalid_subscriptions as $user) { - $this->log->add('user', $user['user_id'], $user['user_ip'] ?? '', 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED', false, [ + $this->log->add('user', ANONYMOUS, '', 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED', false, [ 'reportee_id' => $user['user_id'], $user['username'], ]); diff --git a/tests/notification/webpush_branches_test.php b/tests/notification/webpush_branches_test.php index 9e3a732..034e2f4 100644 --- a/tests/notification/webpush_branches_test.php +++ b/tests/notification/webpush_branches_test.php @@ -44,8 +44,8 @@ public function test_invalid_subscription_is_removed_and_logged(): void $log = $this->createMock(\phpbb\log\log_interface::class); $log->expects(self::once())->method('add')->with( 'user', - 2, - '127.0.0.1', + ANONYMOUS, + '', 'LOG_WEBPUSH_SUBSCRIPTION_REMOVED' ); $user_loader = $this->createMock(\phpbb\user_loader::class); From 7b3e51092c8792c8369b8480f5e8817e36e55e85 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 09:32:51 -0700 Subject: [PATCH 12/18] Move inline code to file --- .../event/overall_footer_body_after.html | 21 ++----------------- styles/all/template/pwa_install_banner.html | 18 ++++++++++++++++ styles/all/template/webpush_popup.html | 2 -- 3 files changed, 20 insertions(+), 21 deletions(-) create mode 100644 styles/all/template/pwa_install_banner.html diff --git a/styles/all/template/event/overall_footer_body_after.html b/styles/all/template/event/overall_footer_body_after.html index 9634394..2baa812 100644 --- a/styles/all/template/event/overall_footer_body_after.html +++ b/styles/all/template/event/overall_footer_body_after.html @@ -17,27 +17,10 @@ {% endif %} -{% if NOTIFICATIONS_WEBPUSH_ENABLE %} +{% if NOTIFICATIONS_WEBPUSH_ENABLE and S_WEBPUSH_POPUP_PROMPT and not S_WEBPUSH_POPUP_DISABLED %} {% include '@phpbb_webpushnotifications/webpush_popup.html' %} {% endif %} {% if S_PWA_SHOW_BANNER %} - {% INCLUDEJS '@phpbb_webpushnotifications/pwa_install_banner.js' %} - {% INCLUDECSS '@phpbb_webpushnotifications/phpbb_pwa.css' %} - + {% include '@phpbb_webpushnotifications/pwa_install_banner.html' %} {% endif %} diff --git a/styles/all/template/pwa_install_banner.html b/styles/all/template/pwa_install_banner.html new file mode 100644 index 0000000..9858c39 --- /dev/null +++ b/styles/all/template/pwa_install_banner.html @@ -0,0 +1,18 @@ +{% INCLUDEJS '@phpbb_webpushnotifications/pwa_install_banner.js' %} +{% INCLUDECSS '@phpbb_webpushnotifications/phpbb_pwa.css' %} + diff --git a/styles/all/template/webpush_popup.html b/styles/all/template/webpush_popup.html index e76c102..2cf761d 100644 --- a/styles/all/template/webpush_popup.html +++ b/styles/all/template/webpush_popup.html @@ -1,4 +1,3 @@ -{% if S_WEBPUSH_POPUP_PROMPT and not S_WEBPUSH_POPUP_DISABLED %} -{% endif %} From 67e102bb9d629f36645283a10a1223bd553e1fd5 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 11:10:49 -0700 Subject: [PATCH 13/18] Replace single-button JavaScript model with mirrored control collections --- notification/method/webpush.php | 3 +- .../notification_dropdown_footer_after.html | 6 +- .../event/overall_header_head_append.html | 2 +- styles/all/template/webpush.js | 74 +++++++++---------- styles/all/template/webpush_popup.html | 2 +- .../ucp_notifications_content_before.html | 4 +- tests/event/listener_test.php | 1 + tests/functional/functional_test.php | 12 ++- .../notification_method_webpush_test.php | 21 ++++++ 9 files changed, 75 insertions(+), 50 deletions(-) diff --git a/notification/method/webpush.php b/notification/method/webpush.php index eeb43aa..e99b535 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -410,7 +410,8 @@ public function get_ucp_template_data(helper $controller_helper, form_helper $fo } return [ - 'NOTIFICATIONS_WEBPUSH_ENABLE' => ($this->config['load_notifications'] && $this->config['allow_board_notifications'] && $this->config['wpn_webpush_dropdown_subscribe']) || stripos($this->user->page['page'], 'notification_options'), + 'NOTIFICATIONS_WEBPUSH_ENABLE' => true, + 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => $this->config['load_notifications'] && $this->config['allow_board_notifications'] && $this->config['wpn_webpush_dropdown_subscribe'], 'U_WEBPUSH_SUBSCRIBE' => $controller_helper->route('phpbb_webpushnotifications_ucp_push_subscribe_controller'), 'U_WEBPUSH_UNSUBSCRIBE' => $controller_helper->route('phpbb_webpushnotifications_ucp_push_unsubscribe_controller'), 'U_WEBPUSH_TOGGLE_POPUP' => $controller_helper->route('phpbb_webpushnotifications_ucp_push_toggle_popup_controller'), diff --git a/styles/all/template/event/notification_dropdown_footer_after.html b/styles/all/template/event/notification_dropdown_footer_after.html index defa510..f8169de 100644 --- a/styles/all/template/event/notification_dropdown_footer_after.html +++ b/styles/all/template/event/notification_dropdown_footer_after.html @@ -1,10 +1,10 @@ -{% if NOTIFICATIONS_WEBPUSH_ENABLE and notification_types is not defined %} +{% if NOTIFICATIONS_WEBPUSH_ENABLE and S_WEBPUSH_DROPDOWN_SUBSCRIBE %} diff --git a/styles/all/template/event/overall_header_head_append.html b/styles/all/template/event/overall_header_head_append.html index 504b8e6..9935a99 100644 --- a/styles/all/template/event/overall_header_head_append.html +++ b/styles/all/template/event/overall_header_head_append.html @@ -11,6 +11,6 @@ {% if U_TOUCH_ICON %}{% endif %} -{% if NOTIFICATIONS_WEBPUSH_ENABLE %} +{% if NOTIFICATIONS_WEBPUSH_ENABLE and (notification_types is defined or S_WEBPUSH_DROPDOWN_SUBSCRIBE or (S_WEBPUSH_POPUP_PROMPT and not S_WEBPUSH_POPUP_DISABLED)) %} {% include '@phpbb_webpushnotifications/ucp_notifications_webpush.html' %} {% endif %} diff --git a/styles/all/template/webpush.js b/styles/all/template/webpush.js index 0f9a42b..989507c 100644 --- a/styles/all/template/webpush.js +++ b/styles/all/template/webpush.js @@ -30,11 +30,11 @@ function PhpbbWebpush() { /** @type {string} VAPID public key */ let vapidPublicKey = ''; - /** @type {HTMLElement} Subscribe button */ - let subscribeButton; + /** @type {NodeListOf} Subscribe buttons */ + let subscribeButtons; - /** @type {HTMLElement} Unsubscribe button */ - let unsubscribeButton; + /** @type {NodeListOf} Unsubscribe buttons */ + let unsubscribeButtons; /** @type {HTMLElement} Toggle popup button */ let togglePopupButton; @@ -61,8 +61,8 @@ function PhpbbWebpush() { unsubscribeError = options.unsubscribeError; vapidPublicKey = options.vapidPublicKey; - subscribeButton = document.querySelector('#subscribe_webpush'); - unsubscribeButton = document.querySelector('#unsubscribe_webpush'); + subscribeButtons = document.querySelectorAll('[data-wpn-action="subscribe"]'); + unsubscribeButtons = document.querySelectorAll('[data-wpn-action="unsubscribe"]'); togglePopupButton = document.querySelector('#toggle_popup_prompt'); // Set up toggle popup button handler if it exists (on UCP settings page) @@ -79,8 +79,8 @@ function PhpbbWebpush() { if ('serviceWorker' in navigator && 'PushManager' in window) { navigator.serviceWorker.register(serviceWorkerUrl) .then(async() => { - subscribeButton.addEventListener('click', subscribeButtonHandler); - unsubscribeButton.addEventListener('click', unsubscribeButtonHandler); + subscribeButtons.forEach(button => button.addEventListener('click', subscribeButtonHandler)); + unsubscribeButtons.forEach(button => button.addEventListener('click', unsubscribeButtonHandler)); await updateButtonState(); initPopupPrompt(); @@ -101,8 +101,6 @@ function PhpbbWebpush() { * @returns {void} */ function setDisabledState() { - subscribeButton.disabled = true; - const notificationList = document.getElementById('notification_list'); const subscribeToggle = notificationList ? notificationList.querySelector('.wpn-notification-dropdown-footer') : null; @@ -110,9 +108,13 @@ function PhpbbWebpush() { subscribeToggle.style.display = 'none'; } - if (subscribeButton.type === 'submit' || subscribeButton.classList.contains('button')) { - subscribeButton.value = subscribeButton.getAttribute('data-l-unsupported'); - } + subscribeButtons.forEach(button => { + button.disabled = true; + + if (button.type === 'submit' || button.classList.contains('button')) { + button.value = button.getAttribute('data-l-unsupported'); + } + }); } /** @@ -190,20 +192,9 @@ function PhpbbWebpush() { popup.style.display = 'flex'; }, 1000); - const allowBtn = document.getElementById('wpn_popup_allow'); const denyBtn = document.getElementById('wpn_popup_deny'); const overlay = document.getElementById('wpn_popup_prompt'); - if (allowBtn) { - allowBtn.addEventListener('click', (event) => { - event.stopPropagation(); - hidePopup(popup); - subscribeButtonHandler(event).catch(error => { - console.error('Subscription handler error:', error); - }); - }); - } - if (denyBtn) { denyBtn.addEventListener('click', (event) => { event.stopPropagation(); @@ -341,13 +332,8 @@ function PhpbbWebpush() { * @param {boolean} subscribed True if subscribed, false if not */ function setSubscriptionState(subscribed) { - if (subscribed) { - subscribeButton.classList.add('hidden'); - unsubscribeButton.classList.remove('hidden'); - } else { - subscribeButton.classList.remove('hidden'); - unsubscribeButton.classList.add('hidden'); - } + subscribeButtons.forEach(button => button.classList.toggle('hidden', subscribed)); + unsubscribeButtons.forEach(button => button.classList.toggle('hidden', !subscribed)); } /** @@ -371,7 +357,7 @@ function PhpbbWebpush() { const data = await response.json(); if (!data.success) { - throw new Error(data.message || subscribeButton.getAttribute('data-l-unsupported')); + throw new Error(data.message || subscribeButtons[0].getAttribute('data-l-unsupported')); } handleSubscribe(data, subscription, previousEndpoint); @@ -419,13 +405,22 @@ function PhpbbWebpush() { async function subscribeButtonHandler(event) { event.preventDefault(); - subscribeButton.removeEventListener('click', subscribeButtonHandler); + const button = event.currentTarget; + subscribeButtons.forEach(control => { + control.disabled = true; + control.removeEventListener('click', subscribeButtonHandler); + }); + + if (button.id === 'wpn_popup_allow') { + event.stopPropagation(); + hidePopup(document.getElementById('wpn_popup_prompt')); + } try { // Prevent the user from clicking the subscribe button multiple times. const result = await Notification.requestPermission(); if (result === 'denied') { - phpbb.alert(subscribeButton.getAttribute('data-l-err'), subscribeButton.getAttribute('data-l-msg')); + phpbb.alert(button.getAttribute('data-l-err'), button.getAttribute('data-l-msg')); return; } @@ -434,7 +429,7 @@ 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') { - throw new Error(subscribeButton.getAttribute('data-l-unsupported')); + throw new Error(button.getAttribute('data-l-unsupported')); } try { @@ -443,15 +438,18 @@ function PhpbbWebpush() { } catch (error) { promptDenied.set(); hidePopup(document.getElementById('wpn_popup_prompt')); - phpbb.alert(ajaxErrorTitle, error.message || subscribeButton.getAttribute('data-l-unsupported')); + phpbb.alert(ajaxErrorTitle, error.message || button.getAttribute('data-l-unsupported')); } } catch (error) { promptDenied.set(); // deny the prompt on error to prevent repeated prompting hidePopup(document.getElementById('wpn_popup_prompt')); console.error('Push subscription error:', error); - phpbb.alert(subscribeButton.getAttribute('data-l-err'), error.message || subscribeButton.getAttribute('data-l-unsupported')); + phpbb.alert(button.getAttribute('data-l-err'), error.message || button.getAttribute('data-l-unsupported')); } finally { - subscribeButton.addEventListener('click', subscribeButtonHandler); + subscribeButtons.forEach(control => { + control.disabled = false; + control.addEventListener('click', subscribeButtonHandler); + }); } } diff --git a/styles/all/template/webpush_popup.html b/styles/all/template/webpush_popup.html index 2cf761d..ca59600 100644 --- a/styles/all/template/webpush_popup.html +++ b/styles/all/template/webpush_popup.html @@ -10,7 +10,7 @@

{{ lang('NOTIFY_WEBPUSH_POPUP_MESSAGE') }}

- +
diff --git a/styles/prosilver/template/event/ucp_notifications_content_before.html b/styles/prosilver/template/event/ucp_notifications_content_before.html index 7d7c8d0..22d8508 100644 --- a/styles/prosilver/template/event/ucp_notifications_content_before.html +++ b/styles/prosilver/template/event/ucp_notifications_content_before.html @@ -5,8 +5,8 @@

{{ lang('NOTIFY_WEBPUSH_ENABLE_EXPLAIN') }}
- - + +
{% if S_WEBPUSH_POPUP_PROMPT %} diff --git a/tests/event/listener_test.php b/tests/event/listener_test.php index a0be270..6cc9563 100644 --- a/tests/event/listener_test.php +++ b/tests/event/listener_test.php @@ -316,6 +316,7 @@ public function test_load_template_data($user_id, $method_data, $subscriptions, $this->callback(function($arg) use ($subscriptions) { $expectedValues = [ 'NOTIFICATIONS_WEBPUSH_ENABLE' => true, + 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => true, 'U_WEBPUSH_SUBSCRIBE' => $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_subscribe_controller'), 'U_WEBPUSH_UNSUBSCRIBE' => $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_unsubscribe_controller'), 'VAPID_PUBLIC_KEY' => $this->config['wpn_webpush_vapid_public'], diff --git a/tests/functional/functional_test.php b/tests/functional/functional_test.php index 9e3c177..a2f6783 100644 --- a/tests/functional/functional_test.php +++ b/tests/functional/functional_test.php @@ -112,12 +112,16 @@ public function test_dropdown_subscribe_button() // Assert subscribe dropdown is present $crawler = self::request('GET', 'index.php'); $this->assertCount(1, $crawler->filter('.wpn-notification-dropdown-footer')); - $this->assertContainsLang('NOTIFY_WEBPUSH_SUBSCRIBE', $crawler->filter('.wpn-notification-dropdown-footer #subscribe_webpush')->html()); - $this->assertContainsLang('NOTIFY_WEBPUSH_UNSUBSCRIBE', $crawler->filter('.wpn-notification-dropdown-footer #unsubscribe_webpush')->html()); + $this->assertContainsLang('NOTIFY_WEBPUSH_SUBSCRIBE', $crawler->filter('#subscribe_webpush_dropdown')->html()); + $this->assertContainsLang('NOTIFY_WEBPUSH_UNSUBSCRIBE', $crawler->filter('#unsubscribe_webpush_dropdown')->html()); - // Assert subscribe button is not displayed in UCP when dropdown subscribe is present + // Assert dropdown and UCP controls coexist without duplicate IDs $crawler = self::request('GET', 'ucp.php?i=ucp_notifications&mode=notification_options'); - $this->assertCount(0, $crawler->filter('.wpn-notification-dropdown-footer')); + $this->assertCount(1, $crawler->filter('.wpn-notification-dropdown-footer')); + $this->assertCount(1, $crawler->filter('#subscribe_webpush')); + $this->assertCount(1, $crawler->filter('#unsubscribe_webpush')); + $this->assertCount(1, $crawler->filter('#subscribe_webpush_dropdown')); + $this->assertCount(1, $crawler->filter('#unsubscribe_webpush_dropdown')); } public function test_manifest() diff --git a/tests/notification/notification_method_webpush_test.php b/tests/notification/notification_method_webpush_test.php index 44ab25d..155d70b 100644 --- a/tests/notification/notification_method_webpush_test.php +++ b/tests/notification/notification_method_webpush_test.php @@ -815,6 +815,8 @@ public function test_get_ucp_template_data_uses_millisecond_expiration_time(): v $template_data = $this->notification_method_webpush->get_ucp_template_data($controller_helper, $form_helper); + $this->assertTrue($template_data['NOTIFICATIONS_WEBPUSH_ENABLE']); + $this->assertTrue($template_data['S_WEBPUSH_DROPDOWN_SUBSCRIBE']); $this->assertSame([ [ 'endpoint' => 'https://fcm.googleapis.com/fcm/send/test_endpoint', @@ -823,6 +825,25 @@ public function test_get_ucp_template_data_uses_millisecond_expiration_time(): v ], $template_data['SUBSCRIPTIONS']); } + public function test_get_ucp_template_data_separates_webpush_availability_from_dropdown(): void + { + $this->user->data['user_id'] = 2; + $this->config['load_notifications'] = true; + $this->config['allow_board_notifications'] = true; + $this->config['wpn_webpush_dropdown_subscribe'] = false; + + $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([]); + + $template_data = $this->notification_method_webpush->get_ucp_template_data($controller_helper, $form_helper); + + $this->assertTrue($template_data['NOTIFICATIONS_WEBPUSH_ENABLE']); + $this->assertFalse($template_data['S_WEBPUSH_DROPDOWN_SUBSCRIBE']); + } + /** * Test is_subscription_unauthorized method with various HTTP status codes */ From 472e118b4a3859804336965e9c87273774c61a72 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 12:55:03 -0700 Subject: [PATCH 14/18] Web Push recovery JavaScript now loads on every eligible authenticated visit --- styles/all/template/event/overall_header_head_append.html | 5 ++++- styles/all/template/ucp_notifications_webpush.html | 1 - styles/all/template/webpush.js | 2 +- tests/functional/functional_test.php | 3 +++ 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/styles/all/template/event/overall_header_head_append.html b/styles/all/template/event/overall_header_head_append.html index 9935a99..300026c 100644 --- a/styles/all/template/event/overall_header_head_append.html +++ b/styles/all/template/event/overall_header_head_append.html @@ -11,6 +11,9 @@ {% if U_TOUCH_ICON %}{% endif %} -{% if NOTIFICATIONS_WEBPUSH_ENABLE and (notification_types is defined or S_WEBPUSH_DROPDOWN_SUBSCRIBE or (S_WEBPUSH_POPUP_PROMPT and not S_WEBPUSH_POPUP_DISABLED)) %} +{% if NOTIFICATIONS_WEBPUSH_ENABLE %} {% include '@phpbb_webpushnotifications/ucp_notifications_webpush.html' %} + {% if notification_types is defined or S_WEBPUSH_DROPDOWN_SUBSCRIBE or (S_WEBPUSH_POPUP_PROMPT and not S_WEBPUSH_POPUP_DISABLED) %} + {% INCLUDECSS '@phpbb_webpushnotifications/phpbb_wpn.css' %} + {% endif %} {% endif %} diff --git a/styles/all/template/ucp_notifications_webpush.html b/styles/all/template/ucp_notifications_webpush.html index 2b896a4..ef4e364 100644 --- a/styles/all/template/ucp_notifications_webpush.html +++ b/styles/all/template/ucp_notifications_webpush.html @@ -20,4 +20,3 @@ {% INCLUDEJS '@phpbb_webpushnotifications/webpush.js' %} -{% INCLUDECSS '@phpbb_webpushnotifications/phpbb_wpn.css' %} diff --git a/styles/all/template/webpush.js b/styles/all/template/webpush.js index 989507c..9a4a185 100644 --- a/styles/all/template/webpush.js +++ b/styles/all/template/webpush.js @@ -357,7 +357,7 @@ function PhpbbWebpush() { const data = await response.json(); if (!data.success) { - throw new Error(data.message || subscribeButtons[0].getAttribute('data-l-unsupported')); + throw new Error(data.message); } handleSubscribe(data, subscription, previousEndpoint); diff --git a/tests/functional/functional_test.php b/tests/functional/functional_test.php index a2f6783..6e22324 100644 --- a/tests/functional/functional_test.php +++ b/tests/functional/functional_test.php @@ -106,12 +106,15 @@ public function test_dropdown_subscribe_button() // Assert subscribe dropdown is not present by default $crawler = self::request('GET', 'index.php'); $this->assertCount(0, $crawler->filter('.wpn-notification-dropdown-footer')); + $this->assertCount(1, $crawler->filter('script[src*="webpush.js"]')); + $this->assertCount(0, $crawler->filter('link[href*="phpbb_wpn.css"]')); $this->set_acp_option('wpn_webpush_dropdown_subscribe', 1); // Assert subscribe dropdown is present $crawler = self::request('GET', 'index.php'); $this->assertCount(1, $crawler->filter('.wpn-notification-dropdown-footer')); + $this->assertCount(1, $crawler->filter('link[href*="phpbb_wpn.css"]')); $this->assertContainsLang('NOTIFY_WEBPUSH_SUBSCRIBE', $crawler->filter('#subscribe_webpush_dropdown')->html()); $this->assertContainsLang('NOTIFY_WEBPUSH_UNSUBSCRIBE', $crawler->filter('#unsubscribe_webpush_dropdown')->html()); From 818895b5622642a2943e82346932b45e96e31d05 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 13:11:54 -0700 Subject: [PATCH 15/18] Remove junk from vendor dir --- build.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build.xml b/build.xml index 68dc29e..0de15b0 100644 --- a/build.xml +++ b/build.xml @@ -15,6 +15,11 @@ --> + + + + + From 58d3b85b1cb99a6fdee627d210d137a2506a527d Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 13:16:37 -0700 Subject: [PATCH 16/18] Accept same-second Web Push form tokens --- form/form_helper.php | 3 +-- tests/form/form_helper_test.php | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/form/form_helper.php b/form/form_helper.php index c737d6f..6ad1715 100644 --- a/form/form_helper.php +++ b/form/form_helper.php @@ -83,8 +83,7 @@ public function check_form_tokens(string $form_name, ?int $timespan = null): boo $diff = time() - $creation_time; - // If creation_time and the time() now is zero we can assume it was not a human doing this (the check for if ($diff)...) - if (defined('DEBUG_TEST') || ($diff && ($diff <= $timespan || $timespan === -1))) + if (defined('DEBUG_TEST') || ($diff >= 0 && ($diff <= $timespan || $timespan === -1))) { $token_sid = ((int) $this->user->data['user_id'] === ANONYMOUS && !empty($this->config['form_token_sid_guests'])) ? $this->user->session_id : ''; $key = sha1($creation_time . $this->user->data['user_form_salt'] . $form_name . $token_sid); diff --git a/tests/form/form_helper_test.php b/tests/form/form_helper_test.php index 7e965ff..5549eed 100644 --- a/tests/form/form_helper_test.php +++ b/tests/form/form_helper_test.php @@ -36,9 +36,11 @@ protected function setUp(): void public function check_data(): array { return [ + 'same-second token' => [2, 60, null, 0, true, true, true], 'valid user token' => [2, 60, null, 10, true, true, true], 'valid guest token with sid' => [ANONYMOUS, 60, null, 10, true, true, true], 'unlimited lifetime' => [2, -1, null, 3600, true, true, true], + 'future-dated token' => [2, 60, null, -1, true, true, false], 'minimum lifetime enforced' => [2, 1, null, 31, true, true, false], 'explicit lifetime' => [2, 60, 5, 10, true, true, false], 'missing creation time' => [2, 60, null, 10, false, true, false], From 046be2fd4eef1df4ee7a277bef64cca24f3b8533 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 13:20:02 -0700 Subject: [PATCH 17/18] Update style colors only when their values changed --- acp/wpn_acp_module.php | 7 ++++++- tests/acp/acp_module_test.php | 29 +++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/acp/wpn_acp_module.php b/acp/wpn_acp_module.php index 9cd4a50..c12a258 100644 --- a/acp/wpn_acp_module.php +++ b/acp/wpn_acp_module.php @@ -244,10 +244,15 @@ public function save_pwa_settings() $pwa_bg_color = $this->request->variable('pwa_bg_color_' . $style_id, ''); $pwa_theme_color = $this->request->variable('pwa_theme_color_' . $style_id, ''); - $updates[$style_id] = [ + $update = [ 'pwa_bg_color' => $this->validate_hex_color($pwa_bg_color) ? $pwa_bg_color : $row['pwa_bg_color'], 'pwa_theme_color' => $this->validate_hex_color($pwa_theme_color) ? $pwa_theme_color : $row['pwa_theme_color'], ]; + + if ($update['pwa_bg_color'] !== $row['pwa_bg_color'] || $update['pwa_theme_color'] !== $row['pwa_theme_color']) + { + $updates[$style_id] = $update; + } } if ($this->display_errors()) diff --git a/tests/acp/acp_module_test.php b/tests/acp/acp_module_test.php index aa93cb2..e004744 100644 --- a/tests/acp/acp_module_test.php +++ b/tests/acp/acp_module_test.php @@ -557,13 +557,34 @@ public function pwa_save_data(): array 'pwa_bg_color' => '', 'pwa_theme_color' => '', ], - [ - 'pwa_bg_color' => '', - 'pwa_theme_color' => '', - ], ], true, ], + 'unchanged style colours skipped' => [ + $valid, + [ + 'pwa_bg_color_1' => '#fff000', + 'pwa_theme_color_1' => '#000fff', + 'pwa_bg_color_2' => '', + 'pwa_theme_color_2' => '', + ], + [ + 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], + 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ], + [ + 'pwa_short_name' => 'Forum', + 'pwa_icon_small' => 'icon192.png', + 'pwa_icon_large' => 'icon512.png', + 'pwa_show_install_banner' => 1, + ], + [], + true, + ], 'too long short name rejected after entity decoding' => [ array_merge($valid, [ 'pwa_short_name' => '123456789012😀', From 348359e45f52392a1bac4f5488553baf72c37735 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Sat, 17 Oct 2026 13:31:21 -0700 Subject: [PATCH 18/18] Extract ACP module into its own controller --- acp/wpn_acp_module.php | 406 +--------- composer.json | 2 +- config/services.yml | 15 + controller/acp_controller.php | 480 ++++++++++++ tests/acp/acp_module_test.php | 903 +---------------------- tests/controller/controller_acp_test.php | 834 +++++++++++++++++++++ 6 files changed, 1365 insertions(+), 1275 deletions(-) create mode 100644 controller/acp_controller.php create mode 100644 tests/controller/controller_acp_test.php diff --git a/acp/wpn_acp_module.php b/acp/wpn_acp_module.php index c12a258..35a94d6 100644 --- a/acp/wpn_acp_module.php +++ b/acp/wpn_acp_module.php @@ -10,14 +10,7 @@ namespace phpbb\webpushnotifications\acp; -use phpbb\config\config; -use phpbb\language\language; -use phpbb\log\log; -use phpbb\request\request; -use phpbb\symfony_request; -use phpbb\template\template; -use phpbb\user; -use phpbb\webpushnotifications\ext; +use phpbb\webpushnotifications\controller\acp_controller; class wpn_acp_module { @@ -25,42 +18,6 @@ class wpn_acp_module public $tpl_name; public $u_action; - /** @var \phpbb\cache\service */ - protected $cache; - - /** @var config $config */ - protected $config; - - /** @var \phpbb\db\driver\driver_interface */ - protected $db; - - /** @var language $lang */ - protected $lang; - - /** @var log $log */ - protected $log; - - /** @var \FastImageSize\FastImageSize $imagesize */ - protected $imagesize; - - /** @var request $request */ - protected $request; - - /** @var symfony_request $symfony_request */ - private $symfony_request; - - /** @var template $template */ - protected $template; - - /** @var user $user */ - protected $user; - - /** @var string */ - protected $root_path; - - /** @var array $errors */ - protected $errors = []; - /** * Main ACP module * @@ -72,368 +29,21 @@ public function main($id, $mode) { global $phpbb_container; - $this->cache = $phpbb_container->get('cache'); - $this->config = $phpbb_container->get('config'); - $this->db = $phpbb_container->get('dbal.conn'); - $this->imagesize = $phpbb_container->get('upload_imagesize'); - $this->lang = $phpbb_container->get('language'); - $this->log = $phpbb_container->get('log'); - $this->request = $phpbb_container->get('request'); - $this->symfony_request = $phpbb_container->get('symfony_request'); - $this->template = $phpbb_container->get('template'); - $this->user = $phpbb_container->get('user'); - $this->root_path = $phpbb_container->getParameter('core.root_path'); - - $form_key = 'phpbb/webpushnotifications'; - add_form_key($form_key); + /** @var acp_controller $controller */ + $controller = $phpbb_container->get('phpbb.wpn.acp.controller'); + $controller->set_page_url($this->u_action); if ($mode === 'webpush') { - // Load a template from adm/style for our ACP page $this->tpl_name = 'wpn_acp_settings'; - - $this->lang->add_lang('webpushnotifications_module_acp', 'phpbb/webpushnotifications'); - - // Set the page title for our ACP page - $this->page_title = $this->lang->lang('ACP_WEBPUSH_EXT_SETTINGS'); - - if ($this->request->is_set_post('submit')) - { - if (!check_form_key($form_key)) - { - trigger_error($this->lang->lang('FORM_INVALID'), E_USER_WARNING); - } - - $this->save_settings(); - } - - $this->display_settings(); + $this->page_title = 'ACP_WEBPUSH_EXT_SETTINGS'; + $controller->handle_webpush(); } else if ($mode === 'pwa') { $this->tpl_name = 'wpn_acp_pwa'; - - $this->lang->add_lang('webpushnotifications_module_acp', 'phpbb/webpushnotifications'); - - $this->page_title = $this->lang->lang('ACP_WEBPUSH_PWA_SETTINGS'); - - if ($this->request->is_set_post('submit')) - { - if (!check_form_key($form_key)) - { - trigger_error($this->lang->lang('FORM_INVALID'), E_USER_WARNING); - } - - $this->save_pwa_settings(); - } - - $this->display_pwa_settings(); - } - } - - /** - * Add settings template vars to the form - */ - public function display_settings() - { - $this->template->assign_vars([ - 'S_WEBPUSH_ENABLE' => $this->config['wpn_webpush_enable'], - 'WEBPUSH_VAPID_PUBLIC' => $this->config['wpn_webpush_vapid_public'], - 'S_VAPID_KEYS_CONFIGURED' => $this->config['wpn_webpush_vapid_public'] && $this->config['wpn_webpush_vapid_private'], - 'S_VAPID_PRIVATE_CONFIGURED' => (bool) $this->config['wpn_webpush_vapid_private'], - 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => (bool) $this->config['wpn_webpush_dropdown_subscribe'], - 'S_WEBPUSH_METHOD_ENABLED' => (bool) $this->config['wpn_webpush_method_enabled'], - 'S_WEBPUSH_POPUP_PROMPT' => (bool) $this->config['wpn_webpush_popup_prompt'], - 'U_ACTION' => $this->u_action, - ]); - - if (!$this->symfony_request->isSecure() && $this->request->server('SERVER_NAME') !== 'localhost') - { - $this->errors[] = $this->lang->lang('WEBPUSH_INSECURE_SERVER_ERROR'); - } - - $this->display_errors(); - } - - /** - * Save settings data to the database - * - * @return void - */ - public function save_settings() - { - $config_array = $this->request->variable('config', ['' => ''], true); - $display_settings = [ - 'wpn_webpush_enable' => ['validate' => 'bool'], - 'wpn_webpush_vapid_public' => ['validate' => 'string:25:255', 'lang' => 'WEBPUSH_VAPID_PUBLIC'], - 'wpn_webpush_vapid_private'=> ['validate' => 'string:25:255', 'lang' => 'WEBPUSH_VAPID_PRIVATE'], - 'wpn_webpush_dropdown_subscribe' => ['validate' => 'bool'], - 'wpn_webpush_method_enabled' => ['validate' => 'bool'], - 'wpn_webpush_popup_prompt' => ['validate' => 'bool'], - ]; - - // An empty private key keeps the stored credential. Stored private keys are never sent to the browser. - if (($config_array['wpn_webpush_vapid_private'] ?? '') === '' && $this->config['wpn_webpush_vapid_private']) - { - unset($display_settings['wpn_webpush_vapid_private'], $config_array['wpn_webpush_vapid_private']); + $this->page_title = 'ACP_WEBPUSH_PWA_SETTINGS'; + $controller->handle_pwa(); } - - if (!empty($config_array['wpn_webpush_enable'])) - { - // Validate config values - validate_config_vars($display_settings, $config_array, $this->errors); - } - - if ($this->display_errors()) - { - return; - } - - $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_CONFIG_WEBPUSH'); - - foreach (array_keys($display_settings) as $config_name) - { - if (array_key_exists($config_name, $config_array)) - { - $this->config->set($config_name, $config_array[$config_name]); - } - } - - trigger_error($this->lang->lang('CONFIG_UPDATED') . adm_back_link($this->u_action), E_USER_NOTICE); - } - - /** - * Add PWA settings template vars to the form - */ - public function display_pwa_settings() - { - $this->template->assign_vars([ - 'S_PWA_SHOW_INSTALL_BANNER' => (bool) $this->config['pwa_show_install_banner'], - 'PWA_SHORT_NAME' => $this->config['pwa_short_name'], - 'PWA_SHORT_NAME_PLACEHOLDER'=> utf8_substr(ext::decode_entities($this->config['sitename'], ENT_QUOTES), 0, 12), - 'PWA_ICON_SMALL' => $this->config['pwa_icon_small'], - 'PWA_ICON_LARGE' => $this->config['pwa_icon_large'], - 'STYLES' => $this->get_styles(), - 'U_ACTION' => $this->u_action, - ]); - - $this->display_errors(); - } - - /** - * Save PWA settings data to the database - * - * @return void - */ - public function save_pwa_settings() - { - $config_array = $this->request->variable('config', ['' => ''], true); - - $config_array['pwa_short_name'] = $config_array['pwa_short_name'] ?? ''; - $config_array['pwa_icon_small'] = $config_array['pwa_icon_small'] ?? ''; - $config_array['pwa_icon_large'] = $config_array['pwa_icon_large'] ?? ''; - - $this->validate_pwa_short_name($config_array['pwa_short_name']); - $this->validate_pwa_icons($config_array['pwa_icon_small'], $config_array['pwa_icon_large']); - - $styles = $this->get_styles(); - $updates = []; - foreach ($styles as $row) - { - $style_id = $row['style_id']; - $pwa_bg_color = $this->request->variable('pwa_bg_color_' . $style_id, ''); - $pwa_theme_color = $this->request->variable('pwa_theme_color_' . $style_id, ''); - - $update = [ - 'pwa_bg_color' => $this->validate_hex_color($pwa_bg_color) ? $pwa_bg_color : $row['pwa_bg_color'], - 'pwa_theme_color' => $this->validate_hex_color($pwa_theme_color) ? $pwa_theme_color : $row['pwa_theme_color'], - ]; - - if ($update['pwa_bg_color'] !== $row['pwa_bg_color'] || $update['pwa_theme_color'] !== $row['pwa_theme_color']) - { - $updates[$style_id] = $update; - } - } - - if ($this->display_errors()) - { - return; - } - - $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_CONFIG_WEBPUSH'); - - // Ensure 4-byte emoji can be stored correctly - $config_array['pwa_short_name'] = utf8_encode_ucr($config_array['pwa_short_name']); - - foreach ([ - 'pwa_short_name', - 'pwa_icon_small', - 'pwa_icon_large', - 'pwa_show_install_banner', - ] as $config_name) - { - $this->config->set($config_name, $config_array[$config_name] ?? 0); - } - - $this->set_styles($updates); - - trigger_error($this->lang->lang('CONFIG_UPDATED') . adm_back_link($this->u_action), E_USER_NOTICE); - } - - /** - * Validate PWA short site name - */ - protected function validate_pwa_short_name(string $short_name): void - { - if ($short_name === '') - { - return; - } - - $short_name = ext::decode_entities($short_name, ENT_QUOTES); - if (utf8_strlen($short_name) > 12) - { - $this->errors[] = $this->lang->lang('PWA_SHORT_NAME_INVALID'); - } - } - - /** - * Validate PWA icon filenames and dimensions - */ - protected function validate_pwa_icons(string $small_icon, string $large_icon): void - { - if ($small_icon === '' && $large_icon === '') - { - return; - } - - if ($small_icon === '') - { - $this->errors[] = $this->lang->lang('PWA_ICON_NOT_PROVIDED', $this->lang->lang('PWA_ICON_SMALL')); - return; - } - - if ($large_icon === '') - { - $this->errors[] = $this->lang->lang('PWA_ICON_NOT_PROVIDED', $this->lang->lang('PWA_ICON_LARGE')); - return; - } - - $this->validate_pwa_icon($small_icon, 192); - $this->validate_pwa_icon($large_icon, 512); - } - - /** - * Validate one PWA icon file - */ - protected function validate_pwa_icon(string $filename, int $size): void - { - if (basename($filename) !== $filename) - { - $this->errors[] = $this->lang->lang('PWA_ICON_INVALID', $filename); - return; - } - - $image = $this->root_path . ext::PWA_ICON_DIR . '/' . $filename; - $image_info = $this->imagesize->getImageSize($image); - if ($image_info === false) - { - $this->errors[] = $this->lang->lang('PWA_ICON_INVALID', $filename); - return; - } - - if ($image_info['width'] !== $size || $image_info['height'] !== $size) - { - $this->errors[] = $this->lang->lang('PWA_ICON_SIZE_INVALID', $filename); - } - - if ($image_info['type'] !== IMAGETYPE_PNG) - { - $this->errors[] = $this->lang->lang('PWA_ICON_MIME_INVALID', $filename); - } - } - - /** - * Validate HTML color hex codes - */ - protected function validate_hex_color(string $code): bool - { - $code = trim($code); - - if ($code === '') - { - return true; - } - - $test = (bool) preg_match('/^#([0-9A-F]{3}){1,2}$/i', $code); - - if ($test === false) - { - $this->errors[] = $this->lang->lang('PWA_INVALID_COLOUR', $code); - } - - return $test; - } - - /** - * Get style data from the styles table - * - * @return array Style data - */ - protected function get_styles(): array - { - $sql = 'SELECT style_id, style_name, pwa_bg_color, pwa_theme_color - FROM ' . STYLES_TABLE . ' - WHERE style_active = 1 - ORDER BY style_name'; - $result = $this->db->sql_query($sql); - - $rows = $this->db->sql_fetchrowset($result); - $this->db->sql_freeresult($result); - - return $rows; - } - - /** - * Set style data in the styles table - * - * @param array $rows Array of style table data to update; style_id is key - * @return void - */ - protected function set_styles(array $rows): void - { - if (!empty($rows)) - { - $this->db->sql_transaction('begin'); - - foreach ($rows as $style_id => $row) - { - $sql = 'UPDATE ' . STYLES_TABLE . ' - SET ' . $this->db->sql_build_array('UPDATE', $row) . ' - WHERE style_id = ' . (int) $style_id; - $this->db->sql_query($sql); - } - - $this->db->sql_transaction('commit'); - - $this->cache->destroy('sql', STYLES_TABLE); - } - } - - /** - * Display any errors - * - * @return bool - */ - public function display_errors() - { - $has_errors = (bool) count($this->errors); - - $this->template->assign_vars([ - 'S_ERROR' => $has_errors, - 'ERROR_MSG' => $has_errors ? implode('
', $this->errors) : '', - ]); - - return $has_errors; } } diff --git a/composer.json b/composer.json index 7e833f6..926ed34 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "type": "phpbb-extension", "description": "An official phpBB extension that allows board users to receive browser-based push notifications.", "homepage": "https://www.phpbb.com/customise/db/extension/webpushnotifications/", - "version": "1.1.1-dev", + "version": "1.2.0-dev", "license": "GPL-2.0-only", "authors": [ { diff --git a/config/services.yml b/config/services.yml index 122a181..231f710 100644 --- a/config/services.yml +++ b/config/services.yml @@ -2,6 +2,21 @@ imports: - { resource: tables.yml } services: + phpbb.wpn.acp.controller: + class: phpbb\webpushnotifications\controller\acp_controller + arguments: + - '@cache' + - '@config' + - '@dbal.conn' + - '@upload_imagesize' + - '@language' + - '@log' + - '@request' + - '@symfony_request' + - '@template' + - '@user' + - '%core.root_path%' + phpbb.wpn.form_helper: class: phpbb\webpushnotifications\form\form_helper arguments: diff --git a/controller/acp_controller.php b/controller/acp_controller.php new file mode 100644 index 0000000..fc0e6db --- /dev/null +++ b/controller/acp_controller.php @@ -0,0 +1,480 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + */ + +namespace phpbb\webpushnotifications\controller; + +use FastImageSize\FastImageSize; +use phpbb\cache\service as cache; +use phpbb\config\config; +use phpbb\db\driver\driver_interface; +use phpbb\language\language; +use phpbb\log\log_interface; +use phpbb\request\request_interface; +use phpbb\symfony_request; +use phpbb\template\template; +use phpbb\user; +use phpbb\webpushnotifications\ext; + +class acp_controller +{ + public const FORM_KEY = 'phpbb/webpushnotifications'; + + /** @var cache */ + protected $cache; + + /** @var config $config */ + protected $config; + + /** @var driver_interface */ + protected $db; + + /** @var language $lang */ + protected $lang; + + /** @var log_interface */ + protected $log; + + /** @var FastImageSize */ + protected $imagesize; + + /** @var request_interface */ + protected $request; + + /** @var symfony_request $symfony_request */ + protected $symfony_request; + + /** @var template $template */ + protected $template; + + /** @var user $user */ + protected $user; + + /** @var string */ + protected $root_path; + + /** @var array $errors */ + protected $errors = []; + + /** @var string */ + protected $u_action; + + /** + * Constructor + * + * @param cache $cache + * @param config $config + * @param driver_interface $db + * @param FastImageSize $imagesize + * @param language $lang + * @param log_interface $log + * @param request_interface $request + * @param symfony_request $symfony_request + * @param template $template + * @param user $user + * @param string $root_path + */ + public function __construct( + cache $cache, + config $config, + driver_interface $db, + FastImageSize $imagesize, + language $lang, + log_interface $log, + request_interface $request, + symfony_request $symfony_request, + template $template, + user $user, + string $root_path + ) + { + $this->cache = $cache; + $this->config = $config; + $this->db = $db; + $this->imagesize = $imagesize; + $this->lang = $lang; + $this->log = $log; + $this->request = $request; + $this->symfony_request = $symfony_request; + $this->template = $template; + $this->user = $user; + $this->root_path = $root_path; + } + + /** + * Set ACP page URL + * + * @param string $u_action + * @return acp_controller + */ + public function set_page_url(string $u_action): self + { + $this->u_action = $u_action; + + return $this; + } + + /** + * Handle Web Push settings + */ + public function handle_webpush(): void + { + $this->prepare(); + + if ($this->request->is_set_post('submit')) + { + $this->check_form_key(); + $this->save_settings(); + } + + $this->display_settings(); + } + + /** + * Handle PWA settings + */ + public function handle_pwa(): void + { + $this->prepare(); + + if ($this->request->is_set_post('submit')) + { + $this->check_form_key(); + $this->save_pwa_settings(); + } + + $this->display_pwa_settings(); + } + + /** + * Prepare shared ACP settings state + */ + protected function prepare(): void + { + $this->lang->add_lang('webpushnotifications_module_acp', 'phpbb/webpushnotifications'); + add_form_key(self::FORM_KEY); + } + + /** + * Validate ACP form key + */ + protected function check_form_key(): void + { + if (!check_form_key(self::FORM_KEY)) + { + trigger_error($this->lang->lang('FORM_INVALID'), E_USER_WARNING); + } + } + + /** + * Add settings template vars to the form + */ + protected function display_settings() + { + $this->template->assign_vars([ + 'S_WEBPUSH_ENABLE' => $this->config['wpn_webpush_enable'], + 'WEBPUSH_VAPID_PUBLIC' => $this->config['wpn_webpush_vapid_public'], + 'S_VAPID_KEYS_CONFIGURED' => $this->config['wpn_webpush_vapid_public'] && $this->config['wpn_webpush_vapid_private'], + 'S_VAPID_PRIVATE_CONFIGURED' => (bool) $this->config['wpn_webpush_vapid_private'], + 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => (bool) $this->config['wpn_webpush_dropdown_subscribe'], + 'S_WEBPUSH_METHOD_ENABLED' => (bool) $this->config['wpn_webpush_method_enabled'], + 'S_WEBPUSH_POPUP_PROMPT' => (bool) $this->config['wpn_webpush_popup_prompt'], + 'U_ACTION' => $this->u_action, + ]); + + if (!$this->symfony_request->isSecure() && $this->request->server('SERVER_NAME') !== 'localhost') + { + $this->errors[] = $this->lang->lang('WEBPUSH_INSECURE_SERVER_ERROR'); + } + + $this->display_errors(); + } + + /** + * Save settings data to the database + * + * @return void + */ + protected function save_settings() + { + $config_array = $this->request->variable('config', ['' => ''], true); + $display_settings = [ + 'wpn_webpush_enable' => ['validate' => 'bool'], + 'wpn_webpush_vapid_public' => ['validate' => 'string:25:255', 'lang' => 'WEBPUSH_VAPID_PUBLIC'], + 'wpn_webpush_vapid_private'=> ['validate' => 'string:25:255', 'lang' => 'WEBPUSH_VAPID_PRIVATE'], + 'wpn_webpush_dropdown_subscribe' => ['validate' => 'bool'], + 'wpn_webpush_method_enabled' => ['validate' => 'bool'], + 'wpn_webpush_popup_prompt' => ['validate' => 'bool'], + ]; + + // An empty private key keeps the stored credential. Stored private keys are never sent to the browser. + if (($config_array['wpn_webpush_vapid_private'] ?? '') === '' && $this->config['wpn_webpush_vapid_private']) + { + unset($display_settings['wpn_webpush_vapid_private'], $config_array['wpn_webpush_vapid_private']); + } + + if (!empty($config_array['wpn_webpush_enable'])) + { + // Validate config values + validate_config_vars($display_settings, $config_array, $this->errors); + } + + if ($this->display_errors()) + { + return; + } + + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_CONFIG_WEBPUSH'); + + foreach (array_keys($display_settings) as $config_name) + { + if (array_key_exists($config_name, $config_array)) + { + $this->config->set($config_name, $config_array[$config_name]); + } + } + + trigger_error($this->lang->lang('CONFIG_UPDATED') . adm_back_link($this->u_action), E_USER_NOTICE); + } + + /** + * Add PWA settings template vars to the form + */ + protected function display_pwa_settings() + { + $this->template->assign_vars([ + 'S_PWA_SHOW_INSTALL_BANNER' => (bool) $this->config['pwa_show_install_banner'], + 'PWA_SHORT_NAME' => $this->config['pwa_short_name'], + 'PWA_SHORT_NAME_PLACEHOLDER'=> utf8_substr(ext::decode_entities($this->config['sitename'], ENT_QUOTES), 0, 12), + 'PWA_ICON_SMALL' => $this->config['pwa_icon_small'], + 'PWA_ICON_LARGE' => $this->config['pwa_icon_large'], + 'STYLES' => $this->get_styles(), + 'U_ACTION' => $this->u_action, + ]); + + $this->display_errors(); + } + + /** + * Save PWA settings data to the database + * + * @return void + */ + protected function save_pwa_settings() + { + $config_array = $this->request->variable('config', ['' => ''], true); + + $config_array['pwa_short_name'] = $config_array['pwa_short_name'] ?? ''; + $config_array['pwa_icon_small'] = $config_array['pwa_icon_small'] ?? ''; + $config_array['pwa_icon_large'] = $config_array['pwa_icon_large'] ?? ''; + + $this->validate_pwa_short_name($config_array['pwa_short_name']); + $this->validate_pwa_icons($config_array['pwa_icon_small'], $config_array['pwa_icon_large']); + + $styles = $this->get_styles(); + $updates = []; + foreach ($styles as $row) + { + $style_id = $row['style_id']; + $pwa_bg_color = $this->request->variable('pwa_bg_color_' . $style_id, ''); + $pwa_theme_color = $this->request->variable('pwa_theme_color_' . $style_id, ''); + + $update = [ + 'pwa_bg_color' => $this->validate_hex_color($pwa_bg_color) ? $pwa_bg_color : $row['pwa_bg_color'], + 'pwa_theme_color' => $this->validate_hex_color($pwa_theme_color) ? $pwa_theme_color : $row['pwa_theme_color'], + ]; + + if ($update['pwa_bg_color'] !== $row['pwa_bg_color'] || $update['pwa_theme_color'] !== $row['pwa_theme_color']) + { + $updates[$style_id] = $update; + } + } + + if ($this->display_errors()) + { + return; + } + + $this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_CONFIG_WEBPUSH'); + + // Ensure 4-byte emoji can be stored correctly + $config_array['pwa_short_name'] = utf8_encode_ucr($config_array['pwa_short_name']); + + foreach ([ + 'pwa_short_name', + 'pwa_icon_small', + 'pwa_icon_large', + 'pwa_show_install_banner', + ] as $config_name) + { + $this->config->set($config_name, $config_array[$config_name] ?? 0); + } + + $this->set_styles($updates); + + trigger_error($this->lang->lang('CONFIG_UPDATED') . adm_back_link($this->u_action), E_USER_NOTICE); + } + + /** + * Validate PWA short site name + */ + protected function validate_pwa_short_name(string $short_name): void + { + if ($short_name === '') + { + return; + } + + $short_name = ext::decode_entities($short_name, ENT_QUOTES); + if (utf8_strlen($short_name) > 12) + { + $this->errors[] = $this->lang->lang('PWA_SHORT_NAME_INVALID'); + } + } + + /** + * Validate PWA icon filenames and dimensions + */ + protected function validate_pwa_icons(string $small_icon, string $large_icon): void + { + if ($small_icon === '' && $large_icon === '') + { + return; + } + + if ($small_icon === '') + { + $this->errors[] = $this->lang->lang('PWA_ICON_NOT_PROVIDED', $this->lang->lang('PWA_ICON_SMALL')); + return; + } + + if ($large_icon === '') + { + $this->errors[] = $this->lang->lang('PWA_ICON_NOT_PROVIDED', $this->lang->lang('PWA_ICON_LARGE')); + return; + } + + $this->validate_pwa_icon($small_icon, 192); + $this->validate_pwa_icon($large_icon, 512); + } + + /** + * Validate one PWA icon file + */ + protected function validate_pwa_icon(string $filename, int $size): void + { + if (basename($filename) !== $filename) + { + $this->errors[] = $this->lang->lang('PWA_ICON_INVALID', $filename); + return; + } + + $image = $this->root_path . ext::PWA_ICON_DIR . '/' . $filename; + $image_info = $this->imagesize->getImageSize($image); + if ($image_info === false) + { + $this->errors[] = $this->lang->lang('PWA_ICON_INVALID', $filename); + return; + } + + if ($image_info['width'] !== $size || $image_info['height'] !== $size) + { + $this->errors[] = $this->lang->lang('PWA_ICON_SIZE_INVALID', $filename); + } + + if ($image_info['type'] !== IMAGETYPE_PNG) + { + $this->errors[] = $this->lang->lang('PWA_ICON_MIME_INVALID', $filename); + } + } + + /** + * Validate HTML color hex codes + */ + protected function validate_hex_color(string $code): bool + { + $code = trim($code); + + if ($code === '') + { + return true; + } + + $test = (bool) preg_match('/^#([0-9A-F]{3}){1,2}$/i', $code); + + if ($test === false) + { + $this->errors[] = $this->lang->lang('PWA_INVALID_COLOUR', $code); + } + + return $test; + } + + /** + * Get style data from the styles table + * + * @return array Style data + */ + protected function get_styles(): array + { + $sql = 'SELECT style_id, style_name, pwa_bg_color, pwa_theme_color + FROM ' . STYLES_TABLE . ' + WHERE style_active = 1 + ORDER BY style_name'; + $result = $this->db->sql_query($sql); + + $rows = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + + return $rows; + } + + /** + * Set style data in the styles table + * + * @param array $rows Array of style table data to update; style_id is key + * @return void + */ + protected function set_styles(array $rows): void + { + if (!empty($rows)) + { + $this->db->sql_transaction('begin'); + + foreach ($rows as $style_id => $row) + { + $sql = 'UPDATE ' . STYLES_TABLE . ' + SET ' . $this->db->sql_build_array('UPDATE', $row) . ' + WHERE style_id = ' . (int) $style_id; + $this->db->sql_query($sql); + } + + $this->db->sql_transaction('commit'); + + $this->cache->destroy('sql', STYLES_TABLE); + } + } + + /** + * Display any errors + * + * @return bool + */ + protected function display_errors() + { + $has_errors = (bool) count($this->errors); + + $this->template->assign_vars([ + 'S_ERROR' => $has_errors, + 'ERROR_MSG' => $has_errors ? implode('
', $this->errors) : '', + ]); + + return $has_errors; + } +} diff --git a/tests/acp/acp_module_test.php b/tests/acp/acp_module_test.php index e004744..999a5f3 100644 --- a/tests/acp/acp_module_test.php +++ b/tests/acp/acp_module_test.php @@ -11,23 +11,14 @@ namespace phpbb\webpushnotifications\tests\acp; use phpbb\webpushnotifications\acp\wpn_acp_module; +use phpbb\webpushnotifications\controller\acp_controller; require_once __DIR__ . '/../../../../../includes/functions_module.php'; -require_once __DIR__ . '/../../../../../includes/functions_acp.php'; class acp_module_test extends \phpbb_test_case { - /** @var bool */ - public static $valid_form = true; - - /** @var \phpbb_mock_cache */ - protected $cache; - - /** @var \phpbb\config\config */ - protected $config; - - /** @var \phpbb\db\driver\driver_interface */ - protected $db; + /** @var acp_controller|\PHPUnit\Framework\MockObject\MockObject */ + protected $controller; /** @var \phpbb_mock_extension_manager */ protected $extension_manager; @@ -35,41 +26,11 @@ class acp_module_test extends \phpbb_test_case /** @var \phpbb\module\module_manager */ protected $module_manager; - /** @var \FastImageSize\FastImageSize|\PHPUnit\Framework\MockObject\MockObject */ - protected $imagesize; - - /** @var \phpbb\language\language|\PHPUnit\Framework\MockObject\MockObject */ - protected $lang; - - /** @var \phpbb\log\log_interface|\PHPUnit\Framework\MockObject\MockObject */ - protected $log; - - /** @var \phpbb\request\request|\PHPUnit\Framework\MockObject\MockObject */ - protected $request; - - /** @var \phpbb\symfony_request|\PHPUnit\Framework\MockObject\MockObject */ - protected $symfony_request; - - /** @var \phpbb\template\template|\PHPUnit\Framework\MockObject\MockObject */ - protected $template; - - /** @var \Symfony\Component\DependencyInjection\ContainerInterface|\PHPUnit\Framework\MockObject\MockObject */ - protected $container; - - /** @var \phpbb\user|\PHPUnit\Framework\MockObject\MockObject */ - protected $user; - - /** @var string */ - protected $root_path; - - /** @var array */ - protected $style_updates; - protected function setUp(): void { - global $phpbb_dispatcher, $phpbb_extension_manager, $phpbb_root_path, $phpEx, $phpbb_container, $template, $user, $language; + global $phpbb_container, $phpbb_dispatcher, $phpbb_extension_manager, $phpbb_root_path, $phpEx; - self::$valid_form = true; + $phpbb_dispatcher = new \phpbb_mock_event_dispatcher(); $this->extension_manager = new \phpbb_mock_extension_manager( $phpbb_root_path, @@ -92,72 +53,13 @@ protected function setUp(): void $phpEx ); - $phpbb_dispatcher = new \phpbb_mock_event_dispatcher(); - - if (!defined('IN_ADMIN')) - { - define('IN_ADMIN', true); - } + $this->controller = $this->createMock(acp_controller::class); - $this->cache = new \phpbb_mock_cache(); - $this->config = new \phpbb\config\config($this->default_config()); - $this->db = $this->createMock('\phpbb\db\driver\driver_interface'); - $this->imagesize = $this->createMock('\FastImageSize\FastImageSize'); - $this->lang = $this->createMock('\phpbb\language\language'); - $this->log = $this->createMock('\phpbb\log\log_interface'); - $this->request = $this->createMock('\phpbb\request\request'); - $this->symfony_request = $this->createMock('\phpbb\symfony_request'); - $this->template = $this->createMock('\phpbb\template\template'); - $this->user = $this->createMock('\phpbb\user'); - $this->root_path = $phpbb_root_path; - $this->style_updates = []; - - $this->user->data = ['user_id' => 42]; - $this->user->ip = '127.0.0.1'; - $this->user->lang = new \phpbb_mock_lang(); - - $this->lang->method('lang') - ->willReturnCallback(function() { - return implode(':', func_get_args()); - }); - - $this->db->method('sql_query') - ->willReturn('result'); - $this->db->method('sql_fetchrowset') - ->with('result') - ->willReturn($this->style_rows()); - $this->db->method('sql_build_array') - ->willReturnCallback(function($query, $row) { - $this->style_updates[] = $row; - return implode(', ', array_map(static function($name, $value) { - return $name . " = '" . $value . "'"; - }, array_keys($row), $row)); - }); - - $this->container = $this->createMock('Symfony\Component\DependencyInjection\ContainerInterface'); - $this->container->method('get') - ->willReturnCallback(function($service) { - return [ - 'cache' => $this->cache, - 'config' => $this->config, - 'dbal.conn' => $this->db, - 'upload_imagesize' => $this->imagesize, - 'language' => $this->lang, - 'log' => $this->log, - 'request' => $this->request, - 'symfony_request' => $this->symfony_request, - 'template' => $this->template, - 'user' => $this->user, - ][$service]; - }); - $this->container->method('getParameter') - ->with('core.root_path') - ->willReturn($phpbb_root_path); - - $phpbb_container = $this->container; - $template = $this->template; - $user = $this->user; - $language = $this->lang; + $phpbb_container = $this->createMock('Symfony\Component\DependencyInjection\ContainerInterface'); + $phpbb_container->expects(self::any()) + ->method('get') + ->with('phpbb.wpn.acp.controller') + ->willReturn($this->controller); } public function test_module_info(): void @@ -170,12 +72,12 @@ public function test_module_info(): void 'webpush' => [ 'title' => 'ACP_WEBPUSH_EXT_SETTINGS', 'auth' => 'ext_phpbb/webpushnotifications && acl_a_server', - 'cat' => ['ACP_CLIENT_COMMUNICATION'] + 'cat' => ['ACP_CLIENT_COMMUNICATION'], ], 'pwa' => [ 'title' => 'ACP_WEBPUSH_PWA_SETTINGS', 'auth' => 'ext_phpbb/webpushnotifications && acl_a_board', - 'cat' => ['ACP_CLIENT_COMMUNICATION'] + 'cat' => ['ACP_CLIENT_COMMUNICATION'], ], ], ], @@ -198,784 +100,33 @@ public function test_module_auth($module_auth, $expected): void self::assertEquals($expected, \p_master::module_auth($module_auth, 0)); } - public function mode_display_data(): array + public function mode_data(): array { return [ - 'webpush secure server' => [ - 'webpush', - 'adm.php?i=test&mode=webpush', - 'wpn_acp_settings', - 'ACP_WEBPUSH_EXT_SETTINGS', - true, - 'example.com', - $this->webpush_template_vars('adm.php?i=test&mode=webpush'), - false, - '', - ], - 'webpush localhost without https' => [ - 'webpush', - 'adm.php?i=test&mode=webpush', - 'wpn_acp_settings', - 'ACP_WEBPUSH_EXT_SETTINGS', - false, - 'localhost', - $this->webpush_template_vars('adm.php?i=test&mode=webpush'), - false, - '', - ], - 'webpush insecure remote server' => [ - 'webpush', - 'adm.php?i=test&mode=webpush', - 'wpn_acp_settings', - 'ACP_WEBPUSH_EXT_SETTINGS', - false, - 'example.com', - $this->webpush_template_vars('adm.php?i=test&mode=webpush'), - true, - 'WEBPUSH_INSECURE_SERVER_ERROR', - ], - 'pwa settings' => [ - 'pwa', - 'adm.php?i=test&mode=pwa', - 'wpn_acp_pwa', - 'ACP_WEBPUSH_PWA_SETTINGS', - true, - 'example.com', - $this->pwa_template_vars('adm.php?i=test&mode=pwa'), - false, - '', - ], + 'webpush' => ['webpush', 'wpn_acp_settings', 'ACP_WEBPUSH_EXT_SETTINGS', 'handle_webpush'], + 'pwa' => ['pwa', 'wpn_acp_pwa', 'ACP_WEBPUSH_PWA_SETTINGS', 'handle_pwa'], ]; } /** - * @dataProvider mode_display_data + * @dataProvider mode_data */ - public function test_main_displays_mode_settings($mode, $u_action, $tpl_name, $page_title, $is_secure, $server_name, array $settings_vars, $has_error, $error_msg): void + public function test_main_delegates_mode($mode, $tpl_name, $page_title, $handler): void { - $this->request->expects($this->once()) - ->method('is_set_post') - ->with('submit') - ->willReturn(false); - - $this->symfony_request->method('isSecure') - ->willReturn($is_secure); + $u_action = 'adm.php?i=test&mode=' . $mode; - $this->request->method('server') - ->with('SERVER_NAME') - ->willReturn($server_name); + $this->controller->expects(self::once()) + ->method('set_page_url') + ->with($u_action) + ->willReturnSelf(); + $this->controller->expects(self::once()) + ->method($handler); - $this->template->expects(self::exactly(2)) - ->method('assign_vars') - ->withConsecutive( - [$settings_vars], - [[ - 'S_ERROR' => $has_error, - 'ERROR_MSG' => $error_msg, - ]] - ); - - $module = $this->create_module($u_action); + $module = new wpn_acp_module(); + $module->u_action = $u_action; $module->main('', $mode); self::assertSame($tpl_name, $module->tpl_name); self::assertSame($page_title, $module->page_title); } - - public function invalid_form_key_data(): array - { - return [ - 'webpush' => ['webpush'], - 'pwa' => ['pwa'], - ]; - } - - /** - * @dataProvider invalid_form_key_data - */ - public function test_main_rejects_invalid_form_key($mode): void - { - self::$valid_form = false; - - $this->request->method('is_set_post') - ->with('submit') - ->willReturn(true); - - $this->assertTriggerError(E_USER_WARNING, 'FORM_INVALID', function() use ($mode) { - $this->create_module('adm.php?i=test&mode=' . $mode)->main('', $mode); - }); - } - - public function valid_submit_mode_data(): array - { - return [ - 'webpush' => ['webpush', 'save_settings'], - 'pwa' => ['pwa', 'save_pwa_settings'], - ]; - } - - /** - * @dataProvider valid_submit_mode_data - */ - public function test_main_routes_valid_submission($mode, $expected_method): void - { - $this->request->method('is_set_post')->with('submit')->willReturn(true); - $module = new \phpbb\webpushnotifications\acp\routing_acp_module(); - $module->u_action = 'adm.php?i=test&mode=' . $mode; - - $module->main('', $mode); - - self::assertSame([$expected_method], $module->calls); - } - - 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_dropdown_subscribe' => 1, - 'wpn_webpush_method_enabled' => 0, - 'wpn_webpush_popup_prompt' => 1, - ]; - - return [ - 'valid enabled settings' => [ - $valid, - [ - 'S_ERROR' => false, - 'ERROR_MSG' => '', - ], - [ - 'wpn_webpush_vapid_public' => str_repeat('a', 25), - 'wpn_webpush_vapid_private' => str_repeat('b', 25), - 'wpn_webpush_method_enabled' => 0, - ], - true, - ], - 'blank existing private key is preserved' => [ - array_merge($valid, [ - 'wpn_webpush_vapid_private' => '', - ]), - [ - 'S_ERROR' => false, - 'ERROR_MSG' => '', - ], - [ - 'wpn_webpush_vapid_public' => str_repeat('a', 25), - 'wpn_webpush_vapid_private' => 'existing-private-key', - ], - true, - ], - 'invalid enabled VAPID keys are rejected' => [ - array_merge($valid, [ - 'wpn_webpush_vapid_public' => 'short', - 'wpn_webpush_vapid_private' => 'short', - ]), - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'SETTING_TOO_SHORT
SETTING_TOO_SHORT', - ], - [ - 'wpn_webpush_vapid_public' => 'existing-public-key', - 'wpn_webpush_vapid_private' => 'existing-private-key', - ], - false, - ], - 'disabled webpush skips VAPID validation' => [ - array_merge($valid, [ - 'wpn_webpush_enable' => 0, - 'wpn_webpush_vapid_public' => '', - 'wpn_webpush_vapid_private' => '', - ]), - [ - 'S_ERROR' => false, - 'ERROR_MSG' => '', - ], - [ - 'wpn_webpush_enable' => 0, - 'wpn_webpush_vapid_public' => '', - 'wpn_webpush_vapid_private' => 'existing-private-key', - ], - true, - ], - ]; - } - - /** - * @dataProvider webpush_save_data - */ - 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->request->method('variable') - ->with('config', ['' => ''], true) - ->willReturn($input); - - $this->template->expects($this->once()) - ->method('assign_vars') - ->with($error_vars); - - $this->log->expects($expect_saved ? $this->once() : $this->never()) - ->method('add') - ->with('admin', 42, '127.0.0.1', 'LOG_CONFIG_WEBPUSH'); - - if ($expect_saved) - { - $this->assertTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED', function() { - $this->create_module()->save_settings(); - }); - } - else - { - $this->create_module()->save_settings(); - } - - foreach ($expected_config as $name => $value) - { - self::assertSame($value, $this->config[$name]); - } - } - - public function test_save_settings_requires_private_key_when_none_is_stored(): void - { - $this->config['wpn_webpush_vapid_public'] = ''; - $this->config['wpn_webpush_vapid_private'] = ''; - - $this->request->method('variable') - ->with('config', ['' => ''], true) - ->willReturn([ - 'wpn_webpush_enable' => 1, - 'wpn_webpush_vapid_public' => str_repeat('a', 25), - 'wpn_webpush_vapid_private' => '', - 'wpn_webpush_dropdown_subscribe' => 1, - 'wpn_webpush_method_enabled' => 0, - 'wpn_webpush_popup_prompt' => 1, - ]); - - $this->template->expects($this->once()) - ->method('assign_vars') - ->with([ - 'S_ERROR' => true, - 'ERROR_MSG' => 'SETTING_TOO_SHORT', - ]); - $this->log->expects($this->never())->method('add'); - - $this->create_module()->save_settings(); - - self::assertSame('', $this->config['wpn_webpush_vapid_private']); - } - - public function test_save_settings_ignores_unrelated_config(): void - { - $this->config['allow_privmsg'] = 1; - $this->request->method('variable') - ->with('config', ['' => ''], true) - ->willReturn([ - 'wpn_webpush_enable' => 0, - 'wpn_webpush_vapid_public' => 'existing-public-key', - 'wpn_webpush_vapid_private' => '', - 'wpn_webpush_dropdown_subscribe' => 1, - 'wpn_webpush_method_enabled' => 1, - 'wpn_webpush_popup_prompt' => 1, - 'allow_privmsg' => 0, - ]); - - $this->assertTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED', function() { - $this->create_module()->save_settings(); - }); - - self::assertSame(1, $this->config['allow_privmsg']); - } - - public function pwa_save_data(): array - { - $valid = [ - 'pwa_short_name' => 'Forum', - 'pwa_icon_small' => 'icon192.png', - 'pwa_icon_large' => 'icon512.png', - 'pwa_show_install_banner' => 1, - ]; - - $valid_style_input = [ - 'pwa_bg_color_1' => '#abcdef', - 'pwa_theme_color_1' => '#ABCDEF', - 'pwa_bg_color_2' => '', - 'pwa_theme_color_2' => '#123', - ]; - - $expected_style_updates = [ - [ - 'pwa_bg_color' => '#abcdef', - 'pwa_theme_color' => '#ABCDEF', - ], - [ - 'pwa_bg_color' => '', - 'pwa_theme_color' => '#123', - ], - ]; - - return [ - 'valid icons and style colours' => [ - $valid, - $valid_style_input, - [ - 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], - 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], - ], - [ - 'S_ERROR' => false, - 'ERROR_MSG' => '', - ], - [ - 'pwa_short_name' => 'Forum', - 'pwa_icon_small' => 'icon192.png', - 'pwa_icon_large' => 'icon512.png', - 'pwa_show_install_banner' => 1, - ], - $expected_style_updates, - true, - ], - 'valid empty name and icons' => [ - [], - [], - [], - [ - 'S_ERROR' => false, - 'ERROR_MSG' => '', - ], - [ - 'pwa_short_name' => '', - 'pwa_icon_small' => '', - 'pwa_icon_large' => '', - 'pwa_show_install_banner' => 0, - ], - [ - [ - 'pwa_bg_color' => '', - 'pwa_theme_color' => '', - ], - ], - true, - ], - 'unchanged style colours skipped' => [ - $valid, - [ - 'pwa_bg_color_1' => '#fff000', - 'pwa_theme_color_1' => '#000fff', - 'pwa_bg_color_2' => '', - 'pwa_theme_color_2' => '', - ], - [ - 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], - 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], - ], - [ - 'S_ERROR' => false, - 'ERROR_MSG' => '', - ], - [ - 'pwa_short_name' => 'Forum', - 'pwa_icon_small' => 'icon192.png', - 'pwa_icon_large' => 'icon512.png', - 'pwa_show_install_banner' => 1, - ], - [], - true, - ], - 'too long short name rejected after entity decoding' => [ - array_merge($valid, [ - 'pwa_short_name' => '123456789012😀', - ]), - $valid_style_input, - [ - 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], - 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], - ], - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'PWA_SHORT_NAME_INVALID', - ], - [ - 'pwa_short_name' => 'Old', - ], - [], - false, - ], - 'missing small icon rejected' => [ - array_merge($valid, [ - 'pwa_icon_small' => '', - ]), - $valid_style_input, - [], - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'PWA_ICON_NOT_PROVIDED:PWA_ICON_SMALL', - ], - [ - 'pwa_icon_small' => 'old-small.png', - ], - [], - false, - ], - 'missing large icon rejected' => [ - array_merge($valid, [ - 'pwa_icon_large' => '', - ]), - $valid_style_input, - [], - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'PWA_ICON_NOT_PROVIDED:PWA_ICON_LARGE', - ], - [ - 'pwa_icon_large' => 'old-large.png', - ], - [], - false, - ], - 'path traversal icon rejected' => [ - array_merge($valid, [ - 'pwa_icon_small' => '../icon192.png', - ]), - $valid_style_input, - [ - 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], - ], - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'PWA_ICON_INVALID:../icon192.png', - ], - [ - 'pwa_icon_small' => 'old-small.png', - ], - [], - false, - ], - 'unreadable icon rejected' => [ - $valid, - $valid_style_input, - [ - 'icon192.png' => false, - 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], - ], - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'PWA_ICON_INVALID:icon192.png', - ], - [ - 'pwa_icon_small' => 'old-small.png', - ], - [], - false, - ], - 'wrong icon size and MIME rejected' => [ - $valid, - $valid_style_input, - [ - 'icon192.png' => ['width' => 191, 'height' => 192, 'type' => IMAGETYPE_JPEG], - 'icon512.png' => ['width' => 512, 'height' => 511, 'type' => IMAGETYPE_PNG], - ], - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'PWA_ICON_SIZE_INVALID:icon192.png
PWA_ICON_MIME_INVALID:icon192.png
PWA_ICON_SIZE_INVALID:icon512.png', - ], - [ - 'pwa_icon_small' => 'old-small.png', - 'pwa_icon_large' => 'old-large.png', - ], - [], - false, - ], - 'invalid style colour rejected' => [ - $valid, - array_merge($valid_style_input, [ - 'pwa_theme_color_1' => 'not-hex', - ]), - [ - 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], - 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], - ], - [ - 'S_ERROR' => true, - 'ERROR_MSG' => 'PWA_INVALID_COLOUR:not-hex', - ], - [ - 'pwa_short_name' => 'Old', - ], - [], - false, - ], - ]; - } - - /** - * @dataProvider pwa_save_data - */ - public function test_save_pwa_settings_validates_and_persists_pwa_config(array $input, array $style_input, array $image_info, array $error_vars, array $expected_config, array $expected_style_updates, $expect_saved): void - { - $this->request->method('variable') - ->willReturnCallback(function($name, $default = null, $multibyte = false) use ($input, $style_input) { - if ($name === 'config') - { - return $input; - } - - return $style_input[$name] ?? $default; - }); - - $this->imagesize->method('getImageSize') - ->willReturnCallback(function($path) use ($image_info) { - return $image_info[basename($path)]; - }); - - $this->template->expects($this->once()) - ->method('assign_vars') - ->with($error_vars); - - $this->log->expects($expect_saved ? $this->once() : $this->never()) - ->method('add') - ->with('admin', 42, '127.0.0.1', 'LOG_CONFIG_WEBPUSH'); - - if ($expect_saved) - { - $this->assertTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED', function() { - $this->create_module('adm.php?i=test&mode=pwa')->save_pwa_settings(); - }); - } - else - { - $this->create_module('adm.php?i=test&mode=pwa')->save_pwa_settings(); - } - - foreach ($expected_config as $name => $value) - { - self::assertSame($value, $this->config[$name]); - } - - self::assertSame($expected_style_updates, $this->style_updates); - } - - public function hex_colour_data(): array - { - return [ - 'empty colour allowed' => ['', true, []], - 'valid 3 digit colour allowed' => [' #fff ', true, []], - 'valid 6 digit colour allowed' => ['#ABCDEF', true, []], - 'missing hash rejected' => ['123456', false, ['PWA_INVALID_COLOUR:123456']], - 'invalid character rejected' => ['#12345g', false, ['PWA_INVALID_COLOUR:#12345g']], - ]; - } - - /** - * @dataProvider hex_colour_data - */ - public function test_validate_hex_color($colour, $expected, array $expected_errors): void - { - $module = $this->create_module(); - - self::assertSame($expected, $this->invoke_method($module, 'validate_hex_color', [$colour])); - self::assertSame($expected_errors, $this->get_protected_property($module, 'errors')); - } - - public function test_display_errors_returns_false_without_errors(): void - { - $this->template->expects($this->once()) - ->method('assign_vars') - ->with([ - 'S_ERROR' => false, - 'ERROR_MSG' => '', - ]); - - self::assertFalse($this->create_module()->display_errors()); - } - - public function test_display_errors_returns_true_with_joined_errors(): void - { - $module = $this->create_module(); - $this->set_protected_property($module, 'errors', ['first', 'second']); - - $this->template->expects($this->once()) - ->method('assign_vars') - ->with([ - 'S_ERROR' => true, - 'ERROR_MSG' => 'first
second', - ]); - - self::assertTrue($module->display_errors()); - } - - protected function create_module($u_action = 'adm.php?i=test&mode=webpush'): wpn_acp_module - { - $module = new wpn_acp_module(); - $module->u_action = $u_action; - - foreach ([ - 'cache' => $this->cache, - 'config' => $this->config, - 'db' => $this->db, - 'imagesize' => $this->imagesize, - 'lang' => $this->lang, - 'log' => $this->log, - 'request' => $this->request, - 'symfony_request' => $this->symfony_request, - 'template' => $this->template, - 'user' => $this->user, - 'root_path' => $this->root_path, - ] as $property => $value) - { - $this->set_protected_property($module, $property, $value); - } - - return $module; - } - - protected function set_protected_property($object, $property, $value): void - { - $reflection = new \ReflectionClass($object); - $property = $reflection->getProperty($property); - $property->setAccessible(true); - $property->setValue($object, $value); - } - - protected function assertTriggerError($errno, $message, callable $callback): void - { - $caught = null; - set_error_handler(static function($severity, $error_message, $file, $line) { - throw new \ErrorException($error_message, 0, $severity, $file, $line); - }, $errno); - - try - { - $callback(); - } - catch (\ErrorException $exception) - { - $caught = $exception; - } - finally - { - restore_error_handler(); - } - - self::assertNotNull($caught, 'Expected trigger_error() was not raised.'); - self::assertSame($errno, $caught->getSeverity()); - self::assertStringContainsString($message, $caught->getMessage()); - } - - protected function get_protected_property($object, $property) - { - $reflection = new \ReflectionClass($object); - $property = $reflection->getProperty($property); - $property->setAccessible(true); - - return $property->getValue($object); - } - - protected function invoke_method($object, $method_name, array $parameters = []) - { - $reflection = new \ReflectionClass($object); - $method = $reflection->getMethod($method_name); - $method->setAccessible(true); - - return $method->invokeArgs($object, $parameters); - } - - protected function default_config(): array - { - return [ - 'sitename' => 'Board & 😀 Forum', - 'wpn_webpush_enable' => 1, - 'wpn_webpush_vapid_public' => 'existing-public-key', - 'wpn_webpush_vapid_private' => 'existing-private-key', - 'wpn_webpush_dropdown_subscribe' => 1, - 'wpn_webpush_method_enabled' => 1, - 'wpn_webpush_popup_prompt' => 1, - 'pwa_show_install_banner' => 1, - 'pwa_short_name' => 'Old', - 'pwa_icon_small' => 'old-small.png', - 'pwa_icon_large' => 'old-large.png', - ]; - } - - protected function style_rows(): array - { - return [ - [ - 'style_id' => 1, - 'style_name' => 'prosilver', - 'pwa_bg_color' => '#fff000', - 'pwa_theme_color' => '#000fff', - ], - [ - 'style_id' => 2, - 'style_name' => 'silverfoo', - 'pwa_bg_color' => '', - 'pwa_theme_color' => '', - ], - ]; - } - - protected function webpush_template_vars($u_action): array - { - return [ - 'S_WEBPUSH_ENABLE' => 1, - 'WEBPUSH_VAPID_PUBLIC' => 'existing-public-key', - 'S_VAPID_KEYS_CONFIGURED' => true, - 'S_VAPID_PRIVATE_CONFIGURED' => true, - 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => 1, - 'S_WEBPUSH_METHOD_ENABLED' => 1, - 'S_WEBPUSH_POPUP_PROMPT' => 1, - 'U_ACTION' => $u_action, - ]; - } - - protected function pwa_template_vars($u_action): array - { - return [ - 'S_PWA_SHOW_INSTALL_BANNER' => true, - 'PWA_SHORT_NAME' => 'Old', - 'PWA_SHORT_NAME_PLACEHOLDER' => 'Board & 😀 Fo', - 'PWA_ICON_SMALL' => 'old-small.png', - 'PWA_ICON_LARGE' => 'old-large.png', - 'STYLES' => $this->style_rows(), - 'U_ACTION' => $u_action, - ]; - } -} - -namespace phpbb\webpushnotifications\acp; - -function add_form_key() -{ -} - -function check_form_key() -{ - return \phpbb\webpushnotifications\tests\acp\acp_module_test::$valid_form; -} - -class routing_acp_module extends wpn_acp_module -{ - public $calls = []; - - public function save_settings() - { - $this->calls[] = __FUNCTION__; - } - - public function save_pwa_settings() - { - $this->calls[] = __FUNCTION__; - } - - public function display_settings() - { - } - - public function display_pwa_settings() - { - } } diff --git a/tests/controller/controller_acp_test.php b/tests/controller/controller_acp_test.php new file mode 100644 index 0000000..25b087e --- /dev/null +++ b/tests/controller/controller_acp_test.php @@ -0,0 +1,834 @@ +cache = $this->createMock('\phpbb\cache\service'); + $this->config = new \phpbb\config\config($this->default_config()); + $this->db = $this->createMock('\phpbb\db\driver\driver_interface'); + $this->imagesize = $this->createMock('\FastImageSize\FastImageSize'); + $this->lang = $this->createMock('\phpbb\language\language'); + $this->log = $this->createMock('\phpbb\log\log_interface'); + $this->request = $this->createMock('\phpbb\request\request'); + $this->symfony_request = $this->createMock('\phpbb\symfony_request'); + $this->template = $this->createMock('\phpbb\template\template'); + $this->user = $this->createMock('\phpbb\user'); + $this->root_path = $phpbb_root_path; + $this->style_updates = []; + + $this->user->data = ['user_id' => 42]; + $this->user->ip = '127.0.0.1'; + $this->user->lang = new \phpbb_mock_lang(); + + $this->lang->method('lang') + ->willReturnCallback(function() { + return implode(':', func_get_args()); + }); + + $this->db->method('sql_query') + ->willReturn('result'); + $this->db->method('sql_fetchrowset') + ->with('result') + ->willReturn($this->style_rows()); + $this->db->method('sql_build_array') + ->willReturnCallback(function($query, $row) { + $this->style_updates[] = $row; + return implode(', ', array_map(static function($name, $value) { + return $name . " = '" . $value . "'"; + }, array_keys($row), $row)); + }); + + $template = $this->template; + $user = $this->user; + $language = $this->lang; + } + + public function mode_display_data(): array + { + return [ + 'webpush secure server' => [ + 'webpush', + 'adm.php?i=test&mode=webpush', + true, + 'example.com', + $this->webpush_template_vars('adm.php?i=test&mode=webpush'), + false, + '', + ], + 'webpush localhost without https' => [ + 'webpush', + 'adm.php?i=test&mode=webpush', + false, + 'localhost', + $this->webpush_template_vars('adm.php?i=test&mode=webpush'), + false, + '', + ], + 'webpush insecure remote server' => [ + 'webpush', + 'adm.php?i=test&mode=webpush', + false, + 'example.com', + $this->webpush_template_vars('adm.php?i=test&mode=webpush'), + true, + 'WEBPUSH_INSECURE_SERVER_ERROR', + ], + 'pwa settings' => [ + 'pwa', + 'adm.php?i=test&mode=pwa', + true, + 'example.com', + $this->pwa_template_vars('adm.php?i=test&mode=pwa'), + false, + '', + ], + ]; + } + + /** + * @dataProvider mode_display_data + */ + public function test_handle_displays_mode_settings($mode, $u_action, $is_secure, $server_name, array $settings_vars, $has_error, $error_msg): void + { + $this->lang->expects($this->once()) + ->method('add_lang') + ->with('webpushnotifications_module_acp', 'phpbb/webpushnotifications'); + + $this->request->expects($this->once()) + ->method('is_set_post') + ->with('submit') + ->willReturn(false); + + $this->symfony_request->method('isSecure') + ->willReturn($is_secure); + + $this->request->method('server') + ->with('SERVER_NAME') + ->willReturn($server_name); + + $this->template->expects(self::exactly(2)) + ->method('assign_vars') + ->withConsecutive( + [$settings_vars], + [[ + 'S_ERROR' => $has_error, + 'ERROR_MSG' => $error_msg, + ]] + ); + + $controller = $this->create_controller($u_action); + $controller->{'handle_' . $mode}(); + } + + public function invalid_form_key_data(): array + { + return [ + 'webpush' => ['webpush'], + 'pwa' => ['pwa'], + ]; + } + + /** + * @dataProvider invalid_form_key_data + */ + public function test_handle_rejects_invalid_form_key($mode): void + { + self::$valid_form = false; + + $this->request->method('is_set_post') + ->with('submit') + ->willReturn(true); + + $this->assertTriggerError(E_USER_WARNING, 'FORM_INVALID', function() use ($mode) { + $this->create_controller('adm.php?i=test&mode=' . $mode)->{'handle_' . $mode}(); + }); + } + + 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_dropdown_subscribe' => 1, + 'wpn_webpush_method_enabled' => 0, + 'wpn_webpush_popup_prompt' => 1, + ]; + + return [ + 'valid enabled settings' => [ + $valid, + [ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ], + [ + 'wpn_webpush_vapid_public' => str_repeat('a', 25), + 'wpn_webpush_vapid_private' => str_repeat('b', 25), + 'wpn_webpush_method_enabled' => 0, + ], + true, + ], + 'blank existing private key is preserved' => [ + array_merge($valid, [ + 'wpn_webpush_vapid_private' => '', + ]), + [ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ], + [ + 'wpn_webpush_vapid_public' => str_repeat('a', 25), + 'wpn_webpush_vapid_private' => 'existing-private-key', + ], + true, + ], + 'invalid enabled VAPID keys are rejected' => [ + array_merge($valid, [ + 'wpn_webpush_vapid_public' => 'short', + 'wpn_webpush_vapid_private' => 'short', + ]), + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'SETTING_TOO_SHORT
SETTING_TOO_SHORT', + ], + [ + 'wpn_webpush_vapid_public' => 'existing-public-key', + 'wpn_webpush_vapid_private' => 'existing-private-key', + ], + false, + ], + 'disabled webpush skips VAPID validation' => [ + array_merge($valid, [ + 'wpn_webpush_enable' => 0, + 'wpn_webpush_vapid_public' => '', + 'wpn_webpush_vapid_private' => '', + ]), + [ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ], + [ + 'wpn_webpush_enable' => 0, + 'wpn_webpush_vapid_public' => '', + 'wpn_webpush_vapid_private' => 'existing-private-key', + ], + true, + ], + ]; + } + + /** + * @dataProvider webpush_save_data + */ + 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->request->method('variable') + ->with('config', ['' => ''], true) + ->willReturn($input); + + $this->template->expects($this->once()) + ->method('assign_vars') + ->with($error_vars); + + $this->log->expects($expect_saved ? $this->once() : $this->never()) + ->method('add') + ->with('admin', 42, '127.0.0.1', 'LOG_CONFIG_WEBPUSH'); + + if ($expect_saved) + { + $controller = $this->create_controller(); + $this->assertTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED', function() use ($controller) { + $this->invoke_method($controller, 'save_settings'); + }); + } + else + { + $this->invoke_method($this->create_controller(), 'save_settings'); + } + + foreach ($expected_config as $name => $value) + { + self::assertSame($value, $this->config[$name]); + } + } + + public function test_save_settings_requires_private_key_when_none_is_stored(): void + { + $this->config['wpn_webpush_vapid_public'] = ''; + $this->config['wpn_webpush_vapid_private'] = ''; + + $this->request->method('variable') + ->with('config', ['' => ''], true) + ->willReturn([ + 'wpn_webpush_enable' => 1, + 'wpn_webpush_vapid_public' => str_repeat('a', 25), + 'wpn_webpush_vapid_private' => '', + 'wpn_webpush_dropdown_subscribe' => 1, + 'wpn_webpush_method_enabled' => 0, + 'wpn_webpush_popup_prompt' => 1, + ]); + + $this->template->expects($this->once()) + ->method('assign_vars') + ->with([ + 'S_ERROR' => true, + 'ERROR_MSG' => 'SETTING_TOO_SHORT', + ]); + $this->log->expects($this->never())->method('add'); + + $this->invoke_method($this->create_controller(), 'save_settings'); + + self::assertSame('', $this->config['wpn_webpush_vapid_private']); + } + + public function test_save_settings_ignores_unrelated_config(): void + { + $this->config['allow_privmsg'] = 1; + $this->request->method('variable') + ->with('config', ['' => ''], true) + ->willReturn([ + 'wpn_webpush_enable' => 0, + 'wpn_webpush_vapid_public' => 'existing-public-key', + 'wpn_webpush_vapid_private' => '', + 'wpn_webpush_dropdown_subscribe' => 1, + 'wpn_webpush_method_enabled' => 1, + 'wpn_webpush_popup_prompt' => 1, + 'allow_privmsg' => 0, + ]); + + $controller = $this->create_controller(); + $this->assertTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED', function() use ($controller) { + $this->invoke_method($controller, 'save_settings'); + }); + + self::assertSame(1, $this->config['allow_privmsg']); + } + + public function pwa_save_data(): array + { + $valid = [ + 'pwa_short_name' => 'Forum', + 'pwa_icon_small' => 'icon192.png', + 'pwa_icon_large' => 'icon512.png', + 'pwa_show_install_banner' => 1, + ]; + + $valid_style_input = [ + 'pwa_bg_color_1' => '#abcdef', + 'pwa_theme_color_1' => '#ABCDEF', + 'pwa_bg_color_2' => '', + 'pwa_theme_color_2' => '#123', + ]; + + $expected_style_updates = [ + [ + 'pwa_bg_color' => '#abcdef', + 'pwa_theme_color' => '#ABCDEF', + ], + [ + 'pwa_bg_color' => '', + 'pwa_theme_color' => '#123', + ], + ]; + + return [ + 'valid icons and style colours' => [ + $valid, + $valid_style_input, + [ + 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], + 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ], + [ + 'pwa_short_name' => 'Forum', + 'pwa_icon_small' => 'icon192.png', + 'pwa_icon_large' => 'icon512.png', + 'pwa_show_install_banner' => 1, + ], + $expected_style_updates, + true, + ], + 'valid empty name and icons' => [ + [], + [], + [], + [ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ], + [ + 'pwa_short_name' => '', + 'pwa_icon_small' => '', + 'pwa_icon_large' => '', + 'pwa_show_install_banner' => 0, + ], + [ + [ + 'pwa_bg_color' => '', + 'pwa_theme_color' => '', + ], + ], + true, + ], + 'unchanged style colours skipped' => [ + $valid, + [ + 'pwa_bg_color_1' => '#fff000', + 'pwa_theme_color_1' => '#000fff', + 'pwa_bg_color_2' => '', + 'pwa_theme_color_2' => '', + ], + [ + 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], + 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ], + [ + 'pwa_short_name' => 'Forum', + 'pwa_icon_small' => 'icon192.png', + 'pwa_icon_large' => 'icon512.png', + 'pwa_show_install_banner' => 1, + ], + [], + true, + ], + 'too long short name rejected after entity decoding' => [ + array_merge($valid, [ + 'pwa_short_name' => '123456789012😀', + ]), + $valid_style_input, + [ + 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], + 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'PWA_SHORT_NAME_INVALID', + ], + [ + 'pwa_short_name' => 'Old', + ], + [], + false, + ], + 'missing small icon rejected' => [ + array_merge($valid, [ + 'pwa_icon_small' => '', + ]), + $valid_style_input, + [], + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'PWA_ICON_NOT_PROVIDED:PWA_ICON_SMALL', + ], + [ + 'pwa_icon_small' => 'old-small.png', + ], + [], + false, + ], + 'missing large icon rejected' => [ + array_merge($valid, [ + 'pwa_icon_large' => '', + ]), + $valid_style_input, + [], + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'PWA_ICON_NOT_PROVIDED:PWA_ICON_LARGE', + ], + [ + 'pwa_icon_large' => 'old-large.png', + ], + [], + false, + ], + 'path traversal icon rejected' => [ + array_merge($valid, [ + 'pwa_icon_small' => '../icon192.png', + ]), + $valid_style_input, + [ + 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'PWA_ICON_INVALID:../icon192.png', + ], + [ + 'pwa_icon_small' => 'old-small.png', + ], + [], + false, + ], + 'unreadable icon rejected' => [ + $valid, + $valid_style_input, + [ + 'icon192.png' => false, + 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'PWA_ICON_INVALID:icon192.png', + ], + [ + 'pwa_icon_small' => 'old-small.png', + ], + [], + false, + ], + 'wrong icon size and MIME rejected' => [ + $valid, + $valid_style_input, + [ + 'icon192.png' => ['width' => 191, 'height' => 192, 'type' => IMAGETYPE_JPEG], + 'icon512.png' => ['width' => 512, 'height' => 511, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'PWA_ICON_SIZE_INVALID:icon192.png
PWA_ICON_MIME_INVALID:icon192.png
PWA_ICON_SIZE_INVALID:icon512.png', + ], + [ + 'pwa_icon_small' => 'old-small.png', + 'pwa_icon_large' => 'old-large.png', + ], + [], + false, + ], + 'invalid style colour rejected' => [ + $valid, + array_merge($valid_style_input, [ + 'pwa_theme_color_1' => 'not-hex', + ]), + [ + 'icon192.png' => ['width' => 192, 'height' => 192, 'type' => IMAGETYPE_PNG], + 'icon512.png' => ['width' => 512, 'height' => 512, 'type' => IMAGETYPE_PNG], + ], + [ + 'S_ERROR' => true, + 'ERROR_MSG' => 'PWA_INVALID_COLOUR:not-hex', + ], + [ + 'pwa_short_name' => 'Old', + ], + [], + false, + ], + ]; + } + + /** + * @dataProvider pwa_save_data + */ + public function test_save_pwa_settings_validates_and_persists_pwa_config(array $input, array $style_input, array $image_info, array $error_vars, array $expected_config, array $expected_style_updates, $expect_saved): void + { + $this->request->method('variable') + ->willReturnCallback(function($name, $default = null, $multibyte = false) use ($input, $style_input) { + if ($name === 'config') + { + return $input; + } + + return $style_input[$name] ?? $default; + }); + + $this->imagesize->method('getImageSize') + ->willReturnCallback(function($path) use ($image_info) { + return $image_info[basename($path)]; + }); + + $this->template->expects($this->once()) + ->method('assign_vars') + ->with($error_vars); + + $this->log->expects($expect_saved ? $this->once() : $this->never()) + ->method('add') + ->with('admin', 42, '127.0.0.1', 'LOG_CONFIG_WEBPUSH'); + + if ($expect_saved) + { + $controller = $this->create_controller('adm.php?i=test&mode=pwa'); + $this->assertTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED', function() use ($controller) { + $this->invoke_method($controller, 'save_pwa_settings'); + }); + } + else + { + $this->invoke_method($this->create_controller('adm.php?i=test&mode=pwa'), 'save_pwa_settings'); + } + + foreach ($expected_config as $name => $value) + { + self::assertSame($value, $this->config[$name]); + } + + self::assertSame($expected_style_updates, $this->style_updates); + } + + public function hex_colour_data(): array + { + return [ + 'empty colour allowed' => ['', true, []], + 'valid 3 digit colour allowed' => [' #fff ', true, []], + 'valid 6 digit colour allowed' => ['#ABCDEF', true, []], + 'missing hash rejected' => ['123456', false, ['PWA_INVALID_COLOUR:123456']], + 'invalid character rejected' => ['#12345g', false, ['PWA_INVALID_COLOUR:#12345g']], + ]; + } + + /** + * @dataProvider hex_colour_data + */ + public function test_validate_hex_color($colour, $expected, array $expected_errors): void + { + $controller = $this->create_controller(); + + self::assertSame($expected, $this->invoke_method($controller, 'validate_hex_color', [$colour])); + self::assertSame($expected_errors, $this->get_protected_property($controller, 'errors')); + } + + public function test_display_errors_returns_false_without_errors(): void + { + $this->template->expects($this->once()) + ->method('assign_vars') + ->with([ + 'S_ERROR' => false, + 'ERROR_MSG' => '', + ]); + + self::assertFalse($this->invoke_method($this->create_controller(), 'display_errors')); + } + + public function test_display_errors_returns_true_with_joined_errors(): void + { + $controller = $this->create_controller(); + $this->set_protected_property($controller, 'errors', ['first', 'second']); + + $this->template->expects($this->once()) + ->method('assign_vars') + ->with([ + 'S_ERROR' => true, + 'ERROR_MSG' => 'first
second', + ]); + + self::assertTrue($this->invoke_method($controller, 'display_errors')); + } + + protected function create_controller($u_action = 'adm.php?i=test&mode=webpush'): acp_controller + { + return (new acp_controller( + $this->cache, + $this->config, + $this->db, + $this->imagesize, + $this->lang, + $this->log, + $this->request, + $this->symfony_request, + $this->template, + $this->user, + $this->root_path + ))->set_page_url($u_action); + } + + protected function set_protected_property($object, $property, $value): void + { + $reflection = new \ReflectionClass($object); + $property = $reflection->getProperty($property); + $property->setAccessible(true); + $property->setValue($object, $value); + } + + protected function assertTriggerError($errno, $message, callable $callback): void + { + $caught = null; + set_error_handler(static function($severity, $error_message, $file, $line) { + throw new \ErrorException($error_message, 0, $severity, $file, $line); + }, $errno); + + try + { + $callback(); + } + catch (\ErrorException $exception) + { + $caught = $exception; + } + finally + { + restore_error_handler(); + } + + self::assertNotNull($caught, 'Expected trigger_error() was not raised.'); + self::assertSame($errno, $caught->getSeverity()); + self::assertStringContainsString($message, $caught->getMessage()); + } + + protected function get_protected_property($object, $property) + { + $reflection = new \ReflectionClass($object); + $property = $reflection->getProperty($property); + $property->setAccessible(true); + + return $property->getValue($object); + } + + protected function invoke_method($object, $method_name, array $parameters = []) + { + $reflection = new \ReflectionClass($object); + $method = $reflection->getMethod($method_name); + $method->setAccessible(true); + + return $method->invokeArgs($object, $parameters); + } + + protected function default_config(): array + { + return [ + 'sitename' => 'Board & 😀 Forum', + 'wpn_webpush_enable' => 1, + 'wpn_webpush_vapid_public' => 'existing-public-key', + 'wpn_webpush_vapid_private' => 'existing-private-key', + 'wpn_webpush_dropdown_subscribe' => 1, + 'wpn_webpush_method_enabled' => 1, + 'wpn_webpush_popup_prompt' => 1, + 'pwa_show_install_banner' => 1, + 'pwa_short_name' => 'Old', + 'pwa_icon_small' => 'old-small.png', + 'pwa_icon_large' => 'old-large.png', + ]; + } + + protected function style_rows(): array + { + return [ + [ + 'style_id' => 1, + 'style_name' => 'prosilver', + 'pwa_bg_color' => '#fff000', + 'pwa_theme_color' => '#000fff', + ], + [ + 'style_id' => 2, + 'style_name' => 'silverfoo', + 'pwa_bg_color' => '', + 'pwa_theme_color' => '', + ], + ]; + } + + protected function webpush_template_vars($u_action): array + { + return [ + 'S_WEBPUSH_ENABLE' => 1, + 'WEBPUSH_VAPID_PUBLIC' => 'existing-public-key', + 'S_VAPID_KEYS_CONFIGURED' => true, + 'S_VAPID_PRIVATE_CONFIGURED' => true, + 'S_WEBPUSH_DROPDOWN_SUBSCRIBE' => 1, + 'S_WEBPUSH_METHOD_ENABLED' => 1, + 'S_WEBPUSH_POPUP_PROMPT' => 1, + 'U_ACTION' => $u_action, + ]; + } + + protected function pwa_template_vars($u_action): array + { + return [ + 'S_PWA_SHOW_INSTALL_BANNER' => true, + 'PWA_SHORT_NAME' => 'Old', + 'PWA_SHORT_NAME_PLACEHOLDER' => 'Board & 😀 Fo', + 'PWA_ICON_SMALL' => 'old-small.png', + 'PWA_ICON_LARGE' => 'old-large.png', + 'STYLES' => $this->style_rows(), + 'U_ACTION' => $u_action, + ]; + } +} + +namespace phpbb\webpushnotifications\controller; + +function add_form_key() +{ +} + +function check_form_key() +{ + return \phpbb\webpushnotifications\tests\controller\controller_acp_test::$valid_form; +}