Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "Move User::changeable(By)Groups methods to UserGroupManager"

Browse source
This commit is contained in:
jenkins-bot 2021-05-27 17:17:34 +00:00 committed by Gerrit Code Review
commit 93fd347463
5 changed files with 260 additions and 119 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
RELEASE-NOTES-1.37
View file

@ -338,6 +338,8 @@ because of Phabricator reports.
* wfLocalFile(), deprecated since 1.34, now emits deprecation warnings.
* wfFindFile(), deprecated since 1.34, now emits deprecation warnings.
* User::getRights(), deprecated since 1.34, now emits deprecation warnings.
* User::changeableGroups and ::changeableByGroup were deprecated, use
corresponding methods in UserGroupManager instead.
* …
=== Other changes in 1.37 ===

105
includes/user/User.php
View file

@ -4067,72 +4067,12 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
* 'remove' => [ removablegroups ],
* 'add-self' => [ addablegroups to self ],
* 'remove-self' => [ removable groups from self ] ]
* @suppress PhanTypeComparisonFromArray False positives with $wgGroupsAddToSelf
* @deprecated since 1.37 Use UserGroupManager::getGroupsChangeableByGroup instead.
*/
public static function changeableByGroup( $group ) {
global $wgAddGroups, $wgRemoveGroups, $wgGroupsAddToSelf, $wgGroupsRemoveFromSelf;
$groups = [
'add' => [],
'remove' => [],
'add-self' => [],
'remove-self' => []
];
if ( empty( $wgAddGroups[$group] ) ) {
// Don't add anything to $groups
} elseif ( $wgAddGroups[$group] === true ) {
// You get everything
$groups['add'] = self::getAllGroups();
} elseif ( is_array( $wgAddGroups[$group] ) ) {
$groups['add'] = $wgAddGroups[$group];
}
// Same thing for remove
if ( empty( $wgRemoveGroups[$group] ) ) {
// Do nothing
} elseif ( $wgRemoveGroups[$group] === true ) {
$groups['remove'] = self::getAllGroups();
} elseif ( is_array( $wgRemoveGroups[$group] ) ) {
$groups['remove'] = $wgRemoveGroups[$group];
}
// Re-map numeric keys of AddToSelf/RemoveFromSelf to the 'user' key for backwards compatibility
if ( empty( $wgGroupsAddToSelf['user'] ) || $wgGroupsAddToSelf['user'] !== true ) {
foreach ( $wgGroupsAddToSelf as $key => $value ) {
if ( is_int( $key ) ) {
$wgGroupsAddToSelf['user'][] = $value;
}
}
}
if ( empty( $wgGroupsRemoveFromSelf['user'] ) || $wgGroupsRemoveFromSelf['user'] !== true ) {
foreach ( $wgGroupsRemoveFromSelf as $key => $value ) {
if ( is_int( $key ) ) {
$wgGroupsRemoveFromSelf['user'][] = $value;
}
}
}
// Now figure out what groups the user can add to him/herself
if ( empty( $wgGroupsAddToSelf[$group] ) ) {
// Do nothing
} elseif ( $wgGroupsAddToSelf[$group] === true ) {
// No idea WHY this would be used, but it's there
$groups['add-self'] = self::getAllGroups();
} elseif ( is_array( $wgGroupsAddToSelf[$group] ) ) {
$groups['add-self'] = $wgGroupsAddToSelf[$group];
}
if ( empty( $wgGroupsRemoveFromSelf[$group] ) ) {
// Do nothing
} elseif ( $wgGroupsRemoveFromSelf[$group] === true ) {
$groups['remove-self'] = self::getAllGroups();
} elseif ( is_array( $wgGroupsRemoveFromSelf[$group] ) ) {
$groups['remove-self'] = $wgGroupsRemoveFromSelf[$group];
}
return $groups;
return MediaWikiServices::getInstance()
->getUserGroupManager()
->getGroupsChangeableByGroup( $group );
}
/**
@ -4141,41 +4081,12 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
* 'remove' => [ removablegroups ],
* 'add-self' => [ addablegroups to self ],
* 'remove-self' => [ removable groups from self ] ]
* @deprecated since 1.37 Use UserGroupManager::getGroupsChangeableBy instead.
*/
public function changeableGroups() {
if ( $this->isAllowed( 'userrights' ) ) {
// This group gives the right to modify everything (reverse-
// compatibility with old "userrights lets you change
// everything")
// Using array_merge to make the groups reindexed
$all = array_merge( self::getAllGroups() );
return [
'add' => $all,
'remove' => $all,
'add-self' => [],
'remove-self' => []
];
}
// Okay, it's not so simple, we will have to go through the arrays
$groups = [
'add' => [],
'remove' => [],
'add-self' => [],
'remove-self' => []
];
$addergroups = $this->getEffectiveGroups();
foreach ( $addergroups as $addergroup ) {
$groups = array_merge_recursive(
$groups, $this->changeableByGroup( $addergroup )
);
$groups['add'] = array_unique( $groups['add'] );
$groups['remove'] = array_unique( $groups['remove'] );
$groups['add-self'] = array_unique( $groups['add-self'] );
$groups['remove-self'] = array_unique( $groups['remove-self'] );
}
return $groups;
return MediaWikiServices::getInstance()
->getUserGroupManager()
->getGroupsChangeableBy( $this );
}
/**

98
includes/user/UserGroupManager.php
View file

@ -30,6 +30,7 @@ use ManualLogEntry;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\HookContainer\HookContainer;
use MediaWiki\HookContainer\HookRunner;
use MediaWiki\Permissions\Authority;
use MediaWiki\Permissions\GroupPermissionsLookup;
use Psr\Log\LoggerInterface;
use ReadOnlyMode;
@ -54,13 +55,17 @@ class UserGroupManager implements IDBAccessObject {
* @internal For use by ServiceWiring
*/
public const CONSTRUCTOR_OPTIONS = [
'AddGroups',
'Autopromote',
'AutopromoteOnce',
'AutopromoteOnceLogInRC',
'EmailAuthentication',
'ImplicitGroups',
'GroupPermissions',
'GroupsAddToSelf',
'GroupsRemoveFromSelf',
'RevokePermissions',
'RemoveGroups',
];
/** @var ServiceOptions */
@ -973,6 +978,99 @@ class UserGroupManager implements IDBAccessObject {
return $purgedRows;
}
/**
* @param array $config
* @param string $group
* @return string[]
*/
private function expandChangeableGroupConfig( array $config, string $group ): array {
if ( empty( $config[$group] ) ) {
return [];
} elseif ( $config[$group] === true ) {
// You get everything
return $this->listAllGroups();
} elseif ( is_array( $config[$group] ) ) {
return $config[$group];
}
return [];
}
/**
* Returns an array of the groups that a particular group can add/remove.
*
* @since 1.37
* @param string $group The group to check for whether it can add/remove
* @return array [
* 'add' => [ addablegroups ],
* 'remove' => [ removablegroups ],
* 'add-self' => [ addablegroups to self ],
* 'remove-self' => [ removable groups from self ] ]
*/
public function getGroupsChangeableByGroup( string $group ): array {
return [
'add' => $this->expandChangeableGroupConfig(
$this->options->get( 'AddGroups' ), $group
),
'remove' => $this->expandChangeableGroupConfig(
$this->options->get( 'RemoveGroups' ), $group
),
'add-self' => $this->expandChangeableGroupConfig(
$this->options->get( 'GroupsAddToSelf' ), $group
),
'remove-self' => $this->expandChangeableGroupConfig(
$this->options->get( 'GroupsRemoveFromSelf' ), $group
),
];
}
/**
* Returns an array of groups that this $actor can add and remove.
*
* @since 1.37
* @param Authority $authority
* @return array [
* 'add' => [ addablegroups ],
* 'remove' => [ removablegroups ],
* 'add-self' => [ addablegroups to self ],
* 'remove-self' => [ removable groups from self ]
* ]
*/
public function getGroupsChangeableBy( Authority $authority ): array {
if ( $authority->isAllowed( 'userrights' ) ) {
// This group gives the right to modify everything (reverse-
// compatibility with old "userrights lets you change
// everything")
// Using array_merge to make the groups reindexed
$all = array_merge( $this->listAllGroups() );
return [
'add' => $all,
'remove' => $all,
'add-self' => [],
'remove-self' => []
];
}
// Okay, it's not so simple, we will have to go through the arrays
$groups = [
'add' => [],
'remove' => [],
'add-self' => [],
'remove-self' => []
];
$actorGroups = $this->getUserEffectiveGroups( $authority->getUser() );
foreach ( $actorGroups as $actorGroup ) {
$groups = array_merge_recursive(
$groups, $this->getGroupsChangeableByGroup( $actorGroup )
);
$groups['add'] = array_unique( $groups['add'] );
$groups['remove'] = array_unique( $groups['remove'] );
$groups['add-self'] = array_unique( $groups['add-self'] );
$groups['remove-self'] = array_unique( $groups['remove-self'] );
}
return $groups;
}
/**
* Cleans cached group memberships for a given user
*

103
tests/phpunit/includes/user/UserGroupManagerTest.php
View file

@ -26,6 +26,7 @@ use LogEntryBase;
use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\MediaWikiServices;
use MediaWiki\Permissions\SimpleAuthority;
use MediaWiki\Session\PHPSessionHandler;
use MediaWiki\Session\SessionManager;
use MediaWiki\User\UserEditTracker;
@ -68,6 +69,7 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
UserGroupManager::CONSTRUCTOR_OPTIONS,
$configOverrides,
[
'AddGroups' => [],
'Autopromote' => [
'autoconfirmed' => [ APCOND_EDITCOUNT, 0 ]
],
@ -77,7 +79,10 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
'runtest' => true,
]
],
'GroupsAddToSelf' => [],
'GroupsRemoveFromSelf' => [],
'ImplicitGroups' => [ '*', 'user', 'autoconfirmed' ],
'RemoveGroups' => [],
'RevokePermissions' => [],
],
$services->getMainConfig()
@ -1053,4 +1058,102 @@ class UserGroupManagerTest extends MediaWikiIntegrationTestCase {
] ]
);
}
private const CHANGEABLE_GROUPS_TEST_CONFIG = [
'GroupPermissions' => [],
'AddGroups' => [
'sysop' => [ 'rollback' ],
'bureaucrat' => [ 'sysop', 'bureaucrat' ],
],
'RemoveGroups' => [
'sysop' => [ 'rollback' ],
'bureaucrat' => [ 'sysop' ],
],
'GroupsAddToSelf' => [
'sysop' => [ 'flood' ],
],
'GroupsRemoveFromSelf' => [
'flood' => [ 'flood' ],
],
];
private function assertGroupsEquals( array $expected, array $actual ) {
// assertArrayEquals can compare without requiring the same order,
// but the elements of an array are still required to be in the same order,
// so just compare each element
$this->assertArrayEquals( $expected['add'], $actual['add'], 'Add must match' );
$this->assertArrayEquals( $expected['remove'], $actual['remove'], 'Remove must match' );
$this->assertArrayEquals( $expected['add-self'], $actual['add-self'], 'Add-self must match' );
$this->assertArrayEquals( $expected['remove-self'], $actual['remove-self'], 'Remove-self must match' );
}
/**
* @covers \MediaWiki\User\UserGroupManager::getGroupsChangeableBy
*/
public function testChangeableGroups() {
$manager = $this->getManager( self::CHANGEABLE_GROUPS_TEST_CONFIG );
$allGroups = $manager->listAllGroups();
$user = $this->getTestUser()->getUser();
$changeableGroups = $manager->getGroupsChangeableBy( new SimpleAuthority( $user, [ 'userrights' ] ) );
$this->assertGroupsEquals(
[
'add' => $allGroups,
'remove' => $allGroups,
'add-self' => [],
'remove-self' => [],
],
$changeableGroups
);
$user = $this->getTestUser( [ 'bureaucrat', 'sysop' ] )->getUser();
$changeableGroups = $manager->getGroupsChangeableBy( new SimpleAuthority( $user, [] ) );
$this->assertGroupsEquals(
[
'add' => [ 'sysop', 'bureaucrat', 'rollback' ],
'remove' => [ 'sysop', 'rollback' ],
'add-self' => [ 'flood' ],
'remove-self' => [],
],
$changeableGroups
);
$user = $this->getTestUser( [ 'flood' ] )->getUser();
$changeableGroups = $manager->getGroupsChangeableBy( new SimpleAuthority( $user, [] ) );
$this->assertGroupsEquals(
[
'add' => [],
'remove' => [],
'add-self' => [],
'remove-self' => [ 'flood' ],
],
$changeableGroups
);
}
public function provideChangeableByGroup() {
yield 'sysop' => [ 'sysop', [
'add' => [ 'rollback' ],
'remove' => [ 'rollback' ],
'add-self' => [ 'flood' ],
'remove-self' => [],
] ];
yield 'flood' => [ 'flood', [
'add' => [],
'remove' => [],
'add-self' => [],
'remove-self' => [ 'flood' ],
] ];
}
/**
* @dataProvider provideChangeableByGroup
* @covers \MediaWiki\User\UserGroupManager::getGroupsChangeableByGroup
* @param string $group
* @param array $expected
*/
public function testChangeableByGroup( string $group, array $expected ) {
$manager = $this->getManager( self::CHANGEABLE_GROUPS_TEST_CONFIG );
$this->assertGroupsEquals( $expected, $manager->getGroupsChangeableByGroup( $group ) );
}
}

