Page MenuHomePhabricator

Forcibly creating a local account causes autoblocks for the user to affect the creating administrator's IP address
Open, MediumPublicBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

  • Forcibly create an account for a user (Special:CreateLocalAccount)
  • Block the new account

What happens?:

  • There is now an autoblock on the creating administrator's IP address

What should have happened instead?:

  • The autoblock should not have been created.

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc.:
Happened on enwiki (MediaWiki 1.39.0-wmf.4)

This has probably never been noticed because a) The forcible creation of a local account is rare, b) that account then being blocked is rare, and c) administrator accounts are unaffected by autoblocks. Their mobile unprivileged accounts, however, are affected.

Event Timeline

I happen to have force-created an account on testwiki a while ago for T302771, so I just tried blocking it there. An autoblock was made, but there's no block notice if I go to edit logged-out... I'm guessing the difference is my IP has probably changed since I force-created the account a month ago, and by blocking the "last IP address" used by the account I force-created, it's really just blocking whatever IP I had the day I created it?

taavi added a project: CheckUser.
taavi subscribed.

There are two code paths leading up to an IP-based autoblock:

  • Values of recentchanges.rc_ip in DatabaseBlockStore::performRetroactiveAutoblock
  • Values of cu_changes.cuc_ip in CheckUser's Hooks::doRetroactiveAutoblock

I verified that rc_ip does not get "wrong" values for the actor ID of the created user. So it must be an issue with the CheckUser code path. Will investigate more, but first I need to set up CU locally.

Wouldn't be a first. In general, CU code base doesn't handle IP association too well when an action is taking place that typically involves a different number of users than usual (and to all the blame is on CU code, it is partly on how we invoke it through hooks):

  • This task is about a workflow that typically involves one user (the target user is typically the one who creates the local account), but in the unusual case that it involves two accounts (the target user and a sysop, the second user who activates the account on behalf of the first user) the attribution of IP and block is poorly done
  • T44345: Blocks from AbuseFilter show up as performed from the target's IP address in Checkuser is about a workflow that typically involves *two* users (typically a human user blocks a target user), but in the unusual case that the first user is replaced by a faux user, namely the AbuseFilter "user", then the IP assignment is again done in an arguably "wrong" way.

I think this is all a reflection that CU hooks should explicitly expect two users (target and actor) along with two sets of IP/UA information to be passed. In many cases, you may choose to pass the same info, but in some cases you may choose to leave one is blank or as a non-specific value (e.g. tasks that are run as jobs get the IP of the server, or for T44345 you may use the server IP too, or as far as the current task is concerned, you may want to make sure the actor (i.e. admin's) IP is not passed in for the block

Oh yeah, see onLocalUserCreated and logUserAccountCreation in CheckUser's Hooks.php. The code assumes that the current client IP is always associated with the account being created.

taavi removed taavi as the assignee of this task.May 16 2022, 5:29 AM
This comment was removed by Dreamy_Jazz.

Actually I was wrong about this affecting ACC, though auto-blocks still were causing issues. I think this is still unresolved.

There are two ways that I see to solve this issue:

  1. The retroactive autoblock hook should ignore any cu_changes items that are forcible creations of local accounts - doing this would require CU data to be stored in a structured way in the DB as this would be the way that the CU extension could determine after the fact that it was a forcible local creation. This therefore would require T145265 to be worked on which has been open without action since 2016 and I don't think there are plans to do so in the near future.
  2. The cu_changes table stores an internal IP or invalid IP (such as the text 'Software triggered action' or nothing) that is either safe to autoblock or is ignored by the CU extension for autoblocks either due to an exclusion list or it being not a valid target for a block. Ideally I would want to see no autoblock. This would be possible once T311340 is merged which has an open patch.
Dreamy_Jazz changed the task status from Open to Stalled.Sep 29 2022, 8:25 PM

Waiting on subtask to have it's patch merged.

Dreamy_Jazz changed the task status from Stalled to Open.Oct 11 2022, 7:45 PM

Subtask is merged and progress can start.