Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExpiredSignature should perhaps not be a subclass of DecodeError #606

Open
joelmichael opened this issue Jul 23, 2024 · 3 comments
Open

Comments

@joelmichael
Copy link

This caused some confusion for us. If a JWT token is expired, this doesn't mean it failed to decode. We were checking for when JWT tokens don't decode correctly due to a formatting error, but this was also catching the case where a (perfectly legitimately encoded) JWT token simply had expired. Thus it affected our error handling for expired tokens unexpectedly.

image

@anakinj
Copy link
Member

anakinj commented Jul 23, 2024

Currently all the errors are subclasses of this DecodeError, so your suggestion (that is perfectly reasonable) applies for all the errors. I agree that some kind of intermediate subclassing logic would make sense.

Think revamping the error hierarchy could be a chore for the next major release, to allow possible breaking changes.

@jkarmel
Copy link

jkarmel commented Jul 24, 2024

Thanks @anakinj! I just wanted to second this - because we use Rails' rescue_from on both DecodeError in general and ExpiredSignature in particular but have different handling for each error. Because ExpiredSignature inherits from DecodeError we now have to order the rescue_from blocks in a specific order which feels pretty brittle.

Appreciate this library, which has been great overall!

@anakinj anakinj added this to the Version 3.0 milestone Sep 29, 2024
@anakinj
Copy link
Member

anakinj commented Sep 29, 2024

Structure to the errors would be great. Some help trying to create some hierarchy appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants