Page MenuHomePhabricator

User rights validation is sometimes malfunctioning (with FlaggedRevs only?)
Closed, ResolvedPublic

Description

There are multiple current instances where users

  • Cannot proceed with an action, because they are not perceived as having rights that they are indeed assigned
  • Can proceed with an action, despite not having the rights needed

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

AFAIK all Flaproblems what i have seen can be explained with settings from wmf-config/flaggedrevs.php is not loaded.

As noted in T233561#5532950 the start of these issues roughly coincides with variant settings being introduced. Since that involves an extra config cache layer, maybe what's happening is that the behavior is different on cache hit and cache miss for some reason.

In T234743#5550291, @Tgr wrote:

As noted in T233561#5532950 the start of these issues roughly coincides with variant settings being introduced. Since that involves an extra config cache layer, maybe what's happening is that the behavior is different on cache hit and cache miss for some reason.

Pinging @Jdforrester-WMF here, then.

In T234743#5550291, @Tgr wrote:

As noted in T233561#5532950 the start of these issues roughly coincides with variant settings being introduced. Since that involves an extra config cache layer, maybe what's happening is that the behavior is different on cache hit and cache miss for some reason.

But… it didn't. The caching code (for /tmp on the appservers) has been moved around and changed format away from serialised PHP, but we've not added any new caching. It's definitely possible that the config output is wrong, but it seems very odd that it'd be unstable and flake between different outcomes (the entire point of the work is to reduce the amount of live code and so chances for inconsistencies).

Failure to load flaggedrevs.php seems very alarming, yes.

Would it help for debugging if we could replicate this in test2.wikipedia.org?

I think that adding unreviewedpages right to sysops in flaggedrevs.php for test2wiki should do the trick.

Jdforrester-WMF renamed this task from User rights validation is malfunctioning to User rights validation is sometimes malfunctioning (with FlaggedRevs only?).Oct 15 2019, 7:58 PM

Wildly speculating, but the switch to properly use extension registration in production for FlaggedRevs may make this better (or, at least, more consitent): https://gerrit.wikimedia.org/r/543180 / T87915 / T140852

Noting this here since I don't think protection itself has been mentioned (here or in T233561), but I've now twice run into permissiondenied errors when trying to change an article from pending changes to semiprotection (enwiki, ofc).

A similar issue happened to me today while deploying this change. After I synced this change, about half the appservers (correctly) believed that $wgGEHomepageSuggestedEditsRequiresOptIn was false on cswiki, and the other half believed it was true. I checked one appserver that was returning the wrong responses, and I found that its copy of InitialiseSettings.php was correct, but running var_dump($wgGEHomepageSuggestedEditsRequiresOptIn) from eval.php returned the wrong result. I re-ran scap sync-file wmf-config/InitialiseSettings.php and that fixed it. Unfortunately I did that pretty early on, before looking at the JSON files in the cache directory in /tmp, so I wasn't able to investigate further.

Based on Manual:$wgExtensionFunctions: This variable is an array that stores functions to be called after most of MediaWiki initialization is complete. Note however that at this point the RequestContext is not yet fully set up, so attempting to use it (or equivalent globals such as $wgUser or $wgTitle) is liable to fail in odd ways. If you need to use the RequestContext, consider the BeforeInitialize and ApiBeforeMain hooks instead.

However, configuration in file wmf-config/flaggedrevs.php was wrapped to function which is called from wgExtensionFunctions hook in commit 606310e502abc426a9f0b1a6cffd31aeb3d1e1c5 at Jun 24 and if $wgDBname is randomly not set up then FR config running with defaults in those cases. This could be reason for T237191 too. (ping @Reedy}

Wildly speculating, but the switch to properly use extension registration in production for FlaggedRevs may make this better (or, at least, more consitent): https://gerrit.wikimedia.org/r/543180 / T87915 / T140852

Probably. Or making these things worse.

The problem presumably happens when things load with the default config, and don't get overridden to turn off the rights

As has been said before, extensions should have sane defaults. But how do you set sane defaults for an extension as complex as FlaggedRevs, which has N different modes that it can be configured

