From f58b84b3712bc00d183dcc5433a383680f6fa39e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= <l.mierzwa@gmail.com>
Date: Wed, 25 Nov 2020 11:03:19 +0000
Subject: [PATCH] fix(backend): don't require auth for /health and /metrics

Fixes #2465
---
 CHANGELOG.md            |  6 ++++++
 cmd/karma/auth.go       | 14 ++++++++++++--
 cmd/karma/main.go       |  8 ++++++--
 cmd/karma/views_test.go | 14 +++++++++++++-
 4 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4630d6015..562e1723a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed auth bypass for `/health` and `/metrics` endpoints.
+  Those endpoints should be always excluded from authentication but that was
+  broken in `v0.73` #2465.
+
 ### Added
 
 - `listen:tls:cert` and `listen:tls:key` config options for listening on HTTPS
diff --git a/cmd/karma/auth.go b/cmd/karma/auth.go
index 8b4802c0a..18190e6f8 100644
--- a/cmd/karma/auth.go
+++ b/cmd/karma/auth.go
@@ -21,9 +21,14 @@ func userGroups(username string) []string {
 	return groups
 }
 
-func headerAuth(name, valueRegex string) func(next http.Handler) http.Handler {
+func headerAuth(name, valueRegex string, allowBypass []string) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if slices.StringInSlice(allowBypass, r.URL.Path) {
+				next.ServeHTTP(w, r)
+				return
+			}
+
 			user := r.Header.Get(name)
 			if user == "" {
 				w.WriteHeader(http.StatusUnauthorized)
@@ -53,9 +58,14 @@ func getUserFromContext(r *http.Request) string {
 	return username.(string)
 }
 
-func basicAuth(creds map[string]string) func(next http.Handler) http.Handler {
+func basicAuth(creds map[string]string, allowBypass []string) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if slices.StringInSlice(allowBypass, r.URL.Path) {
+				next.ServeHTTP(w, r)
+				return
+			}
+
 			user, pass, ok := r.BasicAuth()
 			if !ok {
 				basicAuthFailed(w)
diff --git a/cmd/karma/main.go b/cmd/karma/main.go
index e735a09e0..3c3cdc2c7 100644
--- a/cmd/karma/main.go
+++ b/cmd/karma/main.go
@@ -110,16 +110,20 @@ func setupRouter(router *chi.Mux) {
 		MaxAge:           300,
 	}))
 
+	allowAuthBypass := []string{
+		getViewURL("/health"),
+		getViewURL("/metrics"),
+	}
 	if config.Config.Authentication.Header.Name != "" {
 		config.Config.Authentication.Enabled = true
-		router.Use(headerAuth(config.Config.Authentication.Header.Name, config.Config.Authentication.Header.ValueRegex))
+		router.Use(headerAuth(config.Config.Authentication.Header.Name, config.Config.Authentication.Header.ValueRegex, allowAuthBypass))
 	} else if len(config.Config.Authentication.BasicAuth.Users) > 0 {
 		config.Config.Authentication.Enabled = true
 		users := map[string]string{}
 		for _, u := range config.Config.Authentication.BasicAuth.Users {
 			users[u.Username] = u.Password
 		}
-		router.Use(basicAuth(users))
+		router.Use(basicAuth(users, allowAuthBypass))
 	}
 
 	router.Get(getViewURL("/"), index)
diff --git a/cmd/karma/views_test.go b/cmd/karma/views_test.go
index 730ed840b..6629e0941 100644
--- a/cmd/karma/views_test.go
+++ b/cmd/karma/views_test.go
@@ -1002,6 +1002,10 @@ func TestAuthentication(t *testing.T) {
 				"/silences.json",
 				"/custom.css",
 				"/custom.js",
+				"/health",
+				"/health?foo",
+				"/metrics",
+				"/metrics?bar=foo",
 			} {
 				req := httptest.NewRequest("GET", path, nil)
 				for k, v := range testCase.requestHeaders {
@@ -1010,6 +1014,14 @@ func TestAuthentication(t *testing.T) {
 				req.SetBasicAuth(testCase.requestBasicAuthUser, testCase.requestBasicAuthPassword)
 				resp := httptest.NewRecorder()
 				r.ServeHTTP(resp, req)
+
+				if strings.HasPrefix(path, "/health") || strings.HasPrefix(path, "/metrics") {
+					if resp.Code != 200 {
+						t.Errorf("%s should always return 200, got %d", path, resp.Code)
+					}
+					continue
+				}
+
 				if resp.Code != testCase.responseCode {
 					t.Errorf("Expected %d from %s, got %d", testCase.responseCode, path, resp.Code)
 				}
@@ -2242,7 +2254,7 @@ func TestUpstreamStatus(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.Name, func(t *testing.T) {
-			zerolog.SetGlobalLevel(zerolog.DebugLevel)
+			zerolog.SetGlobalLevel(zerolog.FatalLevel)
 
 			httpmock.Activate()
 			defer httpmock.DeactivateAndReset()