search

BTBurke / caddy-jwt

JWT middleware for the Caddy server
MIT License
114 stars 39 forks source link

Status code and WWW-Authenticate response header in case of failed token validation #17

Closed lucafavatella closed 7 years ago

lucafavatella commented 7 years ago

The relevant RFC section details status code and WWW-Authenticate response header in case of token validation failure. I observed Caddy does not follow that and always return 401. Is this by design on does caddy-jwt aim at improving at handling validation failure following RFC?

I am passing the JWT by Authorization header with Bearer scheme. I know the JWT may be passed by any one of several approaches but I am focusing on the Authorization header as it is the one for which I analyzed the IETF RFCs.

I am using Caddy 0.9.5 with JWT plugin as downloaded from https://caddyserver.com/download/builds/173003033949962/caddy_linux_amd64_custom.tar.gz

BTBurke commented 7 years ago

Yeah you're right it doesn't respond with the correct headers per the RFC. Adding the WWW-Authenticate header would be fairly simple, however I'm not sure that plugins have enough context to determine the realm defined in the Caddyfile. I'll have to research that.

It should probably respond 403 when a token is otherwise valid but access is denied based on an allow or deny rule.

I'll look into whether it can respond with appropriate realm information.

lucafavatella commented 7 years ago

Could you please share your script for testing e.g. CI? It would help contributing.

BTBurke commented 7 years ago

Just run go test and it will run all the existing tests. On Wednesday, April 5, 2017 at 9:04 AM Luca Favatella notifications@github.com wrote:

Could you please share your script for testing e.g. CI? It would help contributing.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

lucafavatella commented 7 years ago

Just run go test and it will run all the existing tests.

I made a proposal of CI in #19.

lucafavatella commented 7 years ago

Is check at https://github.com/BTBurke/caddy-jwt/blob/v2.0.4/jwt.go#L59 in scope for this? It is not clear to me whether that case can ever happen, and if yes it smells more of 500 than of 401/403.

lucafavatella commented 7 years ago

It should probably respond 403 when a token is otherwise valid but access is denied based on an allow or deny rule.

Done in #20.

Yeah you're right it doesn't respond with the correct headers per the RFC. Adding the WWW-Authenticate header would be fairly simple, however I'm not sure that plugins have enough context to determine the realm defined in the Caddyfile. I'll have to research that. ... I'll look into whether it can respond with appropriate realm information.

I am not looking into WWW-Authenticate.

BTBurke commented 7 years ago

Was looking into this more and realized that the RFC you linked to is for OAuth 2.0 which is completely different than JWT. Although OAuth can use JWTs, it's scope is much wider than what this library is going for.

That being said, I did implement the WWW-Authenticate header that will return a realm and a simplified status message.