The blockedtext-tempuser message is being shown by PermissionsManager, while the blockedtext message is being shown by EditPage.php#942. Above the code in EditPage, I see the comment:

// Check if the user is blocked from editing.
// This check must be done on the context user, in order to trigger
// checks for blocks against IP address, XFF, etc, until T221067

T221067: Move block-related methods on User to the BlockManager doesn't appear to have been worked on, but it seems perhaps there was some unintended overlap between what was planned for T221067 and what has been done recently for T345683: Review of MediaWiki block-related code. Removing the code below the aforementioned comment causes only blockedtext-tempuser to be shown, which I believe is what we want. However I haven't tested the different code paths to ensure there's no missing block messages, etc. I could dig through and figure this out, but I feel like @tstarling and @Tchanders would probably know off-hand whether the code in EditPage is still needed. I also advise ensuring we're not stepping on any toes between T221067 and T345683. Can either of you speak to that? Thanks!

It's probably also worth mentioning that blockedtext-tempuser seems to expose the IP to the user. Is that intentional? I would guess that with Temporary accounts enabled, we wouldn't want to show the IP, even though it is the user's actual IP (so not really a security concern, per se).

In T350116#9332184, @MusikAnimal wrote:

It's probably also worth mentioning that blockedtext-tempuser seems to expose the IP to the user. Is that intentional? I would guess that with Temporary accounts enabled, we wouldn't want to show the IP, even though it is the user's actual IP (so not really a security concern, per se).

I strongly suspect that revealing the IP address only to the user themselves would be OK. If not, we'd have to update the block message for fully-registered users, and probably do an audit of where else we show the IP address. @Niharika Could you confirm this is the case? We could add it to the documentation if so.

What's happening here?

The extra block check was added to solve T327307: Blocked temporary account user should see block message on edit page when their IP address is blocked. In short, when a logged-out user is trying to make their first edit, and IP masking is enabled, we check their permissions against a placeholder temporary account. Because this user is different from the (global) context user, the IP address, XFF, cookies, etc weren't being checked for blocks (see docblock here). So we added an extra check for these.

Since https://gerrit.wikimedia.org/r/c/mediawiki/core/+/964610, the PermissionManager looks for IP, XFF, cookie, etc blocks regardless of whether the user is the context user, but we don't find that block when checking User::getBlock, since that goes via the old route - so we still do the extra check, but unnecessarily.

How can we fix it?

Simply: we can remove the extra check, now that PermissionManager checks the right thing. (This task.)

Long story (beyond scope of this task): We were using the placeholder temporary users for permissions checks so that we could disallow anon (IP) users from editing (or whatever action) in the config, but still allow them to edit if the edit will create a temporary account. However, we're going to change that approach as summarised in T336187#9341749: we'll check against the anon (IP) user, and the config must be set to allow the * group to perform the action. We decided this so that other features wouldn't have to check against a temporary placeholder user. But it should also mean we can stop checking against a temporary placeholder user from EditPage too.

Change 977680 had a related patch set uploaded (by Tchanders; author: Tchanders):

[mediawiki/core@master] EditPage: Remove unnecessary extra check for blocks

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

Change 977680 merged by jenkins-bot:

[mediawiki/core@master] EditPage: Remove unnecessary extra check for blocks

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

In T350116#9365050, @gerritbot wrote:

Change 977680 merged by jenkins-bot:

[mediawiki/core@master] EditPage: Remove unnecessary extra check for blocks

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

@Tchanders I think since this change we no longer block globally blocked IPs if they appear in the XFF header, but only on the Edit page (not when trying to edit via the API) and only if IP Masking is enabled.

In T350116#9374582, @dom_walden wrote:

In T350116#9365050, @gerritbot wrote:

Change 977680 merged by jenkins-bot:

[mediawiki/core@master] EditPage: Remove unnecessary extra check for blocks

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

@Tchanders I think since this change we no longer block globally blocked IPs if they appear in the XFF header, but only on the Edit page (not when trying to edit via the API) and only if IP Masking is enabled.

@Tchanders @dom_walden should we revert the patch, then, until we have a fix in place for that scenario? Or make a follow-up task given that we don't have temporary accounts configuration enabled in production yet?

@kostajh I don't think this needs reverting, since it's not enabled in production. I'll look into it and decide whether it needs a fix here or another task.

@Tchanders I think since this change we no longer block globally blocked IPs if they appear in the XFF header, but only on the Edit page (not when trying to edit via the API) and only if IP Masking is enabled.

@dom_walden This is what I've found - is that the same as what you meant?

Setup

• Enable temporary accounts
• Globally block an IP address
• Log out
• Try to edit from an unblocked IP address, but with the blocked IP address in your XFF header

What I observed, with this change

• No block message is shown
• On trying to submit the edit, the XFF IP block prevents it

What I observed, before this change

• A single block message is shown for the XFF IP block (as opposed to duplicated messages as per the task description)

The problem is that GlobalBlocking is still checking for the global/context/session user before it decides whether to check XFF blocks (unlike core, which was updated not to do this - see T350116#9359068). When editing for the first time as a logged out user with temporary accounts enabled, the session user is the IP user, but the user passed to GlobalBlocking is the placeholder temporary account.

I'll file two follow-up tasks here, and doing either or both of these will solve the problem of GlobalBlocking not finding XFF blocks:

1. GlobalBlocking should be using the same conditions as core to decide whether to check for XFF blocks
2. We may be able to stop using a placeholder temporary account for permissions checks from EditPage

In T350116#9386699, @Tchanders wrote:

@Tchanders I think since this change we no longer block globally blocked IPs if they appear in the XFF header, but only on the Edit page (not when trying to edit via the API) and only if IP Masking is enabled.

@dom_walden This is what I've found - is that the same as what you meant?

Yes, that's the one.

I'll file two follow-up tasks here, and doing either or both of these will solve the problem of GlobalBlocking not finding XFF blocks:

Thanks. I will move this task along.