Fixed decoding of token when validation has multiple families of algorithms

[Wassasin]

Currently if you have

        let mut validation = Validation::default();
        validation.algorithms = vec![
            Algorithm::HS256,
            Algorithm::HS384,
            Algorithm::HS512,
            Algorithm::RS256,
            Algorithm::RS384,
            Algorithm::RS512,
        ];

and try to validate a RS256 signed token, you will get a Error::InvalidAlgorithm due to the section:

        for alg in &validation.algorithms {
            if key.family != alg.family() {
                return Err(new_error(ErrorKind::InvalidAlgorithm));
            }
        }

This section should check whether the key is one of the accepted families. Currently it checks if an algorithm is accepted with a different family, and if so rejects the key.

[Keats]

The intent there was to allow only 1 family for the validation but the types were not good enough. In the future, better types for each alg family will be used and the validation fn will require a specific family

[lmm-git]

May I ask why the intent is to allow only one family? The docs state

    /// The validation will check that the `alg` of the header is contained
    /// in the ones provided and will error otherwise. Will error if it is empty.
    ///
    /// Defaults to `vec![Algorithm::HS256]`.

So I would expect that if I allow multiple algorithms (even from different families) this should be fine. But it fails during runtime with a generic error.

Therefore, I would really appreciate to either update the docs or merge this MR.

[Keats]

We can update the docs to say it should only contain alg from the same family.

[lmm-git]

But just to be sure: is there any reason on why allowing only one family makes sense?

For example, I have a use case where the library should verify tokens for their expiration and if they are signed off by a specific IdP. In this use-case it is not relevant (for me) which algorithm family the signing algorithm is belonging to, but I would like to allow some "secure enough" algorithms (which might be from different families. Actually, the algorithm depends on other services the tokens are getting used on later.

[Keats]

You need to pass the https://docs.rs/jsonwebtoken/latest/jsonwebtoken/struct.DecodingKey.html to decode a token and to do that you need to know the family anyway. Also https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/ could be a concern maybe if we allow multiple families

[lmm-git]

Fair point, but as the available algorithm family depends on the DecodingKey, wouldn't it make sense to select the algorithm according to the key? In most JWK the algorithm is specified anyways.

Another option would be to allow to pass JWKs directly to the validator and the validator can accept algorithms allowed in the single JWKs?

I am aware that these changes are breaking, but imho could improve the usability of this API and overall security, as for example I would assume that having projects to handle their JWKs manually like right now is much more error prone.

Apart from that a documentation change would be nice, this probably would have fixed the issue for me in the first place.