From d1cee022471f43decafc4da0bff7892ff52f0664 Mon Sep 17 00:00:00 2001 From: Chris Date: Mon, 20 Nov 2017 11:04:04 -0600 Subject: Refactor password validation and config defaults (#7859) * refactor password validation and config defaults * reorder config lines for clarity --- utils/password.go | 62 ++++++++++++------------ utils/password_test.go | 127 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 31 deletions(-) create mode 100644 utils/password_test.go (limited to 'utils') diff --git a/utils/password.go b/utils/password.go index f64d6f6ab..b24d6c2ec 100644 --- a/utils/password.go +++ b/utils/password.go @@ -11,55 +11,55 @@ import ( ) func IsPasswordValid(password string) *model.AppError { + if len(password) > model.PASSWORD_MAXIMUM_LENGTH || len(password) < model.PASSWORD_MINIMUM_LENGTH { + return model.NewAppError("User.IsValid", "model.user.is_valid.pwd.app_error", map[string]interface{}{"Min": model.PASSWORD_MINIMUM_LENGTH}, "", http.StatusBadRequest) + } + + return nil +} + +func IsPasswordValidWithSettings(password string, settings *model.PasswordSettings) *model.AppError { id := "model.user.is_valid.pwd" isError := false - min := model.PASSWORD_MINIMUM_LENGTH - if IsLicensed() && *License().Features.PasswordRequirements { - if len(password) < *Cfg.PasswordSettings.MinimumLength || len(password) > model.PASSWORD_MAXIMUM_LENGTH { - isError = true - } - - if *Cfg.PasswordSettings.Lowercase { - 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) { + isError = true } - if *Cfg.PasswordSettings.Uppercase { - 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) { + isError = true } - if *Cfg.PasswordSettings.Number { - if !strings.ContainsAny(password, model.NUMBERS) { - isError = true - } + id = id + "_uppercase" + } - id = id + "_number" + if *settings.Number { + if !strings.ContainsAny(password, model.NUMBERS) { + isError = true } - if *Cfg.PasswordSettings.Symbol { - if !strings.ContainsAny(password, model.SYMBOLS) { - isError = true - } + id = id + "_number" + } - id = id + "_symbol" + if *settings.Symbol { + if !strings.ContainsAny(password, model.SYMBOLS) { + isError = true } - min = *Cfg.PasswordSettings.MinimumLength - } 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 { - 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 diff --git a/utils/password_test.go b/utils/password_test.go new file mode 100644 index 000000000..4daf25018 --- /dev/null +++ b/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) + } + }) + } +} -- cgit v1.2.3-1-g7c22