From c57edb6c7b5066da2b0f526e6ab9f7842fd785fb Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Tue, 19 Nov 2019 19:44:58 -0300 Subject: [PATCH] Add password requirement info on error (#9074) * Add password requirement info on error * Move BuildComplexityError to the password pkg * Unexport complexity type * Fix extra line * Update modules/password/password.go Co-Authored-By: Lauris BH --- modules/password/password.go | 66 +++++++++++++++++++++------- modules/password/password_test.go | 4 +- options/locale/locale_en-US.ini | 6 ++- public/css/index.css | 1 + routers/admin/users.go | 4 +- routers/user/auth.go | 4 +- routers/user/setting/account.go | 2 +- routers/user/setting/account_test.go | 2 +- web_src/less/_base.less | 7 +++ 9 files changed, 72 insertions(+), 24 deletions(-) diff --git a/modules/password/password.go b/modules/password/password.go index 92986977e..1c4b9c514 100644 --- a/modules/password/password.go +++ b/modules/password/password.go @@ -5,24 +5,44 @@ package password import ( + "bytes" "crypto/rand" "math/big" "strings" "sync" + "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" ) +// complexity contains information about a particular kind of password complexity +type complexity struct { + ValidChars string + TrNameOne string +} + var ( matchComplexityOnce sync.Once validChars string - requiredChars []string + requiredList []complexity - charComplexities = map[string]string{ - "lower": `abcdefghijklmnopqrstuvwxyz`, - "upper": `ABCDEFGHIJKLMNOPQRSTUVWXYZ`, - "digit": `0123456789`, - "spec": ` !"#$%&'()*+,-./:;<=>?@[\]^_{|}~` + "`", + charComplexities = map[string]complexity{ + "lower": { + `abcdefghijklmnopqrstuvwxyz`, + "form.password_lowercase_one", + }, + "upper": { + `ABCDEFGHIJKLMNOPQRSTUVWXYZ`, + "form.password_uppercase_one", + }, + "digit": { + `0123456789`, + "form.password_digit_one", + }, + "spec": { + ` !"#$%&'()*+,-./:;<=>?@[\]^_{|}~` + "`", + "form.password_special_one", + }, } ) @@ -36,22 +56,22 @@ func NewComplexity() { func setupComplexity(values []string) { if len(values) != 1 || values[0] != "off" { for _, val := range values { - if chars, ok := charComplexities[val]; ok { - validChars += chars - requiredChars = append(requiredChars, chars) + if complex, ok := charComplexities[val]; ok { + validChars += complex.ValidChars + requiredList = append(requiredList, complex) } } - if len(requiredChars) == 0 { + if len(requiredList) == 0 { // No valid character classes found; use all classes as default - for _, chars := range charComplexities { - validChars += chars - requiredChars = append(requiredChars, chars) + for _, complex := range charComplexities { + validChars += complex.ValidChars + requiredList = append(requiredList, complex) } } } if validChars == "" { // No complexities to check; provide a sensible default for password generation - validChars = charComplexities["lower"] + charComplexities["upper"] + charComplexities["digit"] + validChars = charComplexities["lower"].ValidChars + charComplexities["upper"].ValidChars + charComplexities["digit"].ValidChars } } @@ -59,8 +79,8 @@ func setupComplexity(values []string) { func IsComplexEnough(pwd string) bool { NewComplexity() if len(validChars) > 0 { - for _, req := range requiredChars { - if !strings.ContainsAny(req, pwd) { + for _, req := range requiredList { + if !strings.ContainsAny(req.ValidChars, pwd) { return false } } @@ -86,3 +106,17 @@ func Generate(n int) (string, error) { } } } + +// BuildComplexityError builds the error message when password complexity checks fail +func BuildComplexityError(ctx *context.Context) string { + var buffer bytes.Buffer + buffer.WriteString(ctx.Tr("form.password_complexity")) + buffer.WriteString("") + return buffer.String() +} diff --git a/modules/password/password_test.go b/modules/password/password_test.go index d46a6d157..4325086b5 100644 --- a/modules/password/password_test.go +++ b/modules/password/password_test.go @@ -18,6 +18,7 @@ func TestComplexity_IsComplexEnough(t *testing.T) { truevalues []string falsevalues []string }{ + {[]string{"off"}, []string{"1", "-", "a", "A", "ñ", "日本語"}, []string{}}, {[]string{"lower"}, []string{"abc", "abc!"}, []string{"ABC", "123", "=!$", ""}}, {[]string{"upper"}, []string{"ABC"}, []string{"abc", "123", "=!$", "abc!", ""}}, {[]string{"digit"}, []string{"123"}, []string{"abc", "ABC", "=!$", "abc!", ""}}, @@ -25,6 +26,7 @@ func TestComplexity_IsComplexEnough(t *testing.T) { {[]string{"off"}, []string{"abc", "ABC", "123", "=!$", "abc!", ""}, nil}, {[]string{"lower", "spec"}, []string{"abc!"}, []string{"abc", "ABC", "123", "=!$", "abcABC123", ""}}, {[]string{"lower", "upper", "digit"}, []string{"abcABC123"}, []string{"abc", "ABC", "123", "=!$", "abc!", ""}}, + {[]string{""}, []string{"abC=1", "abc!9D"}, []string{"ABC", "123", "=!$", ""}}, } for _, test := range testlist { @@ -70,6 +72,6 @@ func TestComplexity_Generate(t *testing.T) { func testComplextity(values []string) { // Cleanup previous values validChars = "" - requiredChars = make([]string, 0, len(values)) + requiredList = make([]complexity, 0, len(values)) setupComplexity(values) } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index be644c6ea..99304c470 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -328,7 +328,11 @@ team_no_units_error = Allow access to at least one repository section. email_been_used = The email address is already used. openid_been_used = The OpenID address '%s' is already used. username_password_incorrect = Username or password is incorrect. -password_complexity = Password does not pass complexity requirements. +password_complexity = Password does not pass complexity requirements: +password_lowercase_one = At least one lowercase character +password_uppercase_one = At least one uppercase character +password_digit_one = At least one digit +password_special_one = At least one special character (punctuation, brackets, quotes, etc.) enterred_invalid_repo_name = The repository name you entered is incorrect. enterred_invalid_owner_name = The new owner name is not valid. enterred_invalid_password = The password you entered is incorrect. diff --git a/public/css/index.css b/public/css/index.css index 33d1f8046..1265fffad 100644 --- a/public/css/index.css +++ b/public/css/index.css @@ -113,6 +113,7 @@ a{cursor:pointer} .ui .text.nopadding{padding:0} .ui .text.nomargin{margin:0} .ui .message{text-align:center} +.ui .message>ul{margin-left:auto;margin-right:auto;display:table;text-align:left} .ui.bottom.attached.message{font-weight:700;text-align:left;color:#000} .ui.bottom.attached.message .pull-right{color:#000} .ui.bottom.attached.message .pull-right>span,.ui.bottom.attached.message>span{color:#21ba45} diff --git a/routers/admin/users.go b/routers/admin/users.go index 2284f2183..7626fbc0d 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -96,7 +96,7 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) { } if u.LoginType == models.LoginPlain { if !password.IsComplexEnough(form.Password) { - ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplUserNew, &form) + ctx.RenderWithErr(password.BuildComplexityError(ctx), tplUserNew, &form) return } u.MustChangePassword = form.MustChangePassword @@ -208,7 +208,7 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) { return } if !password.IsComplexEnough(form.Password) { - ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplUserEdit, &form) + ctx.RenderWithErr(password.BuildComplexityError(ctx), tplUserEdit, &form) return } u.HashPassword(form.Password) diff --git a/routers/user/auth.go b/routers/user/auth.go index cb5611e04..d0ad4afff 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -1072,7 +1072,7 @@ func SignUpPost(ctx *context.Context, cpt *captcha.Captcha, form auth.RegisterFo } if !password.IsComplexEnough(form.Password) { ctx.Data["Err_Password"] = true - ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplSignUp, &form) + ctx.RenderWithErr(password.BuildComplexityError(ctx), tplSignUp, &form) return } @@ -1343,7 +1343,7 @@ func ResetPasswdPost(ctx *context.Context) { } else if !password.IsComplexEnough(passwd) { ctx.Data["IsResetForm"] = true ctx.Data["Err_Password"] = true - ctx.RenderWithErr(ctx.Tr("form.password_complexity"), tplResetPassword, nil) + ctx.RenderWithErr(password.BuildComplexityError(ctx), tplResetPassword, nil) return } diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go index 73799c8bd..a9064b0e1 100644 --- a/routers/user/setting/account.go +++ b/routers/user/setting/account.go @@ -53,7 +53,7 @@ func AccountPost(ctx *context.Context, form auth.ChangePasswordForm) { } else if form.Password != form.Retype { ctx.Flash.Error(ctx.Tr("form.password_not_match")) } else if !password.IsComplexEnough(form.Password) { - ctx.Flash.Error(ctx.Tr("form.password_complexity")) + ctx.Flash.Error(password.BuildComplexityError(ctx)) } else { var err error if ctx.User.Salt, err = models.GetUserSalt(); err != nil { diff --git a/routers/user/setting/account_test.go b/routers/user/setting/account_test.go index 41783e19d..841ecb8ac 100644 --- a/routers/user/setting/account_test.go +++ b/routers/user/setting/account_test.go @@ -91,7 +91,7 @@ func TestChangePassword(t *testing.T) { Retype: req.Retype, }) - assert.EqualValues(t, req.Message, ctx.Flash.ErrorMsg) + assert.Contains(t, ctx.Flash.ErrorMsg, req.Message) assert.EqualValues(t, http.StatusFound, ctx.Resp.Status()) } } diff --git a/web_src/less/_base.less b/web_src/less/_base.less index 8bf49b1ef..a39977065 100644 --- a/web_src/less/_base.less +++ b/web_src/less/_base.less @@ -471,6 +471,13 @@ code, text-align: center; } + .message > ul { + margin-left: auto; + margin-right: auto; + display: table; + text-align: left; + } + &.bottom.attached.message { font-weight: bold; text-align: left;