'purge' permission is not checked for action=purge
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To

Authored By

	Checker2718
	Sep 18 2021, 2:38 AM

Description

repro:

set $wgGroupPermissions['*']['purge'] = false; in LocalSettings.php
go to a page while not logged in and add ?action=purge to the url
you can purge the page

You can patch this with this short diff:

=== modified file 'includes/actions/PurgeAction.php'
--- includes/actions/PurgeAction.php    2021-09-11 04:42:22 +0000
+++ includes/actions/PurgeAction.php    2021-09-18 02:16:23 +0000
@@ -33,6 +33,10 @@
                return 'purge';
        }

+       public function getRestriction() {
+               return 'purge';
+       }
+
        public function getDescription() {
                return '';
        }

I think ApiPurge is similarly unprotected but I am working on testing that. Should I open another bug if it also doesn't prevent purges?

What happens?:

You can purge the page while not logged in.

What should have happened instead?:

The expected behavior is the 'purge' permission being false for a group prevents the purge action.

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:

MediaWiki 1.36.1

Details

	Subject	Repo	Branch	Lines +/-
	Turn the "purge" permission into an implicit right.	mediawiki/core	REL1_41	+6 -8
	Turn the "purge" permission into an implicit right.	mediawiki/core	master	+6 -8

Customize query in gerrit

Related Objects

Mentioned In: T349792: Gadget definition syntax should allow restricting a gadget to logged in users only
T349348: UserRightsError: User does not have required user right "purge"
Mentioned Here: T349792: Gadget definition syntax should allow restricting a gadget to logged in users only
T135170: Require POST in ?action=purge
T145649: Deprecate and remove use of API action=purge with GET requests
rMWc2ce6a1b6085: Require POST for action=purge in PurgeAction
rMWeada94090990: ApiPurge: Require POST

Event Timeline

Checker2718 created this task.Sep 18 2021, 2:38 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 18 2021, 2:38 AM

Peachey88 added a project: MediaWiki-Core-AuthManager.Sep 18 2021, 2:43 AM

My understanding is that purge userright is now deprecated/obsolete and anyone can purge, as long as they make POST requests.

It is definitely something I'd like to prevent non-logged-in visitors from doing, so it's useful (to me).

Zabe subscribed.Sep 18 2021, 9:23 AM

In T291316#7363549, @Legoktm wrote:

My understanding is that purge userright is now deprecated/obsolete and anyone can purge, as long as they make POST requests.

That was rMWc2ce6a1b6085: Require POST for action=purge in PurgeAction and T135170: Require POST in ?action=purge