Then we have stuff like https://github.com/wikimedia/operations-mediawiki-config/blob/master/wmf-config/flaggedrevs.php#L642-L669

But if we look at other bugs under this one... Rights being assigned by the default FR config, and then being removed at an arbitary time during wmf-config running... And we know complex array configs don't work well with extension registration

Ideally we strip out, and/or turn off most of the config out of FlaggedRev's extension.json (and document?).. And then set the config in flaggedrevs.php... And then strip out most of that into IS anyway? Because most of it is the same as IS...

In T234743#5747045, @Zache wrote:

However, configuration in file wmf-config/flaggedrevs.php was wrapped to function which is called from wgExtensionFunctions hook in commit 606310e502abc426a9f0b1a6cffd31aeb3d1e1c5 at Jun 24 and if $wgDBname is randomly not set up then FR config running with defaults in those cases. This could be reason for T237191 too. (ping @Reedy}

$wgDBname is definitely set before flaggedrevs.php is even loaded

A similar issue happened to me today while deploying this change. After I synced this change, about half the appservers (correctly) believed that $wgGEHomepageSuggestedEditsRequiresOptIn was false on cswiki, and the other half believed it was true. I checked one appserver that was returning the wrong responses, and I found that its copy of InitialiseSettings.php was correct, but running var_dump($wgGEHomepageSuggestedEditsRequiresOptIn) from eval.php returned the wrong result. I re-ran scap sync-file wmf-config/InitialiseSettings.php and that fixed it. Unfortunately I did that pretty early on, before looking at the JSON files in the cache directory in /tmp, so I wasn't able to investigate further.

The fact Roan has seen this with non FR is slightly concerning, but maybe slightly a good thing - it's not "just" FR... If it is the problems in FR and wmf-config for FR being exacerbated by the config rewrite?

I am getting a permission error response if I try to mark pages for translation on mobile (mobile site, to be precise).

Steps to reproduce

  • Find a page that doesn't have their latest version marked for translation
  • Click mark for translation and switch to mobile domain (Example)
  • Try to mark it for translation

Expected results

  • It marks the page for translation and redirects me to Special:PageTranslation homepage just like it does on the desktop site

Actual results

I get an permission error which says
The action you have requested is limited to users in the group: Translation administrators. even though I am a translation administrator on Mediawiki.org

Note

I was able to reproduced this bug on both Beta Commons and Mediawiki.org where I have translation administrator rights.

Note on the screen shot from Cyphoidbomb's failure above, this is referencing a group that doesn't even exist on enwiki "Editors".

A bug like this isn't so surprising since FlaggedRevs is buggy and has no maintainer currently But not sure why it is affecting Translate extension.

Masumrezarock100 wrote:

Note on the screen shot from Cyphoidbomb's failure above, this is referencing a group that doesn't even exist on enwiki "Editors".

A bug like this isn't so surprising since FlaggedRevs is buggy and has no maintainer currently But not sure why it is affecting Translate extension.

This can be still explained with a configuration race condition. At some time times, FlaggedRevs site-specific configurations arent loaded so it is running with defaults. (ie. there is default user groups like "editors" etc)

Problem likely started when FR config was moved to an extension function in summer 2019. And also as catrope noticed it maybe not just to be Flagged revs configuration problem. So it is just more visible with a current configuration where the config is inside extension function.

@DannyS712 could you file a gerrit change to add the right to administrators. This is mildly annoying to say the least.

In T234743#5843756, @Zache wrote:

And also as catrope noticed it maybe not just to be Flagged revs configuration problem.

That's T236104: Cache of wmf-config/InitialiseSettings often 1 step behind. Maybe has a related cause but definitely not the same issue.

@Aklapper @Nemo_bis any timeline on when this will be fixed?

If "Problem likely started when FR config was moved to an extension function in summer 2019", why not just roll back this change?

In T234743#5851888, @Ankit-Maity wrote:

@Aklapper @Nemo_bis any timeline on when this will be fixed?

@Ankit-Maity: Hi, I'm not sure why you are asking us specifically or why you think we could answer that. All that is known to anybody is written in this public task...

