diff --git a/app/import_functions.go b/app/import_functions.go index 960c9520ba..3f3f00c9d3 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -337,8 +337,12 @@ func (a *App) importUser(data *UserImportData, dryRun bool) *model.AppError { password = *data.Password authData = nil } else { + var err error // If no AuthData or Password is specified, we must generate a password. - password = model.GeneratePassword(*a.Config().PasswordSettings.MinimumLength) + password, err = generatePassword(*a.Config().PasswordSettings.MinimumLength) + if err != nil { + return model.NewAppError("importUser", "app.import.generate_password.app_error", nil, err.Error(), http.StatusInternalServerError) + } authData = nil } diff --git a/app/import_utils.go b/app/import_utils.go new file mode 100644 index 0000000000..c27da4b1c8 --- /dev/null +++ b/app/import_utils.go @@ -0,0 +1,60 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "crypto/rand" + "math/big" +) + +const ( + passwordSpecialChars = "!$%^&*(),." + passwordNumbers = "0123456789" + passwordUpperCaseLetters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + passwordLowerCaseLetters = "abcdefghijklmnopqrstuvwxyz" + passwordAllChars = passwordSpecialChars + passwordNumbers + passwordUpperCaseLetters + passwordLowerCaseLetters +) + +func randInt(max int) (int, error) { + val, err := rand.Int(rand.Reader, big.NewInt(int64(max))) + if err != nil { + return 0, err + } + return int(val.Int64()), nil +} + +func generatePassword(minimumLength int) (string, error) { + upperIdx, err := randInt(len(passwordUpperCaseLetters)) + if err != nil { + return "", err + } + numberIdx, err := randInt(len(passwordNumbers)) + if err != nil { + return "", err + } + lowerIdx, err := randInt(len(passwordLowerCaseLetters)) + if err != nil { + return "", err + } + specialIdx, err := randInt(len(passwordSpecialChars)) + if err != nil { + return "", err + } + + // Make sure we are guaranteed at least one of each type to meet any possible password complexity requirements. + password := string([]rune(passwordUpperCaseLetters)[upperIdx]) + + string([]rune(passwordNumbers)[numberIdx]) + + string([]rune(passwordLowerCaseLetters)[lowerIdx]) + + string([]rune(passwordSpecialChars)[specialIdx]) + + for len(password) < minimumLength { + i, err := randInt(len(passwordAllChars)) + if err != nil { + return "", err + } + password = password + string([]rune(passwordAllChars)[i]) + } + + return password, nil +} diff --git a/app/import_utils_test.go b/app/import_utils_test.go new file mode 100644 index 0000000000..e2b8b75468 --- /dev/null +++ b/app/import_utils_test.go @@ -0,0 +1,41 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGeneratePassword(t *testing.T) { + t.Run("Should be the minimum length or 4, whichever is less", func(t *testing.T) { + password1, err := generatePassword(5) + require.NoError(t, err) + assert.Len(t, password1, 5) + password2, err := generatePassword(10) + require.NoError(t, err) + assert.Len(t, password2, 10) + password3, err := generatePassword(1) + require.NoError(t, err) + assert.Len(t, password3, 4) + }) + + t.Run("Should contain at least one of symbols, upper case, lower case and numbers", func(t *testing.T) { + password, err := generatePassword(4) + require.NoError(t, err) + require.Len(t, password, 4) + assert.Contains(t, []rune(passwordUpperCaseLetters), []rune(password)[0]) + assert.Contains(t, []rune(passwordNumbers), []rune(password)[1]) + assert.Contains(t, []rune(passwordLowerCaseLetters), []rune(password)[2]) + assert.Contains(t, []rune(passwordSpecialChars), []rune(password)[3]) + }) + + t.Run("Should not fail on concurrent calls", func(t *testing.T) { + for i := 0; i < 10; i++ { + go generatePassword(10) + } + }) +} diff --git a/i18n/en.json b/i18n/en.json index bd225ef55a..44c02d55e9 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4422,6 +4422,10 @@ "id": "app.import.emoji.bad_file.error", "translation": "Error reading import emoji image file. Emoji with name: \"{{.EmojiName}}\"" }, + { + "id": "app.import.generate_password.app_error", + "translation": "Error generating password." + }, { "id": "app.import.get_teams_by_names.some_teams_not_found.error", "translation": "Some teams not found" diff --git a/model/user.go b/model/user.go index e986d96c6f..0723e35f64 100644 --- a/model/user.go +++ b/model/user.go @@ -9,13 +9,10 @@ import ( "fmt" "io" "io/ioutil" - "math/rand" "net/http" "regexp" "sort" "strings" - "sync" - "time" "unicode/utf8" "golang.org/x/crypto/bcrypt" @@ -940,41 +937,3 @@ func UsersWithGroupsAndCountFromJson(data io.Reader) *UsersWithGroupsAndCount { json.Unmarshal(bodyBytes, uwg) return uwg } - -//msgp:ignore lockedRand -type lockedRand struct { - mu sync.Mutex - rn *rand.Rand -} - -func (r *lockedRand) Intn(n int) int { - r.mu.Lock() - m := r.rn.Intn(n) - r.mu.Unlock() - return m -} - -var passwordRandom = lockedRand{ - rn: rand.New(rand.NewSource(time.Now().Unix())), -} - -var passwordSpecialChars = "!$%^&*(),." -var passwordNumbers = "0123456789" -var passwordUpperCaseLetters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" -var passwordLowerCaseLetters = "abcdefghijklmnopqrstuvwxyz" -var passwordAllChars = passwordSpecialChars + passwordNumbers + passwordUpperCaseLetters + passwordLowerCaseLetters - -func GeneratePassword(minimumLength int) string { - // Make sure we are guaranteed at least one of each type to meet any possible password complexity requirements. - password := string([]rune(passwordUpperCaseLetters)[passwordRandom.Intn(len(passwordUpperCaseLetters))]) + - string([]rune(passwordNumbers)[passwordRandom.Intn(len(passwordNumbers))]) + - string([]rune(passwordLowerCaseLetters)[passwordRandom.Intn(len(passwordLowerCaseLetters))]) + - string([]rune(passwordSpecialChars)[passwordRandom.Intn(len(passwordSpecialChars))]) - - for len(password) < minimumLength { - i := passwordRandom.Intn(len(passwordAllChars)) - password = password + string([]rune(passwordAllChars)[i]) - } - - return password -} diff --git a/model/user_test.go b/model/user_test.go index 10efe33456..e8870a7580 100644 --- a/model/user_test.go +++ b/model/user_test.go @@ -5,7 +5,6 @@ package model import ( "fmt" - "math/rand" "net/http" "strings" "testing" @@ -351,33 +350,3 @@ func TestUserSlice(t *testing.T) { assert.Equal(t, 1, len(nonBotUsers)) }) } - -func TestGeneratePassword(t *testing.T) { - passwordRandom = lockedRand{ - rn: rand.New(rand.NewSource(12345)), - } - - t.Run("Should be the minimum length or 4, whichever is less", func(t *testing.T) { - password1 := GeneratePassword(5) - assert.Len(t, password1, 5) - password2 := GeneratePassword(10) - assert.Len(t, password2, 10) - password3 := GeneratePassword(1) - assert.Len(t, password3, 4) - }) - - t.Run("Should contain at least one of symbols, upper case, lower case and numbers", func(t *testing.T) { - password := GeneratePassword(4) - require.Len(t, password, 4) - assert.Contains(t, []rune(passwordUpperCaseLetters), []rune(password)[0]) - assert.Contains(t, []rune(passwordNumbers), []rune(password)[1]) - assert.Contains(t, []rune(passwordLowerCaseLetters), []rune(password)[2]) - assert.Contains(t, []rune(passwordSpecialChars), []rune(password)[3]) - }) - - t.Run("Should not fail on concurrent calls", func(t *testing.T) { - for i := 0; i < 10; i++ { - go GeneratePassword(10) - } - }) -}