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

chore: update go-jose to v3.0.0 #746

Closed
wants to merge 1 commit into from

Conversation

JustinHook
Copy link

This commit updates go-jose from v2.5.2 to v3.0.0.

It contains a fix 1 for an issue that is stopping us from updating the version of fosite that we use.

Footnotes

  1. https://github.com/go-jose/go-jose/commit/4ac8eda113724c926632d14de694dbf1ff70d2cc

@JustinHook JustinHook requested a review from aeneasr as a code owner May 2, 2023 11:17
@CLAassistant
Copy link

CLAassistant commented May 2, 2023

CLA assistant check
All committers have signed the CLA.

@JustinHook JustinHook changed the title chore: Update go-jose to v3.0.0 chore: update go-jose to v3.0.0 May 2, 2023
This commit updates go-jose from v2.5.2 to v3.0.0
@JustinHook JustinHook force-pushed the update-go-jose-to-v3.0.0 branch from 91f70cb to 5867db4 Compare May 2, 2023 11:24
@JustinHook
Copy link
Author

@aeneasr can you provide some guidance on how to get the conformity tests to pass?

The tests are failing when trying to compile https://github.com/ory/hydra/blob/872720b3c0d92341a78791004ded44ac8d4ff7c8/client/client.go#L24.

This is because the hydra Client will be using the v2 of go-jose but the fosite.OpenIDConnectClient interface will be using v3 of go-jose.

@james-d-elliott
Copy link
Contributor

I would recommend forking hydra, configure a replacement so it utilizes this branches latest commit, and then see what's wrong and needs fixing.

On face value the only issue appears to be that the github.com/ory/hydra/v2/client struct Client implements GetJSONWebKeys() *"gopkg.in/square/go-jose.v2".JSONWebKeySet instead of GetJSONWebKeys() *"github.com/go-jose/go-jose/v3".JSONWebKeySet.

@JustinHook
Copy link
Author

@james-d-elliott I agree about GetJSONWebKeys. How would you recommend it's resolved?

GetJSONWebKeys is part of the fosite.OpenIDConnectClient interface which is coming from fosite v0.44.0.

Until this PR is merged there is no fosite version that has GetJSONWebKeys() *"github.com/go-jose/go-jose/v3".JSONWebKeySet that hydra can use.

@james-d-elliott
Copy link
Contributor

james-d-elliott commented May 3, 2023

Since this change has no bearing on actual code and is just imports I'd recommend that you discuss the change with the maintainers (to plan it out and get there thoughts of migrating to a completely different package, as this is a breaking change) and in the meantime use the go mod replace tooling.

Side note, I'd be cautious about moving to that package until someone with some fairly detalied knowledge about the JOSE specs has had time to review it and its compatibility with fosite.

Also regarding hydra being able to use it, you can actually do it. See this PR which does exactly that: ory/hydra#3403 specifically this line: https://github.com/ory/hydra/pull/3403/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6R247

@JustinHook
Copy link
Author

The version was bumped in 3c2721e

@JustinHook JustinHook closed this Oct 11, 2023
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