Page MenuHomePhabricator
Create Task
Maniphest T409718

Remove $wgCheckUserGroupRequirements and related code
Closed, ResolvedPublic
Actions

Description

Background

The CheckUser config $wgCheckUserGroupRequirements and the core hook SpecialUserRightsChangeableGroups were added in T393615: Impose technical restrictions on granting the `temporary-account-viewer` group as a quick way to enforce group requirements.

Following T409717: Configure temporary-account-viewer group to use RestrictedGroups config, these are no longer needed, so they should be removed.

Acceptance criteria
  • Remove $wgCheckUserGroupRequirements, SpecialUserRightsChangeableGroups and related code

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Remove SpecialUserRightsChangeableGroups hookmediawiki/coreREL1_45+1 -206
Remove SpecialUserRightsChangeableGroups hookmediawiki/coremaster+0 -210
Remove CheckUserGroupRequirementsmediawiki/extensions/CheckUserREL1_45+5 -571
Remove CheckUserGroupRequirementsmediawiki/extensions/CheckUsermaster+5 -571
Customize query in gerrit

Related Objects
Search...

Event Timeline

Tchanders created this task.Nov 10 2025, 11:47 AM
Tchanders added a subtask: T409714: Update UserGroupAssignmentService to check restricted groups.
Tchanders added a subtask: T409717: Configure temporary-account-viewer group to use RestrictedGroups config.
Tchanders removed a subtask: T409714: Update UserGroupAssignmentService to check restricted groups.Nov 10 2025, 11:49 AM
Tchanders edited projects, added Product Safety and Integrity (Crepes au Chocolat (Sprint Nov 10 - Nov 28)); removed Product Safety and Integrity (Sprint Mint Choc Chip Ice Cream (Oct 20 - Nov 7)).
mszwarc mentioned this in T352871: Factor user registration loading to LocalUserRegistrationProvider.Nov 12 2025, 2:49 PM
Tchanders moved this task from Backlog to Ready on the Product Safety and Integrity (Crepes au Chocolat (Sprint Nov 10 - Nov 28)) board.Nov 14 2025, 10:02 AM
Reedy closed subtask T409717: Configure temporary-account-viewer group to use RestrictedGroups config as Resolved.Nov 26 2025, 11:25 AM

Do we want to deprecate CheckUserGroupRequirements et al in any way? Or just remove it wholesale as part of the 1.46 cycle?

Reedy claimed this task.Nov 26 2025, 11:26 AM
Reedy added a comment.Nov 26 2025, 11:31 AM

Looks like it was added in rECHU46a3c65e82d5: Enforce requirements for temporary-account-viewer group under T393615: Impose technical restrictions on granting the `temporary-account-viewer` group.

Is it worth preventing it shipping in MW-1.45-release too? Obviously we wouldn't be backporting all the changes to add the "same" functionality to MW core and CentralAuth etc.

But if we never ship it in a release, it's never considered stable sooooo....

Dreamy_Jazz subscribed.EditedNov 26 2025, 12:07 PM
In T409718#11408980, @Reedy wrote:

Looks like it was added in rECHU46a3c65e82d5: Enforce requirements for temporary-account-viewer group under T393615: Impose technical restrictions on granting the `temporary-account-viewer` group.

Is it worth preventing it shipping in MW-1.45-release too? Obviously we wouldn't be backporting all the changes to add the "same" functionality to MW core and CentralAuth etc.

But if we never ship it in a release, it's never considered stable sooooo....

Could it be marked as unstable in the MW-1.45-release? It would avoid the need to drop the functionality entirely in a release, but makes it clear that in a future wiki version it will no longer exist. IMO I would expect that WMF wikis will be the only users of this feature as it stands in CheckUser.

Reedy added a comment.Nov 26 2025, 12:13 PM

Yeah, I'd presume no one else is likely to use it, or necessarily have a desperate need for this functionality in MW-1.45-release.

I'm not really fussed either way we do this, but definitely want to do something in the release before it's final.

It's not documented on https://www.mediawiki.org/wiki/Extension:CheckUser, but it probably should be if it comes out in a release.

