-
Notifications
You must be signed in to change notification settings - Fork 6
Make errors related to access tokens be more specific and actionable #1288
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
Make errors related to access tokens be more specific and actionable #1288
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error handling for JWT access token validation by replacing generic credential error messages with specific, actionable error messages that provide clear guidance to users.
- Introduces granular error handling for different JWT validation failure scenarios
- Adds detailed error messages that distinguish between invalid subjects, claims errors, expired tokens, and invalidated tokens
- Improves logging by replacing print statements with proper logging and adding exception logging
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good. I think there could still be confusion around what "subject" or "claims" means. It could be good to instead say more plainly what subject and claims means to the support staff. Other than that this is a great addition and provides more information! Optional change as I know there are other priorities right now.
|
Thanks, @hesspnnl. I agree with you that these messages present the words "subject" and "claims" without having introduced the reader to what they are. Thanks for bringing this up. I think I am going to revise this PR so that those words aren't shown to the end user, and are only shown in the server-side logs. As far as educating support people about them, I'll add a comment to the code (later, but before merging), containing the URL of a related reference (i.e. https://auth0.com/docs/secure/tokens/json-web-tokens/json-web-token-claims, which is about "claims", one of which is the "subject" claim). |
|
Tests are passing locally ( |
|
The same test is currently failing on the |
On this branch, I updated the function that processes incoming JWTs so that the user-facing error messages it displays are more specific to the situation and more actionable.
Details
Previously, the error message in all scenarios was the same:
Now, there are several error messages, all actionable, and some include additional details that could help "support staff" distinguish bug reports from one another.
For example:
I consider this to be an improvement over what was there before. For the longer term: I think we (i.e. someone) will revisit this code when we overhaul the auth system.
Related issue(s)
Fixes #1267
Related subsystem(s)
docsdirectory)The authentication flow.
Testing
I tested these changes by confirming all tests pass.
Documentation
docsdirectory)Maintainability
study_id: str)# TODOor# FIXMEblackto format all the Python files I created/modified