Page MenuHomePhabricator

Special:GWToolset does not check permissions correctly
Closed, ResolvedPublic

Description

It appears that Special:GWToolset is checking for membership in the gwtoolset user group rather than checking for the actual gwtoolset user right. So, for example, an admin that has the gwtoolset user right, but isn't in the gwtoolset user group will not be given access to Special:GWToolset.


Version: unspecified
Severity: minor

Details

Reference
bz58602

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:30 AM
bzimport set Reference to bz58602.
bzimport added a subscriber: Unknown Object (MLST).

The short discussion on Commons leads me to believe that the access to the tool should indeed by controlled by user rights rather than user groups.

similar to:

  • bug 58603
  • bug 58607

those bugs have been committed to commons and seem to have resolved the initial ‘right’ issue admins were experiencing. it is true that the extension is using the group and not the right to determine access to the extension, but this is by design.

there is currently a discussion at https://commons.wikimedia.org/wiki/Commons:Administrators%27_noticeboard#GWToolset regarding who should actually have the ability to add/remove this group from a user’s account. once that has been decided the necessary config changes can be made.

These are all unrelated things, Dan.

It is my understanding that extensions should generally use user rights rather than user groups to determine access (after watching a discussion between kaldari and Reedy on #wikimedia-whatever).

In this particular case, there is little difference because the gwtoolset user group is the only one which has the gwtoolset user right (at least in the Wikimedia universe), but I think it's now more about code quality and conventions as well as about the third-party potential.

The discussion on Commons is irrelevant to this; we're talking about the extension code, not about local per-wiki settings, which from now on will be defined in https://noc.wikimedia.org/conf/highlight.php?file=InitialiseSettings.php, and not directly in the extension as it was before bug 58607 got fixed.

(In reply to comment #3)

In this particular case, there is little difference because the gwtoolset
user
group is the only one which has the gwtoolset user right (at least in the
Wikimedia universe), but I think it's now more about code quality and
conventions as well as about the third-party potential.

Yes, MediaWiki is designed under the assumption that groups are for organization only, and a group does not give extra abilities to the user except for whatever rights are contained in the group. There should be no difference between someone in the gwtoolset group and somebody in another group with a different name and the same rights.

okay, i think i understand this issue better now.

• our use case is to only allow certain users access to the extension.

in order to achieve this i originally thought that i needed to add a new group and then test against that new group.

what i understand now is that i need to add a new group, and a new right, which have already been done, and test against the new right only. only users who are part of this new group, gwtoolset, will have the right, gwtoolset assigned to them and thus access to the extension. is this what this bug is getting at?

(In reply to comment #5)

okay, i think i understand this issue better now.

• our use case is to only allow certain users access to the extension.

in order to achieve this i originally thought that i needed to add a new
group
and then test against that new group.

what i understand now is that i need to add a new group, and a new right,
which
have already been done, and test against the new right only. only users who
are
part of this new group, gwtoolset, will have the right, gwtoolset assigned to
them and thus access to the extension. is this what this bug is getting at?

Yep

Change 102814 had a related patch set uploaded by Dan-nl:
correcting gwtoolset permission check

https://gerrit.wikimedia.org/r/102814

Change 102814 merged by Dan-nl:
correcting gwtoolset permission check

https://gerrit.wikimedia.org/r/102814

(In reply to comment #8)

Change 102814 merged by Dan-nl:
correcting gwtoolset permission check

https://gerrit.wikimedia.org/r/102814

Is this bug resolved/fixed, then?

ryan,

the patch has been merged into the master branch and deployed to Commons.
you can see the results when you are not logged into commons and go to
http://commons.wikimedia.org/wiki/Special:GWToolset.

please close this ticket if you’re satisfied with the results. if not, please
indicate what else needs to be addressed.

This bug appears to have been fixed; the extension now checks whether the user in question has the 'gwtoolset' user right instead of checking for their membership in the 'gwtoolset' user group. Please feel to reopen if you don't like it.

Gilles raised the priority of this task from Medium to Unbreak Now!.Dec 4 2014, 10:24 AM
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to Medium.Dec 4 2014, 11:21 AM