Page MenuHomePhabricator
Create Task
Maniphest T241503

Cannot revoke permissions from a global group if the permissions no longer exist
Closed, ResolvedPublic
Actions

Description

Following T211004: Differentiate private filters and private details in rights/methods/variables names, abusefilter-private and abusefilter-private-log were renamed to use privatedetails. While AbuseFilter provides backwards compatibility for user groups that are assigned rights under the former names, global groups that have the rights under the former names cannot have those rights removed, since they don't show up in the Special:GlobalGroupPermissions.

Groups affected:
Beta cluster: staff, stewards
Production: Ombudsmen, staff, stewards

Potential solution (short-term/this case):

  • Add the rights to $wgAvailableRights manually (as is already done for rights that don't exist on meta[1]) for a few minutes to allow for updating user groups

Ideal solution (long term):

  • In the Special:GlobalGroupPermissions interface, show rights that exist in $wgAvailableRights as well rights that don't exist anymore, but are granted to a global group, so that they may be removed

[1] See https://gerrit.wikimedia.org/r/plugins/gitiles/operations/mediawiki-config/+/c5cc284bd89f9de222803bec1c6cf93de42c4514/wmf-config/CommonSettings.php#880

Details

ProjectBranchLines +/-Subject
operations/mediawiki-configmaster+0 -3Revert "[Beta cluster] add a fake 'UselessRightForTesting' to available rights"
operations/mediawiki-configmaster+3 -0[Beta cluster] add a fake 'UselessRightForTesting' to available rights
mediawiki/extensions/CentralAuthmaster+12 -2Make it possible to remove rights that no longer exist
operations/mediawiki-configmaster+0 -6Revert "Temporarily add back old 'abusefilter-private(?:-log) permissions"
operations/mediawiki-configmaster+6 -0Temporarily add back old 'abusefilter-private(?:-log) permissions
Customize query in gerrit

Related Objects

Event Timeline

DannyS712 created this task.Dec 28 2019, 3:20 AM
Restricted Application added a project: User-DannyS712. · View Herald TranscriptDec 28 2019, 3:21 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 added projects: Wikimedia-Site-requests, AbuseFilter, Stewards-and-global-tools, MediaWiki-extensions-CentralAuth, MediaWiki-User-management.Dec 28 2019, 3:21 AM
DannyS712 moved this task from Backlog to Deployment and configuration on the AbuseFilter board.
DannyS712 moved this task from Backlog to User rights on the MediaWiki-User-management board.
DannyS712 moved this task from Unsorted to Reports on the User-DannyS712 board.
DannyS712 updated the task description. (Show Details)
DannyS712 added subscribers: Daimona, Huji.
MarcoAurelio added a subscriber: MarcoAurelio.Jan 1 2020, 7:47 PM
Urbanecm added a subscriber: Urbanecm.Jan 1 2020, 7:57 PM
MarcoAurelio triaged this task as High priority.Jan 1 2020, 8:49 PM

Probably preventing Ombudsmen and Staff accessing the private logs and thus impeding the performance of their respective duties. Please let me know if we can do this today. Thanks.

DannyS712 added a comment.Jan 2 2020, 12:05 AM
In T241503#5769510, @MarcoAurelio wrote:

Probably preventing Ombudsmen and Staff accessing the private logs and thus impeding the performance of their respective duties. Please let me know if we can do this today. Thanks.

This prohibits removing the old rights, but one can still give them the new rights, and the old ones are backwards compatible[1] anyway, so Ombudsmen and Staff should still have access

[1] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/AbuseFilter/+/9552c187cc38022492524ff454bbaab334481c12/includes/AbuseFilterHooks.php#48

MarcoAurelio lowered the priority of this task from High to Medium.Jan 2 2020, 5:08 PM
In T241503#5769693, @DannyS712 wrote:
In T241503#5769510, @MarcoAurelio wrote:

Probably preventing Ombudsmen and Staff accessing the private logs and thus impeding the performance of their respective duties. Please let me know if we can do this today. Thanks.

This prohibits removing the old rights, but one can still give them the new rights, and the old ones are backwards compatible[1] anyway, so Ombudsmen and Staff should still have access

[1] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/AbuseFilter/+/9552c187cc38022492524ff454bbaab334481c12/includes/AbuseFilterHooks.php#48

Thank you. So this is more a "cosmetic" change then (as they can still make use of the privs., etc.). While I could add the new ones now, I'd rather wait to be able to do the whole thing together :-)

MarcoAurelio added a project: Patch-For-Review.Jan 2 2020, 5:30 PM

T241503.patch1 KBDownload

DannyS712 added a comment.Jan 2 2020, 5:36 PM
In T241503#5771518, @MarcoAurelio wrote:

T241503.patch1 KBDownload

CR+1
Nitpick: 'Temporary add old abusefilter permissions' should probably be 'Temporarily...'

sbassett moved this task from Backlog / Other to Other WMF team on the acl*security board.Jan 2 2020, 5:41 PM
• chasemp added a project: Security-Team.Jan 2 2020, 7:35 PM
• chasemp moved this task from Incoming to Watching on the Security-Team board.
MarcoAurelio added a subscriber: sbassett.Jan 2 2020, 7:42 PM
In T241503#5771552, @DannyS712 wrote:
In T241503#5771518, @MarcoAurelio wrote:

T241503.patch1 KBDownload

CR+1
Nitpick: 'Temporary add old abusefilter permissions' should probably be 'Temporarily...'

Will fix. After a chat with @sbassett this and it's revert patch will go through gerrit.

sbassett added a subscriber: Reedy.Jan 2 2020, 7:58 PM

The above patch has been deployed. Please ping @Reedy or I when revert is ready, thanks.

DannyS712 added a comment.Jan 2 2020, 8:12 PM

Revert at https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/561691/

Beta cluster done: https://deployment.wikipedia.beta.wmflabs.org/wiki/Special:GlobalGroupPermissions/staff and https://deployment.wikipedia.beta.wmflabs.org/wiki/Special:GlobalGroupPermissions/steward
Production done: https://meta.wikimedia.org/wiki/Special:GlobalGroupPermissions/steward, https://meta.wikimedia.org/wiki/Special:GlobalGroupPermissions/staff, https://meta.wikimedia.org/wiki/Special:GlobalGroupPermissions/ombudsman

DannyS712 added a comment.Jan 2 2020, 8:15 PM
In T241503#5772050, @sbassett wrote:

The above patch has been deployed. Please ping @Reedy or I when revert is ready, thanks.

Ping

DannyS712 removed a project: Patch-For-Review.EditedJan 2 2020, 8:28 PM

Config change merged, revert merged and deployed[1]

Having accomplished the short-term fixing of global group permissions following this specific renaming of rights,
We should start investigating the long-term solution if a global group is granted rights that no longer exist: should they be shown? removed automatically by maintenance script?

[1] https://tools.wmflabs.org/sal/log/AW9n89y9vrJzePItAP2R

MarcoAurelio added a comment.Jan 2 2020, 8:32 PM

Thanks @sbassett for your help and to @DannyS712 for double-checking everything went to plan.

sbassett added a comment.EditedJan 2 2020, 8:35 PM

The revert has now been deployed:

psyshell
>>> echo $wgAvailableRights['abusefilter-private'];
PHP Notice:  Undefined index
>>> echo $wgAvailableRights['abusefilter-private-log'];
PHP Notice:  Undefined index
In T241503#5772143, @DannyS712 wrote:

Having accomplished the short-term fixing of global group permissions following this specific renaming of rights,
We should start investigating the long-term solution if a global group is granted rights that no longer exist: should they be shown? removed automatically by maintenance script?

Do we want to keep this task open and private or create a new task and resolve this one and make it public? I'd probably prefer the latter, but either is fine.

DannyS712 added a comment.Jan 2 2020, 11:26 PM
In T241503#5772155, @sbassett wrote:

The revert has now been deployed:

>>> echo $wgAvailableRights['abusefilter-private'];
PHP Notice:  Undefined index
>>> echo $wgAvailableRights['abusefilter-private-log'];
PHP Notice:  Undefined index
In T241503#5772143, @DannyS712 wrote:

Having accomplished the short-term fixing of global group permissions following this specific renaming of rights,
We should start investigating the long-term solution if a global group is granted rights that no longer exist: should they be shown? removed automatically by maintenance script?

Do we want to keep this task open and private or create a new task and resolve this one and make it public? I'd probably prefer the latter, but either is fine.

Does this need to remain private?

sbassett updated the task description. (Show Details)Jan 3 2020, 3:23 PM
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
In T241503#5772755, @DannyS712 wrote:

Does this need to remain private?

No. I just made it public since the short-term solution was completed. And added some checkboxes for clarification of what's still being worked upon.

• chasemp added a project: Security.Feb 10 2020, 10:56 PM
• chasemp removed a project: acl*security.Feb 20 2020, 8:06 PM
DannyS712 moved this task from Backlog to External on the Wikimedia-Site-requests board.Mar 26 2020, 5:32 PM

No longer a site request

DannyS712 added a comment.Mar 26 2020, 5:36 PM
In T241503#5769693, @DannyS712 wrote:
In T241503#5769510, @MarcoAurelio wrote:

Probably preventing Ombudsmen and Staff accessing the private logs and thus impeding the performance of their respective duties. Please let me know if we can do this today. Thanks.

This prohibits removing the old rights, but one can still give them the new rights, and the old ones are backwards compatible[1] anyway, so Ombudsmen and Staff should still have access

[1] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/AbuseFilter/+/9552c187cc38022492524ff454bbaab334481c12/includes/AbuseFilterHooks.php#48

Looking again, this would only have affected $wgGroupPermissions, not the rights granted via global groups... anyway, no longer an issue, but sorry about that

gerritbot added a comment.Mar 26 2020, 5:40 PM

Change 583710 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/CentralAuth@master] Make it possible to remove rights that no longer exist

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

