Also per {T390514} the number of writes seems like a bottleneck, and this proposal would increase it significantly. (Although in other ways smaller separate blobs would improve storage space use in a sessionstore with a binlog, so it's possible it would be positive on the net.)

I think the main problem with this (assuming a pure key-value store) would be session ID reset, which requires copying all the data from the old session to the new session.

There are some groups of keys that should probably be written together, for example wsUserID and wsUserName.

In T277834#7083434, @Krinkle wrote:

Ref T158365: Session "{session}": Metadata merge failed: {exception} and T204459: Session "{session}": CentralAuth saved source {saved} != expected source {expected}.

I'm not very familiar with the background of that error, but it seems like perhaps this is a symptom of combined writes losing out?

Showing issues that prevent the save would be nice, too. I think the fundamental limitation here is that the linter / filter / whatever needs to be able to return errors associated with specific lines. That's not always straightforward (see e.g. T174554: AbuseFilter should expose matched text to warning messages for some related discussion), although for blacklists it should be easy to add.

Probably caused by T380500: CentralAuthUser returning outdated data after user creation.

The error is kind of intentional - we could actually remove it and let users add multiple methods by just removing a throw clause, but that would lead to more confusing errors in other workflows. We could replace it with a nicer error page, but we are so close to properly supporting multiple methods now that I don't think it's worth the effort.

We did the testing for t2bot.io (theoretically mautrix-telegram, but one thing we found out was that, at least at the time, it was using a quite old fork of that software). The notes are here.

Yes please.

rECAU85157ca73787: Simplify authentication provider filtering flipped provider filtering in $wgCentralAuthSul3SharedDomainRestrictions, so now the defaults make sense for most wikis. With that and flipping $wgCentralAuthRestrictSharedDomain, I think this is done.

It worked fine today.

I think what's left here is to spot-check some examples and confirm they are failing due to permission checks. If that's the case, this task can be closed as invalid (since that's the expected behavior) and we can follow up in T394733 on whether we want to change that.

In T393656#10813439, @Tgr wrote:

I think this is ultimately a question for stewards, as they are the target audience for these autocreations: when a user registers successfully on some wiki, but their local account on metawiki and/or loginwiki cannot be created because of some local rule (block, abusefilter etc), what would be your preferred outcome for the backfill script (T371267: Create a script to backfill missing local accounts on loginwiki/metawiki for new global accounts)? Obeying the rule or overriding it and creating the account anyway?

In T393656#10804189, @NicoScribe wrote:

So the backfill jobs have been run several times to repair the following cases. Do you have logs showing the cause, when a backfill fails?
1 account for https://meta.wikimedia.org/wiki/Special:CentralAuth/Jenikipedia
2 accounts for https://meta.wikimedia.org/wiki/Special:CentralAuth/Furkan2525
2 accounts for https://meta.wikimedia.org/wiki/Special:CentralAuth/Nisapro
2 accounts for https://meta.wikimedia.org/wiki/Special:CentralAuth/Robertlanelilly
2 accounts for https://meta.wikimedia.org/wiki/Special:CentralAuth/Robertlane2031
2 accounts for https://meta.wikimedia.org/wiki/Special:CentralAuth/Fjshuxicjeb1
2 accounts for https://meta.wikimedia.org/wiki/Special:CentralAuth/BobDobolina1

In T393656#10832792, @Johannnes89 wrote:

As I said on discord: Accounts created by the backfill script show the wrong IP in loginwiki CU. The backfill script has little value to stewards as long as that's the case.

logPersistenceChange() logs cookie writes. Those are not particularly relevant to the current investigation which is about session store writes. They were added for issues where the cookies got cached and sent to the wrong user, but we didn't have such issues for years so probably fine to drop that logging.

Also triggered by one of the integration tests:

1) MediaWiki\Tests\Session\SessionBackendTest::testResetIdOfGlobalSession
Use of $_SESSION was deprecated in MediaWiki 1.27. [Called from session_write_close in (internal function)]

I think this is done, the current logs are good enough.

The stack trace is

from /srv/mediawiki/php-1.45.0-wmf.1/vendor/thecodingmachine/safe/lib/Exceptions/JsonException.php(10)
#0 /srv/mediawiki/php-1.45.0-wmf.1/vendor/thecodingmachine/safe/lib/special_cases.php(35): Safe\Exceptions\JsonException::createFromPhpError()
#1 /srv/mediawiki/php-1.45.0-wmf.1/vendor/web-auth/webauthn-lib/src/PublicKeyCredentialLoader.php(118): Safe\json_decode(string, bool)
#2 /srv/mediawiki/php-1.45.0-wmf.1/extensions/WebAuthn/src/Key/WebAuthnKey.php(485): Webauthn\PublicKeyCredentialLoader->load(string)
#3 /srv/mediawiki/php-1.45.0-wmf.1/extensions/WebAuthn/src/Key/WebAuthnKey.php(297): MediaWiki\Extension\WebAuthn\Key\WebAuthnKey->authenticationCeremony(string, Webauthn\PublicKeyCredentialRequestOptions, MediaWiki\Extension\OATHAuth\OATHUser)
#4 /srv/mediawiki/php-1.45.0-wmf.1/extensions/WebAuthn/src/Module/WebAuthn.php(82): MediaWiki\Extension\WebAuthn\Key\WebAuthnKey->verify(array, MediaWiki\Extension\OATHAuth\OATHUser)
#5 /srv/mediawiki/php-1.45.0-wmf.1/extensions/WebAuthn/src/Authenticator.php(225): MediaWiki\Extension\WebAuthn\Module\WebAuthn->verify(MediaWiki\Extension\OATHAuth\OATHUser, array)
#6 /srv/mediawiki/php-1.45.0-wmf.1/extensions/WebAuthn/src/Auth/WebAuthnSecondaryAuthenticationProvider.php(88): MediaWiki\Extension\WebAuthn\Authenticator->continueAuthentication(array)
#7 /srv/mediawiki/php-1.45.0-wmf.1/extensions/OATHAuth/src/Auth/SecondaryAuthenticationProvider.php(64): MediaWiki\Extension\WebAuthn\Auth\WebAuthnSecondaryAuthenticationProvider->continueSecondaryAuthentication(MediaWiki\User\User, array)
#8 /srv/mediawiki/php-1.45.0-wmf.1/includes/auth/AuthManager.php(759): MediaWiki\Extension\OATHAuth\Auth\SecondaryAuthenticationProvider->continueSecondaryAuthentication(MediaWiki\User\User, array)
#9 /srv/mediawiki/php-1.45.0-wmf.1/includes/specialpage/AuthManagerSpecialPage.php(410): MediaWiki\Auth\AuthManager->continueAuthentication(array)
#10 /srv/mediawiki/php-1.45.0-wmf.1/includes/specialpage/AuthManagerSpecialPage.php(542): MediaWiki\SpecialPage\AuthManagerSpecialPage->performAuthenticationStep(string, array)
#11 /srv/mediawiki/php-1.45.0-wmf.1/includes/htmlform/HTMLForm.php(825): MediaWiki\SpecialPage\AuthManagerSpecialPage->handleFormSubmit(array, MediaWiki\HTMLForm\CodexHTMLForm)
#12 /srv/mediawiki/php-1.45.0-wmf.1/includes/specialpage/AuthManagerSpecialPage.php(473): MediaWiki\HTMLForm\HTMLForm->trySubmit()
#13 /srv/mediawiki/php-1.45.0-wmf.1/includes/specialpage/LoginSignupSpecialPage.php(407): MediaWiki\SpecialPage\AuthManagerSpecialPage->trySubmit()
#14 /srv/mediawiki/php-1.45.0-wmf.1/includes/specialpage/SpecialPage.php(734): MediaWiki\SpecialPage\LoginSignupSpecialPage->execute(null)
#15 /srv/mediawiki/php-1.45.0-wmf.1/includes/specialpage/SpecialPageFactory.php(1738): MediaWiki\SpecialPage\SpecialPage->run(null)
#16 /srv/mediawiki/php-1.45.0-wmf.1/includes/actions/ActionEntryPoint.php(499): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, MediaWiki\Context\RequestContext)
#17 /srv/mediawiki/php-1.45.0-wmf.1/includes/actions/ActionEntryPoint.php(143): MediaWiki\Actions\ActionEntryPoint->performRequest()
#18 /srv/mediawiki/php-1.45.0-wmf.1/includes/MediaWikiEntryPoint.php(198): MediaWiki\Actions\ActionEntryPoint->execute()
#19 /srv/mediawiki/php-1.45.0-wmf.1/index.php(58): MediaWiki\MediaWikiEntryPoint->run()
#20 /srv/mediawiki/w/index.php(3): require(string)
#21 {main}

which is still not terribly helpful (but at least it clarifies that this is a JSON syntax error, not a PHP syntax error). So either some field is too short and cuts off the end of the JSON, or this is some kind of client-side issue.

I have a WIP patch for this, although it tries to do some more complex things too and I got stuck figuring out a nice way to allow hooks to change the HTML message (to do things like add a country name) so feel free to make a more straightforward patch if you want.

So fundamentally I think the problem is that a some things call save() and rely on it generally being a no-op: WebRequest::getSession() via the delaySave() call in SessionManager::getSessionFromInfo(); and Session::persist() via the renew() call. But save() is only a noop when there is no change; if anything changed the session data, it will do a write.

I think this concludes the investigation.

Ability to use arbitrary tokens:

• Session-ish cookies (core session cookie, core user token cookie, CentralAuth session cookie, CentralAuth user token cookie, bot password cookie): yes, could be replaced with any value as long as the session ID / user token / user token hash is recoverable from it (e.g. a JWT with the ID/hash + other data). The JWT could also be a separate cookie, we are already using several cookies (session ID, username, user ID...).
• CentralAuth API token: yes, this is an arbitrary token, fully under our control, and short-lived so easy to change the format.
• NetworkSession: yes, tokens are arbitrary secrets stored in site confiugration.
• OAuth 2: the tokens are already JWTs and there's a hook (OAuthClaimStoreGetClaims) for adding fields to the JWT data, so no change needed.

In T394402#10828026, @DAlangi_WMF wrote:

I've added a few more sub-tasks which are about removing some of these logs:

• T379505: "Expected key {key} not found due to timeout" CentralAuth error is spammy
• T391646: Analyze "temporary logging for T372702" data and then stop logging it

In T394402#10828106, @DAlangi_WMF wrote:

In T394402#10827022, @Tgr wrote:

The Persisting... stuff was added for {T309943} & co, I don't think we need it anymore

I think this will be a partial revert of https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1143959 and I imagine that removing the else block will be enough here?

You can use OAuth which is more great (security-wise at least) and isn't really affected by domains.

Ideally you'd want a rate limit for this kind of thing, ie. "cannot change more than X edges within Y seconds". Some googling suggests Phabricator does rate limiting via PhabricatorSystemActionEngine::willTakeAction() so maybe that can just be called wherever Phabricator actually does the graph updates?

Apparently you cannot add more subtasks to this task (T394493: Too many relationships of type "task.has-subtask") so I'll just link to the last remaining issue instead: T394492: MobileFrontend should not use raw HTML messages

You can reproduce e.g. by opening and trying to save the subtasks menu on T2212.

Can reproduce it but PWB logs aren't helpful. I guess I will have to set up a Python debugger to understand what's going on.
(Normal password-based login works BTW.)

Some thoughts on implementation:

• Session info endpoint: this is just an API handler or PHP entry point that calls WebRequest::getSession() and puts its various properties into the response. Can be done in days if not hours.
• Turn all (or some) session tokens into JWTs:
  • The OAuth 2 access token is already a JWT.
  • For OAuth 1, I think there are two options:
    • Just put JWTs in the oaac_access_token table. Needs a schema change (current column is too short) and probably some sort of migration to convert the existing values.
    • Keep storing short random strings in the table, generate JWTs dynamically. This would 1) require the encryption to be deterministic, 2) would mean the access token gets invalidated any time the JWT changes because some information included in it changes (e.g. the user is added to a new group). We probably don't want to go there.
  • CentralAuth tokens and NetworkSession tokens are arbitrarily and fully under our control, turning them into JWTs should be unproblematic. (For NetworkSession it's probably not really needed since it uses the service mesh so it won't pass through the API gateway. But then, it's trivial to do.)
  • For the various cookie-based schemes, the easiest approach would be to just have add a new cookie on top of the existing ones, and put the JWT in it.
  • If we want to simplify cookies, there are two ways, since there are two different groups of cookies which are both needed, one to authenticate the user in a way that keeps working when visiting other wikis, the other to identify the data in the session backend that the current wiki is associating with the user. So we could:
    • Have a JWT cookie on the parent domain (ie. replace current centralauth_Session and centralauth_Token with a JWT), store the local session ID as a separate cookie. So the same JWT cookie would be shared across different wikis, and it wouldn't change much. (Would we even need a central session backend after this? Maybe not.)
  • Replace the local session ID with a JWT, keep the centralauth_* cookies. In some cases, this JWT could store session data directly, per T394076: Investigate storing anonymous sessions client-side.
  • I guess we could do both of these things at the same time too, not sure if there would be much benefit in it though.

authevents is a dedicated log channel for keeping track of authentication volume. It's primarily for Prometheus, so we can turn off the Logstash backend if someone feels strongly about it, but it's nice to have IMO. The Persisting... stuff was added for {T309943} & co, I don't think we need it anymore. The Session store:... stuff is for {T390514}, we are still actively investigating that. Might want to keep in the longer term too but we'll see. The SUL3 logs aren't needed (the idea was to track bounce rates but due to the massive amount of scraping on the login page, that ended up being completely useless).

The error was on https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/1136758

In T394105#10822934, @kostajh wrote:

I would make it generally available, yes.

As a first approximation, I think we'd want an "URL variant" data structure (could just be an array with arbitrary extension-prefixed keys, like [ 'MobileFrontend.mobile-domain' => true ], a getUrlVariant() method on either WebRequest or RequestContext, and the ability to pass an url variant to any URL generating method (Title, WikiMap, UrlUtils etc). Maybe a new hook to determine the default URL variant when not explicitly provided. Then 90% of mobile domain wrangling could be replaced with "default to the current URL variant when generating links".

Not really a subtask of T214998: RFC: Remove m-dot subdomain, serve mobile and desktop variants through the same URL - if that happened, these hooks wouldn't be necessary at all.

In T336937#10746541, @Tarrow wrote:

If we had used the same keypairs (but are not using CentralAuth or similar) would it have still meant access tokens worked across wikis?

In T392251#10810094, @Tgr wrote:

Usually that just falls back to calling getSession() on the global context's WebRequest, which would then be cached