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>

Author: 2 people

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,31 +56,31 @@ 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
 	}
 }
 
 // IsComplexEnough return True if password meets complexity settings
 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()
+}

modules/password/password_test.go

@@ -18,13 +18,15 @@ 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!", ""}},
 		{[]string{"spec"}, []string{"=!$", "abc!"}, []string{"abc", "ABC", "123", ""}},
 		{[]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)
 }

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.

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}

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)

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
 	}
 

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 {

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())
 	}
 }

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;