Remove unauthorized subscriptions

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

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
 	 */