convert public key from newline delimited string to a real array

Added a compatibility layer that will convert newline delimited keys to array
when the user is fetched from the database.
This code will be removed in future versions please update your public keys,
you only need to resave the users using the REST API.

README.md

@@ -162,7 +162,7 @@ For each account the following properties can be configured:
 
 - `username`
 - `password` used for password authentication. For users created using SFTPGo REST API the password will be stored using argon2id hashing algo. SFTPGo supports checking passwords stored with bcrypt too. Currently, as fallback, there is a clear text password checking but you should not store passwords as clear text and this support could be removed at any time, so please don't depend on it.
-- `public_key` used for public key authentication. At least one between password and public key is mandatory. Multiple public keys are supported, newline delimited (unix line break unescaped).
+- `public_key` array of public keys. At least one public key or the password is mandatory.
 - `home_dir` The user cannot upload or download files outside this directory. Must be an absolute path
 - `uid`, `gid`. If sftpgo runs as root system user then the created files and directories will be assigned to this system uid/gid. Ignored on windows and if sftpgo runs as non root user: in this case files and directories for all SFTP users will be owned by the system user that runs sftpgo.
 - `max_sessions` maximum concurrent sessions. 0 means unlimited

api/api_test.go

@@ -126,7 +126,7 @@ func TestBasicUserHandling(t *testing.T) {
 func TestAddUserNoCredentials(t *testing.T) {
 	u := getTestUser()
 	u.Password = ""
-	u.PublicKey = ""
+	u.PublicKey = []string{}
 	_, err := api.AddUser(u, http.StatusBadRequest)
 	if err != nil {
 		t.Errorf("unexpected error adding user with no credentials: %v", err)
@@ -182,22 +182,22 @@ func TestUserPublicKey(t *testing.T) {
 	u := getTestUser()
 	invalidPubKey := "invalid"
 	validPubKey := "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC03jj0D+djk7pxIf/0OhrxrchJTRZklofJ1NoIu4752Sq02mdXmarMVsqJ1cAjV5LBVy3D1F5U6XW4rppkXeVtd04Pxb09ehtH0pRRPaoHHlALiJt8CoMpbKYMA8b3KXPPriGxgGomvtU2T2RMURSwOZbMtpsugfjYSWenyYX+VORYhylWnSXL961LTyC21ehd6d6QnW9G7E5hYMITMY9TuQZz3bROYzXiTsgN0+g6Hn7exFQp50p45StUMfV/SftCMdCxlxuyGny2CrN/vfjO7xxOo2uv7q1qm10Q46KPWJQv+pgZ/OfL+EDjy07n5QVSKHlbx+2nT4Q0EgOSQaCTYwn3YjtABfIxWwgAFdyj6YlPulCL22qU4MYhDcA6PSBwDdf8hvxBfvsiHdM+JcSHvv8/VeJhk6CmnZxGY0fxBupov27z3yEO8nAg8k+6PaUiW1MSUfuGMF/ktB8LOstXsEPXSszuyXiOv4DaryOXUiSn7bmRqKcEFlJusO6aZP0= nicola@p1"
-	u.PublicKey = invalidPubKey
+	u.PublicKey = []string{invalidPubKey}
 	_, err := api.AddUser(u, http.StatusBadRequest)
 	if err != nil {
 		t.Errorf("unexpected error adding user with invalid pub key: %v", err)
 	}
-	u.PublicKey = validPubKey
+	u.PublicKey = []string{validPubKey}
 	user, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
 	}
-	user.PublicKey = validPubKey + "\n" + invalidPubKey
+	user.PublicKey = []string{validPubKey, invalidPubKey}
 	_, err = api.UpdateUser(user, http.StatusBadRequest)
 	if err != nil {
 		t.Errorf("update user with invalid public key must fail: %v", err)
 	}
-	user.PublicKey = validPubKey + "\n" + validPubKey + "\n" + validPubKey
+	user.PublicKey = []string{validPubKey, validPubKey, validPubKey}
 	_, err = api.UpdateUser(user, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to update user: %v", err)
@@ -238,7 +238,7 @@ func TestUpdateUserNoCredentials(t *testing.T) {
 		t.Errorf("unable to add user: %v", err)
 	}
 	user.Password = ""
-	user.PublicKey = ""
+	user.PublicKey = []string{}
 	// password and public key will be omitted from json serialization if empty and so they will remain unchanged
 	// and no validation error will be raised
 	_, err = api.UpdateUser(user, http.StatusOK)

api/internal_test.go

@@ -42,12 +42,12 @@ func TestCheckUser(t *testing.T) {
 		t.Errorf("actual password must be nil")
 	}
 	actual.Password = ""
-	actual.PublicKey = "pub key"
+	actual.PublicKey = []string{"pub key"}
 	err = checkUser(expected, actual)
 	if err == nil {
 		t.Errorf("actual public key must be nil")
 	}
-	actual.PublicKey = ""
+	actual.PublicKey = []string{}
 	err = checkUser(expected, actual)
 	if err == nil {
 		t.Errorf("actual ID must be > 0")

api/schema/openapi.yaml

@@ -12,7 +12,7 @@ paths:
       tags:
       - connections
       summary: Get the active sftp users and info about their uploads/downloads
-      operationId: get_connections
+      operationId: get_sftp_connections
       responses:
         200:
           description: successful operation
@@ -27,7 +27,7 @@ paths:
       tags:
       - connections
       summary: Terminate an active SFTP connection
-      operationId: close_connection
+      operationId: close_sftp_connection
       parameters: 
       - name: connectionID
         in: path
@@ -81,7 +81,7 @@ paths:
       tags:
       - quota
       summary: Get the active quota scans
-      operationId: get_quota_scan
+      operationId: get_quota_scans
       responses:
         200:
           description: successful operation
@@ -297,7 +297,7 @@ paths:
       - users
       summary: Find user by ID
       description: For security reasons password and public key are empty in the response
-      operationId: getUserByID
+      operationId: get_user_by_id
       parameters: 
       - name: userID
         in: path
@@ -356,8 +356,8 @@ paths:
     put:
       tags:
       - users
-      summary: Update an user
-      operationId: updateUser
+      summary: Update an existing user
+      operationId: update_user
       parameters: 
       - name: userID
         in: path
@@ -426,8 +426,8 @@ paths:
     delete:
       tags:
       - users
-      summary: Delete an user
-      operationId: deleteUser
+      summary: Delete an existing user
+      operationId: delete_user
       parameters: 
       - name: userID
         in: path
@@ -524,9 +524,11 @@ components:
           nullable: true
           description: password or public key are mandatory. For security reasons this field is omitted when you search/get users
         public_key:
-          type: string
+          type: array
+          items:
+            type: string
           nullable: true
-          description: password or public key are mandatory. For security reasons this field is omitted when you search/get users. Multiple public keys are supported, newline delimited, when adding or modifying an user
+          description: a password or at least one public key are mandatory. For security reasons this field is omitted when you search/get users.
         home_dir:
           type: string
           description: path to the user home directory. The user cannot upload or download files outside this directory. SFTPGo tries to automatically create this folder if missing. Must be an absolute path

api/user.go

@@ -65,7 +65,7 @@ func getUserByID(w http.ResponseWriter, r *http.Request) {
 	user, err := dataprovider.GetUserByID(dataProvider, userID)
 	if err == nil {
 		user.Password = ""
-		user.PublicKey = ""
+		user.PublicKey = []string{}
 		render.JSON(w, r, user)
 	} else if err == sql.ErrNoRows {
 		sendAPIResponse(w, r, err, "", http.StatusNotFound)
@@ -86,7 +86,7 @@ func addUser(w http.ResponseWriter, r *http.Request) {
 		user, err = dataprovider.UserExists(dataProvider, user.Username)
 		if err == nil {
 			user.Password = ""
-			user.PublicKey = ""
+			user.PublicKey = []string{}
 			render.JSON(w, r, user)
 		} else {
 			sendAPIResponse(w, r, err, "", http.StatusInternalServerError)

config/config.go

@@ -46,6 +46,7 @@ func init() {
 				Command:             "",
 				HTTPNotificationURL: "",
 			},
+			Keys: []sftpd.Key{},
 		},
 		ProviderConf: dataprovider.Config{
 			Driver:           "sqlite",

dataprovider/dataprovider.go

@@ -226,22 +226,19 @@ func validateUser(user *User) error {
 			return &ValidationError{err: fmt.Sprintf("Invalid permission: %v", p)}
 		}
 	}
-	if !strings.HasPrefix(user.Password, argonPwdPrefix) {
+	if len(user.Password) > 0 && !strings.HasPrefix(user.Password, argonPwdPrefix) {
 		pwd, err := argon2id.CreateHash(user.Password, argon2id.DefaultParams)
 		if err != nil {
 			return err
 		}
 		user.Password = pwd
 	}
-	if len(user.PublicKey) > 0 {
-		for i, k := range strings.Split(user.PublicKey, "\n") {
-			_, _, _, _, err := ssh.ParseAuthorizedKey([]byte(k))
-			if err != nil {
-				return &ValidationError{err: fmt.Sprintf("Could not parse key nr. %d: %s", i, err)}
-			}
+	for i, k := range user.PublicKey {
+		_, _, _, _, err := ssh.ParseAuthorizedKey([]byte(k))
+		if err != nil {
+			return &ValidationError{err: fmt.Sprintf("Could not parse key nr. %d: %s", i, err)}
 		}
 	}
-
 	return nil
 }
 

dataprovider/sqlcommon.go

@@ -39,6 +39,11 @@ func sqlCommonValidateUserAndPass(username string, password string) (User, error
 	if err != nil {
 		logger.Warn(logSender, "error authenticating user: %v, error: %v", username, err)
 	} else {
+		// even if the password is empty inside the database an empty user password
+		// will be refused anyway so it cannot match, additional check to be paranoid
+		if len(user.Password) == 0 {
+			return user, errors.New("Credentials cannot be null or empty")
+		}
 		var match bool
 		if strings.HasPrefix(user.Password, argonPwdPrefix) {
 			match, err = argon2id.ComparePasswordAndHash(password, user.Password)
@@ -77,7 +82,7 @@ func sqlCommonValidateUserAndPubKey(username string, pubKey string) (User, error
 		return user, errors.New("Invalid credentials")
 	}
 
-	for i, k := range strings.Split(user.PublicKey, "\n") {
+	for i, k := range user.PublicKey {
 		storedPubKey, _, _, _, err := ssh.ParseAuthorizedKey([]byte(k))
 		if err != nil {
 			logger.Warn(logSender, "error parsing stored public key %d for user %v: %v", i, username, err)
@@ -170,7 +175,11 @@ func sqlCommonAddUser(user User) error {
 	if err != nil {
 		return err
 	}
-	_, err = stmt.Exec(user.Username, user.Password, user.PublicKey, user.HomeDir, user.UID, user.GID, user.MaxSessions, user.QuotaSize,
+	publicKeys, err := user.GetPublicKeysAsJSON()
+	if err != nil {
+		return err
+	}
+	_, err = stmt.Exec(user.Username, user.Password, string(publicKeys), user.HomeDir, user.UID, user.GID, user.MaxSessions, user.QuotaSize,
 		user.QuotaFiles, string(permissions), user.UploadBandwidth, user.DownloadBandwidth)
 	return err
 }
@@ -191,8 +200,12 @@ func sqlCommonUpdateUser(user User) error {
 	if err != nil {
 		return err
 	}
-	_, err = stmt.Exec(user.Password, user.PublicKey, user.HomeDir, user.UID, user.GID, user.MaxSessions, user.QuotaSize,
-		user.QuotaFiles, permissions, user.UploadBandwidth, user.DownloadBandwidth, user.ID)
+	publicKeys, err := user.GetPublicKeysAsJSON()
+	if err != nil {
+		return err
+	}
+	_, err = stmt.Exec(user.Password, string(publicKeys), user.HomeDir, user.UID, user.GID, user.MaxSessions, user.QuotaSize,
+		user.QuotaFiles, string(permissions), user.UploadBandwidth, user.DownloadBandwidth, user.ID)
 	return err
 }
 
@@ -229,7 +242,7 @@ func sqlCommonGetUsers(limit int, offset int, order string, username string) ([]
 			u, err := getUserFromDbRow(nil, rows)
 			// hide password and public key
 			u.Password = ""
-			u.PublicKey = ""
+			u.PublicKey = []string{}
 			if err == nil {
 				users = append(users, u)
 			} else {
@@ -264,7 +277,17 @@ func getUserFromDbRow(row *sql.Row, rows *sql.Rows) (User, error) {
 		user.Password = password.String
 	}
 	if publicKey.Valid {
-		user.PublicKey = publicKey.String
+		var list []string
+		err = json.Unmarshal([]byte(publicKey.String), &list)
+		if err == nil {
+			user.PublicKey = list
+		} else {
+			// compatibility layer: initially we store public keys as string newline delimited
+			// we need to remove this code in future
+			user.PublicKey = strings.Split(publicKey.String, "\n")
+			logger.Warn(logSender, "public keys loaded using compatibility mode, this will not work in future versions! "+
+				"Number of public keys loaded: %v, username: %v", len(user.PublicKey), user.Username)
+		}
 	}
 	if permissions.Valid {
 		var list []string

dataprovider/user.go

@@ -39,8 +39,8 @@ type User struct {
 	// Currently, as fallback, there is a clear text password checking but you should not store passwords
 	// as clear text and this support could be removed at any time, so please don't depend on it.
 	Password string `json:"password,omitempty"`
-	// PublicKey used for public key authentication. At least one between password and public key is mandatory
-	PublicKey string `json:"public_key,omitempty"`
+	// PublicKey used for public key authentication. At least one between password and a public key is mandatory
+	PublicKey []string `json:"public_key,omitempty"`
 	// The user cannot upload or download files outside this directory. Must be an absolute path
 	HomeDir string `json:"home_dir"`
 	// If sftpgo runs as root system user then the created files and directories will be assigned to this system UID
@@ -80,6 +80,11 @@ func (u *User) GetPermissionsAsJSON() ([]byte, error) {
 	return json.Marshal(u.Permissions)
 }
 
+// GetPublicKeysAsJSON returns the public keys as json byte array
+func (u *User) GetPublicKeysAsJSON() ([]byte, error) {
+	return json.Marshal(u.PublicKey)
+}
+
 // GetUID returns a validate uid, suitable for use with os.Chown
 func (u *User) GetUID() int {
 	if u.UID <= 0 || u.UID > 65535 {

sftpd/sftpd_test.go

@@ -465,7 +465,7 @@ func TestHomeSpecialChars(t *testing.T) {
 
 func TestLogin(t *testing.T) {
 	u := getTestUser(false)
-	u.PublicKey = testPubKey
+	u.PublicKey = []string{testPubKey}
 	user, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -497,7 +497,7 @@ func TestLogin(t *testing.T) {
 		defer client.Close()
 	}
 	// testPubKey1 is not authorized
-	user.PublicKey = testPubKey1
+	user.PublicKey = []string{testPubKey1}
 	user.Password = ""
 	_, err = api.UpdateUser(user, http.StatusOK)
 	if err != nil {
@@ -509,7 +509,7 @@ func TestLogin(t *testing.T) {
 		defer client.Close()
 	}
 	// login a user with multiple public keys, only the second one is valid
-	user.PublicKey = testPubKey1 + "\n" + testPubKey
+	user.PublicKey = []string{testPubKey1, testPubKey}
 	user.Password = ""
 	_, err = api.UpdateUser(user, http.StatusOK)
 	if err != nil {
@@ -538,7 +538,7 @@ func TestLoginAfterUserUpdateEmptyPwd(t *testing.T) {
 		t.Errorf("unable to add user: %v", err)
 	}
 	user.Password = ""
-	user.PublicKey = ""
+	user.PublicKey = []string{}
 	// password and public key should remain unchanged
 	_, err = api.UpdateUser(user, http.StatusOK)
 	if err != nil {
@@ -571,7 +571,7 @@ func TestLoginAfterUserUpdateEmptyPubKey(t *testing.T) {
 		t.Errorf("unable to add user: %v", err)
 	}
 	user.Password = ""
-	user.PublicKey = ""
+	user.PublicKey = []string{}
 	// password and public key should remain unchanged
 	_, err = api.UpdateUser(user, http.StatusOK)
 	if err != nil {
@@ -1178,7 +1178,7 @@ func getTestUser(usePubKey bool) dataprovider.User {
 		Permissions: allPerms,
 	}
 	if usePubKey {
-		user.PublicKey = testPubKey
+		user.PublicKey = []string{testPubKey}
 		user.Password = ""
 	}
 	return user