gerritbot added a project: Patch-For-Review.Mar 26 2020, 5:40 PM
gerritbot added a comment.Mar 26 2020, 8:19 PM

Change 583745 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[operations/mediawiki-config@master] [Beta cluster] add a fake 'UselessRightForTesting' to available rights

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

DannyS712 added a comment.EditedJul 11 2020, 4:31 AM

Note that this has come up before as well - T203464: Remove 'editusercssjs' and 'sendemail-new-users' from global groups and T188278: Remove 'moodbar-admin' from 'staff' global group

gerritbot added a comment.Sep 14 2020, 9:58 PM

Change 583710 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Make it possible to remove rights that no longer exist

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.9; 2020-09-15).Sep 14 2020, 10:00 PM
gerritbot added a comment.Sep 15 2020, 6:14 PM

Change 583745 merged by jenkins-bot:
[operations/mediawiki-config@master] [Beta cluster] add a fake 'UselessRightForTesting' to available rights

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

gerritbot added a comment.Sep 15 2020, 6:25 PM

Change 627453 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[operations/mediawiki-config@master] Revert "[Beta cluster] add a fake 'UselessRightForTesting' to available rights"

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

gerritbot added a comment.Sep 15 2020, 6:26 PM

Change 627453 merged by jenkins-bot:
[operations/mediawiki-config@master] Revert "[Beta cluster] add a fake 'UselessRightForTesting' to available rights"

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

DannyS712 closed this task as Resolved.Sep 15 2020, 6:41 PM
DannyS712 claimed this task.
DannyS712 removed a project: Patch-For-Review.

Tested on the beta cluster, it worked

DannyS712 updated the task description. (Show Details)Sep 15 2020, 6:43 PM
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL