From 3cc3d00bcc94337e946c54ea19b66d6a6c63ed83 Mon Sep 17 00:00:00 2001 From: Petr Pchelko Date: Wed, 21 Aug 2019 12:49:59 -0700 Subject: [PATCH] Move getRestrictionLevels from NamespaceInfo to PermissionManager. Bug: T11977 Change-Id: I051be9148c98086fdf53a66a74bf7c28699016db --- includes/EditPage.php | 4 +- includes/MWNamespace.php | 5 +- includes/Permissions/PermissionManager.php | 83 ++++++++++++++++++- includes/ProtectionForm.php | 14 ++-- includes/editpage/TextboxBuilder.php | 4 +- includes/skins/SkinTemplate.php | 4 +- includes/title/NamespaceInfo.php | 81 ++---------------- .../Permissions/PermissionManagerTest.php | 76 ++++++++++++++++- .../includes/title/NamespaceInfoTest.php | 69 ++++----------- 9 files changed, 199 insertions(+), 141 deletions(-) diff --git a/includes/EditPage.php b/includes/EditPage.php index 550a018e5e..d0a5080d0b 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -4447,8 +4447,8 @@ ERROR; protected function addPageProtectionWarningHeaders() { $out = $this->context->getOutput(); if ( $this->mTitle->isProtected( 'edit' ) && - MediaWikiServices::getInstance()->getNamespaceInfo()->getRestrictionLevels( - $this->mTitle->getNamespace() + MediaWikiServices::getInstance()->getPermissionManager()->getNamespaceRestrictionLevels( + $this->getTitle()->getNamespace() ) !== [ '' ] ) { # Is the title semi-protected? diff --git a/includes/MWNamespace.php b/includes/MWNamespace.php index 0121bd589c..4a911b0819 100644 --- a/includes/MWNamespace.php +++ b/includes/MWNamespace.php @@ -318,8 +318,9 @@ class MWNamespace { * @return array */ public static function getRestrictionLevels( $index, User $user = null ) { - return MediaWikiServices::getInstance()->getNamespaceInfo()-> - getRestrictionLevels( $index, $user ); + return MediaWikiServices::getInstance() + ->getPermissionManager() + ->getNamespaceRestrictionLevels( $index, $user ); } /** diff --git a/includes/Permissions/PermissionManager.php b/includes/Permissions/PermissionManager.php index 2d4885ecb1..dd21a93c3c 100644 --- a/includes/Permissions/PermissionManager.php +++ b/includes/Permissions/PermissionManager.php @@ -68,7 +68,9 @@ class PermissionManager { 'BlockDisablesLogin', 'GroupPermissions', 'RevokePermissions', - 'AvailableRights' + 'AvailableRights', + 'NamespaceProtection', + 'RestrictionLevels' ]; /** @var ServiceOptions */ @@ -1415,6 +1417,85 @@ class PermissionManager { return $this->allRights; } + /** + * Determine which restriction levels it makes sense to use in a namespace, + * optionally filtered by a user's rights. + * + * @param int $index Index to check + * @param UserIdentity|null $user User to check + * @return array + */ + public function getNamespaceRestrictionLevels( $index, UserIdentity $user = null ) { + if ( !isset( $this->options->get( 'NamespaceProtection' )[$index] ) ) { + // All levels are valid if there's no namespace restriction. + // But still filter by user, if necessary + $levels = $this->options->get( 'RestrictionLevels' ); + if ( $user ) { + $levels = array_values( array_filter( $levels, function ( $level ) use ( $user ) { + $right = $level; + if ( $right == 'sysop' ) { + $right = 'editprotected'; // BC + } + if ( $right == 'autoconfirmed' ) { + $right = 'editsemiprotected'; // BC + } + return $this->userHasRight( $user, $right ); + } ) ); + } + return $levels; + } + + // $wgNamespaceProtection can require one or more rights to edit the namespace, which + // may be satisfied by membership in multiple groups each giving a subset of those rights. + // A restriction level is redundant if, for any one of the namespace rights, all groups + // giving that right also give the restriction level's right. Or, conversely, a + // restriction level is not redundant if, for every namespace right, there's at least one + // group giving that right without the restriction level's right. + // + // First, for each right, get a list of groups with that right. + $namespaceRightGroups = []; + foreach ( (array)$this->options->get( 'NamespaceProtection' )[$index] as $right ) { + if ( $right == 'sysop' ) { + $right = 'editprotected'; // BC + } + if ( $right == 'autoconfirmed' ) { + $right = 'editsemiprotected'; // BC + } + if ( $right != '' ) { + $namespaceRightGroups[$right] = $this->getGroupsWithPermission( $right ); + } + } + + // Now, go through the protection levels one by one. + $usableLevels = [ '' ]; + foreach ( $this->options->get( 'RestrictionLevels' ) as $level ) { + $right = $level; + if ( $right == 'sysop' ) { + $right = 'editprotected'; // BC + } + if ( $right == 'autoconfirmed' ) { + $right = 'editsemiprotected'; // BC + } + + if ( $right != '' && + !isset( $namespaceRightGroups[$right] ) && + ( !$user || $this->userHasRight( $user, $right ) ) + ) { + // Do any of the namespace rights imply the restriction right? (see explanation above) + foreach ( $namespaceRightGroups as $groups ) { + if ( !array_diff( $groups, $this->getGroupsWithPermission( $right ) ) ) { + // Yes, this one does. + continue 2; + } + } + // No, keep the restriction level + $usableLevels[] = $level; + } + } + + return $usableLevels; + } + /** * Add temporary user rights, only valid for the current scope. * This is meant for making it possible to programatically trigger certain actions that diff --git a/includes/ProtectionForm.php b/includes/ProtectionForm.php index adca805168..8b5d9956ab 100644 --- a/includes/ProtectionForm.php +++ b/includes/ProtectionForm.php @@ -90,7 +90,7 @@ class ProtectionForm { * Loads the current state of protection into the object. */ function loadData() { - $levels = MediaWikiServices::getInstance()->getNamespaceInfo()->getRestrictionLevels( + $levels = MediaWikiServices::getInstance()->getPermissionManager()->getNamespaceRestrictionLevels( $this->mTitle->getNamespace(), $this->mContext->getUser() ); $this->mCascade = $this->mTitle->areRestrictionsCascading(); @@ -180,7 +180,7 @@ class ProtectionForm { */ function execute() { if ( - MediaWikiServices::getInstance()->getNamespaceInfo()->getRestrictionLevels( + MediaWikiServices::getInstance()->getPermissionManager()->getNamespaceRestrictionLevels( $this->mTitle->getNamespace() ) === [ '' ] ) { @@ -586,10 +586,12 @@ class ProtectionForm { function buildSelector( $action, $selected ) { // If the form is disabled, display all relevant levels. Otherwise, // just show the ones this user can use. - $levels = MediaWikiServices::getInstance()->getNamespaceInfo()->getRestrictionLevels( - $this->mTitle->getNamespace(), - $this->disabled ? null : $this->mContext->getUser() - ); + $levels = MediaWikiServices::getInstance() + ->getPermissionManager() + ->getNamespaceRestrictionLevels( + $this->mTitle->getNamespace(), + $this->disabled ? null : $this->mContext->getUser() + ); $id = 'mwProtect-level-' . $action; diff --git a/includes/editpage/TextboxBuilder.php b/includes/editpage/TextboxBuilder.php index 103b3e5498..8161251d0b 100644 --- a/includes/editpage/TextboxBuilder.php +++ b/includes/editpage/TextboxBuilder.php @@ -75,8 +75,8 @@ class TextboxBuilder { public function getTextboxProtectionCSSClasses( Title $title ) { $classes = []; // Textarea CSS if ( $title->isProtected( 'edit' ) && - MediaWikiServices::getInstance()->getNamespaceInfo()-> - getRestrictionLevels( $title->getNamespace() ) !== [ '' ] + MediaWikiServices::getInstance()->getPermissionManager() + ->getNamespaceRestrictionLevels( $title->getNamespace() ) !== [ '' ] ) { # Is the title semi-protected? if ( $title->isSemiProtected() ) { diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php index af7ec294e9..a31f2a36eb 100644 --- a/includes/skins/SkinTemplate.php +++ b/includes/skins/SkinTemplate.php @@ -1068,8 +1068,8 @@ class SkinTemplate extends Skin { } if ( $title->quickUserCan( 'protect', $user ) && $title->getRestrictionTypes() && - MediaWikiServices::getInstance()->getNamespaceInfo()-> - getRestrictionLevels( $title->getNamespace(), $user ) !== [ '' ] + MediaWikiServices::getInstance()->getPermissionManager() + ->getNamespaceRestrictionLevels( $title->getNamespace(), $user ) !== [ '' ] ) { $mode = $title->isProtected() ? 'unprotect' : 'protect'; $content_navigation['actions'][$mode] = [ diff --git a/includes/title/NamespaceInfo.php b/includes/title/NamespaceInfo.php index 7307cc100b..105eeaa05d 100644 --- a/includes/title/NamespaceInfo.php +++ b/includes/title/NamespaceInfo.php @@ -22,6 +22,7 @@ use MediaWiki\Config\ServiceOptions; use MediaWiki\Linker\LinkTarget; +use MediaWiki\MediaWikiServices; /** * This is a utility class for dealing with namespaces that encodes all the "magic" behaviors of @@ -93,10 +94,8 @@ class NamespaceInfo { 'ExtraNamespaces', 'ExtraSignatureNamespaces', 'NamespaceContentModels', - 'NamespaceProtection', 'NamespacesWithSubpages', 'NonincludableNamespaces', - 'RestrictionLevels', ]; /** @@ -572,82 +571,18 @@ class NamespaceInfo { * Determine which restriction levels it makes sense to use in a namespace, * optionally filtered by a user's rights. * - * @todo Move this to PermissionManager and remove the dependency here on permissions-related - * config settings. - * + * @deprecated since 1.34 User PermissionManager::getNamespaceRestrictionLevels instead. * @param int $index Index to check * @param User|null $user User to check * @return array */ public function getRestrictionLevels( $index, User $user = null ) { - if ( !isset( $this->options->get( 'NamespaceProtection' )[$index] ) ) { - // All levels are valid if there's no namespace restriction. - // But still filter by user, if necessary - $levels = $this->options->get( 'RestrictionLevels' ); - if ( $user ) { - $levels = array_values( array_filter( $levels, function ( $level ) use ( $user ) { - $right = $level; - if ( $right == 'sysop' ) { - $right = 'editprotected'; // BC - } - if ( $right == 'autoconfirmed' ) { - $right = 'editsemiprotected'; // BC - } - return ( $right == '' || $user->isAllowed( $right ) ); - } ) ); - } - return $levels; - } - - // $wgNamespaceProtection can require one or more rights to edit the namespace, which - // may be satisfied by membership in multiple groups each giving a subset of those rights. - // A restriction level is redundant if, for any one of the namespace rights, all groups - // giving that right also give the restriction level's right. Or, conversely, a - // restriction level is not redundant if, for every namespace right, there's at least one - // group giving that right without the restriction level's right. - // - // First, for each right, get a list of groups with that right. - $namespaceRightGroups = []; - foreach ( (array)$this->options->get( 'NamespaceProtection' )[$index] as $right ) { - if ( $right == 'sysop' ) { - $right = 'editprotected'; // BC - } - if ( $right == 'autoconfirmed' ) { - $right = 'editsemiprotected'; // BC - } - if ( $right != '' ) { - $namespaceRightGroups[$right] = User::getGroupsWithPermission( $right ); - } - } - - // Now, go through the protection levels one by one. - $usableLevels = [ '' ]; - foreach ( $this->options->get( 'RestrictionLevels' ) as $level ) { - $right = $level; - if ( $right == 'sysop' ) { - $right = 'editprotected'; // BC - } - if ( $right == 'autoconfirmed' ) { - $right = 'editsemiprotected'; // BC - } - - if ( $right != '' && - !isset( $namespaceRightGroups[$right] ) && - ( !$user || $user->isAllowed( $right ) ) - ) { - // Do any of the namespace rights imply the restriction right? (see explanation above) - foreach ( $namespaceRightGroups as $groups ) { - if ( !array_diff( $groups, User::getGroupsWithPermission( $right ) ) ) { - // Yes, this one does. - continue 2; - } - } - // No, keep the restriction level - $usableLevels[] = $level; - } - } - - return $usableLevels; + // PermissionManager is not injected because adding an explicit dependency + // breaks MW installer by adding a dependency chain on the database before + // it was set up. Also, the method is deprecated and will be soon removed. + return MediaWikiServices::getInstance() + ->getPermissionManager() + ->getNamespaceRestrictionLevels( $index, $user ); } /** diff --git a/tests/phpunit/includes/Permissions/PermissionManagerTest.php b/tests/phpunit/includes/Permissions/PermissionManagerTest.php index 3c5f43bda8..88847e255d 100644 --- a/tests/phpunit/includes/Permissions/PermissionManagerTest.php +++ b/tests/phpunit/includes/Permissions/PermissionManagerTest.php @@ -15,6 +15,7 @@ use MediaWiki\MediaWikiServices; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Revision\MutableRevisionRecord; use MediaWiki\Revision\RevisionLookup; +use MWException; use TestAllServiceOptionsUsed; use Wikimedia\ScopedCallback; use MediaWiki\Session\SessionId; @@ -738,7 +739,9 @@ class PermissionManagerTest extends MediaWikiLangTestCase { 'BlockDisablesLogin' => false, 'GroupPermissions' => [], 'RevokePermissions' => [], - 'AvailableRights' => [] + 'AvailableRights' => [], + 'NamespaceProtection' => [], + 'RestrictionLevels' => [] ] ), $services->getSpecialPageFactory(), @@ -1788,4 +1791,75 @@ class PermissionManagerTest extends MediaWikiLangTestCase { return $revision; } + public function provideGetRestrictionLevels() { + return [ + 'No namespace restriction' => [ [ '', 'autoconfirmed', 'sysop' ], NS_TALK ], + 'Restricted to autoconfirmed' => [ [ '', 'sysop' ], NS_MAIN ], + 'Restricted to sysop' => [ [ '' ], NS_USER ], + 'Restricted to someone in two groups' => [ [ '', 'sysop' ], 101 ], + 'No special permissions' => [ + [ '' ], + NS_TALK, + [] + ], + 'autoconfirmed' => [ + [ '', 'autoconfirmed' ], + NS_TALK, + [ 'autoconfirmed' ] + ], + 'autoconfirmed revoked' => [ + [ '' ], + NS_TALK, + [ 'autoconfirmed', 'noeditsemiprotected' ] + ], + 'sysop' => [ + [ '', 'autoconfirmed', 'sysop' ], + NS_TALK, + [ 'sysop' ] + ], + 'sysop with autoconfirmed revoked (a bit silly)' => [ + [ '', 'sysop' ], + NS_TALK, + [ 'sysop', 'noeditsemiprotected' ] + ], + ]; + } + + /** + * @dataProvider provideGetRestrictionLevels + * @covers \MediaWiki\Permissions\PermissionManager::getNamespaceRestrictionLevels + * + * @param array $expected + * @param int $ns + * @param array|null $userGroups + * @throws MWException + */ + public function testGetRestrictionLevels( array $expected, $ns, array $userGroups = null ) { + $this->setMwGlobals( [ + 'wgGroupPermissions' => [ + '*' => [ 'edit' => true ], + 'autoconfirmed' => [ 'editsemiprotected' => true ], + 'sysop' => [ + 'editsemiprotected' => true, + 'editprotected' => true, + ], + 'privileged' => [ 'privileged' => true ], + ], + 'wgRevokePermissions' => [ + 'noeditsemiprotected' => [ 'editsemiprotected' => true ], + ], + 'wgNamespaceProtection' => [ + NS_MAIN => 'autoconfirmed', + NS_USER => 'sysop', + 101 => [ 'editsemiprotected', 'privileged' ], + ], + 'wgRestrictionLevels' => [ '', 'autoconfirmed', 'sysop' ], + 'wgAutopromote' => [] + ] ); + $this->resetServices(); + $user = is_null( $userGroups ) ? null : $this->getTestUser( $userGroups )->getUser(); + $this->assertSame( $expected, MediaWikiServices::getInstance() + ->getPermissionManager() + ->getNamespaceRestrictionLevels( $ns, $user ) ); + } } diff --git a/tests/phpunit/includes/title/NamespaceInfoTest.php b/tests/phpunit/includes/title/NamespaceInfoTest.php index 028c43823d..7f97a16dfa 100644 --- a/tests/phpunit/includes/title/NamespaceInfoTest.php +++ b/tests/phpunit/includes/title/NamespaceInfoTest.php @@ -54,14 +54,12 @@ class NamespaceInfoTest extends MediaWikiTestCase { 'ExtraNamespaces' => [], 'ExtraSignatureNamespaces' => [], 'NamespaceContentModels' => [], - 'NamespaceProtection' => [], 'NamespacesWithSubpages' => [ NS_TALK => true, NS_USER => true, NS_USER_TALK => true, ], 'NonincludableNamespaces' => [], - 'RestrictionLevels' => [ '', 'autoconfirmed', 'sysop' ], ]; private function newObj( array $options = [] ) : NamespaceInfo { @@ -1245,53 +1243,17 @@ class NamespaceInfoTest extends MediaWikiTestCase { */ /** - * This mock user can only have isAllowed() called on it. - * - * @param array $groups Groups for the mock user to have - * @return User - */ - private function getMockUser( array $groups = [] ) : User { - $groups[] = '*'; - - $mock = $this->createMock( User::class ); - $mock->method( 'isAllowed' )->will( $this->returnCallback( - function ( $action ) use ( $groups ) { - global $wgGroupPermissions, $wgRevokePermissions; - if ( $action == '' ) { - return true; - } - foreach ( $wgRevokePermissions as $group => $rights ) { - if ( !in_array( $group, $groups ) ) { - continue; - } - if ( isset( $rights[$action] ) && $rights[$action] ) { - return false; - } - } - foreach ( $wgGroupPermissions as $group => $rights ) { - if ( !in_array( $group, $groups ) ) { - continue; - } - if ( isset( $rights[$action] ) && $rights[$action] ) { - return true; - } - } - return false; - } - ) ); - $mock->expects( $this->never() )->method( $this->anythingBut( 'isAllowed' ) ); - return $mock; - } - - /** + * TODO: This is superceeded by PermissionManagerTest::testGetNamespaceRestrictionLevels + * Remove when deprecated method is removed. * @dataProvider provideGetRestrictionLevels - * @covers NamespaceInfo::getRestrictionLevels + * @covers NamespaceInfo::getRestrictionLevels * * @param array $expected * @param int $ns - * @param User|null $user + * @param array|null $groups + * @throws MWException */ - public function testGetRestrictionLevels( array $expected, $ns, User $user = null ) { + public function testGetRestrictionLevels( array $expected, $ns, array $groups = null ) { $this->setMwGlobals( [ 'wgGroupPermissions' => [ '*' => [ 'edit' => true ], @@ -1305,14 +1267,17 @@ class NamespaceInfoTest extends MediaWikiTestCase { 'wgRevokePermissions' => [ 'noeditsemiprotected' => [ 'editsemiprotected' => true ], ], - ] ); - $obj = $this->newObj( [ - 'NamespaceProtection' => [ + 'wgNamespaceProtection' => [ NS_MAIN => 'autoconfirmed', NS_USER => 'sysop', 101 => [ 'editsemiprotected', 'privileged' ], ], + 'wgRestrictionLevels' => [ '', 'autoconfirmed', 'sysop' ], + 'wgAutopromote' => [] ] ); + $this->resetServices(); + $obj = $this->newObj(); + $user = is_null( $groups ) ? null : $this->getTestUser( $groups )->getUser(); $this->assertSame( $expected, $obj->getRestrictionLevels( $ns, $user ) ); } @@ -1322,26 +1287,26 @@ class NamespaceInfoTest extends MediaWikiTestCase { 'Restricted to autoconfirmed' => [ [ '', 'sysop' ], NS_MAIN ], 'Restricted to sysop' => [ [ '' ], NS_USER ], 'Restricted to someone in two groups' => [ [ '', 'sysop' ], 101 ], - 'No special permissions' => [ [ '' ], NS_TALK, $this->getMockUser() ], + 'No special permissions' => [ [ '' ], NS_TALK, [] ], 'autoconfirmed' => [ [ '', 'autoconfirmed' ], NS_TALK, - $this->getMockUser( [ 'autoconfirmed' ] ) + [ 'autoconfirmed' ] ], 'autoconfirmed revoked' => [ [ '' ], NS_TALK, - $this->getMockUser( [ 'autoconfirmed', 'noeditsemiprotected' ] ) + [ 'autoconfirmed', 'noeditsemiprotected' ] ], 'sysop' => [ [ '', 'autoconfirmed', 'sysop' ], NS_TALK, - $this->getMockUser( [ 'sysop' ] ) + [ 'sysop' ] ], 'sysop with autoconfirmed revoked (a bit silly)' => [ [ '', 'sysop' ], NS_TALK, - $this->getMockUser( [ 'sysop', 'noeditsemiprotected' ] ) + [ 'sysop', 'noeditsemiprotected' ] ], ]; } -- 2.20.1