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

Closed
opened 2025-11-02 04:52:23 -06:00 by GiteaMirror · 8 comments
Owner

Originally created by @jolheiser on GitHub (Feb 8, 2019).

  • 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.
Originally created by @jolheiser on GitHub (Feb 8, 2019). - Gitea version (or commit ref): master - Can you reproduce the bug at https://try.gitea.io: - [ ] Yes (provide example URL) - [ ] No - [x] 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) ```go // 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.
Author
Owner

@zeripath commented on GitHub (Feb 8, 2019):

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.

@zeripath commented on GitHub (Feb 8, 2019): 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.
Author
Owner

@jolheiser commented on GitHub (Feb 8, 2019):

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.

@jolheiser commented on GitHub (Feb 8, 2019): 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.
Author
Owner

@adelowo commented on GitHub (Feb 8, 2019):

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 commented on GitHub (Feb 8, 2019): 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.
Author
Owner

@adelowo commented on GitHub (Feb 8, 2019):

See https://github.com/go-gitea/gitea/issues/4340

@adelowo commented on GitHub (Feb 8, 2019): See https://github.com/go-gitea/gitea/issues/4340
Author
Owner

@jolheiser commented on GitHub (Feb 8, 2019):

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.

@jolheiser commented on GitHub (Feb 8, 2019): 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.
Author
Owner

@lafriks commented on GitHub (Feb 8, 2019):

@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

@lafriks commented on GitHub (Feb 8, 2019): @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
Author
Owner

@jolheiser commented on GitHub (Feb 8, 2019):

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.

@jolheiser commented on GitHub (Feb 8, 2019): 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.
Author
Owner

@borisovano commented on GitHub (Oct 30, 2019):

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

@borisovano commented on GitHub (Oct 30, 2019): IMHO the issue is with the helpstring. If it would indicate it accepts options the behaviour is not so bad.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#2879