Support for List of JWKS URLs

[aklinkert]

Hi @MicahParks,

I am using JWKS in a service-to-service token communication and would prefer to use keyfunc over the default implementation in https://github.com/gofiber/jwt. In order to have a list of trusted JWKS (one per service) this package would need to accept multiple JWKS endpoints natively.

Right now I am bridging the gap by creating one keyfunc per endpoint and iterating all of the keyfuncs, where trustedJWKSEndpoints holds a []string of trusted JWKS endpoints.

Thank you for providing this library!

Best,
Alex

---

This is what I am doing right now:

func (r *Resolver) buildMiddleware() (err error) {
	resolvers := make(map[string]*keyfunc.JWKS)

	for _, endpoint := range r.trustedJWKSEndpoints {
		resolvers[endpoint], err = keyfunc.Get(endpoint, keyfunc.Options{
			RefreshErrorHandler: func(err error) {
				r.logger.Warnf("failed to refresh JWKS keys from %v: %v", endpoint, err)
			},
			RefreshInterval:   5 * time.Minute,
			RefreshUnknownKID: true,
			RequestFactory:    r.jwksRequestFactory,
		})
		if err != nil {
			return fmt.Errorf("failed to create JWKS key func for %v: %w", endpoint, err)
		}
	}

	config := jwtMiddleware.Config{
		TokenLookup: fmt.Sprintf("header:%s", fiber.HeaderAuthorization),
		ErrorHandler: func(ctx *fiber.Ctx, err error) error {
			return err
		},
		KeyRefreshInterval:   pointer.To(30 * time.Second),
		KeyRefreshRateLimit:  pointer.To(1 * time.Second),
		KeyRefreshUnknownKID: pointer.To(true),
		Claims:               &identitytoken.Claims{},

		KeyFunc: func(token *jwt.Token) (key interface{}, err error) {
			for endpoint, keyFunc := range resolvers {
				key, err = keyFunc.Keyfunc(token)

				// the given keyFunc does provide the required key
				if err == nil {
					return key, nil
				}

				if err != nil && !errors.Is(err, keyfunc.ErrKIDNotFound) {
					return nil, fmt.Errorf("failed to resolve JWK with %v: %w", endpoint, err)
				}
			}

			return nil, keyfunc.ErrKIDNotFound
		},
	}

	r.middleware = jwtMiddleware.New(config)

	return nil
}

[MicahParks]

Thank you for opening this issue, @aklinkert.

It looks like the specific request in this issue is to accept multiple HTTPS URLs for separate JWKS as arguments. Let me know if I read that incorrectly.

I currently do not have plans to change keyfunc.Get to accept multiple HTTPS URLs for separate JWKS. Perhaps adding a new function that can handle multiple JWK Sets would be a good idea.

Click for details about why this would be a new function.

This is primarily because of keyfunc's reliance on the key ID, the kid JWK parameter. It is required that that kid value is unique in order for keyfunc to select a key.

According to RFC 7517 Section 4.5:

When "kid" values are used within a JWK Set, different keys within the JWK Set SHOULD use distinct "kid" values.

While this language isn't strong, it does imply the kid values are supposed to be unique within a JWK Set. Implying that different JWK Sets are independent in selecting kid values. I wouldn't want keyfunc.Get to cause difficult to debug issues. For example, if a user provides HTTPS URLs to JWK Sets A and B. If there is a conflicting kid with the value myRSAKey in A and B a user of keyfunc would likely have a hard time debugging this issue. Also, given the dynamic nature of JWK Sets, this problem could arise in production after the development phase. However, I know in practice this is unlikely, especially in some use cases where JWK Sets use UUID v4 values for key IDs.

I do see the value in a generalized keyfunc for multiple JWK Sets and may add it to the keyfunc project. Probably a generalized implementation and not tied to gofiber. This would come with the caveat that key ID collisions would need to be noted for package users, although I am unsure about the specific implementation details for that case.

I agree with your decision to not use the github.com/gofiber/jwt package at this time. This is because an old, pre-release, version of keyfunc was copied into this project and modified but never updated. It contains multiple known bugs that have since been fixed.

What is your opinion on the default behavior of key ID collisions? An error? Selecting the first one silently? Should a package user of keyfunc be able to specify key ID collision behavior? Could you provide me with more details of what you would like a function that accepts multiple HTTPS URLs for separate JWKS to look like? I am not very familiar with gofiber as a framework and would prefer a more generalized discussion, if possible.

[aklinkert]

Thank you for the quick response @MicahParks!

It looks like the specific request in this issue is to accept multiple HTTPS URLs for separate JWKS as arguments. Let me know if I read that incorrectly.

That is absolutely correct! 💯 I also do agree that it would be better to add an additional method/struct to support multiple JWKS URLs instead of modifying the existing keyFunc.Get - it's good as it is!

What is your opinion on the default behavior of key ID collisions? An error? Selecting the first one silently? Should a package user of keyfunc be able to specify key ID collision behavior?

Uhm, I would leave the decision to the package user, it will depend on the usage. For my case it's not an issue as we're using UUIDv4s as key IDs mainly, so for me it would be select the first one found silently as implemented in the code in the issue body.

Could you provide me with more details of what you would like a function that accepts multiple HTTPS URLs for separate JWKS to look like? I am not very familiar with gofiber as a framework and would prefer a more generalized discussion, if possible.

As far as I can tell this is not more fiber specific than the matching of KeyFunc interfaces. :) I don't know hot it looks like for other users, but for me a simple list of strings with URLs to JWKS endpoints (regardless of HTTP in case of local development or HTTPS after deployment) would be perfectly enough.

If you want I can try to open a PR with what I have so far, otherwise you can use my code if you want and start from there :) However you like!

Best,
Alex

[MicahParks]

Thank you for your input. A pull request would be very welcome!

[MicahParks]

@aklinkert, I'd like to invite you to review #78. I believe this PR addresses this issue.