Page MenuHomePhabricator

Reviewers can't stabilize pages on fiwiki
Closed, ResolvedPublicBUG REPORT

Description

fiwiki VPT has a report of user in the reviewer group not being able to stabilize pages despite having the stablesettings right.

@Urbanecm tested this on a testing account and managed to reproduce. MediaWiki thinks that the user has the stablesettings right:

>>> User::newFromName('Martin Urbanec (test)')->isAllowed('stablesettings')
=> true

this sounds similar to T273317: some users with access are unable to configure pending changes but we suspect this is a different bug

Event Timeline

Majavah created this task.Wed, Feb 17, 12:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptWed, Feb 17, 12:26 PM
Stryn added a subscriber: Stryn.Wed, Feb 17, 1:16 PM

The logic is at https://gerrit.wikimedia.org/g/mediawiki/extensions/FlaggedRevs/+/1cd278d3842323087359d2af03bb26abcc9206b0/business/PageStabilityForm.php#241. Urbanecm did some debugging on mwdebug and the check for stablesettings right seems to be broken, the check for review works just fine.

The stablesettings right is assigned in flaggedrevs.php (inside a $wgExtensionFunctions block) and review is assigned in extension.json. I could not reproduce locally, with or without $wgExtensionFunctions, but adding $wgGroupPermissions['reviewer']['stablesettings'] = true; for test2wiki lets you reproduce it on test2 too..

Zache added a subscriber: Zache.EditedThu, Feb 18, 8:12 AM

Root cause for the problem could be race condition when configuration values are merged (Warning: this was not never validated)

The autopromote problem (T237191) was fixed by this commit which set before FlaggedRevsHooks::onExtensionFunctions (this was also orginal functionality before content of the flaggedrevs.php was moved to inside the $wgExtensionFunctions and autopromote broke)

We've has a similar issue very recently - T273296 - there the TLDR of problem was that extension functions are called very late in MW initialization process, so some services (e.g. UserGroupManager in that case, probably PermissionManager in this case) are already initialized before it, and take their snapshot of the $wgGroupPermissions too early, so they never see the changes made in extensionFunction.

I'm 'fixed' that one by temporary reverting the patch which initiated too early service loading, but seems like something else triggered an exact same problem. The correct fix would be to pull the config setup out of extension function for flagged revs - that's too late to manipulate permissions, we were just lucky before and it worked.

The reason why setting up FlaggedRevs config was put into the extension function in the first place is cause that's the only way to unset permissions from defaults. The correct fix would be to change the defaults for FlaggedRev, set them where needed, but never unset. And move the FlaggedRevs permissions configuration out of extension function.

Xaosflux renamed this task from Reviewers can't stabilize pages on fiwiki to Reviewers can't stabilize pages.Fri, Feb 19, 7:20 PM
Xaosflux changed the subtype of this task from "Task" to "Bug Report".
Xaosflux triaged this task as High priority.Sat, Feb 20, 2:20 AM

Possibly related (if so higher priority) - users are reporting that auto-accept for stabilize is also failing. See report here:

https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_(technical)&oldid=1007816834#Pending_changes_auto-accept_error_again

It sounds there are multiple bugs at once...

I was playing with this a bit at test2wiki and fiwiki. I managed to see the protection form disabled for a test2wiki account with +sysop, but without +editor (with both wgFlaggedRevsProtection=true and wgFlaggedRevsProtection=false). Promoting the account to editor, or giving sysops the right to review "fixes" it. Filled T275293 for that part.

However, while this might fix a part of the issues, it does not fix fiwiki's issue, as reviewers can't modify stabilisation there, while they should. According to my playing, it sounds to be related to 606310e by @Reedy. I'm inclined to try to move it away from ext. registration function, and hope it doesn't reintroduce issues. Alternatively, we can try to move anything that touches wgGroupPermissions, wgAddGroups or wgRemoveGroups to InitialiseSettings.php.

Urbanecm renamed this task from Reviewers can't stabilize pages to Reviewers can't stabilize pages on fiwiki.Sat, Feb 20, 11:35 PM

[...]
However, while this might fix a part of the issues, it does not fix fiwiki's issue, as reviewers can't modify stabilisation there, while they should. According to my playing, it sounds to be related to 606310e by @Reedy. I'm inclined to try to move it away from ext. registration function, and hope it doesn't reintroduce issues. Alternatively, we can try to move anything that touches wgGroupPermissions, wgAddGroups or wgRemoveGroups to InitialiseSettings.php.

Scratch that. Let's move it to IS.php instead, reviewer is already touched there, which might contribute to this problem.

Change 665548 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] fiwiki: Assign stablesettings to reviewers in IS.php rather than FR-specific file

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

Restricted Application added a project: User-Urbanecm. · View Herald TranscriptSat, Feb 20, 11:37 PM
Urbanecm moved this task from Backlog to To deploy on the User-Urbanecm board.Sat, Feb 20, 11:38 PM

if this is really soley a fiwiki problem, are the problems else where (such as on enwiki) being tracked and worked under other tickets?

if this is really soley a fiwiki problem, are the problems else where (such as on enwiki) being tracked and worked under other tickets?

This particular is fiwiki-only, because only fiwiki reviewers should be able to configure pending changes. I checked with an enwiki admin, and they managed to configure pending change for a page.

I reopened T273317 for now, we can discuss the enwiki problem there.

Change 665548 merged by jenkins-bot:
[operations/mediawiki-config@master] fiwiki: Assign stablesettings to reviewers in IS.php rather than FR-specific file

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

Mentioned in SAL (#wikimedia-operations) [2021-02-22T08:11:14Z] <urbanecm@deploy1001> Synchronized wmf-config/InitialiseSettings.php: cea41a2f7736aa29dee8f10de4c0c17353ece963: fiwiki: Assign stablesettings to reviewers in IS.php rather than FR-specific file (T275017; 1/2) (duration: 01m 08s)

Mentioned in SAL (#wikimedia-operations) [2021-02-22T08:12:27Z] <urbanecm@deploy1001> Synchronized wmf-config/flaggedrevs.php: cea41a2f7736aa29dee8f10de4c0c17353ece963: fiwiki: Assign stablesettings to reviewers in IS.php rather than FR-specific file (T275017; 2/2) (duration: 00m 55s)

Urbanecm closed this task as Resolved.Mon, Feb 22, 8:16 AM

This should be fixed now. Reopen if it happens again.