diff --git a/notification/method/webpush.php b/notification/method/webpush.php index 7d58f8f..d558723 100644 --- a/notification/method/webpush.php +++ b/notification/method/webpush.php @@ -282,7 +282,8 @@ protected function notify_using_webpush(): void if (!$report->isSuccess()) { // Fill array of endpoints to remove if subscription has expired - if ($report->isSubscriptionExpired()) + // Library checks for 404/410; we also check for 401 (Unauthorized) + if ($report->isSubscriptionExpired() || $this->is_subscription_unauthorized($report)) { $expired_endpoints[] = $report->getEndpoint(); } @@ -495,4 +496,20 @@ protected function set_endpoint_padding(\Minishlink\WebPush\WebPush $web_push, s } } } + + /** + * Check if subscription push failed with 401 Unauthorized status + * + * 401 indicates the push service no longer accepts this subscription, + * typically due to revoked credentials or subscription no longer being valid. + * + * @param \Minishlink\WebPush\MessageSentReport $report + * + * @return bool True if subscription returned 401 Unauthorized + */ + protected function is_subscription_unauthorized(\Minishlink\WebPush\MessageSentReport $report): bool + { + $response = $report->getResponse(); + return $response && $response->getStatusCode() === 401; + } } diff --git a/tests/notification/notification_method_webpush_test.php b/tests/notification/notification_method_webpush_test.php index 3c472c2..5b03b73 100644 --- a/tests/notification/notification_method_webpush_test.php +++ b/tests/notification/notification_method_webpush_test.php @@ -551,11 +551,143 @@ public function test_notify_expired($notification_type, $post_data, $expected_us } } + /** + * @dataProvider data_notification_webpush + */ + public function test_expired_subscriptions_deleted($notification_type, $post_data, $expected_users) + { + // Skip test if no expected users + if (empty($expected_users)) + { + $this->assertTrue(true); + return; + } + + $subscription_info = []; + foreach ($expected_users as $user_id => $user_data) + { + $subscription_info[$user_id][] = $this->create_subscription_for_user($user_id); + } + + // Get first user and expire their subscription + $first_user_id = array_key_first($expected_users); + $first_user_sub = $subscription_info[$first_user_id][0]; + $this->expire_subscription($first_user_sub['clientHash']); + + // Count subscriptions before notification + $subscriptions_before = $this->get_subscription_count(); + $this->assertEquals(count($expected_users), $subscriptions_before, 'Expected ' . count($expected_users) . ' subscriptions before notification'); + + $post_data = array_merge([ + 'post_time' => 1349413322, + 'poster_id' => 1, + 'topic_title' => '', + 'post_subject' => '', + 'post_username' => '', + 'forum_name' => '', + ], $post_data); + + // Send notifications, which should trigger cleanup of expired subscription + $this->notifications->add_notifications($notification_type, $post_data); + + // Count subscriptions after notification - expired one should be deleted + $subscriptions_after = $this->get_subscription_count(); + $this->assertEquals(count($expected_users) - 1, $subscriptions_after, 'Expected expired subscription to be deleted'); + + // Verify the expired subscription is actually gone + $remaining_subs = $this->get_all_subscriptions(); + foreach ($remaining_subs as $sub) + { + $this->assertNotEquals($first_user_sub['endpoint'], $sub['endpoint'], 'Expired subscription should be deleted'); + } + } + public function test_get_type(): void { $this->assertEquals('notification.method.phpbb.wpn.webpush', $this->notification_method_webpush->get_type()); } + /** + * Test is_subscription_unauthorized method with various HTTP status codes + */ + public function test_is_subscription_unauthorized(): void + { + $reflection = new \ReflectionMethod($this->notification_method_webpush, 'is_subscription_unauthorized'); + $reflection->setAccessible(true); + + // Test 401 status (should return true) + $response_401 = $this->createMockResponse(401); + $request_401 = $this->createMockRequest(); + $report_401 = new \Minishlink\WebPush\MessageSentReport($request_401, $response_401, false, 'Unauthorized'); + $this->assertTrue($reflection->invoke($this->notification_method_webpush, $report_401), 'Expected 401 to be treated as unauthorized'); + + // Test 404 status (should return false, handled by isSubscriptionExpired) + $response_404 = $this->createMockResponse(404); + $request_404 = $this->createMockRequest(); + $report_404 = new \Minishlink\WebPush\MessageSentReport($request_404, $response_404, false, 'Not Found'); + $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_404), 'Expected 404 to not be treated as unauthorized'); + + // Test 410 status (should return false, handled by isSubscriptionExpired) + $response_410 = $this->createMockResponse(410); + $request_410 = $this->createMockRequest(); + $report_410 = new \Minishlink\WebPush\MessageSentReport($request_410, $response_410, false, 'Gone'); + $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_410), 'Expected 410 to not be treated as unauthorized'); + + // Test 429 status (should return false, temporary error) + $response_429 = $this->createMockResponse(429); + $request_429 = $this->createMockRequest(); + $report_429 = new \Minishlink\WebPush\MessageSentReport($request_429, $response_429, false, 'Too Many Requests'); + $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_429), 'Expected 429 to not be treated as unauthorized'); + + // Test 500 status (should return false, temporary error) + $response_500 = $this->createMockResponse(500); + $request_500 = $this->createMockRequest(); + $report_500 = new \Minishlink\WebPush\MessageSentReport($request_500, $response_500, false, 'Internal Server Error'); + $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_500), 'Expected 500 to not be treated as unauthorized'); + + // Test null response (network failure - should return false) + $request_null = $this->createMockRequest(); + $report_null = new \Minishlink\WebPush\MessageSentReport($request_null, null, false, 'Network error'); + $this->assertFalse($reflection->invoke($this->notification_method_webpush, $report_null), 'Expected null response to not be treated as unauthorized'); + } + + /** + * Create a mock PSR-7 ResponseInterface with specified status code + */ + protected function createMockResponse(int $status_code): \Psr\Http\Message\ResponseInterface + { + $response = $this->getMockBuilder(\Psr\Http\Message\ResponseInterface::class) + ->getMock(); + $response->method('getStatusCode') + ->willReturn($status_code); + return $response; + } + + /** + * Create a mock PSR-7 RequestInterface + */ + protected function createMockRequest(): \Psr\Http\Message\RequestInterface + { + $uri = $this->getMockBuilder(\Psr\Http\Message\UriInterface::class) + ->getMock(); + $uri->method('__toString') + ->willReturn('http://localhost:9012/notify/test'); + + $request = $this->getMockBuilder(\Psr\Http\Message\RequestInterface::class) + ->getMock(); + $request->method('getUri') + ->willReturn($uri); + + $body = $this->getMockBuilder(\Psr\Http\Message\StreamInterface::class) + ->getMock(); + $body->method('getContents') + ->willReturn('test payload'); + $request->method('getBody') + ->willReturn($body); + + return $request; + } + /** * @dataProvider data_notification_webpush */ @@ -751,6 +883,28 @@ protected function get_notifications(): array return $sql_ary; } + protected function get_subscription_count(): int + { + $push_subscriptions_table = $this->container->getParameter('tables.phpbb.wpn.push_subscriptions'); + $sql = 'SELECT COUNT(*) as count FROM ' . $push_subscriptions_table; + $result = $this->db->sql_query($sql); + $count = (int) $this->db->sql_fetchfield('count'); + $this->db->sql_freeresult($result); + + return $count; + } + + protected function get_all_subscriptions(): array + { + $push_subscriptions_table = $this->container->getParameter('tables.phpbb.wpn.push_subscriptions'); + $sql = 'SELECT * FROM ' . $push_subscriptions_table; + $result = $this->db->sql_query($sql); + $sql_ary = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + + return $sql_ary; + } + /** * @depends test_get_subscription */