Recent changes in MediaWiki caused that changing user groups from $wgExtensionFunctions no longer works reliably. That caused a couple of bugs in Wikimedia production, and I do think that a lot of third party users are also going to be affected. I'll add the Wikimedia bugs as subtasks soon, trying to fix them in our own environment, but I also think we should make it work as before for MW-1.36-notes, or clearly communicate the breakage (and perhaps throw an exception to make it clear).
Description
Description
Details
Details
Customize query in gerrit
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
@Pchelolo I created this task to track fixing of the root cause of the recent issues (wgExtensionFunctions can't no longer reliably change permissions). Some of the subtasks don't have a simple config-only solution, considering it's impossible to ie. delete extension-provided groups in config outside of an extension function. In T275333#6847294, you said T273511: Convert CentralAuthUtilityService to Authority instead of PermissionManager and T273510: Convert AuthManager to Authority. Any thoughts on whether anything can be done in Wikimedia config are appreciated.
Comment Actions
$wgExtensionFunctions callbacks are run late in the bootstrap sequence, when setup is complete. At that point, changing globals may no longer have any affect, since service instances may already have been created.
Running $wgExtensionFunctions earlier would break the expectation that setup is complete when these functions run.
So, if we run the callbacks late (as we do now), we break the expectation that they can manipulate config. If we run them earlier, we break the expectation that bootstrap is complete.
The only solution I currently see for extensions that want to change globals during setup in a way that gets picked up by the service container will have to use the MediaWikiServices hook instead.
In the future, when moving away from global variables for configuration, we may introduce a hook especially for manipulating config settings.
Comment Actions
Thanks for summarizing the issue @daniel. Breaking the assumption extension functions can modify config sadly means that there is no longer any way for local site administrators to alter extension provided config. If I unset a group in LocalSettings.php, it doesn't work, because the extensions are loaded after LS.php is. If I do it in extension function, it doesn't work, because some services are already initiated. Where else can I do this kind of a task?
We need something that runs after extensions are loaded, but before any config is actually used by services.
Comment Actions
For config settings that extensions expose via the "config" key, that works properly, right?
In general, extensions should not override config coming from LocalSettings.php. And I think in general, they don't. Things get tricky if the change you want to make is to unset it. Then the extension would put it back. But I'd expect that to be rare. What are the concrete use cases?
If I unset a group in LocalSettings.php, it doesn't work, because the extensions are loaded after LS.php is. If I do it in extension function, it doesn't work, because some services are already initiated. Where else can I do this kind of a task?
We need something that runs after extensions are loaded, but before any config is actually used by services.
The MediaWikiServices hook. It can be used from LocalSettings, though that's kind of hacky.
Comment Actions
Negative. See T275336 as an example of our own config being unable to delete a group introduced via the config key by an extension.
In general, extensions should not override config coming from LocalSettings.php. And I think in general, they don't. Things get tricky if the change you want to make is to unset it. Then the extension would put it back. But I'd expect that to be rare. What are the concrete use cases?
Maybe there's a slight misunderstanding. I'm not talking about extensions modifying LS.php config. I'm talking about LS.php modifying config introduced by extensions, which is pretty common, I would say. For one example, see T275336. Other usecases can be seen in flaggedrevs.php in our own config.
I'm pretty sure there are also third party administrators who do not want certain groups to be introduced to their systems, perhaps because they assign the rights needed via some other means, or to other groups.
If I unset a group in LocalSettings.php, it doesn't work, because the extensions are loaded after LS.php is. If I do it in extension function, it doesn't work, because some services are already initiated. Where else can I do this kind of a task?
We need something that runs after extensions are loaded, but before any config is actually used by services.The MediaWikiServices hook. It can be used from LocalSettings, though that's kind of hacky.
I agree it's hacky, on the other hand, the extension functions are also a pretty hacky concept. Should we note the hook as the proper way to do the things I mentioned? Or should we instead figure out a better solution?
Comment Actions
Change 673159 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[operations/mediawiki-config@master] Define confirmed group in MediaWikiServices hook
Comment Actions
Change 673159 merged by jenkins-bot:
[operations/mediawiki-config@master] Define confirmed group in MediaWikiServices hook
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2021-03-17T23:40:18Z] <urbanecm@deploy1002> Synchronized wmf-config/CommonSettings.php: 5c14e7d2045f0905f7e85b249e821bbe8d69c600: Define confirmed group in MediaWikiServices hook (T275334, T277704, T275310, T275333) (duration: 01m 08s)
Comment Actions
Relying on hooks and extension functions to implement *configuration* is always going to go badly. I suggest we add:
- $wgGroupInheritsPermissions = ['confirmed' => 'autoconfirmed'] - instructs that "confirmed" inherits all the rights that "autoconfirmed" has, in addition to any rights set explicitly (do we need multi-inheritance here?)
- $wgDisabledGroups = ['whatever' => true] - entirely disables a user group from MediaWiki, even if an extension is the one creating it.
Both directives would be evaluated at runtime in PermissionsManager (or elsewhere) so no hook hackery is needed.
Comment Actions
That could work for the common use case of manipulating groups. But it seems like we will need a more generic approach for programmatically manipulating settings.
Comment Actions
Hey there, should this be moved to 1.37? The cut for 1.36 has happened, and 1.36.0-rc.0 will be cut in a fortnight or so, after which feature changes shouldn't be landed and back-ported.
Comment Actions
Hello James, let me re-state what will happen if the current code is released: it will break any third-party installation that relies on extension functions to change permissions (at least, possibly other config too). I personally feel that's a pretty common usage that we shouldn't break, but it's not really my call.
Comment Actions
T273317: some users with access are unable to configure pending changes is fixed (for the extension; not for Wikimedia yet, but that does not block the release), T275335: suppressor group exists sometimes at Wikimedia wikis and T275336: push-subscription-manager group is sometimes available at all wikis are caused by Wikimedia configuration, not extension code. So I think there is no release blocker here, other than communicating that ExtensionFunctions should not be used for config changes. (That probably never worked fully reliably; FlaggedRevs has had issues for two years. Nevertheless, might be worth a release note.)
The task description also says "perhaps throw an exception to make it clear", but I don't see how that would be feasible.
Comment Actions
Change 682560 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):
[mediawiki/core@REL1_36] Warn about config changes done in ExtensionFunctions
Comment Actions
Change 682560 merged by jenkins-bot:
[mediawiki/core@REL1_36] Warn about config changes done in ExtensionFunctions
Comment Actions
Change 682707 had a related patch set uploaded (by Jforrester; author: Gergő Tisza):
[mediawiki/core@master] Warn about config changes done in ExtensionFunctions
Comment Actions
Change 682707 merged by jenkins-bot:
[mediawiki/core@master] Warn about config changes done in ExtensionFunctions
Comment Actions
Change 710134 had a related patch set uploaded (by Legoktm; author: Legoktm):
[mediawiki/core@master] Support having groups inherit permissions from another group
Comment Actions
Change 710136 had a related patch set uploaded (by Legoktm; author: Legoktm):
[mediawiki/core@master] [WIP] Allow disabling user groups
Comment Actions
I took a stab at implementing what I proposed in T275334#6923618, group inheritance seems to be straightforward, disabling groups a little less so.
Comment Actions
Change 710134 merged by jenkins-bot:
[mediawiki/core@master] Support having groups inherit permissions from another group
Comment Actions
Change 747977 had a related patch set uploaded (by Legoktm; author: Legoktm):
[operations/mediawiki-config@master] Use $wgGroupInheritsPermissions for \"confirmed\" group
Comment Actions
I'm still interested in adding the "disable a group" functionality, which would close out this ticket, but blocked on resolving the dependency loop in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/710136
Comment Actions
Change 747977 merged by jenkins-bot:
[operations/mediawiki-config@master] Use $wgGroupInheritsPermissions for \"confirmed\" group
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2022-02-16T08:09:35Z] <urbanecm@deploy1002> Synchronized wmf-config/InitialiseSettings.php: 9001a8ce7d94408c9af072d4743e2cc9ab25abbe: Use $wgGroupInheritsPermissions for "confirmed" group (T275334; 1/2) (duration: 00m 51s)
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2022-02-16T08:13:40Z] <urbanecm@deploy1002> Synchronized wmf-config/CommonSettings.php: 9001a8ce7d94408c9af072d4743e2cc9ab25abbe: Use $wgGroupInheritsPermissions for "confirmed" group (T275334; 2/2) (duration: 03m 39s)