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)

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.

In T275334#6923618, @Legoktm wrote:

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.

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.

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.

In T275334#6989015, @Jdforrester-WMF wrote:

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.

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.

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.

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

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

Change 682560 merged by jenkins-bot:

[mediawiki/core@REL1_36] Warn about config changes done in ExtensionFunctions

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

Change 682707 had a related patch set uploaded (by Jforrester; author: Gergő Tisza):

[mediawiki/core@master] Warn about config changes done in ExtensionFunctions

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

Change 682707 merged by jenkins-bot:

[mediawiki/core@master] Warn about config changes done in ExtensionFunctions

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

Change 710134 had a related patch set uploaded (by Legoktm; author: Legoktm):

[mediawiki/core@master] Support having groups inherit permissions from another group

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

Change 710136 had a related patch set uploaded (by Legoktm; author: Legoktm):

[mediawiki/core@master] [WIP] Allow disabling user groups

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

Change 710134 merged by jenkins-bot:

[mediawiki/core@master] Support having groups inherit permissions from another group

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

Change 747977 had a related patch set uploaded (by Legoktm; author: Legoktm):

[operations/mediawiki-config@master] Use $wgGroupInheritsPermissions for \"confirmed\" group

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

Change 747977 merged by jenkins-bot:

[operations/mediawiki-config@master] Use $wgGroupInheritsPermissions for \"confirmed\" group

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

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)

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)

I've updated the documentation, as it was still recommending the $wgExtensionFunctions way with no warnings (and still mentioning require_once for loading extensions). FWIW, I agree that there should be a better way of doing this than with extension callbacks, and that's regardless of what does and doesn't work in current MW. Given the lack of progress on this task, I'm wondering if the docs should just reocmmend the MediaWikiServices hook instead.

Probably should, since this task is dying.

Change #1124811 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[operations/mediawiki-config@master] Unset 'push-subscription-manager' group using hook callback

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

Change #1124811 abandoned by Dreamy Jazz:

[operations/mediawiki-config@master] Unset 'push-subscription-manager' group using hook callback

Reason:

Duplicate to Ib3668159e3c35a98560777a497053261ec45b567

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

Change #710136 abandoned by Hashar:

[mediawiki/core@master] [WIP] Allow disabling user groups

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

Change #710136 restored by Thcipriani:

[mediawiki/core@master] [WIP] Allow disabling user groups

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

See also T271574: Phase out $wgExtensionFunctions

At a glance, I think this should be done with registration callbacks instead. @Urbanecm do you remember the context of this task? Is it still an issue? Can it be closed, or is there code that might still be broken by this issue?

Before closing this make sure registration callbacks don't have the same issue.

In T275334#12146008, @daniel wrote:

At a glance, I think this should be done with registration callbacks instead. @Urbanecm do you remember the context of this task? Is it still an issue? Can it be closed, or is there code that might still be broken by this issue?

That seems ideal, AFAIK.

(Reminder to update the docs with the new proposed solution if it's confirmed that it works)

IIRC callbacks were too early, although I don't remember the specifics. We actually used the MediaWikiServices hook as a replacement for most affected code (e.g. FlagRev).

In T275334#12149284, @Tgr wrote:

IIRC callbacks were too early, although I don't remember the specifics. We actually used the MediaWikiServices hook as a replacement for most affected code (e.g. FlagRev).

That sounds like a workable solution, even though it doesn't quite match the intent of that hook.

Can we close this ticket? Should we update the documentation to point to MediaWikiServices as an alternative to registration callbacks and extension functions?