From: Thalia Date: Fri, 5 Apr 2019 19:13:17 +0000 (+0100) Subject: Introduce a BlockManager service X-Git-Tag: 1.34.0-rc.0~1834^2 X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_ecrire%28%22sites_tous%22%29%20.%20%22?a=commitdiff_plain;h=52f7720227491d80156880e8ddfbaf744f8e676f;p=lhc%2Fweb%2Fwiklou.git Introduce a BlockManager service This introduces a minimal BlockManager service, for getting blocks that apply to a User. Move the part of User::getBlockedStatus that checks for the blocks into BlockManager::getUserBlock, and move the related helper methods from User to BlockManager. Hard deprecate or remove these helper methods, and move to private methods in the BlockManager: * User::getBlockFromCookieValue * User::isLocallyBlockedProxy * User::inDnsBlacklist Soft deprecate these helper methods, and move to public methods in the BlockManager: * User::isDnsBlacklisted Add tests to cover the methods moved to BlockManager. Bug: T219441 Change-Id: I0af658d71288376735cebe541215383b56bb72e5 --- diff --git a/RELEASE-NOTES-1.34 b/RELEASE-NOTES-1.34 index 126e00bf6a..834ec895ee 100644 --- a/RELEASE-NOTES-1.34 +++ b/RELEASE-NOTES-1.34 @@ -116,6 +116,10 @@ because of Phabricator reports. * User::isBlocked() is deprecated since it does not tell you if the user is blocked from editing a particular page. Use User::getBlock() or PermissionManager::isBlockedFrom() or PermissionManager::userCan() instead. +* User::isLocallyBlockedProxy and User::inDnsBlacklist are deprecated and moved + to the BlockManager as private helper methods. +* User::isDnsBlacklisted is deprecated. Use BlockManager::isDnsBlacklisted + instead. * … === Other changes in 1.34 === diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 3590633f5c..c374a62523 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -13,6 +13,7 @@ use GlobalVarConfig; use Hooks; use IBufferingStatsdDataFactory; use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface; +use MediaWiki\Block\BlockManager; use MediaWiki\Block\BlockRestrictionStore; use MediaWiki\Http\HttpRequestFactory; use MediaWiki\Permissions\PermissionManager; @@ -437,6 +438,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'BlobStoreFactory' ); } + /** + * @since 1.34 + * @return BlockManager + */ + public function getBlockManager() : BlockManager { + return $this->getService( 'BlockManager' ); + } + /** * @since 1.33 * @return BlockRestrictionStore diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 832cee89c2..93ddee924f 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -39,6 +39,7 @@ use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface; use MediaWiki\Auth\AuthManager; +use MediaWiki\Block\BlockManager; use MediaWiki\Block\BlockRestrictionStore; use MediaWiki\Config\ConfigRepository; use MediaWiki\Interwiki\ClassicInterwikiLookup; @@ -85,6 +86,23 @@ return [ ); }, + 'BlockManager' => function ( MediaWikiServices $services ) : BlockManager { + $config = $services->getMainConfig(); + $context = RequestContext::getMain(); + return new BlockManager( + $context->getUser(), + $context->getRequest(), + $config->get( 'ApplyIpBlocksToXff' ), + $config->get( 'CookieSetOnAutoblock' ), + $config->get( 'CookieSetOnIpBlock' ), + $config->get( 'DnsBlacklistUrls' ), + $config->get( 'EnableDnsBlacklist' ), + $config->get( 'ProxyList' ), + $config->get( 'ProxyWhitelist' ), + $config->get( 'SoftBlockRanges' ) + ); + }, + 'BlockRestrictionStore' => function ( MediaWikiServices $services ) : BlockRestrictionStore { return new BlockRestrictionStore( $services->getDBLoadBalancer() diff --git a/includes/auth/AuthManager.php b/includes/auth/AuthManager.php index 3515a7012c..5915d35c96 100644 --- a/includes/auth/AuthManager.php +++ b/includes/auth/AuthManager.php @@ -1021,7 +1021,10 @@ class AuthManager implements LoggerAwareInterface { } $ip = $this->getRequest()->getIP(); - if ( $creator->isDnsBlacklisted( $ip, true /* check $wgProxyWhitelist */ ) ) { + if ( + MediaWikiServices::getInstance()->getBlockManager() + ->isDnsBlacklisted( $ip, true /* check $wgProxyWhitelist */ ) + ) { return Status::newFatal( 'sorbs_create_account_reason' ); } diff --git a/includes/block/BlockManager.php b/includes/block/BlockManager.php new file mode 100644 index 0000000000..3ef35d754a --- /dev/null +++ b/includes/block/BlockManager.php @@ -0,0 +1,370 @@ +getBlockManager(). + * + * @since 1.34 Refactored from User and Block. + */ +class BlockManager { + // TODO: This should be UserIdentity instead of User + /** @var User */ + private $currentUser; + + /** @var WebRequest */ + private $currentRequest; + + /** @var bool */ + private $applyIpBlocksToXff; + + /** @var bool */ + private $cookieSetOnAutoblock; + + /** @var bool */ + private $cookieSetOnIpBlock; + + /** @var array */ + private $dnsBlacklistUrls; + + /** @var bool */ + private $enableDnsBlacklist; + + /** @var array */ + private $proxyList; + + /** @var array */ + private $proxyWhitelist; + + /** @var array */ + private $softBlockRanges; + + /** + * @param User $currentUser + * @param WebRequest $currentRequest + * @param bool $applyIpBlocksToXff + * @param bool $cookieSetOnAutoblock + * @param bool $cookieSetOnIpBlock + * @param array $dnsBlacklistUrls + * @param bool $enableDnsBlacklist + * @param array $proxyList + * @param array $proxyWhitelist + * @param array $softBlockRanges + */ + public function __construct( + $currentUser, + $currentRequest, + $applyIpBlocksToXff, + $cookieSetOnAutoblock, + $cookieSetOnIpBlock, + $dnsBlacklistUrls, + $enableDnsBlacklist, + $proxyList, + $proxyWhitelist, + $softBlockRanges + ) { + $this->currentUser = $currentUser; + $this->currentRequest = $currentRequest; + $this->applyIpBlocksToXff = $applyIpBlocksToXff; + $this->cookieSetOnAutoblock = $cookieSetOnAutoblock; + $this->cookieSetOnIpBlock = $cookieSetOnIpBlock; + $this->dnsBlacklistUrls = $dnsBlacklistUrls; + $this->enableDnsBlacklist = $enableDnsBlacklist; + $this->proxyList = $proxyList; + $this->proxyWhitelist = $proxyWhitelist; + $this->softBlockRanges = $softBlockRanges; + } + + /** + * Get the blocks that apply to a user and return the most relevant one. + * + * TODO: $user should be UserIdentity instead of User + * + * @internal This should only be called by User::getBlockedStatus + * @param User $user + * @param bool $fromReplica Whether to check the replica DB first. + * To improve performance, non-critical checks are done against replica DBs. + * Check when actually saving should be done against master. + * @return Block|null The most relevant block, or null if there is no block. + */ + public function getUserBlock( User $user, $fromReplica ) { + $isAnon = $user->getId() === 0; + + // TODO: If $user is the current user, we should use the current request. Otherwise, + // we should not look for XFF or cookie blocks. + $request = $user->getRequest(); + + # We only need to worry about passing the IP address to the Block generator if the + # user is not immune to autoblocks/hardblocks, and they are the current user so we + # know which IP address they're actually coming from + $ip = null; + $sessionUser = $this->currentUser; + // the session user is set up towards the end of Setup.php. Until then, + // assume it's a logged-out user. + $globalUserName = $sessionUser->isSafeToLoad() + ? $sessionUser->getName() + : IP::sanitizeIP( $this->currentRequest->getIP() ); + if ( $user->getName() === $globalUserName && !$user->isAllowed( 'ipblock-exempt' ) ) { + $ip = $this->currentRequest->getIP(); + } + + // User/IP blocking + // TODO: remove dependency on Block + $block = Block::newFromTarget( $user, $ip, !$fromReplica ); + + // Cookie blocking + if ( !$block instanceof Block ) { + $block = $this->getBlockFromCookieValue( $user, $request ); + } + + // Proxy blocking + if ( !$block instanceof Block && $ip !== null && !in_array( $ip, $this->proxyWhitelist ) ) { + // Local list + if ( $this->isLocallyBlockedProxy( $ip ) ) { + $block = new Block( [ + 'byText' => wfMessage( 'proxyblocker' )->text(), + 'reason' => wfMessage( 'proxyblockreason' )->plain(), + 'address' => $ip, + 'systemBlock' => 'proxy', + ] ); + } elseif ( $isAnon && $this->isDnsBlacklisted( $ip ) ) { + $block = new Block( [ + 'byText' => wfMessage( 'sorbs' )->text(), + 'reason' => wfMessage( 'sorbsreason' )->plain(), + 'address' => $ip, + 'systemBlock' => 'dnsbl', + ] ); + } + } + + // (T25343) Apply IP blocks to the contents of XFF headers, if enabled + if ( !$block instanceof Block + && $this->applyIpBlocksToXff + && $ip !== null + && !in_array( $ip, $this->proxyWhitelist ) + ) { + $xff = $request->getHeader( 'X-Forwarded-For' ); + $xff = array_map( 'trim', explode( ',', $xff ) ); + $xff = array_diff( $xff, [ $ip ] ); + // TODO: remove dependency on Block + $xffblocks = Block::getBlocksForIPList( $xff, $isAnon, !$fromReplica ); + // TODO: remove dependency on Block + $block = Block::chooseBlock( $xffblocks, $xff ); + if ( $block instanceof Block ) { + # Mangle the reason to alert the user that the block + # originated from matching the X-Forwarded-For header. + $block->setReason( wfMessage( 'xffblockreason', $block->getReason() )->plain() ); + } + } + + if ( !$block instanceof Block + && $ip !== null + && $isAnon + && IP::isInRanges( $ip, $this->softBlockRanges ) + ) { + $block = new Block( [ + 'address' => $ip, + 'byText' => 'MediaWiki default', + 'reason' => wfMessage( 'softblockrangesreason', $ip )->plain(), + 'anonOnly' => true, + 'systemBlock' => 'wgSoftBlockRanges', + ] ); + } + + return $block; + } + + /** + * Try to load a Block from an ID given in a cookie value. + * + * @param UserIdentity $user + * @param WebRequest $request + * @return Block|bool The Block object, or false if none could be loaded. + */ + private function getBlockFromCookieValue( + UserIdentity $user, + WebRequest $request + ) { + $blockCookieVal = $request->getCookie( 'BlockID' ); + $response = $request->response(); + + // Make sure there's something to check. The cookie value must start with a number. + if ( strlen( $blockCookieVal ) < 1 || !is_numeric( substr( $blockCookieVal, 0, 1 ) ) ) { + return false; + } + // Load the Block from the ID in the cookie. + // TODO: remove dependency on Block + $blockCookieId = Block::getIdFromCookieValue( $blockCookieVal ); + if ( $blockCookieId !== null ) { + // An ID was found in the cookie. + // TODO: remove dependency on Block + $tmpBlock = Block::newFromID( $blockCookieId ); + if ( $tmpBlock instanceof Block ) { + switch ( $tmpBlock->getType() ) { + case Block::TYPE_USER: + $blockIsValid = !$tmpBlock->isExpired() && $tmpBlock->isAutoblocking(); + $useBlockCookie = ( $this->cookieSetOnAutoblock === true ); + break; + case Block::TYPE_IP: + case Block::TYPE_RANGE: + // If block is type IP or IP range, load only if user is not logged in (T152462) + $blockIsValid = !$tmpBlock->isExpired() && $user->getId() === 0; + $useBlockCookie = ( $this->cookieSetOnIpBlock === true ); + break; + default: + $blockIsValid = false; + $useBlockCookie = false; + } + + if ( $blockIsValid && $useBlockCookie ) { + // Use the block. + return $tmpBlock; + } + + // If the block is not valid, remove the cookie. + // TODO: remove dependency on Block + Block::clearCookie( $response ); + } else { + // If the block doesn't exist, remove the cookie. + // TODO: remove dependency on Block + Block::clearCookie( $response ); + } + } + return false; + } + + /** + * Check if an IP address is in the local proxy list + * + * @param string $ip + * @return bool + */ + private function isLocallyBlockedProxy( $ip ) { + if ( !$this->proxyList ) { + return false; + } + + if ( !is_array( $this->proxyList ) ) { + // Load values from the specified file + $this->proxyList = array_map( 'trim', file( $this->proxyList ) ); + } + + $resultProxyList = []; + $deprecatedIPEntries = []; + + // backward compatibility: move all ip addresses in keys to values + foreach ( $this->proxyList as $key => $value ) { + $keyIsIP = IP::isIPAddress( $key ); + $valueIsIP = IP::isIPAddress( $value ); + if ( $keyIsIP && !$valueIsIP ) { + $deprecatedIPEntries[] = $key; + $resultProxyList[] = $key; + } elseif ( $keyIsIP && $valueIsIP ) { + $deprecatedIPEntries[] = $key; + $resultProxyList[] = $key; + $resultProxyList[] = $value; + } else { + $resultProxyList[] = $value; + } + } + + if ( $deprecatedIPEntries ) { + wfDeprecated( + 'IP addresses in the keys of $wgProxyList (found the following IP addresses in keys: ' . + implode( ', ', $deprecatedIPEntries ) . ', please move them to values)', '1.30' ); + } + + $proxyListIPSet = new IPSet( $resultProxyList ); + return $proxyListIPSet->match( $ip ); + } + + /** + * Whether the given IP is in a DNS blacklist. + * + * @param string $ip IP to check + * @param bool $checkWhitelist Whether to check the whitelist first + * @return bool True if blacklisted. + */ + public function isDnsBlacklisted( $ip, $checkWhitelist = false ) { + if ( !$this->enableDnsBlacklist || + ( $checkWhitelist && in_array( $ip, $this->proxyWhitelist ) ) + ) { + return false; + } + + return $this->inDnsBlacklist( $ip, $this->dnsBlacklistUrls ); + } + + /** + * Whether the given IP is in a given DNS blacklist. + * + * @param string $ip IP to check + * @param array $bases Array of Strings: URL of the DNS blacklist + * @return bool True if blacklisted. + */ + private function inDnsBlacklist( $ip, array $bases ) { + $found = false; + // @todo FIXME: IPv6 ??? (https://bugs.php.net/bug.php?id=33170) + if ( IP::isIPv4( $ip ) ) { + // Reverse IP, T23255 + $ipReversed = implode( '.', array_reverse( explode( '.', $ip ) ) ); + + foreach ( $bases as $base ) { + // Make hostname + // If we have an access key, use that too (ProjectHoneypot, etc.) + $basename = $base; + if ( is_array( $base ) ) { + if ( count( $base ) >= 2 ) { + // Access key is 1, base URL is 0 + $host = "{$base[1]}.$ipReversed.{$base[0]}"; + } else { + $host = "$ipReversed.{$base[0]}"; + } + $basename = $base[0]; + } else { + $host = "$ipReversed.$base"; + } + + // Send query + $ipList = gethostbynamel( $host ); + + if ( $ipList ) { + wfDebugLog( 'dnsblacklist', "Hostname $host is {$ipList[0]}, it's a proxy says $basename!" ); + $found = true; + break; + } + + wfDebugLog( 'dnsblacklist', "Requested $host, not found in $basename." ); + } + } + + return $found; + } + +} diff --git a/includes/user/User.php b/includes/user/User.php index 13467a485a..d2a07072d3 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -1813,13 +1813,14 @@ class User implements IDBAccessObject, UserIdentity { /** * Get blocking information + * + * TODO: Move this into the BlockManager, along with block-related properties. + * * @param bool $fromReplica Whether to check the replica DB first. * To improve performance, non-critical checks are done against replica DBs. * Check when actually saving should be done against master. */ private function getBlockedStatus( $fromReplica = true ) { - global $wgProxyWhitelist, $wgApplyIpBlocksToXff, $wgSoftBlockRanges; - if ( $this->mBlockedby != -1 ) { return; } @@ -1833,79 +1834,10 @@ class User implements IDBAccessObject, UserIdentity { // overwriting mBlockedby, surely? $this->load(); - # We only need to worry about passing the IP address to the Block generator if the - # user is not immune to autoblocks/hardblocks, and they are the current user so we - # know which IP address they're actually coming from - $ip = null; - $sessionUser = RequestContext::getMain()->getUser(); - // the session user is set up towards the end of Setup.php. Until then, - // assume it's a logged-out user. - $globalUserName = $sessionUser->isSafeToLoad() - ? $sessionUser->getName() - : IP::sanitizeIP( $sessionUser->getRequest()->getIP() ); - if ( $this->getName() === $globalUserName && !$this->isAllowed( 'ipblock-exempt' ) ) { - $ip = $this->getRequest()->getIP(); - } - - // User/IP blocking - $block = Block::newFromTarget( $this, $ip, !$fromReplica ); - - // Cookie blocking - if ( !$block instanceof Block ) { - $block = $this->getBlockFromCookieValue( $this->getRequest()->getCookie( 'BlockID' ) ); - } - - // Proxy blocking - if ( !$block instanceof Block && $ip !== null && !in_array( $ip, $wgProxyWhitelist ) ) { - // Local list - if ( self::isLocallyBlockedProxy( $ip ) ) { - $block = new Block( [ - 'byText' => wfMessage( 'proxyblocker' )->text(), - 'reason' => wfMessage( 'proxyblockreason' )->plain(), - 'address' => $ip, - 'systemBlock' => 'proxy', - ] ); - } elseif ( $this->isAnon() && $this->isDnsBlacklisted( $ip ) ) { - $block = new Block( [ - 'byText' => wfMessage( 'sorbs' )->text(), - 'reason' => wfMessage( 'sorbsreason' )->plain(), - 'address' => $ip, - 'systemBlock' => 'dnsbl', - ] ); - } - } - - // (T25343) Apply IP blocks to the contents of XFF headers, if enabled - if ( !$block instanceof Block - && $wgApplyIpBlocksToXff - && $ip !== null - && !in_array( $ip, $wgProxyWhitelist ) - ) { - $xff = $this->getRequest()->getHeader( 'X-Forwarded-For' ); - $xff = array_map( 'trim', explode( ',', $xff ) ); - $xff = array_diff( $xff, [ $ip ] ); - $xffblocks = Block::getBlocksForIPList( $xff, $this->isAnon(), !$fromReplica ); - $block = Block::chooseBlock( $xffblocks, $xff ); - if ( $block instanceof Block ) { - # Mangle the reason to alert the user that the block - # originated from matching the X-Forwarded-For header. - $block->setReason( wfMessage( 'xffblockreason', $block->getReason() )->plain() ); - } - } - - if ( !$block instanceof Block - && $ip !== null - && $this->isAnon() - && IP::isInRanges( $ip, $wgSoftBlockRanges ) - ) { - $block = new Block( [ - 'address' => $ip, - 'byText' => 'MediaWiki default', - 'reason' => wfMessage( 'softblockrangesreason', $ip )->plain(), - 'anonOnly' => true, - 'systemBlock' => 'wgSoftBlockRanges', - ] ); - } + $block = MediaWikiServices::getInstance()->getBlockManager()->getUserBlock( + $this, + $fromReplica + ); if ( $block instanceof Block ) { wfDebug( __METHOD__ . ": Found block.\n" ); @@ -1928,82 +1860,30 @@ class User implements IDBAccessObject, UserIdentity { Hooks::run( 'GetBlockedStatus', [ &$thisUser ] ); } - /** - * Try to load a Block from an ID given in a cookie value. - * @param string|null $blockCookieVal The cookie value to check. - * @return Block|bool The Block object, or false if none could be loaded. - */ - protected function getBlockFromCookieValue( $blockCookieVal ) { - // Make sure there's something to check. The cookie value must start with a number. - if ( strlen( $blockCookieVal ) < 1 || !is_numeric( substr( $blockCookieVal, 0, 1 ) ) ) { - return false; - } - // Load the Block from the ID in the cookie. - $blockCookieId = Block::getIdFromCookieValue( $blockCookieVal ); - if ( $blockCookieId !== null ) { - // An ID was found in the cookie. - $tmpBlock = Block::newFromID( $blockCookieId ); - if ( $tmpBlock instanceof Block ) { - $config = RequestContext::getMain()->getConfig(); - - switch ( $tmpBlock->getType() ) { - case Block::TYPE_USER: - $blockIsValid = !$tmpBlock->isExpired() && $tmpBlock->isAutoblocking(); - $useBlockCookie = ( $config->get( 'CookieSetOnAutoblock' ) === true ); - break; - case Block::TYPE_IP: - case Block::TYPE_RANGE: - // If block is type IP or IP range, load only if user is not logged in (T152462) - $blockIsValid = !$tmpBlock->isExpired() && !$this->isLoggedIn(); - $useBlockCookie = ( $config->get( 'CookieSetOnIpBlock' ) === true ); - break; - default: - $blockIsValid = false; - $useBlockCookie = false; - } - - if ( $blockIsValid && $useBlockCookie ) { - // Use the block. - return $tmpBlock; - } - - // If the block is not valid, remove the cookie. - Block::clearCookie( $this->getRequest()->response() ); - } else { - // If the block doesn't exist, remove the cookie. - Block::clearCookie( $this->getRequest()->response() ); - } - } - return false; - } - /** * Whether the given IP is in a DNS blacklist. * + * @deprecated since 1.34 Use BlockManager::isDnsBlacklisted. * @param string $ip IP to check * @param bool $checkWhitelist Whether to check the whitelist first * @return bool True if blacklisted. */ public function isDnsBlacklisted( $ip, $checkWhitelist = false ) { - global $wgEnableDnsBlacklist, $wgDnsBlacklistUrls, $wgProxyWhitelist; - - if ( !$wgEnableDnsBlacklist || - ( $checkWhitelist && in_array( $ip, $wgProxyWhitelist ) ) - ) { - return false; - } - - return $this->inDnsBlacklist( $ip, $wgDnsBlacklistUrls ); + return MediaWikiServices::getInstance()->getBlockManager() + ->isDnsBlacklisted( $ip, $checkWhitelist ); } /** * Whether the given IP is in a given DNS blacklist. * + * @deprecated since 1.34 Check via BlockManager::isDnsBlacklisted instead. * @param string $ip IP to check * @param string|array $bases Array of Strings: URL of the DNS blacklist * @return bool True if blacklisted. */ public function inDnsBlacklist( $ip, $bases ) { + wfDeprecated( __METHOD__, '1.34' ); + $found = false; // @todo FIXME: IPv6 ??? (https://bugs.php.net/bug.php?id=33170) if ( IP::isIPv4( $ip ) ) { @@ -2045,11 +1925,13 @@ class User implements IDBAccessObject, UserIdentity { /** * Check if an IP address is in the local proxy list * + * @deprecated since 1.34 Use BlockManager::getUserBlock instead. * @param string $ip - * * @return bool */ public static function isLocallyBlockedProxy( $ip ) { + wfDeprecated( __METHOD__, '1.34' ); + global $wgProxyList; if ( !$wgProxyList ) { diff --git a/tests/phpunit/includes/auth/AuthManagerTest.php b/tests/phpunit/includes/auth/AuthManagerTest.php index d5e18797cf..209ed55fd4 100644 --- a/tests/phpunit/includes/auth/AuthManagerTest.php +++ b/tests/phpunit/includes/auth/AuthManagerTest.php @@ -1471,10 +1471,12 @@ class AuthManagerTest extends \MediaWikiTestCase { ], 'wgProxyWhitelist' => [], ] ); + $this->overrideMwServices(); $status = $this->manager->checkAccountCreatePermissions( new \User ); $this->assertFalse( $status->isOK() ); $this->assertTrue( $status->hasMessage( 'sorbs_create_account_reason' ) ); $this->setMwGlobals( 'wgProxyWhitelist', [ '127.0.0.1' ] ); + $this->overrideMwServices(); $status = $this->manager->checkAccountCreatePermissions( new \User ); $this->assertTrue( $status->isGood() ); } diff --git a/tests/phpunit/includes/block/BlockManagerTest.php b/tests/phpunit/includes/block/BlockManagerTest.php new file mode 100644 index 0000000000..414566503e --- /dev/null +++ b/tests/phpunit/includes/block/BlockManagerTest.php @@ -0,0 +1,226 @@ +user = $this->getTestUser()->getUser(); + $this->sysopId = $this->getTestSysop()->getUser()->getId(); + } + + private function getBlockManager( $overrideConfig ) { + $blockManagerConfig = array_merge( [ + 'wgApplyIpBlocksToXff' => true, + 'wgCookieSetOnAutoblock' => true, + 'wgCookieSetOnIpBlock' => true, + 'wgDnsBlacklistUrls' => [], + 'wgEnableDnsBlacklist' => true, + 'wgProxyList' => [], + 'wgProxyWhitelist' => [], + 'wgSoftBlockRanges' => [], + ], $overrideConfig ); + return new BlockManager( + $this->user, + $this->user->getRequest(), + ...array_values( $blockManagerConfig ) + ); + } + + /** + * @dataProvider provideGetBlockFromCookieValue + * @covers ::getBlockFromCookieValue + */ + public function testGetBlockFromCookieValue( $options, $expected ) { + $blockManager = $this->getBlockManager( [ + 'wgCookieSetOnAutoblock' => true, + 'wgCookieSetOnIpBlock' => true, + ] ); + + $block = new Block( array_merge( [ + 'address' => $options[ 'target' ] ?: $this->user, + 'by' => $this->sysopId, + ], $options[ 'blockOptions' ] ) ); + $block->insert(); + + $class = new ReflectionClass( BlockManager::class ); + $method = $class->getMethod( 'getBlockFromCookieValue' ); + $method->setAccessible( true ); + + $user = $options[ 'loggedIn' ] ? $this->user : new User(); + $user->getRequest()->setCookie( 'BlockID', $block->getCookieValue() ); + + $this->assertSame( $expected, (bool)$method->invoke( + $blockManager, + $user, + $user->getRequest() + ) ); + + $block->delete(); + } + + public static function provideGetBlockFromCookieValue() { + return [ + 'Autoblocking user block' => [ + [ + 'target' => '', + 'loggedIn' => true, + 'blockOptions' => [ + 'enableAutoblock' => true + ], + ], + true, + ], + 'Non-autoblocking user block' => [ + [ + 'target' => '', + 'loggedIn' => true, + 'blockOptions' => [], + ], + false, + ], + 'IP block for anonymous user' => [ + [ + 'target' => '127.0.0.1', + 'loggedIn' => false, + 'blockOptions' => [], + ], + true, + ], + 'IP block for logged in user' => [ + [ + 'target' => '127.0.0.1', + 'loggedIn' => true, + 'blockOptions' => [], + ], + false, + ], + 'IP range block for anonymous user' => [ + [ + 'target' => '127.0.0.0/8', + 'loggedIn' => false, + 'blockOptions' => [], + ], + true, + ], + ]; + } + + /** + * @dataProvider provideIsLocallyBlockedProxy + * @covers ::isLocallyBlockedProxy + */ + public function testIsLocallyBlockedProxy( $proxyList, $expected ) { + $class = new ReflectionClass( BlockManager::class ); + $method = $class->getMethod( 'isLocallyBlockedProxy' ); + $method->setAccessible( true ); + + $blockManager = $this->getBlockManager( [ + 'wgProxyList' => $proxyList + ] ); + + $ip = '1.2.3.4'; + $this->assertSame( $expected, $method->invoke( $blockManager, $ip ) ); + } + + public static function provideIsLocallyBlockedProxy() { + return [ + 'Proxy list is empty' => [ [], false ], + 'Proxy list contains IP' => [ [ '1.2.3.4' ], true ], + 'Proxy list contains IP as value' => [ [ 'test' => '1.2.3.4' ], true ], + 'Proxy list contains range that covers IP' => [ [ '1.2.3.0/16' ], true ], + ]; + } + + /** + * @covers ::isLocallyBlockedProxy + */ + public function testIsLocallyBlockedProxyDeprecated() { + $proxy = '1.2.3.4'; + + $this->hideDeprecated( + 'IP addresses in the keys of $wgProxyList (found the following IP ' . + 'addresses in keys: ' . $proxy . ', please move them to values)' + ); + + $class = new ReflectionClass( BlockManager::class ); + $method = $class->getMethod( 'isLocallyBlockedProxy' ); + $method->setAccessible( true ); + + $blockManager = $this->getBlockManager( [ + 'wgProxyList' => [ $proxy => 'test' ] + ] ); + + $ip = '1.2.3.4'; + $this->assertSame( true, $method->invoke( $blockManager, $ip ) ); + } + + /** + * @dataProvider provideIsDnsBlacklisted + * @covers ::isDnsBlacklisted + * @covers ::inDnsBlacklist + */ + public function testIsDnsBlacklisted( $options, $expected ) { + $blockManager = $this->getBlockManager( [ + 'wgEnableDnsBlacklist' => true, + 'wgDnsBlacklistUrls' => $options[ 'inBlacklist' ] ? [ 'local.wmftest.net' ] : [], + 'wgProxyWhitelist' => $options[ 'inWhitelist' ] ? [ '127.0.0.1' ] : [], + ] ); + + $ip = '127.0.0.1'; + $this->assertSame( + $expected, + $blockManager->isDnsBlacklisted( $ip, $options[ 'check' ] ) + ); + } + + public static function provideIsDnsBlacklisted() { + return [ + 'IP is blacklisted' => [ + [ + 'inBlacklist' => true, + 'inWhitelist' => false, + 'check' => false, + ], + true, + ], + 'IP is not blacklisted' => [ + [ + 'inBlacklist' => false, + 'inWhitelist' => false, + 'check' => false, + ], + false, + ], + 'IP is blacklisted and whitelisted; whitelist is checked' => [ + [ + 'inBlacklist' => true, + 'inWhitelist' => true, + 'check' => false, + ], + true, + ], + 'IP is blacklisted and whitelisted; whitelist is not checked' => [ + [ + 'inBlacklist' => true, + 'inWhitelist' => true, + 'check' => true, + ], + false, + ], + ]; + } +} diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php index e7bedc2d73..ec127af639 100644 --- a/tests/phpunit/includes/user/UserTest.php +++ b/tests/phpunit/includes/user/UserTest.php @@ -783,6 +783,7 @@ class UserTest extends MediaWikiTestCase { RequestContext::getMain()->setRequest( $request ); TestingAccessWrapper::newFromObject( $user )->mRequest = $request; $request->getSession()->setUser( $user ); + $this->overrideMwServices(); }; $this->setMwGlobals( 'wgSoftBlockRanges', [ '10.0.0.0/8' ] ); @@ -980,7 +981,7 @@ class UserTest extends MediaWikiTestCase { $this->assertFalse( $user->getExperienceLevel() ); } - public static function provideIsLocallBlockedProxy() { + public static function provideIsLocallyBlockedProxy() { return [ [ '1.2.3.4', '1.2.3.4' ], [ '1.2.3.4', '1.2.3.0/16' ], @@ -988,10 +989,12 @@ class UserTest extends MediaWikiTestCase { } /** - * @dataProvider provideIsLocallBlockedProxy + * @dataProvider provideIsLocallyBlockedProxy * @covers User::isLocallyBlockedProxy */ public function testIsLocallyBlockedProxy( $ip, $blockListEntry ) { + $this->hideDeprecated( 'User::isLocallyBlockedProxy' ); + $this->setMwGlobals( 'wgProxyList', [] );