Page MenuHomePhabricator

Define temporary account behavior on Wikimedia wikis which have IP masking disabled
Closed, ResolvedPublic

Description

During IP masking rollout to Wikimedia wikis, some pilot wikis will have IP masking enabled while other wikis won't. We want pilot wikis to share the same account and login state, so we need to centrally log the user in. But if we want the user to act as a normal anonymous user on non-pilot wikis, we need to somehow suppress central login state there.

Desired behavior
We want to restrict temporary accounts only to wikis that have IP Masking enabled.

Say wiki A has IP Masking enabled and wiki B does not. Alice first edits on wiki A (as in they create a temporary account on wiki A). Their actions on wiki A will be attributed to the temporary account. Then she wanders off to wiki B and edit there. Since wiki B does not have IP Masking enabled, it should appear as if Alice edited with her IP address there (status quo).

We don't want the community on wiki B to get the impression that we have enabled IP Masking there without their approval. Neither should we be showing them temporary accounts without giving them tools to handle those accounts correctly. This should, hopefully, make for a smoother transition process on the whole.

Plan:

On non-pilot wikis, the temp user prefix will be reserved with $wgAutoCreateTempUser['reservedPattern'], so it will not be possible to manually create a local account which has the same name as a global temp user. This seems prudent and was a design assumption for T307064 which introduced reservedPattern.
Make UserNameUtils::isUsable() return false if isTempReserved() is true but isTemp() is false. So temp user reservations will be analogous to $wgReservedUsernames.
Thus CentralAuthSessionProvider and CentralAuthTokenSessionProvider will reject the global session due to isUsable() returning false.
In AuthManager::autoCreateUser(), where it says that we switched from isCreatable() to isValid() to support temp users, this will be changed again to instead call isUsable(). So auto-creation will be denied for foreign temp users on non-pilot wikis when it is requested by Special:CentralAutoLogin, ApiCreateLocalAccount, etc.

Test plan:

Set up CentralAuth with two pilot wikis and one non-pilot wiki with reservedPattern as described above.
Set up $wgForeignUploadTargets and use it to perform a foreign upload as a new normal user.
Create a temp user on the first pilot wiki.
Verify that it is not possible to act as the temp user on the non-pilot wiki using mw.ForeignApi, by attempting a foreign upload.
Visit the non-pilot wiki and confirm that the global session was rejected.
Verify that ApiCreateLocalAccount fails. Use debug logs to verify that the new logic in AuthManager::autoCreateUser() was reached.
Return to the pilot wiki and confirm that the temp user session is still valid.
Visit the second pilot wiki and confirm that auto-creation still works for central temp users.

Event Timeline

Tried setting

if ( !in_array( $wgDBname, $ipMaskingWikis, true ) && $wgDBname !== $wgCentralAuthLoginWiki ) {
    $wgHooks['SessionCheckInfo'][] = function ( &$reason, \MediaWiki\Session\SessionInfo $info ) {
        $userInfo = $info->getUserInfo();
        $userName = $userInfo ? $userInfo->getName() : null;
        if ( $userName !== null
            && \MediaWiki\MediaWikiServices::getInstance()->getTempUserConfig()->isTempName( $userName )
        ) {
            $reason = 'temporary account on non-pilot wiki';
            return false;
        }
    };
}

in configuration, but that while that causes the user to get the anon interface, CentralAuth thinks the login was successful (SessionCheckInfo only runs when loading a session, not when persisting one) so the CentralAuthAnon flag never gets set and it retries login on every request. If want the user to be logged in on pilot wikis and not logged in elsewhere, I think we'll either need to hardcode that behavior in MediaWiki, or add a hook to Special:CentralAutoLogin.

But if we want the user to act as a normal anonymous user on non-pilot wikis, we need to somehow suppress central login state there.

@Niharika Is this what we want?

But if we want the user to act as a normal anonymous user on non-pilot wikis, we need to somehow suppress central login state there.

@Niharika Is this what we want?

Yes, this is correct. To elaborate -

Say wiki A has IP Masking enabled and wiki B does not. Alice first edits on wiki A (as in they create a temporary account on wiki A). Their actions on wiki A will be attributed to the temporary account. Then she wanders off to wiki B and edit there. Since wiki B does not have IP Masking enabled, it should appear as if Alice edited with her IP address there (status quo).

We don't want the community on wiki B to get the impression that we have enabled IP Masking there without their approval. Neither should we be showing them temporary accounts without giving them tools to handle those accounts correctly. This should, hopefully, make for a smoother transition process on the whole.

@Tchanders what do you think?

@Niharika Thanks for outlining that. A couple of follow-up questions:

  • If the user wanders off to wiki B, but only ever reads, should they see their temporary account name in the interface?
  • If the user wanders back to wiki A again, should they still be signed in as the same temporary account?

@Niharika Thanks for outlining that. A couple of follow-up questions:

  • If the user wanders off to wiki B, but only ever reads, should they see their temporary account name in the interface?

I don't think so. The idea is to preserve the current experience on wiki B - so it would be best not to create the impression that temp accounts exist on wiki B. It would be misleading to do that when we end up attributing the edits to their IP address when they eventually edit.

  • If the user wanders back to wiki A again, should they still be signed in as the same temporary account?

Yes.

Plan:

  • On non-pilot wikis, the temp user prefix will be reserved with $wgAutoCreateTempUser['reservedPattern'], so it will not be possible to manually create a local account which has the same name as a global temp user. This seems prudent and was a design assumption for T307064 which introduced reservedPattern.
  • Make UserNameUtils::isUsable() return false if isTempReserved() is true but isTemp() is false. So temp user reservations will be analogous to $wgReservedUsernames.
  • Thus CentralAuthSessionProvider and CentralAuthTokenSessionProvider will reject the global session due to isUsable() returning false.
  • In AuthManager::autoCreateUser(), where it says that we switched from isCreatable() to isValid() to support temp users, this will be changed again to instead call isUsable(). So auto-creation will be denied for foreign temp users on non-pilot wikis when it is requested by Special:CentralAutoLogin, ApiCreateLocalAccount, etc.

Test plan:

  • Set up CentralAuth with two pilot wikis and one non-pilot wiki with reservedPattern as described above.
  • Set up $wgForeignUploadTargets and use it to perform a foreign upload as a new normal user.
  • Create a temp user on the first pilot wiki.
  • Verify that it is not possible to act as the temp user on the non-pilot wiki using mw.ForeignApi, by attempting a foreign upload.
  • Visit the non-pilot wiki and confirm that the global session was rejected.
  • Verify that ApiCreateLocalAccount fails. Use debug logs to verify that the new logic in AuthManager::autoCreateUser() was reached.
  • Return to the pilot wiki and confirm that the temp user session is still valid.
  • Visit the second pilot wiki and confirm that auto-creation still works for central temp users.

Ideally tests should be done with various combinations of the wikis being in the same and different cookie domains. But I don't really want to add more wikis to my CentralAuth farm if I can help it, since there's not much automation. It's easier to repeat the tests with different $wgAutoCreateTempUser configurations. I have four wikis, which are analogous to loginwiki, enwiki, frwiki and enwikiquote.

I think loginwiki has to be a pilot wiki. Otherwise there would be no loginwiki cookies and navigation between pilot wikis in different cookie domains would definitely not transfer the temp account. (This feature may be broken by browser privacy settings anyway, but we can at least try.)

mw.ForeignAPI doesn't depend on cookie domains, so I don't think those tests have to be repeated. So the most essential test matrix is:

Same cookie domainDifferent cookie domain
Successful session transfer to pilot wiki
Failed session transfer to non-pilot wiki

Change 950061 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Make "temp reserved" usernames not be "usable"

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

Change 950062 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/CentralAuth@master] Don't delete central session if it is rejected for local reasons

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

  • Set up CentralAuth with two pilot wikis and one non-pilot wiki with reservedPattern as described above.
  • Set up $wgForeignUploadTargets and use it to perform a foreign upload as a new normal user.

Done

  • Create a temp user on the first pilot wiki.
  • Verify that it is not possible to act as the temp user on the non-pilot wiki using mw.ForeignApi, by attempting a foreign upload.

Done. A pre-existing issue is that the CORS header is not sent when there is a pre-main error. The foreign wiki API response says "The username \"*Unregistered 63\" is not usable on this wiki", which is nice and informative, but it comes without Access-Control-Allow-Origin so the client can't read it. The user just sees "Failed to load the configuration for file uploads to the foreign file repository."

  • Visit the non-pilot wiki and confirm that the global session was rejected.
  • Different domain: done.
  • Same domain: This was a tricky case. It required a CentralAuth patch.

I tried the implicit and explicit cases with cleared cookies and localStorage. In both cases, Special:CentralAutoLogin/setCookies gets up to autoCreateUser() which fails as expected. I edited a page and confirmed that my IP address is shown.

  • Verify that ApiCreateLocalAccount fails. Use debug logs to verify that the new logic in AuthManager::autoCreateUser() was reached.

I commented out the permission check to make this work. Then it fails as expected and autoCreateUser() is indeed reached.

  • Return to the pilot wiki and confirm that the temp user session is still valid.

Done.

  • Visit the second pilot wiki and confirm that auto-creation still works for central temp users.
  • Same domain: done.
  • Different domain: This worked once I set $wgCentralAuthAutoLoginWikis = []. Originally it contained the same wiki, a case which is suppressed in WMF using a config hack. This redundant auto-login destroyed the loginwiki session and broke everything.

This might be the first time that AHT has tested SUL. Here are some testing notes. (I tested both the core and CentralAuth patches together.)

Config

Temp user config

Pilot wiki: $wgAutoCreateTempUser['enabled'] = true;

Non-pilot wiki: $wgAutoCreateTempUser['reservedPattern'] = '*$1'; (same as the pilot wiki's $wgAutoCreateTempUser[matchPattern]).

CentralAuth Config

I set the basic config as outlined in https://www.mediawiki.org/wiki/Manual:$wgConf#Example (I put them all in LocalSettings.php). These I had had in place for a while because I'd been testing CentralAuth with other things that didn't need SUL, so others might already have these too.

Additionally I set:

// For SUL - see https://www.mediawiki.org/wiki/Extension:CentralAuth#%22SUL2%22_behavior
$wgCentralAuthCookies = true;
$wgCentralAuthLoginWiki = 'mediawiki'; // The name of one of my databases in the $wgLocalDatabases array
$wgCentralAuthAutoMigrate = true;
$wgCookieSameSite = "None";
$wgUseSameSiteLegacyCookies = true;

// For SUL with database caching - see https://www.mediawiki.org/wiki/Extension:CentralAuth#Cache_issues
$wgSharedDB = 'centralauth22'; // The name of my database in $wgCentralAuthDatabase
$wgSharedTables = [ 'objectcache' ]; // This table must be created - see docs above
$wgCentralAuthSessionCacheType = CACHE_DB;

Testing

Further to the testing @tstarling did in T342475#9100851 I checked:

Edit on a pilot and non-pilot wiki at the same time

  1. Made a logged-out edit on the pilot wiki (this created a temp user)
  2. Visited the non-pilot wiki and made an edit (as expected, this was saved as an IP edit)
  3. Visited the first wiki again and made an edit (as expected, this was assigned to the original temp user)

Try to create a user with the pattern reserved for temp users on the non-pilot wiki

  1. Visited Special:CreateAccount on the non-pilot wiki
  2. Entered a user name starting with the reserved pattern

As expected, form submission failed with error: You have not specified a valid username.

Log in on non-pilot wiki while pilot wiki session is active

  1. Made a logged-out edit on the pilot wiki (this created a temp user)
  2. Visited the non-pilot wiki (I was logged out)
  3. Logged in as a user Admin on the non-pilot wiki
  4. Visited the pilot wiki again (I was still logged in as Admin)

Unexpected errors

I did sometimes see this error when logging in on my non-CentralAuthLoginWiki wiki:

image.png (197×503 px, 19 KB)

But this also happened with both repos on master, so I assume is something to do with my local setup?

@Tgr I've read over the code and tested, and it looks good to me. But this is the first time I've really delved into authentication, so would you have a moment to review this too?

Change 950061 merged by jenkins-bot:

[mediawiki/core@master] Make "temp reserved" usernames not be "usable"

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

Change 950062 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Don't delete central session if it is rejected for local reasons

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

Change 965879 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[operations/mediawiki-config@master] [beta] Make temp user config SUL-friendly

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

Change 965879 merged by jenkins-bot:

[operations/mediawiki-config@master] [beta] Make temp user config SUL-friendly

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

Quick question - if our apps send temporary account cookies to a non-pilot wiki, will the non-pilot wiki ignore them? That seems to be the case from the testing that I see in this ticket, but I just want to confirm. I think without this the apps would need to be aware of which wikis are pilot vs. non-pilot, and prevent temporary cookies from sending to non-pilot.

Quick question - if our apps send temporary account cookies to a non-pilot wiki, will the non-pilot wiki ignore them?

Yes. It wouldn't be much different from what a browser does (which will also send some of those cookies since they are set on wikipedia.org and similar shared domains).

Tchanders added a subscriber: Dreamy_Jazz.

Trust and Safety Product can do more QA on this.

We've noticed that this isn't working as intended on beta (thanks @Dreamy_Jazz).

Steps:

  1. make a logged-out edit on https://de.wikipedia.beta.wmflabs.org/ to create a new temp account
  2. visit https://en.wikipedia.beta.wmflabs.org/ (where temp accounts is not enabled)

Expected: you appear as an IP actor on en beta
Actual: you appear as a registered user on en beta but with your temp account name

@tstarling @Tgr Is there something about beta that would make us expect this not to work properly there?

rOMWC9c0cc5114cc8: Update beta configs to reflect new temp account naming pattern tries to set reservedPattern for pilot wikis but it gets overridden a few lines later.

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

[operations/mediawiki-config@master] [beta] tempaccounts: Use same reservedPattern whether enabled/disabled

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

Change 994826 merged by jenkins-bot:

[operations/mediawiki-config@master] [beta] tempaccounts: Use same reservedPattern whether enabled/disabled

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

Based on my testing

We've noticed that this isn't working as intended on beta (thanks @Dreamy_Jazz).

Steps:

  1. make a logged-out edit on https://de.wikipedia.beta.wmflabs.org/ to create a new temp account
  2. visit https://en.wikipedia.beta.wmflabs.org/ (where temp accounts is not enabled)

Expected: you appear as an IP actor on en beta
Actual: you appear as a registered user on en beta but with your temp account name

Based on my testing after the above config change, this is now no longer the case and so is resolved.

Per above, I think this is done. Please feel free to reopen if you think something else is needed (or prefer a different workflow for this task).