-
Notifications
You must be signed in to change notification settings - Fork 15
Return HTTP status 400 if missing JWT #13
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
Conversation
|
Hi @aldas I am sorry, just a kind reminder to review in case you have missed this, thank you! |
aldas
left a comment
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.
LGTM
Ref. labstack/echo-jwt#13 Signed-off-by: Andreas Gerstmayr <[email protected]>
* Bump github.com/labstack/echo-jwt/v4 from 4.2.0 to 4.3.0 Bumps [github.com/labstack/echo-jwt/v4](https://github.com/labstack/echo-jwt) from 4.2.0 to 4.3.0. - [Release notes](https://github.com/labstack/echo-jwt/releases) - [Changelog](https://github.com/labstack/echo-jwt/blob/main/CHANGELOG.md) - [Commits](labstack/echo-jwt@v4.2.0...v4.3.0) --- updated-dependencies: - dependency-name: github.com/labstack/echo-jwt/v4 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * update expected status code Ref. labstack/echo-jwt#13 Signed-off-by: Andreas Gerstmayr <[email protected]> --------- Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Andreas Gerstmayr <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andreas Gerstmayr <[email protected]>
|
Hi, just wondering - why was the status code changed? I would heavily expect a 401, even more so after considering https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status#client_error_responses. The issue with 400 is: 400 is commonly used to signal that the response (body) itself is malformed. I would not expect a token related issue in a 400, but only in a 401 or 403. |
|
to be honest I do not remember actual details but I would assume looking at the error messages - the reasoning was that for some cases ('header' for example) it would distinguish cases when you have incorrectly composed the requests (did not add jwt or added somewhere where middleware can not extract) from cases when you are sending unparseable/verifiable JWTs that failed cryptographic authentication. |
Fixes
echo-jwt/jwt.go
Lines 159 to 161 in 6944ffe
https://echo.labstack.com/docs/middleware/jwt
Should return HTTP status 400 if missing JWT as documented.