Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a breakage change in the jwt-go v4 update #37

Merged
merged 1 commit into from
Mar 8, 2021

Conversation

suhaibmujahid
Copy link
Contributor

The package is not compiling. The package dgrijalva/jwt-go got updated to v4. However, the new version changes the type for the claim's time form int64 to jwt.Time.

 % go test
# github.com/bradleyfalzon/ghinstallation [github.com/bradleyfalzon/ghinstallation.test]
./appsTransport.go:68:3: cannot use time.Now().Unix() (type int64) as type *jwt.Time in field value
./appsTransport.go:69:3: cannot use time.Now().Add(time.Minute).Unix() (type int64) as type *jwt.Time in field value
FAIL    github.com/bradleyfalzon/ghinstallation [build failed]

Such bug could be caught by running the tests. I suggest utilizing the Github Actions to build and run the tests on each commit. I can submit a pull request to setup the Github Actions if you agree to do so.

@suhaibmujahid
Copy link
Contributor Author

@bradleyfalzon could you please look at this!

@suhaibmujahid
Copy link
Contributor Author

Even with this patch, jwt-go v4 itself still has an issue when I tried it in my application.

Note: jwt-go v4 still in preview (see here)

@suhaibmujahid
Copy link
Contributor Author

From jwt-go's README file:

NEW VERSION COMING: There have been a lot of improvements suggested since the version 3.0.0 released in 2016. I'm working now on cutting two different releases: 3.2.0 will contain any non-breaking changes or enhancements. 4.0.0 will follow shortly which will include breaking changes. See the 4.0.0 milestone to get an idea of what's coming. If you have other ideas, or would like to participate in 4.0.0, now's the time. If you depend on this library and don't want to be interrupted, I recommend you use your dependency mangement tool to pin to version 3.

@lollipopman
Copy link

@bradleyfalzon would it be possible to revert the jwt-go v4 upgrade, but still keep the update of go-github made in b7490f6?

@wlynch
Copy link
Collaborator

wlynch commented Mar 8, 2021

Sorry this has been neglected!

I'm not familiar with the reasoning behind the upgrade to v4 - so I'm hesitant to downgrade without knowing more.

Separately I think there might be an argument to move away from jwt-go altogether due to some outstanding CVEs (https://nvd.nist.gov/vuln/detail/CVE-2020-26160) and concerns around its maintenance (go-chi/jwtauth#50 (comment)).

I'll merge this as-is to fix the breakage, and we can tackle v3 vs jwt-go alternatives in another issue/PR.

@wlynch wlynch merged commit 47f828e into bradleyfalzon:master Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants