labstack
Requests missing the jwt do not result in 401 Unauthorized but instead in 400 Bad Request
In jwt.go:117 a custom error exists for this purpose to return 401
// ErrJWTMissing denotes an error raised when JWT token value could not be extracted from request
var ErrJWTMissing = echo.NewHTTPError(http.StatusUnauthorized, "missing or malformed jwt")
But when trying to extract the jwt instead of returning ErrJWTMissing another new error is created and returned with status 400.
if lastTokenErr == nil {
return echo.NewHTTPError(http.StatusBadRequest, "missing or malformed jwt").SetInternal(err)
}
I think the intention is to do the following instead
if lastTokenErr == nil {
return ErrJWTMissing.SetInternal(err)
}
... not sure about the .SetInternal(err) though
This behavior seems to be against the definition of 401. Or is there a reason for that?
You definitely should not do ErrJWTMissing.SetInternal(err) because ErrJWTMissing is global and you could introduce data race by mutating it with SetInternal. This is because Go standard library HTTP server runs each request in separate goroutine.
have you tried creating your own config.ErrorHandler function and return whatever suits you?
Using the config.ErrorHandler works 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?
Next minor version will revert back to 401. See https://github.com/labstack/echo-jwt/pull/39
I'll tag/release new version on Monday. So those at work will not have Friday surprises and potential weekend incidents.