[MISC] Improve documentation around headers used by verify endpoint. (#620)

* Explicit document missing X-Forwarded-Proto and X-Fowarded-Host headers.

* Add the name of the authorization header in error messages.

* Add error and debug logs about X-Original-URL header.

* Add error log when not able to parse target URL in verify endpoint.

* Fix unit tests.

internal/handlers/const.go

@@ -11,7 +11,7 @@ const ResetPasswordAction = "ResetPassword"
 
 const authPrefix = "Basic "
 
-const authorizationHeader = "Proxy-Authorization"
+const AuthorizationHeader = "Proxy-Authorization"
 const remoteUserHeader = "Remote-User"
 const remoteGroupsHeader = "Remote-Groups"
 

internal/handlers/errors.go

@@ -9,4 +9,5 @@ const InternalError = "Internal error."
 // UnauthorizedError is the error message sent when the user is not authorized.
 const UnauthorizedError = "You're not authorized."
 
-var errMissingHeadersForTargetURL = errors.New("Missing headers for detecting target URL")
+var errMissingXForwardedHost = errors.New("Missing header X-Fowarded-Host used to detect target URL")
+var errMissingXForwardedProto = errors.New("Missing header X-Fowarded-Proto used to detect target URL")

internal/handlers/handler_verify.go

@@ -20,31 +20,34 @@ func getOriginalURL(ctx *middlewares.AutheliaCtx) (*url.URL, error) {
 	if originalURL != nil {
 		url, err := url.ParseRequestURI(string(originalURL))
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("Unable to parse URL extracted from X-Original-URL header: %v", err)
 		}
 		return url, nil
+	} else {
+		ctx.Logger.Debug("No X-Original-URL header detected, fallback to combination of " +
+			"X-Fowarded-Proto, X-Forwarded-Host and X-Forwarded-URI headers")
 	}
 
 	forwardedProto := ctx.XForwardedProto()
 	forwardedHost := ctx.XForwardedHost()
 	forwardedURI := ctx.XForwardedURI()
 
-	if forwardedProto == nil || forwardedHost == nil {
-		return nil, errMissingHeadersForTargetURL
+	if forwardedProto == nil {
+		return nil, errMissingXForwardedProto
+	}
+
+	if forwardedHost == nil {
+		return nil, errMissingXForwardedHost
 	}
 
 	var requestURI string
 	scheme := append(forwardedProto, protoHostSeparator...)
-	if forwardedURI == nil {
-		requestURI = string(append(scheme, forwardedHost...))
-	}
-
 	requestURI = string(append(scheme,
 		append(forwardedHost, forwardedURI...)...))
 
-	url, err := url.ParseRequestURI(string(requestURI))
+	url, err := url.ParseRequestURI(requestURI)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Unable to parse URL %s: %v", requestURI, err)
 	}
 	return url, nil
 }
