Page MenuHomePhabricator

Cannot revoke permissions from a global group if the permissions no longer exist
Open, MediumPublic

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

Event Timeline

Restricted Application added a project: User-DannyS712. · View Herald TranscriptDec 28 2019, 3:21 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
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.

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

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 :-)

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

chasemp moved this task from Incoming to Watching on the Security-Team board.

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.

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

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

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.

The revert has now been deployed:

>>> echo $wgAvailableRights['abusefilter-private'];
PHP Notice:  Undefined index
>>> echo $wgAvailableRights['abusefilter-private-log'];
PHP Notice:  Undefined index

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)".

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.

No longer a site request

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

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

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