Remove unimplemented pass_through auth strategy (#2753)

jhrozek · web-flow · commit 49812fa1b87f · 2025-11-27T12:23:44.000+01:00
The pass_through authentication strategy was defined in types,
validation, and documentation but was never actually implemented.
This creates confusion as users might try to use it expecting it
to work.

- Remove BackendAuthTypePassThrough constant and enum value
- Update webhook validation to remove pass_through case
- Update test fixtures to use valid strategies
- Remove pass_through examples from documentation
- Regenerate CRD manifests and API docs
diff --git a/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go b/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go
@@ -108,7 +108,7 @@ type OutgoingAuthConfig struct {
 // BackendAuthConfig defines authentication configuration for a backend MCPServer
 type BackendAuthConfig struct {
 	// Type defines the authentication type
-	// +kubebuilder:validation:Enum=discovered;pass_through;external_auth_config_ref
+	// +kubebuilder:validation:Enum=discovered;external_auth_config_ref
 	// +kubebuilder:validation:Required
 	Type string `json:"type"`
 
@@ -517,9 +517,6 @@ const (
 	// BackendAuthTypeDiscovered automatically discovers from backend's externalAuthConfigRef
 	BackendAuthTypeDiscovered = "discovered"
 
-	// BackendAuthTypePassThrough forwards client token unchanged
-	BackendAuthTypePassThrough = "pass_through"
-
 	// BackendAuthTypeExternalAuthConfigRef references an MCPExternalAuthConfig resource
 	BackendAuthTypeExternalAuthConfigRef = "external_auth_config_ref"
 )
diff --git a/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go b/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go
@@ -273,13 +273,6 @@ func TestBackendAuthConfigTypes(t *testing.T) {
 			},
 			isValid: true,
 		},
-		{
-			name: "pass_through_auth",
-			authConfig: BackendAuthConfig{
-				Type: BackendAuthTypePassThrough,
-			},
-			isValid: true,
-		},
 		{
 			name: "external_auth_config_ref_valid",
 			authConfig: BackendAuthConfig{
diff --git a/cmd/thv-operator/api/v1alpha1/virtualmcpserver_webhook.go b/cmd/thv-operator/api/v1alpha1/virtualmcpserver_webhook.go
@@ -144,12 +144,12 @@ func (*VirtualMCPServer) validateBackendAuth(backendName string, auth BackendAut
 			return fmt.Errorf("spec.outgoingAuth.backends[%s].externalAuthConfigRef.name is required", backendName)
 		}
 
-	case BackendAuthTypeDiscovered, BackendAuthTypePassThrough:
+	case BackendAuthTypeDiscovered:
 		// No additional validation needed
 
 	default:
 		return fmt.Errorf(
-			"spec.outgoingAuth.backends[%s].type must be one of: discovered, pass_through, service_account, external_auth_config_ref",
+			"spec.outgoingAuth.backends[%s].type must be one of: discovered, external_auth_config_ref",
 			backendName)
 	}
 
diff --git a/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go b/cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go
@@ -101,7 +101,7 @@ func TestConvertOutgoingAuth(t *testing.T) {
 			outgoingAuth: &mcpv1alpha1.OutgoingAuthConfig{
 				Source: "inline",
 				Default: &mcpv1alpha1.BackendAuthConfig{
-					Type: mcpv1alpha1.BackendAuthTypePassThrough,
+					Type: mcpv1alpha1.BackendAuthTypeDiscovered,
 				},
 			},
 			expectedSource: "inline",
@@ -114,7 +114,7 @@ func TestConvertOutgoingAuth(t *testing.T) {
 				Source: "mixed",
 				Backends: map[string]mcpv1alpha1.BackendAuthConfig{
 					"backend-1": {
-						Type: mcpv1alpha1.BackendAuthTypePassThrough,
+						Type: mcpv1alpha1.BackendAuthTypeDiscovered,
 					},
 				},
 			},
@@ -165,11 +165,11 @@ func TestConvertBackendAuthConfig(t *testing.T) {
 		hasMetadata  bool
 	}{
 		{
-			name: "pass through",
+			name: "discovered",
 			authConfig: &mcpv1alpha1.BackendAuthConfig{
-				Type: mcpv1alpha1.BackendAuthTypePassThrough,
+				Type: mcpv1alpha1.BackendAuthTypeDiscovered,
 			},
-			expectedType: mcpv1alpha1.BackendAuthTypePassThrough,
+			expectedType: mcpv1alpha1.BackendAuthTypeDiscovered,
 			hasMetadata:  false,
 		},
 		{
@@ -520,7 +520,7 @@ func TestYAMLMarshalingDeterminism(t *testing.T) {
 						Type: mcpv1alpha1.BackendAuthTypeDiscovered,
 					},
 					"backend-alpha": {
-						Type: mcpv1alpha1.BackendAuthTypePassThrough,
+						Type: mcpv1alpha1.BackendAuthTypeDiscovered,
 					},
 					"backend-middle": {
 						Type: mcpv1alpha1.BackendAuthTypeDiscovered,
diff --git a/cmd/vmcp/README.md b/cmd/vmcp/README.md
@@ -67,7 +67,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 aggregation:
   conflict_resolution: prefix
   conflict_resolution_config:
diff --git a/deploy/charts/operator-crds/Chart.yaml b/deploy/charts/operator-crds/Chart.yaml
@@ -2,5 +2,5 @@ apiVersion: v2
 name: toolhive-operator-crds
 description: A Helm chart for installing the ToolHive Operator CRDs into Kubernetes.
 type: application
-version: 0.0.64
+version: 0.0.65
 appVersion: "0.0.1"
diff --git a/deploy/charts/operator-crds/README.md b/deploy/charts/operator-crds/README.md
@@ -1,6 +1,6 @@
 # ToolHive Operator CRDs Helm Chart
 
-![Version: 0.0.64](https://img.shields.io/badge/Version-0.0.64-informational?style=flat-square)
+![Version: 0.0.65](https://img.shields.io/badge/Version-0.0.65-informational?style=flat-square)
 ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square)
 
 A Helm chart for installing the ToolHive Operator CRDs into Kubernetes.
diff --git a/deploy/charts/operator-crds/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/crds/toolhive.stacklok.dev_virtualmcpservers.yaml
@@ -609,7 +609,6 @@ spec:
                           description: Type defines the authentication type
                           enum:
                           - discovered
-                          - pass_through
                           - external_auth_config_ref
                           type: string
                       required:
@@ -639,7 +638,6 @@ spec:
                         description: Type defines the authentication type
                         enum:
                         - discovered
-                        - pass_through
                         - external_auth_config_ref
                         type: string
                     required:
diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md
diff --git a/docs/operator/virtualmcpserver-api.md b/docs/operator/virtualmcpserver-api.md
@@ -90,7 +90,7 @@ spec:
   outgoingAuth:
     source: discovered
     default:
-      type: pass_through
+      type: discovered
 ```
 
 **Example (inline mode)**:
@@ -119,7 +119,7 @@ spec:
   outgoingAuth:
     source: mixed
     default:
-      type: pass_through
+      type: discovered
     backends:
       # Override specific backends while others use discovery
       slack:
@@ -139,10 +139,7 @@ spec:
 **Fields**:
 - `type` (string, required): Authentication type
   - `discovered`: Automatically discover from backend
-  - `pass_through`: Forward client token unchanged
-  - `service_account`: Use service account credentials
   - `external_auth_config_ref`: Reference an MCPExternalAuthConfig resource
-- `serviceAccount` (ServiceAccountAuth, optional): Service account configuration (when type=service_account)
 - `externalAuthConfigRef` (ExternalAuthConfigRef, optional): Auth config reference (when type=external_auth_config_ref)
 
 ### `.spec.aggregation` (optional)
@@ -440,7 +437,7 @@ spec:
   outgoingAuth:
     source: discovered
     default:
-      type: pass_through
+      type: discovered
     backends:
       slack:  # Override for specific backend
         type: service_account
diff --git a/examples/vmcp-config.yaml b/examples/vmcp-config.yaml
@@ -49,8 +49,6 @@ outgoing_auth:
   # Default behavior for backends without explicit config
   default:
     type: unauthenticated  # unauthenticated | header_injection | token_exchange
-    # TODO: Uncomment when pass_through is implemented
-    # type: pass_through  # Forward client token unchanged
 
   # Per-backend authentication configurations
   # IMPORTANT: These tokens are for backend APIs (e.g., github-api, jira-api),
@@ -93,10 +91,6 @@ outgoing_auth:
     #     audience: "jira-api"  # Token audience for Jira API
     #     scopes: ["read:jira-work", "write:jira-work"]
 
-    # TODO: Uncomment when pass_through strategy is implemented
-    # internal-db:
-    #   type: pass_through  # Forward client token unchanged
-
 # ===== TOKEN CACHING =====
 # Token cache configuration
 token_cache:
diff --git a/pkg/vmcp/auth/strategies/unauthenticated.go b/pkg/vmcp/auth/strategies/unauthenticated.go
@@ -20,7 +20,7 @@ import (
 //
 // Security Warning: Only use this strategy when you are certain the backend
 // requires no authentication. For production deployments, prefer explicit
-// authentication strategies (pass_through, header_injection, token_exchange).
+// authentication strategies (header_injection, token_exchange).
 //
 // Configuration: No metadata required, but any metadata is accepted and ignored.
 //
diff --git a/pkg/vmcp/client/client_test.go b/pkg/vmcp/client/client_test.go
@@ -271,14 +271,14 @@ func TestAuthRoundTripper_RoundTrip(t *testing.T) {
 			name: "successful authentication adds headers and forwards request",
 			target: &vmcp.BackendTarget{
 				WorkloadID:   "backend-1",
-				AuthStrategy: "pass_through",
+				AuthStrategy: "header_injection",
 				AuthMetadata: map[string]any{"key": "value"},
 			},
 			setupStrategy: func(ctrl *gomock.Controller) auth.Strategy {
 				mockStrategy := authmocks.NewMockStrategy(ctrl)
 				mockStrategy.EXPECT().
 					Name().
-					Return("pass_through").
+					Return("header_injection").
 					AnyTimes()
 				mockStrategy.EXPECT().
 					Authenticate(
@@ -351,14 +351,14 @@ func TestAuthRoundTripper_RoundTrip(t *testing.T) {
 			name: "authentication failure returns error without calling base transport",
 			target: &vmcp.BackendTarget{
 				WorkloadID:   "backend-1",
-				AuthStrategy: "pass_through",
+				AuthStrategy: "header_injection",
 				AuthMetadata: map[string]any{"key": "value"},
 			},
 			setupStrategy: func(ctrl *gomock.Controller) auth.Strategy {
 				mockStrategy := authmocks.NewMockStrategy(ctrl)
 				mockStrategy.EXPECT().
 					Name().
-					Return("pass_through").
+					Return("header_injection").
 					AnyTimes()
 				mockStrategy.EXPECT().
 					Authenticate(
@@ -382,14 +382,14 @@ func TestAuthRoundTripper_RoundTrip(t *testing.T) {
 			name: "base transport error propagates after successful auth",
 			target: &vmcp.BackendTarget{
 				WorkloadID:   "backend-1",
-				AuthStrategy: "pass_through",
+				AuthStrategy: "header_injection",
 				AuthMetadata: map[string]any{"key": "value"},
 			},
 			setupStrategy: func(ctrl *gomock.Controller) auth.Strategy {
 				mockStrategy := authmocks.NewMockStrategy(ctrl)
 				mockStrategy.EXPECT().
 					Name().
-					Return("pass_through").
+					Return("header_injection").
 					AnyTimes()
 				mockStrategy.EXPECT().
 					Authenticate(
@@ -413,14 +413,14 @@ func TestAuthRoundTripper_RoundTrip(t *testing.T) {
 			name: "request immutability - original request unchanged",
 			target: &vmcp.BackendTarget{
 				WorkloadID:   "backend-1",
-				AuthStrategy: "pass_through",
+				AuthStrategy: "header_injection",
 				AuthMetadata: map[string]any{"key": "value"},
 			},
 			setupStrategy: func(ctrl *gomock.Controller) auth.Strategy {
 				mockStrategy := authmocks.NewMockStrategy(ctrl)
 				mockStrategy.EXPECT().
 					Name().
-					Return("pass_through").
+					Return("header_injection").
 					AnyTimes()
 				mockStrategy.EXPECT().
 					Authenticate(
diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go
@@ -171,8 +171,7 @@ type OutgoingAuthConfig struct {
 
 // BackendAuthStrategy defines how to authenticate to a specific backend.
 type BackendAuthStrategy struct {
-	// Type is the auth strategy: "pass_through", "token_exchange", "client_credentials",
-	// "service_account", "header_injection", "oauth_proxy"
+	// Type is the auth strategy: "unauthenticated", "header_injection", "token_exchange"
 	Type string `json:"type" yaml:"type"`
 
 	// Metadata contains strategy-specific configuration.
diff --git a/pkg/vmcp/config/validator_test.go b/pkg/vmcp/config/validator_test.go
@@ -212,17 +212,6 @@ func TestValidator_ValidateOutgoingAuth(t *testing.T) {
 			},
 			wantErr: false,
 		},
-		// TODO: Uncomment when pass_through strategy is implemented
-		// {
-		// 	name: "valid inline source with pass_through default",
-		// 	auth: &OutgoingAuthConfig{
-		// 		Source: "inline",
-		// 		Default: &BackendAuthStrategy{
-		// 			Type: "pass_through",
-		// 		},
-		// 	},
-		// 	wantErr: false,
-		// },
 		// TODO: Uncomment when token_exchange strategy is implemented
 		// {
 		// 	name: "valid token_exchange backend",
diff --git a/pkg/vmcp/config/yaml_loader_test.go b/pkg/vmcp/config/yaml_loader_test.go
@@ -53,7 +53,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 
 aggregation:
   conflict_resolution: prefix
@@ -100,7 +100,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 
 aggregation:
   conflict_resolution: prefix
@@ -139,7 +139,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 
 aggregation:
   conflict_resolution: prefix
@@ -184,7 +184,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 
 aggregation:
   conflict_resolution: prefix
@@ -258,7 +258,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 
 aggregation:
   conflict_resolution: prefix
@@ -289,7 +289,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 
 aggregation:
   conflict_resolution: prefix
@@ -317,7 +317,7 @@ incoming_auth:
 outgoing_auth:
   source: inline
   default:
-    type: pass_through
+    type: unauthenticated
 
 aggregation:
   conflict_resolution: prefix
diff --git a/pkg/vmcp/registry_test.go b/pkg/vmcp/registry_test.go
@@ -30,7 +30,7 @@ func TestNewImmutableRegistry(t *testing.T) {
 					BaseURL:       "http://localhost:8080",
 					TransportType: "streamable-http",
 					HealthStatus:  BackendHealthy,
-					AuthStrategy:  "pass_through",
+					AuthStrategy:  "unauthenticated",
 					AuthMetadata:  map[string]any{"key": "value"},
 					Metadata:      map[string]string{"env": "production"},
 				},
diff --git a/pkg/vmcp/types.go b/pkg/vmcp/types.go
@@ -47,7 +47,7 @@ type BackendTarget struct {
 
 	// AuthStrategy identifies the authentication strategy for this backend.
 	// The actual authentication is handled by OutgoingAuthRegistry interface.
-	// Examples: "pass_through", "token_exchange", "client_credentials", "oauth_proxy"
+	// Examples: "unauthenticated", "header_injection", "token_exchange"
 	AuthStrategy string
 
 	// AuthMetadata contains strategy-specific authentication metadata.

Original file line number	Diff line number	Diff line change
`@@ -144,12 +144,12 @@ func (*VirtualMCPServer) validateBackendAuth(backendName string, auth BackendAut`
`144`	`144`	`return fmt.Errorf("spec.outgoingAuth.backends[%s].externalAuthConfigRef.name is required", backendName)`
`145`	`145`	`}`
`146`	`146`
`147`		`- case BackendAuthTypeDiscovered, BackendAuthTypePassThrough:`
	`147`	`+ case BackendAuthTypeDiscovered:`
`148`	`148`	`// No additional validation needed`
`149`	`149`
`150`	`150`	`default:`
`151`	`151`	`return fmt.Errorf(`
`152`		`- "spec.outgoingAuth.backends[%s].type must be one of: discovered, pass_through, service_account, external_auth_config_ref",`
	`152`	`+ "spec.outgoingAuth.backends[%s].type must be one of: discovered, external_auth_config_ref",`
`153`	`153`	`backendName)`
`154`	`154`	`}`
`155`	`155`