From 4c6e2da088cf092a9790df5c84b7b338508fede7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 01:22:14 +0800 Subject: [PATCH] Improve "must-change-password" logic and document (#30472) Unify the behaviors of "user create" and "user change-password". Co-authored-by: KN4CK3R --- cmd/admin_user_change_password.go | 14 +++---- cmd/admin_user_create.go | 38 +++++++++++-------- .../administration/command-line.en-us.md | 5 +-- models/db/engine.go | 4 +- 4 files changed, 31 insertions(+), 30 deletions(-) diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go index 824d66d11..bd9063a8e 100644 --- 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 a257ce21c..403e3ee8d 100644 --- 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 5049df35e..752a8d4c6 100644 --- 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 8684c4e2f..26abf0b96 100755 --- 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