Repository navigation
Requests missing the jwt do not result in 401 Unauthorized but instead in 400 Bad Request #38
Description
Activity
You definitely should not do
ErrJWTMissing.SetInternal(err)becauseErrJWTMissingis global and you could introduce data race by mutating it withSetInternal. This is because Go standard library HTTP server runs each request in separate goroutine.Reacted by Thomashave you tried creating your own
config.ErrorHandlerfunction and return whatever suits you?Using the
config.ErrorHandlerworks of course and is what I did for now.
And you are right about the data race. Was stupid of myself sorry.But my main point is that it seems to me that echo-jwt is not reacting correctly on a missing jwt when looking at the mozilla definition: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/401
Which means instead of this:
if lastTokenErr == nil { return echo.NewHTTPError(http.StatusBadRequest, "missing or malformed jwt").SetInternal(err) } return echo.NewHTTPError(http.StatusUnauthorized, "invalid or expired jwt").SetInternal(err)
We should do this
if lastTokenErr == nil { // changing StatusBadRequest-->StatusUnauthorized return echo.NewHTTPError(http.StatusUnauthorized, "missing or malformed jwt").SetInternal(err) } return echo.NewHTTPError(http.StatusUnauthorized, "invalid or expired jwt").SetInternal(err)
Do you agree (which means I could provide a simple PR if wanted) or is there a reason why it should still return StatusBadRequest and not follow the Mozilla definition?
Great and thanks for the update!
I'll tag/release new version on Monday. So those at work will not have Friday surprises and potential weekend incidents.
In jwt.go:117 a custom error exists for this purpose to return 401
But when trying to extract the jwt instead of returning
ErrJWTMissinganother new error is created and returned with status 400.jwt.go.258
I think the intention is to do the following instead
... not sure about the .SetInternal(err) though
This behavior seems to be against the definition of 401. Or is there a reason for that?