Browse Source

Password Complexity Checks (#6230)

Add password complexity checks. The default settings require a lowercase, uppercase, number and a special character within passwords.

Co-Authored-By: T-M-A <maxim.tkachenko@gmail.com>
Co-Authored-By: Lanre Adelowo <adelowomailbox@gmail.com>
Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>
Co-Authored-By: Lauris BH <lauris@nix.lv>
for-closed-social
Maxim Tkachenko 5 years ago
committed by zeripath
parent
commit
db657192d0
11 changed files with 207 additions and 37 deletions
  1. +10
    -9
      cmd/admin.go
  2. +4
    -1
      custom/conf/app.ini.sample
  3. +5
    -0
      docs/content/doc/advanced/config-cheat-sheet.en-us.md
  4. +73
    -0
      modules/password/password.go
  5. +22
    -0
      modules/setting/setting.go
  6. +1
    -0
      options/locale/locale_en-US.ini
  7. +9
    -1
      routers/admin/users.go
  8. +13
    -1
      routers/api/v1/admin/user.go
  9. +6
    -5
      routers/user/auth.go
  10. +3
    -0
      routers/user/setting/account.go
  11. +61
    -20
      routers/user/setting/account_test.go

+ 10
- 9
cmd/admin.go View File

@ -13,9 +13,9 @@ import (
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/auth/oauth2" "code.gitea.io/gitea/modules/auth/oauth2"
"code.gitea.io/gitea/modules/generate"
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
pwd "code.gitea.io/gitea/modules/password"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"github.com/urfave/cli" "github.com/urfave/cli"
@ -233,7 +233,9 @@ func runChangePassword(c *cli.Context) error {
if err := initDB(); err != nil { if err := initDB(); err != nil {
return err return err
} }
if !pwd.IsComplexEnough(c.String("password")) {
return errors.New("Password does not meet complexity requirements")
}
uname := c.String("username") uname := c.String("username")
user, err := models.GetUserByName(uname) user, err := models.GetUserByName(uname)
if err != nil { if err != nil {
@ -243,6 +245,7 @@ func runChangePassword(c *cli.Context) error {
return err return err
} }
user.HashPassword(c.String("password")) user.HashPassword(c.String("password"))
if err := models.UpdateUserCols(user, "passwd", "salt"); err != nil { if err := models.UpdateUserCols(user, "passwd", "salt"); err != nil {
return err return err
} }
@ -275,26 +278,24 @@ func runCreateUser(c *cli.Context) error {
fmt.Fprintf(os.Stderr, "--name flag is deprecated. Use --username instead.\n") fmt.Fprintf(os.Stderr, "--name flag is deprecated. Use --username instead.\n")
} }
var password string
if err := initDB(); err != nil {
return err
}
var password string
if c.IsSet("password") { if c.IsSet("password") {
password = c.String("password") password = c.String("password")
} else if c.IsSet("random-password") { } else if c.IsSet("random-password") {
var err error var err error
password, err = generate.GetRandomString(c.Int("random-password-length"))
password, err = pwd.Generate(c.Int("random-password-length"))
if err != nil { if err != nil {
return err return err
} }
fmt.Printf("generated random password is '%s'\n", password) fmt.Printf("generated random password is '%s'\n", password)
} else { } else {
return errors.New("must set either password or random-password flag") return errors.New("must set either password or random-password flag")
} }
if err := initDB(); err != nil {
return err
}
// always default to true // always default to true
var changePassword = true var changePassword = true

+ 4
- 1
custom/conf/app.ini.sample View File

@ -332,6 +332,9 @@ MIN_PASSWORD_LENGTH = 6
IMPORT_LOCAL_PATHS = false IMPORT_LOCAL_PATHS = false
; Set to true to prevent all users (including admin) from creating custom git hooks ; Set to true to prevent all users (including admin) from creating custom git hooks
DISABLE_GIT_HOOKS = false DISABLE_GIT_HOOKS = false
;Comma separated list of character classes required to pass minimum complexity.
;If left empty or no valid values are specified, the default values (`lower,upper,digit,spec`) will be used.
PASSWORD_COMPLEXITY = lower,upper,digit,spec
; Password Hash algorithm, either "pbkdf2", "argon2", "scrypt" or "bcrypt" ; Password Hash algorithm, either "pbkdf2", "argon2", "scrypt" or "bcrypt"
PASSWORD_HASH_ALGO = pbkdf2 PASSWORD_HASH_ALGO = pbkdf2
; Set false to allow JavaScript to read CSRF cookie ; Set false to allow JavaScript to read CSRF cookie
@ -415,7 +418,7 @@ DEFAULT_ALLOW_CREATE_ORGANIZATION = true
; Public is for everyone ; Public is for everyone
DEFAULT_ORG_VISIBILITY = public DEFAULT_ORG_VISIBILITY = public
; Default value for DefaultOrgMemberVisible ; Default value for DefaultOrgMemberVisible
; True will make the membership of the users visible when added to the organisation
; True will make the membership of the users visible when added to the organisation
DEFAULT_ORG_MEMBER_VISIBLE = false DEFAULT_ORG_MEMBER_VISIBLE = false
; Default value for EnableDependencies ; Default value for EnableDependencies
; Repositories will use dependencies by default depending on this setting ; Repositories will use dependencies by default depending on this setting

+ 5
- 0
docs/content/doc/advanced/config-cheat-sheet.en-us.md View File

@ -208,6 +208,11 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
- `INTERNAL_TOKEN_URI`: **<empty>**: Instead of defining internal token in the configuration, this configuration option can be used to give Gitea a path to a file that contains the internal token (example value: `file:/etc/gitea/internal_token`) - `INTERNAL_TOKEN_URI`: **<empty>**: Instead of defining internal token in the configuration, this configuration option can be used to give Gitea a path to a file that contains the internal token (example value: `file:/etc/gitea/internal_token`)
- `PASSWORD_HASH_ALGO`: **pbkdf2**: The hash algorithm to use \[pbkdf2, argon2, scrypt, bcrypt\]. - `PASSWORD_HASH_ALGO`: **pbkdf2**: The hash algorithm to use \[pbkdf2, argon2, scrypt, bcrypt\].
- `CSRF_COOKIE_HTTP_ONLY`: **true**: Set false to allow JavaScript to read CSRF cookie. - `CSRF_COOKIE_HTTP_ONLY`: **true**: Set false to allow JavaScript to read CSRF cookie.
- `PASSWORD_COMPLEXITY`: **lower,upper,digit,spec**: Comma separated list of character classes required to pass minimum complexity. If left empty or no valid values are specified, the default values will be used. Possible values are:
- lower - use one or more lower latin characters
- upper - use one or more upper latin characters
- digit - use one or more digits
- spec - use one or more special characters as ``][!"#$%&'()*+,./:;<=>?@\^_{|}~`-`` and space symbol.
## OpenID (`openid`) ## OpenID (`openid`)

+ 73
- 0
modules/password/password.go View File

@ -0,0 +1,73 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.
package password
import (
"crypto/rand"
"math/big"
"regexp"
"sync"
"code.gitea.io/gitea/modules/setting"
)
var matchComplexities = map[string]regexp.Regexp{}
var matchComplexityOnce sync.Once
var validChars string
var validComplexities = map[string]string{
"lower": "abcdefghijklmnopqrstuvwxyz",
"upper": "ABCDEFGHIJKLMNOPQRSTUVWXYZ",
"digit": "0123456789",
"spec": `][ !"#$%&'()*+,./:;<=>?@\^_{|}~` + "`-",
}
// NewComplexity for preparation
func NewComplexity() {
matchComplexityOnce.Do(func() {
if len(setting.PasswordComplexity) > 0 {
for key, val := range setting.PasswordComplexity {
matchComplexity := regexp.MustCompile(val)
matchComplexities[key] = *matchComplexity
validChars += validComplexities[key]
}
} else {
for _, val := range validComplexities {
validChars += val
}
}
})
}
// IsComplexEnough return True if password is Complexity
func IsComplexEnough(pwd string) bool {
if len(setting.PasswordComplexity) > 0 {
NewComplexity()
for _, val := range matchComplexities {
if !val.MatchString(pwd) {
return false
}
}
}
return true
}
// Generate a random password
func Generate(n int) (string, error) {
NewComplexity()
buffer := make([]byte, n)
max := big.NewInt(int64(len(validChars)))
for {
for j := 0; j < n; j++ {
rnd, err := rand.Int(rand.Reader, max)
if err != nil {
return "", err
}
buffer[j] = validChars[rnd.Int64()]
}
if IsComplexEnough(string(buffer)) && string(buffer[0]) != " " && string(buffer[n-1]) != " " {
return string(buffer), nil
}
}
}

+ 22
- 0
modules/setting/setting.go View File

@ -146,6 +146,7 @@ var (
MinPasswordLength int MinPasswordLength int
ImportLocalPaths bool ImportLocalPaths bool
DisableGitHooks bool DisableGitHooks bool
PasswordComplexity map[string]string
PasswordHashAlgo string PasswordHashAlgo string
// UI settings // UI settings
@ -774,6 +775,27 @@ func NewContext() {
InternalToken = loadInternalToken(sec) InternalToken = loadInternalToken(sec)
var dictPC = map[string]string{
"lower": "[a-z]+",
"upper": "[A-Z]+",
"digit": "[0-9]+",
"spec": `][ !"#$%&'()*+,./:;<=>?@\\^_{|}~` + "`-",
}
PasswordComplexity = make(map[string]string)
cfgdata := sec.Key("PASSWORD_COMPLEXITY").Strings(",")
for _, y := range cfgdata {
ts := strings.TrimSpace(y)
for a := range dictPC {
if strings.ToLower(ts) == a {
PasswordComplexity[ts] = dictPC[ts]
break
}
}
}
if len(PasswordComplexity) == 0 {
PasswordComplexity = dictPC
}
sec = Cfg.Section("attachment") sec = Cfg.Section("attachment")
AttachmentPath = sec.Key("PATH").MustString(path.Join(AppDataPath, "attachments")) AttachmentPath = sec.Key("PATH").MustString(path.Join(AppDataPath, "attachments"))
if !filepath.IsAbs(AttachmentPath) { if !filepath.IsAbs(AttachmentPath) {

+ 1
- 0
options/locale/locale_en-US.ini View File

@ -315,6 +315,7 @@ team_no_units_error = Allow access to at least one repository section.
email_been_used = The email address is already used. email_been_used = The email address is already used.
openid_been_used = The OpenID address '%s' is already used. openid_been_used = The OpenID address '%s' is already used.
username_password_incorrect = Username or password is incorrect. username_password_incorrect = Username or password is incorrect.
password_complexity = Password does not pass complexity requirements.
enterred_invalid_repo_name = The repository name you entered is incorrect. enterred_invalid_repo_name = The repository name you entered is incorrect.
enterred_invalid_owner_name = The new owner name is not valid. enterred_invalid_owner_name = The new owner name is not valid.
enterred_invalid_password = The password you entered is incorrect. enterred_invalid_password = The password you entered is incorrect.

+ 9
- 1
routers/admin/users.go View File

@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/password"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/routers" "code.gitea.io/gitea/routers"
"code.gitea.io/gitea/services/mailer" "code.gitea.io/gitea/services/mailer"
@ -94,7 +95,10 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) {
u.LoginName = form.LoginName u.LoginName = form.LoginName
} }
} }
if !password.IsComplexEnough(form.Password) {
ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplUserNew, &form)
return
}
if err := models.CreateUser(u); err != nil { if err := models.CreateUser(u); err != nil {
switch { switch {
case models.IsErrUserAlreadyExist(err): case models.IsErrUserAlreadyExist(err):
@ -201,6 +205,10 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) {
ctx.ServerError("UpdateUser", err) ctx.ServerError("UpdateUser", err)
return return
} }
if !password.IsComplexEnough(form.Password) {
ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplUserEdit, &form)
return
}
u.HashPassword(form.Password) u.HashPassword(form.Password)
} }

+ 13
- 1
routers/api/v1/admin/user.go View File

@ -6,9 +6,12 @@
package admin package admin
import ( import (
"errors"
"code.gitea.io/gitea/models" "code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/password"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/routers/api/v1/convert" "code.gitea.io/gitea/routers/api/v1/convert"
"code.gitea.io/gitea/routers/api/v1/user" "code.gitea.io/gitea/routers/api/v1/user"
@ -73,7 +76,11 @@ func CreateUser(ctx *context.APIContext, form api.CreateUserOption) {
if ctx.Written() { if ctx.Written() {
return return
} }
if !password.IsComplexEnough(form.Password) {
err := errors.New("PasswordComplexity")
ctx.Error(400, "PasswordComplexity", err)
return
}
if err := models.CreateUser(u); err != nil { if err := models.CreateUser(u); err != nil {
if models.IsErrUserAlreadyExist(err) || if models.IsErrUserAlreadyExist(err) ||
models.IsErrEmailAlreadyUsed(err) || models.IsErrEmailAlreadyUsed(err) ||
@ -131,6 +138,11 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) {
} }
if len(form.Password) > 0 { if len(form.Password) > 0 {
if !password.IsComplexEnough(form.Password) {
err := errors.New("PasswordComplexity")
ctx.Error(400, "PasswordComplexity", err)
return
}
var err error var err error
if u.Salt, err = models.GetUserSalt(); err != nil { if u.Salt, err = models.GetUserSalt(); err != nil {
ctx.Error(500, "UpdateUser", err) ctx.Error(500, "UpdateUser", err)

+ 6
- 5
routers/user/auth.go View File

@ -17,6 +17,7 @@ import (
"code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/password"
"code.gitea.io/gitea/modules/recaptcha" "code.gitea.io/gitea/modules/recaptcha"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
@ -1334,6 +1335,11 @@ func ResetPasswdPost(ctx *context.Context) {
ctx.Data["Err_Password"] = true ctx.Data["Err_Password"] = true
ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplResetPassword, nil) ctx.RenderWithErr(ctx.Tr("auth.password_too_short", setting.MinPasswordLength), tplResetPassword, nil)
return return
} else if !password.IsComplexEnough(passwd) {
ctx.Data["IsResetForm"] = true
ctx.Data["Err_Password"] = true
ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplResetPassword, nil)
return
} }
var err error var err error
@ -1364,7 +1370,6 @@ func ResetPasswdPost(ctx *context.Context) {
func MustChangePassword(ctx *context.Context) { func MustChangePassword(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("auth.must_change_password") ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password" ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password"
ctx.HTML(200, tplMustChangePassword) ctx.HTML(200, tplMustChangePassword)
} }
@ -1372,16 +1377,12 @@ func MustChangePassword(ctx *context.Context) {
// account was created by an admin // account was created by an admin
func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form auth.MustChangePasswordForm) { func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form auth.MustChangePasswordForm) {
ctx.Data["Title"] = ctx.Tr("auth.must_change_password") ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password" ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/settings/change_password"
if ctx.HasError() { if ctx.HasError() {
ctx.HTML(200, tplMustChangePassword) ctx.HTML(200, tplMustChangePassword)
return return
} }
u := ctx.User u := ctx.User
// Make sure only requests for users who are eligible to change their password via // Make sure only requests for users who are eligible to change their password via
// this method passes through // this method passes through
if !u.MustChangePassword { if !u.MustChangePassword {

+ 3
- 0
routers/user/setting/account.go View File

@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/password"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/services/mailer" "code.gitea.io/gitea/services/mailer"
@ -52,6 +53,8 @@ func AccountPost(ctx *context.Context, form auth.ChangePasswordForm) {
ctx.Flash.Error(ctx.Tr("settings.password_incorrect")) ctx.Flash.Error(ctx.Tr("settings.password_incorrect"))
} else if form.Password != form.Retype { } else if form.Password != form.Retype {
ctx.Flash.Error(ctx.Tr("form.password_not_match")) ctx.Flash.Error(ctx.Tr("form.password_not_match"))
} else if !password.IsComplexEnough(form.Password) {
ctx.Flash.Error(ctx.Tr("settings.password_complexity"))
} else { } else {
var err error var err error
if ctx.User.Salt, err = models.GetUserSalt(); err != nil { if ctx.User.Salt, err = models.GetUserSalt(); err != nil {

+ 61
- 20
routers/user/setting/account_test.go View File

@ -19,36 +19,77 @@ import (
func TestChangePassword(t *testing.T) { func TestChangePassword(t *testing.T) {
oldPassword := "password" oldPassword := "password"
setting.MinPasswordLength = 6 setting.MinPasswordLength = 6
setting.PasswordComplexity = map[string]string{
"lower": "[a-z]+",
"upper": "[A-Z]+",
"digit": "[0-9]+",
"spec": "[-_]+",
}
var pcLUN = map[string]string{
"lower": "[a-z]+",
"upper": "[A-Z]+",
"digit": "[0-9]+",
}
var pcLU = map[string]string{
"lower": "[a-z]+",
"upper": "[A-Z]+",
}
for _, req := range []struct { for _, req := range []struct {
OldPassword string
NewPassword string
Retype string
Message string
OldPassword string
NewPassword string
Retype string
Message string
PasswordComplexity map[string]string
}{ }{
{ {
OldPassword: oldPassword,
NewPassword: "123456",
Retype: "123456",
Message: "",
OldPassword: oldPassword,
NewPassword: "Qwerty123456-",
Retype: "Qwerty123456-",
Message: "",
PasswordComplexity: setting.PasswordComplexity,
},
{
OldPassword: oldPassword,
NewPassword: "12345",
Retype: "12345",
Message: "auth.password_too_short",
PasswordComplexity: setting.PasswordComplexity,
},
{
OldPassword: "12334",
NewPassword: "123456",
Retype: "123456",
Message: "settings.password_incorrect",
PasswordComplexity: setting.PasswordComplexity,
},
{
OldPassword: oldPassword,
NewPassword: "123456",
Retype: "12345",
Message: "form.password_not_match",
PasswordComplexity: setting.PasswordComplexity,
}, },
{ {
OldPassword: oldPassword,
NewPassword: "12345",
Retype: "12345",
Message: "auth.password_too_short",
OldPassword: oldPassword,
NewPassword: "Qwerty",
Retype: "Qwerty",
Message: "settings.password_complexity",
PasswordComplexity: setting.PasswordComplexity,
}, },
{ {
OldPassword: "12334",
NewPassword: "123456",
Retype: "123456",
Message: "settings.password_incorrect",
OldPassword: oldPassword,
NewPassword: "Qwerty",
Retype: "Qwerty",
Message: "settings.password_complexity",
PasswordComplexity: pcLUN,
}, },
{ {
OldPassword: oldPassword,
NewPassword: "123456",
Retype: "12345",
Message: "form.password_not_match",
OldPassword: oldPassword,
NewPassword: "QWERTY",
Retype: "QWERTY",
Message: "settings.password_complexity",
PasswordComplexity: pcLU,
}, },
} { } {
models.PrepareTestEnv(t) models.PrepareTestEnv(t)

Loading…
Cancel
Save