add separate permissions to delete and rename files and dirs

perm_delete and perm_rename still exist for backward compatibility,
now they are an alias to assign both new split permissions

.github/workflows/development.yml

@@ -280,7 +280,7 @@ jobs:
           - arch: armv7
             distro: ubuntu18.04
             go: latest
-            go-arch: arm
+            go-arch: arm7
     steps:
       - uses: actions/checkout@v2
         with:

.github/workflows/release.yml

@@ -213,9 +213,9 @@ jobs:
             tar-arch: ppc64le
           - arch: armv7
             distro: ubuntu18.04
-            go-arch: arm
+            go-arch: arm7
             deb-arch: armhf
-            rpm-arch: arm
+            rpm-arch: armv7hl
             tar-arch: armv7
 
     steps:

common/common_test.go

@@ -822,6 +822,23 @@ func TestHideConfidentialData(t *testing.T) {
 	a.HideConfidentialData()
 }
 
+func TestUserPerms(t *testing.T) {
+	u := dataprovider.User{}
+	u.Permissions = make(map[string][]string)
+	u.Permissions["/"] = []string{dataprovider.PermUpload, dataprovider.PermDelete}
+	assert.True(t, u.HasAnyPerm([]string{dataprovider.PermRename, dataprovider.PermDelete}, "/"))
+	assert.False(t, u.HasAnyPerm([]string{dataprovider.PermRename, dataprovider.PermCreateDirs}, "/"))
+	u.Permissions["/"] = []string{dataprovider.PermDelete, dataprovider.PermCreateDirs}
+	assert.True(t, u.HasPermsDeleteAll("/"))
+	assert.False(t, u.HasPermsRenameAll("/"))
+	u.Permissions["/"] = []string{dataprovider.PermDeleteDirs, dataprovider.PermDeleteFiles, dataprovider.PermRenameDirs}
+	assert.True(t, u.HasPermsDeleteAll("/"))
+	assert.False(t, u.HasPermsRenameAll("/"))
+	u.Permissions["/"] = []string{dataprovider.PermDeleteDirs, dataprovider.PermRenameFiles, dataprovider.PermRenameDirs}
+	assert.False(t, u.HasPermsDeleteAll("/"))
+	assert.True(t, u.HasPermsRenameAll("/"))
+}
+
 func BenchmarkBcryptHashing(b *testing.B) {
 	bcryptPassword := "bcryptpassword"
 	for i := 0; i < b.N; i++ {

common/connection.go

@@ -259,7 +259,7 @@ func (c *BaseConnection) CreateDir(virtualPath string) error {
 
 // IsRemoveFileAllowed returns an error if removing this file is not allowed
 func (c *BaseConnection) IsRemoveFileAllowed(virtualPath string) error {
-	if !c.User.HasPerm(dataprovider.PermDelete, path.Dir(virtualPath)) {
+	if !c.User.HasAnyPerm([]string{dataprovider.PermDeleteFiles, dataprovider.PermDelete}, path.Dir(virtualPath)) {
 		return c.GetPermissionDeniedError()
 	}
 	if !c.User.IsFileAllowed(virtualPath) {
@@ -323,7 +323,7 @@ func (c *BaseConnection) IsRemoveDirAllowed(fs vfs.Fs, fsPath, virtualPath strin
 		c.Log(logger.LevelWarn, "removing a directory mapped as virtual folder is not allowed: %#v", fsPath)
 		return c.GetPermissionDeniedError()
 	}
-	if !c.User.HasPerm(dataprovider.PermDelete, path.Dir(virtualPath)) {
+	if !c.User.HasAnyPerm([]string{dataprovider.PermDeleteDirs, dataprovider.PermDelete}, path.Dir(virtualPath)) {
 		return c.GetPermissionDeniedError()
 	}
 	return nil
@@ -650,11 +650,11 @@ func (c *BaseConnection) checkRecursiveRenameDirPermissions(fsSrc, fsDst vfs.Fs,
 		// inside the parent path was checked. If the current dir has no subdirs with defined permissions inside it
 		// and it has all the possible permissions we can stop scanning
 		if !c.User.HasPermissionsInside(path.Dir(virtualSrcPath)) && !c.User.HasPermissionsInside(path.Dir(virtualDstPath)) {
-			if c.User.HasPerm(dataprovider.PermRename, path.Dir(virtualSrcPath)) &&
-				c.User.HasPerm(dataprovider.PermRename, path.Dir(virtualDstPath)) {
+			if c.User.HasPermsRenameAll(path.Dir(virtualSrcPath)) &&
+				c.User.HasPermsRenameAll(path.Dir(virtualDstPath)) {
 				return ErrSkipPermissionsCheck
 			}
-			if c.User.HasPerm(dataprovider.PermDelete, path.Dir(virtualSrcPath)) &&
+			if c.User.HasPermsDeleteAll(path.Dir(virtualSrcPath)) &&
 				c.User.HasPerms(dstPerms, path.Dir(virtualDstPath)) {
 				return ErrSkipPermissionsCheck
 			}
@@ -673,19 +673,42 @@ func (c *BaseConnection) checkRecursiveRenameDirPermissions(fsSrc, fsDst vfs.Fs,
 }
 
 func (c *BaseConnection) hasRenamePerms(virtualSourcePath, virtualTargetPath string, fi os.FileInfo) bool {
-	if c.User.HasPerm(dataprovider.PermRename, path.Dir(virtualSourcePath)) &&
-		c.User.HasPerm(dataprovider.PermRename, path.Dir(virtualTargetPath)) {
+	if c.User.HasPermsRenameAll(path.Dir(virtualSourcePath)) &&
+		c.User.HasPermsRenameAll(path.Dir(virtualTargetPath)) {
 		return true
 	}
-	if !c.User.HasPerm(dataprovider.PermDelete, path.Dir(virtualSourcePath)) {
+	if fi == nil {
+		// we don't know if this is a file or a directory we have to check all the required permissions
+		if !c.User.HasPermsDeleteAll(path.Dir(virtualSourcePath)) {
+			return false
+		}
+		dstPerms := []string{
+			dataprovider.PermCreateDirs,
+			dataprovider.PermUpload,
+			dataprovider.PermCreateSymlinks,
+		}
+		return c.User.HasPerms(dstPerms, path.Dir(virtualTargetPath))
+	}
+	if fi.IsDir() {
+		if c.User.HasPerm(dataprovider.PermRenameDirs, path.Dir(virtualSourcePath)) &&
+			c.User.HasPerm(dataprovider.PermRenameDirs, path.Dir(virtualTargetPath)) {
+			return true
+		}
+		if !c.User.HasAnyPerm([]string{dataprovider.PermDeleteDirs, dataprovider.PermDelete}, path.Dir(virtualSourcePath)) {
+			return false
+		}
+		return c.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(virtualTargetPath))
+	}
+	// file or symlink
+	if c.User.HasPerm(dataprovider.PermRenameFiles, path.Dir(virtualSourcePath)) &&
+		c.User.HasPerm(dataprovider.PermRenameFiles, path.Dir(virtualTargetPath)) {
+		return true
+	}
+	if !c.User.HasAnyPerm([]string{dataprovider.PermDeleteFiles, dataprovider.PermDelete}, path.Dir(virtualSourcePath)) {
 		return false
 	}
-	if fi != nil {
-		if fi.IsDir() {
-			return c.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(virtualTargetPath))
-		} else if fi.Mode()&os.ModeSymlink != 0 {
-			return c.User.HasPerm(dataprovider.PermCreateSymlinks, path.Dir(virtualTargetPath))
-		}
+	if fi.Mode()&os.ModeSymlink != 0 {
+		return c.User.HasPerm(dataprovider.PermCreateSymlinks, path.Dir(virtualTargetPath))
 	}
 	return c.User.HasPerm(dataprovider.PermUpload, path.Dir(virtualTargetPath))
 }

common/connection_test.go

@@ -148,6 +148,31 @@ func TestRenameVirtualFolders(t *testing.T) {
 	assert.False(t, res)
 }
 
+func TestRenamePerms(t *testing.T) {
+	src := "source"
+	target := "target"
+	u := dataprovider.User{}
+	u.Permissions = map[string][]string{}
+	u.Permissions["/"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload, dataprovider.PermCreateSymlinks,
+		dataprovider.PermDeleteFiles}
+	conn := NewBaseConnection("", ProtocolSFTP, "", "", u)
+	assert.False(t, conn.hasRenamePerms(src, target, nil))
+	u.Permissions["/"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload, dataprovider.PermCreateSymlinks,
+		dataprovider.PermDeleteFiles, dataprovider.PermDeleteDirs}
+	assert.True(t, conn.hasRenamePerms(src, target, nil))
+	u.Permissions["/"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload, dataprovider.PermDeleteFiles,
+		dataprovider.PermDeleteDirs}
+	assert.False(t, conn.hasRenamePerms(src, target, nil))
+
+	info := vfs.NewFileInfo(src, true, 0, time.Now(), false)
+	u.Permissions["/"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload, dataprovider.PermDeleteFiles}
+	assert.False(t, conn.hasRenamePerms(src, target, info))
+	u.Permissions["/"] = []string{dataprovider.PermCreateDirs, dataprovider.PermUpload, dataprovider.PermDeleteDirs}
+	assert.True(t, conn.hasRenamePerms(src, target, info))
+	u.Permissions["/"] = []string{dataprovider.PermDownload, dataprovider.PermUpload, dataprovider.PermDeleteDirs}
+	assert.False(t, conn.hasRenamePerms(src, target, info))
+}
+
 func TestUpdateQuotaAfterRename(t *testing.T) {
 	user := dataprovider.User{
 		BaseUser: sdk.BaseUser{

common/dataretention.go

@@ -236,13 +236,13 @@ func (c *RetentionCheck) removeFile(virtualPath string, info os.FileInfo) error
 }
 
 func (c *RetentionCheck) cleanupFolder(folderPath string) error {
-	cleanupPerms := []string{dataprovider.PermListItems, dataprovider.PermDelete}
+	deleteFilesPerms := []string{dataprovider.PermDelete, dataprovider.PermDeleteFiles}
 	startTime := time.Now()
 	result := &folderRetentionCheckResult{
 		Path: folderPath,
 	}
 	c.results = append(c.results, result)
-	if !c.conn.User.HasPerms(cleanupPerms, folderPath) {
+	if !c.conn.User.HasPerm(dataprovider.PermListItems, folderPath) || !c.conn.User.HasAnyPerm(deleteFilesPerms, folderPath) {
 		result.Elapsed = time.Since(startTime)
 		result.Info = "data retention check skipped: no permissions"
 		c.conn.Log(logger.LevelInfo, "user %#v does not have permissions to check retention on %#v, retention check skipped",
@@ -316,7 +316,11 @@ func (c *RetentionCheck) cleanupFolder(folderPath string) error {
 }
 
 func (c *RetentionCheck) checkEmptyDirRemoval(folderPath string) {
-	if folderPath != "/" && c.conn.User.HasPerm(dataprovider.PermDelete, path.Dir(folderPath)) {
+	if folderPath != "/" && c.conn.User.HasAnyPerm([]string{
+		dataprovider.PermDelete,
+		dataprovider.PermDeleteDirs,
+	}, path.Dir(folderPath),
+	) {
 		files, err := c.conn.ListDir(folderPath)
 		if err == nil && len(files) == 0 {
 			err = c.conn.RemoveDir(folderPath)

common/protocol_test.go

@@ -2668,6 +2668,92 @@ func TestRenameSymlink(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestSplittedDeletePerms(t *testing.T) {
+	u := getTestUser()
+	u.Permissions["/"] = []string{dataprovider.PermListItems, dataprovider.PermUpload, dataprovider.PermDeleteDirs,
+		dataprovider.PermCreateDirs}
+	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+	conn, client, err := getSftpClient(user)
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+		err = writeSFTPFile(testFileName, 4096, client)
+		assert.NoError(t, err)
+		err = client.Remove(testFileName)
+		assert.Error(t, err)
+		err = client.Mkdir(testDir)
+		assert.NoError(t, err)
+		err = client.RemoveDirectory(testDir)
+		assert.NoError(t, err)
+	}
+	u.Permissions["/"] = []string{dataprovider.PermListItems, dataprovider.PermUpload, dataprovider.PermDeleteFiles,
+		dataprovider.PermCreateDirs, dataprovider.PermOverwrite}
+	_, _, err = httpdtest.UpdateUser(u, http.StatusOK, "")
+	assert.NoError(t, err)
+
+	conn, client, err = getSftpClient(user)
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+		err = writeSFTPFile(testFileName, 4096, client)
+		assert.NoError(t, err)
+		err = client.Remove(testFileName)
+		assert.NoError(t, err)
+		err = client.Mkdir(testDir)
+		assert.NoError(t, err)
+		err = client.RemoveDirectory(testDir)
+		assert.Error(t, err)
+	}
+	_, err = httpdtest.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(user.GetHomeDir())
+	assert.NoError(t, err)
+}
+
+func TestSplittedRenamePerms(t *testing.T) {
+	u := getTestUser()
+	u.Permissions["/"] = []string{dataprovider.PermListItems, dataprovider.PermUpload, dataprovider.PermRenameDirs,
+		dataprovider.PermCreateDirs}
+	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+	conn, client, err := getSftpClient(user)
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+		err = writeSFTPFile(testFileName, 4096, client)
+		assert.NoError(t, err)
+		err = client.Mkdir(testDir)
+		assert.NoError(t, err)
+		err = client.Rename(testFileName, testFileName+"_renamed")
+		assert.Error(t, err)
+		err = client.Rename(testDir, testDir+"_renamed")
+		assert.NoError(t, err)
+	}
+	u.Permissions["/"] = []string{dataprovider.PermListItems, dataprovider.PermUpload, dataprovider.PermRenameFiles,
+		dataprovider.PermCreateDirs, dataprovider.PermOverwrite}
+	_, _, err = httpdtest.UpdateUser(u, http.StatusOK, "")
+	assert.NoError(t, err)
+
+	conn, client, err = getSftpClient(user)
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+		err = writeSFTPFile(testFileName, 4096, client)
+		assert.NoError(t, err)
+		err = client.Mkdir(testDir)
+		assert.NoError(t, err)
+		err = client.Rename(testFileName, testFileName+"_renamed")
+		assert.NoError(t, err)
+		err = client.Rename(testDir, testDir+"_renamed")
+		assert.Error(t, err)
+	}
+	_, err = httpdtest.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(user.GetHomeDir())
+	assert.NoError(t, err)
+}
+
 func TestSFTPLoopError(t *testing.T) {
 	user1 := getTestUser()
 	user2 := getTestUser()

dataprovider/dataprovider.go

@@ -114,8 +114,9 @@ var (
 	SupportedProviders = []string{SQLiteDataProviderName, PGSQLDataProviderName, MySQLDataProviderName,
 		BoltDataProviderName, MemoryDataProviderName, CockroachDataProviderName}
 	// ValidPerms defines all the valid permissions for a user
-	ValidPerms = []string{PermAny, PermListItems, PermDownload, PermUpload, PermOverwrite, PermRename, PermDelete,
-		PermCreateDirs, PermCreateSymlinks, PermChmod, PermChown, PermChtimes}
+	ValidPerms = []string{PermAny, PermListItems, PermDownload, PermUpload, PermOverwrite, PermCreateDirs, PermRename,
+		PermRenameFiles, PermRenameDirs, PermDelete, PermDeleteFiles, PermDeleteDirs, PermCreateSymlinks, PermChmod,
+		PermChown, PermChtimes}
 	// ValidLoginMethods defines all the valid login methods
 	ValidLoginMethods = []string{SSHLoginMethodPublicKey, LoginMethodPassword, SSHLoginMethodKeyboardInteractive,
 		SSHLoginMethodKeyAndPassword, SSHLoginMethodKeyAndKeyboardInt, LoginMethodTLSCertificate,

dataprovider/user.go

@@ -38,8 +38,16 @@ const (
 	PermOverwrite = "overwrite"
 	// delete files or directories is allowed
 	PermDelete = "delete"
+	// delete files is allowed
+	PermDeleteFiles = "delete_files"
+	// delete directories is allowed
+	PermDeleteDirs = "delete_dirs"
 	// rename files or directories is allowed
 	PermRename = "rename"
+	// rename files is allowed
+	PermRenameFiles = "rename_files"
+	// rename directories is allowed
+	PermRenameDirs = "rename_dirs"
 	// create directories is allowed
 	PermCreateDirs = "create_dirs"
 	// create symbolic links is allowed
@@ -66,6 +74,9 @@ const (
 
 var (
 	errNoMatchingVirtualFolder = errors.New("no matching virtual folder found")
+	permsRenameAny             = []string{PermRename, PermRenameDirs, PermRenameFiles}
+	permsDeleteAny             = []string{PermDelete, PermDeleteDirs, PermDeleteFiles}
+	permsCreateAny             = []string{PermUpload, PermCreateDirs}
 )
 
 // User defines a SFTPGo user
@@ -564,7 +575,21 @@ func (u *User) HasPerm(permission, path string) bool {
 	return util.IsStringInSlice(permission, perms)
 }
 
-// HasPerms return true if the user has all the given permissions
+// HasAnyPerm returns true if the user has at least one of the given permissions
+func (u *User) HasAnyPerm(permissions []string, path string) bool {
+	perms := u.GetPermissionsForPath(path)
+	if util.IsStringInSlice(PermAny, perms) {
+		return true
+	}
+	for _, permission := range permissions {
+		if util.IsStringInSlice(permission, perms) {
+			return true
+		}
+	}
+	return false
+}
+
+// HasPerms returns true if the user has all the given permissions
 func (u *User) HasPerms(permissions []string, path string) bool {
 	perms := u.GetPermissionsForPath(path)
 	if util.IsStringInSlice(PermAny, perms) {
@@ -578,6 +603,46 @@ func (u *User) HasPerms(permissions []string, path string) bool {
 	return true
 }
 
+// HasPermsDeleteAll returns true if the user can delete both files and directories
+// for the given path
+func (u *User) HasPermsDeleteAll(path string) bool {
+	perms := u.GetPermissionsForPath(path)
+	canDeleteFiles := false
+	canDeleteDirs := false
+	for _, permission := range perms {
+		if permission == PermAny || permission == PermDelete {
+			return true
+		}
+		if permission == PermDeleteFiles {
+			canDeleteFiles = true
+		}
+		if permission == PermDeleteDirs {
+			canDeleteDirs = true
+		}
+	}
+	return canDeleteFiles && canDeleteDirs
+}
+
+// HasPermsRenameAll returns true if the user can rename both files and directories
+// for the given path
+func (u *User) HasPermsRenameAll(path string) bool {
+	perms := u.GetPermissionsForPath(path)
+	canRenameFiles := false
+	canRenameDirs := false
+	for _, permission := range perms {
+		if permission == PermAny || permission == PermRename {
+			return true
+		}
+		if permission == PermRenameFiles {
+			canRenameFiles = true
+		}
+		if permission == PermRenameDirs {
+			canRenameDirs = true
+		}
+	}
+	return canRenameFiles && canRenameDirs
+}
+
 // HasNoQuotaRestrictions returns true if no quota restrictions need to be applyed
 func (u *User) HasNoQuotaRestrictions(checkFiles bool) bool {
 	if u.QuotaSize == 0 && (!checkFiles || u.QuotaFiles == 0) {
@@ -782,13 +847,13 @@ func (u *User) CanRenameFromWeb(src, dest string) bool {
 	if util.IsStringInSlice(sdk.WebClientWriteDisabled, u.Filters.WebClient) {
 		return false
 	}
-	if u.HasPerm(PermRename, src) && u.HasPerm(PermRename, dest) {
+	if u.HasAnyPerm(permsRenameAny, src) && u.HasAnyPerm(permsRenameAny, dest) {
 		return true
 	}
-	if !u.HasPerm(PermDelete, src) {
+	if !u.HasAnyPerm(permsDeleteAny, src) {
 		return false
 	}
-	return u.HasPerm(PermUpload, dest) || u.HasPerm(PermCreateDirs, dest)
+	return u.HasAnyPerm(permsCreateAny, dest)
 }
 
 // CanDeleteFromWeb returns true if the client can delete objects from the web UI.
@@ -797,7 +862,7 @@ func (u *User) CanDeleteFromWeb(target string) bool {
 	if util.IsStringInSlice(sdk.WebClientWriteDisabled, u.Filters.WebClient) {
 		return false
 	}
-	return u.HasPerm(PermDelete, target)
+	return u.HasAnyPerm(permsDeleteAny, target)
 }
 
 // GetSignature returns a signature for this admin.

httpd/schema/openapi.yaml

@@ -3424,7 +3424,7 @@ paths:
       tags:
         - user APIs
       summary: Rename a directory
-      description: Rename a directory for the logged in user. The rename is allowed for empty directory or for non empty, local directories, with no virtual folders inside
+      description: Rename a directory for the logged in user. The rename is allowed for empty directory or for non empty local directories, with no virtual folders inside
       operationId: rename_user_dir
       parameters:
         - in: query
@@ -3772,7 +3772,11 @@ components:
         - upload
         - overwrite
         - delete
+        - delete_files
+        - delete_dirs
         - rename
+        - rename_files
+        - rename_dirs
         - create_dirs
         - create_symlinks
         - chmod
@@ -3786,7 +3790,11 @@ components:
           * `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
+          * `delete_files` - delete files is allowed
+          * `delete_dirs` - delete directories is allowed
           * `rename` - rename files or directories is allowed
+          * `rename_files` - rename files is allowed
+          * `rename_dirs` - rename directories is allowed
           * `create_dirs` - create directories is allowed
           * `create_symlinks` - create links is allowed
           * `chmod` changing file or directory permissions is allowed

pkgs/build.sh

@@ -1,12 +1,11 @@
 #!/bin/bash
 
-NFPM_VERSION=2.8.0
+NFPM_VERSION=2.9.2
 NFPM_ARCH=${NFPM_ARCH:-amd64}
 if [ -z ${SFTPGO_VERSION} ]
 then
   LATEST_TAG=$(git describe --tags $(git rev-list --tags --max-count=1))
   NUM_COMMITS_FROM_TAG=$(git rev-list ${LATEST_TAG}.. --count)
-  #COMMIT_HASH=$(git rev-parse --short HEAD)
   VERSION=$(echo "${LATEST_TAG}" | awk -F. -v OFS=. '{$NF++;print}')-dev.${NUM_COMMITS_FROM_TAG}
 else
   VERSION=${SFTPGO_VERSION}
@@ -87,10 +86,11 @@ contents:
     dst: "/etc/sftpgo/sftpgo.json"
     type: "config|noreplace"
 
+  - dst: "/srv/sftpgo"
+    type: dir
 
-empty_folders:
-  - /var/lib/sftpgo
-  - /srv/sftpgo
+  - dst: "/var/lib/sftpgo"
+    type: dir
 
 overrides:
   deb: