Support for jwt v5

[oxisto]

This serves as an experimental draft PR for the upcoming v5 version of jwt. Since we are most likely changing the Claims interface, this also has an effect on this library.

Basically there are two options here:

• Upgrading to the v5 claims at some point, possibly also with another major version
• Supporting both v4 and v5 for example using two different functions.

Any thought on this? I am happy to make the transition as smooth as possible from the jwt side of things, but I fear that we do need to make this break in v4->v5, otherwise we will never succeed with a proper validation API.

[MicahParks]

For easy reference, here is the upstream PR: golang-jwt/jwt#234

Thanks for the heads-up and draft PR, @oxisto! Please feel free to make any changes that would improve the upstream JWT package as you see fit. I think the narrow scope of this project isolates it from any changes except a major semver increment or jwt.Keyfunc signature change. Updating the jwt.Claims interface shouldn't affect this project directly.

I believe the Go compiler would treat any semver major upgrade as a breaking change since the function signature from https://pkg.go.dev/github.com/golang-jwt/jwt/v4#Keyfunc is tied to semver major version 4 of the token: https://pkg.go.dev/github.com/golang-jwt/jwt/v4#Token. An upgrade to semver major version 5 would change that reference to version 5 of the token, and I think the Go compiler would say those are incompatible types, even if there were no changes to the underlying data structure.

This will require an update to this package and I think you've identified both options that I would consider.

Upgrading to the v5 claims at some point, possibly also with another major version.

I like the above option because of it's simplicity, however, the drawback is that I'll need to do a major semver upgrade and make that clear to any package users.

Supporting both v4 and v5 for example using two different functions.

I like this option because it does not require a major semver upgrade and users can simply do a minor upgrade. It also requires fewer changes to the code and documentation overall. However, this is the option I picked with github.com/dgrijalva/jwt-go when github.com/MicahParks/keyfunc was in pre-release. This was the biggest drawback:

Vulnerability scanners marked this package as being affected by CVEs via the dependencies. Even though the CVE didn't affect keyfunc or its usage, here are two examples of requests to remove that dependency: #15 and #20. If there were a vulnerability in github.com/golang-jwt/jwt/v4, I imagine there would be similar requests if the upstream project didn't release a fix for /v4. Also, I understand it's nice to have "minimal" dependencies. If a developer was using the latest upstream JWT package, it would be best to only depend on the latest. I made github.com/MicahParks/compatibility-keyfunc to support older versions to cover legacy forks. Additionally, here's an example where it was resorted to copy/paste an older version of the package to remove an unneeded dependency: gofiber/jwt#57. Unfortunately, almost a year later, the code copied into that project still contains some bugs from the pre-release of keyfunc ☹️ I wouldn't want that to happen again.

For those reasons, I'm leaning towards the former option. If I choose the former option, I have two ways to implement that. Either via git tag or a sub-directory. The official advice is to use a sub-directory:

The recommended strategy is to develop v2+ modules in a directory named after the major version suffix.

However, I've always used the git tag strategy and I've also seen that more often than the sub-directory option. So I'm leaning towards using git tag again. This means a keyfunc/v2 would remove all support for github.com/golang-jwt/jwt/v4 from this repository. If there were significant changes to this repository, I may consider updating github.com/MicahParks/compatibility-keyfunc to support github.com/golang-jwt/jwt/v4 with any new features from keyfunc/v2. I'd likely implement #40 under github.com/golang-jwt/jwt/v4 before merging on a /v2 release.

If anyone has any comments or suggestions, please feel free to leave them here for consideration.

[oxisto]

For easy reference, here is the upstream PR: golang-jwt/jwt#234

