Page MenuHomePhabricator

Redundancies in user group rights
Closed, ResolvedPublic

Description

Author: zigger

Description:
loadFromDatabase() in User.php seems to apply '*' and 'user' group rights to
logged-in users.
This would mean that there are redundant assignments in the default
$wgGroupPermissions in DefaultSettings.php for the 'user' group ('read') and the
'sysop' group ('createaccount', 'move', 'upload').

$wgAvailableRights includes an 'undelete' right which is not needed as
SpecialUndelete.php is registered to use the 'delete' right as the restriction.

The deprecated isBureaucrat() (for cross-version extensions?) in User.php checks
the right 'makesysop' which does not exist in $wgAvailableRights or the default
$wgGroupPermissions.

The message 'makesysop' also seems to be redundant in the language files.

Also, some Special pages have checks on the relevant right (e.g.
SpecialLockdb.php, SpecialBlockip.php), but other do not (e.g.
SpecialUndelete.php, SpecialUserrights.php).

SpecialGroups.php still seems to exist in CVS but is not currently used, and
$wgAvailableRights still includes 'grouprights'.


Version: 1.5.x
Severity: trivial

Details

Reference
bz2498

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.
StatusSubtypeAssignedTask
ResolvedNone
ResolvedNone

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 8:34 PM
bzimport set Reference to bz2498.
bzimport added a subscriber: Unknown Object (MLST).

Best to break these into separate bugs where applicable...

(In reply to comment #0)

loadFromDatabase() in User.php seems to apply '*' and 'user' group rights to
logged-in users.
This would mean that there are redundant assignments in the default
$wgGroupPermissions in DefaultSettings.php for the 'user' group ('read') and the
'sysop' group ('createaccount', 'move', 'upload').

These are intentional for ease of configuration, so you can block out those rights at one level
without explicitly adding it back in.

$wgAvailableRights includes an 'undelete' right which is not needed as
SpecialUndelete.php is registered to use the 'delete' right as the restriction.

Perhaps it should be used, then...

The deprecated isBureaucrat() (for cross-version extensions?) in User.php checks
the right 'makesysop' which does not exist in $wgAvailableRights or the default
$wgGroupPermissions.

isBureaucrat() should be removed. If anything uses it it should be replaced with an appropriate
check.

The message 'makesysop' also seems to be redundant in the language files.

If used should be moved to the extension's localization files. (We ought to have a clean
standard system for that, too.)

Also, some Special pages have checks on the relevant right (e.g.
SpecialLockdb.php, SpecialBlockip.php), but other do not (e.g.
SpecialUndelete.php, SpecialUserrights.php).

The definition of the special page in SpecialPage.php will specify a permission required to use
that page; the page itself only needs to check if there is different behavior under different
available permission levels.

SpecialGroups.php still seems to exist in CVS but is not currently used, and
$wgAvailableRights still includes 'grouprights'.

Should probably be removed; if we re-add it it can be recovered from CVS history.

zigger wrote:

(In reply to comment #1)

Best to break these into separate bugs where applicable...

Okay. I'll resolve this bug as invalid once all the relevant individual bugs
are re-entered.

...
> $wgAvailableRights includes an 'undelete' right which is not needed as
> SpecialUndelete.php is registered to use the 'delete' right as the restriction.

Perhaps it should be used, then...
...

Entered as bug 2500 for further discussion.

zigger wrote:

Ævar removed 'undelete' from $wgAvailableRights in Defines.php CVS v1.34 a few
days ago.

removed User::isBureaucrat , User::Developer , User::isSysop with r14158
they give a backtrace as of v1.7. Should be removed in v1.8

robchur wrote:

Most of this seems to have been addressed now.