@@ -53,7 +56,7 @@ func getOriginalURL(ctx *middlewares.AutheliaCtx) (*url.URL, error) {
 // "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==" returns ("Aladdin", "open sesame", true).
 func parseBasicAuth(auth string) (username, password string, err error) {
 	if !strings.HasPrefix(auth, authPrefix) {
-		return "", "", fmt.Errorf("%s prefix not found in authorization header", strings.Trim(authPrefix, " "))
+		return "", "", fmt.Errorf("%s prefix not found in %s header", strings.Trim(authPrefix, " "), AuthorizationHeader)
 	}
 	c, err := base64.StdEncoding.DecodeString(auth[len(authPrefix):])
 	if err != nil {
@@ -62,7 +65,7 @@ func parseBasicAuth(auth string) (username, password string, err error) {
 	cs := string(c)
 	s := strings.IndexByte(cs, ':')
 	if s < 0 {
-		return "", "", fmt.Errorf("Format for basic auth must be user:password")
+		return "", "", fmt.Errorf("Format of %s header must be user:password", AuthorizationHeader)
 	}
 	return cs[:s], cs[s+1:], nil
 }
@@ -105,13 +108,13 @@ func verifyBasicAuth(auth []byte, targetURL url.URL, ctx *middlewares.AutheliaCt
 	username, password, err := parseBasicAuth(string(auth))
 
 	if err != nil {
-		return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to parse basic auth: %s", err)
+		return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to parse content of %s header: %s", AuthorizationHeader, err)
 	}
 
 	authenticated, err := ctx.Providers.UserProvider.CheckUserPassword(username, password)
 
 	if err != nil {
-		return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to check password in basic auth mode: %s", err)
+		return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to check credentials extracted from %s header: %s", AuthorizationHeader, err)
 	}
 
 	// If the user is not correctly authenticated, send a 401.
@@ -123,7 +126,7 @@ func verifyBasicAuth(auth []byte, targetURL url.URL, ctx *middlewares.AutheliaCt
 	details, err := ctx.Providers.UserProvider.GetDetails(username)
 
 	if err != nil {
-		return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to retrieve user details in basic auth mode: %s", err)
+		return "", nil, authentication.NotAuthenticated, fmt.Errorf("Unable to retrieve details of user %s: %s", username, err)
 	}
 
 	return username, details.Groups, authentication.OneFactor, nil
@@ -200,7 +203,7 @@ func VerifyGet(ctx *middlewares.AutheliaCtx) {
 	var groups []string
 	var authLevel authentication.Level
 
-	proxyAuthorization := ctx.Request.Header.Peek(authorizationHeader)
+	proxyAuthorization := ctx.Request.Header.Peek(AuthorizationHeader)
 	hasBasicAuth := proxyAuthorization != nil
 
 	if hasBasicAuth {

internal/handlers/handler_verify_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/authelia/authelia/internal/mocks"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -49,7 +50,7 @@ func TestShouldGetOriginalURLFromForwardedHeadersWithURI(t *testing.T) {
 	mock.Ctx.Request.Header.Set("X-Original-URL", "htt-ps//home?-.example.com")
 	_, err := getOriginalURL(mock.Ctx)
 	assert.Error(t, err)
-	assert.Equal(t, "parse htt-ps//home?-.example.com: invalid URI for request", err.Error())
+	assert.Equal(t, "Unable to parse URL extracted from X-Original-URL header: parse htt-ps//home?-.example.com: invalid URI for request", err.Error())
 }
 
 func TestShouldRaiseWhenTargetUrlIsMalformed(t *testing.T) {
@@ -71,14 +72,47 @@ func TestShouldRaiseWhenNoHeaderProvidedToDetectTargetURL(t *testing.T) {
 	defer mock.Close()
 	_, err := getOriginalURL(mock.Ctx)
 	assert.Error(t, err)
-	assert.Equal(t, "Missing headers for detecting target URL", err.Error())
+	assert.Equal(t, "Missing header X-Fowarded-Proto used to detect target URL", err.Error())
+}
+
+func TestShouldRaiseWhenNoXForwardedHostHeaderProvidedToDetectTargetURL(t *testing.T) {
+	mock := mocks.NewMockAutheliaCtx(t)
+	defer mock.Close()
+
+	mock.Ctx.Request.Header.Set("X-Forwarded-Proto", "https")
+	_, err := getOriginalURL(mock.Ctx)
+	assert.Error(t, err)
+	assert.Equal(t, "Missing header X-Fowarded-Host used to detect target URL", err.Error())
+}
+
+func TestShouldRaiseWhenXForwardedProtoIsNotParseable(t *testing.T) {
+	mock := mocks.NewMockAutheliaCtx(t)
+	defer mock.Close()
+
+	mock.Ctx.Request.Header.Set("X-Forwarded-Proto", "!:;;:,")
+	mock.Ctx.Request.Header.Set("X-Forwarded-Host", "myhost.local")
+	_, err := getOriginalURL(mock.Ctx)
+	assert.Error(t, err)
+	assert.Equal(t, "Unable to parse URL !:;;:,://myhost.local: parse !:;;:,://myhost.local: invalid URI for request", err.Error())
+}
+
+func TestShouldRaiseWhenXForwardedURIIsNotParseable(t *testing.T) {
+	mock := mocks.NewMockAutheliaCtx(t)
+	defer mock.Close()
+
+	mock.Ctx.Request.Header.Set("X-Forwarded-Proto", "https")
+	mock.Ctx.Request.Header.Set("X-Forwarded-Host", "myhost.local")
+	mock.Ctx.Request.Header.Set("X-Forwarded-URI", "!:;;:,")
+	_, err := getOriginalURL(mock.Ctx)
+	require.Error(t, err)
+	assert.Equal(t, "Unable to parse URL https://myhost.local!:;;:,: parse https://myhost.local!:;;:,: invalid port \":,\" after host", err.Error())
 }
 
 // Test parseBasicAuth
 func TestShouldRaiseWhenHeaderDoesNotContainBasicPrefix(t *testing.T) {
 	_, _, err := parseBasicAuth("alzefzlfzemjfej==")
 	assert.Error(t, err)
-	assert.Equal(t, "Basic prefix not found in authorization header", err.Error())
+	assert.Equal(t, "Basic prefix not found in Proxy-Authorization header", err.Error())
 }
 
 func TestShouldRaiseWhenCredentialsAreNotInBase64(t *testing.T) {
@@ -91,7 +125,7 @@ func TestShouldRaiseWhenCredentialsAreNotInCorrectForm(t *testing.T) {
 	// the decoded format should be user:password.
 	_, _, err := parseBasicAuth("Basic am9obiBwYXNzd29yZA==")
 	assert.Error(t, err)
-	assert.Equal(t, "Format for basic auth must be user:password", err.Error())
+	assert.Equal(t, "Format of Proxy-Authorization header must be user:password", err.Error())
 }
 
 func TestShouldReturnUsernameAndPassword(t *testing.T) {