I based it on the validation API PR for now (golang-jwt/jwt#236), but this will end up in the v5 PR you mentioned at some point.

Thanks for the heads-up and draft PR, @oxisto! Please feel free to make any changes that would improve the upstream JWT package as you see fit. I think the narrow scope of this project isolates it from any changes except a major semver increment or jwt.Keyfunc signature change. Updating the jwt.Claims interface shouldn't affect this project directly.

Unfortunately, it does. The problem isn't arising through the semver but rather the problem is that the jwt.Claims interfaces changes drastically. I removed the Valid function and added new functions that describe getters to standard claims that always must be there. Now since Keyfunc references Token and Token has a property with Claims, this basically also changes Keyfunc.

A very very very very hacky way to work around this would be to include the "old" Valid() error function in the new claims interface as well. This way the compiler won't complain but in case someone would call Valid() call hell brakes loose because they are supposed to go through the new Validator or use Token.Valid. I have no way of injecting the validator in the claims so a call to Valid cannot produce any thing good. So while this would keep "compatibility" with your project, it would probably create complete chaos :)

I believe the Go compiler would treat any semver major upgrade as a breaking change since the function signature from https://pkg.go.dev/github.com/golang-jwt/jwt/v4#Keyfunc is tied to semver major version 4 of the token: https://pkg.go.dev/github.com/golang-jwt/jwt/v4#Token. An upgrade to semver major version 5 would change that reference to version 5 of the token, and I think the Go compiler would say those are incompatible types, even if there were no changes to the underlying data structure.

See above, actually, no. As long as the data structures would stay the same, they would be compatible, but as mentioned above the meaning would be totally different.

This will require an update to this package and I think you've identified both options that I would consider.

Upgrading to the v5 claims at some point, possibly also with another major version.

I like the above option because of it's simplicity, however, the drawback is that I'll need to do a major semver upgrade and make that clear to any package users.

Supporting both v4 and v5 for example using two different functions.

I like this option because it does not require a major semver upgrade and users can simply do a minor upgrade. It also requires fewer changes to the code and documentation overall. However, this is the option I picked with github.com/dgrijalva/jwt-go when github.com/MicahParks/keyfunc was in pre-release. This was the biggest drawback:

Vulnerability scanners marked this package as being affected by CVEs via the dependencies. Even though the CVE didn't affect keyfunc or its usage, here are two examples of requests to remove that dependency: #15 and #20. If there were a vulnerability in github.com/golang-jwt/jwt/v4, I imagine there would be similar requests if the upstream project didn't release a fix for /v4. Also, I understand it's nice to have "minimal" dependencies. If a developer was using the latest upstream JWT package, it would be best to only depend on the latest. I made github.com/MicahParks/compatibility-keyfunc to support older versions to cover legacy forks. Additionally, here's an example where it was resorted to copy/paste an older version of the package to remove an unneeded dependency: gofiber/jwt#57. Unfortunately, almost a year later, the code copied into that project still contains some bugs from the pre-release of keyfunc ☹️ I wouldn't want that to happen again.

For those reasons, I'm leaning towards the former option. If I choose the former option, I have two ways to implement that. Either via git tag or a sub-directory. The official advice is to use a sub-directory:

The recommended strategy is to develop v2+ modules in a directory named after the major version suffix.

However, I've always used the git tag strategy and I've also seen that more often than the sub-directory option. So I'm leaning towards using git tag again. This means a keyfunc/v2 would remove all support for github.com/golang-jwt/jwt/v4 from this repository. If there were significant changes to this repository, I may consider updating github.com/MicahParks/compatibility-keyfunc to support github.com/golang-jwt/jwt/v4 with any new features from keyfunc/v2. I'd likely implement #40 under github.com/golang-jwt/jwt/v4 before merging on a /v2 release.

If anyone has any comments or suggestions, please feel free to leave them here for consideration.

Yeah, I feel you. I would also prefer option 2 but its a hassle because you will transitively introduce a dependency to an old package. Even though the Go compiler is smart enough to only include this if you would use the old function, a lot of vulnerability scanners won't. Maybe one option would be to actually have separate go modules, one for "core", one for v4, one for v5. But a this point its probably just best to swallow the pill and release a v2 and sort of tie yourself to the upstream major versions: v1 -> v4, v2 -> v5 and so on.

[MicahParks]

Sorry for the wording, what I meant to say by "a major semver increment" was that the Go module declaration for github.com/golang-jwt/jwt/v4 will change to github.com/golang-jwt/jwt/v5 causing the "concrete" types in /v5 to differ from those in /v4 for the Go compiler. Specifically, the *jwt.Token type from /v5 cannot be used when the type from /v4 is expected. What I meant to convey is that a change in the Go module declaration for github.com/golang-jwt/jwt/v4 would require an update to the keyfunc project's go.mod, go.sum, and some import statements. Other than that, I think the planned changes are compatible for the purposes of the keyfunc project.

After glancing at the keyfunc project's code base, it looks like it does not access JWT claims or use the jwt.Claims interface, I believe the only time it accesses a field or method of the *jwt.Token type is here:

keyfunc/keyfunc.go

Line 19 in 7892f09

kidInter, ok := token.Header["kid"]

It checks the token's header for a kid JSON attribute in order to select a cryptographic key.

If the new Validator changes how a JWT is validated from accessing *jwt.Token's Valid field (shown below), I will need to update the documentation and examples to reflect the proper way in github.com/golang-jwt/jwt/v5.

keyfunc/examples/recommended_options/main.go

Line 52 in 7892f09

if !token.Valid {

I don't mean to dismiss any concerns about the planned changes' effect on the keyfunc project's code base. If I missed a reference to JWT claims or other planned change, we should identify that for later.

But a this point its probably just best to swallow the pill and release a v2 and sort of tie yourself to the upstream major versions: v1 -> v4, v2 -> v5 and so on.

This is what I'm currently leaning towards. It will likely make reading the documentation and making decisions about versions simpler for package users.

[oxisto]

Sorry for the wording, what I meant to say by "a major semver increment" was that the Go module declaration for github.com/golang-jwt/jwt/v4 will change to github.com/golang-jwt/jwt/v5 causing the "concrete" types in /v5 to differ from those in /v4 for the Go compiler. Specifically, the *jwt.Token type from /v5 cannot be used when the type from /v4 is expected. What I meant to convey is that a change in the Go module declaration for github.com/golang-jwt/jwt/v4 would require an update to the keyfunc project's go.mod, go.sum, and some import statements. Other than that, I think the planned changes are compatible for the purposes of the keyfunc project.

I don't want to start an argument here, but just so that we are on the same track of understanding: The Go compiler actually does not care about v4 or v5. I tried that. As long as I do not change any structure of the Keyfunc, Token and Claims types, the compiler will happily accept a Keyfunc of this project which takes a v4.Token and accepts it for the ParseWithClaims structure of v5. It seems that the compiler compares the actual structure down to the individual non-struct types. It would even still accept it if I would only add functions to the Claims because then v5.Claims would still satisfy the interface of v4.Claims. Which is actually a good thing, BUT.

The problem lies within my previous statement that we need to change the Claims structure and in a way that actually removes Valid(). This then propagates up the chain from Token to Keyfunc, which then makes v4.Keyfunc and v5.Keyfunc in-compatible. Even though you are not directly accessing claims (as written below), just the simple fact that the function signature uses the "changed" Keyfunc makes it incompatible.

So, same result, but I just wanted to make sure that we have the same understanding of our options :)

It checks the token's header for a kid JSON attribute in order to select a cryptographic key.
If the new Validator changes how a JWT is validated from accessing *jwt.Token's Valid field (shown below), I will need to update the documentation and examples to reflect the proper way in github.com/golang-jwt/jwt/v5.

keyfunc/examples/recommended_options/main.go

Line 52 in 7892f09

if !token.Valid {

That is still the preferred way to do it. I was referring to some code that I have seen which used the Valid() function of Claims instead of Token.Valid to check, and this was always possible but discouraged.

I don't mean to dismiss any concerns about the planned changes' effect on the keyfunc project's code base. If I missed a reference to JWT claims or other planned change, we should identify that for later.

The change is in this PR. golang-jwt/jwt#236

But a this point its probably just best to swallow the pill and release a v2 and sort of tie yourself to the upstream major versions: v1 -> v4, v2 -> v5 and so on.

This is what I'm currently leaning towards. It will likely make reading the documentation and making decisions about versions simpler for package users.

That sounds reasonable

[oxisto]

We have arrived at the first release candidate for v5.

[dkiser]

Are there any updates on when this might be available?

[MicahParks]

Are there any updates on when this might be available?

Please note the upstream project, github.com/golang-jwt/jwt is currently still in /v4. There are release candidates for /v5, but no official release. Please follow the upstream project for updates.

[oxisto]

Are there any updates on when this might be available?

Please note the upstream project, github.com/golang-jwt/jwt is currently still in /v4. There are release candidates for /v5, but no official release. Please follow the upstream project for updates.

Don’t tell anyone but I will release v5 later tonight ;)

[MicahParks]

I expect to release github.com/MicahParks/keyfunc/v2 before 9pm Easter Daylight Time (EDT) tomorrow. This should be compatible with github.com/golang-jwt/jwt/v5. Thank you, @oxisto, for the pull request and your work to make github.com/golang-jwt/jwt/v5 possible.

Unless discussed otherwise, the version of MicahParks/keyfunc will follow the version of golang-jwt/jwt three major versions behind. github.com/MicahParks/keyfunc/v2 will be compatible with github.com/golang-jwt/jwt/v5, github.com/MicahParks/keyfunc/v3 will be compatible with github.com/golang-jwt/jwt/v6, and so on.

This is in line with the first option discussed in the earliest comments in the thread. In this option, keyfunc upgrades to the newest upstream version and stops supporting the older version. The README.md of this project should note the latest semver release of keyfunc that works with /v4. If an update is necessary, I'll publish a release to github.com/MicahParks/compatibility-keyfunc with /v4 as a newly supported legacy version.

If someone would like to discuss the second option, a function for both /v4 and /v5, please start that discussion soon.

I plan on using breaking change opportunity to reclaim the names of some deprecated functions. For example, NewGivenEdDSACustomWithOptions will become NewGivenEdDSA. If anyone has ideas for other breaking changes to include in /v2, please let me know.

I should also update the go.mod module declaration to include /v2 and change the minimum Go version to 1.18, to reflect the upstream requirement and enable the newer features in this code base, if a use case fits.

[MicahParks]

If you'd like to follow along with the final edits, please see this pull request:
#84