Page MenuHomePhabricator

AbuseFilter permissions spread all over InitialiseSettings and abusefilter.php: they should be in one place
Open, Needs TriagePublic

Description

Currently abusefilter configuration is spread in abusefilter.php and InitialiseSettings.php. For consistency, this should be just stored in one place. Which one is something we should agree to in here as well.

Event Timeline

It already has its own file, might as well use it.

I guess moving everything to abusefilter.php would make more sense, wouldn't it?

I'd support moving to abusefilter.php so we can see all configs in one place, yes. Similar to what happens with flaggedrevs.php. Thank you.

Change 477063 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[operations/mediawiki-config@master] Move all AbuseFilter config to abusefilter.php

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

Daimona moved this task from Future to Under review on the User-Daimona board.

Change 477063 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[operations/mediawiki-config@master] Move all AbuseFilter config to abusefilter.php

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

Wondering about the best place for permissions. For instance, after @Daimona's patch is merged, cswiki's arbcom group would be defined in two places, IS.php and abusefilter.php. Since having groups set randomly in both places isn't ideal, I would go for "permissions in IS.php and the rest in AF.php". What do you think?

Wondering about the best place for permissions. For instance, after @Daimona's patch is merged, cswiki's arbcom group would be defined in two places, IS.php and abusefilter.php. Since having groups set randomly in both places isn't ideal, I would go for "permissions in IS.php and the rest in AF.php". What do you think?

I can't see an obviously-better choice. At the moment, the majority of group rights related to AF are in abusefilter.php, and I'd say to move everything there. If only to reduce the size of the gigantic IS.php. I also see a few other extensions do the same (mostly flaggedrevs). But again, yeah, this is something we can discuss.

Repeating what I've said at T145931#4546587. My personal choice would be to store everything related to AbuseFilter in abusefilter.php due to the size of InitialiseSettings.php which is becomming quite large a difficult to work with sometimes (for me). That being said, I don't know if for performance or for other reasons we would like to move stuff to CommonSettings.php and InitialiseSettings.php. In any case, having the config spread into several places is not optimal IMHO. I agree with @Daimona above.

I don't have problems with having AF-only groups in abusefilter.php, but I don't think having one group configured in multiple places is a good idea.

I see 3 options:

  1. have a separate abusefilter.php file, but don't include any rights configurations
  2. have a separate abusefilter.php file, but only have user groups that are part of the extension configured there (abusefilter, abusefilter-helper)
  3. have a seperate abusefilter.php file, and include all abusefilter configuration

Personally, I support option 3. Option 1 grows the size of IS, and option 2 is the worst of both world: what happens to enwiki's abusefilter-helper group that is also assigned spamblacklistlog - is that a part of IS (as it is now, or a part of AF, because that is where the group comes from)? I see option 3 implemented as:

  1. $wgGroupPermissions[ $group ]['abusefilter-' $right] = $bool; being a part of AF, for all abuse filter related rights
  2. $wgGroupPermissions[ 'abusefilter'/'abusefilter-helper' ][ $right] = $bool; being a part of IS, for all other configuration of rights that are given to the same group (like changetags, etc)

I know that splitting groups isn't the best idea, but we already have configurations spread out - for each wiki, user rights are given first in groupOverrides on a per-wiki level, then in the commonsuploads entry for all wikis in that group, then defaulting to groupOverrides2, all of which are in IS (and then defaulting to CommonSettings, and then to DefaultSettings.php) My point is, we already split up group assignments for convenience, and given the size of IS I believe doing so is a good thing. Thus, I believe that that option 3 would be a net positive.

The obvious fix is scrapping abusefilter.php (and mobile.php and FlaggedRevs.php), which is on my to-do list, eventually.

Stang added a subscriber: Stang.

Support the third choice DannyS712 mentioned - we already have flaggedrevs.php doing the same thing. @Daimona Any progress of your patch?