diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index 0c34c964de..1744a72b8e 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -162,6 +162,137 @@ func TestCreateUser(t *testing.T) { }, "Should not be able to define the RemoteID of a user through the API") } +func TestCreateUserPasswordValidation(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + ruser := model.User{ + Nickname: "Corey Hulen", + Password: "hello1", + Roles: model.SystemAdminRoleId + " " + model.SystemUserRoleId, + EmailVerified: true, + } + + for name, tc := range map[string]struct { + Password string + Settings *model.PasswordSettings + ExpectedError string + }{ + "Short": { + Password: strings.Repeat("x", 5), + Settings: &model.PasswordSettings{ + MinimumLength: model.NewPointer(5), + Lowercase: model.NewPointer(false), + Uppercase: model.NewPointer(false), + Number: model.NewPointer(false), + Symbol: model.NewPointer(false), + }, + }, + "Long": { + Password: strings.Repeat("x", model.PasswordMaximumLength), + Settings: &model.PasswordSettings{ + Lowercase: model.NewPointer(false), + Uppercase: model.NewPointer(false), + Number: model.NewPointer(false), + Symbol: model.NewPointer(false), + }, + }, + "TooShort": { + Password: strings.Repeat("x", 2), + Settings: &model.PasswordSettings{ + MinimumLength: model.NewPointer(5), + Lowercase: model.NewPointer(false), + Uppercase: model.NewPointer(false), + Number: model.NewPointer(false), + Symbol: model.NewPointer(false), + }, + ExpectedError: "model.user.is_valid.pwd_min_length.app_error", + }, + "TooLong": { + Password: strings.Repeat("x", model.PasswordMaximumLength+1), + Settings: &model.PasswordSettings{ + Lowercase: model.NewPointer(false), + Uppercase: model.NewPointer(false), + Number: model.NewPointer(false), + Symbol: model.NewPointer(false), + }, + ExpectedError: "model.user.is_valid.pwd_max_length.app_error", + }, + "MissingLower": { + Password: "AAAAAAAAAAASD123!@#", + Settings: &model.PasswordSettings{ + Lowercase: model.NewPointer(true), + Uppercase: model.NewPointer(false), + Number: model.NewPointer(false), + Symbol: model.NewPointer(false), + }, + ExpectedError: "model.user.is_valid.pwd_lowercase.app_error", + }, + "MissingUpper": { + Password: "aaaaaaaaaaaaasd123!@#", + Settings: &model.PasswordSettings{ + Uppercase: model.NewPointer(true), + Lowercase: model.NewPointer(false), + Number: model.NewPointer(false), + Symbol: model.NewPointer(false), + }, + ExpectedError: "model.user.is_valid.pwd_uppercase.app_error", + }, + "MissingNumber": { + Password: "asasdasdsadASD!@#", + Settings: &model.PasswordSettings{ + Number: model.NewPointer(true), + Lowercase: model.NewPointer(false), + Uppercase: model.NewPointer(false), + Symbol: model.NewPointer(false), + }, + ExpectedError: "model.user.is_valid.pwd_number.app_error", + }, + "MissingSymbol": { + Password: "asdasdasdasdasdASD123", + Settings: &model.PasswordSettings{ + Symbol: model.NewPointer(true), + Lowercase: model.NewPointer(false), + Uppercase: model.NewPointer(false), + Number: model.NewPointer(false), + }, + ExpectedError: "model.user.is_valid.pwd_symbol.app_error", + }, + "MissingMultiple": { + Password: "asdasdasdasdasdasd", + Settings: &model.PasswordSettings{ + Lowercase: model.NewPointer(true), + Uppercase: model.NewPointer(true), + Number: model.NewPointer(true), + Symbol: model.NewPointer(true), + }, + ExpectedError: "model.user.is_valid.pwd_uppercase_number_symbol.app_error", + }, + "Everything": { + Password: "asdASD!@#123", + Settings: &model.PasswordSettings{ + Lowercase: model.NewPointer(true), + Uppercase: model.NewPointer(true), + Number: model.NewPointer(true), + Symbol: model.NewPointer(true), + }, + }, + } { + t.Run(name, func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.PasswordSettings = *tc.Settings }) + ruser.Email = th.GenerateTestEmail() + ruser.Password = tc.Password + ruser.Username = GenerateTestUsername() + if _, resp, err := th.Client.CreateUser(context.Background(), &ruser); tc.ExpectedError == "" { + assert.NoError(t, err) + } else { + CheckErrorID(t, err, tc.ExpectedError) + CheckBadRequestStatus(t, resp) + } + }) + } +} + func TestCreateUserAudit(t *testing.T) { logFile, err := os.CreateTemp("", "adv.log") require.NoError(t, err) diff --git a/server/channels/app/user.go b/server/channels/app/user.go index dab6c0837c..abc7b3c101 100644 --- a/server/channels/app/user.go +++ b/server/channels/app/user.go @@ -255,7 +255,7 @@ func (a *App) createUserOrGuest(c request.CTX, user *model.User, guest bool) (*m case errors.Is(nErr, users.AcceptedDomainError): return nil, model.NewAppError("createUserOrGuest", "api.user.create_user.accepted_domain.app_error", nil, "", http.StatusBadRequest).Wrap(nErr) case errors.As(nErr, &nfErr): - return nil, model.NewAppError("createUserOrGuest", "api.user.check_user_password.invalid.app_error", nil, "", http.StatusBadRequest).Wrap(nErr) + return nil, model.NewAppError("createUserOrGuest", nfErr.Id(), map[string]interface{}{"Min": *a.Config().PasswordSettings.MinimumLength}, "", http.StatusBadRequest) case errors.Is(nErr, users.UserStoreIsEmptyError): return nil, model.NewAppError("createUserOrGuest", "app.user.store_is_empty.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr) case errors.As(nErr, &invErr): diff --git a/server/channels/app/users/password.go b/server/channels/app/users/password.go index b96ffbf373..4282c7e650 100644 --- a/server/channels/app/users/password.go +++ b/server/channels/app/users/password.go @@ -37,47 +37,48 @@ func (us *UserService) isPasswordValid(password string) error { func IsPasswordValidWithSettings(password string, settings *model.PasswordSettings) error { id := "model.user.is_valid.pwd" isError := false + isMinMaxError := false if len(password) < *settings.MinimumLength { isError = true + isMinMaxError = true id = id + "_min_length" } if len(password) > model.PasswordMaximumLength { isError = true + isMinMaxError = true id = id + "_max_length" } - if *settings.Lowercase { - if !strings.ContainsAny(password, model.LowercaseLetters) { - isError = true + if !isMinMaxError { + if *settings.Lowercase { + if !strings.ContainsAny(password, model.LowercaseLetters) { + isError = true + id = id + "_lowercase" + } } - id = id + "_lowercase" - } - - if *settings.Uppercase { - if !strings.ContainsAny(password, model.UppercaseLetters) { - isError = true + if *settings.Uppercase { + if !strings.ContainsAny(password, model.UppercaseLetters) { + isError = true + id = id + "_uppercase" + } } - id = id + "_uppercase" - } - - if *settings.Number { - if !strings.ContainsAny(password, model.NUMBERS) { - isError = true + if *settings.Number { + if !strings.ContainsAny(password, model.NUMBERS) { + isError = true + id = id + "_number" + } } - id = id + "_number" - } - - if *settings.Symbol { - if !strings.ContainsAny(password, model.SYMBOLS) { - isError = true + if *settings.Symbol { + if !strings.ContainsAny(password, model.SYMBOLS) { + isError = true + id = id + "_symbol" + } } - - id = id + "_symbol" } if isError { diff --git a/server/channels/app/users/password_test.go b/server/channels/app/users/password_test.go index 2aa765483f..971cc38173 100644 --- a/server/channels/app/users/password_test.go +++ b/server/channels/app/users/password_test.go @@ -107,7 +107,7 @@ func TestIsPasswordValidWithSettings(t *testing.T) { Number: model.NewPointer(true), Symbol: model.NewPointer(true), }, - ExpectedError: "model.user.is_valid.pwd_lowercase_uppercase_number_symbol.app_error", + ExpectedError: "model.user.is_valid.pwd_uppercase_number_symbol.app_error", }, "Everything": { Password: "asdASD!@#123",