Invalid multiple algorithm logic

[ereOn]

The Validation takes a Vec of Algorithm but the validation code here incorrectly ensures that the JWT's algorithm is compatible with all the specified algorithms, instead of just ensuring it finds at least one that matches.

Put otherwise, this:

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

Should be more like:

  if validation.validate_signature {
        for alg in &validation.algorithms {
            if key.family == alg.family() {
                // Success: let's move on.
            }
        }

        return Err(new_error(ErrorKind::InvalidAlgorithm));
   }

As it stands, it is impossible to use more than one algorithm in a validation because of this bug.

[Keats]

As it stands, it is impossible to use more than one algorithm in a validation because of this bug.

Yeah that's intentional. Ideally we would solve it at the type level to only allow a single family but I didn't want to change the types too much.

[beanow-at-crabnebula]

Ran into the same problem. So if I understand correctly, in a scenario where all of the validations options are identical but supporting multiple algorithms. We should maintain multiple Validation struct copies per family?

I'm curious why that's intentional.

Is it possible the Validation struct gained a responsibility to ensure a specific DecodingKey is appropriate rather than the DecodingKey's family being used for this?

[Keats]

Most of the time, the Validation struct can be defined in a const way (well lazy_static or once_cell).

Is it possible the Validation struct gained a responsibility to ensure a specific DecodingKey is appropriate rather than the DecodingKey's family being used for this?

As in passing the decoding key to the validation? It's possible but you lose the build once only approach

[beanow-at-crabnebula]

Most of the time, the Validation struct can be defined in a const way (well lazy_static or once_cell).

The issue remains that we'd still have to separate them by algorithm family. E.g. a (static) VALIDATE_RSA, VALIDATE_EC, ...

As pointed out before the current logic is that: If anything in validation.algorithms has a different family than the DecodingKey family, return InvalidAlgorithm. So if validation.algorithms contains mixed families, this will always be true.

For reference, my use-case is that we're allowing several algorithms and obtaining the public keys from JWKS. I was hoping the allowlist could be enforced (again) by specifying the allowed algorithms in Validation.