[MM-15490] Rework default password requirements (#10844)

* Rework default password requirements * Update API Test Lib Default User PW * Remove unused function; Disable password reqs in dev mode * Disable strict password requirements for unit tests * Update unit tests
2025-02-25 18:55:24 -06:00 · 2019-05-21 20:03:36 +02:00
parent 0b6acaa9ba
commit d269891476
9 changed files with 89 additions and 57 deletions
--- a/api4/apitestlib.go
+++ b/api4/apitestlib.go
@@ -108,6 +108,15 @@ func setupTestHelper(enterprise bool, updateConfig func(*model.Config)) *TestHel

 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableOpenServer = true })

+	// Disable strict password requirements for test
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.PasswordSettings.MinimumLength = 5
+		*cfg.PasswordSettings.Lowercase = false
+		*cfg.PasswordSettings.Uppercase = false
+		*cfg.PasswordSettings.Symbol = false
+		*cfg.PasswordSettings.Number = false
+	})
+
 	if enterprise {
 		th.App.SetLicense(model.NewTestLicense())
 	} else {
@@ -274,7 +283,7 @@ func (me *TestHelper) CreateUserWithClient(client *model.Client4) *model.User {
 		Nickname:  "nn_" + id,
 		FirstName: "f_" + id,
 		LastName:  "l_" + id,
-		Password:  "Password1",
+		Password:  "Pa$$word11",
 	}

 	utils.DisableDebugLogForTest()
@@ -283,7 +292,7 @@ func (me *TestHelper) CreateUserWithClient(client *model.Client4) *model.User {
 		panic(response.Error)
 	}

-	ruser.Password = "Password1"
+	ruser.Password = "Pa$$word11"
 	store.Must(me.App.Srv.Store.User().VerifyEmail(ruser.Id, ruser.Email))
 	utils.EnableDebugLogForTest()
 	return ruser
--- a/app/authentication.go
+++ b/app/authentication.go
@@ -37,6 +37,11 @@ func (tl TokenLocation) String() string {
 }

 func (a *App) IsPasswordValid(password string) *model.AppError {
+
+	if *a.Config().ServiceSettings.EnableDeveloper {
+		return nil
+	}
+
 	return utils.IsPasswordValidWithSettings(password, &a.Config().PasswordSettings)
 }

--- a/app/helper_test.go
+++ b/app/helper_test.go
@@ -71,6 +71,15 @@ func setupTestHelper(enterprise bool, tb testing.TB) *TestHelper {

 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableOpenServer = true })

