Keats / jsonwebtoken

JWT lib in rust
MIT License
1.66k stars 266 forks source link

Question: expected audience in validation but not encoded in the JWT seems a valid option #376

Closed VGLoic closed 6 months ago

VGLoic commented 6 months ago

Hello!

First thanks a lot for this crate, it helps me greatly!

I have a question, I have created a Validation where I set my expected audience. I then encode a JWT without giving any audience parameter. When I verify its validity using the decode method, I don't have any issue. Is it an expected behaviour? Thanks in advance!

I forked the repo and created a dummy PR with a test change (that pass in CI) to illustrate the thing :)

Also here the code below

let my_claims = Claims {
        sub: "b@b.com".to_string(),
        company: "ACME".to_string(),
        exp: OffsetDateTime::now_utc().unix_timestamp() + 10000,
    };
    let privkey_pem = include_bytes!("private_rsa_key_pkcs1.pem");
    let pubkey_pem = include_bytes!("public_rsa_key_pkcs1.pem");
    let certificate_pem = include_bytes!("certificate_rsa_key_pkcs1.crt");

    for &alg in RSA_ALGORITHMS {
        let mut validation = Validation::new(alg);
        validation.set_audience(&["https://my-random-audience.com"]);

       // Does not reject
        let token =
            encode(&Header::new(alg), &my_claims, &EncodingKey::from_rsa_pem(privkey_pem).unwrap())
                .unwrap();
        let token_data =
            decode::<Claims>(&token, &DecodingKey::from_rsa_pem(pubkey_pem).unwrap(), &validation)
                .unwrap();
        assert_eq!(my_claims, token_data.claims);
        assert!(token_data.header.kid.is_none());
    }
Keats commented 6 months ago

You need to also set validate_aut to True https://docs.rs/jsonwebtoken/latest/jsonwebtoken/struct.Validation.html

VGLoic commented 6 months ago

Thanks for the answer!

I updated the PR with the addition of the validation.validate_aud = true; but the tests are still passing

PS: I disabled the pinned version of the tests as it was failing because of setup issues

Keats commented 6 months ago

I had a look. In practice you should set validation.set_required_spec_claims(&["exp", "aud"]); to validate the presence of each field because the validation only happens for fields present. See https://github.com/Keats/jsonwebtoken/issues/190 for the original issue. I do find it confusing though and i indeed forgot about that

VGLoic commented 6 months ago

Ah nice! Thanks a lot for having a look! I missed it when reading the code, my bad!

Would it be worth to complete the examples/validation or the README?

If you consider it's not worth it, please do not hesitate to close the issue :)

Keats commented 6 months ago

Yes definitely, and also explain it in the Validation struct doc

VGLoic commented 6 months ago

I will take one week of holiday but happy to prepare a PR for it at my return :)

Obviously, feel free to take it fast if you want it out of your system ahah