| • Pchelolo | |
| Aug 27 2020, 9:28 PM |
| F79968499: image.png | |
| May 6 2026, 10:35 PM |
In 2020, we in the Core Platform Team forked the thephpleague/oauth2-server package as wikimedia/oauth2-server to incorporate changes while waiting for upstream. They don't seem likely to necessarily include these changes (and one has been declined already), so we need to decide on a longer term solution.
Relevant PR:
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Reedy | T389272 MWCallbackStream write method has a differing signature to upstream libraries | |||
| Resolved | Reedy | T397068 Upgrade psr/http-message to 2.0 or later | |||
| Resolved | JTweed-WMF | T398815 WE5.1.2 Verifiable MediaWiki sessions | |||
| Resolved | Tgr | T399243 Support JWT generation for session tokens in MediaWiki core | |||
| Resolved | • matmarex | T405541 Replace MediaWiki\Json\ClockAdapter | |||
| Resolved | Reedy | T415956 OAuth BearerTokenValidatorTest::testValidateAuthorization test failure on PHP 8.4 | |||
| Resolved | PRODUCTION ERROR | • matmarex | T418668 TypeError: MediaWiki\Extension\OAuth\Repository\ClaimStore::getClaims(): Argument #3 ($userIdentifier) must be of type ?string, int given, called in /srv/mediawiki/php-1.46.0-wmf.17/vendor/league/oauth2-server/src/Grant/Refresh | ||
| Resolved | • matmarex | T261462 Migrate OAuth extension back from wikimedia/oauth2-server fork to upstream | |||
| Resolved | Tgr | T419226 Archive wikimedia/oauth2-server |
IMO the next step would be to make it clear how the fork differs from upstream, write instructions on how to recreate it (as a FORK.md file or something), and switch from periodically merging upstream into the fork to starting with the upstream version for each release and then applying our patches (and squashing the private claims patches into one to make it more manageable). And switch back OAuth's composer JSON from pinning a commit to using a branch.
Yeah, from memory, the short version is that there's 2 or 3 patches ontop, which it would look like you have deciphered.
I believe (it's over 3 years ago) that after bringing in upstream changes, the head of the branch actually didn't work for us anymore, so it got pinned against an older version before it was pulled in.
The failure being T302757: OAuth test failures. And then trying to (unsuccessfully) report/fix this upstream https://github.com/thephpleague/oauth2-server/issues/1266 and https://github.com/thephpleague/oauth2-server/pull/1138
This branch is 39 commits ahead of, 492 commits behind thephpleague/oauth2-server:master.
https://github.com/thephpleague/oauth2-server/compare/master...wikimedia:oauth2-server:v9.0.0-alpha
I think we should fix this by the time we switch to PHP 8.3. It would be nice to move to library versions which were tested on that version. At least for lcobucci/jwt (which is pinned to an old version because the old version of oauth-server requires that) that's not the case today. (We can fix that without unforking, by just merging in some upstream changes, but I'm not sure it would be less effort, and we'd just be rolling the ball forward.)
With T399243: Support JWT generation for session tokens in MediaWiki core adding JWT generation to core, core and wikimedia/oauth2-server now both depend on lcobucci/jwt and will eventually get into a version conflict, so that's one more reason to clean this up.
2026 is basically upon us...
This has been blocking stuff for a while, never mind this task being open for coming up 5.5 years. And it looks like it's starting to get in the way of numerous other things too.
I believe most of the changes are either in the originally referenced PR.
https://github.com/thephpleague/oauth2-server/pull/1122 is a stack of 40 commits (ugh), whereas https://github.com/thephpleague/oauth2-server/pull/1138 is "just" one.
Your comment at https://github.com/thephpleague/oauth2-server/issues/1137#issuecomment-1474717945 seems to have been ignored (for 2.5 years), which pointed out one of the changes we wanted was in the spec now...
Which has been forked in https://github.com/thephpleague/oauth2-server/issues/1434...
As of writing, https://github.com/thephpleague/oauth2-server/releases/tag/9.3.0 is the latest release.
Would taking that, and trying to re-apply the two PR on it be a good way forward?
I've created a currently empty https://github.com/wikimedia/oauth2-server/tree/9.3.0-WMF based on https://github.com/thephpleague/oauth2-server/releases/tag/9.3.0, with currently no changes.
I've updated/rebased/whatever (mostly fixed so their CI passes) https://github.com/thephpleague/oauth2-server/pull/1138 into https://github.com/wikimedia/oauth2-server/pull/19 as a PR against https://github.com/wikimedia/oauth2-server/tree/9.3.0-WMF
Based on https://patch-diff.githubusercontent.com/raw/thephpleague/oauth2-server/pull/1138.patch
I also did some CR at https://github.com/thephpleague/oauth2-server/pull/1138/changes as there were a few broken changes (some in examples, so not a big deal)...
Also, phpstan is so annoying as a static analyser, it seems unable to look at a guarding conditional... Ignore it is!
------ -----------------------------------------------------------------
Line src/Entities/Traits/AccessTokenTrait.php (in context of class
LeagueTests\Stubs\AccessTokenEntity)
------ -----------------------------------------------------------------
76 Parameter #1 $issuer of method Lcobucci\JWT\Builder::issuedBy()
expects non-empty-string, string given.
------ ----------------------------------------------------------------------- -----------------------------------------------------------------
Line src/Entities/Traits/AccessTokenTrait.php (in context of class
LeagueTests\Stubs\AccessTokenEntity)
------ -----------------------------------------------------------------
76 Casting to string something that's already string.
76 Parameter #1 $issuer of method Lcobucci\JWT\Builder::issuedBy()
expects non-empty-string, string given.
------ -----------------------------------------------------------------https://github.com/wikimedia/oauth2-server/pull/19 is passing their CI with minimal disabling/exclusions in place.
https://github.com/thephpleague/oauth2-server/pull/1122 redone in https://github.com/wikimedia/oauth2-server/pull/20
I think both end up conflicting with each other...
So lets see if I can get both to merge in one go...
Change #1222274 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/vendor@master] Update league/oauth2-server and dependancies
Change #1222275 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/extensions/OAuth@master] Update league/oauth2-server
Change #1222278 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/core@master] Upgrade psr/http-message and lcobucci/jwt
Change #1222278 abandoned by Reedy:
[mediawiki/core@master] Upgrade psr/http-message and lcobucci/jwt
Change #1222279 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/core@master] Upgrade psr/http-message and lcobucci/jwt
Change #1222278 restored by Reedy:
[mediawiki/core@master] Upgrade psr/http-message and lcobucci/jwt
Change #1222278 abandoned by Reedy:
[mediawiki/core@master] Upgrade psr/http-message and lcobucci/jwt
I note that the work I've done on this doesn't actually fix the state goal of moving away from the fork... But it does move us forward (~3-3.5 years) on our version of the oauth2 server, unblocking T397068: Upgrade psr/http-message to 2.0 or later among others.
Change #1229647 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/vendor@master] Update league/oauth2-server and dependancies
Change #1222274 abandoned by Reedy:
[mediawiki/vendor@master] Update league/oauth2-server and dependancies
Reason:
New change id - https://gerrit.wikimedia.org/r/1229647
Change #1229647 merged by Gergő Tisza:
[mediawiki/vendor@master] Update league/oauth2-server and dependancies
Change #1222279 merged by jenkins-bot:
[mediawiki/core@master] Upgrade psr/http-message and lcobucci/jwt
Change #1222275 merged by jenkins-bot:
[mediawiki/extensions/OAuth@master] Update league/oauth2-server and dependancies
Change #1239176 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/vendor@REL1_43] Upgrading psr/http-message (1.1 => 2.0)
Change #1239177 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/vendor@REL1_44] Upgrading psr/http-message (1.1 => 2.0)
Change #1239178 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/vendor@REL1_45] Upgrading psr/http-message (1.1 => 2.0)
Change #1239182 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/core@REL1_45] Upgrade psr/http-message and lcobucci/jwt
Change #1239185 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/core@REL1_44] Upgrade psr/http-message
Change #1239187 had a related patch set uploaded (by Reedy; author: Reedy):
[mediawiki/core@REL1_43] Upgrade psr/http-message
Reedy's work made it much clearer what exactly we need from the fork, and I ended up reading a lot of oauth2-server code while debugging T417839, so I feel like I know enough to give this a try.
Change #1240644 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):
[mediawiki/extensions/OAuth@master] Migrate from wikimedia/oauth2-server fork to league/oauth2-server
Change #1240651 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):
[mediawiki/vendor@master] Migrate from wikimedia/oauth2-server fork to league/oauth2-server
Change #1240651 merged by jenkins-bot:
[mediawiki/vendor@master] Migrate from wikimedia/oauth2-server fork to league/oauth2-server
Change #1240644 merged by jenkins-bot:
[mediawiki/extensions/OAuth@master] Migrate from wikimedia/oauth2-server fork to league/oauth2-server
It's migrated!
@Reedy I'm not sure what's up with the REL1_43/44/45 patches. Is this something you're still hoping to backport?
I assume we won't be backporting it, give that it would be quite messy and risky.
I think this is all done then!
That means we're never going to support PHP 8.4+ on REL1_45 and before in MW core (and this extension), then. Is that definitely the right call to make?
I just realized that OAuth is not a bundled extension. Can you explain why this problem would affect the supported versions of MW core then? (edit: Oh – some of the libraries we updated in this task are bundled with MediaWiki core, not OAuth.)
I installed MediaWiki 1.43 on PHP 8.5. It seems to work.
Can we be optimistic and assume that 1.43-1.45 and PHP 8.4-8.5 probably all work together, until we get bug reports saying otherwise?
I've just found T424270/T424271/T424272, which were filed to backport things necessary to get the tests passing, and where we've apparently found that MW 1.43, 1.44 and 1.45 pass the tests on PHP 8.4 and 8.5 already.
I think no backports are needed here. If anything comes up, let's continue on those tasks.
Testing on MW 1.43, I just found T425689: Method ReflectionMethod::setAccessible() is deprecated since 8.5, as it has no effect since PHP 8.1 and T425690: Method ReflectionMethod::setAccessible() is deprecated since 8.5, as it has no effect since PHP 8.1 in https://integration.wikimedia.org/ci/job/quibble-composer-mysql-php84-non-voting/99/console
But then OAuth specifically on 1.43 - T425691: OAuth test failures on PHP 8.4+ in MW 1.43/1.44/1.45
Change #1239177 abandoned by Hashar:
[mediawiki/vendor@REL1_44] Upgrading psr/http-message (1.1 => 2.0)
Change #1239185 abandoned by Hashar:
[mediawiki/core@REL1_44] Upgrade psr/http-message