From 63908266180192a5293289eb0363eea1b4d5cfd2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Michaud?= <clement.michaud34@gmail.com>
Date: Tue, 18 Feb 2020 23:39:07 +0100
Subject: [PATCH] [MISC] Add several logs to help users detect misconfiguration
 issues (#639)

* Help users detect misconfiguration of their protected domain.

Sometimes users try to visit an URL pointing to a domain which is
not protected by Authelia and thus authentication fails. This log
line will help users detect those cases.

* Add a log to detect bad schemes in target URLs.

This helps users detect when an URL is http while it should be https.
Indeed, cookies are transported solely over a secure connection for
security reasons.
---
 internal/handlers/handler_verify.go      | 22 +++++++++++++++
 internal/handlers/handler_verify_test.go | 36 ++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/internal/handlers/handler_verify.go b/internal/handlers/handler_verify.go
index 991191d6b..05c1312ab 100644
--- a/internal/handlers/handler_verify.go
+++ b/internal/handlers/handler_verify.go
@@ -14,6 +14,14 @@ import (
 	"github.com/valyala/fasthttp"
 )
 
+func isURLUnderProtectedDomain(url *url.URL, domain string) bool {
+	return strings.HasSuffix(url.Hostname(), domain)
+}
+
+func isSchemeHTTPS(url *url.URL) bool {
+	return url.Scheme == "https"
+}
+
 // getOriginalURL extract the URL from the request headers (X-Original-URI or X-Forwarded-* headers).
 func getOriginalURL(ctx *middlewares.AutheliaCtx) (*url.URL, error) {
 	originalURL := ctx.XOriginalURL()
@@ -199,6 +207,20 @@ func VerifyGet(ctx *middlewares.AutheliaCtx) {
 		return
 	}
 
+	if !isSchemeHTTPS(targetURL) {
+		ctx.Logger.Error(fmt.Errorf("Scheme of target URL %s must be 'https' since cookies are "+
+			"only transported over a secure connection for security reasons", targetURL.String()))
+		ctx.ReplyUnauthorized()
+		return
+	}
+
+	if !isURLUnderProtectedDomain(targetURL, ctx.Configuration.Session.Domain) {
+		ctx.Logger.Error(fmt.Errorf("The target URL %s is not under the protected domain %s",
+			targetURL.String(), ctx.Configuration.Session.Domain))
+		ctx.ReplyUnauthorized()
+		return
+	}
+
 	var username string
 	var groups []string
 	var authLevel authentication.Level
diff --git a/internal/handlers/handler_verify_test.go b/internal/handlers/handler_verify_test.go
index 3d435f238..4eaaaa5c0 100644
--- a/internal/handlers/handler_verify_test.go
+++ b/internal/handlers/handler_verify_test.go
@@ -556,3 +556,39 @@ func TestShouldURLEncodeRedirectionURLParameter(t *testing.T) {
 	assert.Equal(t, "Found. Redirecting to https://auth.mydomain.com?rd=https%3A%2F%2Ftwo-factor.example.com",
 		string(mock.Ctx.Response.Body()))
 }
+
+func TestIsDomainProtected(t *testing.T) {
+	GetURL := func(u string) *url.URL {
+		x, err := url.ParseRequestURI(u)
+		require.NoError(t, err)
+		return x
+	}
+
+	assert.True(t, isURLUnderProtectedDomain(
+		GetURL("http://mytest.example.com/abc/?query=abc"), "example.com"))
+
+	assert.True(t, isURLUnderProtectedDomain(
+		GetURL("http://example.com/abc/?query=abc"), "example.com"))
+
+	assert.True(t, isURLUnderProtectedDomain(
+		GetURL("https://mytest.example.com/abc/?query=abc"), "example.com"))
+
+	// cookies readable by a service on a machine is also readable by a service on the same machine
+	// with a different port as mentioned in https://tools.ietf.org/html/rfc6265#section-8.5
+	assert.True(t, isURLUnderProtectedDomain(
+		GetURL("https://mytest.example.com:8080/abc/?query=abc"), "example.com"))
+}
+
+func TestSchemeIsHTTPS(t *testing.T) {
+	GetURL := func(u string) *url.URL {
+		x, err := url.ParseRequestURI(u)
+		require.NoError(t, err)
+		return x
+	}
+
+	assert.False(t, isSchemeHTTPS(
+		GetURL("http://mytest.example.com/abc/?query=abc")))
+	assert.True(t, isSchemeHTTPS(
+		GetURL("https://mytest.example.com/abc/?query=abc")))
+
+}