From cb4b0f66518c3d2e7ba604b8ff3d9477dd5760f2 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Tue, 19 Nov 2019 12:34:57 -0300 Subject: [PATCH 1/5] Add password requirement info on error --- modules/password/password.go | 55 ++++++++++++++++++++-------- modules/password/password_test.go | 5 ++- options/locale/locale_en-US.ini | 6 ++- public/css/index.css | 1 + routers/admin/users.go | 5 ++- routers/user/auth.go | 19 +++++++++- routers/user/setting/account.go | 3 +- routers/user/setting/account_test.go | 2 +- web_src/less/_base.less | 7 ++++ 9 files changed, 79 insertions(+), 24 deletions(-) diff --git a/modules/password/password.go b/modules/password/password.go index 92986977ec6dd..f82f48d9fc182 100644 --- a/modules/password/password.go +++ b/modules/password/password.go @@ -13,16 +13,34 @@ import ( "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`, + "password_lowercase_one", + }, + "upper": { + `ABCDEFGHIJKLMNOPQRSTUVWXYZ`, + "password_uppercase_one", + }, + "digit": { + `0123456789`, + "password_digit_one", + }, + "spec": { + ` !"#$%&'()*+,-./:;<=>?@[\]^_{|}~` + "`", + "password_special_one", + }, } ) @@ -36,22 +54,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 +77,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 +104,8 @@ func Generate(n int) (string, error) { } } } + +// GetActiveComplexities returns a list of the active complexities (may differ from the settings) +func GetActiveComplexities() []Complexity { + return requiredList +} diff --git a/modules/password/password_test.go b/modules/password/password_test.go index d46a6d1571e9e..145f5dfdfb2f6 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 { @@ -34,6 +36,7 @@ func TestComplexity_IsComplexEnough(t *testing.T) { } for _, val := range test.falsevalues { assert.False(t, IsComplexEnough(val)) + } } @@ -70,6 +73,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 4b3edc9057bc4..9ea09e724d362 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 21141d78ff2a9..b4401d7898890 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 2284f21838c6a..3a2fa0364b967 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers" + "code.gitea.io/gitea/routers/user" "code.gitea.io/gitea/services/mailer" "github.com/unknwon/com" @@ -96,7 +97,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(user.BuildPasswordComplexityError(ctx), tplUserNew, &form) return } u.MustChangePassword = form.MustChangePassword @@ -208,7 +209,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(user.BuildPasswordComplexityError(ctx), tplUserEdit, &form) return } u.HashPassword(form.Password) diff --git a/routers/user/auth.go b/routers/user/auth.go index cb5611e0459ba..a95508996814a 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -6,6 +6,7 @@ package user import ( + "bytes" "errors" "fmt" "net/http" @@ -1072,7 +1073,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(BuildPasswordComplexityError(ctx), tplSignUp, &form) return } @@ -1343,7 +1344,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(BuildPasswordComplexityError(ctx), tplResetPassword, nil) return } @@ -1433,3 +1434,17 @@ func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form aut ctx.Redirect(setting.AppSubURL + "/") } + +// BuildPasswordComplexityError builds the error message when password complexity checks fail +func BuildPasswordComplexityError(ctx *context.Context) string { + var buffer bytes.Buffer + buffer.WriteString(ctx.Tr("form.password_complexity")) + buffer.WriteString(`") + return buffer.String() +} diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go index 73799c8bd72e9..e2abdca3c4c24 100644 --- a/routers/user/setting/account.go +++ b/routers/user/setting/account.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/routers/user" "code.gitea.io/gitea/services/mailer" ) @@ -53,7 +54,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(user.BuildPasswordComplexityError(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 41783e19d736a..841ecb8ac25c5 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 8bf49b1ef904e..a39977065d8ec 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; From f85ca83e70564ffed4f54ce9a2c39306d7d8d823 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Tue, 19 Nov 2019 13:59:01 -0300 Subject: [PATCH 2/5] Move BuildComplexityError to the password pkg --- modules/password/password.go | 25 ++++++++++++++++++------- routers/admin/users.go | 5 ++--- routers/user/auth.go | 19 ++----------------- routers/user/setting/account.go | 3 +-- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/modules/password/password.go b/modules/password/password.go index f82f48d9fc182..8b52610ac022b 100644 --- a/modules/password/password.go +++ b/modules/password/password.go @@ -5,11 +5,13 @@ package password import ( + "bytes" "crypto/rand" "math/big" "strings" "sync" + "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" ) @@ -27,19 +29,19 @@ var ( charComplexities = map[string]Complexity{ "lower": { `abcdefghijklmnopqrstuvwxyz`, - "password_lowercase_one", + "form.password_lowercase_one", }, "upper": { `ABCDEFGHIJKLMNOPQRSTUVWXYZ`, - "password_uppercase_one", + "form.password_uppercase_one", }, "digit": { `0123456789`, - "password_digit_one", + "form.password_digit_one", }, "spec": { ` !"#$%&'()*+,-./:;<=>?@[\]^_{|}~` + "`", - "password_special_one", + "form.password_special_one", }, } ) @@ -105,7 +107,16 @@ func Generate(n int) (string, error) { } } -// GetActiveComplexities returns a list of the active complexities (may differ from the settings) -func GetActiveComplexities() []Complexity { - return requiredList +// 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/routers/admin/users.go b/routers/admin/users.go index 3a2fa0364b967..7626fbc0d0919 100644 --- a/routers/admin/users.go +++ b/routers/admin/users.go @@ -15,7 +15,6 @@ import ( "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/routers" - "code.gitea.io/gitea/routers/user" "code.gitea.io/gitea/services/mailer" "github.com/unknwon/com" @@ -97,7 +96,7 @@ func NewUserPost(ctx *context.Context, form auth.AdminCreateUserForm) { } if u.LoginType == models.LoginPlain { if !password.IsComplexEnough(form.Password) { - ctx.RenderWithErr(user.BuildPasswordComplexityError(ctx), tplUserNew, &form) + ctx.RenderWithErr(password.BuildComplexityError(ctx), tplUserNew, &form) return } u.MustChangePassword = form.MustChangePassword @@ -209,7 +208,7 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) { return } if !password.IsComplexEnough(form.Password) { - ctx.RenderWithErr(user.BuildPasswordComplexityError(ctx), 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 a95508996814a..d0ad4afffd21b 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -6,7 +6,6 @@ package user import ( - "bytes" "errors" "fmt" "net/http" @@ -1073,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(BuildPasswordComplexityError(ctx), tplSignUp, &form) + ctx.RenderWithErr(password.BuildComplexityError(ctx), tplSignUp, &form) return } @@ -1344,7 +1343,7 @@ func ResetPasswdPost(ctx *context.Context) { } else if !password.IsComplexEnough(passwd) { ctx.Data["IsResetForm"] = true ctx.Data["Err_Password"] = true - ctx.RenderWithErr(BuildPasswordComplexityError(ctx), tplResetPassword, nil) + ctx.RenderWithErr(password.BuildComplexityError(ctx), tplResetPassword, nil) return } @@ -1434,17 +1433,3 @@ func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form aut ctx.Redirect(setting.AppSubURL + "/") } - -// BuildPasswordComplexityError builds the error message when password complexity checks fail -func BuildPasswordComplexityError(ctx *context.Context) string { - var buffer bytes.Buffer - buffer.WriteString(ctx.Tr("form.password_complexity")) - buffer.WriteString(`") - return buffer.String() -} diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go index e2abdca3c4c24..a9064b0e15749 100644 --- a/routers/user/setting/account.go +++ b/routers/user/setting/account.go @@ -16,7 +16,6 @@ import ( "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/routers/user" "code.gitea.io/gitea/services/mailer" ) @@ -54,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(user.BuildPasswordComplexityError(ctx)) + ctx.Flash.Error(password.BuildComplexityError(ctx)) } else { var err error if ctx.User.Salt, err = models.GetUserSalt(); err != nil { From 3f7c35d5c46257f5fdb9c42be04ecf8e7d0ec3e9 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Tue, 19 Nov 2019 14:03:34 -0300 Subject: [PATCH 3/5] Unexport complexity type --- modules/password/password.go | 8 ++++---- modules/password/password_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/password/password.go b/modules/password/password.go index 8b52610ac022b..4e7d6d63fee41 100644 --- a/modules/password/password.go +++ b/modules/password/password.go @@ -15,8 +15,8 @@ import ( "code.gitea.io/gitea/modules/setting" ) -// Complexity contains information about a particular kind of password complexity -type Complexity struct { +// complexity contains information about a particular kind of password complexity +type complexity struct { ValidChars string TrNameOne string } @@ -24,9 +24,9 @@ type Complexity struct { var ( matchComplexityOnce sync.Once validChars string - requiredList []Complexity + requiredList []complexity - charComplexities = map[string]Complexity{ + charComplexities = map[string]complexity{ "lower": { `abcdefghijklmnopqrstuvwxyz`, "form.password_lowercase_one", diff --git a/modules/password/password_test.go b/modules/password/password_test.go index 145f5dfdfb2f6..fe2c8e88b5856 100644 --- a/modules/password/password_test.go +++ b/modules/password/password_test.go @@ -73,6 +73,6 @@ func TestComplexity_Generate(t *testing.T) { func testComplextity(values []string) { // Cleanup previous values validChars = "" - requiredList = make([]Complexity, 0, len(values)) + requiredList = make([]complexity, 0, len(values)) setupComplexity(values) } From cce05c8ecb43871c188ad8f4250305e00884c386 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Tue, 19 Nov 2019 14:05:46 -0300 Subject: [PATCH 4/5] Fix extra line --- modules/password/password_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/password/password_test.go b/modules/password/password_test.go index fe2c8e88b5856..4325086b50b80 100644 --- a/modules/password/password_test.go +++ b/modules/password/password_test.go @@ -36,7 +36,6 @@ func TestComplexity_IsComplexEnough(t *testing.T) { } for _, val := range test.falsevalues { assert.False(t, IsComplexEnough(val)) - } } From 5583bea17dcb4b42965d2ccb6534c6599b5dd0eb Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Tue, 19 Nov 2019 14:50:18 -0300 Subject: [PATCH 5/5] Update modules/password/password.go Co-Authored-By: Lauris BH --- modules/password/password.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/password/password.go b/modules/password/password.go index 4e7d6d63fee41..1c4b9c514a459 100644 --- a/modules/password/password.go +++ b/modules/password/password.go @@ -111,7 +111,7 @@ func Generate(n int) (string, error) { func BuildComplexityError(ctx *context.Context) string { var buffer bytes.Buffer buffer.WriteString(ctx.Tr("form.password_complexity")) - buffer.WriteString(`
    `) + buffer.WriteString("
      ") for _, c := range requiredList { buffer.WriteString("
    • ") buffer.WriteString(ctx.Tr(c.TrNameOne))