Page MenuHomePhabricator

Replace specific PermissionManager calls with GroupPermissionsLookup calls in all deployed extensions
Closed, ResolvedPublic

Description

The following replacements should be made in all deployed extensions

  1. PermissionManager::groupHasPermission -> GroupPermissionsLookup::groupHasPermission
  2. PermissionManager::getGroupPermissions -> GroupPermissionsLookup::getGroupPermissions
  3. PermissionManager::getGroupsWithPermission -> GroupPermissionsLookup::getGroupsWithPermission

After that, the methods in PermissionManager should be hard-deprecated.

Event Timeline

Pchelolo created this task.Jan 8 2021, 11:10 PM
Reedy added a subscriber: Reedy.Jan 8 2021, 11:29 PM

I note, this is going to cause more potential backport problems for the LTS.

We haven't even fully migrated to PermissionManager I think... And when we backport to 1.31 currently, we need to remember to replace permission manager calls with applicable calls (User::isAllowed() is still in master, not hard deprecated either, soft deprecated since 1.34).

So when PermissionManager methods are hard deprecated, that means we're using GroupPermissionsLookup everywhere applicable... But then it means for future bug reports, and likely security patches... GroupPermissionsLookup changes will need changing for PermissionMananger, or possibly User::isAllowed() (though REL1_31 goes EOL in the middle of this year.

It may be worth considering to see what we can backport of GroupPermissionsLookup into 1.35, even if it ends up being a thin wrapper as a compat layer...

The 3 methods in question are much less widely used then isAllowed, about 50 usages altogether, but sure, we can back port GroupPermissionsLookup, it's rather small.

Reedy renamed this task from Replace PermissionManager with GroupPermissionsLookup in all deployed extensions to Replace specific PermissionManager calls with GroupPermissionsLookup calls in all deployed extensions.Jan 8 2021, 11:44 PM
daniel added a comment.Jan 9 2021, 9:41 AM

Note that we are about to go full circle pn User::isAllowed with T231930

Change 656204 had a related patch set uploaded (by Peter.ovchyn; owner: Peter.ovchyn):
[mediawiki/extensions/CodeReview@master] Replace static User::getGroupsWithPermission in favour of GroupPermissionsLookup

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

Change 656904 had a related patch set uploaded (by Vlad.shapik; owner: Peter.ovchyn):
[mediawiki/extensions/CodeReview@master] Replace static User::getGroupsWithPermission in favour of GroupPermissionsLookup

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

Change 656904 abandoned by Vlad.shapik:
[mediawiki/extensions/CodeReview@master] Replace static User::getGroupsWithPermission in favour of GroupPermissionsLookup

Reason:

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

Pchelolo triaged this task as Medium priority.Jan 19 2021, 6:26 PM

Change 656204 merged by jenkins-bot:
[mediawiki/extensions/CodeReview@master] Replace static User::getGroupsWithPermission in favour of GroupPermissionsLookup

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

Pchelolo closed this task as Resolved.Mon, Feb 1, 6:38 PM