Page MenuHomePhabricator

Remove CentralAuth support for mixed-protocol HTTP/HTTPS wikis
Closed, ResolvedPublic

Description

CentralAuth login mechanisms have a bunch of code to support mixed-protocol HTTP/HTTPS wikis. We haven't used this configuration for Wikimedia wikis for years, and we don't support third-party users. (Besides, it seems rare for websites to do that these days – either they're on HTTPS or on HTTP only.)

We should only support HTTPS-only and HTTP-only wikis (the latter merely for ease of local developer setup), and simplify our code to avoid future maintenance.

Event Timeline

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

Change 965729 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@master] Remove success notification when we can't get autologin response

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

Change 965729 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Remove success notification when we can't get autologin response

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

For the record, I'm not planning to find and completely remove all of this code – some of it probably can't be removed, because the same feature exists in core MediaWiki AuthManager, and CentralAuth must go through the motions of supporting it (unless we wanted to remove it there too, which would be a bigger task, and which would at least require earlier deprecation).

However, I've been running into weird protocol-handling code all over the place just as I was trying to learn how CentralAuth works recently, and the best case scenario is that it does nothing – in worse cases, it may be responsible for the various issues we don't understand (the patch above actually closed 3 old bugs). So, I would like to remove things as I run into them, so that I never have to debug them in the future.

Change 966270 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@master] SpecialCentralLogin: Remove protocol redirect thing

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

Change 966899 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@master] LoginCompleteHookHandler: Remove support for mixed-protocol wikis

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

Change 966900 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@master] Remove 'currentProto'/'finalProto'/'proto' business

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

Change 967233 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@master] Replace WikiReference::getFullUrl() with getCanonicalUrl()

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

Change 966270 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] SpecialCentralLogin: Remove protocol redirect thing

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

Change 966899 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] LoginCompleteHookHandler: Remove support for mixed-protocol wikis

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

Change 966900 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Remove 'currentProto'/'finalProto'/'proto' business

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

Change 967233 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Replace WikiReference::getFullUrl() with getCanonicalUrl()

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

Change 967195 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@wmf/1.42.0-wmf.1] Remove 'currentProto'/'finalProto'/'proto' business

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

This needs to be backported, because the data it reads and writes is shared among all wikis, and when we have both wmf.1 and wmf.2 in production, there could be some problems with rollbacks or with wikis running different versions talking to each other.

This needs to be backported, because the data it reads and writes is shared among all wikis, and when we have both wmf.1 and wmf.2 in production, there could be some problems with rollbacks or with wikis running different versions talking to each other.

In that case this task should be a train blocker IMHO, to ensure that, if the backport doesn’t happen for whatever reason, the train doesn’t roll forward without it (since the code was already merged into master and will therefore be in wmf.2).

Change 967195 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@wmf/1.42.0-wmf.1] Remove 'currentProto'/'finalProto'/'proto' business

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

Mentioned in SAL (#wikimedia-operations) [2023-10-23T13:17:12Z] <urbanecm@deploy2002> Started scap: Backport for [[gerrit:967195|Remove 'currentProto'/'finalProto'/'proto' business (T348852)]], [[gerrit:966915|Remove unused $wgIncludeLegacyJavaScript]], [[gerrit:966919|Remove $wgApiFrameOptions override for enwiki and zhwiki (T131183)]]

Mentioned in SAL (#wikimedia-operations) [2023-10-23T13:18:26Z] <urbanecm@deploy2002> matmarex and urbanecm: Backport for [[gerrit:967195|Remove 'currentProto'/'finalProto'/'proto' business (T348852)]], [[gerrit:966915|Remove unused $wgIncludeLegacyJavaScript]], [[gerrit:966919|Remove $wgApiFrameOptions override for enwiki and zhwiki (T131183)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2023-10-23T13:29:09Z] <urbanecm@deploy2002> Finished scap: Backport for [[gerrit:967195|Remove 'currentProto'/'finalProto'/'proto' business (T348852)]], [[gerrit:966915|Remove unused $wgIncludeLegacyJavaScript]], [[gerrit:966919|Remove $wgApiFrameOptions override for enwiki and zhwiki (T131183)]] (duration: 11m 56s)

matmarex claimed this task.

I think this is done, or at least done enough that I don't want to spend more time on it. I think I removed all of the explicit redirects between protocols.

There is some more suspicious code that is probably not all needed: https://codesearch.wmcloud.org/deployed/?q=forcehttps|prefershttps|requireshttps|stickhttps|securecookies|cookiesecure&repos=mediawiki%2Fextensions%2FCentralAuth&i=fosho – however, some of it is needed to support both HTTPS-only and HTTP-only wikis, and some is needed to integrate with MediaWiki's support for mixed-protocol wikis,

The CentralAuthPostLoginRedirect hook parameter is not needed I think, but changing hook signatures is way too much effort. Maybe worth adding a comment that it is now deprecated and ignored?

CentralAuthSessionProvider is an extension of core's CookieSessionProvider so while it doesn't really need the HTTPS flags, it inherits a bunch of logic that does use it so removing it might be tricky.

The secureCookies and stickHTTPS flags in LoginCompleteHookHandler/SpecialCentralLogin could be dropped I think, they are only used to set the local session's forceHTTPS flag, and that's a no-op on HTTPS-only sites. it just sets the local session's but it more or less just copies what AuthManager does in the local session.

secureCookies in SpecialCentralAutoLogin is the same, only used for the forceHTTPS flag.

Hmm, alright, I'm convinced, let's delete some more.

Change 969189 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@master] Remove support for HTTPS-only sessions on HTTP/HTTPS wikis

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

Change 969189 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Remove support for HTTPS-only sessions on HTTP/HTTPS wikis

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

Change 973800 had a related patch set uploaded (by Gergő Tisza; author: Bartosz Dziewoński):

[mediawiki/extensions/CentralAuth@wmf/1.42.0-wmf.4] Remove support for HTTPS-only sessions on HTTP/HTTPS wikis

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

Change 973800 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@wmf/1.42.0-wmf.4] Remove support for HTTPS-only sessions on HTTP/HTTPS wikis

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

Mentioned in SAL (#wikimedia-operations) [2023-11-13T22:22:57Z] <tgr@deploy2002> Started scap: Backport for [[gerrit:973800|Remove support for HTTPS-only sessions on HTTP/HTTPS wikis (T348852)]]

Mentioned in SAL (#wikimedia-operations) [2023-11-13T22:24:19Z] <tgr@deploy2002> tgr: Backport for [[gerrit:973800|Remove support for HTTPS-only sessions on HTTP/HTTPS wikis (T348852)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

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

[mediawiki/extensions/CentralAuth@master] session: Remove incorrect warning

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

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

[mediawiki/extensions/CentralAuth@wmf/1.42.0-wmf.4] session: Remove incorrect warning

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

Mentioned in SAL (#wikimedia-operations) [2023-11-13T22:41:15Z] <tgr@deploy2002> Finished scap: Backport for [[gerrit:973800|Remove support for HTTPS-only sessions on HTTP/HTTPS wikis (T348852)]] (duration: 18m 17s)

Change 973879 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] session: Remove incorrect warning

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

Change 973801 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@wmf/1.42.0-wmf.4] session: Remove incorrect warning

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

Mentioned in SAL (#wikimedia-operations) [2023-11-13T22:47:07Z] <tgr@deploy2002> Started scap: Backport for [[gerrit:973801|session: Remove incorrect warning (T348852)]]

Mentioned in SAL (#wikimedia-operations) [2023-11-13T22:48:25Z] <tgr@deploy2002> tgr: Backport for [[gerrit:973801|session: Remove incorrect warning (T348852)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2023-11-13T22:55:11Z] <tgr@deploy2002> Finished scap: Backport for [[gerrit:973801|session: Remove incorrect warning (T348852)]] (duration: 08m 03s)