feat(auth): configurable Kubernetes API token validation (#252)

Signed-off-by: Marc Nuri <marc@marcnuri.com>
2025-10-17 14:27:48 +03:00 · 2025-08-08 10:23:12 +03:00
parent 7b11c1667a
commit dfcecd5089
5 changed files with 101 additions and 83 deletions
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -23,8 +23,12 @@ type StaticConfig struct {
 	EnabledTools       []string `toml:"enabled_tools,omitempty"`
 	DisabledTools      []string `toml:"disabled_tools,omitempty"`
 	RequireOAuth       bool     `toml:"require_oauth,omitempty"`
+
+	//Authorization related fields
 	// OAuthAudience is the valid audience for the OAuth tokens, used for offline JWT claim validation.
 	OAuthAudience string `toml:"oauth_audience,omitempty"`
+	// ValidateToken indicates whether the server should validate the token against the Kubernetes API Server using TokenReview.
+	ValidateToken bool `toml:"validate_token,omitempty"`
 	// AuthorizationURL is the URL of the OIDC authorization server.
 	// It is used for token validation and for STS token exchange.
 	AuthorizationURL     string `toml:"authorization_url,omitempty"`
--- a/pkg/http/authorization.go
+++ b/pkg/http/authorization.go
@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"strings"

+	"github.com/containers/kubernetes-mcp-server/pkg/mcp"
 	"github.com/coreos/go-oidc/v3/oidc"
 	"github.com/go-jose/go-jose/v4"
 	"github.com/go-jose/go-jose/v4/jwt"
@@ -13,11 +14,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/utils/strings/slices"

-	"github.com/containers/kubernetes-mcp-server/pkg/mcp"
-)
-
-const (
-	Audience = "mcp-server"
+	"github.com/containers/kubernetes-mcp-server/pkg/config"
 )

 type KubernetesApiTokenVerifier interface {
@@ -42,30 +39,31 @@ type KubernetesApiTokenVerifier interface {
 //
 //	    2.1. Raw Token Validation (oidcProvider is nil):
 //	         - The token is validated offline for basic sanity checks (expiration).
-//	         - If audience is set, the token is validated against the audience.
-//	         - The token is then used against the Kubernetes API Server for TokenReview.
+//	         - If OAuthAudience is set, the token is validated against the audience.
+//	         - If ValidateToken is set, the token is then used against the Kubernetes API Server for TokenReview.
 //
 //	    2.2. OIDC Provider Validation (oidcProvider is not nil):
 //	         - The token is validated offline for basic sanity checks (audience and expiration).
+//	         - If OAuthAudience is set, the token is validated against the audience.
 //	         - The token is then validated against the OIDC Provider.
-//	         - The token is then used against the Kubernetes API Server for TokenReview.
+//	         - If ValidateToken is set, the token is then used against the Kubernetes API Server for TokenReview.
 //
 //	    2.3. OIDC Token Exchange (oidcProvider is not nil and xxx):
-func AuthorizationMiddleware(requireOAuth bool, audience string, oidcProvider *oidc.Provider, verifier KubernetesApiTokenVerifier) func(http.Handler) http.Handler {
+func AuthorizationMiddleware(staticConfig *config.StaticConfig, oidcProvider *oidc.Provider, verifier KubernetesApiTokenVerifier) func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			if r.URL.Path == healthEndpoint || slices.Contains(WellKnownEndpoints, r.URL.EscapedPath()) {
 				next.ServeHTTP(w, r)
 				return
 			}
-			if !requireOAuth {
+			if !staticConfig.RequireOAuth {
 				next.ServeHTTP(w, r)
 				return
 			}

 			wwwAuthenticateHeader := "Bearer realm=\"Kubernetes MCP Server\""
-			if audience != "" {
-				wwwAuthenticateHeader += fmt.Sprintf(`, audience="%s"`, audience)
+			if staticConfig.OAuthAudience != "" {
+				wwwAuthenticateHeader += fmt.Sprintf(`, audience="%s"`, staticConfig.OAuthAudience)
 			}

 			authHeader := r.Header.Get("Authorization")
@@ -80,11 +78,27 @@ func AuthorizationMiddleware(requireOAuth bool, audience string, oidcProvider *o
 			token := strings.TrimPrefix(authHeader, "Bearer ")

 			claims, err := ParseJWTClaims(token)
-			if err == nil && claims != nil {
-				err = claims.ValidateOffline(audience)
+			if err == nil && claims == nil {
+				// Impossible case, but just in case
+				err = fmt.Errorf("failed to parse JWT claims from token")
 			}
-			if err == nil && claims != nil {
-				err = claims.ValidateWithProvider(r.Context(), audience, oidcProvider)
+			// Offline validation
+			if err == nil {
+				err = claims.ValidateOffline(staticConfig.OAuthAudience)
+			}
+			// Online OIDC provider validation
+			if err == nil {
+				err = claims.ValidateWithProvider(r.Context(), staticConfig.OAuthAudience, oidcProvider)
+			}
+			// Scopes propagation, they are likely to be used for authorization.
+			if err == nil {
+				scopes := claims.GetScopes()
+				klog.V(2).Infof("JWT token validated - Scopes: %v", scopes)
+				r = r.WithContext(context.WithValue(r.Context(), mcp.TokenScopesContextKey, scopes))
+			}
+			// Kubernetes API Server TokenReview validation
+			if err == nil && staticConfig.ValidateToken {
+				err = claims.ValidateWithKubernetesApi(r.Context(), staticConfig.OAuthAudience, verifier)
 			}
 			if err != nil {
 				klog.V(1).Infof("Authentication failed - JWT validation error: %s %s from %s, error: %v", r.Method, r.URL.Path, r.RemoteAddr, err)
@@ -94,32 +108,6 @@ func AuthorizationMiddleware(requireOAuth bool, audience string, oidcProvider *o
 				return
 			}

-			// Scopes are likely to be used for authorization.
-			scopes := claims.GetScopes()
-			klog.V(2).Infof("JWT token validated - Scopes: %v", scopes)
-			r = r.WithContext(context.WithValue(r.Context(), mcp.TokenScopesContextKey, scopes))
-
-			// Now, there are a couple of options:
-			// 1. If there is no authorization url configured for this MCP Server,
-			// that means this token will be used against the Kubernetes API Server.
-			// So that we need to validate the token using Kubernetes TokenReview API beforehand.
-			// 2. If there is an authorization url configured for this MCP Server,
-			// that means up to this point, the token is validated against the OIDC Provider already.
-			// 2. a. If this is the only token in the headers, this validated token
-			// is supposed to be used against the Kubernetes API Server as well. Therefore,
-			// TokenReview request must succeed.
-			// 2. b. If this is not the only token in the headers, the token in here is used
-			// only for authentication and authorization. Therefore, we need to send TokenReview request
-			// with the other token in the headers (TODO: still need to validate aud and exp of this token separately).
-			_, _, err = verifier.KubernetesApiVerifyToken(r.Context(), token, audience)
-			if err != nil {
-				klog.V(1).Infof("Authentication failed - API Server token validation error: %s %s from %s, error: %v", r.Method, r.URL.Path, r.RemoteAddr, err)
-
-				w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Bearer realm="Kubernetes MCP Server", audience="%s", error="invalid_token"`, audience))
-				http.Error(w, "Unauthorized: Invalid token", http.StatusUnauthorized)
-				return
-			}
-
 			next.ServeHTTP(w, r)
 		})
 	}
@@ -180,6 +168,16 @@ func (c *JWTClaims) ValidateWithProvider(ctx context.Context, audience string, p
 	return nil
 }

+func (c *JWTClaims) ValidateWithKubernetesApi(ctx context.Context, audience string, verifier KubernetesApiTokenVerifier) error {
+	if verifier != nil {
+		_, _, err := verifier.KubernetesApiVerifyToken(ctx, c.Token, audience)
+		if err != nil {
+			return fmt.Errorf("kubernetes API token validation error: %v", err)
+		}
+	}
+	return nil
+}
+
 func ParseJWTClaims(token string) (*JWTClaims, error) {
 	tkn, err := jwt.ParseSigned(token, allSignatureAlgorithms)
 	if err != nil {
--- a/pkg/http/http.go
+++ b/pkg/http/http.go
@@ -28,7 +28,7 @@ func Serve(ctx context.Context, mcpServer *mcp.Server, staticConfig *config.Stat
 	mux := http.NewServeMux()

 	wrappedMux := RequestMiddleware(
-		AuthorizationMiddleware(staticConfig.RequireOAuth, staticConfig.OAuthAudience, oidcProvider, mcpServer)(mux),
+		AuthorizationMiddleware(staticConfig, oidcProvider, mcpServer)(mux),
 	)

 	httpServer := &http.Server{
--- a/pkg/http/http_test.go
+++ b/pkg/http/http_test.go
@@ -284,7 +284,7 @@ func TestHealthCheck(t *testing.T) {
 		})
 	})
 	// Health exposed even when require Authorization
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, ValidateToken: true}}, func(ctx *httpContext) {
 		resp, err := http.Get(fmt.Sprintf("http://%s/healthz", ctx.HttpAddress))
 		if err != nil {
 			t.Fatalf("Failed to get health check endpoint with OAuth: %v", err)
@@ -305,7 +305,7 @@ func TestWellKnownReverseProxy(t *testing.T) {
 		".well-known/openid-configuration",
 	}
 	// With No Authorization URL configured
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, ValidateToken: true}}, func(ctx *httpContext) {
 		for _, path := range cases {
 			resp, err := http.Get(fmt.Sprintf("http://%s/%s", ctx.HttpAddress, path))
 			t.Cleanup(func() { _ = resp.Body.Close() })
@@ -329,7 +329,7 @@ func TestWellKnownReverseProxy(t *testing.T) {
 		_, _ = w.Write([]byte(`{"issuer": "https://example.com","scopes_supported":["mcp-server"]}`))
 	}))
 	t.Cleanup(testServer.Close)
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{AuthorizationURL: testServer.URL, RequireOAuth: true}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{AuthorizationURL: testServer.URL, RequireOAuth: true, ValidateToken: true}}, func(ctx *httpContext) {
 		for _, path := range cases {
 			resp, err := http.Get(fmt.Sprintf("http://%s/%s", ctx.HttpAddress, path))
 			t.Cleanup(func() { _ = resp.Body.Close() })
@@ -377,7 +377,7 @@ func TestMiddlewareLogging(t *testing.T) {

 func TestAuthorizationUnauthorized(t *testing.T) {
 	// Missing Authorization header
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, ValidateToken: true}}, func(ctx *httpContext) {
 		resp, err := http.Get(fmt.Sprintf("http://%s/mcp", ctx.HttpAddress))
 		if err != nil {
 			t.Fatalf("Failed to get protected endpoint: %v", err)
@@ -402,7 +402,7 @@ func TestAuthorizationUnauthorized(t *testing.T) {
 		})
 	})
 	// Authorization header without Bearer prefix
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, ValidateToken: true}}, func(ctx *httpContext) {
 		req, err := http.NewRequest("GET", fmt.Sprintf("http://%s/mcp", ctx.HttpAddress), nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
@@ -427,7 +427,7 @@ func TestAuthorizationUnauthorized(t *testing.T) {
 		})
 	})
 	// Invalid Authorization header
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, ValidateToken: true}}, func(ctx *httpContext) {
 		req, err := http.NewRequest("GET", fmt.Sprintf("http://%s/mcp", ctx.HttpAddress), nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
@@ -458,7 +458,7 @@ func TestAuthorizationUnauthorized(t *testing.T) {
 		})
 	})
 	// Expired Authorization Bearer token
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, ValidateToken: true}}, func(ctx *httpContext) {
 		req, err := http.NewRequest("GET", fmt.Sprintf("http://%s/mcp", ctx.HttpAddress), nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
@@ -489,7 +489,7 @@ func TestAuthorizationUnauthorized(t *testing.T) {
 		})
 	})
 	// Invalid audience claim Bearer token
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "expected-audience"}}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "expected-audience", ValidateToken: true}}, func(ctx *httpContext) {
 		req, err := http.NewRequest("GET", fmt.Sprintf("http://%s/mcp", ctx.HttpAddress), nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
@@ -522,7 +522,7 @@ func TestAuthorizationUnauthorized(t *testing.T) {
 	// Failed OIDC validation
 	key, oidcProvider, httpServer := NewOidcTestServer(t)
 	t.Cleanup(httpServer.Close)
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "mcp-server"}, OidcProvider: oidcProvider}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "mcp-server", ValidateToken: true}, OidcProvider: oidcProvider}, func(ctx *httpContext) {
 		req, err := http.NewRequest("GET", fmt.Sprintf("http://%s/mcp", ctx.HttpAddress), nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
@@ -559,7 +559,7 @@ func TestAuthorizationUnauthorized(t *testing.T) {
 		"aud": "mcp-server"
 	}`
 	validOidcToken := oidctest.SignIDToken(key, "test-oidc-key-id", oidc.RS256, rawClaims)
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "mcp-server"}, OidcProvider: oidcProvider}, func(ctx *httpContext) {
+	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "mcp-server", ValidateToken: true}, OidcProvider: oidcProvider}, func(ctx *httpContext) {
 		req, err := http.NewRequest("GET", fmt.Sprintf("http://%s/mcp", ctx.HttpAddress), nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
@@ -583,7 +583,8 @@ func TestAuthorizationUnauthorized(t *testing.T) {
 			}
 		})
 		t.Run("Protected resource with INVALID KUBERNETES Authorization header logs error", func(t *testing.T) {
-			if !strings.Contains(ctx.LogBuffer.String(), "Authentication failed - API Server token validation error") {
+			if !strings.Contains(ctx.LogBuffer.String(), "Authentication failed - JWT validation error") ||
+				!strings.Contains(ctx.LogBuffer.String(), "kubernetes API token validation error: failed to create token review") {
 				t.Errorf("Expected log entry for Kubernetes TokenReview error, got: %s", ctx.LogBuffer.String())
 			}
 		})
@@ -607,12 +608,17 @@ func TestAuthorizationRequireOAuthFalse(t *testing.T) {
 }

 func TestAuthorizationRawToken(t *testing.T) {
-	cases := []string{
-		"",
-		"mcp-server",
+	cases := []struct {
+		audience      string
+		validateToken bool
+	}{
+		{"", false},           // No audience, no validation
+		{"", true},            // No audience, validation enabled
+		{"mcp-server", false}, // Audience set, no validation
+		{"mcp-server", true},  // Audience set, validation enabled
 	}
-	for _, audience := range cases {
-		testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: audience}}, func(ctx *httpContext) {
+	for _, c := range cases {
+		testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: c.audience, ValidateToken: c.validateToken}}, func(ctx *httpContext) {
 			ctx.mockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 				if req.URL.EscapedPath() == "/apis/authentication.k8s.io/v1/tokenreviews" {
 					w.Header().Set("Content-Type", "application/json")
@@ -630,7 +636,7 @@ func TestAuthorizationRawToken(t *testing.T) {
 				t.Fatalf("Failed to get protected endpoint: %v", err)
 			}
 			t.Cleanup(func() { _ = resp.Body.Close() })
-			t.Run("Protected resource with audience = '"+audience+"' with VALID Authorization header returns 200 - OK", func(t *testing.T) {
+			t.Run(fmt.Sprintf("Protected resource with audience = '%s' and validate-token = '%t', with VALID Authorization header returns 200 - OK", c.audience, c.validateToken), func(t *testing.T) {
 				if resp.StatusCode != http.StatusOK {
 					t.Errorf("Expected HTTP 200 OK, got %d", resp.StatusCode)
 				}
@@ -649,7 +655,9 @@ func TestAuthorizationOidcToken(t *testing.T) {
 		"aud": "mcp-server"
 	}`
 	validOidcToken := oidctest.SignIDToken(key, "test-oidc-key-id", oidc.RS256, rawClaims)
-	testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "mcp-server"}, OidcProvider: oidcProvider}, func(ctx *httpContext) {
+	cases := []bool{false, true}
+	for _, validateToken := range cases {
+		testCaseWithContext(t, &httpContext{StaticConfig: &config.StaticConfig{RequireOAuth: true, OAuthAudience: "mcp-server", ValidateToken: validateToken}, OidcProvider: oidcProvider}, func(ctx *httpContext) {
 			ctx.mockServer.Handle(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 				if req.URL.EscapedPath() == "/apis/authentication.k8s.io/v1/tokenreviews" {
 					w.Header().Set("Content-Type", "application/json")
@@ -667,10 +675,12 @@ func TestAuthorizationOidcToken(t *testing.T) {
 				t.Fatalf("Failed to get protected endpoint: %v", err)
 			}
 			t.Cleanup(func() { _ = resp.Body.Close() })
-		t.Run("Protected resource with VALID OIDC Authorization header returns 200 - OK", func(t *testing.T) {
+			t.Run(fmt.Sprintf("Protected resource with validate-token='%t' with VALID OIDC Authorization header returns 200 - OK", validateToken), func(t *testing.T) {
 				if resp.StatusCode != http.StatusOK {
 					t.Errorf("Expected HTTP 200 OK, got %d", resp.StatusCode)
 				}
 			})
 		})
+
+	}
 }
--- a/pkg/kubernetes-mcp-server/cmd/root.go
+++ b/pkg/kubernetes-mcp-server/cmd/root.go
@@ -63,6 +63,7 @@ type MCPServerOptions struct {
 	DisableDestructive   bool
 	RequireOAuth         bool
 	OAuthAudience        string
+	ValidateToken        bool
 	AuthorizationURL     string
 	CertificateAuthority string
 	ServerURL            string
@@ -122,6 +123,8 @@ func NewMCPServer(streams genericiooptions.IOStreams) *cobra.Command {
 	_ = cmd.Flags().MarkHidden("require-oauth")
 	cmd.Flags().StringVar(&o.OAuthAudience, "oauth-audience", o.OAuthAudience, "OAuth audience for token claims validation. Optional. If not set, the audience is not validated. Only valid if require-oauth is enabled.")
 	_ = cmd.Flags().MarkHidden("oauth-audience")
+	cmd.Flags().BoolVar(&o.ValidateToken, "validate-token", o.ValidateToken, "If true, validates the token against the Kubernetes API Server using TokenReview. Optional. If not set, the token is not validated. Only valid if require-oauth is enabled.")
+	_ = cmd.Flags().MarkHidden("validate-token")
 	cmd.Flags().StringVar(&o.AuthorizationURL, "authorization-url", o.AuthorizationURL, "OAuth authorization server URL for protected resource endpoint. If not provided, the Kubernetes API server host will be used. Only valid if require-oauth is enabled.")
 	_ = cmd.Flags().MarkHidden("authorization-url")
 	cmd.Flags().StringVar(&o.ServerURL, "server-url", o.ServerURL, "Server URL of this application. Optional. If set, this url will be served in protected resource metadata endpoint and tokens will be validated with this audience. If not set, expected audience is kubernetes-mcp-server. Only valid if require-oauth is enabled.")
@@ -185,6 +188,9 @@ func (m *MCPServerOptions) loadFlags(cmd *cobra.Command) {
 	if cmd.Flag("oauth-audience").Changed {
 		m.StaticConfig.OAuthAudience = m.OAuthAudience
 	}
+	if cmd.Flag("validate-token").Changed {
+		m.StaticConfig.ValidateToken = m.ValidateToken
+	}
 	if cmd.Flag("authorization-url").Changed {
 		m.StaticConfig.AuthorizationURL = m.AuthorizationURL
 	}
@@ -212,8 +218,8 @@ func (m *MCPServerOptions) Validate() error {
 	if m.Port != "" && (m.SSEPort > 0 || m.HttpPort > 0) {
 		return fmt.Errorf("--port is mutually exclusive with deprecated --http-port and --sse-port flags")
 	}
-	if !m.StaticConfig.RequireOAuth && (m.StaticConfig.AuthorizationURL != "" || m.StaticConfig.ServerURL != "" || m.StaticConfig.CertificateAuthority != "") {
-		return fmt.Errorf("authorization-url, server-url and certificate-authority are only valid if require-oauth is enabled. Missing --port may implicitly set require-oauth to false")
+	if !m.StaticConfig.RequireOAuth && (m.StaticConfig.ValidateToken || m.StaticConfig.OAuthAudience != "" || m.StaticConfig.AuthorizationURL != "" || m.StaticConfig.ServerURL != "" || m.StaticConfig.CertificateAuthority != "") {
+		return fmt.Errorf("validate-token, oauth-audience, authorization-url, server-url and certificate-authority are only valid if require-oauth is enabled. Missing --port may implicitly set require-oauth to false")
 	}
 	if m.StaticConfig.AuthorizationURL != "" {
 		u, err := url.Parse(m.StaticConfig.AuthorizationURL)