While this could be handled in downstream code, I think it would make more sense (and prevent accidental disclosures) to handle it by default in HTMLUserTextField, when checking for user existence. That would make the default behavior consistent with other pages like Special:Contribs.

Not sure if it was clear but Miraheze use CentralAuth. Not sure if that is the issue or just the forms system in general.

So addressing this within HTMLUserTextField would likely involve adding a check of $user->getBlock()->getHideName() within this conditional block, though not sure if it makes more sense in the if or elseif, given the slight differences in error messages.

@mmartorana: hi, you changed this to 'In Progress' - who exactly is working on this?

In T309894#7987574, @sbassett wrote:

So addressing this within HTMLUserTextField would likely involve adding a check of $user->getBlock()->getHideName() within this conditional block, though not sure if it makes more sense in the if or elseif, given the slight differences in error messages.

htmlform-user-not-exists sounds like the correct one in this case I think.

Hi @Majavah, in progress means that we are actively processing this task, not necessarily that we are working on a patch.

In this case, since this pertains MediaWiki core, Platform Engineering and Editing-team should be in charge of that.

Can you explain why Editing-team? Sure, we would be able to help if there's somehow no one else capable of resolving this, but I am very confused why anyone would think we're responsible for it…

Hi @matmarex - we may have made a mistake in our assignment, but the reasoning was likely due to the fact that you are part of the Editing-team.

Could you please point me to someone capable of resolving this?

Thank you

Security patch:

There are several non-security bugs in the existing logic though: T177329, T311948. I submitted a patch to fix those to Gerrit: https://gerrit.wikimedia.org/r/c/813727. If it is merged, this is the security patch to use instead:

The behavior can be most easily tested on Special:EmailUser.

@matmarex thanks a lot.

Hey all -

I just deployed @matmarex's v1 patch from above (thanks again) to 1.39.0-wmf.19. Tested with enwiki:Special:EmailUser on mwdebug1002 and then in prod and all looks good - it seems to be throwing the correct error message for hidden users now. No related errors in logstash either. If anybody sees anything off about it, let me know. Otherwise, this task/patch should be held for the next mediawiki security release (T311775).

In T309894#8076745, @matmarex wrote:

But if you're looking for someone responsible for it, then there is no one. In the past, members of the Security team would have resolved security issues with no maintainers like this.

Just a follow-up note on this - yes, this is true in that members of the Security Team were traditionally tasked with "resolving" security issues reported for any Wikimedia codebase. Unfortunately, given the enormous backlog of security issues that accumulated under this policy, this was never a sustainable policy and is quite far from the industry standard of code maintainers owning the risk for their security issues. I think we've failed as a team in communicating this concept to the Foundation and Community over the past several years, but this is going to change in the very near future when our team begins pushing smaller, more frequent updates out over multiple communications channels. It is hoped that this will better set expectations regarding responsibilities, policies and workflows related to the current incarnation of the Security Team.

In T309894#8086210, @sbassett wrote:

Hey all -

I just deployed @matmarex's v1 patch from above (thanks again) to 1.39.0-wmf.19.

Thanks for the deployment. I ran into your patch when doing another, unrelated deployment (the applied version of the patch did not have any prefix identifying it as a secpatch, so I had to check whether it is an unmarked sec patch, or uncommitted code). Since it turns out to be a sec patch, I slightly changed the patch file to include the SECURITY: prefix, to avoid confusing others. FTR, this is the currently applied version:

In T309894#8086253, @sbassett wrote:

In T309894#8076745, @matmarex wrote:

But if you're looking for someone responsible for it, then there is no one. In the past, members of the Security team would have resolved security issues with no maintainers like this.

Just a follow-up note on this - yes, this is true in that members of the Security Team were traditionally tasked with "resolving" security issues reported for any Wikimedia codebase. Unfortunately, given the enormous backlog of security issues that accumulated under this policy, this was never a sustainable policy and is quite far from the industry standard of code maintainers owning the risk for their security issues. I think we've failed as a team in communicating this concept to the Foundation and Community over the past several years, but this is going to change in the very near future when our team begins pushing smaller, more frequent updates out over multiple communications channels. It is hoped that this will better set expectations regarding responsibilities, policies and workflows related to the current incarnation of the Security Team.

I understand, and I don't mean to say that you should be responsible for it, I agree that it's a bad situation to have a "security team" fixing all the issues in other people's code. However, there really is no one else responsible for it, and I'm afraid that just communicating that won't solve that issue – I think it's a management/leadership problem. I would actually enjoy doing more of this work, but right now I feel like I'm neglecting my more "official" responsibilities when I do.

In T309894#8086935, @Urbanecm wrote:

