From 570d80d2db0f1b94dc2b75891b4d864850982748 Mon Sep 17 00:00:00 2001
From: Paul Abel <p.abel@f5.com>
Date: Fri, 31 Jan 2025 10:57:04 +0000
Subject: [PATCH] move basic auth policy validation to CRD

---
 config/crd/bases/k8s.nginx.org_policies.yaml   |  7 +++++++
 deploy/crds.yaml                               |  7 +++++++
 pkg/apis/configuration/v1/types.go             |  8 +++++++-
 pkg/apis/configuration/validation/policy.go    | 13 -------------
 .../configuration/validation/policy_test.go    | 18 ------------------
 5 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml
index 7bf119c71b..93742544b6 100644
--- a/config/crd/bases/k8s.nginx.org_policies.yaml
+++ b/config/crd/bases/k8s.nginx.org_policies.yaml
@@ -90,9 +90,16 @@ spec:
                 description: BasicAuth holds HTTP Basic authentication configuration
                 properties:
                   realm:
+                    description: The realm for basic authentication
+                    pattern: ^([^"$\\]|\\[^$])*$
                     type: string
                   secret:
+                    description: The name of the Kubernetes secret that stores the
+                      Htpasswd configuration
+                    pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                     type: string
+                required:
+                - secret
                 type: object
               egressMTLS:
                 description: EgressMTLS defines an Egress MTLS policy.
diff --git a/deploy/crds.yaml b/deploy/crds.yaml
index c6601ee07f..330bd5cbc4 100644
--- a/deploy/crds.yaml
+++ b/deploy/crds.yaml
@@ -252,9 +252,16 @@ spec:
                 description: BasicAuth holds HTTP Basic authentication configuration
                 properties:
                   realm:
+                    description: The realm for basic authentication
+                    pattern: ^([^"$\\]|\\[^$])*$
                     type: string
                   secret:
+                    description: The name of the Kubernetes secret that stores the
+                      Htpasswd configuration
+                    pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                     type: string
+                required:
+                - secret
                 type: object
               egressMTLS:
                 description: EgressMTLS defines an Egress MTLS policy.
diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go
index cac87569ab..4fdc4668b6 100644
--- a/pkg/apis/configuration/v1/types.go
+++ b/pkg/apis/configuration/v1/types.go
@@ -623,7 +623,13 @@ type JWTAuth struct {
 
 // BasicAuth holds HTTP Basic authentication configuration
 type BasicAuth struct {
-	Realm  string `json:"realm"`
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:Pattern=`^([^"$\\]|\\[^$])*$`
+	// The realm for basic authentication
+	Realm string `json:"realm,omitempty"`
+	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
+	// The name of the Kubernetes secret that stores the Htpasswd configuration
 	Secret string `json:"secret"`
 }
 
diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go
index 98d8626d08..e7b606a89c 100644
--- a/pkg/apis/configuration/validation/policy.go
+++ b/pkg/apis/configuration/validation/policy.go
@@ -45,7 +45,6 @@ func validatePolicySpec(spec *v1.PolicySpec, fieldPath *field.Path, isPlus, enab
 	}
 
 	if spec.BasicAuth != nil {
-		allErrs = append(allErrs, validateBasic(spec.BasicAuth, fieldPath.Child("basicAuth"))...)
 		fieldCount++
 	}
 
@@ -206,18 +205,6 @@ func validateJWT(jwt *v1.JWTAuth, fieldPath *field.Path) field.ErrorList {
 	return allErrs
 }
 
-func validateBasic(basic *v1.BasicAuth, fieldPath *field.Path) field.ErrorList {
-	if basic.Secret == "" {
-		return field.ErrorList{field.Required(fieldPath.Child("secret"), "")}
-	}
-
-	allErrs := field.ErrorList{}
-	if basic.Realm != "" {
-		allErrs = append(allErrs, validateRealm(basic.Realm, fieldPath.Child("realm"))...)
-	}
-	return append(allErrs, validateSecretName(basic.Secret, fieldPath.Child("secret"))...)
-}
-
 func validateIngressMTLS(ingressMTLS *v1.IngressMTLS, fieldPath *field.Path) field.ErrorList {
 	if ingressMTLS.ClientCertSecret == "" {
 		return field.ErrorList{field.Required(fieldPath.Child("clientCertSecret"), "")}
diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go
index 542bfaba24..cbaf9c81e0 100644
--- a/pkg/apis/configuration/validation/policy_test.go
+++ b/pkg/apis/configuration/validation/policy_test.go
@@ -1982,24 +1982,6 @@ func TestValidateWAF_FailsOnInvalidApPolicy(t *testing.T) {
 	}
 }
 
-func TestValidateBasic_PassesOnNotEmptySecret(t *testing.T) {
-	t.Parallel()
-
-	errList := validateBasic(&v1.BasicAuth{Realm: "", Secret: "secret"}, field.NewPath("secret"))
-	if len(errList) != 0 {
-		t.Errorf("want no errors, got %v", errList)
-	}
-}
-
-func TestValidateBasic_FailsOnMissingSecret(t *testing.T) {
-	t.Parallel()
-
-	errList := validateBasic(&v1.BasicAuth{Realm: "realm", Secret: ""}, field.NewPath("secret"))
-	if len(errList) == 0 {
-		t.Error("want error on invalid input")
-	}
-}
-
 func TestValidateWAF_FailsOnPresentBothApLogBundleAndApLogConf(t *testing.T) {
 	t.Parallel()