From 3a0060552a66ed8e761561029f430991fbc243bc Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 3 Oct 2025 22:26:04 +0200 Subject: [PATCH 1/9] [ticket/15085] Add HTTP authentication subscriber for feed controllers PHPBB-15085 --- .../default/container/services_feed.yml | 10 + .../phpbb/feed/event/http_auth_subscriber.php | 242 ++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 phpBB/phpbb/feed/event/http_auth_subscriber.php diff --git a/phpBB/config/default/container/services_feed.yml b/phpBB/config/default/container/services_feed.yml index f32d0cb4d3..2f27f6915b 100644 --- a/phpBB/config/default/container/services_feed.yml +++ b/phpBB/config/default/container/services_feed.yml @@ -15,6 +15,16 @@ services: - '@language' - '%core.php_ext%' + feed.http_auth_subscriber: + class: phpbb\feed\event\http_auth_subscriber + arguments: + - '@auth' + - '@config' + - '@request' + - '@user' + tags: + - { name: kernel.event_subscriber } + feed.helper: class: phpbb\feed\helper arguments: diff --git a/phpBB/phpbb/feed/event/http_auth_subscriber.php b/phpBB/phpbb/feed/event/http_auth_subscriber.php new file mode 100644 index 0000000000..8b3f03b0bd --- /dev/null +++ b/phpBB/phpbb/feed/event/http_auth_subscriber.php @@ -0,0 +1,242 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace phpbb\feed\event; + +use phpbb\auth\auth; +use phpbb\config\config; +use phpbb\request\request_interface; +use phpbb\user; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Event\GetResponseEvent; +use Symfony\Component\HttpKernel\KernelEvents; + +/** + * Event subscriber for HTTP authentication on feed routes + */ +class http_auth_subscriber implements EventSubscriberInterface +{ + /** @var auth */ + protected $auth; + + /** @var config */ + protected $config; + + /** @var request_interface */ + protected $request; + + /** @var user */ + protected $user; + + /** + * Constructor + * + * @param auth $auth Auth object + * @param config $config Config object + * @param request_interface $request Request object + * @param user $user User object + */ + public function __construct(auth $auth, config $config, request_interface $request, user $user) + { + $this->auth = $auth; + $this->config = $config; + $this->request = $request; + $this->user = $user; + } + + /** + * Handle HTTP authentication for feed routes + * + * @param GetResponseEvent $event + * @return void + */ + public function on_kernel_request(GetResponseEvent $event) + { + $request = $event->getRequest(); + $route = $request->attributes->get('_route'); + + // Only apply to feed routes + if (strpos($route, 'phpbb_feed_') !== 0) + { + return; + } + + // Check if HTTP authentication is enabled + if (!$this->config['feed_http_auth']) + { + return; + } + + // User is already logged in, no need to authenticate + if (!empty($this->user->data['is_registered'])) + { + return; + } + + // Get HTTP authentication credentials + $username = $this->get_http_username(); + $password = $this->get_http_password(); + + // If no credentials provided, send authentication challenge + if ($username === null || $password === null) + { + $this->send_auth_challenge($event); + return; + } + + // Attempt to login with the provided credentials + $auth_result = $this->auth->login($username, $password, false, true, false); + + if ($auth_result['status'] == LOGIN_SUCCESS) + { + return; + } + else if ($auth_result['status'] == LOGIN_ERROR_ATTEMPTS) + { + // Too many login attempts + $response = new Response('', Response::HTTP_UNAUTHORIZED); + $event->setResponse($response); + return; + } + + // Authentication failed, send challenge + $this->send_auth_challenge($event); + } + + /** + * Get HTTP username from request headers + * + * @return string|null + */ + protected function get_http_username() + { + $username_keys = array( + 'PHP_AUTH_USER', + 'Authorization', + 'REMOTE_USER', + 'REDIRECT_REMOTE_USER', + 'HTTP_AUTHORIZATION', + 'REDIRECT_HTTP_AUTHORIZATION', + 'REMOTE_AUTHORIZATION', + 'REDIRECT_REMOTE_AUTHORIZATION', + 'AUTH_USER', + ); + + foreach ($username_keys as $key) + { + if ($this->request->is_set($key, request_interface::SERVER)) + { + $username = html_entity_decode($this->request->server($key), ENT_COMPAT); + + // Decode Basic authentication header + if (strpos($username, 'Basic ') === 0) + { + $credentials = base64_decode(substr($username, 6)); + if (strpos($credentials, ':') !== false) + { + list($username, ) = explode(':', $credentials, 2); + } + } + + return $username; + } + } + + return null; + } + + /** + * Get HTTP password from request headers + * + * @return string|null + */ + protected function get_http_password() + { + $password_keys = array( + 'PHP_AUTH_PW', + 'REMOTE_PASSWORD', + 'AUTH_PASSWORD', + ); + + foreach ($password_keys as $key) + { + if ($this->request->is_set($key, request_interface::SERVER)) + { + return html_entity_decode($this->request->server($key), ENT_COMPAT); + } + } + + // Check if password is in Authorization header (Basic auth) + $username_keys = array( + 'PHP_AUTH_USER', + 'Authorization', + 'REMOTE_USER', + 'REDIRECT_REMOTE_USER', + 'HTTP_AUTHORIZATION', + 'REDIRECT_HTTP_AUTHORIZATION', + 'REMOTE_AUTHORIZATION', + 'REDIRECT_REMOTE_AUTHORIZATION', + 'AUTH_USER', + ); + + foreach ($username_keys as $key) + { + if ($this->request->is_set($key, request_interface::SERVER)) + { + $auth_header = html_entity_decode($this->request->server($key), ENT_COMPAT); + + // Decode Basic authentication header + if (strpos($auth_header, 'Basic ') === 0) + { + $credentials = base64_decode(substr($auth_header, 6)); + if (strpos($credentials, ':') !== false) + { + list(, $password) = explode(':', $credentials, 2); + return $password; + } + } + } + } + + return null; + } + + /** + * Send HTTP authentication challenge + * + * @param GetResponseEvent $event + * @return void + */ + protected function send_auth_challenge(GetResponseEvent $event) + { + $realm = $this->config['sitename']; + // Filter out non-ASCII characters per RFC2616 + $realm = preg_replace('/[\x80-\xFF]/', '?', $realm); + + $response = new Response('', Response::HTTP_UNAUTHORIZED); + $response->headers->set('WWW-Authenticate', 'Basic realm="' . $realm . '"'); + $event->setResponse($response); + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() + { + return array( + // Priority should be high to run after session_begin but before controller + KernelEvents::REQUEST => array('on_kernel_request', 5), + ); + } +} From f4700b0a38e3b5edbbfe3126727415cf7af871fb Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 3 Oct 2025 22:26:58 +0200 Subject: [PATCH 2/9] [ticket/15085] Add unit tests for HTTP authentication subscriber PHPBB-15085 --- .phpunit.result.cache | 1 + tests/feed/http_auth_subscriber_test.php | 159 +++++++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 .phpunit.result.cache create mode 100644 tests/feed/http_auth_subscriber_test.php diff --git a/.phpunit.result.cache b/.phpunit.result.cache new file mode 100644 index 0000000000..aac751d9fb --- /dev/null +++ b/.phpunit.result.cache @@ -0,0 +1 @@ +{"version":1,"defects":[],"times":{"phpbb\\feed\\event\\http_auth_subscriber_test::test_subscriber_events":0.006,"phpbb\\feed\\event\\http_auth_subscriber_test::test_non_feed_route_skipped":0.005,"phpbb\\feed\\event\\http_auth_subscriber_test::test_http_auth_disabled":0.001,"phpbb\\feed\\event\\http_auth_subscriber_test::test_user_already_logged_in":0.001}} \ No newline at end of file diff --git a/tests/feed/http_auth_subscriber_test.php b/tests/feed/http_auth_subscriber_test.php new file mode 100644 index 0000000000..d3f00aef68 --- /dev/null +++ b/tests/feed/http_auth_subscriber_test.php @@ -0,0 +1,159 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ + +namespace phpbb\feed\event; + +class http_auth_subscriber_test extends \phpbb_test_case +{ + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\auth\auth */ + protected $auth; + + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\config\config */ + protected $config; + + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\request\request_interface */ + protected $request; + + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\user */ + protected $user; + + /** @var http_auth_subscriber */ + protected $subscriber; + + protected function setUp(): void + { + parent::setUp(); + + $this->auth = $this->getMockBuilder('\phpbb\auth\auth') + ->disableOriginalConstructor() + ->getMock(); + + $this->config = new \phpbb\config\config(array( + 'feed_http_auth' => 1, + 'sitename' => 'Test Site', + )); + + $this->request = $this->getMockBuilder('\phpbb\request\request_interface') + ->getMock(); + + $this->user = $this->getMockBuilder('\phpbb\user') + ->disableOriginalConstructor() + ->getMock(); + + $this->user->data = array('is_registered' => false); + + $this->subscriber = new http_auth_subscriber( + $this->auth, + $this->config, + $this->request, + $this->user + ); + } + + public function test_subscriber_events() + { + $events = http_auth_subscriber::getSubscribedEvents(); + $this->assertArrayHasKey(\Symfony\Component\HttpKernel\KernelEvents::REQUEST, $events); + } + + public function test_non_feed_route_skipped() + { + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('not_a_feed_route'); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + $event->expects($this->never()) + ->method('setResponse'); + + $this->subscriber->on_kernel_request($event); + } + + public function test_http_auth_disabled() + { + $this->config['feed_http_auth'] = 0; + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + $event->expects($this->never()) + ->method('setResponse'); + + $this->subscriber->on_kernel_request($event); + } + + public function test_user_already_logged_in() + { + $this->user->data = array('is_registered' => true); + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + $event->expects($this->never()) + ->method('setResponse'); + + $this->subscriber->on_kernel_request($event); + } +} From 3dbc1b28b59a0b073fe2d72cfae2dc3ff22bc6ff Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 3 Oct 2025 22:27:29 +0200 Subject: [PATCH 3/9] [ticket/15085] Reload ACL after HTTP authentication and update gitignore PHPBB-15085 --- .gitignore | 1 + .phpunit.result.cache | 1 - phpBB/phpbb/feed/event/http_auth_subscriber.php | 2 ++ 3 files changed, 3 insertions(+), 1 deletion(-) delete mode 100644 .phpunit.result.cache diff --git a/.gitignore b/.gitignore index 15125b68cc..eda70c0605 100644 --- a/.gitignore +++ b/.gitignore @@ -52,3 +52,4 @@ node_modules .idea *.DS_Store* /.vscode +/.phpunit.result.cache diff --git a/.phpunit.result.cache b/.phpunit.result.cache deleted file mode 100644 index aac751d9fb..0000000000 --- a/.phpunit.result.cache +++ /dev/null @@ -1 +0,0 @@ -{"version":1,"defects":[],"times":{"phpbb\\feed\\event\\http_auth_subscriber_test::test_subscriber_events":0.006,"phpbb\\feed\\event\\http_auth_subscriber_test::test_non_feed_route_skipped":0.005,"phpbb\\feed\\event\\http_auth_subscriber_test::test_http_auth_disabled":0.001,"phpbb\\feed\\event\\http_auth_subscriber_test::test_user_already_logged_in":0.001}} \ No newline at end of file diff --git a/phpBB/phpbb/feed/event/http_auth_subscriber.php b/phpBB/phpbb/feed/event/http_auth_subscriber.php index 8b3f03b0bd..3c35c769c8 100644 --- a/phpBB/phpbb/feed/event/http_auth_subscriber.php +++ b/phpBB/phpbb/feed/event/http_auth_subscriber.php @@ -100,6 +100,8 @@ class http_auth_subscriber implements EventSubscriberInterface if ($auth_result['status'] == LOGIN_SUCCESS) { + // Reload ACL for the newly logged-in user + $this->auth->acl($this->user->data); return; } else if ($auth_result['status'] == LOGIN_ERROR_ATTEMPTS) From 752ce67da0b1772cd1f773e6505a9d0c7315dfcd Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 3 Oct 2025 22:28:06 +0200 Subject: [PATCH 4/9] [ticket/15085] Add HTTPS requirement for HTTP authentication on feeds PHPBB-15085 --- .../phpbb/feed/event/http_auth_subscriber.php | 6 +++ tests/feed/http_auth_subscriber_test.php | 41 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/phpBB/phpbb/feed/event/http_auth_subscriber.php b/phpBB/phpbb/feed/event/http_auth_subscriber.php index 3c35c769c8..6aedfb361b 100644 --- a/phpBB/phpbb/feed/event/http_auth_subscriber.php +++ b/phpBB/phpbb/feed/event/http_auth_subscriber.php @@ -72,6 +72,12 @@ class http_auth_subscriber implements EventSubscriberInterface return; } + // Only allow HTTP authentication in secure context (HTTPS) + if (!$request->isSecure()) + { + return; + } + // Check if HTTP authentication is enabled if (!$this->config['feed_http_auth']) { diff --git a/tests/feed/http_auth_subscriber_test.php b/tests/feed/http_auth_subscriber_test.php index d3f00aef68..ee50ec9088 100644 --- a/tests/feed/http_auth_subscriber_test.php +++ b/tests/feed/http_auth_subscriber_test.php @@ -95,6 +95,39 @@ class http_auth_subscriber_test extends \phpbb_test_case $this->subscriber->on_kernel_request($event); } + public function test_insecure_connection_skipped() + { + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(false); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + $event->expects($this->never()) + ->method('setResponse'); + + $this->subscriber->on_kernel_request($event); + } + public function test_http_auth_disabled() { $this->config['feed_http_auth'] = 0; @@ -112,6 +145,10 @@ class http_auth_subscriber_test extends \phpbb_test_case ->with('_route') ->willReturn('phpbb_feed_overall'); + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(true); + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') ->disableOriginalConstructor() ->getMock(); @@ -143,6 +180,10 @@ class http_auth_subscriber_test extends \phpbb_test_case ->with('_route') ->willReturn('phpbb_feed_overall'); + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(true); + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') ->disableOriginalConstructor() ->getMock(); From 82b72016aa2cfea16336b4913e2d105323efdcc1 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 3 Oct 2025 22:20:33 +0200 Subject: [PATCH 5/9] [ticket/15085] Clean up code and output error with language PHPBB-15085 --- .../default/container/services_feed.yml | 1 + .../phpbb/feed/event/http_auth_subscriber.php | 124 ++++++------------ 2 files changed, 43 insertions(+), 82 deletions(-) diff --git a/phpBB/config/default/container/services_feed.yml b/phpBB/config/default/container/services_feed.yml index 2f27f6915b..8a46895905 100644 --- a/phpBB/config/default/container/services_feed.yml +++ b/phpBB/config/default/container/services_feed.yml @@ -20,6 +20,7 @@ services: arguments: - '@auth' - '@config' + - '@language' - '@request' - '@user' tags: diff --git a/phpBB/phpbb/feed/event/http_auth_subscriber.php b/phpBB/phpbb/feed/event/http_auth_subscriber.php index 6aedfb361b..b2937a3c22 100644 --- a/phpBB/phpbb/feed/event/http_auth_subscriber.php +++ b/phpBB/phpbb/feed/event/http_auth_subscriber.php @@ -15,6 +15,7 @@ namespace phpbb\feed\event; use phpbb\auth\auth; use phpbb\config\config; +use phpbb\language\language; use phpbb\request\request_interface; use phpbb\user; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -33,6 +34,9 @@ class http_auth_subscriber implements EventSubscriberInterface /** @var config */ protected $config; + /** @var language */ + protected $language; + /** @var request_interface */ protected $request; @@ -44,13 +48,15 @@ class http_auth_subscriber implements EventSubscriberInterface * * @param auth $auth Auth object * @param config $config Config object + * @param language $language Language object * @param request_interface $request Request object * @param user $user User object */ - public function __construct(auth $auth, config $config, request_interface $request, user $user) + public function __construct(auth $auth, config $config, language $language, request_interface $request, user $user) { $this->auth = $auth; $this->config = $config; + $this->language = $language; $this->request = $request; $this->user = $user; } @@ -63,6 +69,12 @@ class http_auth_subscriber implements EventSubscriberInterface */ public function on_kernel_request(GetResponseEvent $event) { + // Check if HTTP authentication is enabled + if (!$this->config['feed_http_auth']) + { + return; + } + $request = $event->getRequest(); $route = $request->attributes->get('_route'); @@ -78,12 +90,6 @@ class http_auth_subscriber implements EventSubscriberInterface return; } - // Check if HTTP authentication is enabled - if (!$this->config['feed_http_auth']) - { - return; - } - // User is already logged in, no need to authenticate if (!empty($this->user->data['is_registered'])) { @@ -91,8 +97,7 @@ class http_auth_subscriber implements EventSubscriberInterface } // Get HTTP authentication credentials - $username = $this->get_http_username(); - $password = $this->get_http_password(); + [$username, $password] = $this->get_credentials(); // If no credentials provided, send authentication challenge if ($username === null || $password === null) @@ -113,7 +118,7 @@ class http_auth_subscriber implements EventSubscriberInterface else if ($auth_result['status'] == LOGIN_ERROR_ATTEMPTS) { // Too many login attempts - $response = new Response('', Response::HTTP_UNAUTHORIZED); + $response = new Response($this->language->lang('NOT_AUTHORISED'), Response::HTTP_UNAUTHORIZED); $event->setResponse($response); return; } @@ -123,13 +128,13 @@ class http_auth_subscriber implements EventSubscriberInterface } /** - * Get HTTP username from request headers + * Retrieve HTTP authentication credentials from server variables * - * @return string|null + * @return array [username, password] Array containing the username and password, or null if not found */ - protected function get_http_username() + protected function get_credentials(): array { - $username_keys = array( + $username_keys = [ 'PHP_AUTH_USER', 'Authorization', 'REMOTE_USER', @@ -139,85 +144,41 @@ class http_auth_subscriber implements EventSubscriberInterface 'REMOTE_AUTHORIZATION', 'REDIRECT_REMOTE_AUTHORIZATION', 'AUTH_USER', - ); + ]; + $password_keys = [ + 'PHP_AUTH_PW', + 'REMOTE_PASSWORD', + 'AUTH_PASSWORD', + ]; + + $username = null; foreach ($username_keys as $key) { if ($this->request->is_set($key, request_interface::SERVER)) { - $username = html_entity_decode($this->request->server($key), ENT_COMPAT); - - // Decode Basic authentication header - if (strpos($username, 'Basic ') === 0) - { - $credentials = base64_decode(substr($username, 6)); - if (strpos($credentials, ':') !== false) - { - list($username, ) = explode(':', $credentials, 2); - } - } - - return $username; + $username = htmlspecialchars_decode($this->request->server($key)); + break; } } - return null; - } - - /** - * Get HTTP password from request headers - * - * @return string|null - */ - protected function get_http_password() - { - $password_keys = array( - 'PHP_AUTH_PW', - 'REMOTE_PASSWORD', - 'AUTH_PASSWORD', - ); - + $password = null; foreach ($password_keys as $key) { if ($this->request->is_set($key, request_interface::SERVER)) { - return html_entity_decode($this->request->server($key), ENT_COMPAT); + $password = htmlspecialchars_decode($this->request->server($key)); + break; } } - // Check if password is in Authorization header (Basic auth) - $username_keys = array( - 'PHP_AUTH_USER', - 'Authorization', - 'REMOTE_USER', - 'REDIRECT_REMOTE_USER', - 'HTTP_AUTHORIZATION', - 'REDIRECT_HTTP_AUTHORIZATION', - 'REMOTE_AUTHORIZATION', - 'REDIRECT_REMOTE_AUTHORIZATION', - 'AUTH_USER', - ); - - foreach ($username_keys as $key) + // Decode Basic authentication header if needed + if (!is_null($username) && is_null($password) && strpos($username, 'Basic ') === 0) { - if ($this->request->is_set($key, request_interface::SERVER)) - { - $auth_header = html_entity_decode($this->request->server($key), ENT_COMPAT); - - // Decode Basic authentication header - if (strpos($auth_header, 'Basic ') === 0) - { - $credentials = base64_decode(substr($auth_header, 6)); - if (strpos($credentials, ':') !== false) - { - list(, $password) = explode(':', $credentials, 2); - return $password; - } - } - } + [$username, $password] = explode(':', base64_decode(substr($username, 6)), 2); } - return null; + return [$username, $password]; } /** @@ -232,19 +193,18 @@ class http_auth_subscriber implements EventSubscriberInterface // Filter out non-ASCII characters per RFC2616 $realm = preg_replace('/[\x80-\xFF]/', '?', $realm); - $response = new Response('', Response::HTTP_UNAUTHORIZED); - $response->headers->set('WWW-Authenticate', 'Basic realm="' . $realm . '"'); + $response = new Response($this->language->lang('NOT_AUTHORISED'), Response::HTTP_UNAUTHORIZED); + $response->headers->set('WWW-Authenticate', 'Basic realm="' . $realm . ' - Feed"'); $event->setResponse($response); } /** * {@inheritdoc} */ - public static function getSubscribedEvents() + public static function getSubscribedEvents(): array { - return array( - // Priority should be high to run after session_begin but before controller - KernelEvents::REQUEST => array('on_kernel_request', 5), - ); + return [ + KernelEvents::REQUEST => ['on_kernel_request', 5], + ]; } } From 2bc90da6876ebf2d272117199a5c398355836e50 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 4 Oct 2025 13:21:06 +0200 Subject: [PATCH 6/9] [ticket/15085] Extend unit tests for http_auth_subscriber PHPBB-15085 --- .../phpbb/feed/event/http_auth_subscriber.php | 1 + tests/feed/http_auth_subscriber_test.php | 327 ++++++++++++++++-- 2 files changed, 303 insertions(+), 25 deletions(-) diff --git a/phpBB/phpbb/feed/event/http_auth_subscriber.php b/phpBB/phpbb/feed/event/http_auth_subscriber.php index b2937a3c22..14426f3aad 100644 --- a/phpBB/phpbb/feed/event/http_auth_subscriber.php +++ b/phpBB/phpbb/feed/event/http_auth_subscriber.php @@ -190,6 +190,7 @@ class http_auth_subscriber implements EventSubscriberInterface protected function send_auth_challenge(GetResponseEvent $event) { $realm = $this->config['sitename']; + // Filter out non-ASCII characters per RFC2616 $realm = preg_replace('/[\x80-\xFF]/', '?', $realm); diff --git a/tests/feed/http_auth_subscriber_test.php b/tests/feed/http_auth_subscriber_test.php index ee50ec9088..f25859c535 100644 --- a/tests/feed/http_auth_subscriber_test.php +++ b/tests/feed/http_auth_subscriber_test.php @@ -1,26 +1,33 @@ -* @license GNU General Public License, version 2 (GPL-2.0) -* -* For full copyright and license information, please see -* the docs/CREDITS.txt file. -* -*/ + * + * This file is part of the phpBB Forum Software package. + * + * @copyright (c) phpBB Limited + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ -namespace phpbb\feed\event; - -class http_auth_subscriber_test extends \phpbb_test_case +class phpbb_feed_http_auth_subscriber_test extends \phpbb_test_case { /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\auth\auth */ protected $auth; - /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\config\config */ + /** @var \PHPUnit\Framework\MockObject\MockObject|config */ protected $config; + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\language\language */ + protected $language; + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\request\request_interface */ protected $request; @@ -37,12 +44,26 @@ class http_auth_subscriber_test extends \phpbb_test_case $this->auth = $this->getMockBuilder('\phpbb\auth\auth') ->disableOriginalConstructor() ->getMock(); + $this->auth->method('login') + ->willReturnMap([ + ['valid_user', 'valid_password', false, true, false, ['status' => LOGIN_SUCCESS]], + ['invalid_user', 'invalid_password', false, true, false, ['status' => LOGIN_ERROR_USERNAME]], + ['attempts_user', 'valid_password', false, true, false, ['status' => LOGIN_ERROR_ATTEMPTS]], + ]); - $this->config = new \phpbb\config\config(array( + $this->config = new config(array( 'feed_http_auth' => 1, 'sitename' => 'Test Site', )); + $this->language = $this->getMockBuilder('\phpbb\language\language') + ->disableOriginalConstructor() + ->getMock(); + $this->language->method('lang') + ->willReturnMap([ + ['NOT_AUTHORISED', 'NOT_AUTHORISED'], + ]); + $this->request = $this->getMockBuilder('\phpbb\request\request_interface') ->getMock(); @@ -55,6 +76,7 @@ class http_auth_subscriber_test extends \phpbb_test_case $this->subscriber = new http_auth_subscriber( $this->auth, $this->config, + $this->language, $this->request, $this->user ); @@ -140,22 +162,18 @@ class http_auth_subscriber_test extends \phpbb_test_case ->disableOriginalConstructor() ->getMock(); - $request->attributes->expects($this->once()) - ->method('get') - ->with('_route') - ->willReturn('phpbb_feed_overall'); + $request->attributes->expects($this->never()) + ->method('get'); - $request->expects($this->once()) - ->method('isSecure') - ->willReturn(true); + $request->expects($this->never()) + ->method('isSecure'); $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') ->disableOriginalConstructor() ->getMock(); - $event->expects($this->once()) - ->method('getRequest') - ->willReturn($request); + $event->expects($this->never()) + ->method('getRequest'); $event->expects($this->never()) ->method('setResponse'); @@ -197,4 +215,263 @@ class http_auth_subscriber_test extends \phpbb_test_case $this->subscriber->on_kernel_request($event); } + + public function test_no_credentials() + { + $this->user->data = ['is_registered' => false]; + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(true); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + /** @var Response $response */ + $response = null; + $event->expects($this->once()) + ->method('setResponse') + ->with($this->isInstanceOf('\Symfony\Component\HttpFoundation\Response')) + ->will($this->returnCallback(function ($newResponse) use (&$response) { + $response = $newResponse; + })); + + $this->subscriber->on_kernel_request($event); + + $this->assertEquals(Response::HTTP_UNAUTHORIZED, $response->getStatusCode()); + $this->assertEquals('NOT_AUTHORISED', $response->getContent()); + $this->assertTrue($response->headers->has('WWW-Authenticate')); + } + + public function test_valid_credentials() + { + $this->user->data = ['is_registered' => false]; + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $this->request->method('is_set') + ->willReturnMap([ + ['PHP_AUTH_USER', request_interface::SERVER, true], + ['PHP_AUTH_PW', request_interface::SERVER, true], + ]); + + $this->request->method('server') + ->willReturnMap([ + ['PHP_AUTH_USER', '', 'valid_user'], + ['PHP_AUTH_PW', '', 'valid_password'], + ]); + + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(true); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + /** @var Response $response */ + $response = null; + $event->expects($this->never()) + ->method('setResponse'); + + $this->subscriber->on_kernel_request($event); + + $this->assertNull($response); + } + + public function test_valid_credentials_base64() + { + $this->user->data = ['is_registered' => false]; + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $this->request->method('is_set') + ->willReturnMap([ + ['Authorization', request_interface::SERVER, true], + ]); + + $this->request->method('server') + ->willReturnMap([ + ['Authorization', '', 'Basic dmFsaWRfdXNlcjp2YWxpZF9wYXNzd29yZA=='], + ]); + + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(true); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + /** @var Response $response */ + $response = null; + $event->expects($this->never()) + ->method('setResponse'); + + $this->subscriber->on_kernel_request($event); + + $this->assertNull($response); + } + + public function test_too_many_attempts() + { + $this->user->data = ['is_registered' => false]; + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $this->request->method('is_set') + ->willReturnMap([ + ['PHP_AUTH_USER', request_interface::SERVER, true], + ['PHP_AUTH_PW', request_interface::SERVER, true], + ]); + + $this->request->method('server') + ->willReturnMap([ + ['PHP_AUTH_USER', '', 'attempts_user'], + ['PHP_AUTH_PW', '', 'valid_password'], + ]); + + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(true); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + /** @var Response $response */ + $response = null; + $event->expects($this->once()) + ->method('setResponse') + ->with($this->isInstanceOf('\Symfony\Component\HttpFoundation\Response')) + ->will($this->returnCallback(function ($newResponse) use (&$response) { + $response = $newResponse; + })); + + $this->subscriber->on_kernel_request($event); + + $this->assertEquals(Response::HTTP_UNAUTHORIZED, $response->getStatusCode()); + $this->assertEquals('NOT_AUTHORISED', $response->getContent()); + $this->assertFalse($response->headers->has('WWW-Authenticate')); + } + + public function test_wrong_credentials() + { + $this->user->data = ['is_registered' => false]; + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->once()) + ->method('get') + ->with('_route') + ->willReturn('phpbb_feed_overall'); + + $this->request->method('is_set') + ->willReturnMap([ + ['PHP_AUTH_USER', request_interface::SERVER, true], + ['PHP_AUTH_PW', request_interface::SERVER, true], + ]); + + $this->request->method('server') + ->willReturnMap([ + ['PHP_AUTH_USER', '', 'invalid_user'], + ['PHP_AUTH_PW', '', 'invalid_password'], + ]); + + $request->expects($this->once()) + ->method('isSecure') + ->willReturn(true); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->once()) + ->method('getRequest') + ->willReturn($request); + + /** @var Response $response */ + $response = null; + $event->expects($this->once()) + ->method('setResponse') + ->with($this->isInstanceOf('\Symfony\Component\HttpFoundation\Response')) + ->will($this->returnCallback(function ($newResponse) use (&$response) { + $response = $newResponse; + })); + + $this->subscriber->on_kernel_request($event); + + $this->assertEquals(Response::HTTP_UNAUTHORIZED, $response->getStatusCode()); + $this->assertEquals('NOT_AUTHORISED', $response->getContent()); + $this->assertTrue($response->headers->has('WWW-Authenticate')); + } } From 8ef5c5549db69abe0f06f967fb1aed17c67f867f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 4 Oct 2025 13:27:54 +0200 Subject: [PATCH 7/9] [ticket/15085] Add info that HTTP auth is only possible via https PHPBB-15085 --- phpBB/language/en/acp/board.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/language/en/acp/board.php b/phpBB/language/en/acp/board.php index e99632e442..bd0e8599de 100644 --- a/phpBB/language/en/acp/board.php +++ b/phpBB/language/en/acp/board.php @@ -298,7 +298,7 @@ $lang = array_merge($lang, array( 'ACP_FEED_OVERALL_FORUMS_EXPLAIN' => 'Enables the “All forums” feed, which displays a list of forums.', 'ACP_FEED_HTTP_AUTH' => 'Allow HTTP Authentication', - 'ACP_FEED_HTTP_AUTH_EXPLAIN' => 'Enables HTTP authentication, which allows users to receive content that is hidden to guest users by adding the auth=http parameter to the feed URL. Please note that some PHP setups require additional changes to the .htaccess file. Instructions can be found in that file.', + 'ACP_FEED_HTTP_AUTH_EXPLAIN' => 'Enables HTTP authentication, allowing users to access content hidden from guests by adding the auth=http parameter to the feed URL. Please note that some PHP configurations may require additional changes to the .htaccess file; refer to that file for guidance. HTTP authentication is only supported over encrypted (https) connections.', 'ACP_FEED_ITEM_STATISTICS' => 'Item statistics', 'ACP_FEED_ITEM_STATISTICS_EXPLAIN' => 'Display individual statistics underneath feed items
(e.g. posted by, date and time, replies, views)', 'ACP_FEED_EXCLUDE_ID' => 'Exclude these forums', From 033fb28fdc3bc707eee04fab99808612e7db46ca Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 4 Oct 2025 13:36:27 +0200 Subject: [PATCH 8/9] [ticket/15085] Improve commit message check hook output PHPBB-15085 --- git-tools/hooks/commit-msg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-tools/hooks/commit-msg b/git-tools/hooks/commit-msg index 6405d5b7c8..95b8e313ed 100755 --- a/git-tools/hooks/commit-msg +++ b/git-tools/hooks/commit-msg @@ -337,7 +337,7 @@ done # If EOF is expected exit cleanly echo "$expecting" | grep -q "eof" || ( # Unexpected EOF, error - complain "Unexpected EOF encountered" >&2; + complain "Expected to see footer (e.g. PHPBB-12345) or description line, but reached end of file." >&2; quit $ERR_EOF; ) && ( # Do post scan checks From a9b62f338a5a8bc15038d8962c1d97a70f8a1cb0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 7 Oct 2025 19:44:01 +0200 Subject: [PATCH 9/9] [ticket/15085] Output login error with attempts info in http auth PHPBB-15085 --- phpBB/phpbb/feed/event/http_auth_subscriber.php | 2 +- tests/feed/http_auth_subscriber_test.php | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/phpBB/phpbb/feed/event/http_auth_subscriber.php b/phpBB/phpbb/feed/event/http_auth_subscriber.php index 14426f3aad..4ff411666e 100644 --- a/phpBB/phpbb/feed/event/http_auth_subscriber.php +++ b/phpBB/phpbb/feed/event/http_auth_subscriber.php @@ -118,7 +118,7 @@ class http_auth_subscriber implements EventSubscriberInterface else if ($auth_result['status'] == LOGIN_ERROR_ATTEMPTS) { // Too many login attempts - $response = new Response($this->language->lang('NOT_AUTHORISED'), Response::HTTP_UNAUTHORIZED); + $response = new Response($this->language->lang('LOGIN_ERROR_ATTEMPTS'), Response::HTTP_UNAUTHORIZED); $event->setResponse($response); return; } diff --git a/tests/feed/http_auth_subscriber_test.php b/tests/feed/http_auth_subscriber_test.php index f25859c535..5fede1572b 100644 --- a/tests/feed/http_auth_subscriber_test.php +++ b/tests/feed/http_auth_subscriber_test.php @@ -62,6 +62,7 @@ class phpbb_feed_http_auth_subscriber_test extends \phpbb_test_case $this->language->method('lang') ->willReturnMap([ ['NOT_AUTHORISED', 'NOT_AUTHORISED'], + ['LOGIN_ERROR_ATTEMPTS', 'LOGIN_ERROR_ATTEMPTS'] ]); $this->request = $this->getMockBuilder('\phpbb\request\request_interface') @@ -414,7 +415,7 @@ class phpbb_feed_http_auth_subscriber_test extends \phpbb_test_case $this->subscriber->on_kernel_request($event); $this->assertEquals(Response::HTTP_UNAUTHORIZED, $response->getStatusCode()); - $this->assertEquals('NOT_AUTHORISED', $response->getContent()); + $this->assertEquals('LOGIN_ERROR_ATTEMPTS', $response->getContent()); $this->assertFalse($response->headers->has('WWW-Authenticate')); }