Skip to content

Commit

Permalink
Improve "must-change-password" logic and document (#30472) (#30478)
Browse files Browse the repository at this point in the history
Backport #30472 by wxiaoguang

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
  • Loading branch information
3 people authored Apr 14, 2024
1 parent e64926c commit b266c78
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 30 deletions.
14 changes: 5 additions & 9 deletions cmd/admin_user_change_password.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var microcmdUserChangePassword = &cli.Command{
&cli.BoolFlag{
Name: "must-change-password",
Usage: "User must change password",
Value: true,
},
},
}
Expand All @@ -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
}
Expand Down
38 changes: 22 additions & 16 deletions cmd/admin_user_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand All @@ -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") {
Expand Down Expand Up @@ -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]()
Expand All @@ -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,
}

Expand Down
5 changes: 2 additions & 3 deletions docs/content/administration/command-line.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`:
Expand Down
4 changes: 2 additions & 2 deletions models/db/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b266c78

Please sign in to comment.