Page MenuHomePhabricator

Ensure temp accounts can be safely disabled after being enabled
Closed, ResolvedPublic

Description

Background

As we deploy temporary accounts, we should allow for the possibility of needing to disable them in an emergency for some reason, having enabled them.

Known issues
  • If logged in as a temporary user when $wgAutoCreateTempUser['enabled'] is set to false, you stay logged in and you now have a real account: you can save preferences, set a password, etc.
  • When looking at history, logs, etc, contributions that were from temporary accounts are no longer highlighted as being from temporary accounts
Related

T342475: Define temporary account behavior on Wikimedia wikis which have IP masking disabled

Event Timeline

Does RealTempUserConfig::isTempName() need to check if $wgAutoCreateTempUser['enabled'] is true? It would be nice if we could assume that the genPattern, matchPattern, and reservedPatterns should always be respected by the system, whether or not the global flag for enabling temp account creation is enabled.

Change #1023485 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] temporary accounts: Add a shutoff switch

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

It seems that once $wgAutoCreateTempUser['enabled'] = true; is set, it should stay that way, unless the wiki operator intends to permanently disable the feature. As that's out of scope for our purposes, I'm going to focus on a temporary switch off only.

For that case, I think we should always leave $wgAutoCreateTempUser['enabled'] = true; in place.

To prevent new temp accounts from being created, we could use $wgGroupPermissions['*']['edit'] = false;. That would tell logged-out users that they need to be logged-in in order to edit. The UX for this is not great but probably good enough for an emergency switchoff type of situation. An alternative would be to use a shutoff config property, detailed below.

  • If logged in as a temporary user when $wgAutoCreateTempUser['enabled'] is set to false, you stay logged in and you now have a real account: you can save preferences, set a password, etc.

We could add a config property to AutoCreateTempUser like AutoCreateTempUser['shutoff']. Then, early in the web or API request cycle, see if the user is a temp name and if AutoCreateTempUser['shutoff'] is true. If that's the case, then call $user->logout().

Alternatively, we could create a maintenance script to iterate over all temp accounts and log them out. I like the logout-on-request approach, though, because if the shutoff is a temporary one, we won't unnecessarily logout temporary account users who aren't actively on the site, and writing a script to handle logging out a large number of users will be more effort, and require more time to run in practice.

Some situations I can think of:

  1. Within an hour or so of deployment to a wiki, there is some emergency situation that necessitates the need to shut-off temporary accounts and go back to the status quo
  2. Weeks after deployment, there is some emergency situation that necessitates the need to shut-off temporary accounts temporarily while a fix is implemented.
  3. Too many temporary accounts are being created and DBA is concerned about the size of the user table (or related tables in the centralauth DB) growing out of control
  4. Too many visitors are logged into a temporary account, and caching differences between temporary accounts and IPs cause increased server lag.
  5. A coordinated mass attack using temporary accounts (i.e. clearing cookies and then changing IPs) is occurring, which leads to too many distinct users performing vandalism.

Some situations I can think of:

  1. Within an hour or so of deployment to a wiki, there is some emergency situation that necessitates the need to shut-off temporary accounts and go back to the status quo

In my opinion, we should have a way to still allow the status quo (of IP editing) if the shutoff needed to be used a few hours after deployment. This should also not necessarily prevent continuing to use temporary accounts on wikis that have it stably deployed.

Here's a summary of notes from meeting with @Tchanders and @Dreamy_Jazz earlier today.

Goals:

  • Need to be able to switch off creation of new temp accounts after the feature is enabled
  • Existing temp users should have their session expire if the autocreation of temp accounts is disabled
  • We should allow for legacy IP editing if autocreation of temp accounts is disabled

Non-goals:

  • There should be a way to fully remove temp accounts from a wiki (e.g. delete old temp user accounts and associated metadata)

General points:

  • Model a third state in the TempUserConfig, which is "Autocreation of temp users is disabled, but the feature was activated in the past"
  • Update UserNameUtils to work if temp accounts are enabled or were enabled
  • Audit usage of TempUserConfig. We are changing the contract of what isEnabled means in TempUserConfig
  • Log out users when the config switch is set to disable creation of new temp accounts
    • Don't worry about UX of logging temp users out. We are not making guarantees to temp users.
  • Allow IP editing when config switch is set to disable creation of new temp accounts

General points:

  • Model a third state in the TempUserConfig, which is "Autocreation of temp users is disabled, but the feature was activated in the past"

I am not sure that this is needed. What I've done in the patch is to narrow the scope of what isEnabled() means, which is very narrowly defined to be "Auto-creation of temp users happens for configured actions". All other methods in TempUserConfig for generation pattern, match pattern, etc are no longer gated behind isEnabled(). This doesn't seem problematic and IMHO is more clear than creating a second "enabled-actually-fully-enabled" state.

Change #1040111 had a related patch set uploaded (by Kosta Harlan; author: Tchanders):

[mediawiki/core@master] [temp accounts] Introduce 'known' config flag

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

Change #1043027 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] [temp accounts] Make use of isKnown config flag

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