+	// Disable strict password requirements for test
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.PasswordSettings.MinimumLength = 5
+		*cfg.PasswordSettings.Lowercase = false
+		*cfg.PasswordSettings.Uppercase = false
+		*cfg.PasswordSettings.Symbol = false
+		*cfg.PasswordSettings.Number = false
+	})
+
 	if enterprise {
 		th.App.SetLicense(model.NewTestLicense())
 	} else {
--- a/cmd/mattermost/commands/cmdtestlib.go
+++ b/cmd/mattermost/commands/cmdtestlib.go
@@ -81,6 +81,14 @@ func (h *testHelper) ConfigPath() string {
 // SetConfig replaces the configuration passed to a running command.
 func (h *testHelper) SetConfig(config *model.Config) {
 	config.SqlSettings = *mainHelper.GetSqlSettings()
+
+	// Disable strict password requirements for test
+	*config.PasswordSettings.MinimumLength = 5
+	*config.PasswordSettings.Lowercase = false
+	*config.PasswordSettings.Uppercase = false
+	*config.PasswordSettings.Symbol = false
+	*config.PasswordSettings.Number = false
+
 	h.config = config

 	if err := ioutil.WriteFile(h.configFilePath, []byte(config.ToJson()), 0600); err != nil {
--- a/config/default.json
+++ b/config/default.json
@@ -159,11 +159,11 @@
        "FileLocation": ""
    },
    "PasswordSettings": {
-        "MinimumLength": 5,
-        "Lowercase": false,
-        "Number": false,
-        "Uppercase": false,
-        "Symbol": false
+        "MinimumLength": 10,
+        "Lowercase": true,
+        "Number": true,
+        "Uppercase": true,
+        "Symbol": true
    },
    "FileSettings": {
        "EnableFileAttachments": true,
--- a/model/config.go
+++ b/model/config.go
@@ -962,23 +962,23 @@ type PasswordSettings struct {

 func (s *PasswordSettings) SetDefaults() {
 	if s.MinimumLength == nil {
-		s.MinimumLength = NewInt(PASSWORD_MINIMUM_LENGTH)
+		s.MinimumLength = NewInt(10)
 	}

 	if s.Lowercase == nil {
-		s.Lowercase = NewBool(false)
+		s.Lowercase = NewBool(true)
 	}

 	if s.Number == nil {
-		s.Number = NewBool(false)
+		s.Number = NewBool(true)
 	}

 	if s.Uppercase == nil {
-		s.Uppercase = NewBool(false)
+		s.Uppercase = NewBool(true)
 	}

 	if s.Symbol == nil {
-		s.Symbol = NewBool(false)
+		s.Symbol = NewBool(true)
 	}
 }

--- a/utils/password.go
+++ b/utils/password.go
@@ -10,14 +10,6 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )

-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
--- a/utils/password_test.go
+++ b/utils/password_test.go
@@ -9,36 +9,6 @@ import (
 	"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
@@ -49,54 +19,84 @@ func TestIsPasswordValidWithSettings(t *testing.T) {
 			Password: strings.Repeat("x", 3),
 			Settings: &model.PasswordSettings{
 				MinimumLength: model.NewInt(3),
+				Lowercase: model.NewBool(false),
+				Uppercase: model.NewBool(false),
+				Number: model.NewBool(false),
+				Symbol: model.NewBool(false),
 			},
 		},
 		"Long": {
 			Password: strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH),
-			Settings: &model.PasswordSettings{},
+			Settings: &model.PasswordSettings{
+				Lowercase: model.NewBool(false),
+				Uppercase: model.NewBool(false),
+				Number: model.NewBool(false),
+				Symbol: model.NewBool(false),
+			},
 		},
 		"TooShort": {
 			Password: strings.Repeat("x", 2),
 			Settings: &model.PasswordSettings{
 				MinimumLength: model.NewInt(3),
+				Lowercase: model.NewBool(false),
+				Uppercase: model.NewBool(false),
+				Number: model.NewBool(false),
+				Symbol: model.NewBool(false),
 			},
 			ExpectedError: "model.user.is_valid.pwd.app_error",
 		},
 		"TooLong": {
 			Password:      strings.Repeat("x", model.PASSWORD_MAXIMUM_LENGTH+1),
-			Settings:      &model.PasswordSettings{},
+			Settings:      &model.PasswordSettings{
+				Lowercase: model.NewBool(false),
+				Uppercase: model.NewBool(false),
+				Number: model.NewBool(false),
+				Symbol: model.NewBool(false),
+			},
 			ExpectedError: "model.user.is_valid.pwd.app_error",
 		},
 		"MissingLower": {
-			Password: "ASD123!@#",
+			Password: "AAAAAAAAAAASD123!@#",
 			Settings: &model.PasswordSettings{
 				Lowercase: model.NewBool(true),
+				Uppercase: model.NewBool(false),
+				Number: model.NewBool(false),
+				Symbol: model.NewBool(false),
 			},
 			ExpectedError: "model.user.is_valid.pwd_lowercase.app_error",
 		},
 		"MissingUpper": {
-			Password: "asd123!@#",
+			Password: "aaaaaaaaaaaaasd123!@#",
 			Settings: &model.PasswordSettings{
 				Uppercase: model.NewBool(true),
+				Lowercase: model.NewBool(false),
+				Number: model.NewBool(false),
+				Symbol: model.NewBool(false),
 			},
 			ExpectedError: "model.user.is_valid.pwd_uppercase.app_error",
 		},
 		"MissingNumber": {
-			Password: "asdASD!@#",
+			Password: "asasdasdsadASD!@#",
 			Settings: &model.PasswordSettings{
 				Number: model.NewBool(true),
+				Lowercase: model.NewBool(false),
+				Uppercase: model.NewBool(false),
+				Symbol: model.NewBool(false),
 			},
 			ExpectedError: "model.user.is_valid.pwd_number.app_error",
 		},
 		"MissingSymbol": {
-			Password: "asdASD123",
+			Password: "asdasdasdasdasdASD123",
 			Settings: &model.PasswordSettings{
 				Symbol: model.NewBool(true),
+				Lowercase: model.NewBool(false),
+				Uppercase: model.NewBool(false),
+				Number: model.NewBool(false),
 			},
 			ExpectedError: "model.user.is_valid.pwd_symbol.app_error",
 		},
 		"MissingMultiple": {
-			Password: "asd",
+			Password: "asdasdasdasdasdasd",
 			Settings: &model.PasswordSettings{
 				Lowercase: model.NewBool(true),
 				Uppercase: model.NewBool(true),
--- a/web/web_test.go
+++ b/web/web_test.go
@@ -66,6 +66,15 @@ func Setup() *TestHelper {
 	}
 	a.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ListenAddress = prevListenAddress })

+	// Disable strict password requirements for test
+	a.UpdateConfig(func(cfg *model.Config) {
+		*cfg.PasswordSettings.MinimumLength = 5
+		*cfg.PasswordSettings.Lowercase = false
+		*cfg.PasswordSettings.Uppercase = false
+		*cfg.PasswordSettings.Symbol = false
+		*cfg.PasswordSettings.Number = false
+	})
+
 	web := New(s, s.AppOptions, s.Router)
 	URL = fmt.Sprintf("http://localhost:%v", a.Srv.ListenAddr.Port)
 	ApiClient = model.NewAPIv4Client(URL)