In T234743#5851888, @Ankit-Maity wrote:

@Aklapper @Nemo_bis any timeline on when this will be fixed?

@Ankit-Maity: Hi, I'm not sure why you are asking us specifically or why you think we could answer that. All that is known to anybody is written in this public task...

Okay, maybe. But, from what I can see there is no timeline mentioned anywhere. I'll take it that means there is no timeline on this and/or no one is working on fixing this atm?

Edit: Nemo is a member of the project and you are added as a default subscriber for phab tasks, I don't know who else I can ask.

In T234743#5851941, @Ankit-Maity wrote:

from what I can see there is no timeline mentioned anywhere. I'll take it that means there is no timeline on this and/or no one is working on fixing this atm?

Correct; see https://www.mediawiki.org/wiki/Bug_management/Development_prioritization for some more background.

I have OS rights on en.wiki but couldn't revert using pending changes. Ok, I understand priorities, but still.

I have OS rights on en.wiki but couldn't revert using pending changes. Ok, I understand priorities, but still.

Not sure how Oversight right is related to pending changes.

It's not, I just mean that it's ridiculous to have OS and CU and not be able to do pending changes. I've been given the pending changes rights since posting here, it will be interesting to see if that makes a difference. On the other hand, I was able to do pc yesterday.

@DougWeller, I'm a bit lost in your comment. On enwiki at least, accepting pending changes is part of the (review) permission, this is included in the "administrators" (sysop) group and the "pending changes reviewers" (reviewer) group. I can't see what this has to do in the least with oversight and even further from what it would have to do with checkuser. It is very certainly possible to be a member of both the CU and OS group and not have access to "review" just as you wouldn't have access to many other functions.

You can't have OS without being a sysop. I apologise for confusing you. As
you say, I should obviously have access to pc review and have always had
it, until I didn't.

I believe what Doug Weller is trying to say is that enwiki oversighters are all admins and admins on enwiki hold the "review" permission. And yet sometimes there is a permission error for oversighters. Which is a bug as it shouldn't happen to users who have the "review" permission via their usergroup.

I believe what Doug Weller is trying to say is that enwiki oversighters are all admins and admins on enwiki hold the "review" permission.

Confirmed, there is setting $wgGroupPermissions['sysop']['review'] = true; in enwiki's settings at flaggedrevs.php

OK, so I think it is confirmed that this has nothing at all to do with OS, which you absolutely can be a member of without being a member of sysop and has no nested review permissions; so this is still the original problem, that sometimes people with review permissions (such as reviwers and sysops on enwiki) are intermittently failing.

This sometimes happens to me too. Either the review interface doesn't appear, or I get an error message about a lack of permission if it appears and I use it. This time, at "Tyler1" on enwiki. A pending changes reviewer has kindly reviewed the sysop edits, but shouldn't have had to. I wonder if the bug can be circumvented by adding a sysop to the pending changes reviewer group.

I've just added myself to the group and the interface appears. The bug can be circumvented by adding a sysop to the pending changes reviewer group.

I, and at least two other folks on fawiki, are experiencing this bug too. On fawiki, we don't have the "circumventing" workaround explained above. Is there anyway I/we could help with resolving this?

In T234743#6033372, @Huji wrote:

I, and at least two other folks on fawiki, are experiencing this bug too. On fawiki, we don't have the "circumventing" workaround explained above. Is there anyway I/we could help with resolving this?

