diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f64671c..e73a288 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,26 @@ permissions: contents: read jobs: + lint: + name: Manifest, language and package checks + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: shivammathur/setup-php@v2 + with: + php-version: '8.4' + coverage: none + - name: XML files are well-formed + run: | + sudo apt-get install -y -q libxml2-utils > /dev/null + find . -name '*.xml' -not -path './.git/*' -print0 | xargs -0 xmllint --noout + - name: Language files + run: php tests/lint/check-language.php bfstop.xml PLG_SYSTEM_BFSTOP + - name: Release zip contains everything the manifest references + run: | + ./deploy.sh zip + php tests/lint/check-zip.php bfstop-*.zip bfstop.xml + unit: name: Syntax check and unit tests (PHP ${{ matrix.php }}) runs-on: ubuntu-latest diff --git a/deploy.sh b/deploy.sh index a9bad45..2fec4ff 100755 --- a/deploy.sh +++ b/deploy.sh @@ -10,7 +10,7 @@ dstdir= # internal variables to be updated when files are added: extname=bfstop sqlfiles="sql" -srcfiles="$extname.php helpers $extname.xml $sqlfiles updatescript.php index.html src services" +srcfiles="$extname.xml $sqlfiles updatescript.php index.html src services" langfiles="language" docs="CHANGELOG LICENSE.txt README" plgtype="system" diff --git a/src/Extension/Bfstop.php b/src/Extension/Bfstop.php index 52e3662..20255c2 100644 --- a/src/Extension/Bfstop.php +++ b/src/Extension/Bfstop.php @@ -470,11 +470,12 @@ private function isPasswordRecoveryRequest() /** * Detects a request that actually submits login credentials, for - * "Login Only" block mode (issue #187): Joomla routes credential - * submission through com_users on both the frontend (task=user.login) - * and the backend (task=login, e.g. the entry_url Joomla itself builds - * for the admin login form) - merely viewing the login form (no task, - * or a display task) doesn't match, so it stays reachable. + * "Login Only" block mode (issue #187): on the frontend, credentials + * are submitted to com_users (task=user.login), the backend login form + * posts to com_login (task=login); com_users with task=login is kept + * for backend entry URLs built that way - merely viewing the login + * form (no task, or a display task) doesn't match, so it stays + * reachable. */ private function isLoginAttemptRequest() { @@ -482,7 +483,8 @@ private function isLoginAttemptRequest() $option = $input->getCmd('option', ''); $task = $input->getCmd('task', ''); $result = (strcmp($option, 'com_users') == 0 && - (strcmp($task, 'user.login') == 0 || strcmp($task, 'login') == 0)); + (strcmp($task, 'user.login') == 0 || strcmp($task, 'login') == 0)) || + (strcmp($option, 'com_login') == 0 && strcmp($task, 'login') == 0); if ($result) { $this->logger->log('Detected a login-attempt request (task='.$task.')', Log::DEBUG); diff --git a/tests/Integration/BlockedRequestTest.php b/tests/Integration/BlockedRequestTest.php new file mode 100644 index 0000000..a142dfb --- /dev/null +++ b/tests/Integration/BlockedRequestTest.php @@ -0,0 +1,164 @@ +setQuery("SELECT params FROM #__extensions WHERE type='plugin' AND element='bfstop'"); + self::$originalParams = $db->loadResult(); + } + + public static function tearDownAfterClass(): void + { + if (self::$originalParams !== null) + { + $db = Factory::getDbo(); + $db->setQuery('UPDATE #__extensions SET params='.$db->quote(self::$originalParams). + " WHERE type='plugin' AND element='bfstop'"); + $db->execute(); + } + } + + private function configure(array $params = array()) + { + $this->setPluginParams(json_encode($params + array( + 'blockMode' => 'full', + 'blockedMessage' => self::BlockedMessage, + 'logLevel' => 8, // errors only + ))); + } + + private function block($ip = self::Ip, $minutesAgo = 0, $duration = 60) + { + return $this->insert('#__bfstop_bannedip', array('ipaddress' => $ip, + 'crdate' => self::minutesAgo($minutesAgo), 'duration' => $duration), 'id'); + } + + private function request($query = '', $ip = self::Ip) + { + $command = escapeshellarg(PHP_BINARY).' '.escapeshellarg(__DIR__.'/fixtures/request.php').' '. + escapeshellarg($ip).' '.escapeshellarg($query).' 2>&1'; + exec($command, $output, $exitCode); + $output = implode("\n", $output); + $this->assertSame(0, $exitCode, $output); + return $output; + } + + private function assertBlocked($query = '', $ip = self::Ip) + { + $output = $this->request($query, $ip); + $this->assertStringContainsString(self::BlockedMessage, $output, "request '$query' should be blocked"); + $this->assertStringNotContainsString('NOT BLOCKED', $output); + } + + private function assertNotBlocked($query = '', $ip = self::Ip) + { + $output = $this->request($query, $ip); + $this->assertStringContainsString('NOT BLOCKED', $output, "request '$query' should not be blocked"); + $this->assertStringNotContainsString(self::BlockedMessage, $output); + } + + public function testFullModeBlocksEverything() + { + $this->configure(); + $this->block(); + $this->assertBlocked(); + $this->assertBlocked('option=com_content&view=article&id=1'); + $this->assertBlocked('option=com_users&task=user.login'); + $this->assertNotBlocked('', '203.0.113.42'); + } + + public function testBlockedMessageCanShowIp() + { + $this->configure(array('blockedMsgShowIP' => 1)); + $this->block(); + $this->assertStringContainsString(self::Ip, $this->request()); + } + + public function testExpiredBlockNoLongerApplies() + { + $this->configure(); + $this->block(self::Ip, 61, 60); + $this->assertNotBlocked(); + } + + public function testPermanentBlockNeverExpires() + { + $this->configure(); + // duration 0 = "forever"; 5 years is still within DatabaseHelper::$UNLIMITED_DURATION + $this->block(self::Ip, 5 * 365 * 24 * 60, 0); + $this->assertBlocked(); + } + + public function testLoginOnlyModeOnlyRejectsLoginAttempts() + { + $this->configure(array('blockMode' => 'loginonly')); + $this->block(); + $this->assertNotBlocked(); + $this->assertNotBlocked('option=com_users&view=login'); + // frontend login form + $this->assertBlocked('option=com_users&task=user.login'); + // backend login form + $this->assertBlocked('option=com_login&task=login'); + } + + public function testPasswordRecoveryStaysReachable() + { + // a blocked legitimate user must still be able to reset their password + $this->configure(); + $this->block(); + $this->assertNotBlocked('option=com_users&view=reset'); + $this->assertNotBlocked('option=com_users&view=remind'); + $this->assertBlocked('option=com_users&view=login'); + } + + public function testValidUnblockTokenGetsThrough() + { + $this->configure(); + $blockId = $this->block(); + $token = (new DatabaseHelper($this->logger))->getNewUnblockToken($blockId, str_repeat('ab', 20)); + $this->assertNotBlocked('option=com_bfstop&view=tokenunblock&token='.$token); + $this->assertBlocked('option=com_bfstop&view=tokenunblock&token='.str_repeat('0', 40)); + $this->assertBlocked('option=com_bfstop&view=tokenunblock'); + } + + public function testRejectedRequestsAreCounted() + { + $this->configure(); + $blockId = $this->block(); + $this->assertBlocked(); + $this->assertBlocked(); + $this->assertSame(2, (int) $this->queryValue('SELECT attempts FROM #__bfstop_bannedip WHERE id='.$blockId)); + $this->assertNotNull($this->queryValue('SELECT last_attempt FROM #__bfstop_bannedip WHERE id='.$blockId)); + } + + public function testUnblockedBlockNoLongerApplies() + { + $this->configure(); + $blockId = $this->block(); + $this->insert('#__bfstop_unblock', array('block_id' => $blockId, 'source' => 0, 'crdate' => self::minutesAgo(0))); + $this->assertNotBlocked(); + } +} diff --git a/tests/Integration/GeoHelperTest.php b/tests/Integration/GeoHelperTest.php new file mode 100644 index 0000000..1901075 --- /dev/null +++ b/tests/Integration/GeoHelperTest.php @@ -0,0 +1,46 @@ +logger; + $this->assertNull(GeoHelper::getCountryCode($logger, '', '203.0.113.5')); + $this->assertNull(GeoHelper::getCityDetails($logger, '', '203.0.113.5')); + $this->assertCount(0, $logger->messages); + } + + public function testUnreadableDbPathReturnsNullWithWarning() + { + $logger = $this->logger; + $this->assertNull(GeoHelper::getCountryCode($logger, '/nonexistent/GeoLite2-Country.mmdb', '203.0.113.5')); + $this->assertTrue($logger->hasMessage(Log::WARNING, 'not readable')); + } + + public function testCorruptDbReturnsNullWithWarning() + { + $file = tempnam(sys_get_temp_dir(), 'bfstop-geo'); + file_put_contents($file, str_repeat('not a maxmind database', 100)); + try + { + $logger = $this->logger; + $this->assertNull(GeoHelper::getCountryCode($logger, $file, '203.0.113.5')); + $this->assertTrue($logger->hasMessage(Log::WARNING, 'lookup failed')); + } + finally + { + unlink($file); + } + } +} diff --git a/tests/Integration/HtaccessHelperTest.php b/tests/Integration/HtaccessHelperTest.php new file mode 100644 index 0000000..17b10ef --- /dev/null +++ b/tests/Integration/HtaccessHelperTest.php @@ -0,0 +1,162 @@ +dir = sys_get_temp_dir().'/bfstop-htaccess-'.bin2hex(random_bytes(4)); + mkdir($this->dir); + } + + protected function tearDown(): void + { + @chmod($this->dir.'/.htaccess', 0644); + @unlink($this->dir.'/.htaccess'); + @rmdir($this->dir); + } + + private function helper($logger = null) + { + return new HtaccessHelper($this->dir, $logger ?? $this->logger); + } + + private function content() + { + return file_get_contents($this->dir.'/.htaccess'); + } + + public function testFileName() + { + $this->assertSame($this->dir.'/.htaccess', $this->helper()->getFileName()); + } + + public function testCheckRequirements() + { + $_SERVER['SERVER_SOFTWARE'] = 'Apache/2.4.58 (Ubuntu)'; + $req = $this->helper()->checkRequirements(); + $this->assertNotFalse($req['apacheserver']); + $this->assertFalse($req['found']); + + touch($this->dir.'/.htaccess'); + $_SERVER['SERVER_SOFTWARE'] = 'nginx/1.25'; + $req = $this->helper()->checkRequirements(); + $this->assertFalse($req['apacheserver']); + $this->assertTrue($req['found']); + $this->assertTrue($req['readable']); + $this->assertTrue($req['writeable']); + } + + public function testNoFileMeansNoDeniedIPs() + { + $this->assertSame(array(), $this->helper()->getDeniedIPs()); + } + + public function testDenyCreatesFileWithBlock() + { + $h = $this->helper(); + $this->assertNotFalse($h->denyIP('203.0.113.5')); + $this->assertSame(array('203.0.113.5'), array_values($h->getDeniedIPs())); + $this->assertSame( + "# BEGIN BFStop Blocks\n\nRequire all granted\nRequire not ip 203.0.113.5\n\n# END BFStop Blocks\n\n", + $this->content()); + } + + public function testDenyIsIdempotentAndUndenyRemoves() + { + $h = $this->helper(); + $h->denyIP('203.0.113.5'); + $h->denyIP('2001:db8::/32'); + $h->denyIP('203.0.113.5'); + $this->assertSame(array('203.0.113.5', '2001:db8::/32'), array_values($h->getDeniedIPs())); + + $this->assertNotFalse($h->undenyIP('203.0.113.5')); + $this->assertSame(array('2001:db8::/32'), array_values($h->getDeniedIPs())); + + // removing something which isn't there is not an error + $this->assertTrue($h->undenyIP('198.51.100.1')); + } + + public function testExistingContentIsPreserved() + { + $existing = "RewriteEngine On\nRewriteRule ^foo$ bar [L]\n"; + file_put_contents($this->dir.'/.htaccess', $existing); + $h = $this->helper(); + $h->denyIP('203.0.113.5'); + $h->denyIP('203.0.113.6'); + $h->undenyIP('203.0.113.5'); + $content = $this->content(); + $this->assertStringEndsWith($existing, $content); + $this->assertStringStartsWith("# BEGIN BFStop Blocks\n", $content); + $this->assertSame(1, substr_count($content, '# BEGIN BFStop Blocks')); + $this->assertSame(array('203.0.113.6'), array_values($h->getDeniedIPs())); + } + + public function testBlockInMiddleOfFileIsUpdatedInPlace() + { + file_put_contents($this->dir.'/.htaccess', + "# top\n# BEGIN BFStop Blocks\n\nRequire all granted\nRequire not ip 192.0.2.1\n\n# END BFStop Blocks\n# bottom\n"); + $h = $this->helper(); + $this->assertSame(array('192.0.2.1'), array_values($h->getDeniedIPs())); + $h->denyIP('192.0.2.2'); + $content = $this->content(); + $this->assertStringStartsWith("# top\n# BEGIN BFStop Blocks\n", $content); + $this->assertStringEndsWith("# END BFStop Blocks\n# bottom\n", $content); + $this->assertSame(array('192.0.2.1', '192.0.2.2'), array_values($h->getDeniedIPs())); + } + + public function test403Message() + { + $h = $this->helper(); + $h->denyIP('203.0.113.5'); + $this->assertNotFalse($h->edit403Message('Go away')); + $this->assertStringContainsString("ErrorDocument 403 \"Go away\"\n", $this->content()); + $this->assertSame(array('203.0.113.5'), array_values($h->getDeniedIPs())); + + $h->edit403Message('Changed'); + $this->assertStringNotContainsString('Go away', $this->content()); + $this->assertSame(1, substr_count($this->content(), 'ErrorDocument 403')); + + $h->edit403Message(''); + $this->assertStringNotContainsString('ErrorDocument 403', $this->content()); + $this->assertSame(array('203.0.113.5'), array_values($h->getDeniedIPs())); + } + + public function testCorruptMarkersAreNotOverwritten() + { + $corrupt = "# BEGIN BFStop Blocks\nRequire not ip 192.0.2.1\n"; + file_put_contents($this->dir.'/.htaccess', $corrupt); + $logger = $this->logger; + $this->assertFalse($this->helper($logger)->denyIP('203.0.113.5')); + $this->assertSame($corrupt, $this->content()); + $this->assertTrue($logger->hasMessage(Log::ERROR, 'END')); + $logger->errors = array(); // expected + } + + public function testReadOnlyFileIsReported() + { + if (function_exists('posix_geteuid') && posix_geteuid() === 0) + { + $this->markTestSkipped('root can write read-only files'); + } + file_put_contents($this->dir.'/.htaccess', "# existing\n"); + chmod($this->dir.'/.htaccess', 0444); + $logger = $this->logger; + $this->assertFalse($this->helper($logger)->denyIP('203.0.113.5')); + $this->assertTrue($logger->hasMessage(Log::ERROR, 'not writable')); + $logger->errors = array(); // expected + } +} diff --git a/tests/Integration/IntegrationTestCase.php b/tests/Integration/IntegrationTestCase.php index 8a66e23..491b05c 100644 --- a/tests/Integration/IntegrationTestCase.php +++ b/tests/Integration/IntegrationTestCase.php @@ -21,6 +21,8 @@ class RecordingLogger extends LoggerHelper { public $errors = array(); + /** everything logged, as array('message' => ..., 'priority' => ...) */ + public $messages = array(); public function __construct() { @@ -32,8 +34,21 @@ public function isEnabled($priority = Log::ERROR) return true; } + public function hasMessage($priority, $substring = '') + { + foreach ($this->messages as $m) + { + if ($m['priority'] === $priority && ($substring === '' || str_contains($m['message'], $substring))) + { + return true; + } + } + return false; + } + public function log($msg, $priority) { + $this->messages[] = array('message' => $msg, 'priority' => $priority); if ($priority <= Log::ERROR) { $this->errors[] = $msg; diff --git a/tests/Integration/IpHelperAddressTest.php b/tests/Integration/IpHelperAddressTest.php new file mode 100644 index 0000000..3ffc441 --- /dev/null +++ b/tests/Integration/IpHelperAddressTest.php @@ -0,0 +1,132 @@ +setQuery("SELECT params FROM #__extensions WHERE type='plugin' AND element='bfstop'"); + self::$originalParams = $db->loadResult(); + } + + public static function tearDownAfterClass(): void + { + if (self::$originalParams !== null) + { + $db = \Joomla\CMS\Factory::getDbo(); + $db->setQuery('UPDATE #__extensions SET params='.$db->quote(self::$originalParams). + " WHERE type='plugin' AND element='bfstop'"); + $db->execute(); + self::forgetPlugins(); + } + } + + private static function forgetPlugins() + { + (new \ReflectionProperty(PluginHelper::class, 'plugins'))->setValue(null, null); + } + + protected function setUp(): void + { + parent::setUp(); + $this->serverBackup = $_SERVER; + $_SERVER['REMOTE_ADDR'] = '198.51.100.7'; + foreach (IpHelper::KnownProxyHeaders as $header) + { + unset($_SERVER[$header]); + } + } + + protected function tearDown(): void + { + $_SERVER = $this->serverBackup; + } + + private function configure(array $params) + { + $this->setPluginParams(json_encode($params)); + self::forgetPlugins(); + } + + public function testProxyDisabledIgnoresHeader() + { + $this->configure(array('useProxy' => 0, 'proxyIpAddress' => '198.51.100.7')); + $_SERVER['HTTP_X_FORWARDED_FOR'] = '203.0.113.5'; + $this->assertSame('198.51.100.7', IpHelper::getAddress($this->logger)); + } + + public function testProxyEnabledButRequestNotFromProxyIgnoresHeader() + { + $this->configure(array('useProxy' => 1, 'proxyIpAddress' => '192.0.2.1')); + $_SERVER['HTTP_X_FORWARDED_FOR'] = '203.0.113.5'; + $this->assertSame('198.51.100.7', IpHelper::getAddress($this->logger)); + $this->assertTrue($this->logger->hasMessage(Log::WARNING, 'did not originate from the configured proxy')); + } + + public function testProxyEnabledWithoutConfiguredProxyIpIgnoresHeader() + { + $this->configure(array('useProxy' => 1, 'proxyIpAddress' => '')); + $_SERVER['HTTP_X_FORWARDED_FOR'] = '203.0.113.5'; + $this->assertSame('198.51.100.7', IpHelper::getAddress($this->logger)); + $this->assertTrue($this->logger->hasMessage(Log::WARNING)); + } + + public static function trustedProxyHeaderProvider() + { + return array( + 'single public IP' => array('203.0.113.5', '203.0.113.5'), + 'first public in list' => array('203.0.113.5, 198.51.100.9', '203.0.113.5'), + 'skips private' => array('10.0.0.1, 192.168.1.1, 203.0.113.5', '203.0.113.5'), + 'skips loopback/reserved' => array('127.0.0.1,0.0.0.0, 203.0.113.5', '203.0.113.5'), + 'whitespace is trimmed' => array(' 203.0.113.5 ', '203.0.113.5'), + 'garbage is skipped' => array('not-an-ip, 203.0.113.5', '203.0.113.5'), + 'public IPv6' => array('fd00::1, 2001:4860:4860::8888', '2001:4860:4860::8888'), + ); + } + + #[DataProvider('trustedProxyHeaderProvider')] + public function testTrustedProxyUsesFirstPublicIpFromHeader($header, $expected) + { + $this->configure(array('useProxy' => 1, 'proxyIpAddress' => '198.51.100.7')); + $_SERVER['HTTP_X_FORWARDED_FOR'] = $header; + $this->assertSame($expected, IpHelper::getAddress($this->logger)); + } + + public function testTrustedProxyWithOnlyPrivateAddressesFallsBack() + { + $this->configure(array('useProxy' => 1, 'proxyIpAddress' => '198.51.100.7')); + $_SERVER['HTTP_X_FORWARDED_FOR'] = '10.1.2.3, 172.16.0.1'; + $this->assertSame('198.51.100.7', IpHelper::getAddress($this->logger)); + $this->assertTrue($this->logger->hasMessage(Log::WARNING, 'falling back to REMOTE_ADDR')); + } + + public function testConfiguredHeaderSourceIsHonoured() + { + $this->configure(array('useProxy' => 1, 'proxyIpAddress' => '198.51.100.7', 'proxyHeaderSource' => 'HTTP_CLIENT_IP')); + $_SERVER['HTTP_X_FORWARDED_FOR'] = '203.0.113.5'; + $_SERVER['HTTP_CLIENT_IP'] = '203.0.113.77'; + $this->assertSame('203.0.113.77', IpHelper::getAddress($this->logger)); + } +} diff --git a/tests/Integration/IpValidateHelperTest.php b/tests/Integration/IpValidateHelperTest.php new file mode 100644 index 0000000..9b6ae64 --- /dev/null +++ b/tests/Integration/IpValidateHelperTest.php @@ -0,0 +1,125 @@ +messages(); // start with an empty message queue + } + + /** returns and clears the application's message queue */ + private function messages() + { + return Factory::getApplication()->getMessageQueue(true); + } + + public static function cidrMatchProvider() + { + return array( + 'v4 exact, no prefix' => array('192.0.2.1', '192.0.2.1', true), + 'v4 exact, no prefix, other' => array('192.0.2.2', '192.0.2.1', false), + 'v4 /32' => array('192.0.2.1', '192.0.2.1/32', true), + 'v4 /24 inside' => array('192.0.2.200', '192.0.2.0/24', true), + 'v4 /24 outside' => array('192.0.3.1', '192.0.2.0/24', false), + 'v4 /24 misaligned subnet' => array('192.0.2.200', '192.0.2.99/24', true), + 'v4 /0 matches everything' => array('203.0.113.9', '0.0.0.0/0', true), + 'v6 exact, no prefix' => array('2001:db8::1', '2001:db8::1', true), + 'v6 exact, other' => array('2001:db8::2', '2001:db8::1', false), + 'v6 /64 inside' => array('2001:db8:0:1:abcd::1', '2001:db8:0:1::/64', true), + 'v6 /64 outside' => array('2001:db8:0:2::1', '2001:db8:0:1::/64', false), + 'v6 /64 misaligned subnet' => array('2001:db8:0:1::5', '2001:db8:0:1:ffff::/64', true), + 'v6 /33 odd bits inside' => array('2001:db8:8000::1', '2001:db8:ffff::/33', true), + 'v6 /33 odd bits outside' => array('2001:db8:7fff::1', '2001:db8:ffff::/33', false), + 'v4 address vs v6 range' => array('192.0.2.1', '2001:db8::/32', false), + 'invalid address vs v6' => array('garbage', '2001:db8::/32', false), + ); + } + + #[DataProvider('cidrMatchProvider')] + public function testCidrMatch($ip, $range, $expected) + { + $this->assertSame($expected, IpValidateHelper::cidrMatch($ip, $range)); + } + + public static function validRangeProvider() + { + return array( + array('203.0.113.5'), + array('203.0.113.0/24'), + array('203.0.113.0/0'), + array('203.0.113.0/32'), + array('2001:4860:4860::8888'), + array('2001:4860::/32'), + array('2001:4860::/128'), + ); + } + + #[DataProvider('validRangeProvider')] + public function testValidPublicRange($range) + { + $this->assertTrue(IpValidateHelper::validIPRange($range)); + $this->assertSame(array(), $this->messages()); + } + + public static function invalidRangeProvider() + { + return array( + 'v4 subnet too large' => array('203.0.113.0/33', 'COM_BFSTOP_IP_INVALID_SUBNET', '33'), + 'v6 subnet too large' => array('2001:4860::/129', 'COM_BFSTOP_IP_INVALID_SUBNET', '129'), + 'negative subnet' => array('203.0.113.0/-1', 'COM_BFSTOP_IP_INVALID_SUBNET', '-1'), + 'non-numeric subnet' => array('203.0.113.0/abc', 'COM_BFSTOP_IP_INVALID_SUBNET', 'abc'), + 'empty subnet' => array('203.0.113.0/', 'COM_BFSTOP_IP_INVALID_SUBNET', ''), + 'not an address' => array('foo.bar', 'COM_BFSTOP_IP_INVALID_ADDRESS', 'foo.bar'), + 'octet out of range' => array('203.0.113.256', 'COM_BFSTOP_IP_INVALID_ADDRESS', '203.0.113.256'), + 'empty' => array('', 'COM_BFSTOP_IP_INVALID_ADDRESS', ''), + 'invalid with subnet' => array('1.2.3/24', 'COM_BFSTOP_IP_INVALID_ADDRESS', '1.2.3'), + ); + } + + #[DataProvider('invalidRangeProvider')] + public function testInvalidRange($range, $expectedKey, $expectedArg) + { + $this->assertFalse(IpValidateHelper::validIPRange($range)); + $this->assertSame(array(array('message' => Text::sprintf($expectedKey, $expectedArg), 'type' => 'warning')), $this->messages()); + } + + public static function privateRangeProvider() + { + return array(array('10.0.0.1'), array('192.168.0.0/16'), array('127.0.0.1'), array('fd00::1'), array('::1')); + } + + #[DataProvider('privateRangeProvider')] + public function testPrivateOrReservedRangeIsAcceptedWithWarning($range) + { + $this->assertTrue(IpValidateHelper::validIPRange($range)); + $ip = explode('/', $range)[0]; + $this->assertSame(array(array('message' => Text::sprintf('COM_BFSTOP_IP_PRIVATE_OR_RESERVED', $ip), 'type' => 'warning')), $this->messages()); + } +} diff --git a/tests/Integration/RiskHelperTest.php b/tests/Integration/RiskHelperTest.php new file mode 100644 index 0000000..2f43117 --- /dev/null +++ b/tests/Integration/RiskHelperTest.php @@ -0,0 +1,157 @@ +serverBackup = $_SERVER; + $_SERVER['HTTP_USER_AGENT'] = 'Mozilla/5.0 (X11; Linux x86_64)'; + } + + protected function tearDown(): void + { + $_SERVER = $this->serverBackup; + } + + /** + * All signals which are on by default switched off, so that each test + * can enable exactly the one it is interested in. + */ + private static function allOff(array $overrides = array()) + { + return new Registry(array_merge(array( + 'riskKnownIpEnabled' => 0, + 'riskCommonUsernameEnabled' => 0, + 'riskUserAgentEnabled' => 0, + 'riskGeoEnabled' => 0, + 'riskReverseDnsEnabled' => 0, + ), $overrides)); + } + + private function db($knownIp = false) + { + $db = $this->createMock(DatabaseHelper::class); + $db->method('isKnownIpUsername')->willReturn($knownIp); + return $db; + } + + public function testAllSignalsDisabledScoresZero() + { + $db = $this->createMock(DatabaseHelper::class); + $db->expects($this->never())->method('isKnownIpUsername'); + $db->expects($this->never())->method('getCachedHostname'); + $_SERVER['HTTP_USER_AGENT'] = ''; + $this->assertSame(0, RiskHelper::computeScore($db, $this->logger, self::allOff(), '203.0.113.5', 'admin')); + } + + public function testDefaultsWithTrustworthyRequestScoreZero() + { + // defaults: known-IP, common-username and user-agent signals on; geo and rDNS off + $params = new Registry(array('riskCommonUsernames' => "admin\nroot")); + $this->assertSame(0, RiskHelper::computeScore($this->db(false), $this->logger, $params, '203.0.113.5', 'jdoe')); + } + + public function testKnownIpReducesScore() + { + $params = self::allOff(array('riskKnownIpEnabled' => 1)); + $this->assertSame(-5, RiskHelper::computeScore($this->db(true), $this->logger, $params, '203.0.113.5', 'jdoe')); + $params->set('riskKnownIpPoints', 8); + $this->assertSame(-8, RiskHelper::computeScore($this->db(true), $this->logger, $params, '203.0.113.5', 'jdoe')); + $this->assertSame(0, RiskHelper::computeScore($this->db(false), $this->logger, $params, '203.0.113.5', 'jdoe')); + } + + public function testKnownIpLookupFailureFailsSafe() + { + $db = $this->createMock(DatabaseHelper::class); + $db->method('isKnownIpUsername')->willThrowException(new \RuntimeException('db down')); + $logger = $this->logger; + $params = self::allOff(array('riskKnownIpEnabled' => 1)); + $this->assertSame(0, RiskHelper::computeScore($db, $logger, $params, '203.0.113.5', 'jdoe')); + $this->assertTrue($logger->hasMessage(Log::WARNING, 'db down')); + } + + public function testCommonUsernameMatchesCaseInsensitivelyWithCrlfList() + { + $params = self::allOff(array('riskCommonUsernameEnabled' => 1, + 'riskCommonUsernames' => "admin\r\n Root \r\n\r\ntest")); + $logger = $this->logger; + $this->assertSame(2, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', 'ADMIN')); + $this->assertSame(2, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', 'root')); + $this->assertSame(0, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', 'administrator')); + $this->assertSame(0, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', '')); + $params->set('riskCommonUsernamePoints', 4); + $this->assertSame(4, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', 'test')); + } + + public function testMissingUserAgentIncreasesScore() + { + $params = self::allOff(array('riskUserAgentEnabled' => 1)); + $logger = $this->logger; + $this->assertSame(0, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', 'jdoe')); + $_SERVER['HTTP_USER_AGENT'] = ' '; + $this->assertSame(2, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', 'jdoe')); + unset($_SERVER['HTTP_USER_AGENT']); + $this->assertSame(2, RiskHelper::computeScore($this->db(), $logger, $params, '203.0.113.5', 'jdoe')); + } + + public function testGeoWithoutDatabaseScoresZero() + { + $params = self::allOff(array('riskGeoEnabled' => 1, 'geoDbPath' => '', 'riskGeoHomeCountries' => 'AT,DE')); + $this->assertSame(0, RiskHelper::computeScore($this->db(), $this->logger, $params, '203.0.113.5', 'jdoe')); + } + + public function testReverseDnsUsesCachedHostname() + { + $params = self::allOff(array('riskReverseDnsEnabled' => 1)); + + $db = $this->createMock(DatabaseHelper::class); + $db->method('getCachedHostname')->willReturn('host.example.org'); + $db->expects($this->never())->method('cacheHostname'); + $this->assertSame(0, RiskHelper::computeScore($db, $this->logger, $params, '203.0.113.5', 'jdoe')); + + $db = $this->createMock(DatabaseHelper::class); + $db->method('getCachedHostname')->willReturn(null); // cached "no PTR record" + $db->expects($this->never())->method('cacheHostname'); + $this->assertSame(2, RiskHelper::computeScore($db, $this->logger, $params, '203.0.113.5', 'jdoe')); + } + + public function testReverseDnsFailureFailsSafe() + { + $params = self::allOff(array('riskReverseDnsEnabled' => 1)); + $db = $this->createMock(DatabaseHelper::class); + $db->method('getCachedHostname')->willThrowException(new \RuntimeException('cache broken')); + $logger = $this->logger; + $this->assertSame(0, RiskHelper::computeScore($db, $logger, $params, '203.0.113.5', 'jdoe')); + $this->assertTrue($logger->hasMessage(Log::WARNING, 'cache broken')); + } + + public function testSignalsAreSummed() + { + $params = new Registry(array( + 'riskCommonUsernames' => 'admin', + 'riskReverseDnsEnabled' => 1, + )); + $db = $this->createMock(DatabaseHelper::class); + $db->method('isKnownIpUsername')->willReturn(true); + $db->method('getCachedHostname')->willReturn(null); + unset($_SERVER['HTTP_USER_AGENT']); + // known IP -5, common username +2, no user agent +2, no rDNS +2 + $this->assertSame(1, RiskHelper::computeScore($db, $this->logger, $params, '203.0.113.5', 'admin')); + } +} diff --git a/tests/Integration/fixtures/request.php b/tests/Integration/fixtures/request.php index 006c42f..61a6afc 100644 --- a/tests/Integration/fixtures/request.php +++ b/tests/Integration/fixtures/request.php @@ -7,7 +7,9 @@ **/ // Simulates the start of a site request from the IP address given as first // argument, as far as the plugin is concerned: prints the plugin's block -// message if it rejects the request, "NOT BLOCKED" otherwise. +// message if it rejects the request, "NOT BLOCKED" otherwise. An optional +// second argument gives the request parameters as a query string (e.g. +// "option=com_users&task=user.login"). // Used by PluginEventsTest; needs the same environment as the tests. use Joomla\CMS\Factory; @@ -18,6 +20,11 @@ $_SERVER['REMOTE_ADDR'] = $argv[1]; $app = Factory::getApplication(); +parse_str($argv[2] ?? '', $params); +foreach ($params as $name => $value) +{ + $app->input->set($name, $value); +} PluginHelper::importPlugin('system', 'bfstop', true, $app->getDispatcher()); $app->getDispatcher()->dispatch('onAfterInitialise', new Event('onAfterInitialise', array())); echo "NOT BLOCKED\n"; diff --git a/tests/README.md b/tests/README.md index 911cf9c..c941dde 100644 --- a/tests/README.md +++ b/tests/README.md @@ -1,6 +1,7 @@ # Tests -- `Unit/`: tests needing nothing but PHP and [PHPUnit](https://phpunit.de) 11. +- `Unit/`: tests needing nothing but PHP and [PHPUnit](https://phpunit.de) 11 + (the component's IP range tests only run if `COM_BFSTOP_ROOT` is set). - `Integration/`: tests against a real Joomla site with bfstop installed. They run every database query of the plugin (and of the component, if `COM_BFSTOP_ROOT` is set) on the site's database, and let the plugin react @@ -11,6 +12,10 @@ The classes under test are always loaded from this checkout (and the component checkout), not from the copies installed into the Joomla site, so the site only needs to be set up again when the database schema changes. +- `lint/`: checks of the language files (`check-language.php`) and of the + release zip built by `deploy.sh zip` (`check-zip.php`); both scripts are + kept identical in the com_bfstop repository. + GitHub Actions runs everything on each push and pull request, on Joomla 5 and 6 with MySQL, MariaDB and PostgreSQL, see `.github/workflows/ci.yml`. diff --git a/tests/Unit/IpRangeHelperTest.php b/tests/Unit/IpRangeHelperTest.php new file mode 100644 index 0000000..8a4eb1b --- /dev/null +++ b/tests/Unit/IpRangeHelperTest.php @@ -0,0 +1,95 @@ +markTestSkipped('COM_BFSTOP_ROOT not set, see tests/README.md'); + } + } + + public function testIsIPv6() + { + $this->assertTrue(IpRangeHelper::isIPv6('2001:db8::1')); + $this->assertTrue(IpRangeHelper::isIPv6('::1')); + $this->assertFalse(IpRangeHelper::isIPv6('192.0.2.1')); + } + + public static function rangeProvider() + { + return array( + 'v4 /32' => array('192.0.2.7/32', '192.0.2.7', '192.0.2.7'), + 'v4 /24' => array('192.0.2.0/24', '192.0.2.0', '192.0.2.255'), + 'v4 /24 misaligned' => array('192.0.2.77/24', '192.0.2.0', '192.0.2.255'), + 'v4 /20' => array('10.1.17.3/20', '10.1.16.0', '10.1.31.255'), + 'v4 /1' => array('200.0.0.1/1', '128.0.0.0', '255.255.255.255'), + 'v4 /0' => array('192.0.2.1/0', '0.0.0.0', '255.255.255.255'), + 'v6 /128' => array('2001:db8::1/128', '2001:db8::1', '2001:db8::1'), + 'v6 /64' => array('2001:db8:0:1::/64', '2001:db8:0:1::', '2001:db8:0:1:ffff:ffff:ffff:ffff'), + 'v6 /64 misaligned' => array('2001:db8:0:1:2:3:4:5/64', '2001:db8:0:1::', '2001:db8:0:1:ffff:ffff:ffff:ffff'), + 'v6 /33 odd bits' => array('2001:db8:ffff::/33', '2001:db8:8000::', '2001:db8:ffff:ffff:ffff:ffff:ffff:ffff'), + 'v6 /0' => array('2001:db8::/0', '::', 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff'), + ); + } + + #[DataProvider('rangeProvider')] + public function testCidrToRange($cidr, $start, $end) + { + $this->assertSame(array($start, $end), IpRangeHelper::cidrToRange($cidr)); + } + + public function testNumOfAddresses() + { + $this->assertEquals(1, IpRangeHelper::numOfAddresses('192.0.2.1/32')); + $this->assertEquals(256, IpRangeHelper::numOfAddresses('192.0.2.0/24')); + $this->assertEquals(4294967296, IpRangeHelper::numOfAddresses('0.0.0.0/0')); + $this->assertEquals(1, IpRangeHelper::numOfAddresses('2001:db8::1/128')); + $this->assertEquals(pow(2, 64), IpRangeHelper::numOfAddresses('2001:db8::/64')); + } + + public function testFormatCount() + { + $this->assertSame('256', IpRangeHelper::formatCount(256)); + $this->assertSame('18446744073709551616', IpRangeHelper::formatCount(IpRangeHelper::numOfAddresses('2001:db8::/64'))); + $this->assertStringNotContainsString('E', IpRangeHelper::formatCount(IpRangeHelper::numOfAddresses('::/0'))); + } + + public static function maskProvider() + { + return array( + array(0, str_repeat("\x00", 16)), + array(1, "\x80".str_repeat("\x00", 15)), + array(7, "\xfe".str_repeat("\x00", 15)), + array(8, "\xff".str_repeat("\x00", 15)), + array(9, "\xff\x80".str_repeat("\x00", 14)), + array(127, str_repeat("\xff", 15)."\xfe"), + array(128, str_repeat("\xff", 16)), + ); + } + + #[DataProvider('maskProvider')] + public function testIpv6Mask($bits, $expected) + { + $mask = IpRangeHelper::ipv6Mask($bits); + $this->assertSame(16, strlen($mask)); + $this->assertSame(bin2hex($expected), bin2hex($mask)); + } +} diff --git a/tests/lint/check-language.php b/tests/lint/check-language.php new file mode 100644 index 0000000..e002888 --- /dev/null +++ b/tests/lint/check-language.php @@ -0,0 +1,144 @@ + [...] + * + * Errors (exit code 1): + * - an .ini file which PHP/Joomla cannot parse + * - a key without one of the given prefixes (only if prefixes are given) + * - a file whose language tag does not match the folder it is in + * - a language file referenced in the manifest which does not exist + * Warnings (reported, but exit code 0): + * - keys present in en-GB but missing in a translation, or vice versa + * + * NOTE: this file is kept identical in the bfstop and com_bfstop repositories. +**/ + +if ($argc < 2) +{ + fwrite(STDERR, "Usage: php {$argv[0]} [...]\n"); + exit(2); +} + +$manifest = $argv[1]; +$prefixes = array_slice($argv, 2); +$root = dirname(realpath($manifest)); +$errors = array(); +$warnings = array(); + +// 1. every language file referenced in the manifest must exist +$xml = simplexml_load_file($manifest); +if ($xml === false) +{ + fwrite(STDERR, "ERROR: cannot parse manifest $manifest\n"); + exit(1); +} +foreach ($xml->xpath('//languages') as $languages) +{ + $folder = (string) $languages['folder']; + foreach ($languages->language as $language) + { + $path = $root.'/'.($folder !== '' ? $folder.'/' : '').(string) $language; + if (!is_file($path)) + { + $errors[] = "manifest references missing language file $path"; + } + } +} + +// 2. parse all .ini files, grouped by "|" +$groups = array(); +$it = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($root, FilesystemIterator::SKIP_DOTS)); +foreach ($it as $file) +{ + $path = $file->getPathname(); + if ($file->getExtension() !== 'ini' || preg_match('#/(vendor|node_modules|\.git)/#', $path)) + { + continue; + } + $rel = substr($path, strlen($root) + 1); + if (!preg_match('/^([a-z]{2,3}-[A-Z]{2})\.(.+)$/', $file->getFilename(), $m)) + { + $errors[] = "$rel: file name does not start with a language tag"; + continue; + } + [, $tag, $base] = $m; + if (basename(dirname($path)) !== $tag) + { + $errors[] = "$rel: file for language $tag is not located in a $tag/ folder (Joomla will never load it)"; + continue; + } + + // Joomla parses language files like this (see LanguageHelper::parseIniFile) + $contents = str_replace('"_QQ_"', '\\"', file_get_contents($path)); + $strings = @parse_ini_string($contents, false, INI_SCANNER_RAW); + if ($strings === false) + { + $err = error_get_last(); + $errors[] = "$rel: cannot be parsed: ".($err['message'] ?? 'unknown error'); + continue; + } + foreach (array_keys($strings) as $key) + { + $ok = count($prefixes) === 0; + foreach ($prefixes as $prefix) + { + if (str_starts_with($key, $prefix)) + { + $ok = true; + break; + } + } + if (!$ok) + { + $errors[] = "$rel: key '$key' does not start with ".implode(' or ', $prefixes); + } + } + $groups[dirname($path, 2).'|'.$base][$tag] = array('rel' => $rel, 'keys' => array_keys($strings)); +} + +// 3. compare translations against en-GB +foreach ($groups as $group => $langs) +{ + if (!isset($langs['en-GB'])) + { + $warnings[] = "no en-GB reference file for group $group"; + continue; + } + $reference = $langs['en-GB']['keys']; + foreach ($langs as $tag => $info) + { + if ($tag === 'en-GB') + { + continue; + } + $missing = array_diff($reference, $info['keys']); + $extra = array_diff($info['keys'], $reference); + if ($missing) + { + $warnings[] = $info['rel'].': '.count($missing).' key(s) missing (en-GB fallback is used): '.implode(', ', $missing); + } + if ($extra) + { + $warnings[] = $info['rel'].': '.count($extra).' key(s) not in en-GB (obsolete?): '.implode(', ', $extra); + } + } +} + +$gha = getenv('GITHUB_ACTIONS') === 'true'; +foreach ($warnings as $w) +{ + echo ($gha ? '::warning::' : 'WARNING: ').$w."\n"; +} +foreach ($errors as $e) +{ + echo ($gha ? '::error::' : 'ERROR: ').$e."\n"; +} +echo count($errors).' error(s), '.count($warnings)." warning(s)\n"; +exit(count($errors) > 0 ? 1 : 0); diff --git a/tests/lint/check-zip.php b/tests/lint/check-zip.php new file mode 100644 index 0000000..f76bc38 --- /dev/null +++ b/tests/lint/check-zip.php @@ -0,0 +1,86 @@ + + * + * NOTE: this file is kept identical in the bfstop and com_bfstop repositories. +**/ + +if ($argc !== 3) +{ + fwrite(STDERR, "Usage: php {$argv[0]} \n"); + exit(2); +} +[, $zipFile, $manifestName] = $argv; + +$zip = new ZipArchive(); +if ($zip->open($zipFile) !== true) +{ + fwrite(STDERR, "ERROR: cannot open $zipFile\n"); + exit(1); +} +$entries = array(); +for ($i = 0; $i < $zip->numFiles; ++$i) +{ + $entries[] = rtrim($zip->getNameIndex($i), '/'); +} + +$manifest = $zip->getFromName($manifestName); +if ($manifest === false) +{ + fwrite(STDERR, "ERROR: manifest $manifestName is not at the top level of $zipFile\n"); + exit(1); +} +$xml = simplexml_load_string($manifest); + +$expected = array(); +$prefixed = function ($folder, $path) { + return ltrim(($folder !== '' ? $folder.'/' : '').$path, '/'); +}; +foreach ($xml->xpath('//files') as $files) +{ + $folder = (string) $files['folder']; + foreach ($files->children() as $child) + { + $expected[] = $prefixed($folder, (string) $child); + } +} +foreach ($xml->xpath('//languages') as $languages) +{ + foreach ($languages->language as $language) + { + $expected[] = $prefixed((string) $languages['folder'], (string) $language); + } +} +foreach ($xml->xpath('//scriptfile | //install/sql/file | //uninstall/sql/file | //update/schemas/schemapath') as $node) +{ + $expected[] = (string) $node; +} + +$errors = 0; +foreach ($expected as $path) +{ + $found = false; + foreach ($entries as $entry) + { + if ($entry === $path || str_starts_with($entry, $path.'/')) + { + $found = true; + break; + } + } + if (!$found) + { + echo (getenv('GITHUB_ACTIONS') === 'true' ? '::error::' : 'ERROR: ')."$zipFile is missing '$path' (referenced in $manifestName)\n"; + ++$errors; + } +} +echo count($expected)." manifest entries checked, $errors missing\n"; +exit($errors > 0 ? 1 : 0);