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/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 diff --git a/phpBB/config/default/container/services_feed.yml b/phpBB/config/default/container/services_feed.yml index f32d0cb4d3..8a46895905 100644 --- a/phpBB/config/default/container/services_feed.yml +++ b/phpBB/config/default/container/services_feed.yml @@ -15,6 +15,17 @@ services: - '@language' - '%core.php_ext%' + feed.http_auth_subscriber: + class: phpbb\feed\event\http_auth_subscriber + arguments: + - '@auth' + - '@config' + - '@language' + - '@request' + - '@user' + tags: + - { name: kernel.event_subscriber } + feed.helper: class: phpbb\feed\helper arguments: 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', 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..4ff411666e --- /dev/null +++ b/phpBB/phpbb/feed/event/http_auth_subscriber.php @@ -0,0 +1,211 @@ + +* @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\language\language; +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 language */ + protected $language; + + /** @var request_interface */ + protected $request; + + /** @var user */ + protected $user; + + /** + * Constructor + * + * @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, language $language, request_interface $request, user $user) + { + $this->auth = $auth; + $this->config = $config; + $this->language = $language; + $this->request = $request; + $this->user = $user; + } + + /** + * Handle HTTP authentication for feed routes + * + * @param GetResponseEvent $event + * @return void + */ + 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'); + + // Only apply to feed routes + if (strpos($route, 'phpbb_feed_') !== 0) + { + return; + } + + // Only allow HTTP authentication in secure context (HTTPS) + if (!$request->isSecure()) + { + return; + } + + // User is already logged in, no need to authenticate + if (!empty($this->user->data['is_registered'])) + { + return; + } + + // Get HTTP authentication credentials + [$username, $password] = $this->get_credentials(); + + // 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) + { + // Reload ACL for the newly logged-in user + $this->auth->acl($this->user->data); + return; + } + else if ($auth_result['status'] == LOGIN_ERROR_ATTEMPTS) + { + // Too many login attempts + $response = new Response($this->language->lang('LOGIN_ERROR_ATTEMPTS'), Response::HTTP_UNAUTHORIZED); + $event->setResponse($response); + return; + } + + // Authentication failed, send challenge + $this->send_auth_challenge($event); + } + + /** + * Retrieve HTTP authentication credentials from server variables + * + * @return array [username, password] Array containing the username and password, or null if not found + */ + protected function get_credentials(): array + { + $username_keys = [ + 'PHP_AUTH_USER', + 'Authorization', + 'REMOTE_USER', + 'REDIRECT_REMOTE_USER', + 'HTTP_AUTHORIZATION', + 'REDIRECT_HTTP_AUTHORIZATION', + '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 = htmlspecialchars_decode($this->request->server($key)); + break; + } + } + + $password = null; + foreach ($password_keys as $key) + { + if ($this->request->is_set($key, request_interface::SERVER)) + { + $password = htmlspecialchars_decode($this->request->server($key)); + break; + } + } + + // Decode Basic authentication header if needed + if (!is_null($username) && is_null($password) && strpos($username, 'Basic ') === 0) + { + [$username, $password] = explode(':', base64_decode(substr($username, 6)), 2); + } + + return [$username, $password]; + } + + /** + * 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($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(): array + { + return [ + KernelEvents::REQUEST => ['on_kernel_request', 5], + ]; + } +} diff --git a/tests/feed/http_auth_subscriber_test.php b/tests/feed/http_auth_subscriber_test.php new file mode 100644 index 0000000000..5fede1572b --- /dev/null +++ b/tests/feed/http_auth_subscriber_test.php @@ -0,0 +1,478 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +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|config */ + protected $config; + + /** @var \PHPUnit\Framework\MockObject\MockObject|\phpbb\language\language */ + protected $language; + + /** @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->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 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'], + ['LOGIN_ERROR_ATTEMPTS', 'LOGIN_ERROR_ATTEMPTS'] + ]); + + $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->language, + $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_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; + + $request = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Request') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes = $this->getMockBuilder('\Symfony\Component\HttpFoundation\ParameterBag') + ->disableOriginalConstructor() + ->getMock(); + + $request->attributes->expects($this->never()) + ->method('get'); + + $request->expects($this->never()) + ->method('isSecure'); + + $event = $this->getMockBuilder('\Symfony\Component\HttpKernel\Event\GetResponseEvent') + ->disableOriginalConstructor() + ->getMock(); + + $event->expects($this->never()) + ->method('getRequest'); + + $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'); + + $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); + + $event->expects($this->never()) + ->method('setResponse'); + + $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('LOGIN_ERROR_ATTEMPTS', $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')); + } +}