That patch did indeed remove the if ( $user->isAllowed( 'purge' ) ) {

And the API was done in rMWeada94090990: ApiPurge: Require POST and T145649: Deprecate and remove use of API action=purge with GET requests.

If the userright is indeed deprecated, we really should clean this up, removing from DefaultSettings permissions, grants etc. And update https://www.mediawiki.org/wiki/Manual:User_rights, as looking (superficially) at the documentation there doesn't seem to be any reference to it being so.

As currently it does look rather orphaned

In T291316#7363551, @Checker2718 wrote:

It is definitely something I'd like to prevent non-logged-in visitors from doing, so it's useful (to me).

Out of interest, what problem(s) are you experiencing with anon users being able to purge pages (even if they have to POST)?

Out of interest, what problem(s) are you experiencing with anon users being able to purge pages (even if they have to POST)?

I'm just trying to reduce potential load and tighten up any kind of DOS weak spots. Since purging and recreating the page presumably is higher load on the server (as mentioned in the bug you link), it seems reasonable to have it be on a permission. I have varnish in front of MediaWiki, so purging actually clears the MediaWiki cache and sends a purge to varnish too.

I can gather some performance stats if you really need them although it sounds like it's accepted that it's a performance thing from reading the related bugs, it just seems like an obvious thing to have behind a permission, and the code complexity is basically negligible as shown in my patch (and I'd assume there's an equivalently simple patch for the api, I just haven't done that work yet).

For the API, probably just a call like $this->checkUserRightsAny( 'purge' ); at the top of the execute function will be enough.

You can use rate limits to throttle down anons (not sure if a throttle of 0 is possible, but 1 in 30 days or such could be a work around, maybe with a big cidr range)

https://www.mediawiki.org/wiki/Manual:$wgRateLimits

Umherirrender unsubscribed.Jun 19 2022, 7:31 PM

Xaosflux subscribed.Apr 4 2023, 1:30 PM

The inconsistency should be dealt with, but I think there are people that have latched on to 'purge' permission being something that only users have for purposes of things like loading gadgets or not, so removal should be done with lots of communications. Documentation is also out of sync between Manual:User_rights and Manual:Purge (in that yes users still have this permission - but that this permission appears to now do nothing)

It looks like the purge permission has not been checked anywhere since 2016 (not by ApiPurge, ActionPurge, or SpecialPurge). I think it's time to recognize this as the status quo and document this fact. purge can be rate limited, but does not function as a permission.

I accidentally "fixed" this for ApiPurge in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/960660, which promply broke CI for RESTbase. We have Mocha tests that rely on the abilit to purge pages without logging in. This is now blocking RESTbase deprecation.

Since 1.41, MediaWiki models actions that can be rate limited but not revoked as "implicit right" (per PermisionManager::IMPLICIT_RIGHTS). linkpurge is already an implicit right, and purge has been treated as such for many years and releases. So I think the correct fix at this point is to make it official.

Change 964878 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Turn the "purge" permission into an implicite right.

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

gerritbot added a project: Patch-For-Review.Oct 10 2023, 10:31 AM

Test wiki created on Patch demo by JGiannelos (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/4f1dd476ae/w

It certainly COULD be fixed, and if you want a project to have it for everyone grant it to the (*) group? Suppose we need a determination If the concept of needing permission for this action is appropriate or not

In T291316#9238795, @Xaosflux wrote:

It certainly COULD be fixed, and if you want a project to have it for everyone grant it to the (*) group? Suppose we need a determination If the concept of needing permission for this action is appropriate or not

Since nobody has come up with a use case in the past 7 years, I see no compelling reason to support the permission.

Turning this into an "implicite right" is slightly simpler, and the symmetry with linkpurge makes it seem like a good choice.

Change 964878 merged by jenkins-bot:

[mediawiki/core@master] Turn the "purge" permission into an implicit right.

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

Change 964602 had a related patch set uploaded (by Jforrester; author: Daniel Kinzler):

[mediawiki/core@REL1_41] Turn the "purge" permission into an implicit right.

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

Change 964602 merged by jenkins-bot:

[mediawiki/core@REL1_41] Turn the "purge" permission into an implicit right.

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

ReleaseTaggerBot added projects: MW-1.41-notes, MW-1.42-notes (1.42.0-wmf.1; 2023-10-17).Oct 10 2023, 10:00 PM

Maintenance_bot removed a project: Patch-For-Review.Oct 10 2023, 10:10 PM

JJMC89 mentioned this in T349348: UserRightsError: User does not have required user right "purge".Oct 19 2023, 8:37 PM

@SD0001 connected this task to a discussion on en.wp where @Jdlrobson says he caught a regression in delivery of assets. See from his comment. Is this task the cause? If it is, should gadgets-definition be changed or should gadgets extension support this use? (I have no issue with either, but the former could probably use a User-notice [rights=createpage won't work for all wikis] .)

In T291316#9277203, @Izno wrote:

@SD0001 connected this task to a discussion on en.wp where @Jdlrobson says he caught a regression in delivery of assets. See from his comment. Is this task the cause? If it is, should gadgets-definition be changed or should gadgets extension support this use? (I have no issue with either, but the former could probably use a User-notice [rights=createpage won't work for all wikis] .)

Is that link correct? That comment seems to be about some CSS issue, I don't see how it would be related. There is a discussion about the purge API a bit further up the page though: https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Purge_API_broken?. Is that what you mean?

In any case, I commented there.

In T291316#9278078, @daniel wrote:

In T291316#9277203, @Izno wrote:

@SD0001 connected this task to a discussion on en.wp where @Jdlrobson says he caught a regression in delivery of assets. See from his comment. Is this task the cause? If it is, should gadgets-definition be changed or should gadgets extension support this use? (I have no issue with either, but the former could probably use a User-notice [rights=createpage won't work for all wikis] .)

Is that link correct? That comment seems to be about some CSS issue, I don't see how it would be related. There is a discussion about the purge API a bit further up the page though: https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Purge_API_broken?. Is that what you mean?

In any case, I commented there.

Yes, that's the correct link. In several gadgets definitions we have |rights=purge, which should prevent loading gadgets for users who don't have the purge right (i.e. unregistered users). Jon apparently discovered an increased load (there's a Grafana lying around somewhere) with the previous deploy, seemingly indicating that this prevention is no longer occurring. (If this task is relevant. I think it probably is.)

@daniel see bump (CSS desktop version) on 20th October:
https://grafana.wikimedia.org/d/000000205/reading-web-performance?orgId=1&viewPanel=144&from=now-30d&to=now

Ah, I was unaware of this mechansim in Gadgets. Thank you for the explanation.

If the intent is to load the gadget for registered users, perhaps it would be best to introduce a groups parameter for the gadget definition.

Change 968633 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/extensions/Gadgets@master] Experiment: introduce groups parameter

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

gerritbot added a project: Patch-For-Review.Oct 25 2023, 11:21 AM

I made a patch that allows groups to be specified in a gadget definition, e.g.

*bar[ResourceLoader|groups=autoconfirmed,sysop]|bar.js'

Someone (tm) would need to approve this approach from a product perspective. Who owns the Gadgets extension these days, anyway?

In T291316#9279296, @daniel wrote:
I made a patch that allows groups to be specified in a gadget definition, e.g.
*bar[ResourceLoader|groups=autoconfirmed,sysop]|bar.js'

I think generally |rights= is a bit more flexible if a bit less intentional. In some cases anyway, since it can more intentionally say "I don't care who has this right" since sometimes rights are allocated to more than one group. And at this point in time, I wouldn't personally want to add another axis of customization that is as similar as it is to an existing one. I'm sure this question was discussed around the time that |rights was originally discussed too, but that's history now and I don't want to dig. :)

I think mostly you just got unlucky and happened to pick the right to adjust/fix that we had settled on informally to indicate "registered users". If I take look at Special:ListGroupRights I see a few others that could also potentially use in user, I'm just not sure which is most stable for the moment. Probably (view|edit)mywatchlist has the least likelihood of being removed or reframed into multiple rights or having this task happen again, as well as being a right provided in the default installation and not likely to be removed from the group to be added to some sort of "extended user" group?

I hadn't gotten around to looking at that list before I commented earlier, but that list is why I said that I'm pretty sure a User-notice about this change would be an acceptable outcome instead of adjustment of the extension (whether supporting |rights=purge or adding something like your |groups). A global interface editor could change all the definitions everywhere regarding User-notice or individual wikis can take care of themselves, etc. etc.

Someone (tm) would need to approve this approach from a product perspective. Who owns the Gadgets extension these days, anyway?

No one owns it according to https://www.mediawiki.org/wiki/Developers/Maintainers . That one lists a few names as individual maintainers (from WMF folk in that list, I think @Krinkle has been the most responsive on the Phab project).

MediaWiki has always permitted anyone to purge pages, this has never required a user right of any kind. All that changed recently is that the rights=purge filter now works correctly to recognise this fact by removing the unused "purge" right.

Prior to 2016 (in change 304157), MediaWiki used a right named "purge", separately from the action called "purge", to give to logged-in users a way to execute action=purge by opening a link ("GET" request). Users without this right were instead presented with an HTML form to submit first ("POST" request).

Since 2016, all users are presented with an HTML form, logged-in users as well. Both before and after, everyone had the right to perform the "purge" action. After 2016, the "right" no longer existed. Two weeks ago (10 Oct 2023), the unused "right" was removed. This meant that rights=purge now correclty matches all users, including anons.

In other words, this change from 2016 did not increase exposure but actually reduced it, it raised the requirement such that now everybody is required to submit an HTML form first, just like for anons already prior to the above change.

Discussion on Gadgets split off to T349792.

Test wiki on Patch demo by JGiannelos (WMF) using patch(es) linked to this task was deleted:

~~https://patchdemo.wmflabs.org/wikis/4f1dd476ae/w/~~

Anything left to do here?

Closing as resolved. If anyone wants to re-open the discussion about restricting purges or providing a better mechanism for loading gadgets only for logged in users, please file a separate ticket.

'purge' permission is not checked for action=purgeClosed, ResolvedPublicBUG REPORTActions

Description

Details

Related Objects

Event Timeline

'purge' permission is not checked for action=purge
Closed, ResolvedPublicBUG REPORT
Actions