cfsamson / azure-jwt

A library for validating OAuth tokens from Azure AD
MIT License
10 stars 8 forks source link

update dependencies #4

Open SorenHolstHansen opened 7 months ago

SorenHolstHansen commented 7 months ago

Thanks for the nice library!

I was looking at my Cargo.lock file, and saw that wasm-bindgen was included because of this lib. It is because chrono includes it by default. So tidied up the chrono dependency and bumped jsonwebtoken as well

cfsamson commented 7 months ago

Thanks for the PR!

This looks good, but I get an error when running cargo test. Digging a little deeper (removing the assert in generate_test_token) gives me this error:

thread 'tests::decode_token' panicked at src/lib.rs:701:40:
called `Result::unwrap()` on an `Err` value: InvalidToken(Error(Base64(InvalidPadding)))

So it seems like there is something that's changed either in the jwt library itself or in Base64.

I can't really figure out the nature of this error right now but if you can figure out what causes this error I'd be happy to merge the PR.

SorenHolstHansen commented 7 months ago

Thanks, I will look into it

SorenHolstHansen commented 7 months ago

I was unable to get it to work. I fixed the invalid padding error, but now i get

thread 'tests::decode_token' panicked at src/lib.rs:710:40:
called `Result::unwrap()` on an `Err` value: InvalidToken(Error(InvalidSignature))

which I could not fix. Perhaps you have an idea what could be the issue?

This might also be the cause of the error in the assert in generate_test_token

cfsamson commented 7 months ago

Hi, and thanks.

After looking a bit more into this, it seems that the base64 library has improved its detection of invalid padding. I also see reports on OpenSSL generating keys in base64 with invalid padding, so it might be that the version of OpenSSL I used to generate the tests generated invalid padding (at least according to the base64 library). I'm a little bit wary of merging this until we get it resolved.

If I recall correctly, the keys need to be padded to a certain length, so using the *_NO_PAD version in the base64 library might very well produce an invalid signature, which is to be expected if I'm right (I'll have to confirm this as I take it out of memory from a long time ago, I could be mistaken here).

I think the next thing to try is to generate new pem files for the tests and see if that works. I'll have to sit down and try that out when I get more time since I remember getting the tests up and running was a huge part of creating this library (creating the tests was not trivial).

SorenHolstHansen commented 7 months ago

Sounds good! Thanks for the help, and no stress in getting it fixed, just when you get the time :)