Problem
Before UserIsHidden is deprecated in T228948, CentralAuth will need to be updated.
Proposed Solution
Replace the use of UserIsHidden with GetUserBlock (which replaces GetBlockedStatus)
Problem
Before UserIsHidden is deprecated in T228948, CentralAuth will need to be updated.
Proposed Solution
Replace the use of UserIsHidden with GetUserBlock (which replaces GetBlockedStatus)
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Resolved | dbarratt | T228950 Replace UserIsHidden with GetUserBlock in CentralAuth | |||
Resolved | • Tchanders | T228948 PermissionManager::isBlockedFrom() can return true even if the user does not have a block |
Change 527251 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/CentralAuth@master] Use the GetUserBlock hook instead of UserIsHidden
Change 527251 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Use the GetUserBlock hook instead of UserIsHidden
If a user is hidden via Special:CentralAuth, any blocks they have (unless they are hidden blocks) will be overridden by a system block.
System blocks don't block Special:EmailUser and nor does CentralAuth[1]. If the user has a non-hidden block applying to email, it will no longer be effective (and user will be able to email).
Normally when Special:CentralAuth hides a user it also automatically creates hidden database blocks locally on each connected wiki. Therefore, the above is not a problem.
But, there may be cases where these blocks aren't created automatically, for example where there is a preexisting database block on that user. Or if someone decides to replace the automatically created block (I don't know why).
@Niharika Is this a concern? Perhaps I should just raise a bug on CentralAuth not blocking Special:EmailUser (I have been meaning to do this for a while).
(also if @Tchanders could confirm what I said above. I'm pretty sure it's right.)
P.S. some things that list users or user info (e.g. Special:ListUsers) get info about a user's hidden status directly from the database. Therefore, only hidden database blocks are effective, not system blocks. So, for example, a globally hidden user can still be seen in Special:ListUsers, unless they have a local hidden block. But, this is no different to what it was before.
Notes
It sounds like the following would be a better approach:
Special:EmailUser does not use PermissionManager [...]
some things that list users or user info (e.g. Special:ListUsers) get info about a user's hidden status directly from the database [...]
These sound worth filing and fixing to me.
Thanks for the detailed explanation, @dom_walden!
This makes sense to me as well.
Special:EmailUser does not use PermissionManager [...]
some things that list users or user info (e.g. Special:ListUsers) get info about a user's hidden status directly from the database [...]
These sound worth filing and fixing to me.
Me too. @dom_walden It'd be great if you can file these. :)
Change 533910 had a related patch set uploaded (by Tchanders; owner: Tchanders):
[mediawiki/extensions/CentralAuth@master] Add to existing blocks if a blocked user is centrally hidden
Change 533910 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Add to existing blocks if a blocked user is centrally hidden
Change 534849 had a related patch set uploaded (by Dbarratt; owner: Dbarratt):
[mediawiki/extensions/CentralAuth@master] Allow users to be hidden from lists, without being blocked
@Niharika there is an open question from @Tchanders, I think we talked about it but didn't come to a resolution:
With this change, the account is no longer hidden when an admin chooses "Account is hidden from the global users list" in Special:CentralAuth.
If we wanted to allow a user to be hidden but not blocked, we'd need to reintroduce something like the UserIsHidden hook, and set the mHideName flag directly on the User, without setting a block.
I don't think we should do that - the fact that a user cannot now be hidden without a block is not a regression, but an intended consequence of deprecating the UserIsHidden hook (explained in T228948).
I think that option only effects that single page, so we can either remove the option completely, or fix the patch.
Hmmm, without any data about how often it is used, it's hard to make a judgement call on this. I'm fine with Thalia's suggestion to remove the option.
Can we find out if that option is also used elsewhere?
@Tchanders & @Niharika when the UI says: "global users list" is that just Special:GlobalUsers ? If so, it looks like GlobalUsersPager::getQueryInfo to ensure that only users who are not hidden in any way are shown:
$conds = [ 'gu_hidden' => CentralAuthUser::HIDDEN_NONE ];
or are we talking about some other page?
Perhaps hiding users on non-global pages was an unintended consequence of the previous hook? Or rather... it looks like what the UI says and what the code was actually doing is not the same thing?
Based on the wording, I would assume it just means Special:GlobalUsers. Let's work with that assumption.
Perhaps hiding users on non-global pages was an unintended consequence of the previous hook? Or rather... it looks like what the UI says and what the code was actually doing is not the same thing?
I don't think I understand this fully. Are you saying that the option was not working as expected and nobody noticed?
I'm saying the previous behavior was that the user would be removed from local user lists (like Special:ListUsers) but not have a block. I guess I'm saying it didn't work as the UI said it would, but I'm not sure what the users actually expect now.
I think the behavior should match the UI. We can either restore the previous behavior and change the UI, or fix the behavior to match the UI, but right now it's inconsistent and has been for (I assume) some time.
The behavior should definitely match the UI. I am inclined to go with @Tchanders' suggestion to remove the option completely for now. We have no data about whether it was used and what the use case was. And that nobody (to our knowledge) reported the behavior/UI mismatch tells me that it was probably not very popular. If we hear that someone misses that option, we can reconsider what we want to do with it but let's get rid of it until then. I believe that's also the least amount of work for us from all the options, right?
I believe the least amount of work would be making the option consistent with the UI, as that is already done with this patch https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralAuth/+/534849 :)
Change 534849 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Allow users to be hidden from lists, without being blocked
Sorry for a late reply. That would be fine by me and I believe that's what we ended up with doing?
A user who is "Account is hidden from the global users list" does not appear in Special:GlobalUsers, but the user is not blocked from doing anything.
A user who is "Account is hidden completely" does not appear in Special:GlobalUsers, has local sitewide blocks created on connected wikis, and is blocked with a system block.
Tested on my local vagrant environment 1.35.0-alpha (869164a) 17:18, 27 October 2019 (as I don't have CentralAuth permissions on beta).