Page MenuHomePhabricator

NamespaceInfo::getRestrictionLevels() does not correctly handle namespace restrictions that require more than one permission
Closed, ResolvedPublic

Description

NamespaceInfo::getRestrictionLevels() was added by Anomie in 8862991ce1b1 to not display the option to protect namespaces that are already only editable by privileged users. E.g., there's no point in protecting a page in the MediaWiki namespace so only sysops can edit it if only sysops can already edit the whole namespace.

To compute which levels should be displayed for the namespace, it would get a list of groups that could edit the namespace. For each possible restriction level, it would check if any group existed that would be able to edit the namespace if the page was not protected, but would be stopped by the protection. If it found no group for a given restriction level, it would omit that level from the UI.

The problem is that namespaces can be protected such that you need multiple different permissions to edit them. (I don't know if anyone uses this, but it should be done correctly if we want to support it.) This means that there might be a combination of groups whereby no single group allows editing the namespace, but all of them together do, and would be stopped by the protection. But the current code just sees that no one of the groups allows editing the namespace, so it thinks the protection level isn't useful.

The correct solution is to check if for some permission that's required to edit the namespace, every group with that permission also has permission to overcome the protection. If so, the protection won't do anything. Inversely, if for every permission that's required to edit the namespace, there exists some group with that permission that does not have permission to overcome the protection, the protection is potentially useful.

I wrote a patch for this, which I will link with this bug shortly. Daniel asked that I file this issue to clarify what's happening here.

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 6 2019, 10:03 AM

Change 508279 had a related patch set uploaded (by simetrical; owner: simetrical):
[mediawiki/core@master] Fix logic in NamespaceInfo::getRestrictionLevels

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

daniel added a comment.May 7 2019, 2:35 PM

Pinging @Anomie and @tstarling for input.

Change 508279 merged by jenkins-bot:
[mediawiki/core@master] Fix logic in NamespaceInfo::getRestrictionLevels

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

daniel closed this task as Resolved.May 22 2019, 10:10 AM
sbassett moved this task from Incoming to Done on the Security-Team board.Jun 11 2019, 6:10 PM