From be27181956c738e2bcd5a1a01c63678d1dd36325 Mon Sep 17 00:00:00 2001 From: David Barratt Date: Tue, 30 Oct 2018 14:19:22 -0400 Subject: [PATCH] Add NamespaceRestriction class so that BlockRestriction can handle namespaces. This begins work on making namespaces a valid restriction type. The CRUD operations of BlockRestriction can now handle namespaces. Since NamespaceRestriction implements Restriction, enforcement should start working immediately, but testing enforcement will come in a subsequent patch since it's impossible to create them. Bug: T204991 Change-Id: I7264b452d9ad788c146d6ea25d01d4d7cb5ac4f6 --- includes/Block.php | 75 ++++++++++++++++ includes/block/BlockRestriction.php | 19 ++-- .../block/Restriction/AbstractRestriction.php | 24 ++++++ .../Restriction/NamespaceRestriction.php | 44 ++++++++++ .../block/Restriction/PageRestriction.php | 21 ++--- includes/block/Restriction/Restriction.php | 4 +- includes/user/User.php | 21 +++-- tests/phpunit/includes/BlockTest.php | 83 +++++++++++++++++- .../includes/block/BlockRestrictionTest.php | 86 ++++++++++++++----- .../Restriction/NamespaceRestrictionTest.php | 37 ++++++++ tests/phpunit/includes/user/UserTest.php | 12 +++ 11 files changed, 377 insertions(+), 49 deletions(-) create mode 100644 includes/block/Restriction/NamespaceRestriction.php create mode 100644 tests/phpunit/includes/block/Restriction/NamespaceRestrictionTest.php diff --git a/includes/Block.php b/includes/Block.php index 7138301d94..ea76cd6689 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -24,6 +24,8 @@ use Wikimedia\Rdbms\Database; use Wikimedia\Rdbms\IDatabase; use MediaWiki\Block\BlockRestriction; use MediaWiki\Block\Restriction\Restriction; +use MediaWiki\Block\Restriction\NamespaceRestriction; +use MediaWiki\Block\Restriction\PageRestriction; use MediaWiki\MediaWikiServices; class Block { @@ -1827,4 +1829,77 @@ class Block { return false; } + + /** + * Checks if a block applies to a particular namespace + * + * @since 1.33 + * + * @param int $ns + * @return bool + */ + public function appliesToNamespace( $ns ) { + if ( $this->isSitewide() ) { + return true; + } + + // Blocks do not apply to virtual namespaces. + if ( $ns < 0 ) { + return false; + } + + $restriction = $this->findRestriction( NamespaceRestriction::TYPE, $ns ); + + return (bool)$restriction; + } + + /** + * Checks if a block applies to a particular page + * + * This check does not consider whether `$this->prevents( 'editownusertalk' )` + * returns false, as the identity of the user making the hypothetical edit + * isn't known here (particularly in the case of IP hardblocks, range + * blocks, and auto-blocks). + * + * @since 1.33 + * + * @param int $pageId + * @return bool + */ + public function appliesToPage( $pageId ) { + if ( $this->isSitewide() ) { + return true; + } + + // If the pageId is not over zero, the block cannot apply to it. + if ( $pageId <= 0 ) { + return false; + } + + $restriction = $this->findRestriction( PageRestriction::TYPE, $pageId ); + + return (bool)$restriction; + } + + /** + * Find Restriction by type and value. + * + * @param string $type + * @param int $value + * @return Restriction|null + */ + private function findRestriction( $type, $value ) { + $restrictions = $this->getRestrictions(); + foreach ( $restrictions as $restriction ) { + if ( $restriction->getType() !== $type ) { + continue; + } + + if ( $restriction->getValue() === $value ) { + return $restriction; + } + } + + return null; + } } diff --git a/includes/block/BlockRestriction.php b/includes/block/BlockRestriction.php index 3baa063af9..b5574f9fe8 100644 --- a/includes/block/BlockRestriction.php +++ b/includes/block/BlockRestriction.php @@ -22,6 +22,7 @@ namespace MediaWiki\Block; +use MediaWiki\Block\Restriction\NamespaceRestriction; use MediaWiki\Block\Restriction\PageRestriction; use MediaWiki\Block\Restriction\Restriction; use Wikimedia\Rdbms\IResultWrapper; @@ -29,6 +30,14 @@ use Wikimedia\Rdbms\IDatabase; class BlockRestriction { + /** + * Map of all of the restriction types. + */ + private static $types = [ + PageRestriction::TYPE_ID => PageRestriction::class, + NamespaceRestriction::TYPE_ID => NamespaceRestriction::class, + ]; + /** * Retrieves the restrictions from the database by block id. * @@ -415,11 +424,11 @@ class BlockRestriction { * @return Restriction|null */ private static function rowToRestriction( \stdClass $row ) { - switch ( $row->ir_type ) { - case PageRestriction::TYPE_ID: - return PageRestriction::newFromRow( $row ); - default: - return null; + if ( array_key_exists( (int)$row->ir_type, self::$types ) ) { + $class = self::$types[ (int)$row->ir_type ]; + return call_user_func( [ $class, 'newFromRow' ], $row ); } + + return null; } } diff --git a/includes/block/Restriction/AbstractRestriction.php b/includes/block/Restriction/AbstractRestriction.php index 8c3e27fb80..c89ed72b1e 100644 --- a/includes/block/Restriction/AbstractRestriction.php +++ b/includes/block/Restriction/AbstractRestriction.php @@ -24,6 +24,16 @@ namespace MediaWiki\Block\Restriction; abstract class AbstractRestriction implements Restriction { + /** + * @var string + */ + const TYPE = ''; + + /** + * @var int + */ + const TYPE_ID = 0; + /** * @var int */ @@ -46,6 +56,20 @@ abstract class AbstractRestriction implements Restriction { $this->value = (int)$value; } + /** + * {@inheritdoc} + */ + public static function getType() { + return static::TYPE; + } + + /** + * {@inheritdoc} + */ + public static function getTypeId() { + return static::TYPE_ID; + } + /** * {@inheritdoc} */ diff --git a/includes/block/Restriction/NamespaceRestriction.php b/includes/block/Restriction/NamespaceRestriction.php new file mode 100644 index 0000000000..bf9769f4d1 --- /dev/null +++ b/includes/block/Restriction/NamespaceRestriction.php @@ -0,0 +1,44 @@ +getValue() === $title->getNamespace(); + } + +} diff --git a/includes/block/Restriction/PageRestriction.php b/includes/block/Restriction/PageRestriction.php index b72baeace2..bf7ef04ac0 100644 --- a/includes/block/Restriction/PageRestriction.php +++ b/includes/block/Restriction/PageRestriction.php @@ -24,33 +24,26 @@ namespace MediaWiki\Block\Restriction; class PageRestriction extends AbstractRestriction { - const TYPE = 'page'; - const TYPE_ID = 1; - /** - * @var \Title + * {@inheritdoc} */ - protected $title; + const TYPE = 'page'; /** * {@inheritdoc} */ - public function matches( \Title $title ) { - return $title->equals( $this->getTitle() ); - } + const TYPE_ID = 1; /** - * {@inheritdoc} + * @var \Title */ - public function getType() { - return self::TYPE; - } + protected $title; /** * {@inheritdoc} */ - public function getTypeId() { - return self::TYPE_ID; + public function matches( \Title $title ) { + return $title->equals( $this->getTitle() ); } /** diff --git a/includes/block/Restriction/Restriction.php b/includes/block/Restriction/Restriction.php index a89ca38e0e..fdf223a5ad 100644 --- a/includes/block/Restriction/Restriction.php +++ b/includes/block/Restriction/Restriction.php @@ -55,7 +55,7 @@ interface Restriction { * @since 1.33 * @return string */ - public function getType(); + public static function getType(); /** * Gets the id of the type of restriction. This id is used in the database. @@ -63,7 +63,7 @@ interface Restriction { * @since 1.33 * @return string */ - public function getTypeId(); + public static function getTypeId(); /** * Creates a new Restriction from a database row. diff --git a/includes/user/User.php b/includes/user/User.php index 79889ae963..f23c8ee586 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -2305,14 +2305,25 @@ class User implements IDBAccessObject, UserIdentity { // Special handling for a user's own talk page. The block is not aware // of the user, so this must be done here. if ( $title->equals( $this->getTalkPage() ) ) { - // If the block is sitewide, then whatever is set is what is honored. if ( $block->isSitewide() ) { + // If the block is sitewide, whatever is set is what is honored. + // This must be checked here, because Block::appliesToPage will + // return true for a sitewide block. $blocked = $block->prevents( 'editownusertalk' ); } else { - // If the block is partial, ignore 'editownusertalk' unless - // there is a restriction on the user talk namespace. - // TODO: To be implemented with Namespace restrictions - $blocked = $block->appliesToTitle( $title ); + // The page restrictions always take precedence over the namespace + // restrictions. If the user is explicity blocked from their own + // talk page, nothing can change that. + $blocked = $block->appliesToPage( $title->getArticleID() ); + + // If the block applies to the user talk namespace, then whatever is + // set is what is honored. + if ( !$blocked && $block->appliesToNamespace( NS_USER_TALK ) ) { + $blocked = $block->prevents( 'editownusertalk' ); + } + + // If another type of restriction is added, it should be checked + // here. } } else { $blocked = $block->appliesToTitle( $title ); diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php index 7d844b5869..e4dce12801 100644 --- a/tests/phpunit/includes/BlockTest.php +++ b/tests/phpunit/includes/BlockTest.php @@ -2,6 +2,7 @@ use MediaWiki\Block\BlockRestriction; use MediaWiki\Block\Restriction\PageRestriction; +use MediaWiki\Block\Restriction\NamespaceRestriction; /** * @group Database @@ -654,12 +655,92 @@ class BlockTest extends MediaWikiLangTestCase { $pageFoo = $this->getExistingTestPage( 'Foo' ); $pageBar = $this->getExistingTestPage( 'Bar' ); + $pageJohn = $this->getExistingTestPage( 'User:John' ); $pageRestriction = new PageRestriction( $block->getId(), $pageFoo->getId() ); - BlockRestriction::insert( [ $pageRestriction ] ); + $namespaceRestriction = new NamespaceRestriction( $block->getId(), NS_USER ); + BlockRestriction::insert( [ $pageRestriction, $namespaceRestriction ] ); $this->assertTrue( $block->appliesToTitle( $pageFoo->getTitle() ) ); $this->assertFalse( $block->appliesToTitle( $pageBar->getTitle() ) ); + $this->assertTrue( $block->appliesToTitle( $pageJohn->getTitle() ) ); + + $block->delete(); + } + + /** + * @covers Block::appliesToNamespace + * @covers Block::appliesToPage + */ + public function testAppliesToReturnsTrueOnSitewideBlock() { + $user = $this->getTestUser()->getUser(); + $block = new Block( [ + 'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 40 * 60 * 60 ) ), + 'allowUsertalk' => true, + 'sitewide' => true + ] ); + + $block->setTarget( $user ); + $block->setBlocker( $this->getTestSysop()->getUser() ); + $block->insert(); + + $title = $this->getExistingTestPage()->getTitle(); + + $this->assertTrue( $block->appliesToPage( $title->getArticleID() ) ); + $this->assertTrue( $block->appliesToNamespace( NS_MAIN ) ); + $this->assertTrue( $block->appliesToNamespace( NS_USER_TALK ) ); + + $block->delete(); + } + + /** + * @covers Block::appliesToPage + */ + public function testAppliesToPageOnPartialPageBlock() { + $user = $this->getTestUser()->getUser(); + $block = new Block( [ + 'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 40 * 60 * 60 ) ), + 'allowUsertalk' => true, + 'sitewide' => false + ] ); + + $block->setTarget( $user ); + $block->setBlocker( $this->getTestSysop()->getUser() ); + $block->insert(); + + $title = $this->getExistingTestPage()->getTitle(); + + $pageRestriction = new PageRestriction( + $block->getId(), + $title->getArticleID() + ); + BlockRestriction::insert( [ $pageRestriction ] ); + + $this->assertTrue( $block->appliesToPage( $title->getArticleID() ) ); + + $block->delete(); + } + + /** + * @covers Block::appliesToNamespace + */ + public function testAppliesToNamespaceOnPartialNamespaceBlock() { + $user = $this->getTestUser()->getUser(); + $block = new Block( [ + 'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 40 * 60 * 60 ) ), + 'allowUsertalk' => true, + 'sitewide' => false + ] ); + + $block->setTarget( $user ); + $block->setBlocker( $this->getTestSysop()->getUser() ); + $block->insert(); + + $namespaceRestriction = new NamespaceRestriction( $block->getId(), NS_MAIN ); + BlockRestriction::insert( [ $namespaceRestriction ] ); + + $this->assertTrue( $block->appliesToNamespace( NS_MAIN ) ); + $this->assertFalse( $block->appliesToNamespace( NS_USER ) ); $block->delete(); } diff --git a/tests/phpunit/includes/block/BlockRestrictionTest.php b/tests/phpunit/includes/block/BlockRestrictionTest.php index 7889f368c2..2d78018c4a 100644 --- a/tests/phpunit/includes/block/BlockRestrictionTest.php +++ b/tests/phpunit/includes/block/BlockRestrictionTest.php @@ -3,6 +3,7 @@ namespace MediaWiki\Tests\Block; use MediaWiki\Block\BlockRestriction; +use MediaWiki\Block\Restriction\NamespaceRestriction; use MediaWiki\Block\Restriction\PageRestriction; use MediaWiki\Block\Restriction\Restriction; @@ -30,13 +31,14 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { $pageBar = $this->getExistingTestPage( 'Bar' ); BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $pageFoo->getId() ), - new PageRestriction( $block->getId(), $pageBar->getId() ), + new PageRestriction( $block->getId(), $pageFoo->getId() ), + new PageRestriction( $block->getId(), $pageBar->getId() ), + new NamespaceRestriction( $block->getId(), NS_USER ), ] ); $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 2, $restrictions ); + $this->assertCount( 3, $restrictions ); } /** @@ -74,12 +76,14 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { // valid type $this->insertRestriction( $block->getId(), PageRestriction::TYPE_ID, $pageFoo->getId() ); + $this->insertRestriction( $block->getId(), NamespaceRestriction::TYPE_ID, NS_USER ); // invalid type $this->insertRestriction( $block->getId(), 9, $pageBar->getId() ); + $this->insertRestriction( $block->getId(), 10, NS_FILE ); $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); + $this->assertCount( 2, $restrictions ); } /** @@ -87,13 +91,14 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { * @covers ::resultToRestrictions * @covers ::rowToRestriction */ - public function testMappingRestrictionObject() { + public function testMappingPageRestrictionObject() { $block = $this->insertBlock(); $title = 'Lady Macbeth'; $page = $this->getExistingTestPage( $title ); + // Test Page Restrictions. BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), + new PageRestriction( $block->getId(), $page->getId() ), ] ); $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); @@ -106,6 +111,27 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { $this->assertEquals( $pageRestriction->getTitle()->getText(), $title ); } + /** + * @covers ::loadByBlockId + * @covers ::resultToRestrictions + * @covers ::rowToRestriction + */ + public function testMappingNamespaceRestrictionObject() { + $block = $this->insertBlock(); + + BlockRestriction::insert( [ + new NamespaceRestriction( $block->getId(), NS_USER ), + ] ); + + $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); + + list( $namespaceRestriction ) = $restrictions; + $this->assertInstanceOf( NamespaceRestriction::class, $namespaceRestriction ); + $this->assertEquals( $block->getId(), $namespaceRestriction->getBlockId() ); + $this->assertSame( NS_USER, $namespaceRestriction->getValue() ); + $this->assertEquals( $namespaceRestriction->getType(), NamespaceRestriction::TYPE ); + } + /** * @covers ::insert */ @@ -119,6 +145,7 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { new \stdClass(), new PageRestriction( $block->getId(), $pageFoo->getId() ), new PageRestriction( $block->getId(), $pageBar->getId() ), + new NamespaceRestriction( $block->getId(), NS_USER ) ]; $result = BlockRestriction::insert( $restrictions ); @@ -144,14 +171,6 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { $pageFoo = $this->getExistingTestPage( 'Foo' ); $pageBar = $this->getExistingTestPage( 'Bar' ); - $namespace = $this->createMock( Restriction::class ); - $namespace->method( 'toRow' ) - ->willReturn( [ - 'ir_ipb_id' => $block->getId(), - 'ir_type' => 2, - 'ir_value' => 0, - ] ); - $invalid = $this->createMock( Restriction::class ); $invalid->method( 'toRow' ) ->willReturn( [ @@ -164,7 +183,7 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { new \stdClass(), new PageRestriction( $block->getId(), $pageFoo->getId() ), new PageRestriction( $block->getId(), $pageBar->getId() ), - $namespace, + new NamespaceRestriction( $block->getId(), NS_USER ), $invalid, ]; @@ -172,7 +191,7 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { $this->assertTrue( $result ); $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 2, $restrictions ); + $this->assertCount( 3, $restrictions ); } /** @@ -191,6 +210,7 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { BlockRestriction::update( [ new \stdClass(), new PageRestriction( $block->getId(), $pageBar->getId() ), + new NamespaceRestriction( $block->getId(), NS_USER ), ] ); $db = wfGetDb( DB_REPLICA ); @@ -200,7 +220,7 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { [ 'ir_ipb_id' => $block->getId() ] ); - $this->assertEquals( 1, $result->numRows() ); + $this->assertEquals( 2, $result->numRows() ); $row = $result->fetchObject(); $this->assertEquals( $block->getId(), $row->ir_ipb_id ); $this->assertEquals( $pageBar->getId(), $row->ir_value ); @@ -261,7 +281,7 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { $block = $this->insertBlock(); $page = $this->getExistingTestPage( 'Foo' ); BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), + new PageRestriction( $block->getId(), $page->getId() ), ] ); BlockRestriction::update( [ @@ -502,6 +522,24 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { ], true ], + [ + [ + new NamespaceRestriction( 1, NS_USER ), + ], + [ + new NamespaceRestriction( 1, NS_USER ), + ], + true + ], + [ + [ + new NamespaceRestriction( 1, NS_USER ), + ], + [ + new NamespaceRestriction( 1, NS_TALK ), + ], + false + ], ]; } @@ -513,14 +551,18 @@ class BlockRestrictionTest extends \MediaWikiLangTestCase { new \stdClass(), new PageRestriction( 1, 1 ), new PageRestriction( 1, 2 ), + new NamespaceRestriction( 1, NS_USER ), ]; - $result = BlockRestriction::setBlockId( 2, $restrictions ); - $this->assertSame( 1, $restrictions[1]->getBlockId() ); $this->assertSame( 1, $restrictions[2]->getBlockId() ); - $this->assertSame( 2, $result[0]->getBlockId() ); - $this->assertSame( 2, $result[1]->getBlockId() ); + $this->assertSame( 1, $restrictions[3]->getBlockId() ); + + $result = BlockRestriction::setBlockId( 2, $restrictions ); + + foreach ( $result as $restriction ) { + $this->assertSame( 2, $restriction->getBlockId() ); + } } protected function insertBlock() { diff --git a/tests/phpunit/includes/block/Restriction/NamespaceRestrictionTest.php b/tests/phpunit/includes/block/Restriction/NamespaceRestrictionTest.php new file mode 100644 index 0000000000..4356240f20 --- /dev/null +++ b/tests/phpunit/includes/block/Restriction/NamespaceRestrictionTest.php @@ -0,0 +1,37 @@ +getClass(); + $page = $this->getExistingTestPage( 'Saturn' ); + $restriction = new $class( 1, NS_MAIN ); + $this->assertTrue( $restriction->matches( $page->getTitle() ) ); + + $page = $this->getExistingTestPage( 'Talk:Saturn' ); + $this->assertFalse( $restriction->matches( $page->getTitle() ) ); + } + + public function testGetType() { + $class = $this->getClass(); + $restriction = new $class( 1, 2 ); + $this->assertEquals( 'ns', $restriction->getType() ); + } + + /** + * {@inheritdoc} + */ + protected function getClass() { + return NamespaceRestriction::class; + } +} diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php index 50a9e50a9d..84f9378788 100644 --- a/tests/phpunit/includes/user/UserTest.php +++ b/tests/phpunit/includes/user/UserTest.php @@ -4,6 +4,7 @@ define( 'NS_UNITTEST', 5600 ); define( 'NS_UNITTEST_TALK', 5601 ); use MediaWiki\Block\Restriction\PageRestriction; +use MediaWiki\Block\Restriction\NamespaceRestriction; use MediaWiki\MediaWikiServices; use MediaWiki\User\UserIdentityValue; use Wikimedia\TestingAccessWrapper; @@ -1245,6 +1246,9 @@ class UserTest extends MediaWikiTestCase { ); $restrictions[] = new PageRestriction( 0, $page->getId() ); } + foreach ( $options['namespaceRestrictions'] ?? [] as $ns ) { + $restrictions[] = new NamespaceRestriction( 0, $ns ); + } $block = new Block( [ 'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 40 * 60 * 60 ) ), @@ -1319,6 +1323,14 @@ class UserTest extends MediaWikiTestCase { 'blockAllowsUTEdit' => false, ] ], + 'Partial namespace block, not allowing user talk' => [ self::USER_TALK_PAGE, true, [ + 'allowUsertalk' => false, + 'namespaceRestrictions' => [ NS_USER_TALK ], + ] ], + 'Partial namespace block, not allowing user talk' => [ self::USER_TALK_PAGE, false, [ + 'allowUsertalk' => true, + 'namespaceRestrictions' => [ NS_USER_TALK ], + ] ], ]; } -- 2.20.1