Browse Source

add a new permission for overwriting existing files

The upload permission is required to allow file overwrite
Nicola Murino 6 years ago
parent
commit
df96ea7e9f

+ 2 - 0
README.md

@@ -282,6 +282,7 @@ For each account the following properties can be configured:
     - `list` list items is allowed
     - `download` download files is allowed
     - `upload` upload files is allowed
+    - `overwrite` overwrite an existing file, while uploading, is allowed. `upload` permission is required to allow file overwrite
     - `delete` delete files or directories is allowed
     - `rename` rename files or directories is allowed
     - `create_dirs` create directories is allowed
@@ -400,6 +401,7 @@ The logs can be divided into the following categories:
 - [viper](https://github.com/spf13/viper)
 - [cobra](https://github.com/spf13/cobra)
 - [xid](https://github.com/rs/xid)
+- [nathanaelle/password](https://github.com/nathanaelle/password)
 
 Some code was initially taken from [Pterodactyl sftp server](https://github.com/pterodactyl/sftp-server)
 

+ 2 - 0
api/schema/openapi.yaml

@@ -511,6 +511,7 @@ components:
         - list
         - download
         - upload
+        - overwrite
         - delete
         - rename
         - create_dirs
@@ -521,6 +522,7 @@ components:
           * `list` - list items is allowed
           * `download` - download files is allowed
           * `upload` - upload files is allowed
+          * `overwrite` - overwrite an existing file, while uploading, is allowed. upload permission is required to allow file overwrite
           * `delete` - delete files or directories is allowed
           * `rename` - rename files or directories is allowed
           * `create_dirs` - create directories is allowed

+ 1 - 1
dataprovider/dataprovider.go

@@ -55,7 +55,7 @@ var (
 	provider           Provider
 	sqlPlaceholders    []string
 	validPerms         = []string{PermAny, PermListItems, PermDownload, PermUpload, PermDelete, PermRename,
-		PermCreateDirs, PermCreateSymlinks}
+		PermCreateDirs, PermCreateSymlinks, PermOverwrite}
 	hashPwdPrefixes = []string{argonPwdPrefix, bcryptPwdPrefix, pbkdf2SHA1Prefix, pbkdf2SHA256Prefix,
 		pbkdf2SHA512Prefix, sha512cryptPwdPrefix}
 	pbkdfPwdPrefixes   = []string{pbkdf2SHA1Prefix, pbkdf2SHA256Prefix, pbkdf2SHA512Prefix}

+ 3 - 0
dataprovider/user.go

@@ -17,6 +17,9 @@ const (
 	PermDownload = "download"
 	// upload files is allowed
 	PermUpload = "upload"
+	// overwrite an existing file, while uploading, is allowed
+	// upload permission is required to allow file overwrite
+	PermOverwrite = "overwrite"
 	// delete files or directories is allowed
 	PermDelete = "delete"
 	// rename files or directories is allowed

+ 3 - 2
scripts/README.md

@@ -41,7 +41,7 @@ Let's see a sample usage for each REST API.
 Command:
 
 ```
-python sftpgo_api_cli.py add-user test_username --password "test_pwd" --home-dir="/tmp/test_home_dir" --uid 33 --gid 1000 --max-sessions 2 --quota-size 0 --quota-files 3 --permissions "list" "download" "upload" "delete" "rename" "create_dirs" --upload-bandwidth 100 --download-bandwidth 60
+python sftpgo_api_cli.py add-user test_username --password "test_pwd" --home-dir="/tmp/test_home_dir" --uid 33 --gid 1000 --max-sessions 2 --quota-size 0 --quota-files 3 --permissions "list" "download" "upload" "delete" "rename" "create_dirs" "overwrite" --upload-bandwidth 100 --download-bandwidth 60
 ```
 
 Output:
@@ -62,7 +62,8 @@ Output:
     "upload",
     "delete",
     "rename",
-    "create_dirs"
+    "create_dirs",
+    "overwrite"
   ],
   "used_quota_size": 0,
   "used_quota_files": 0,

+ 2 - 2
scripts/sftpgo_api_cli.py

@@ -62,7 +62,7 @@ class SFTPGoApiRequests:
 					download_bandwidth=0):
 		user = {"id":user_id, "username":username, "uid":uid, "gid":gid,
 			"max_sessions":max_sessions, "quota_size":quota_size, "quota_files":quota_files,
-			"upload_bandwidth":upload_bandwidth,"download_bandwidth":download_bandwidth}
+			"upload_bandwidth":upload_bandwidth, "download_bandwidth":download_bandwidth}
 		if password:
 			user.update({"password":password})
 		if public_keys:
@@ -136,7 +136,7 @@ def addCommonUserArguments(parser):
 					help='Maximum size allowed as bytes. 0 means unlimited. Default: %(default)s')
 	parser.add_argument('-F', '--quota-files', type=int, default=0, help="default: %(default)s")
 	parser.add_argument('-G', '--permissions', type=str, nargs='+', default=[],
-					choices=['*', 'list', 'download', 'upload', 'delete', 'rename', 'create_dirs',
+					choices=['*', 'list', 'download', 'upload', 'overwrite', 'delete', 'rename', 'create_dirs',
 							'create_symlinks'], help='Default: %(default)s')
 	parser.add_argument('-U', '--upload-bandwidth', type=int, default=0,
 					help='Maximum upload bandwidth as KB/s, 0 means unlimited. Default: %(default)s')

+ 4 - 0
sftpd/handler.go

@@ -131,6 +131,10 @@ func (c Connection) Filewrite(request *sftp.Request) (io.WriterAt, error) {
 		return nil, sftp.ErrSshFxOpUnsupported
 	}
 
+	if !c.User.HasPerm(dataprovider.PermOverwrite) {
+		return nil, sftp.ErrSshFxPermissionDenied
+	}
+
 	return c.handleSFTPUploadToExistingFile(request.Pflags(), p, filePath, stat.Size())
 }
 

+ 8 - 1
sftpd/scp.go

@@ -242,7 +242,7 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 	updateConnectionActivity(c.connection.ID)
 	if !c.connection.User.HasPerm(dataprovider.PermUpload) {
 		err := fmt.Errorf("Permission denied")
-		c.connection.Log(logger.LevelWarn, logSenderSCP, "error uploading file: %#v, permission denied", uploadFilePath)
+		c.connection.Log(logger.LevelWarn, logSenderSCP, "cannot upload file: %#v, permission denied", uploadFilePath)
 		c.sendErrorMessage(err.Error())
 		return err
 	}
@@ -275,6 +275,13 @@ func (c *scpCommand) handleUpload(uploadFilePath string, sizeToRead int64) error
 		return err
 	}
 
+	if !c.connection.User.HasPerm(dataprovider.PermOverwrite) {
+		err := fmt.Errorf("Permission denied")
+		c.connection.Log(logger.LevelWarn, logSenderSCP, "cannot overwrite file: %#v, permission denied", uploadFilePath)
+		c.sendErrorMessage(err.Error())
+		return err
+	}
+
 	if uploadMode == uploadModeAtomic {
 		err = os.Rename(p, filePath)
 		if err != nil {

+ 85 - 7
sftpd/sftpd_test.go

@@ -1233,7 +1233,7 @@ func TestPermList(t *testing.T) {
 	usePubKey := true
 	u := getTestUser(usePubKey)
 	u.Permissions = []string{dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete, dataprovider.PermRename,
-		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks}
+		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite}
 	user, _, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -1263,7 +1263,7 @@ func TestPermDownload(t *testing.T) {
 	usePubKey := true
 	u := getTestUser(usePubKey)
 	u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermUpload, dataprovider.PermDelete, dataprovider.PermRename,
-		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks}
+		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite}
 	user, _, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -1305,7 +1305,7 @@ func TestPermUpload(t *testing.T) {
 	usePubKey := false
 	u := getTestUser(usePubKey)
 	u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermDelete, dataprovider.PermRename,
-		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks}
+		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite}
 	user, _, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -1334,11 +1334,48 @@ func TestPermUpload(t *testing.T) {
 	os.RemoveAll(user.GetHomeDir())
 }
 
+func TestPermOverwrite(t *testing.T) {
+	usePubKey := false
+	u := getTestUser(usePubKey)
+	u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete,
+		dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks}
+	user, _, err := api.AddUser(u, http.StatusOK)
+	if err != nil {
+		t.Errorf("unable to add user: %v", err)
+	}
+	client, err := getSftpClient(user, usePubKey)
+	if err != nil {
+		t.Errorf("unable to create sftp client: %v", err)
+	} else {
+		defer client.Close()
+		testFileName := "test_file.dat"
+		testFilePath := filepath.Join(homeBasePath, testFileName)
+		testFileSize := int64(65535)
+		err = createTestFile(testFilePath, testFileSize)
+		if err != nil {
+			t.Errorf("unable to create test file: %v", err)
+		}
+		err = sftpUploadFile(testFilePath, testFileName, testFileSize, client)
+		if err != nil {
+			t.Errorf("error uploading file: %v", err)
+		}
+		err = sftpUploadFile(testFilePath, testFileName, testFileSize, client)
+		if err == nil {
+			t.Errorf("file overwrite without permission should not succeed")
+		}
+	}
+	_, err = api.RemoveUser(user, http.StatusOK)
+	if err != nil {
+		t.Errorf("unable to remove user: %v", err)
+	}
+	os.RemoveAll(user.GetHomeDir())
+}
+
 func TestPermDelete(t *testing.T) {
 	usePubKey := false
 	u := getTestUser(usePubKey)
 	u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermRename,
-		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks}
+		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite}
 	user, _, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -1375,7 +1412,7 @@ func TestPermRename(t *testing.T) {
 	usePubKey := false
 	u := getTestUser(usePubKey)
 	u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete,
-		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks}
+		dataprovider.PermCreateDirs, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite}
 	user, _, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -1416,7 +1453,7 @@ func TestPermCreateDirs(t *testing.T) {
 	usePubKey := false
 	u := getTestUser(usePubKey)
 	u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete,
-		dataprovider.PermRename, dataprovider.PermCreateSymlinks}
+		dataprovider.PermRename, dataprovider.PermCreateSymlinks, dataprovider.PermOverwrite}
 	user, _, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -1453,7 +1490,7 @@ func TestPermSymlink(t *testing.T) {
 	usePubKey := false
 	u := getTestUser(usePubKey)
 	u.Permissions = []string{dataprovider.PermListItems, dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDelete,
-		dataprovider.PermRename, dataprovider.PermCreateDirs}
+		dataprovider.PermRename, dataprovider.PermCreateDirs, dataprovider.PermOverwrite}
 	user, _, err := api.AddUser(u, http.StatusOK)
 	if err != nil {
 		t.Errorf("unable to add user: %v", err)
@@ -1794,6 +1831,47 @@ func TestSCPPermUpload(t *testing.T) {
 	}
 }
 
