TimWolla/haproxy-auth-request

Do not ignore 401

schklom opened this issue · 4 comments

Currently; 401 codes are ignored; see below

-- response_ok means 2xx: allow request.
if response_ok then
set_var(txn, "txn.auth_response_successful", true)
-- Don't allow codes < 200 or >= 300.
-- Forward the response to the client if required.
elseif terminate_on_failure then
send_response(txn, response, hdr_fail)
-- Codes with Location: Passthrough location at redirect.
elseif response.status_code == 301 or response.status_code == 302 or response.status_code == 303 or response.status_code == 307 or response.status_code == 308 then
set_var(txn, "txn.auth_response_location", response:get_header("location", "last"))
-- 401 / 403: Do nothing, everything else: log.
elseif response.status_code ~= 401 and response.status_code ~= 403 then
txn:Warning("Invalid status code in auth-request backend '" .. be .. "': " .. response.status_code)
end

The Wikipedia page describes 401 as (https://en.wikipedia.org/wiki/List_of_HTTP_status_codes)

Similar to 403 Forbidden, but specifically for use when authentication is required and has failed or has not yet been provided. The response must include a WWW-Authenticate header field containing a challenge applicable to the requested resource. See Basic access authentication and Digest access authentication. 401 semantically means "unauthorised", the user does not have valid authentication credentials for the target resource.

Authelia seems to follow that definition and returns 401 in order to redirect the user to a login screen. The competing reverse-proxy Traefik handles the 401 by redirecting the user to the login screen with Authelia.

Can this repository be adapted to follow this?
Here is my attempt, which seems to work on my HAProxy, started from the authelia/authelia#5277

	-- response_ok means 2xx: allow request.
	if response_ok then
		set_var(txn, "txn.auth_response_successful", true)
	-- Don't allow codes < 200 or >= 300.
	-- Forward the response to the client if required.
	elseif terminate_on_failure then
		send_response(txn, response, hdr_fail)
	-- Codes with Location: Passthrough location at redirect.
	elseif response.status_code == 301 or response.status_code == 302 or response.status_code == 303 or response.status_code == 307 or response.status_code == 308 or response.status_code ~= 403 then
		set_var(txn, "txn.auth_response_location", response:get_header("location", "last"))
	-- 401: Do nothing, everything else: log.
	elseif response.status_code ~= 401 then
		send_response(txn, response, hdr_fail)
	end
end

Sorry, I don't follow, can you elaborate? HTTP 401 is not ignored, but correctly handled by not setting the auth_response_successful variable and not emitting an error message in the logs.

The proposed patch is definitely incorrect and I do not understand what it's attempting to do.

Im not entirely sure if any change in code is required here or whether you think this should be handled in the haproxy configuration @TimWolla.

If I'm reading the docs right in the case of the auth_request nginx plugin if a 401/403 is received it's passed back to the client.

Which is why in Authelia we utilise the 401 to redirect back to the portal for authentication and if a 403 is received nginx presents a forbidden access denied page.

whether you think this should be handled in the haproxy configuration

This likely needs to be handled in your HAProxy config. You should be able to either:

  1. Direct the request to a different (authentication) backend when encountering the 401:
use_backend my_auth_service if ! { var(txn.auth_response_successful) -m bool } { var(txn.auth_response_code) eq 401 }

or something similar.

  1. Leverage lua.auth-intercept to return the response to the client (see README.md, this was contributed via a PR and is not something I use myself).

@TimWolla thanks for the feedback, I think this can be closed as I've provided feedback to @schklom downstream.