71
tests/phpunit/includes/user/UserTest.php
View file

@ -1946,32 +1946,33 @@ class UserTest extends MediaWikiIntegrationTestCase {
$this->assertSame( [ 'test3' ], $user->getGroups(), 'Hooks can stop removal of a group' );
}
private const CHANGEABLE_GROUPS_TEST_CONFIG = [
'wgGroupPermissions' => [
'doEverything' => [
'userrights' => true,
],
],
'wgAddGroups' => [
'sysop' => [ 'rollback' ],
'bureaucrat' => [ 'sysop', 'bureaucrat' ],
],
'wgRemoveGroups' => [
'sysop' => [ 'rollback' ],
'bureaucrat' => [ 'sysop' ],
],
'wgGroupsAddToSelf' => [
'sysop' => [ 'flood' ],
],
'wgGroupsRemoveFromSelf' => [
'flood' => [ 'flood' ],
],
];
/**
* @covers User::changeableGroups
*/
public function testChangeableGroups() {
// todo: test changeableByGroup here as well
$this->setMwGlobals( [
'wgGroupPermissions' => [
'doEverything' => [
'userrights' => true,
],
],
'wgAddGroups' => [
'sysop' => [ 'rollback' ],
'bureaucrat' => [ 'sysop', 'bureaucrat' ],
],
'wgRemoveGroups' => [
'sysop' => [ 'rollback' ],
'bureaucrat' => [ 'sysop' ],
],
'wgGroupsAddToSelf' => [
'sysop' => [ 'flood' ],
],
'wgGroupsRemoveFromSelf' => [
'flood' => [ 'flood' ],
],
] );
$this->setMwGlobals( self::CHANGEABLE_GROUPS_TEST_CONFIG );
$allGroups = User::getAllGroups();
@ -2012,6 +2013,32 @@ class UserTest extends MediaWikiIntegrationTestCase {
);
}
public function provideChangeableByGroup() {
yield 'sysop' => [ 'sysop', [
'add' => [ 'rollback' ],
'remove' => [ 'rollback' ],
'add-self' => [ 'flood' ],
'remove-self' => [],
] ];
yield 'flood' => [ 'flood', [
'add' => [],
'remove' => [],
'add-self' => [],
'remove-self' => [ 'flood' ],
] ];
}
/**
* @dataProvider provideChangeableByGroup
* @covers User::changeableByGroup
* @param string $group
* @param array $expected
*/
public function testChangeableByGroup( string $group, array $expected ) {
$this->setMwGlobals( self::CHANGEABLE_GROUPS_TEST_CONFIG );
$this->assertGroupsEquals( $expected, User::changeableByGroup( $group ) );
}
private function assertGroupsEquals( array $expected, array $actual ) {
// assertArrayEquals can compare without requiring the same order,
// but the elements of an array are still required to be in the same order,