+func TestSCPPermOverwrite(t *testing.T) {
+	if len(scpPath) == 0 {
+		t.Skip("scp command not found, unable to execute this test")
+	}
+	usePubKey := true
+	u := getTestUser(usePubKey)
+	u.Permissions = []string{dataprovider.PermUpload, dataprovider.PermCreateDirs}
+	user, _, err := api.AddUser(u, http.StatusOK)
+	if err != nil {
+		t.Errorf("unable to add user: %v", err)
+	}
+	testFileName := "test_file.dat"
+	testFilePath := filepath.Join(homeBasePath, testFileName)
+	testFileSize := int64(65536)
+	err = createTestFile(testFilePath, testFileSize)
+	if err != nil {
+		t.Errorf("unable to create test file: %v", err)
+	}
+	remoteUpPath := fmt.Sprintf("%[email protected]:%v", user.Username, "/tmp")
+	err = scpUpload(testFilePath, remoteUpPath, true, false)
+	if err != nil {
+		t.Errorf("scp upload error: %v", err)
+	}
+	err = scpUpload(testFilePath, remoteUpPath, true, false)
+	if err == nil {
+		t.Errorf("scp upload must fail, the user cannot ovewrite existing files")
+	}
+	err = os.Remove(testFilePath)
+	if err != nil {
+		t.Errorf("error removing test file")
+	}
+	err = os.RemoveAll(user.GetHomeDir())
+	if err != nil {
+		t.Errorf("error removing uploaded files")
+	}
+	_, err = api.RemoveUser(user, http.StatusOK)
+	if err != nil {
+		t.Errorf("unable to remove user: %v", err)
+	}
+}
+
 func TestSCPPermDownload(t *testing.T) {
 	if len(scpPath) == 0 {
 		t.Skip("scp command not found, unable to execute this test")