[WIP] OCPBUGS-85705: Add client certificate authentication for HCP metrics

pkg/lib/server/dual_auth_test.go

@@ -0,0 +1,144 @@
+package server
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestMetricsHandler_ClientCertAuthentication tests that requests with
+// client certificates are accepted (HCP use case)
+func TestMetricsHandler_ClientCertAuthentication(t *testing.T) {
+	// Create a test handler that mimics our dual auth logic
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Check for client cert (HCP path)
+		if r.TLS != nil && len(r.TLS.VerifiedChains) > 0 {
+			w.WriteHeader(http.StatusOK)
+			w.Write([]byte("# metrics"))
+			return
+		}
+
+		// No client cert and no bearer token
+		if r.Header.Get("Authorization") == "" {
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
+			return
+		}
+
+		// Bearer token path would validate here
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte("# metrics"))
+	})
+
+	// Test 1: Request WITH client certificate succeeds
+	req := httptest.NewRequest("GET", "/metrics", nil)
+	req.TLS = &tls.ConnectionState{
+		PeerCertificates: []*x509.Certificate{
+			&x509.Certificate{}, // Just need a non-nil cert for the test
+		},
+		VerifiedChains: [][]*x509.Certificate{
+			{&x509.Certificate{}}, // Verified chain indicates cert was validated
+		},
+	}
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code, "Request with client cert should succeed")
+	assert.Contains(t, rec.Body.String(), "metrics", "Should return metrics")
+}
+
+// TestMetricsHandler_BearerTokenPath tests that requests without client certs
+// but with Authorization header go to bearer token validation path
+func TestMetricsHandler_BearerTokenPath(t *testing.T) {
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// No client cert
+		if r.TLS == nil || len(r.TLS.VerifiedChains) == 0 {
+			// Check for bearer token
+			if r.Header.Get("Authorization") != "" {
+				// In real code, this validates the token
+				// For test, just accept it
+				w.WriteHeader(http.StatusOK)
+				w.Write([]byte("# metrics"))
+				return
+			}
+		}
+
+		http.Error(w, "Unauthorized", http.StatusUnauthorized)
+	})
+
+	// Test: Request with Authorization header (no client cert)
+	req := httptest.NewRequest("GET", "/metrics", nil)
+	req.Header.Set("Authorization", "Bearer test-token")
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code, "Request with bearer token should reach validation path")
+}
+
+// TestMetricsHandler_NoAuthentication tests that requests without
+// client cert or bearer token are rejected
+func TestMetricsHandler_NoAuthentication(t *testing.T) {
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// No client cert and no bearer token
+		if (r.TLS == nil || len(r.TLS.VerifiedChains) == 0) && r.Header.Get("Authorization") == "" {
+			http.Error(w, "Unauthorized", http.StatusUnauthorized)
+			return
+		}
+
+		w.WriteHeader(http.StatusOK)
+	})
+
+	// Test: Request without any authentication
+	req := httptest.NewRequest("GET", "/metrics", nil)
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusUnauthorized, rec.Code, "Request without auth should be rejected")
+	assert.Contains(t, rec.Body.String(), "Unauthorized", "Should return unauthorized error")
+}
+
+// TestMetricsHandler_ClientCertTakesPrecedence tests that when both
+// client cert and bearer token are present, client cert is used
+func TestMetricsHandler_ClientCertTakesPrecedence(t *testing.T) {
+	clientCertUsed := false
+	bearerTokenUsed := false
+
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Check client cert first (HCP)
+		if r.TLS != nil && len(r.TLS.VerifiedChains) > 0 {
+			clientCertUsed = true
+			w.WriteHeader(http.StatusOK)
+			return
+		}
+
+		// Check bearer token (standalone OCP)
+		if r.Header.Get("Authorization") != "" {
+			bearerTokenUsed = true
+			w.WriteHeader(http.StatusOK)
+			return
+		}
+
+		http.Error(w, "Unauthorized", http.StatusUnauthorized)
+	})
+
+	// Test: Request with BOTH client cert and bearer token
+	req := httptest.NewRequest("GET", "/metrics", nil)
+	req.TLS = &tls.ConnectionState{
+		PeerCertificates: []*x509.Certificate{
+			&x509.Certificate{}, // Just need a non-nil cert for the test
+		},
+		VerifiedChains: [][]*x509.Certificate{
+			{&x509.Certificate{}}, // Verified chain indicates cert was validated
+		},
+	}
+	req.Header.Set("Authorization", "Bearer test-token")
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	assert.Equal(t, http.StatusOK, rec.Code, "Request should succeed")
+	assert.True(t, clientCertUsed, "Client cert should be checked first")
+	assert.False(t, bearerTokenUsed, "Bearer token should NOT be checked when client cert is present")
+}

