fix:添加命令行模式,支持修改 password。需要输入用户名#2188
Conversation
|
@copilot 做一下安全性和可行性分析 |
There was a problem hiding this comment.
Pull request overview
Adds a CLI execution path to the server binary so operators can reset a user’s password (by username) from the command line (e.g., in Docker), while preserving existing HTTP server startup behavior when only common args like -c/--config are provided.
Changes:
- Introduces a Cobra-based CLI (
--reset-password {username}) and routesRunServer()to CLI vs HTTP server based on args. - Adds
utils.RandomPassword()and aUserService.ResetPasswordByUsername()helper to support password resets by username. - Replaces the previous
flagparsing for-cwith manual argv scanning so config selection works alongside the new CLI mode.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| server/utils/password.go | Adds random password generator used by the reset-password CLI. |
| server/service/system/sys_user.go | Adds ResetPasswordByUsername to reset a password by username (delegates to existing reset-by-ID). |
| server/core/cli.go | New CLI entrypoint and arg detection helpers (hasCommandArgs, config lookup). |
| server/core/server.go | Splits HTTP startup into runHttpServer() and adds CLI dispatch in RunServer(). |
| server/core/viper.go | Switches config path parsing from flag to argv scanning (lookupConfigPathArg). |
| server/core/server_run.go | Removes unused server interface and uses http.Server directly. |
| server/go.mod | Adds CLI dependencies (Cobra / updated pflag). |
| server/go.sum | Updates dependency checksums for newly added modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if i+1 < len(args) { | ||
| return args[i+1], true | ||
| } | ||
| return "", false |
There was a problem hiding this comment.
lookupConfigPathArg returns ("", false) when "-c/--config" is present but missing its value (e.g., server -c). That causes getConfigPath to fall back to env/default config instead of failing fast, which is a behavior change from the previous flag-based parsing (which errors on missing values). Please treat this as an invalid invocation and return a detectable signal (or plumb an error) so the process can exit with a clear message.
| return "", false | |
| // Flag present but missing its value: signal presence (true) with empty path | |
| return "", true |
| if cliConfig, ok := lookupConfigPathArg(os.Args[1:]); ok && cliConfig != "" { | ||
| config = cliConfig | ||
| fmt.Printf("您正在使用命令行的 '-c' 参数传递的值, config 的路径为 %s\n", config) | ||
| return |
There was a problem hiding this comment.
getConfigPath prints the selected config path to stdout. In CLI mode (--reset-password), this will be mixed into stdout along with the “new password” output, which can break scripts that expect stdout to contain only the password block. Consider sending these informational messages to stderr / logger, or suppressing them when running CLI subcommands.
| github.com/spf13/afero v1.12.0 // indirect | ||
| github.com/spf13/cast v1.7.1 // indirect | ||
| github.com/spf13/pflag v1.0.5 // indirect | ||
| github.com/spf13/cobra v1.10.2 // indirect |
There was a problem hiding this comment.
go.mod marks github.com/spf13/cobra as "// indirect", but it is imported directly by server/core/cli.go. Running go mod tidy (or removing the indirect marker) will keep dependency metadata accurate and avoid confusion about which deps are direct vs indirect.
| github.com/spf13/cobra v1.10.2 // indirect | |
| github.com/spf13/cobra v1.10.2 |
| func (userService *UserService) ResetPasswordByUsername(username, password string) (err error) { | ||
| if global.GVA_DB == nil { | ||
| return fmt.Errorf("db not init") | ||
| } | ||
|
|
||
| var user system.SysUser |
There was a problem hiding this comment.
This new public method is missing the file’s standard function header annotations (e.g., //@author, //@function, //@description) that are present on surrounding UserService methods. Please add the same style of comment block for ResetPasswordByUsername to keep documentation generation/consistency intact.
| password := make([]byte, length) | ||
| for i, b := range buf { | ||
| password[i] = passwordAlphabet[int(b)%len(passwordAlphabet)] | ||
| } |
There was a problem hiding this comment.
RandomPassword maps random bytes to the alphabet using modulo (b % len(passwordAlphabet)), which introduces modulo bias and makes some characters slightly more likely. For password generation, prefer unbiased selection (e.g., crypto/rand.Int with a big.Int bound, or rejection sampling) so each character is uniformly distributed.
| func RandomPassword(length int) (string, error) { | ||
| if length <= 0 { | ||
| return "", errors.New("password length must be greater than 0") | ||
| } |
There was a problem hiding this comment.
There are existing unit tests under server/utils (e.g., json_test.go, human_duration_test.go), but RandomPassword is new and currently untested. Please add tests for error handling (length <= 0) and basic properties (returned length matches input; output only contains characters from the allowed alphabet).
| if i+1 < len(args) { | ||
| i++ |
There was a problem hiding this comment.
filterCommonArgs silently drops "-c/--config" even when it is provided without a value (e.g., server -c), which can make RunServer incorrectly treat the invocation as “no command args” and start the HTTP server instead of surfacing an error. Consider detecting the missing value case here (or earlier) and returning an explicit error/exit so mis-specified config flags don’t get ignored.
| if i+1 < len(args) { | |
| i++ | |
| if i+1 < len(args) { | |
| // Skip the next argument, which should be the config path. | |
| i++ | |
| } else { | |
| // Config flag provided without a value: surface an explicit error | |
| // instead of silently dropping the flag and continuing. | |
| fmt.Fprintln(os.Stderr, "error: missing value for -c/--config flag") | |
| os.Exit(1) |
添加 cli 模式。在 docker 中可以使用 server -c config.docker.yaml --reset-password {username} 重置用户密码;
成功后会返回类似于:
++++++++++++++++++++++++++++++++++
the new password is: oSsVyYBCeN9Z7jzP
++++++++++++++++++++++++++++++++++