Define a "jwt base class" for exceptions

[prince-chrismc]

closes #215

[prince-chrismc]

There's actually a lot of these left

[Thalhammer]

This feels really weird cause now signature_verification_exception inherits from std::exception twice.
Could we get away with inheriting exception from std::system_error (which inherits std::runtime_error, which in turn inherits std::exception) ?

[Thalhammer]

See above, given that we dont actually set a dynamic error message, we should probably add a "invalid_json" error code and stick that in.

[prince-chrismc]

I think we need a more clear goal for this one. I was not expecting it to be soo big 🙈

On the server side if you only do this

try { 
     process_jwt(token);
catch (jwt_exception e) {
     //... handling 
}

How can you know to return 401 vs 403 😕 there's a difference for the token being malformed and not granting permission 🤔

[Thalhammer]

there's a difference for the token being malformed and not granting permission

Wouldn't this normally be handled in two different places ?
First you check if the token is a valid token (i.e. not malformed, not expired, etc.) then you check if the token (which you can now trust) actually allows access to the resource at hand. At least in my code bases this doesn't even happen in the same place, the first check is inside a middleware whereas the second is in the individual handler.

Normally the difference between multiple error groups is handled by std::error_category, if the token is malformed you'd get signature_verification_error_category() as the category, whereas a expired token (or one missing claims,etc) would give you a token_verification_error_category() error. So your handling (assuming you check the resource access inside the verifier) would look something like this:

try { 
     process_jwt(token);
catch (jwt_exception e) {
    if(e.code().category() == signature_verification_error_category())
         return 401;
    if(e.code().category() == token_verification_error_category())
         return 403;
    return 500;
}

You could also dig down into the actual error code, e.g. to distinguish token_expired and audience_missmatch.

Regardless, the fact that all the exception types now inherit std::exception twice definitly feels wrong and will almost certainly cause issues. What we could do is have a jwt::exception inherit std::system_error (and thus std::runtime_error and std::exception) and then inherit something like invalid_json_exception from jwt::exception so you can still catch those individually instead of checking the error category if you want.

[prince-chrismc]

Tomorrow while I travel, I will look at this again... I agree this isn't up to par. Theres 130+ exceptions that are from the STL... some are for sure traits but others I have no clue...

Once we know what the exceptions are doing we can figure out a few user stories.

Come up with a plan to improve this.

[Thalhammer]

Theres 130+ exceptions that are from the STL

You're not really supposed to use any of them though except for the 9 in <stdexcept> and std::exception.

[prince-chrismc]

I mean we are using them a lot 🤷 despite the other ways we are also using

[prince-chrismc]

Excluding the traits... there's 5 left

• 2 are from the evp key ref
• 2 are invalid tokens
• last is claim compare in verifier