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 <neilalexander@users.noreply.github.com>

2 files changed, 72 insertions, 50 deletions

diff --git a/cmd/create-account/main.go b/cmd/create-account/main.go
index 4d01a9f4..2719f868 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))
+	}
+
+	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, "create-account")
+	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 d06eafe4..8e8b946b 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)
 			}
 		})