trusch / caddy-extauth

an external authentication plugin for caddy
MIT License
3 stars 1 forks source link

Support for handling errors #3

Open francislavoie opened 3 years ago

francislavoie commented 3 years ago

An issue I noticed while reviewing the code is that on error, this handler just returns nil

https://github.com/trusch/caddy-extauth/blob/2cb3268137c2e841289fb87acca630c585c32f86/extauth.go#L81-L86

This means that users don't get an opportunity to handle the error case with the handle_errors directive: https://caddyserver.com/docs/caddyfile/directives/handle_errors

The way this should probably work is that you return caddyhttp.Error(http.StatusUnauthorized, fmt.Errorf("not authenticated")) instead, similarly to the caddyauth module:

https://github.com/caddyserver/caddy/blob/653a0d3f6bd7b66197abd1e00e366164876a9f2b/modules/caddyhttp/caddyauth/caddyauth.go#L88

Using caddyhttp.Error gives Caddy an opportunity to handle the error and do something else with it:

https://github.com/caddyserver/caddy/blob/653a0d3f6bd7b66197abd1e00e366164876a9f2b/modules/caddyhttp/server.go#L238

trusch commented 3 years ago

Thanks for your input! I'll adjust and test that.