Improve "must-change-password" logic and document (#30472)

Unify the behaviors of "user create" and "user change-password".

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>

diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go
index 824d66d11259ea5aef2d96914e9d2251b0cbca49..bd9063a8e4b8de4f24db327042586c837e5deb9f 100644 (file)
--- a/cmd/admin_user_change_password.go
+++ b/cmd/admin_user_change_password.go
@@ -36,6 +36,7 @@ var microcmdUserChangePassword = &cli.Command{
                &cli.BoolFlag{
                        Name:  "must-change-password",
                        Usage: "User must change password",
+                       Value: true,
                },
        },
 }
@@ -57,23 +58,18 @@ func runChangePassword(c *cli.Context) error {
                return err
        }
 
-       var mustChangePassword optional.Option[bool]
-       if c.IsSet("must-change-password") {
-               mustChangePassword = optional.Some(c.Bool("must-change-password"))
-       }
-
        opts := &user_service.UpdateAuthOptions{
                Password:           optional.Some(c.String("password")),
-               MustChangePassword: mustChangePassword,
+               MustChangePassword: optional.Some(c.Bool("must-change-password")),
        }
        if err := user_service.UpdateAuth(ctx, user, opts); err != nil {
                switch {
                case errors.Is(err, password.ErrMinLength):
-                       return fmt.Errorf("Password is not long enough. Needs to be at least %d", setting.MinPasswordLength)
+                       return fmt.Errorf("password is not long enough, needs to be at least %d characters", setting.MinPasswordLength)
                case errors.Is(err, password.ErrComplexity):
-                       return errors.New("Password does not meet complexity requirements")
+                       return errors.New("password does not meet complexity requirements")
                case errors.Is(err, password.ErrIsPwned):
-                       return errors.New("The password you chose is on a list of stolen passwords previously exposed in public data breaches. Please try again with a different password.\nFor more details, see https://haveibeenpwned.com/Passwords")
+                       return errors.New("the password is in a list of stolen passwords previously exposed in public data breaches, please try again with a different password, to see more details: https://haveibeenpwned.com/Passwords")
                default:
                        return err
                }

diff --git a/cmd/admin_user_create.go b/cmd/admin_user_create.go
index a257ce21c8df3cec85caeb83fb77f14b7ad62fb5..403e3ee8d82dc0eb8714be878c362fd1ca704dd4 100644 (file)
--- a/cmd/admin_user_create.go
+++ b/cmd/admin_user_create.go
@@ -8,6 +8,7 @@ import (
        "fmt"
 
        auth_model "code.gitea.io/gitea/models/auth"
+       "code.gitea.io/gitea/models/db"
        user_model "code.gitea.io/gitea/models/user"
        pwd "code.gitea.io/gitea/modules/auth/password"
        "code.gitea.io/gitea/modules/optional"
@@ -46,8 +47,9 @@ var microcmdUserCreate = &cli.Command{
                        Usage: "Generate a random password for the user",
                },
                &cli.BoolFlag{
-                       Name:  "must-change-password",
-                       Usage: "Set this option to false to prevent forcing the user to change their password after initial login, (Default: true)",
+                       Name:               "must-change-password",
+                       Usage:              "Set to false to prevent forcing the user to change their password after initial login",
+                       DisableDefaultText: true,
                },
                &cli.IntFlag{
                        Name:  "random-password-length",
@@ -71,10 +73,10 @@ func runCreateUser(c *cli.Context) error {
        }
 
        if c.IsSet("name") && c.IsSet("username") {
-               return errors.New("Cannot set both --name and --username flags")
+               return errors.New("cannot set both --name and --username flags")
        }
        if !c.IsSet("name") && !c.IsSet("username") {
-               return errors.New("One of --name or --username flags must be set")
+               return errors.New("one of --name or --username flags must be set")
        }
 
        if c.IsSet("password") && c.IsSet("random-password") {
@@ -110,17 +112,21 @@ func runCreateUser(c *cli.Context) error {
                return errors.New("must set either password or random-password flag")
        }
 
-       // always default to true
-       changePassword := true
-
-       // If this is the first user being created.
-       // Take it as the admin and don't force a password update.
-       if n := user_model.CountUsers(ctx, nil); n == 0 {
-               changePassword = false
-       }
-
+       isAdmin := c.Bool("admin")
+       mustChangePassword := true // always default to true
        if c.IsSet("must-change-password") {
-               changePassword = c.Bool("must-change-password")
+               // if the flag is set, use the value provided by the user
+               mustChangePassword = c.Bool("must-change-password")
+       } else {
+               // check whether there are users in the database
+               hasUserRecord, err := db.IsTableNotEmpty(&user_model.User{})
+               if err != nil {
+                       return fmt.Errorf("IsTableNotEmpty: %w", err)
+               }
+               if !hasUserRecord && isAdmin {
+                       // if this is the first admin being created, don't force to change password (keep the old behavior)
+                       mustChangePassword = false
+               }
        }
 
        restricted := optional.None[bool]()
@@ -136,8 +142,8 @@ func runCreateUser(c *cli.Context) error {
                Name:               username,
                Email:              c.String("email"),
                Passwd:             password,
-               IsAdmin:            c.Bool("admin"),
-               MustChangePassword: changePassword,
+               IsAdmin:            isAdmin,
+               MustChangePassword: mustChangePassword,
                Visibility:         visibility,
        }
 

diff --git a/docs/content/administration/command-line.en-us.md b/docs/content/administration/command-line.en-us.md
index 5049df35e050479d3d83e6bf560422ef56c0b5ca..752a8d4c6fd3d832e0bc1d26080a932cb90f0412 100644 (file)
--- a/docs/content/administration/command-line.en-us.md
+++ b/docs/content/administration/command-line.en-us.md
@@ -83,8 +83,7 @@ Admin operations:
         - `--email value`: Email. Required.
         - `--admin`: If provided, this makes the user an admin. Optional.
         - `--access-token`: If provided, an access token will be created for the user. Optional. (default: false).
-        - `--must-change-password`: If provided, the created user will be required to choose a newer password after the
-          initial login. Optional. (default: true).
+        - `--must-change-password`: The created user will be required to set a new password after the initial login, default: true. It could be disabled by `--must-change-password=false`.
         - `--random-password`: If provided, a randomly generated password will be used as the password of the created
           user. The value of `--password` will be discarded. Optional.
         - `--random-password-length`: If provided, it will be used to configure the length of the randomly generated
@@ -95,7 +94,7 @@ Admin operations:
       - Options:
         - `--username value`, `-u value`: Username. Required.
         - `--password value`, `-p value`: New password. Required.
-        - `--must-change-password`: If provided, the user is required to choose a new password after the login. Optional.
+        - `--must-change-password`: The user is required to set a new password after the login, default: true. It could be disabled by `--must-change-password=false`.
       - Examples:
         - `gitea admin user change-password --username myname --password asecurepassword`
     - `must-change-password`:

diff --git a/models/db/engine.go b/models/db/engine.go
index 8684c4e2f162fd7ae0236537bcecb84601eab8e7..26abf0b96c21e055e43c6b168fe13977e6d4f1ff 100755 (executable)
--- a/models/db/engine.go
+++ b/models/db/engine.go
@@ -284,8 +284,8 @@ func MaxBatchInsertSize(bean any) int {
 }
 
 // IsTableNotEmpty returns true if table has at least one record
-func IsTableNotEmpty(tableName string) (bool, error) {
-       return x.Table(tableName).Exist()
+func IsTableNotEmpty(beanOrTableName any) (bool, error) {
+       return x.Table(beanOrTableName).Exist()
 }
 
 // DeleteAllRecords will delete all the records of this table