Merge pull request #677 from dciabrin/custom-pw-validation

Enable custom password requirements and rejects

Author: fmount

modules/common/secret/password.go

@@ -22,12 +22,12 @@ import (
 )
 
 // PasswordValidator implements the Validator interface
-type PasswordValidator struct{}
-
-// Validate - implements the Validator interface and calls the underlying
-// ValidatePassword
-func (v PasswordValidator) Validate(value string) error {
-	return ValidatePassword(value)
+// +kubebuilder:object:generate=false
+type PasswordValidator struct {
+	// Patterns that the password should adhere to
+	Requirements *[]Rule
+	// Patterns that are forbidden for the password
+	Rejects *[]Rule
 }
 
 // Rule - pattern to match when rejecting or accepting a string
@@ -76,22 +76,32 @@ var (
 	ErrPasswordRequirements = errors.New("password does not meet the requirements")
 )
 
-// ValidatePassword validates a password against security requirements
-// Returns error when invalid/rejected
-func ValidatePassword(pwd string) error {
+// Validate - implements the Validator interface
+// If requirements or rejects rules are not specified in the
+// structure, the function uses the default rule set defined
+// in this package.
+func (v PasswordValidator) Validate(value string) error {
 	// Check if password is empty
-	if pwd == "" {
+	if value == "" {
 		return ErrEmptyPassword
 	}
 
-	for _, rule := range requirements {
-		if !rule.pattern.MatchString(pwd) {
+	reqs := &requirements
+	if v.Requirements != nil {
+		reqs = v.Requirements
+	}
+	for _, rule := range *reqs {
+		if !rule.pattern.MatchString(value) {
 			return ErrPasswordRequirements
 		}
 	}
 
-	for _, rule := range rejects {
-		if rule.pattern.MatchString(pwd) {
+	rejs := &rejects
+	if v.Rejects != nil {
+		rejs = v.Rejects
+	}
+	for _, rule := range *rejs {
+		if rule.pattern.MatchString(value) {
 			return ErrPasswordRequirements
 		}
 	}

modules/common/secret/password_test.go

@@ -17,10 +17,11 @@ limitations under the License.
 package secret
 
 import (
-	. "github.com/onsi/gomega" // nolint:revive
 	"regexp"
 	"slices"
 	"testing"
+
+	. "github.com/onsi/gomega" // nolint:revive
 )
 
 const ErrMsg string = "password does not meet the requirements"
@@ -279,12 +280,15 @@ func TestValidatePassword(t *testing.T) {
 		alphaNumPattern,
 		fernetPattern,
 	)
+
+	validator := PasswordValidator{}
+
 	// Execute ValidatePassword against the generated TestVector
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			err := ValidatePassword(tt.password)
+			err := validator.Validate(tt.password)
 
 			if tt.wantErr {
 				g.Expect(err).To(HaveOccurred())
@@ -295,3 +299,127 @@ func TestValidatePassword(t *testing.T) {
 		})
 	}
 }
+
+func TestValidatePasswordWithCustomRules(t *testing.T) {
+	// Define custom requirements (strict rules)
+	customRequirements := []Rule{
+		{
+			description: "Must only contain alphanumerical and safe special characters",
+			pattern:     *regexp.MustCompile(`^[a-zA-Z0-9@#%^*-_=+:,.!~]+$`),
+		},
+	}
+
+	// Define custom rejects (forbid specific patterns)
+	customRejects := []Rule{
+		{
+			description: "Must not contain carriage return",
+			pattern:     *regexp.MustCompile(`[\n]`),
+		},
+	}
+
+	tests := []struct {
+		name      string
+		validator PasswordValidator
+		password  string
+		wantErr   bool
+		errMsg    string
+	}{
+		// Test with nil rules (should use defaults)
+		{
+			name:      "nil rules uses defaults - valid",
+			validator: PasswordValidator{},
+			password:  "ValidPassword123",
+			wantErr:   false,
+		},
+		{
+			name:      "nil rules uses defaults - shell expansion rejected",
+			validator: PasswordValidator{},
+			password:  "Password123$HOME",
+			wantErr:   true,
+			errMsg:    ErrMsg,
+		},
+		{
+			name:      "nil rules uses defaults - empty password rejected",
+			validator: PasswordValidator{},
+			password:  "",
+			wantErr:   true,
+			errMsg:    "empty password not allowed",
+		},
+
+		// Test with custom requirements only
+		{
+			name: "custom requirements - safe special characters",
+			validator: PasswordValidator{
+				Requirements: &customRequirements,
+			},
+			password: "#S3cure!Pass#",
+			wantErr:  false,
+		},
+
+		// Test with custom rejects only
+		{
+			name: "custom rejects - must not contains '\n'",
+			validator: PasswordValidator{
+				Rejects: &customRejects,
+			},
+			password: "MyPASSWORD123\n",
+			wantErr:  true,
+			errMsg:   ErrMsg,
+		},
+
+		// Test with both custom requirements and rejects
+		{
+			name: "custom both - fully valid password",
+			validator: PasswordValidator{
+				Requirements: &customRequirements,
+				Rejects:      &customRejects,
+			},
+			password: "MyS3cure!Pass",
+			wantErr:  false,
+		},
+
+		// Test empty requirements/rejects slices
+		{
+			name: "empty requirements slice - allows any non-empty password",
+			validator: PasswordValidator{
+				Requirements: &[]Rule{},
+			},
+			password: "a",
+			wantErr:  false,
+		},
+		{
+			name: "empty rejects slice - no rejections",
+			validator: PasswordValidator{
+				Rejects: &[]Rule{},
+			},
+			password: "anything$HOME$(cmd)`test`",
+			wantErr:  false,
+		},
+		{
+			name: "both empty - allows any non-empty password",
+			validator: PasswordValidator{
+				Requirements: &[]Rule{},
+				Rejects:      &[]Rule{},
+			},
+			password: "x",
+			wantErr:  false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			err := tt.validator.Validate(tt.password)
+
+			if tt.wantErr {
+				g.Expect(err).To(HaveOccurred())
+				if tt.errMsg != "" {
+					g.Expect(err.Error()).To(ContainSubstring(tt.errMsg))
+				}
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
+			}
+		})
+	}
+}