Add a static JWKS for testing purposes

[borgoat]

According to #179 , the way to override the proper OIDC discovery of well-known endpoints is by implementing a KeySet which can e.g. verify tokens with a provided JSONWebKeySet.

This is basically an implementation of what is shown here https://godoc.org/github.com/coreos/go-oidc#NewVerifier which I needed for testing purposes. Knowing that some do want to avoid the discovery and just put public keys in configuration I thought this may be useful for general use

[borgoat]

@ericchiang a question: I saw in your implementation that verifying multiple signatures is not supported. But then found out square/go-jose has this.. What do you suggest we do?

[mikedanese]

In general, mocks should be implemented in the test package that they are used in. KeySet is an interface to allow for external implementations. This change also adds a new dependency to our public API.

[ericchiang]

Yeah I agree with @mikedanese here.

Can this be an example instead? It's not a ton of code:

type staticKeySet struct {
	keySet jose.JSONWebKeySet
}

func (s *staticKeySet) VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) {
	jws, err := jose.ParseSigned(jwt)
	if err != nil {
		return nil, fmt.Errorf("parse jwt: %v", er)
	}
	for _, key := range s.keySet.Keys {
		if payload, err := jws.Verify(key); err == nil {
			return payload, nil
		}
	}
	return nil, fmt.Errorf("no keys were able to verify payload")
}

[borgoat]

As you suggest @ericchiang , that's what I started doing in my tests, but then realised it may in fact be useful for others...

I know of companies that won't use OpenID Connect Discovery but only out-of-band config (as mentioned above, by adding public keys in those config files).
I guess this has to do with some OpenID Providers not supporting that part of the spec, plus some security rules that some may want/have to adhere to.

If I understand correctly, Discovery seems to be an optional part of the spec? So Relying Parties should be able to work without that.

[...] This change also adds a new dependency to our public API.

@mikedanese on this one.. isn't that ok if we add a method? I think the problem with an API would be if we remove or altered an existing function

[ericchiang]

There are a lot of JWT Go libraries, adding go-jose to our package API means that we're not allowed to switch in the future, and also that users have to import go-jose in their own package to use our API. That design decision was intentional since there's been a lot of churn in the Go community in the past :) though admittedly that's less of an issue today.

The current key set implementation in the package is nice because it handles key rotation. Even if you distribute keys out-of-band you still want to think about rotation, which this API doesn't do.

Basically, I think this would be most useful for testing, and I'd rather have an example or documentation for that.

[borgoat]

Hey @ericchiang and @mikedanese !

Apologies for the delay, I finally found some time to fix this..

I now did as you suggested and just moved the code in the docs, simply adding to that section which I linked in the beginning of the thread
https://godoc.org/github.com/coreos/go-oidc#NewVerifier

I guess in this case it's okay if we reference the go-jose library? In the end this is just meant to be a suggestion on how one could implement the KeySet interface..

[mitar]

I was reading documentation of NewVerifier and read about newStatisKeySet in the example, but I was surprised that newStatisKeySet is not simply provided as well in this package. It looks pretty straightforward to do so, no?

newStatisKeySet as documented (so accepting multiple keys) is already enough to handle key rotation as well: you introduce a new key in the configuration, adding it alongside the old key, you deploy, have both keys around for some time, after everything else stops using the old key, you remove the old key and redeploy with only one key.

So +1 for adding newStatisKeySet to the library itself.

[ericchiang]

I don't think the opinion in #223 (comment) has changed. We don't want to do this because it'd require exposing the JWT library in our API. And we'd potentially like to be able to use a different one in the future (e.g. if one ever came to golang.org/x/crypto)

[mitar]

I made even simpler one for my purposes (it does not support key rotation):

type staticKeySet struct {
	publicKey interface{}
}

func (s *staticKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) {
	jws, err := jose.ParseSigned(jwt)
	if err != nil {
		return nil, err
	}
	payload, err := jws.Verify(s.publicKey)
	if err != nil {
		return nil, err
	}
	return payload, nil
}

I do not see where is the JWT library exposed?

[ericchiang]

jwt.Verify technically takes jose.JSONWebKey, which we probably want to discourage.

I would be okay with an API like

// StaticKeySet is a verifier that validates JWT against a static set of public keys.
type StaticKeySet struct {
    // PublicKeys used to verify the JWT. Supported types are *rsa.PublicKey and *ecdsa.PublicKey.
    PublicKeys []crypto.PublicKey
}

// VerifySignature compares the signature against a static set of public keys.
func (s *StaticKeySet) VerifySignature(ctx context.Context, jwt string) ([]byte, error) {
    // ...
}

[mitar]

I think this looks great.

[ericchiang]

Cool! sent #337

[mitar]

This can be closed now, no?