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>
(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.
This commit is contained in:
wxiaoguang 2024-04-15 01:22:14 +08:00 committed by Gergely Nagy
parent d8258482e2
commit b122c6ef8b
No known key found for this signature in database
3 changed files with 31 additions and 24 deletions

View file

@ -36,6 +36,7 @@ var microcmdUserChangePassword = &cli.Command{
&cli.BoolFlag{ &cli.BoolFlag{
Name: "must-change-password", Name: "must-change-password",
Usage: "User must change password", Usage: "User must change password",
Value: true,
}, },
}, },
} }
@ -57,23 +58,18 @@ func runChangePassword(c *cli.Context) error {
return err 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{ opts := &user_service.UpdateAuthOptions{
Password: optional.Some(c.String("password")), 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 { if err := user_service.UpdateAuth(ctx, user, opts); err != nil {
switch { switch {
case errors.Is(err, password.ErrMinLength): 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): 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): 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: default:
return err return err
} }

View file

@ -8,6 +8,7 @@ import (
"fmt" "fmt"
auth_model "code.gitea.io/gitea/models/auth" auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
pwd "code.gitea.io/gitea/modules/auth/password" pwd "code.gitea.io/gitea/modules/auth/password"
"code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/optional"
@ -46,9 +47,10 @@ var microcmdUserCreate = &cli.Command{
Usage: "Generate a random password for the user", Usage: "Generate a random password for the user",
}, },
&cli.BoolFlag{ &cli.BoolFlag{
Name: "must-change-password", Name: "must-change-password",
Usage: "Set this option to false to prevent forcing the user to change their password after initial login", Usage: "Set this option to false to prevent forcing the user to change their password after initial login",
Value: true, Value: true,
DisableDefaultText: true,
}, },
&cli.IntFlag{ &cli.IntFlag{
Name: "random-password-length", Name: "random-password-length",
@ -72,10 +74,10 @@ func runCreateUser(c *cli.Context) error {
} }
if c.IsSet("name") && c.IsSet("username") { 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") { 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") { 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") return errors.New("must set either password or random-password flag")
} }
changePassword := c.Bool("must-change-password") isAdmin := c.Bool("admin")
mustChangePassword := true // always default to true
// If this is the first user being created. if c.IsSet("must-change-password") {
// Take it as the admin and don't force a password update. // if the flag is set, use the value provided by the user
if n := user_model.CountUsers(ctx, nil); n == 0 { mustChangePassword = c.Bool("must-change-password")
changePassword = false } 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]() restricted := optional.None[bool]()
@ -132,8 +143,8 @@ func runCreateUser(c *cli.Context) error {
Name: username, Name: username,
Email: c.String("email"), Email: c.String("email"),
Passwd: password, Passwd: password,
IsAdmin: c.Bool("admin"), IsAdmin: isAdmin,
MustChangePassword: changePassword, MustChangePassword: mustChangePassword,
Visibility: visibility, Visibility: visibility,
} }

View file

@ -293,8 +293,8 @@ func MaxBatchInsertSize(bean any) int {
} }
// IsTableNotEmpty returns true if table has at least one record // IsTableNotEmpty returns true if table has at least one record
func IsTableNotEmpty(tableName string) (bool, error) { func IsTableNotEmpty(beanOrTableName any) (bool, error) {
return x.Table(tableName).Exist() return x.Table(beanOrTableName).Exist()
} }
// DeleteAllRecords will delete all the records of this table // DeleteAllRecords will delete all the records of this table