SpecialUserrights::userCanChangeRights is a funky method that is
called by things outside of SpecialUserrights to see if they should
display a link to it. It had some bugs and missing documentation.
* SpecialUserrights::userCanChangeRights relied on $this->isself to
see if the acting user and target user are the same, but it gets set
in execute() which never runs when used like this.
* It wasn't clear whether the $user parameter represented the acting
user or the target user, resulting in incorrect usage in
SkinTemplate.
The net effect of these bugs is that skin sidebar would display
"Change user groups" instead of "View user groups" for all users when
the current user was only allowed to change some of their own groups.
Change-Id: Ie47b9c7463b373fe17006567239aa09e824b015d
if ( !$user->isAnon() ) {
$sur = new UserrightsPage;
$sur->setContext( $this->getContext() );
if ( !$user->isAnon() ) {
$sur = new UserrightsPage;
$sur->setContext( $this->getContext() );
- $canChange = $sur->userCanChangeRights( $this->getUser(), false );
+ $canChange = $sur->userCanChangeRights( $user );
$nav_urls['userrights'] = [
'text' => $this->msg(
$canChange ? 'tool-link-userrights' : 'tool-link-userrights-readonly',
$nav_urls['userrights'] = [
'text' => $this->msg(
$canChange ? 'tool-link-userrights' : 'tool-link-userrights-readonly',
- * @param User $user
- * @param bool $checkIfSelf
+ * Check whether the current user (from context) can change the target user's rights.
+ *
+ * @param User $targetUser User whose rights are being changed
+ * @param bool $checkIfSelf If false, assume that the current user can add/remove groups defined
+ * in $wgGroupsAddToSelf / $wgGroupsRemoveFromSelf, without checking if it's the same as target
+ * user
- public function userCanChangeRights( $user, $checkIfSelf = true ) {
+ public function userCanChangeRights( $targetUser, $checkIfSelf = true ) {
+ $isself = $this->getUser()->equals( $targetUser );
+
$available = $this->changeableGroups();
$available = $this->changeableGroups();
- if ( $user->getId() == 0 ) {
+ if ( $targetUser->getId() == 0 ) {
return false;
}
return !empty( $available['add'] )
|| !empty( $available['remove'] )
return false;
}
return !empty( $available['add'] )
|| !empty( $available['remove'] )
- || ( ( $this->isself || !$checkIfSelf ) &&
+ || ( ( $isself || !$checkIfSelf ) &&
( !empty( $available['add-self'] )
|| !empty( $available['remove-self'] ) ) );
}
( !empty( $available['add-self'] )
|| !empty( $available['remove-self'] ) ) );
}