Change #1023485 abandoned by Kosta Harlan:

[mediawiki/core@master] [WIP] Temp accounts: Make it possible to safely disable creation

Reason:

Split up into the chain of patches in I4ce534a847461230f7fa276a565bdc1d6c9857e1

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

Change #1043726 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] expireTemporaryAccounts: Provide support for overriding the expiry period

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

After some iteration, here's the current state of the patches:

So, to answer the question, how can we safely disable temp accounts:

  • Push a config change to operations/mediawiki-config that sets enabled to false and known to true for $wgAutoCreateTempUser
  • Optionally, run php maintenance/run.php --wiki enwiki expireTemporaryAccounts --frequency=1 --expiry=0 to expire all logged in temporary accounts (probably best to avoid if possible, because if someone is in mid-edit, they will see an error that they could not edit due to loss of session data, and a warning that their IP will be recorded with the edit, but surely some non-zero percentage of users will ignore those messages and not realize what has happened)

Change #1040111 merged by jenkins-bot:

[mediawiki/core@master] [temp accounts] Introduce 'known' config flag

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

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

[mediawiki/core@master] util.js: Don't exit isTemporaryUser if temp accounts are known about

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

Change #1043798 merged by jenkins-bot:

[mediawiki/core@master] util.js: Don't exit isTemporaryUser if temp accounts are known about

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

Change #1043820 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] UncachedMenteeOverviewDataProvider: Use isKnown flag for temp user config

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

Change #1043821 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/CentralAuth@master] GlobalUserSelectQueryBuilder: Use isKnown flag for temp user config

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

Change #1043822 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/CheckUser@master] SpecialPageInitListHandler/ToolLinksHandler: Use isKnown flag for temp user config

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

Change #1043820 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] UncachedMenteeOverviewDataProvider: Use isKnown flag for temp user config

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

Change #1043027 merged by jenkins-bot:

[mediawiki/core@master] [temp accounts] Make use of isKnown config flag

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

Change #1043726 merged by jenkins-bot:

[mediawiki/core@master] expireTemporaryAccounts: Provide support for overriding the expiry period

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

Change #1043822 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] SpecialPageInitListHandler/ToolLinksHandler: Use isKnown flag for temp user config

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

Change #1043821 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] GlobalUserSelectQueryBuilder: Use isKnown flag for temp user config

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

I believe this is done now.

I searched core and extensions for isEnabled, 'enabled' and AutoCreateTempUser, and didn't find anything else that obviously needed updating.

Testing notes

I don't think the extensions need extra testing beyond what was done for code review.

For core, the main things to test are the points that were highlighted in the task description:

  • If logged in as a temporary user when $wgAutoCreateTempUser['enabled'] is set to false, you stay logged in and you now have a real account: you can save preferences, set a password, etc.
  • When looking at history, logs, etc, contributions that were from temporary accounts are no longer highlighted as being from temporary accounts

These should no longer be the case. Setup for testing this:

  1. Enable temp accounts
  2. Make some temporary users by editing, and make sure the last one is still logged in
  3. Set the following config:
$wgAutoCreateTempUser['enabled'] = false;
$wgAutoCreateTempUser['known'] = true;

The two issues above should no longer be the case.

It should still be possible to edit logged out, and the edit will be assigned to a user whose name is their IP address, the same as before temp accounts were enabled.

Change #1053371 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] RealTempUserConfig: Check that pattern is valid before calling isMatch

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

Change #1053371 abandoned by Kosta Harlan:

[mediawiki/core@master] RealTempUserConfig: Check that pattern is valid before calling isMatch

Reason:

Nevermind -- matchPattern should be required if known is set to true

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

dom_walden subscribed.

I turned off temporary accounts (as per T356524#9898566) on a local wiki which had had it enabled for a long time. Looking at the impact this has on IPs, temporary accounts and admin users.

IPs:

  • I am able to make IP edits.

Temporary accounts:

  • All the temporary accounts whose session was still open have remained open for at least the last few days of testing.
  • They can continue to edit and have it recorded as a temporary account edit.
  • They can login and create accounts.
  • Notifications work for temporary accounts.
  • My local docker setup has a loginwiki for central login. If a temporary account was created on a connected wiki, but was not created locally on the loginwiki. After disabling temporary accounts (including on the loginwiki), if I then visit loginwiki the local account will be created. For example, rows get created in the user, actor and localuser tables.
  • They were not able to do things like go to Special:Preferences, email users, change password, etc.

Admins:

  • Admins continue to be able to block, globally hide and delete temporary accounts. When hidden or deleted, the temporary account is logged out.
  • Admins can still use the "Show IP" on temp accounts.
  • Admins can still use Special:IPContributions and see temporary accounts.
  • We can do CheckUser investigations on temp accounts.
  • When looking at AbuseFilter logs for edits made by temporary accounts, we can continue to see the user_unnamed_ip variable, so we can see temporary account IPs.

I also ran the expireTemporaryAccounts maintenance script as outlined in T356524#9892003 and saw that the temporary accounts were expired. I did sometimes have to experiment with different values of --frequency in order to expire all the temporary accounts.