adding support for JWKS #813
Conversation
|
|
authorization.go
Outdated
| func validateJWT(encodedToken string, jwtConfig *jwtConfig, jwksURL string) (*claims, error) { | ||
| var keyFunc jwt.Keyfunc | ||
|
|
||
| if jwksURL != "" { |
There was a problem hiding this comment.
nitpicking: I would invert the test to avoid the negation as there is an else.
There was a problem hiding this comment.
The reason I did that is to prioritize jwks when both jwks and jwt config are present. Do you see a way we could combine these?
authorization.go
Outdated
| if jwksURL != "" { | ||
| jwks, err := keyfunc.Get(jwksURL, keyfunc.Options{}) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get the JWKS from the given URL.\nError: %w", err) |
There was a problem hiding this comment.
| return nil, fmt.Errorf("failed to get the JWKS from the given URL.\nError: %w", err) | |
| return nil, fmt.Errorf("failed to get the JWKS from the given URL: %w", err) |
authorization.go
Outdated
|
|
||
| keyFunc = jwks.Keyfunc | ||
| } else { | ||
| keyFunc = func(token *jwt.Token) (interface{}, error) { |
There was a problem hiding this comment.
Maybe could you use a real symbol instead of an anonymous function to ease testing?
hub.go
Outdated
| @@ -146,6 +146,22 @@ func WithSubscriberJWT(key []byte, alg string) Option { | |||
| } | |||
| } | |||
|
|
|||
| func WithSubscriberJWKS(jwks string) Option { | |||
There was a problem hiding this comment.
It could be cool to support all methods supported by the underlying lib (URL, JSON, and raw key). WDYT?
There was a problem hiding this comment.
As far as I have worked with JWKS, I have always worked with a URL. I think using JSON and raw key defeats the purpose of using the JWKS support as you would need to update those and restart the services in the event of key rotation. I would like to support the keyfunc.Options though. What would you suggest for those options in Caddy config? I was thinking of a nested block.
There was a problem hiding this comment.
Nested blocks looks great. I'm pretty sure that some advanced users will want to use other options so we should support both yet. For instance, advanced CI/CD pipelines may want to bundle the trusted keys.
|
@dunglas Finallly had some time to come back to this again. It now supports URL, json or key as jwks config. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
dunglas
added
pinned
|
Please consider applying. |
|
We cannot merge the path as-is because it uses the deprecated version of Edit: upgrade to |
|
I might have some time to work on #851 in coming weeks. |
|
@broncha btw, do you mind if I force-push my rebase in your branch (I squashed all the commits)? |
|
@dunglas Please go ahead. Ill pick it up from there. |
|
I would also like to unify the jwt and jwks configs too. But I am not sure about the current status of the configs. Ill check those when I get a change to look at this. |
|
Also for the record, one issue have been repeatedly facing is clock skew, where jwt creation and validation happens almost at the same time. It would be good to be able to support, which landed in golang-jwt v5. |
Adds support for JWKS in Mercure. With this update, you would simply configure the JWKS URL and Mercure would validate the subscriber and publisher JWT based on the Key ID and the keys in the JWKS.
Still a work in progress, as most tests that look for JWT need to be duplicated to test JWKS