Validate MCPOIDCConfig inline issuer and JWKS URLs

cmd/thv-operator/api/v1beta1/mcpoidcconfig_types.go

@@ -141,7 +141,7 @@ type InlineOIDCSharedConfig struct {
 	// +optional
 	ProtectedResourceAllowPrivateIP bool `json:"protectedResourceAllowPrivateIP"`
 
-	// InsecureAllowHTTP allows HTTP (non-HTTPS) OIDC issuers for development/testing.
+	// InsecureAllowHTTP allows HTTP (non-HTTPS) OIDC issuer and JWKS URLs for development/testing.
 	// WARNING: This is insecure and should NEVER be used in production.
 	// +kubebuilder:default=false
 	// +optional

cmd/thv-operator/controllers/mcpoidcconfig_controller.go

@@ -21,6 +21,7 @@ import (
 
 	mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
 	ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil"
+	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/validation"
 )
 
 const (
@@ -87,7 +88,7 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 
 	// Validate spec configuration early
-	if err := oidcConfig.Validate(); err != nil {
+	if err := validateOIDCConfigSpec(oidcConfig); err != nil {
 		logger.Error(err, "MCPOIDCConfig spec validation failed")
 		return r.handleValidationFailure(ctx, oidcConfig, err)
 	}
@@ -135,6 +136,26 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	return ctrl.Result{}, nil
 }
 
+// validateOIDCConfigSpec validates the MCPOIDCConfig spec: type/config
+// consistency via Validate(), then issuer and JWKS URL checks for inline
+// configs. The URL checks run here rather than inside Validate() because the
+// validation package imports the v1beta1 API package (for example
+// validation.ValidateCABundleSource), so calling it from the types package
+// would create an import cycle. This mirrors the MCPRemoteProxy controller,
+// which applies validation.ValidateRemoteURL in its validateSpec step.
+func validateOIDCConfigSpec(oidcConfig *mcpv1beta1.MCPOIDCConfig) error {
+	if err := oidcConfig.Validate(); err != nil {
+		return err
+	}
+	if oidcConfig.Spec.Type != mcpv1beta1.MCPOIDCConfigTypeInline || oidcConfig.Spec.Inline == nil {
+		return nil
+	}
+	if err := validation.ValidateOIDCIssuerURL(oidcConfig.Spec.Inline.Issuer, oidcConfig.Spec.Inline.InsecureAllowHTTP); err != nil {
+		return err
+	}
+	return validation.ValidateJWKSURL(oidcConfig.Spec.Inline.JWKSURL, oidcConfig.Spec.Inline.InsecureAllowHTTP)
+}
+
 // handleValidationFailure records the Valid=False condition for a spec that
 // failed validation and emits a one-shot Warning event on the transition into
 // the invalid state. It never requeues — the user must fix the spec.

cmd/thv-operator/controllers/mcpoidcconfig_controller_test.go

@@ -513,6 +513,211 @@ func TestMCPOIDCConfig_Validate(t *testing.T) {
 	}
 }
 
+func TestValidateOIDCConfigSpec(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		config      *mcpv1beta1.MCPOIDCConfig
+		expectError bool
+	}{
+		{
+			name: "valid inline config with HTTPS issuer and JWKS URL",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:   "https://accounts.google.com",
+						JWKSURL:  "https://www.googleapis.com/oauth2/v3/certs",
+						ClientID: "test-client",
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "valid inline config with empty JWKS URL (auto-discovery)",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:   "https://accounts.google.com",
+						ClientID: "test-client",
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "valid inline config with HTTP issuer when insecureAllowHTTP is set",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:            "http://issuer.dev.local",
+						ClientID:          "test-client",
+						InsecureAllowHTTP: true,
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "invalid inline config with HTTP issuer without insecureAllowHTTP",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:   "http://issuer.example.com",
+						ClientID: "test-client",
+					},
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "invalid inline config with malformed issuer",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:   "not-a-url",
+						ClientID: "test-client",
+					},
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "invalid inline config with issuer using unsupported scheme",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:   "ftp://issuer.example.com",
+						ClientID: "test-client",
+					},
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "invalid inline config with non-HTTPS JWKS URL",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:   "https://accounts.google.com",
+						JWKSURL:  "http://www.googleapis.com/oauth2/v3/certs",
+						ClientID: "test-client",
+					},
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "valid inline config with HTTP JWKS URL and insecureAllowHTTP",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+						Issuer:            "http://keycloak:8080/realms/toolhive",
+						JWKSURL:           "http://keycloak:8080/realms/toolhive/protocol/openid-connect/certs",
+						ClientID:          "test-client",
+						InsecureAllowHTTP: true,
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "kubernetesServiceAccount config skips URL validation",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeKubernetesServiceAccount,
+					KubernetesServiceAccount: &mcpv1beta1.KubernetesServiceAccountOIDCConfig{
+						ServiceAccount: "test-sa",
+						// Would fail the URL checks if they applied to this type
+						Issuer: "not-a-url",
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "type consistency failure is still reported",
+			config: &mcpv1beta1.MCPOIDCConfig{
+				Spec: mcpv1beta1.MCPOIDCConfigSpec{
+					Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+					// Missing Inline config
+				},
+			},
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			err := validateOIDCConfigSpec(tt.config)
+
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestMCPOIDCConfigReconciler_URLValidationFailureSetsCondition(t *testing.T) {
+	t.Parallel()
+
+	ctx := t.Context()
+
+	// Invalid config: HTTP issuer without insecureAllowHTTP
+	oidcConfig := &mcpv1beta1.MCPOIDCConfig{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "insecure-issuer-config",
+			Namespace:  "default",
+			Finalizers: []string{OIDCConfigFinalizerName},
+			Generation: 1,
+		},
+		Spec: mcpv1beta1.MCPOIDCConfigSpec{
+			Type: mcpv1beta1.MCPOIDCConfigTypeInline,
+			Inline: &mcpv1beta1.InlineOIDCSharedConfig{
+				Issuer:   "http://issuer.example.com",
+				ClientID: "test-client",
+			},
+		},
+	}
+
+	r, fakeClient := newTestMCPOIDCConfigReconciler(t, oidcConfig)
+
+	req := reconcile.Request{
+		NamespacedName: types.NamespacedName{
+			Name:      oidcConfig.Name,
+			Namespace: oidcConfig.Namespace,
+		},
+	}
+
+	// Reconcile should not return error (validation failures are not requeued)
+	_, err := r.Reconcile(ctx, req)
+	require.NoError(t, err)
+
+	var updatedConfig mcpv1beta1.MCPOIDCConfig
+	err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig)
+	require.NoError(t, err)
+
+	cond := meta.FindStatusCondition(updatedConfig.Status.Conditions, mcpv1beta1.ConditionTypeOIDCConfigValid)
+	require.NotNil(t, cond, "Should have a Valid condition")
+	assert.Equal(t, metav1.ConditionFalse, cond.Status, "Valid condition should be False")
+	assert.Equal(t, mcpv1beta1.ConditionReasonOIDCConfigInvalid, cond.Reason)
+	assert.Contains(t, cond.Message, "http://issuer.example.com",
+		"Message should surface the offending issuer URL")
+}
+
 // TestMCPOIDCConfigReconciler_ReconcileKeepsExistingForeignCondition verifies
 // that when the controller observes a foreign-owned condition already on the
 // object and then writes its own Valid=True, it folds its condition into the

