Page MenuHomePhabricator

Start running backfillLocalAccounts.php
Closed, ResolvedPublic

Description

The next step after T371267: Create a script to backfill missing local accounts on loginwiki/metawiki for new global accounts is to do a test run in production (e.g. autocreate the missing accounts for the last month on loginwiki), and if all went well, create a cronjob for running the script regularly.

One thing that probably needs to be fixed is that I think the script now requires an absolute start date, while for the cronjob it would be better to accept things like one month ago. (It could be solved in other ways, by complicating the cron command, but it's much harder to test / debug things there so probably easier to handle it on the MediaWiki side.)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@Tgr --startdate "5 days ago" works as expected, you're all set already.

I have a patch ready but no point in pushing it up before the script lands via the train. Maybe I'll just claim this task in advance anyways. Swipe!

Did some dry runs for loginwiki backfill. Results:

  • for one day's worth, 10622 uids checked, 44 accounts would be created.
  • for 7 days, 52582 uids checked, 232 accounts would be created, of which two were temporary accounts and would fail.
  • for 2 hours, 709 uids checked and two accounts would be created.

All dry runs took less than ten minutes.

I'll do a real run of the 2 hour interval now, looking for errors and timing impact; if that looks ok, I'll do a day's worth and collect the same info.

Typical test run (in screen on mwmaint2001):

date; bash mwscript extensions/CentralAuth/maintenance/backfillLocalAccounts.php  --wiki=loginwiki --startdate '1 hour ago' --dryrun  --verbose  ; date

I ran with '2 hours ago' and it was fine. One user was autocreated with an appropriate IP and user agent.

I ran with '--startdate yesterday', and there were several failures, because the IP address for the user had been globally blocked with reason 'no open proxies'.
As to the rest, a grand total of three accounts were backfilled with the proper IP and user agent.
The remaining 35 or so got the IP of 127.0.0.1 and an empty user agent string, because those accounts had all been force created by an admin responding to https://en.wikipedia.org/wiki/Wikipedia:Request_an_account

I am not sure how much value we are really adding here. But in any case, that run was also under ten minutes, so we could run this every hour if desired. What do the stewards have to say, given the above data?

Change #1088552 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[operations/puppet@production] systemd job to create missing local accounts on loginwiki/metawiki

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

One more comment:: do we want to check for global blocks in advance before trying to autocreate?

Change #1088995 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/extensions/CentralAuth@master] backfill accounts creation script now will skip globally blocked accounts

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

there were several failures, because the IP address for the user had been globally blocked with reason 'no open proxies'.

How/where are these accounts getting created in the first place if the IP is blocked?

The remaining 35 or so got the IP of 127.0.0.1 and an empty user agent string, because those accounts had all been force created by an admin responding to https://en.wikipedia.org/wiki/Wikipedia:Request_an_account

Are you fetching the correct data? These should have real IPs/UAs from the person that created the account.

To skip over temp account names and not attemp to create them until they are enabled on metawiki and loginwiki, we'd need to update InitialiseSettings.php to set $wgAutoCreateTempUser['known'] to true for those wikis, so that temp names will be recognized (otherwise we get a blanket false response from the check). Is that okay?

there were several failures, because the IP address for the user had been globally blocked with reason 'no open proxies'.

How/where are these accounts getting created in the first place if the IP is blocked?

The remaining 35 or so got the IP of 127.0.0.1 and an empty user agent string, because those accounts had all been force created by an admin responding to https://en.wikipedia.org/wiki/Wikipedia:Request_an_account

Are you fetching the correct data? These should have real IPs/UAs from the person that created the account.

There is no entry in the CheckUser tables we query, for forced account creation. Maybe there should be, I can't really say.

There is no entry in the CheckUser tables we query, for forced account creation. Maybe there should be, I can't really say.

Unless something is failing in CA/CU, the data should be there for accounts created by the ACC team.

I know because the loginwiki CU data lead to an incorrect mass lock of ~270 accounts created by one of the ACC team members in September, which I reversed after re-checking the data on loginwiki.

To skip over temp account names and not attemp to create them until they are enabled on metawiki and loginwiki, we'd need to update InitialiseSettings.php to set $wgAutoCreateTempUser['known'] to true for those wikis, so that temp names will be recognized (otherwise we get a blanket false response from the check). Is that okay?

Temp accounts are in use on loginwiki already and need to be created there, not skipped.

Change #1089023 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/extensions/CentralAuth@master] skip over temp usernames in backfill account creation script

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

@Tgr --startdate "5 days ago" works as expected, you're all set already.

Oh right, I forgot it already uses strtotime. Thanks for the tests!

How/where are these accounts getting created in the first place if the IP is blocked?

Presumably it gets blocked between account creation and the script running.

There is no entry in the CheckUser tables we query, for forced account creation. Maybe there should be, I can't really say.

Probably the cupe_log_action filter in AccountCreationDetailsLookup is missing the relevant log type?

To skip over temp account names and not attemp to create them until they are enabled on metawiki and loginwiki, we'd need to update InitialiseSettings.php to set $wgAutoCreateTempUser['known'] to true for those wikis, so that temp names will be recognized (otherwise we get a blanket false response from the check). Is that okay?

It was set on metawiki a while ago (e8809585). And they are enabled on loginwiki, so in theory both wikis should work.

I am not sure how much value we are really adding here.

Not much, right now. But that's because most of the time the account gets autocreated by central autologin, and SUL3 will remove central autologin for named users. So missing accounts would go up by 1-2 orders of magnitude.

One more comment:: do we want to check for global blocks in advance before trying to autocreate?

I wouldn't. It's one of many legitimate reasons for autocreation to fail. Others include local blocks, abuse filters, title blacklists etc. If you want to differentiate between normal an unexpected failures and not log the former, then probably the most robust way is to do something like

$authManager = $services->getAuthManager();
$providers = $authManager->getPreAuthenticationProviders() +
    $authManager->getPrimaryAuthenticationProviders() +
    $authManager->getSecondaryAuthenticationProviders();
foreach ( $providers as $provider ) {
    $status = $provider->testUserForCreation( $user, false, $options );
    if ( !$status->isGood() ) {
        // skip the user
    }
}

$status = new PermissionStatus();
$performer->definitelyCan( 'autocreateaccount', SpecialPage::getTitleFor( 'CreateAccount' ), $status );
if ( !$status->isGood() ) {
    // skip the user
}

I don't think it's worth the effort though. I think it's fine to just let such cases fail and log the failure. If we want to be honest, no one ever looks at maintenance script error output anyway.

If you are concerned about monitoring errors, I think a better approach might be to just send the autocreation status to Logstash, and there it's easy to filter out unwanted reasons. In fact, I think @pmiazga just did that in c49bb4bf. That doesn't differentiate between different call sites for autocreations, but you can filter logs by script name to get a good overview of errors (or even a dashboard, if wanted).

...

I don't think it's worth the effort though. I think it's fine to just let such cases fail and log the failure. If we want to be honest, no one ever looks at maintenance script error output anyway.

One thing about those errors is that we'll see an error for the same one 23 times, due to the run frequency and time period covered by the script. If that's still ok, I can abandon the other patches.

One thing about those errors is that we'll see an error for the same one 23 times, due to the run frequency and time period covered by the script. If that's still ok, I can abandon the other patches.

Yeah I think that's fine. If we later decide it's too annyoing we can always come back to it, or filter it in AuthManagerStatsdHandler (that's probably more flexible).

Unless something is failing in CA/CU, the data should be there for accounts created by the ACC team.

I know because the loginwiki CU data lead to an incorrect mass lock of ~270 accounts created by one of the ACC team members in September, which I reversed after re-checking the data on loginwiki.

@JJMC89 are you saying having a system IP for forced account creation is a problem for steward workflows? I'd have assumed that both the account creator's IP and the system IP are pretty useless for investigations so it doesn't matter.

@JJMC89 are you saying having a system IP for forced account creation is a problem for steward workflows? I'd have assumed that both the account creator's IP and the system IP are pretty useless for investigations so it doesn't matter.

Yes. Any (unblocked) user can create another account while logged in to their account, not just known good actors like admins or the ACC team. You may as well not even backfill a loginwiki account using a system IP as that has no value.

When using backfillLocalAccounts, the CU data on loginwiki should match the data on the account's origin wiki, just like if the loginwiki account were correctly created when the origin wiki account was.

Change #1089023 abandoned by ArielGlenn:

[mediawiki/extensions/CentralAuth@master] skip over temp usernames in backfill account creation script

Reason:

pursuant to https://phabricator.wikimedia.org/T378401#10310113

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

Change #1088995 abandoned by ArielGlenn:

[mediawiki/extensions/CentralAuth@master] backfill accounts creation script now will skip globally blocked accounts

Reason:

pursuant to https://phabricator.wikimedia.org/T378401#10310113 and previous discussion.

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

Change #1089824 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/extensions/CentralAuth@master] fix up account creation test for backfill script

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

Change #1089824 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] fix up account creation test for backfill script

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

Change #1090919 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/extensions/CheckUser@master] extend account creation lookup service to cover forced creations by others

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

Change #1090920 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/extensions/CentralAuth@master] extend accont creation backfill script to forced account creations by others

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

The above two patches, or something like them, might be able to handle the case of forced user creations by others. Leaving them here for comment and discussion.

@ArielGlenn asked me to take a look at the changes here. My understanding is that the backfilling would attempt to determine the IP for the creator via CheckUser, and that would work (per @JJMC89's request) for both self-created accounts and assisted-created accounts. Should this fail (due to a CU bug, for example), then the account would still be created, but using 127.0.0.1 as a placeholder IP.

Assuming that interpretation is correct, then it seems like it should cover all usecases for loginwiki CheckUser. I think a missing account would be better than a placeholder IP for the error case, as that is easier to see and harder to get confused by accidentally. Otherwise, this looks all good.

@JJMC89 Can I get your opinion on the above (Urbanecm's comment)? I want to make sure we are meeting everyone's needs here before we roll this out. Thanks!

Assuming that interpretation is correct, then it seems like it should cover all usecases for loginwiki CheckUser. I think a missing account would be better than a placeholder IP for the error case, as that is easier to see and harder to get confused by accidentally. Otherwise, this looks all good.

I was thinking the same thing.

FYI, the script should account for client hints once those are added in T345817.

Okay, the script has been updated to deal with account creations by other performers, if the IP/User Agent for those performers can be found, and to skip creation of accounts for any where the info is no longer available.

I did local testing of the different performer case, but kludging things enough for an integration test to fake a creation of a local account on another wiki, with a global account, with the right logging table entry etc, just felt like it was too fragile.

kludging things enough for an integration test to fake a creation of a local account on another wiki, with a global account, with the right logging table entry etc, just felt like it was too fragile.

With our current CI setup, such kind of tests are unfortunately nearly impossible. There is T379925: Add support for multi-DB testing to MediaWiki requesting a change in that domain.

Just to clarify: I think both the service and the maintenance script are ready for last "fix this please" or merge, since the requested functionality for stewards is present.

Change #1090919 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] extend account creation lookup service to cover forced creations by others

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

Change #1098956 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/extensions/CheckUser@wmf/1.44.0-wmf.5] extend account creation lookup service to cover forced creations by others

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

Change #1098965 had a related patch set uploaded (by ArielGlenn; author: ArielGlenn):

[mediawiki/extensions/CentralAuth@wmf/1.44.0-wmf.5] extend account creation backfill script to forced account creations by others

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

Change #1098965 abandoned by ArielGlenn:

[mediawiki/extensions/CentralAuth@wmf/1.44.0-wmf.5] extend account creation backfill script to forced account creations by others

Reason:

fixed a typo in the commit message in the original patch

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

Change #1090920 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] extend account creation backfill script to forced account creations by others

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

Change #1098965 restored by ArielGlenn:

[mediawiki/extensions/CentralAuth@wmf/1.44.0-wmf.5] extend account creation backfill script to forced account creations by others

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

Change #1098956 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@wmf/1.44.0-wmf.5] extend account creation lookup service to cover forced creations by others

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

Change #1098965 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@wmf/1.44.0-wmf.5] extend account creation backfill script to forced account creations by others

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

Mentioned in SAL (#wikimedia-operations) [2024-11-28T21:39:25Z] <tgr@deploy2002> Started scap sync-world: Backport for [[gerrit:1098990|Localisation updates (November 26) (T372175)]], [[gerrit:1098956|extend account creation lookup service to cover forced creations by others (T378401)]], [[gerrit:1098965|extend account creation backfill script to forced account creations by others (T378401)]], [[gerrit:1098929|ReportIncident: Setup $wgReportIncidentLocalLinks for ptwiki pilot deplo

Mentioned in SAL (#wikimedia-operations) [2024-11-28T21:53:42Z] <tgr@deploy2002> tgr, ariel, matmarex, mszabo: Backport for [[gerrit:1098990|Localisation updates (November 26) (T372175)]], [[gerrit:1098956|extend account creation lookup service to cover forced creations by others (T378401)]], [[gerrit:1098965|extend account creation backfill script to forced account creations by others (T378401)]], [[gerrit:1098929|ReportIncident: Setup $wgReportIncidentLocalLinks for ptwiki pilot

Mentioned in SAL (#wikimedia-operations) [2024-11-28T22:17:13Z] <tgr@deploy2002> Finished scap sync-world: Backport for [[gerrit:1098990|Localisation updates (November 26) (T372175)]], [[gerrit:1098956|extend account creation lookup service to cover forced creations by others (T378401)]], [[gerrit:1098965|extend account creation backfill script to forced account creations by others (T378401)]], [[gerrit:1098929|ReportIncident: Setup $wgReportIncidentLocalLinks for ptwiki pilot depl

I've done a test dry run of the current version of the script, to backfill one day's worth of missing local accounts on loginwiki. The mix included some accounts created by stewards, some accounts that were locally created elsewhere by the user themselves, and one interesting wikitech merged account from some years back.

After reviewing the output for some time, it all looked reasonable, so I did an actual run and hand-checked all results.
Some accounts were properly failed due to blocks (vpn/proxy), some were created with the steward IP/UA, and one was not created because in the intervening time before starting the real run, the loginwiki local account showed up.

I plan to deploy the puppet change to automate this script Monday morning UTC.

Change #1088552 merged by ArielGlenn:

[operations/puppet@production] systemd job to create missing local accounts on loginwiki/metawiki

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

This script is now live in production. I'll leave the task open for a day so we can make sure nothing strange happens.

Nothing reported, so that's a wrap.

  NODES
HOME 1
Interesting 1
Note 7
os 19
Users 1