pkg/lib/server/server.go

@@ -127,36 +127,61 @@ func (sc serverConfig) getListenAndServeFunc() (func() error, error) {
 
 	// Set up authenticated metrics endpoint if kubeConfig is provided
 	if sc.kubeConfig != nil && tlsEnabled {
-		sc.logger.Info("Setting up authenticated metrics endpoint")
-		// Create HTTP client with proper TLS configuration from kubeConfig
-		// This is necessary for TokenReview/SubjectAccessReview API calls to verify API server certificates
+		sc.logger.Info("Setting up authenticated metrics endpoint with client cert and bearer token support")
+
+		// Create HTTP client and auth filter once for bearer token validation
+		// These are reused across requests to avoid expensive allocations and TLS handshakes
 		httpClient, err := rest.HTTPClientFor(sc.kubeConfig)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create http client for authentication: %w", err)
 		}
-		// Create authentication filter using controller-runtime
+
 		filter, err := filters.WithAuthenticationAndAuthorization(sc.kubeConfig, httpClient)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create authentication filter: %w", err)
 		}
-		// Create authenticated metrics handler
+
 		logger := log.FromContext(context.Background())
-		authenticatedMetricsHandler, err := filter(logger, promhttp.Handler())
+		bearerTokenHandler, err := filter(logger, promhttp.Handler())
 		if err != nil {
 			return nil, fmt.Errorf("failed to wrap metrics handler with authentication: %w", err)
 		}
-		// Add request logging for debugging if debug mode is enabled
-		if sc.debug {
-			debugAuthHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				sc.logger.Infof("Metrics request from %s, Auth header present: %v, User-Agent: %s",
-					r.RemoteAddr, r.Header.Get("Authorization") != "", r.Header.Get("User-Agent"))
-				authenticatedMetricsHandler.ServeHTTP(w, r)
-			})
-			mux.Handle("/metrics", debugAuthHandler)
-		} else {
-			mux.Handle("/metrics", authenticatedMetricsHandler)
-		}
-		sc.logger.Info("Metrics endpoint configured with authentication and authorization")
+
+		// Create metrics handler that supports BOTH:
+		// 1. Bearer token auth (for standalone OCP where Prometheus uses bearer tokens)
+		// 2. Client cert auth (for HCP where Prometheus uses client certificates)
+		//
+		// HCP ServiceMonitors configure client certificates (tlsConfig.cert/keySecret)
+		// while standalone OCP uses bearerTokenFile. The TLS layer (--client-ca)
+		// already verifies client certs; we just need to accept them as authenticated.
+
+		metricsHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// Check if client presented a verified certificate (HCP)
+			if r.TLS != nil && len(r.TLS.VerifiedChains) > 0 {
+				// Client cert was verified by TLS layer via --client-ca
+				// Accept it as authenticated and serve metrics
+				if sc.debug {
+					sc.logger.Infof("Metrics request authenticated via client certificate from %s, CN: %s",
+						r.RemoteAddr, r.TLS.PeerCertificates[0].Subject.CommonName)
+				}
+				promhttp.Handler().ServeHTTP(w, r)
+				return
+			}
+
+			// No client cert - try bearer token auth (standalone OCP)
+			authHeader := r.Header.Get("Authorization")
+			if authHeader == "" {
+				sc.logger.Warnf("Metrics request rejected: no client cert and no bearer token from %s", r.RemoteAddr)
+				http.Error(w, "Unauthorized: client certificate or bearer token required", http.StatusUnauthorized)
+				return
+			}
+
+			// Let controller-runtime's filter handle bearer token validation
+			bearerTokenHandler.ServeHTTP(w, r)
+		})
+
+		mux.Handle("/metrics", metricsHandler)
+		sc.logger.Info("Metrics endpoint configured with dual authentication (client cert + bearer token)")
 	} else {
 		// Fallback to unprotected metrics (for development/testing)
 		mux.Handle("/metrics", promhttp.Handler())