From b122c6ef8b9254120432aed373cbe075331132ac 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 (cherry picked from commit 4c6e2da088cf092a9790df5c84b7b338508fede7) Conflicts: - cmd/admin_user_create.go Resolved by favoring Gitea's version of the conflicting areas. - docs/content/administration/command-line.en-us.md Removed, Gitea specific. --- cmd/admin_user_change_password.go | 14 +++++------- cmd/admin_user_create.go | 37 ++++++++++++++++++++----------- models/db/engine.go | 4 ++-- 3 files changed, 31 insertions(+), 24 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 10965c7e8..caafef536 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,9 +47,10 @@ 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", - Value: true, + Name: "must-change-password", + Usage: "Set this option to false to prevent forcing the user to change their password after initial login", + Value: true, + DisableDefaultText: true, }, &cli.IntFlag{ Name: "random-password-length", @@ -72,10 +74,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") { @@ -111,12 +113,21 @@ func runCreateUser(c *cli.Context) error { return errors.New("must set either password or random-password flag") } - changePassword := c.Bool("must-change-password") - - // 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") { + // 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]() @@ -132,8 +143,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/models/db/engine.go b/models/db/engine.go index 09f64b6c8..1f0cd1a5d 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -293,8 +293,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