Skip to content

Add must-change-password cli parameter #27626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 3, 2024
Merged

Add must-change-password cli parameter #27626

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3a08cc3
Add must-change-password parameter.
KN4CK3R Oct 14, 2023
77e2e23
Use `must-not-change-password`.
KN4CK3R Oct 17, 2023
7977a72
Merge branch 'main' of https://github.com/go-gitea/gitea into enhance…
KN4CK3R Oct 17, 2023
dab43ff
Revert "Use `must-not-change-password`."
6543 Oct 20, 2023
2b0145d
true by default
6543 Oct 20, 2023
6b75f97
Add suggestion.
KN4CK3R Feb 3, 2024
ea0e975
Merge branch 'main' of https://github.com/go-gitea/gitea into enhance…
KN4CK3R Feb 3, 2024
908e69e
Use real value.
KN4CK3R Feb 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion cmd/admin_user_change_password.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ var microcmdUserChangePassword = &cli.Command{
Value: "",
Usage: "New password to set for user",
},
&cli.BoolFlag{
Name: "must-change-password",
Value: true,
Usage: "User must change password",
},
},
}

Expand Down Expand Up @@ -69,7 +74,9 @@ func runChangePassword(c *cli.Context) error {
return err
}

if err = user_model.UpdateUserCols(ctx, user, "passwd", "passwd_hash_algo", "salt"); err != nil {
user.MustChangePassword = c.Bool("must-change-password")

if err = user_model.UpdateUserCols(ctx, user, "must_change_password", "passwd", "passwd_hash_algo", "salt"); err != nil {
return err
}

Expand Down
1 change: 1 addition & 0 deletions docs/content/administration/command-line.en-us.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,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. (default: true).
Copy link
Contributor

@wxiaoguang wxiaoguang Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `--must-change-password`: If provided, the user is required to choose a new password after the login. Optional. (default: true).
- `--must-change-password`, `--must-change-password=true/false`: If provided, the user is required to choose a new password after the login. Optional.

The (default: true) and optional might cause misunderstanding IMO.

I think everyone could understand that --must-change-password means --must-change-password=true

Or just follow the wording above, like - `--admin`: If provided, this makes the user an admin. Optional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any other suggestion? If no, could I commit this suggestion and approve?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestion sounds good

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the wording. Now the default is false if the flag is not set. I still think this should default to true.

Copy link
Contributor

@wxiaoguang wxiaoguang Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but I think if it defaults to true, then it might needs some new wording. (the old one makes me think for quite a long time to understand what is "optional" and what is "default: true" ....)

Maybe like this?


- `--must-change-password`: Default to true, the user is required to choose a new password after login. Use `--must-change-password=false` to disable this option.

Anyway, no better idea from my side. Either is fine to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I just used the wording from here:

gitea/docs/content/administration/command-line.en-us.md

Lines 86 to 87 in 360b3fd

- `--must-change-password`: If provided, the created user will be required to choose a newer password after the
initial login. Optional. (default: true).

Maybe we should have a common format for optional and required parameters.

- Examples:
- `gitea admin user change-password --username myname --password asecurepassword`
- `must-change-password`:
Expand Down