I don't think it'll be much work to effectively just revert that commit in master and cherrypick/similar, so I don't have massively strong feelings either way.

Reedy added a comment.Nov 26 2025, 1:28 PM

rECHUfcb4ac4b23ed: Cache external permissions checks for Special:GlobalContributions adds some overlap of code usages too, using the same hook handler.

gerritbot added a comment.Nov 26 2025, 1:36 PM

Change #1211675 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CheckUser@master] Remove CheckUserGroupRequirements

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

gerritbot added a project: Patch-For-Review.Nov 26 2025, 1:36 PM
Tchanders moved this task from Ready to In progress on the Product Safety and Integrity (Crepes au Chocolat (Sprint Nov 10 - Nov 28)) board.Nov 26 2025, 2:06 PM
Tchanders added a comment.Nov 26 2025, 2:26 PM
In T409718#11409094, @Dreamy_Jazz wrote:
In T409718#11408980, @Reedy wrote:

Looks like it was added in rECHU46a3c65e82d5: Enforce requirements for temporary-account-viewer group under T393615: Impose technical restrictions on granting the `temporary-account-viewer` group.

Is it worth preventing it shipping in MW-1.45-release too? Obviously we wouldn't be backporting all the changes to add the "same" functionality to MW core and CentralAuth etc.

But if we never ship it in a release, it's never considered stable sooooo....

Could it be marked as unstable in the MW-1.45-release? It would avoid the need to drop the functionality entirely in a release, but makes it clear that in a future wiki version it will no longer exist. IMO I would expect that WMF wikis will be the only users of this feature as it stands in CheckUser.

I think we should drop CheckUserGroupRequirements, and the SpecialUserRightsChangeableGroups hook if possible. They were only added for our IP viewer use-case, and not easily generalizable. It's difficult to imagine third parties already adopting it.

Reedy added a comment.Nov 26 2025, 2:38 PM

I think it should be a trivial backport (to REL1_45) from master, so if no one has any good reason to keep it around, let's get rid!

gerritbot added a comment.Nov 26 2025, 8:45 PM

Change #1211786 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/core@master] Remove SpecialUserRightsChangeableGroups hook

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

gerritbot added a comment.Nov 27 2025, 9:25 AM

Change #1211675 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Remove CheckUserGroupRequirements

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

ReleaseTaggerBot added a project: MW-1.46-notes (1.46.0-wmf.5; 2025-12-02).Nov 27 2025, 10:00 AM
gerritbot added a comment.Nov 27 2025, 10:06 AM

Change #1212079 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CheckUser@REL1_45] Remove CheckUserGroupRequirements

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

gerritbot added a comment.Nov 27 2025, 10:43 AM

Change #1212079 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_45] Remove CheckUserGroupRequirements

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

gerritbot added a comment.Nov 27 2025, 11:08 AM

Change #1211786 merged by jenkins-bot:

[mediawiki/core@master] Remove SpecialUserRightsChangeableGroups hook

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

gerritbot added a comment.Nov 27 2025, 11:15 AM

Change #1212105 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/core@REL1_45] Remove SpecialUserRightsChangeableGroups hook

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

OKryva-WMF moved this task from In progress to Needs review on the Product Safety and Integrity (Crepes au Chocolat (Sprint Nov 10 - Nov 28)) board.Nov 27 2025, 1:00 PM
Reedy added a comment.Nov 27 2025, 5:46 PM

The backport for REL1_45 is a bit annoying...

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1212105/2 (not PS2, not PS3)

I guess the little bit of divergence from/lag behind master is unhelpful, and chasing a load of backports probably won't be a good idea.

The error messages seem to revolve around the same thing..

PS3 is ontop of a backport of https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1212197/1/includes/user/UserGroupAssignmentService.php which itself is going to need more changes (hence the comment above)...

That commit adds a $groups['restricted'] = []; and there is a change to other setting of restricted...

