Sometimes extensions modified User::$mRights to add pseudo-rights for the current request, mainly so that an account without the bot right can perform some special action with a bot flag. This was a dirty hack, but there wasn't really any non-dirty alternative. In Wikimedia production this is used (optionally) for mass user lockout (Special:MultiLock in CentralAuth); other extensions relying on it include GraphViz, UserGroups and ArticleFeedbackv5. (code search)
rMWdd6b94024c53: Re-apply: Factors out permissions check from User into PermissionManager service removes this ability; trying to write User::$mRights (outside of unit tests) now throws an exception. Either PermissionManager should provide an alternative way to achieve this, or Special:MultiLock should inject the bot flag in some other way, or the functionality should be removed cleanly from CentralAuth. (Preferably the other extensions too but that's up to their own maintainers.)
Description
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Release | • LarsWirzenius | T220739 1.34.0-wmf.14 deployment blockers | ||
| Resolved | Tgr | T227772 Fix or remove capability to override user rights for the current request |
Event Timeline
If we did have a more sensible way to achieve this sort of thing, it might provide a useful way around changecontentmodel right restrictions for extensions just creating new pages with different content content models (massmessage lists, collaboration hubs), without us needing to totally redo how we handle content model changes in general...
Not sure if that's really the right approach with that either, but there might also be other use cases to consider with regards to adding an alternative here as well.
Only the "mark as bot" option of Special:MultiLock, if I understand the code correctly.
I mean potential use cases, where the extensions/whatever aren't doing this because it's too hacky, but might actually want to do something more sane if the option were available.
May I please please ask to stop the removal of this function (the bot flag one) before a solution is found? MultiLock is the most heavily used tool for stewards at this moment. Flooding off recent changes is annoying and prevents patrolling the wiki. At this time we're dealing with a spambot attack. That means our locking activity has doubled in the last days. I don't think keeping recent changes unreadable for our users is okay. Thanks for your consideration.
To be clear, this is already broken and will throw an exception if you try using it now. The change that broke it (a thorough refactoring of permission handling) is "too big to fail", I don't think it would be a good idea to revert it over this.
MassMessage uses the UserGetRights hook for this purpose (https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/MassMessage/+/master/includes/MassMessageHooks.php#134). Extensions could register a temporary hook and force reloading the user or something?
If it is too big to fail (as in revert), can we get the functionality back in any possible way? It is also too big to fail for our workflow.
I hope a solution can be found then. This puts a significant burden in our job now with having us to go with the bot flag on-and-off dance if we want to avoid recent changes pollution, which in turn will cause pollution of the user rights logs for pointless user rights changes. I am between sad and frustrated that this has happened without finding an alternative first.
Change 522148 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Add mechanism for temporary user rights
Change 522152 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/CentralAuth@master] Fix SpecialMultiLock markasbot option
Change 522148 merged by jenkins-bot:
[mediawiki/core@master] Add mechanism for temporary user rights
Change 523840 had a related patch set uploaded (by Tim Starling; owner: Gergő Tisza):
[mediawiki/core@wmf/1.34.0-wmf.13] [1.34.0-wmf.13] Add mechanism for temporary user rights
Change 523841 had a related patch set uploaded (by Tim Starling; owner: Gergő Tisza):
[mediawiki/core@wmf/1.34.0-wmf.14] [1.34.0-wmf.14] Add mechanism for temporary user rights
Change 523840 merged by jenkins-bot:
[mediawiki/core@wmf/1.34.0-wmf.13] [1.34.0-wmf.13] Add mechanism for temporary user rights
Change 523841 merged by jenkins-bot:
[mediawiki/core@wmf/1.34.0-wmf.14] [1.34.0-wmf.14] Add mechanism for temporary user rights
Change 522152 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Fix SpecialMultiLock markasbot option
Change 523843 had a related patch set uploaded (by Tim Starling; owner: Gergő Tisza):
[mediawiki/extensions/CentralAuth@wmf/1.34.0-wmf.13] [1.34.0-wmf.13] Fix SpecialMultiLock markasbot option
Change 523844 had a related patch set uploaded (by Tim Starling; owner: Gergő Tisza):
[mediawiki/extensions/CentralAuth@wmf/1.34.0-wmf.14] [1.34.0-wmf.14] Fix SpecialMultiLock markasbot option
Change 523843 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@wmf/1.34.0-wmf.13] [1.34.0-wmf.13] Fix SpecialMultiLock markasbot option
Change 523844 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@wmf/1.34.0-wmf.14] [1.34.0-wmf.14] Fix SpecialMultiLock markasbot option
Mentioned in SAL (#wikimedia-operations) [2019-07-17T03:42:53Z] <tstarling@deploy1001> Synchronized php-1.34.0-wmf.13/extensions/CentralAuth/includes/specials/SpecialMultiLock.php: T227772 (duration: 00m 56s)
Mentioned in SAL (#wikimedia-operations) [2019-07-17T03:46:16Z] <tstarling@deploy1001> Synchronized php-1.34.0-wmf.14/extensions/CentralAuth/includes/specials/SpecialMultiLock.php: T227772 (duration: 00m 54s)
Thanks Tim!
Filed T228248: Replace broken User::$mRights access in GraphViz and T228249: Replace broken User::$mRights access in ArticleFeedbackv5 (UserGroups has no Phab project).
Per T228025: Translate sandbox signup is broken extensions that use UserGetRights for temporary user rights might also be affected (although that bug might be specific to using it during registration). Filed T228252: Use PermissionManager::addTemporaryUserRights() in MassMessage just in case.
The bigger deal (not related to writing User::$mRights, I just noticed it while working on that ) is that user rights are now cached by user ID instead of per User object, so the cache will get polluted whenever there are significantly different User objects with the same ID. There are two cases I can think of:
- anonymous or global users - filed T228253: PermissionManager should not cache anonymous rights under ID 0
- session restrictions, but that's sort of broken already in a different way (T218674: User::getRights() applies session rights restrictions to non-session users)
The fix was reverted in 1.34.0-wmf.14: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralAuth/+/524276
For the record, the patch landed in master after the 1.34-wmf.14 branch cut.
Yesterday, a backport to 1.34-wmf.14 was created and merged, but not successfully deployed. I found the repo in a dirty state today, and reverted in Gerrit so the git branch matches what's in production.
Lowering priority again since .14 has been rolled back, so the fix is still fully live.
@Agusbou2015: You have been asked several times not to add unhelpful comments. Please read and understand the previous comments if you would like to stay active on Wikimedia Phabricator. Thank you for your understanding.
Change 524702 had a related patch set uploaded (by Tim Starling; owner: Tim Starling):
[mediawiki/extensions/CentralAuth@wmf/1.34.0-wmf.14] [1.34.0-wmf.14] Fix SpecialMultiLock markasbot option (2nd attempt)
Change 524702 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@wmf/1.34.0-wmf.14] [1.34.0-wmf.14] Fix SpecialMultiLock markasbot option (2nd attempt)
Mentioned in SAL (#wikimedia-operations) [2019-07-22T04:17:00Z] <tstarling@deploy1001> Synchronized php-1.34.0-wmf.14/extensions/CentralAuth/includes/specials/SpecialMultiLock.php: fix UBN bug T227772 (duration: 00m 56s)
Actually the fix was not deployed on metawiki for the last three days, metawiki was running .14 before and after the train rollback. But it should be fixed now.