From 873c4d7e2c4256444d03700446c6a814fff6f958 Mon Sep 17 00:00:00 2001 From: S7evinK <2353100+S7evinK@users.noreply.github.com> Date: Fri, 25 Mar 2022 14:38:24 +0100 Subject: [PATCH] Fixes for `create-account` (#2285) * Check user existence Fallback to asking for the password if non is defined * Add missing tests * Update to not use pointers, verify username length * Re-add possibilty to create passwordless account * Fix config issue * Fix test again Co-authored-by: Neil Alexander --- cmd/create-account/main.go | 81 +++++++++++++++++++-------------- cmd/create-account/main_test.go | 41 ++++++++++------- 2 files changed, 72 insertions(+), 50 deletions(-) diff --git a/cmd/create-account/main.go b/cmd/create-account/main.go index 4d01a9f42..2719f8680 100644 --- a/cmd/create-account/main.go +++ b/cmd/create-account/main.go @@ -24,12 +24,11 @@ import ( "regexp" "strings" + "github.com/matrix-org/dendrite/setup" "github.com/matrix-org/dendrite/setup/base" + "github.com/matrix-org/dendrite/userapi/api" "github.com/sirupsen/logrus" "golang.org/x/term" - - "github.com/matrix-org/dendrite/setup" - "github.com/matrix-org/dendrite/userapi/api" ) const usage = `Usage: %s @@ -43,7 +42,7 @@ Example: # use password from file %s --config dendrite.yaml -username alice -passwordfile my.pass # ask user to provide password - %s --config dendrite.yaml -username alice -ask-pass + %s --config dendrite.yaml -username alice # read password from stdin %s --config dendrite.yaml -username alice -passwordstdin < my.pass cat my.pass | %s --config dendrite.yaml -username alice -passwordstdin @@ -56,10 +55,10 @@ Arguments: var ( username = flag.String("username", "", "The username of the account to register (specify the localpart only, e.g. 'alice' for '@alice:domain.com')") - password = flag.String("password", "", "The password to associate with the account (optional, account will be password-less if not specified)") + password = flag.String("password", "", "The password to associate with the account") pwdFile = flag.String("passwordfile", "", "The file to use for the password (e.g. for automated account creation)") pwdStdin = flag.Bool("passwordstdin", false, "Reads the password from stdin") - askPass = flag.Bool("ask-pass", false, "Ask for the password to use") + pwdLess = flag.Bool("passwordless", false, "Create a passwordless account, e.g. if only an accesstoken is required") isAdmin = flag.Bool("admin", false, "Create an admin account") resetPassword = flag.Bool("reset-password", false, "Resets the password for the given username") validUsernameRegex = regexp.MustCompile(`^[0-9a-z_\-=./]+$`) @@ -78,22 +77,44 @@ func main() { os.Exit(1) } + if *pwdLess && *resetPassword { + logrus.Fatalf("Can not reset to an empty password, unable to login afterwards.") + } + if !validUsernameRegex.MatchString(*username) { logrus.Warn("Username can only contain characters a-z, 0-9, or '_-./='") os.Exit(1) } - pass := getPassword(password, pwdFile, pwdStdin, askPass, os.Stdin) + if len(fmt.Sprintf("@%s:%s", *username, cfg.Global.ServerName)) > 255 { + logrus.Fatalf("Username can not be longer than 255 characters: %s", fmt.Sprintf("@%s:%s", *username, cfg.Global.ServerName)) + } - b := base.NewBaseDendrite(cfg, "create-account") + var pass string + var err error + if !*pwdLess { + pass, err = getPassword(*password, *pwdFile, *pwdStdin, os.Stdin) + if err != nil { + logrus.Fatalln(err) + } + } + + b := base.NewBaseDendrite(cfg, "Monolith") accountDB := b.CreateAccountsDB() accType := api.AccountTypeUser if *isAdmin { accType = api.AccountTypeAdmin } - var err error + + available, err := accountDB.CheckAccountAvailability(context.Background(), *username) + if err != nil { + logrus.Fatalln("Unable check username existence.") + } if *resetPassword { + if available { + logrus.Fatalln("Username could not be found.") + } err = accountDB.SetPassword(context.Background(), *username, pass) if err != nil { logrus.Fatalf("Failed to update password for user %s: %s", *username, err.Error()) @@ -104,6 +125,9 @@ func main() { logrus.Infof("Updated password for user %s and invalidated all logins\n", *username) return } + if !available { + logrus.Fatalln("Username is already in use.") + } _, err = accountDB.CreateAccount(context.Background(), *username, pass, "", accType) if err != nil { @@ -113,53 +137,44 @@ func main() { logrus.Infoln("Created account", *username) } -func getPassword(password, pwdFile *string, pwdStdin, askPass *bool, r io.Reader) string { - // no password option set, use empty password - if password == nil && pwdFile == nil && pwdStdin == nil && askPass == nil { - return "" - } - // password defined as parameter - if password != nil && *password != "" { - return *password - } - +func getPassword(password, pwdFile string, pwdStdin bool, r io.Reader) (string, error) { // read password from file - if pwdFile != nil && *pwdFile != "" { - pw, err := ioutil.ReadFile(*pwdFile) + if pwdFile != "" { + pw, err := ioutil.ReadFile(pwdFile) if err != nil { - logrus.Fatalln("Unable to read password from file:", err) + return "", fmt.Errorf("Unable to read password from file: %v", err) } - return strings.TrimSpace(string(pw)) + return strings.TrimSpace(string(pw)), nil } // read password from stdin - if pwdStdin != nil && *pwdStdin { + if pwdStdin { data, err := ioutil.ReadAll(r) if err != nil { - logrus.Fatalln("Unable to read password from stdin:", err) + return "", fmt.Errorf("Unable to read password from stdin: %v", err) } - return strings.TrimSpace(string(data)) + return strings.TrimSpace(string(data)), nil } - // ask the user to provide the password - if *askPass { + // If no parameter was set, ask the user to provide the password + if password == "" { fmt.Print("Enter Password: ") bytePassword, err := term.ReadPassword(int(os.Stdin.Fd())) if err != nil { - logrus.Fatalln("Unable to read password:", err) + return "", fmt.Errorf("Unable to read password: %v", err) } fmt.Println() fmt.Print("Confirm Password: ") bytePassword2, err := term.ReadPassword(int(os.Stdin.Fd())) if err != nil { - logrus.Fatalln("Unable to read password:", err) + return "", fmt.Errorf("Unable to read password: %v", err) } fmt.Println() if strings.TrimSpace(string(bytePassword)) != strings.TrimSpace(string(bytePassword2)) { - logrus.Fatalln("Entered passwords don't match") + return "", fmt.Errorf("Entered passwords don't match") } - return strings.TrimSpace(string(bytePassword)) + return strings.TrimSpace(string(bytePassword)), nil } - return "" + return password, nil } diff --git a/cmd/create-account/main_test.go b/cmd/create-account/main_test.go index d06eafe46..8e8b946be 100644 --- a/cmd/create-account/main_test.go +++ b/cmd/create-account/main_test.go @@ -8,45 +8,48 @@ import ( func Test_getPassword(t *testing.T) { type args struct { - password *string - pwdFile *string - pwdStdin *bool - askPass *bool + password string + pwdFile string + pwdStdin bool reader io.Reader } pass := "mySecretPass" passwordFile := "testdata/my.pass" - passwordStdin := true reader := &bytes.Buffer{} _, err := reader.WriteString(pass) if err != nil { t.Errorf("unable to write to buffer: %+v", err) } tests := []struct { - name string - args args - want string + name string + args args + want string + wantErr bool }{ - { - name: "no password defined", - args: args{}, - want: "", - }, { name: "password defined", - args: args{password: &pass}, + args: args{ + password: pass, + }, want: pass, }, { name: "pwdFile defined", - args: args{pwdFile: &passwordFile}, + args: args{ + pwdFile: passwordFile, + }, want: pass, }, + { + name: "pwdFile does not exist", + args: args{pwdFile: "iDontExist"}, + wantErr: true, + }, { name: "read pass from stdin defined", args: args{ - pwdStdin: &passwordStdin, + pwdStdin: true, reader: reader, }, want: pass, @@ -54,7 +57,11 @@ func Test_getPassword(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := getPassword(tt.args.password, tt.args.pwdFile, tt.args.pwdStdin, tt.args.askPass, tt.args.reader); got != tt.want { + got, err := getPassword(tt.args.password, tt.args.pwdFile, tt.args.pwdStdin, tt.args.reader) + if !tt.wantErr && err != nil { + t.Errorf("expected no error, but got %v", err) + } + if got != tt.want { t.Errorf("getPassword() = '%v', want '%v'", got, tt.want) } })