Since FlaggedRevs is not being maintained (and no one is working on this and no one is sure why it's happening), the fastest way would be to make a pendingchanges usergroup on fawiki.

In T234743#6033557, @QEDK wrote:
In T234743#6033372, @Huji wrote:

I, and at least two other folks on fawiki, are experiencing this bug too. On fawiki, we don't have the "circumventing" workaround explained above. Is there anyway I/we could help with resolving this?

Since FlaggedRevs is not being maintained (and no one is working on this and no one is sure why it's happening), the fastest way would be to make a pendingchanges usergroup on fawiki.

Can you point me to the WMF setting file in which a similar group is defined for another wiki?

In T234743#6035999, @Huji wrote:
In T234743#6033557, @QEDK wrote:
In T234743#6033372, @Huji wrote:

I, and at least two other folks on fawiki, are experiencing this bug too. On fawiki, we don't have the "circumventing" workaround explained above. Is there anyway I/we could help with resolving this?

Since FlaggedRevs is not being maintained (and no one is working on this and no one is sure why it's happening), the fastest way would be to make a pendingchanges usergroup on fawiki.

Can you point me to the WMF setting file in which a similar group is defined for another wiki?

See https://gerrit.wikimedia.org/r/plugins/gitiles/operations/mediawiki-config/+/master/wmf-config/flaggedrevs.php line 252 for the enwiki config, you also have to readd reviewer in fawiki.

In T234743#6035999, @Huji wrote:

Can you point me to the WMF setting file in which a similar group is defined for another wiki?

Similar group already exists in fawiki and it's the same 'patroller' group that you mentioned in the other ticket. Enwiki just decided to name the group 'pendingchanges reviewer' maybe because patroller is used for something else there. The relevant internal rights are: review and autoreview which allow reviewing others and automatic self-reviewing. Both groups have them common besides other ancillary ones not related to FR extension. The settings are defined in /wmf-config/flaggedrevs.php configuration file

In T234743#6035999, @Huji wrote:

Can you point me to the WMF setting file in which a similar group is defined for another wiki?

Similar group already exists in fawiki and it's the same 'patroller' group that you mentioned in the other ticket. Enwiki just decided to name the group 'pendingchanges reviewer' maybe because patroller is used for something else there. The relevant internal rights are: review and autoreview which allow reviewing others and automatic self-reviewing. Both groups have them common besides other ancillary ones not related to FR extension. The settings are defined in /wmf-config/flaggedrevs.php configuration file

If we have similar groups and (as far as I can see) similar settings, then why does the "workaround" not work for fawiki?

In T234743#6036270, @Huji wrote:
In T234743#6035999, @Huji wrote:

Can you point me to the WMF setting file in which a similar group is defined for another wiki?

Similar group already exists in fawiki and it's the same 'patroller' group that you mentioned in the other ticket. Enwiki just decided to name the group 'pendingchanges reviewer' maybe because patroller is used for something else there. The relevant internal rights are: review and autoreview which allow reviewing others and automatic self-reviewing. Both groups have them common besides other ancillary ones not related to FR extension. The settings are defined in /wmf-config/flaggedrevs.php configuration file

If we have similar groups and (as far as I can see) similar settings, then why does the "workaround" not work for fawiki?

It's not the same, @Ammarpad . reviewer is the default group used by FlaggedRevs, fawiki unsets it and instead assigns reviewer rights to patroller. If you're not using the default reviewer group, the workaround does not work, even if you give identical rights to another group, and no way to currently explain this behaviour.

I am talking about the internal review and autoreview rights that the groups share in common. I did not mention 'reviewer' group in my comment. The rights I mentioned are what the default group also have (in addition to others), so unless flaggedrevs.php is not being loaded or loaded very late they should behave the same.

The similarities of their names (review and reviewer) confused me. Thanks for clarifying.

Does this still exist or was this solved too when [[phab:T237191]] was fixed? (by https://gerrit.wikimedia.org/r/627332)

This is caused by $wgGroupPermissions being incorrect and that patch only touched $wgFlaggedRevsAutopromote / $wgFlaggedRevsAutoconfirm so probably not.

I have written a bot that reviews revisions that wasn't autoreviewed due to this bug. It did many reviews every day to May 11, 2020, after that day it does much less reviews, so, looks like problem was solved, at least partially, on May 11.

https://ru.wikipedia.org/w/index.php?title=Special:Log&offset=&limit=500&type=review&user=MBHbot&page=&wpdate=&tagfilter=&subtype=accept

We have recently deployed rEFLR90edbb9f7c01: Run setup from MediaWikiServices hook. and rOMWC9722a37ca149: flaggedrevs.php: Use MediaWikiServices, not an extension function which might fix the issue. If you have a way to check whether the issue is still happening, your testing would be appreciated.

All subtasks are resolved so assuming this is fixed.