Admin CLI create-user not honoring lack of must-change-password flag

[jolheiser]

• Gitea version (or commit ref): master
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
  • Not relevant

Description

If the CLI is used to create a user, lack of the must-change-password flag would imply that the new user doesn't need to change their password, however that is not what happens.

When creating a user via the CLI, the must-change-password flag loses meaning after the first user is created (presumably the admin)

// always default to true
var changePassword = true

// If this is the first user being created.
// Take it as the admin and don't force a password update.
if n := models.CountUsers(); n == 0 {
    changePassword = false
}

if c.IsSet("must-change-password") {
    changePassword = c.Bool("must-change-password")
}

I think there are probably two options based on the "wanted" default behavior.

1. Set changePassword to false by default, as that will line up with the intended use of the flag.
2. Remove the must-change-password flag and optionally add a different flag with the opposite meaning. This would mean that users created via CLI would, by default, need to change their password, unless the new flag is applied.

[zeripath]

I think I have been bitten by this before, the simple workaround is to always set --must-change-password=false when you don't want that set.

[jolheiser]

I thought I remembered that being a thing as well, but couldn't find it in the urfave docs. Thanks!

At least there's a workaround for now, but I still think the flag should be changed to be more clear on how to use it vs what is default behavior.

[adelowo]

It is supposed to default to true actually. All users created by an admin are required to change their passwords.

Edit:

I think the naming is fine but the docs should be updated to show it defaults as true.

[adelowo]

See #4340

[jolheiser]

My only (admittedly nit-picky) issue with it would be that it's simply an extra step for an otherwise unused flag.

It's fine that it defaults to true as that's understandably more secure, however I would argue that a bool flag should toggle something by default.

gitea admin create-user --name myname --password asecurepassword --email me@example.com
and
gitea admin create-user --name myname --password asecurepassword --email me@example.com --must-change-password
do the exact same thing currently (past the first user)

Since there is a way to toggle the flag and this process is following intended behavior, I will close this issue, however I still think it is a somewhat misleading flag.

[lafriks]

@jolheiser if there is suggestion for better name keeping current behavior (that by default user must change password) I would not mind changing it or adding other with opposite behavior. My initial thought on --not-require-password-change seems too long&strange

[jolheiser]

Hah, yes I agree.
I couldn't think of a better name when I made the issue, either. If I think of one, maybe I'll send in a PR and get more feedback.

[borisovano]

IMHO the issue is with the helpstring. If it would indicate it accepts options the behaviour is not so bad.