Thanks for the deployment. I ran into your patch when doing another, unrelated deployment (the applied version of the patch did not have any prefix identifying it as a secpatch, so I had to check whether it is an unmarked sec patch, or uncommitted code). Since it turns out to be a sec patch, I slightly changed the patch file to include the SECURITY: prefix, to avoid confusing others. FTR, this is the currently applied version:

Ok, thanks for fixing that and cleaning everything up on deployment.

In T309894#8087050, @matmarex wrote:

I understand, and I don't mean to say that you should be responsible for it, I agree that it's a bad situation to have a "security team" fixing all the issues in other people's code. However, there really is no one else responsible for it, and I'm afraid that just communicating that won't solve that issue – I think it's a management/leadership problem. I would actually enjoy doing more of this work, but right now I feel like I'm neglecting my more "official" responsibilities when I do.

Agreed. I think some combination of efforts will be necessary to enact any real change. Better communication and expectation-setting on the part of the Security-Team is one small thing we can easily change which could have an impact. I think some members of our team would also like to help write and CR more security patches, but we'll need to find a middle-ground between not doing any of this work and doing all of it, neither of which are reasonable positions IMO.

(https://gerrit.wikimedia.org/r/c/813727 was merged, so you'll need to v2 patch for next WMF branch)

In T309894#8096314, @matmarex wrote:

(https://gerrit.wikimedia.org/r/c/813727 was merged, so you'll need to v2 patch for next WMF branch)

Ok, thanks for the heads up. Looks like this didn't make 1.39.0-wmf.21, but I'll go ahead and update the patch on deployment for next week's train.

Sadly, it looks like the v2 patch fell off of production releases for a bit, but is now back on wmf.23 and wmf.25 and redeployed (sal 1, sal 2).

In T309894#8076784, @matmarex wrote:

Security patch:

v1-SECURITY-HTMLUserTextField-Treat-hidden-users-as-unr.patch1 KBDownload

There are several non-security bugs in the existing logic though: T177329, T311948. I submitted a patch to fix those to Gerrit: https://gerrit.wikimedia.org/r/c/813727. If it is merged, this is the security patch to use instead:

v2-SECURITY-HTMLUserTextField-Treat-hidden-users-as-unr.patch1 KBDownload

The behavior can be most easily tested on Special:EmailUser.

TLDR here/for confirmation...

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813727 made it into REL1_39 (and master), so it should get

For <= REL1_38, we should use

01-T309894.patch (F35321391) doesn't apply for me on master. It looks identical to my v1 patch. I wonder if you uploaded the right file?

In T309894#8266128, @matmarex wrote:

01-T309894.patch (F35321391) doesn't apply for me on master. It looks identical to my v1 patch. I wonder if you uploaded the right file?

I didn't upload any files, I just reused the linked files from above..

Oh, my bad, I didn't notice where it came from.

It looks like it was a replacement for the v1 patch with a tweaked commit message:

In T309894#8086935, @Urbanecm wrote:

Thanks for the deployment. I ran into your patch when doing another, unrelated deployment (the applied version of the patch did not have any prefix identifying it as a secpatch, so I had to check whether it is an unmarked sec patch, or uncommitted code). Since it turns out to be a sec patch, I slightly changed the patch file to include the SECURITY: prefix, to avoid confusing others. FTR, this is the currently applied version:

01-T309894.patch1 KBDownload

So, I think the correct procedure is: use the original patches on whichever branches they apply to, but change the commit message of both to read:

SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

…as I used the wrong convention for the prefix.

Yeah, I usually fix up descriptions etc when I apply the patches.

It's just trying to confirm upfront which patches should be applied, especially when we have multiple versions flying around.

In T309894#8076784, @matmarex wrote:

Security patch:

v1-SECURITY-HTMLUserTextField-Treat-hidden-users-as-unr.patch1 KBDownload

There are several non-security bugs in the existing logic though: T177329, T311948. I submitted a patch to fix those to Gerrit: https://gerrit.wikimedia.org/r/c/813727. If it is merged, this is the security patch to use instead:

v2-SECURITY-HTMLUserTextField-Treat-hidden-users-as-unr.patch1 KBDownload

The behavior can be most easily tested on Special:EmailUser.

^ I was going based on that, as https://gerrit.wikimedia.org/r/c/813727 had been merged, and then the superceded one...

Change 836892 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/core@REL1_35] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836897 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/core@REL1_37] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836901 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/core@REL1_38] HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836906 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/core@REL1_39] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836911 had a related patch set uploaded (by Reedy; author: Bartosz Dziewoński):

[mediawiki/core@master] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836892 merged by jenkins-bot:

[mediawiki/core@REL1_35] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836897 merged by Reedy:

[mediawiki/core@REL1_37] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836901 merged by Reedy:

[mediawiki/core@REL1_38] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836911 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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

Change 836906 merged by jenkins-bot:

[mediawiki/core@REL1_39] SECURITY: HTMLUserTextField: Treat hidden users as unregistered if current user can't view them

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