From e44b4f5b1e02ba47a36d606425aebbc0570e8dd3 Mon Sep 17 00:00:00 2001
From: Alexei Dodon <adodon@cisco.com>
Date: Wed, 27 Sep 2023 23:48:15 +0300
Subject: [PATCH] fix: metrics endpoint must be secured behind authN

Signed-off-by: Alexei Dodon <adodon@cisco.com>
---
 examples/config-policy.json                  |  2 +-
 pkg/api/controller_test.go                   | 20 +++----
 pkg/api/errors/errors.go                     |  4 +-
 pkg/extensions/extension_metrics_disabled.go |  1 +
 pkg/extensions/extensions_test.go            |  6 +-
 test/blackbox/metrics.bats                   | 62 +++++++++++++++++++-
 test/blackbox/pushpull_authn.bats            |  2 +-
 7 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/examples/config-policy.json b/examples/config-policy.json
index eaf45e28a7..231abbdc0c 100644
--- a/examples/config-policy.json
+++ b/examples/config-policy.json
@@ -60,7 +60,7 @@
             }
           ],
           "defaultPolicy": ["read"]
-        }  
+        }
       },
       "adminPolicy": {
         "users": ["admin"],
diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go
index 41f395dbfa..a0e06319d0 100644
--- a/pkg/api/controller_test.go
+++ b/pkg/api/controller_test.go
@@ -2195,7 +2195,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2225,7 +2225,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2254,7 +2254,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2283,7 +2283,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2307,7 +2307,7 @@ func TestBearerAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2392,7 +2392,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2416,7 +2416,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2445,7 +2445,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2474,7 +2474,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -2498,7 +2498,7 @@ func TestBearerAuthWithAllowReadAccess(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go
index cfc4adb718..29b7d6f7a1 100644
--- a/pkg/api/errors/errors.go
+++ b/pkg/api/errors/errors.go
@@ -136,8 +136,8 @@ func NewError(code ErrorCode) *Error {
 
 		UNAUTHORIZED: {
 			Message: "authentication required",
-			Description: "The access controller was unable to authenticate the client." +
-				"Often this will be accompanied by a Www-Authenticate HTTP response header " +
+			Description: "The access controller was unable to authenticate the client. " +
+				"Often this will be accompanied by a WWW-Authenticate HTTP response header " +
 				"indicating how to authenticate.",
 		},
 
diff --git a/pkg/extensions/extension_metrics_disabled.go b/pkg/extensions/extension_metrics_disabled.go
index 41cdccb8d5..6d280b9ba6 100644
--- a/pkg/extensions/extension_metrics_disabled.go
+++ b/pkg/extensions/extension_metrics_disabled.go
@@ -29,5 +29,6 @@ func SetupMetricsRoutes(conf *config.Config, router *mux.Router,
 		zcommon.WriteJSON(w, http.StatusOK, m)
 	}
 
+	router.Use(authFunc)
 	router.HandleFunc("/metrics", getMetrics).Methods("GET")
 }
diff --git a/pkg/extensions/extensions_test.go b/pkg/extensions/extensions_test.go
index 04034b72c3..9f10636751 100644
--- a/pkg/extensions/extensions_test.go
+++ b/pkg/extensions/extensions_test.go
@@ -799,7 +799,7 @@ func TestMgmtWithBearer(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -829,7 +829,7 @@ func TestMgmtWithBearer(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
@@ -853,7 +853,7 @@ func TestMgmtWithBearer(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
 
-		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("Www-Authenticate"))
+		authorizationHeader = authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate"))
 		resp, err = resty.R().
 			SetQueryParam("service", authorizationHeader.Service).
 			SetQueryParam("scope", authorizationHeader.Scope).
diff --git a/test/blackbox/metrics.bats b/test/blackbox/metrics.bats
index df7825d014..2b6346c455 100644
--- a/test/blackbox/metrics.bats
+++ b/test/blackbox/metrics.bats
@@ -27,7 +27,14 @@ function setup_file() {
     echo ${zot_root_dir}
     zot_log_file=${zot_root_dir}/zot-log.json
     zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json
+    zot_htpasswd_file=${BATS_FILE_TMPDIR}/zot_htpasswd
+    htpasswd -Bbn test test123 >> ${zot_htpasswd_file}
+
+    zot_minimal_root_dir=${BATS_FILE_TMPDIR}/zot-minimal
+    zot_minimal_config_file=${BATS_FILE_TMPDIR}/zot_minimal_config.json
+
     mkdir -p ${zot_root_dir}
+    mkdir -p ${zot_minimal_root_dir}
     touch ${zot_log_file}
     cat >${zot_config_file} <<EOF
 {
@@ -37,7 +44,12 @@ function setup_file() {
     },
     "http": {
         "address": "0.0.0.0",
-        "port": "8080"
+        "port": "8080",
+        "auth": {
+            "htpasswd": {
+                "path": "${zot_htpasswd_file}"
+            }
+        }
     },
     "log": {
         "level": "debug",
@@ -52,11 +64,35 @@ function setup_file() {
         }
     }
 }
+EOF
+
+    cat >${zot_minimal_config_file} <<EOF
+{
+    "distSpecVersion": "1.1.0-dev",
+    "storage": {
+        "rootDirectory": "${zot_minimal_root_dir}"
+    },
+    "http": {
+        "address": "0.0.0.0",
+        "port": "9000",
+        "auth": {
+            "htpasswd": {
+                "path": "${zot_htpasswd_file}"
+            }
+        }
+    },
+    "log": {
+        "level": "debug"
+    }
+}
 EOF
 
     zot_serve ${ZOT_PATH} ${zot_config_file}
     wait_zot_reachable 8080
 
+    zot_serve ${ZOT_MINIMAL_PATH} ${zot_minimal_config_file}
+    wait_zot_reachable 9000
+
 }
 
 function teardown() {
@@ -68,8 +104,30 @@ function teardown_file() {
     zot_stop_all
 }
 
-@test "metric enabled" {
+@test "unauthorized request to metrics" {
+    local servername="http://127.0.0.1:8080/metrics"
+    status_code=$(curl --write-out '%{http_code}' --silent --output /dev/null ${servername})
+
+    [ "$status_code" -eq 401 ]
+}
+
+@test "authorized request: metrics enabled" {
     local servername="http://127.0.0.1:8080/metrics"
+    status_code=$(curl --write-out '%{http_code}' -u test:test123 --silent --output /dev/null ${servername})
+
+    [ "$status_code" -eq 200 ]
+}
+
+@test "zot-minimal: unauthorized request to metrics" {
+    local servername="http://127.0.0.1:9000/metrics"
     status_code=$(curl --write-out '%{http_code}' --silent --output /dev/null ${servername})
+
+    [ "$status_code" -eq 401 ]
+}
+
+@test "zot-minimal authorized request: metrics enabled" {
+    local servername="http://127.0.0.1:9000/metrics"
+    status_code=$(curl --write-out '%{http_code}' -u test:test123 --silent --output /dev/null ${servername})
+
     [ "$status_code" -eq 200 ]
 }
diff --git a/test/blackbox/pushpull_authn.bats b/test/blackbox/pushpull_authn.bats
index 93d3647715..ca2fb2fb29 100644
--- a/test/blackbox/pushpull_authn.bats
+++ b/test/blackbox/pushpull_authn.bats
@@ -30,7 +30,7 @@ function setup_file() {
     local zot_config_file=${BATS_FILE_TMPDIR}/zot_config.json
     local zot_htpasswd_file=${BATS_FILE_TMPDIR}/zot_htpasswd
     htpasswd -Bbn test test123 >> ${zot_htpasswd_file}
-    
+
     echo ${zot_root_dir} >&3
 
     mkdir -p ${zot_root_dir}