16:35:29 There were 8 errors:
16:35:29 
16:35:29 1) SpecialUserRightsTest::testShowForm
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:134
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:131
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:60
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageTestBase.php:81
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:46
Logs generated by test
16:35:29 
16:35:29 2) SpecialUserRightsTest::testShowFormViewMode
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:134
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:131
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:60
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageTestBase.php:81
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:59
Logs generated by test
16:35:29 
16:35:29 3) SpecialUserRightsTest::testSaveUserGroups
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:134
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:131
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:60
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageTestBase.php:81
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:91
Logs generated by test
16:35:29 
16:35:29 4) SpecialUserRightsTest::testSaveUserGroups_change
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:134
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:131
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:60
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageTestBase.php:81
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:142
Logs generated by test
16:35:29 
16:35:29 5) SpecialUserRightsTest::testSaveUserGroups_change_expiry
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:134
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:131
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:60
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageTestBase.php:81
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:173
Logs generated by test
16:35:29 
16:35:29 6) SpecialUserRightsTest::testDisplayCurrentGroups
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/vendor/wikimedia/testing-access-wrapper/src/TestingAccessWrapper.php:114
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:278
Logs generated by test
16:35:29 
16:35:29 7) SpecialUserRightsTest::testSystemUserNotice
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:134
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:131
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:60
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageTestBase.php:81
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:311
Logs generated by test
16:35:29 
16:35:29 8) SpecialUserRightsTest::testSupportsWatchUser with data set "User on local wiki" (Closure Object (...), true)
16:35:29 Undefined array key "restricted"
16:35:29 
16:35:29 /workspace/src/includes/specialpage/UserGroupsSpecialPage.php:88
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:204
16:35:29 /workspace/src/includes/specials/SpecialUserRights.php:134
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:131
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageExecutor.php:60
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialPageTestBase.php:81
16:35:29 /workspace/src/tests/phpunit/includes/specials/SpecialUserRightsTest.php:318
Logs generated by test
16:35:29 
16:35:29 --
16:35:29 
16:35:29 There were 2 failures:
16:35:29 
16:35:29 1) UserGroupAssignmentServiceTest::testGetChangeableGroups with data set "Not self" (false)
16:35:29 Failed asserting that two arrays are identical.
16:35:29 --- Expected
16:35:29 +++ Actual
16:35:29 @@ @@
16:35:29      'remove' => Array &2 (
16:35:29          0 => 'remove-group1'
16:35:29      )
16:35:29 -    'restricted' => Array &3 ()
16:35:29  )
16:35:29 
16:35:29 /workspace/src/tests/phpunit/includes/user/UserGroupAssignmentServiceTest.php:166
Logs generated by test
16:35:29 
16:35:29 2) UserGroupAssignmentServiceTest::testGetChangeableGroups with data set "Self" (true)
16:35:29 Failed asserting that two arrays are identical.
16:35:29 --- Expected
16:35:29 +++ Actual
16:35:29 @@ @@
16:35:29          0 => 'remove-group1'
16:35:29          1 => 'remove-group2'
16:35:29      )
16:35:29 -    'restricted' => Array &3 ()
16:35:29  )
16:35:29 
16:35:29 /workspace/src/tests/phpunit/includes/user/UserGroupAssignmentServiceTest.php:160
Logs generated by test
Tchanders added a comment.Nov 27 2025, 5:49 PM

Ah. If backporting it is more costly than the deprecation, then we can just deprecate...

Reedy added a comment.Nov 27 2025, 5:50 PM

Lets see if https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1212105/4 passes, because I suspect it's going to be a tweak of something along those lines

Reedy added a comment.Nov 27 2025, 6:19 PM

Yeah, looks like https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1212105/4/includes/user/UserGroupAssignmentService.php#132 is all that was missing. Yay

gerritbot added a comment.Nov 27 2025, 6:36 PM

Change #1212105 merged by jenkins-bot:

[mediawiki/core@REL1_45] Remove SpecialUserRightsChangeableGroups hook

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

Reedy closed this task as Resolved.Nov 27 2025, 6:45 PM
ReleaseTaggerBot added a project: MW-1.45-notes.Nov 27 2025, 7:00 PM
Maintenance_bot removed a project: Patch-For-Review.Nov 27 2025, 7:30 PM
EMill-WMF added a comment.Nov 28 2025, 7:08 PM

Yay for deleting code!!

EMill-WMF awarded a token.Nov 28 2025, 7:08 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits