search

limosa-io / openid-connect-server

This is an OpenID Connect Server written in PHP.
GNU Affero General Public License v3.0
46 stars 9 forks source link

Chain extra claims #27

Closed Robinnaiitor closed 1 year ago

Robinnaiitor commented 1 year ago

The way lcobucci/jwt adds claims to the token has changed in v5.x and this breaks the IdToken (See commit: https://github.com/lcobucci/jwt/commit/3c04dd998ce254d2d69aa6cd9cc73ae73a9b17c4).

arietimmerman commented 1 year ago

Thanks, but shouldn't the composer.json/.lock file be updated too?

Robinnaiitor commented 1 year ago

Thanks, but shouldn't the composer.json/.lock file be updated too?

It's already indirectly required by league/oauth2-server, if you lock lcobucci/jwt in this project to ^5.0 it might break BC in other projects which still rely on ^4.3 (as is supported by league/oauth2-server). The code works on both versions though, so it should not really matter

RobertMe commented 1 year ago

Thanks, but shouldn't the composer.json/.lock file be updated too?

A composer.lock isn't relevant for libraries and will never be used. Most composer packages (which are libraries) don't even have a composer.lock file in the repository because composer won't use it when the package is installed as dependency anyway.

But IMO the composer.json should have an explicit dependency set on lcobucci/jwt. This as, this PR proves, this library does have a dependency on it (in other words: this package actually does use lcobucci/jwt) and thus is "vulnerable" to API breakages of lcobucci/jwt and should thus state with which versions it is compatible. So previously this should have been ^4.3 and with this PR it should be ^4.3|^5.0. In case a version 6 of lcobucci/jwt would be released updating to this version would be disallowed so users won't end up with a broken codebase / project after running a composer update. And it would be my guess that dependabot, which is configured on this repository, would then notify about updates for lcobucci/jwt after which compatibility can be checked and the version constrained can be widened (which would then become ^4.3|^5.0|^6.0)

arietimmerman commented 1 year ago

Fixed by #28