Skip to content
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

Support for handling errors #3

Open
francislavoie opened this issue Feb 9, 2021 · 1 comment
Open

Support for handling errors #3

francislavoie opened this issue Feb 9, 2021 · 1 comment

Comments

@francislavoie
Copy link

francislavoie commented Feb 9, 2021

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

caddy-extauth/extauth.go

Lines 81 to 86 in 2cb3268

if err != nil || resp.StatusCode != http.StatusOK {
// something went wrong or the server responded with something != 200 -> reject request with 401
log.Error().Str("url", r.URL.RequestURI()).Msg("failed to authenticate")
w.WriteHeader(http.StatusUnauthorized)
return nil
}

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
Copy link
Owner

trusch commented Feb 9, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants