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 <lauris@nix.lv>

diff --git a/modules/password/password.go b/modules/password/password.go
index 92986977ec6dd49ab8b8d6cf6095592181efef99..1c4b9c514a459f95c9122a735ae45ca41e15ef72 100644 (file)
--- 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("<ul>")
+       for _, c := range requiredList {
+               buffer.WriteString("<li>")
+               buffer.WriteString(ctx.Tr(c.TrNameOne))
+               buffer.WriteString("</li>")
+       }
+       buffer.WriteString("</ul>")
+       return buffer.String()
+}

diff --git a/modules/password/password_test.go b/modules/password/password_test.go
index d46a6d1571e9ef60d2d4d600bbcbf87a1ab5b32d..4325086b50b8058b2996fb43c3e502a04b53feff 100644 (file)
--- 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 be644c6ea9a8325d54b2a39b6a3175adc1c99a01..99304c470d39dfc73b88489282b11326397f5168 100644 (file)
--- 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 33d1f8046a3f335daf005a1af5ed5a1bfd15c059..1265fffad0bb89875d0214a1d8134f5defcf5e8d 100644 (file)
--- 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 2284f21838c6a71ab0695a00e736acbe3887c0b9..7626fbc0d0919708d24e6689d874830b3633b4af 100644 (file)
--- 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 cb5611e0459ba8e053c6083753c7df60448069d6..d0ad4afffd21bdc5cc1d5913684e14b7e4272fce 100644 (file)
--- 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 73799c8bd72e96a5be818c707e3399e59fbf0682..a9064b0e15749274787866a0460e376ccbe5c979 100644 (file)
--- 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 41783e19d736add6dbfd22dad701e7422da354cb..841ecb8ac25c57042767f3d80497dbe1d957252e 100644 (file)
--- 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 8bf49b1ef904efac1b4e5beeca0f09a2a8a1d558..a39977065d8ec278f02690995d08e425bc9c8367 100644 (file)
--- 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;