cmd/thv-operator/pkg/validation/url_validation.go

@@ -116,9 +116,13 @@ func validateHostNotInternal(host string) error {
 
 // ValidateJWKSURL validates that rawURL, if non-empty, is a well-formed HTTPS
 // URL with a non-empty host. JWKS endpoints serve key material and must use
-// HTTPS. An empty rawURL is allowed because JWKS discovery can determine the
-// endpoint automatically.
-func ValidateJWKSURL(rawURL string) error {
+// HTTPS. If allowInsecure is true, HTTP is permitted (for development/testing
+// only), matching ValidateOIDCIssuerURL. An empty rawURL is allowed because
+// JWKS discovery can determine the endpoint automatically.
+//
+// Schemes other than http and https are always rejected, regardless of
+// allowInsecure.
+func ValidateJWKSURL(rawURL string, allowInsecure bool) error {
 	if rawURL == "" {
 		return nil
 	}
@@ -128,8 +132,15 @@ func ValidateJWKSURL(rawURL string) error {
 		return fmt.Errorf("JWKS URL is invalid: %w", err)
 	}
 
-	if u.Scheme != schemeHTTPS {
-		return fmt.Errorf("JWKS URL must use HTTPS scheme, got %q", u.Scheme)
+	if u.Scheme == schemeHTTP && !allowInsecure {
+		return fmt.Errorf(
+			"JWKS URL %q uses HTTP scheme, which is insecure; "+
+				"use HTTPS or set insecureAllowHTTP: true for development only", rawURL,
+		)
+	}
+
+	if u.Scheme != schemeHTTP && u.Scheme != schemeHTTPS {
+		return fmt.Errorf("JWKS URL %q has unsupported scheme %q; must be http or https", rawURL, u.Scheme)
 	}
 
 	if u.Host == "" {

cmd/thv-operator/pkg/validation/url_validation_test.go

@@ -236,10 +236,11 @@ func TestValidateJWKSURL(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name        string
-		rawURL      string
-		wantErr     bool
-		errContains string
+		name          string
+		rawURL        string
+		allowInsecure bool
+		wantErr       bool
+		errContains   string
 	}{
 		{
 			name:    "empty URL allowed",
@@ -255,13 +256,26 @@ func TestValidateJWKSURL(t *testing.T) {
 			name:        "http rejected",
 			rawURL:      "http://jwks.example.com",
 			wantErr:     true,
-			errContains: "HTTPS",
+			errContains: "insecureAllowHTTP",
+		},
+		{
+			name:          "http allowed with insecureAllowHTTP",
+			rawURL:        "http://jwks.example.com",
+			allowInsecure: true,
+			wantErr:       false,
 		},
 		{
 			name:        "unsupported scheme",
 			rawURL:      "ftp://jwks.example.com",
 			wantErr:     true,
-			errContains: "HTTPS",
+			errContains: "unsupported scheme",
+		},
+		{
+			name:          "unsupported scheme rejected even with insecureAllowHTTP",
+			rawURL:        "ftp://jwks.example.com",
+			allowInsecure: true,
+			wantErr:       true,
+			errContains:   "unsupported scheme",
 		},
 		{
 			name:        "missing host",
@@ -275,7 +289,7 @@ func TestValidateJWKSURL(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			err := validation.ValidateJWKSURL(tt.rawURL)
+			err := validation.ValidateJWKSURL(tt.rawURL, tt.allowInsecure)
 
 			if tt.wantErr {
 				require.Error(t, err)

cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_authserverref_integration_test.go

@@ -288,7 +288,8 @@ func newMCPOIDCConfig(name, namespace string) *mcpv1beta1.MCPOIDCConfig {
 		Spec: mcpv1beta1.MCPOIDCConfigSpec{
 			Type: mcpv1beta1.MCPOIDCConfigTypeInline,
 			Inline: &mcpv1beta1.InlineOIDCSharedConfig{
-				Issuer: "http://localhost:9090",
+				Issuer:            "http://localhost:9090",
+				InsecureAllowHTTP: true,
 			},
 		},
 	}

cmd/thv-operator/test-integration/mcp-remote-proxy/mcpremoteproxy_cabundle_integration_test.go

@@ -25,7 +25,8 @@ func newMCPOIDCConfigWithCABundle(name, namespace, cmName, key string) *mcpv1bet
 		Spec: mcpv1beta1.MCPOIDCConfigSpec{
 			Type: mcpv1beta1.MCPOIDCConfigTypeInline,
 			Inline: &mcpv1beta1.InlineOIDCSharedConfig{
-				Issuer: "http://localhost:9090",
+				Issuer:            "http://localhost:9090",
+				InsecureAllowHTTP: true,
 				CABundleRef: &mcpv1beta1.CABundleSource{
 					ConfigMapRef: &corev1.ConfigMapKeySelector{
 						LocalObjectReference: corev1.LocalObjectReference{Name: cmName},