Refactor password validation and config defaults (#7859)

* refactor password validation and config defaults

* reorder config lines for clarity

api/admin.go

@@ -310,7 +310,7 @@ func adminResetPassword(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	newPassword := props["new_password"]
-	if err := utils.IsPasswordValid(newPassword); err != nil {
+	if err := c.App.IsPasswordValid(newPassword); err != nil {
 		c.Err = err
 		return
 	}

api/user.go

@@ -873,7 +873,7 @@ func oauthToEmail(c *Context, w http.ResponseWriter, r *http.Request) {
 	props := model.MapFromJson(r.Body)
 
 	password := props["password"]
-	if err := utils.IsPasswordValid(password); err != nil {
+	if err := c.App.IsPasswordValid(password); err != nil {
 		c.Err = err
 		return
 	}
@@ -955,7 +955,7 @@ func ldapToEmail(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	emailPassword := props["email_password"]
-	if err := utils.IsPasswordValid(emailPassword); err != nil {
+	if err := c.App.IsPasswordValid(emailPassword); err != nil {
 		c.Err = err
 		return
 	}

app/authentication.go

@@ -11,6 +11,13 @@ import (
 	"github.com/mattermost/mattermost-server/utils"
 )
 
+func (a *App) IsPasswordValid(password string) *model.AppError {
+	if utils.IsLicensed() && *utils.License().Features.PasswordRequirements {
+		return utils.IsPasswordValidWithSettings(password, &a.Config().PasswordSettings)
+	}
+	return utils.IsPasswordValid(password)
+}
+
 func (a *App) CheckPasswordAndAllCriteria(user *model.User, password string, mfaToken string) *model.AppError {
 	if err := a.CheckUserAdditionalAuthenticationCriteria(user, mfaToken); err != nil {
 		return err

app/user.go

@@ -213,7 +213,7 @@ func (a *App) CreateUser(user *model.User) (*model.User, *model.AppError) {
 func (a *App) createUser(user *model.User) (*model.User, *model.AppError) {
 	user.MakeNonNil()
 
-	if err := utils.IsPasswordValid(user.Password); user.AuthService == "" && err != nil {
+	if err := a.IsPasswordValid(user.Password); user.AuthService == "" && err != nil {
 		return nil, err
 	}
 
@@ -1086,7 +1086,7 @@ func (a *App) UpdatePasswordByUserIdSendEmail(userId, newPassword, method string
 }
 
 func (a *App) UpdatePassword(user *model.User, newPassword string) *model.AppError {
-	if err := utils.IsPasswordValid(newPassword); err != nil {
+	if err := a.IsPasswordValid(newPassword); err != nil {
 		return err
 	}
 

utils/password.go

@@ -11,55 +11,55 @@ import (
 )
 
 func IsPasswordValid(password string) *model.AppError {
-	id := "model.user.is_valid.pwd"
+	if len(password) > model.PASSWORD_MAXIMUM_LENGTH || len(password) < model.PASSWORD_MINIMUM_LENGTH {
-	isError := false
+		return model.NewAppError("User.IsValid", "model.user.is_valid.pwd.app_error", map[string]interface{}{"Min": model.PASSWORD_MINIMUM_LENGTH}, "", http.StatusBadRequest)
-	min := model.PASSWORD_MINIMUM_LENGTH
+	}
-
+
-	if IsLicensed() && *License().Features.PasswordRequirements {
+	return nil
-		if len(password) < *Cfg.PasswordSettings.MinimumLength || len(password) > model.PASSWORD_MAXIMUM_LENGTH {
+}
-			isError = true
+
-		}
+func IsPasswordValidWithSettings(password string, settings *model.PasswordSettings) *model.AppError {
-
+	id := "model.user.is_valid.pwd"
-		if *Cfg.PasswordSettings.Lowercase {
+	isError := false
-			if !strings.ContainsAny(password, model.LOWERCASE_LETTERS) {
+
-				isError = true
+	if len(password) < *settings.MinimumLength || len(password) > model.PASSWORD_MAXIMUM_LENGTH {
-			}
+		isError = true
-
+	}
-			id = id + "_lowercase"
+
-		}
+	if *settings.Lowercase {
-
+		if !strings.ContainsAny(password, model.LOWERCASE_LETTERS) {
-		if *Cfg.PasswordSettings.Uppercase {
+			isError = true
-			if !strings.ContainsAny(password, model.UPPERCASE_LETTERS) {
+		}
-				isError = true
+
-			}
+		id = id + "_lowercase"
-
+	}
-			id = id + "_uppercase"
+
-		}
+	if *settings.Uppercase {
-
+		if !strings.ContainsAny(password, model.UPPERCASE_LETTERS) {
-		if *Cfg.PasswordSettings.Number {
+			isError = true
-			if !strings.ContainsAny(password, model.NUMBERS) {
+		}
-				isError = true
+
-			}
+		id = id + "_uppercase"
-
+	}
-			id = id + "_number"
+
-		}
+	if *settings.Number {
-
+		if !strings.ContainsAny(password, model.NUMBERS) {
-		if *Cfg.PasswordSettings.Symbol {
+			isError = true
-			if !strings.ContainsAny(password, model.SYMBOLS) {
+		}
-				isError = true
+
-			}
+		id = id + "_number"
-
+	}
-			id = id + "_symbol"
+
-		}
+	if *settings.Symbol {
-
+		if !strings.ContainsAny(password, model.SYMBOLS) {
-		min = *Cfg.PasswordSettings.MinimumLength
+			isError = true
-	} else if len(password) > model.PASSWORD_MAXIMUM_LENGTH || len(password) < model.PASSWORD_MINIMUM_LENGTH {
+		}
-		isError = true
+
-		min = model.PASSWORD_MINIMUM_LENGTH
+		id = id + "_symbol"
-	}
+	}
-
+
-	if isError {
+	if isError {
-		return model.NewAppError("User.IsValid", id+".app_error", map[string]interface{}{"Min": min}, "", http.StatusBadRequest)
+		return model.NewAppError("User.IsValid", id+".app_error", map[string]interface{}{"Min": *settings.MinimumLength}, "", http.StatusBadRequest)
 	}
 
 	return nil

utils/password_test.go

@@ -0,0 +1,127 @@
+package utils
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/mattermost/mattermost-server/model"
+)
+
+func TestIsPasswordValid(t *testing.T) {
+	for name, tc := range map[string]struct {
+		Password      string
+		ExpectedError string
+	}{
+		"Short": {
+			Password: strings.Repeat("x", model.PASSWORD_MINIMUM_LENGTH),
+		},
+		"Long": {
+			Password: strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH),
+		},
+		"TooShort": {
+			Password:      strings.Repeat("x", model.PASSWORD_MINIMUM_LENGTH-1),
+			ExpectedError: "model.user.is_valid.pwd.app_error",
+		},
+		"TooLong": {
+			Password:      strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH+1),
+			ExpectedError: "model.user.is_valid.pwd.app_error",
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			if err := IsPasswordValid(tc.Password); tc.ExpectedError == "" {
+				assert.Nil(t, err)
+			} else {
+				assert.Equal(t, tc.ExpectedError, err.Id)
+			}
+		})
+	}
+}
+
+func TestIsPasswordValidWithSettings(t *testing.T) {
+	for name, tc := range map[string]struct {
+		Password      string
+		Settings      *model.PasswordSettings
+		ExpectedError string
+	}{
+		"Short": {
+			Password: strings.Repeat("x", 3),
+			Settings: &model.PasswordSettings{
+				MinimumLength: model.NewInt(3),
+			},
+		},
+		"Long": {
+			Password: strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH),
+			Settings: &model.PasswordSettings{},
+		},
+		"TooShort": {
+			Password: strings.Repeat("x", 2),
+			Settings: &model.PasswordSettings{
+				MinimumLength: model.NewInt(3),
+			},
+			ExpectedError: "model.user.is_valid.pwd.app_error",
+		},
+		"TooLong": {
+			Password:      strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH+1),
+			Settings:      &model.PasswordSettings{},
+			ExpectedError: "model.user.is_valid.pwd.app_error",
+		},
+		"MissingLower": {
+			Password: "ASD123!@#",
+			Settings: &model.PasswordSettings{
+				Lowercase: model.NewBool(true),
+			},
+			ExpectedError: "model.user.is_valid.pwd_lowercase.app_error",
+		},
+		"MissingUpper": {
+			Password: "asd123!@#",
+			Settings: &model.PasswordSettings{
+				Uppercase: model.NewBool(true),
+			},
+			ExpectedError: "model.user.is_valid.pwd_uppercase.app_error",
+		},
+		"MissingNumber": {
+			Password: "asdASD!@#",
+			Settings: &model.PasswordSettings{
+				Number: model.NewBool(true),
+			},
+			ExpectedError: "model.user.is_valid.pwd_number.app_error",
+		},
+		"MissingSymbol": {
+			Password: "asdASD123",
+			Settings: &model.PasswordSettings{
+				Symbol: model.NewBool(true),
+			},
+			ExpectedError: "model.user.is_valid.pwd_symbol.app_error",
+		},
+		"MissingMultiple": {
+			Password: "asd",
+			Settings: &model.PasswordSettings{
+				Lowercase: model.NewBool(true),
+				Uppercase: model.NewBool(true),
+				Number:    model.NewBool(true),
+				Symbol:    model.NewBool(true),
+			},
+			ExpectedError: "model.user.is_valid.pwd_lowercase_uppercase_number_symbol.app_error",
+		},
+		"Everything": {
+			Password: "asdASD!@#123",
+			Settings: &model.PasswordSettings{
+				Lowercase: model.NewBool(true),
+				Uppercase: model.NewBool(true),
+				Number:    model.NewBool(true),
+				Symbol:    model.NewBool(true),
+			},
+		},
+	} {
+		tc.Settings.SetDefaults()
+		t.Run(name, func(t *testing.T) {
+			if err := IsPasswordValidWithSettings(tc.Password, tc.Settings); tc.ExpectedError == "" {
+				assert.Nil(t, err)
+			} else {
+				assert.Equal(t, tc.ExpectedError, err.Id)
+			}
+		})
+	}
+}