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

fix: improve jwt errors #3933

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

taimoorzaeem
Copy link
Collaborator

@taimoorzaeem taimoorzaeem commented Mar 5, 2025

BREAKING CHANGE

We were exposing internal errors via Show. This commit improves error messages for better user experience.
Closes #3600, closes #3926.

  • Implementation
  • Tests
  • Docs

I'll add docs once this design/change is approved.

@taimoorzaeem taimoorzaeem marked this pull request as draft March 5, 2025 17:03
@taimoorzaeem taimoorzaeem force-pushed the fix/jwt-error branch 4 times, most recently from 9e3bc64 to 2069fe9 Compare March 10, 2025 08:22
@taimoorzaeem taimoorzaeem marked this pull request as ready for review March 11, 2025 00:55
Comment on lines 74 to 77
hasMin32Chars :: ByteString -> Either Error ByteString
hasMin32Chars token
| BS.length token >= 32 = Right token
| otherwise = Left $ JwtErr $ JwtDecodeError "JWT must be at least 32 characters"
Copy link
Member

@steve-chavez steve-chavez Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't the jwt-secret the one that needed to be at least 32 chars? Not the JWT that's sent per request?

So isn't this unnecessary here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, now that I think of it, it is unnecessary. I just thought it would be a little nice to give this error, I'll remove it.

@taimoorzaeem taimoorzaeem force-pushed the fix/jwt-error branch 2 times, most recently from 77e3adb to 5a1c2a3 Compare March 11, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants