restore fast path for recursive permissions check and update some docs

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

common/connection.go

@@ -502,7 +502,8 @@ func (c *BaseConnection) Rename(virtualSourcePath, virtualTargetPath string) err
 				virtualSourcePath)
 			return c.GetOpUnsupportedError()
 		}
-		if err = c.checkRecursiveRenameDirPermissions(fsSrc, fsDst, fsSourcePath, fsTargetPath); err != nil {
+		if err = c.checkRecursiveRenameDirPermissions(fsSrc, fsDst, fsSourcePath, fsTargetPath,
+			virtualSourcePath, virtualTargetPath, srcInfo); err != nil {
 			c.Log(logger.LevelDebug, "error checking recursive permissions before renaming %#v: %+v", fsSourcePath, err)
 			return err
 		}
@@ -767,7 +768,24 @@ func (c *BaseConnection) truncateFile(fs vfs.Fs, fsPath, virtualPath string, siz
 	return err
 }
 
-func (c *BaseConnection) checkRecursiveRenameDirPermissions(fsSrc, fsDst vfs.Fs, sourcePath, targetPath string) error {
+func (c *BaseConnection) checkRecursiveRenameDirPermissions(fsSrc, fsDst vfs.Fs, sourcePath, targetPath,
+	virtualSourcePath, virtualTargetPath string, fi os.FileInfo,
+) error {
+	if !c.User.HasPermissionsInside(virtualSourcePath) &&
+		!c.User.HasPermissionsInside(virtualTargetPath) {
+		if !c.isRenamePermitted(fsSrc, fsDst, sourcePath, targetPath, virtualSourcePath, virtualTargetPath, fi) {
+			c.Log(logger.LevelInfo, "rename %#v -> %#v is not allowed, virtual destination path: %#v",
+				sourcePath, targetPath, virtualTargetPath)
+			return c.GetPermissionDeniedError()
+		}
+		// if all rename permissions are granted we have finished, otherwise we have to walk
+		// because we could have the rename dir permission but not the rename file and the dir to
+		// rename could contain files
+		if c.User.HasPermsRenameAll(path.Dir(virtualSourcePath)) && c.User.HasPermsRenameAll(path.Dir(virtualTargetPath)) {
+			return nil
+		}
+	}
+
 	return fsSrc.Walk(sourcePath, func(walkedPath string, info os.FileInfo, err error) error {
 		if err != nil {
 			return c.GetFsError(fsSrc, err)
@@ -810,7 +828,9 @@ func (c *BaseConnection) hasRenamePerms(virtualSourcePath, virtualTargetPath str
 		c.User.HasAnyPerm(perms, path.Dir(virtualTargetPath))
 }
 
-func (c *BaseConnection) isRenamePermitted(fsSrc, fsDst vfs.Fs, fsSourcePath, fsTargetPath, virtualSourcePath, virtualTargetPath string, fi os.FileInfo) bool {
+func (c *BaseConnection) isRenamePermitted(fsSrc, fsDst vfs.Fs, fsSourcePath, fsTargetPath, virtualSourcePath,
+	virtualTargetPath string, fi os.FileInfo,
+) bool {
 	if !c.isLocalOrSameFolderRename(virtualSourcePath, virtualTargetPath) {
 		c.Log(logger.LevelInfo, "rename %#v->%#v is not allowed: the paths must be local or on the same virtual folder",
 			virtualSourcePath, virtualTargetPath)

common/connection_test.go

@@ -116,10 +116,28 @@ func TestSetStatMode(t *testing.T) {
 }
 
 func TestRecursiveRenameWalkError(t *testing.T) {
-	fs := vfs.NewOsFs("", os.TempDir(), "")
-	conn := NewBaseConnection("", ProtocolWebDAV, "", "", dataprovider.User{})
-	err := conn.checkRecursiveRenameDirPermissions(fs, fs, "/source", "/target")
+	fs := vfs.NewOsFs("", filepath.Clean(os.TempDir()), "")
+	conn := NewBaseConnection("", ProtocolWebDAV, "", "", dataprovider.User{
+		BaseUser: sdk.BaseUser{
+			Permissions: map[string][]string{
+				"/": {dataprovider.PermListItems, dataprovider.PermUpload,
+					dataprovider.PermDownload, dataprovider.PermRenameDirs},
+			},
+		},
+	})
+	err := conn.checkRecursiveRenameDirPermissions(fs, fs, filepath.Join(os.TempDir(), "/source"),
+		filepath.Join(os.TempDir(), "/target"), "/source", "/target",
+		vfs.NewFileInfo("source", true, 0, time.Now(), false))
 	assert.ErrorIs(t, err, os.ErrNotExist)
+	conn.User.Permissions["/"] = []string{dataprovider.PermListItems, dataprovider.PermUpload,
+		dataprovider.PermDownload, dataprovider.PermRenameFiles}
+	// no dir rename permission, the quick check path returns permission error without walking
+	err = conn.checkRecursiveRenameDirPermissions(fs, fs, filepath.Join(os.TempDir(), "/source"),
+		filepath.Join(os.TempDir(), "/target"), "/source", "/target",
+		vfs.NewFileInfo("source", true, 0, time.Now(), false))
+	if assert.Error(t, err) {
+		assert.EqualError(t, err, conn.GetPermissionDeniedError().Error())
+	}
 }
 
 func TestCrossRenameFsErrors(t *testing.T) {

dataprovider/user.go

@@ -743,7 +743,10 @@ func (u *User) HasVirtualFoldersInside(virtualPath string) bool {
 // HasPermissionsInside returns true if the specified virtualPath has no permissions itself and
 // no subdirs with defined permissions
 func (u *User) HasPermissionsInside(virtualPath string) bool {
-	for dir := range u.Permissions {
+	for dir, perms := range u.Permissions {
+		if len(perms) == 1 && perms[0] == PermAny {
+			continue
+		}
 		if dir == virtualPath {
 			return true
 		} else if len(dir) > len(virtualPath) {

docs/httpfs.md

@@ -2,6 +2,8 @@
 
 SFTPGo can use custom storage backend implementations compliant with the REST API documented [here](./../openapi/httpfs.yaml).
 
+:warning: HTTPFs is a work in progress and makes no API stability promises.
+
 The only required parameter is the HTTP/S endpoint that SFTPGo must use to make API calls.
 If you define `http://127.0.0.1:9999/api/v1` as endpoint, SFTPGo will add the API path, for example for the `stat` API it will invoke `http://127.0.0.1:9999/api/v1/stat/{name}`.
 

docs/repo.md

@@ -9,7 +9,7 @@ Supported distributions:
 - Debian 10 "buster"
 - Debian 11 "bullseye"
 
-Import the public key used by the package management system using the following command:
+Import the public key used by the package management system:
 
 ```shell
 curl -sS https://ftp.osuosl.org/pub/sftpgo/apt/gpg.key | sudo gpg --dearmor -o /usr/share/keyrings/sftpgo-archive-keyring.gpg
@@ -39,7 +39,7 @@ sudo apt install sftpgo
 
 The YUM repository supports generic Red Hat based distributions.
 
-Create the SFTPGo repository using the following command:
+Create the SFTPGo repository:
 
 ```shell
 ARCH=`uname -m`

httpd/httpd_test.go

@@ -2429,6 +2429,12 @@ func TestAddUserInvalidFsConfig(t *testing.T) {
 	if assert.NoError(t, err) {
 		assert.Contains(t, string(resp), "cannot save a user with a redacted secret")
 	}
+	u.FsConfig.HTTPConfig.APIKey = nil
+	u.FsConfig.HTTPConfig.Endpoint = "/api/v1"
+	_, resp, err = httpdtest.AddUser(u, http.StatusBadRequest)
+	if assert.NoError(t, err) {
+		assert.Contains(t, string(resp), "invalid endpoint schema")
+	}
 }
 
 func TestUserRedactedPassword(t *testing.T) {

openapi/httpfs.yaml

@@ -3,7 +3,9 @@ tags:
   - name: fs
 info:
   title: SFTPGo HTTPFs
-  description: 'SFTPGo HTTP Filesystem API'
+  description: |
+    SFTPGo can use custom storage backend implementations compliant with the API defined here.
+    HTTPFs is a work in progress and makes no API stability promises.
   version: 0.1.0
 servers:
 - url: /v1

sftpd/internal_test.go

@@ -1991,7 +1991,18 @@ func TestRecursiveCopyErrors(t *testing.T) {
 		args:       []string{"adir", "another"},
 	}
 	// try to copy a missing directory
-	err = sshCmd.checkRecursiveCopyPermissions(fs, fs, "adir", "another", "/another")
+	sshCmd.connection.User.Permissions["/another"] = []string{
+		dataprovider.PermCreateDirs,
+		dataprovider.PermCreateSymlinks,
+		dataprovider.PermListItems,
+	}
+	err = sshCmd.checkRecursiveCopyPermissions(fs, fs, "adir", "another/sub", "/adir", "/another/sub")
+	assert.Error(t, err)
+	sshCmd.connection.User.Permissions["/another"] = []string{
+		dataprovider.PermListItems,
+		dataprovider.PermCreateDirs,
+	}
+	err = sshCmd.checkRecursiveCopyPermissions(fs, fs, "adir", "another", "/adir/sub", "/another/sub/dir")
 	assert.Error(t, err)
 }
 

sftpd/sftpd_test.go

@@ -4799,10 +4799,9 @@ func TestVirtualFolders(t *testing.T) {
 		assert.NoError(t, err)
 		err = sftpUploadFile(testFilePath, path.Join("vdir2", testFileName), testFileSize, client)
 		assert.NoError(t, err)
-		// we don't have upload permission on testDir, we can only create dirs
+		// we don't have rename permission in testDir and vdir2 contains a file
 		err = client.Rename("vdir2", testDir)
 		assert.Error(t, err)
-		// on testDir1 only symlink aren't allowed
 		err = client.Rename("vdir2", testDir1)
 		assert.NoError(t, err)
 		err = client.Rename(testDir1, "vdir2")
@@ -8633,7 +8632,8 @@ func TestSSHCopy(t *testing.T) {
 			assert.NoError(t, err)
 			err = os.Chmod(subPath, 0001)
 			assert.NoError(t, err)
-			// checkRecursiveCopyPermissions will fail scanning subdirs
+			// c.connection.fs.GetDirSize(fsSourcePath) will fail scanning subdirs
+			// checkRecursiveCopyPermissions will work since it will skip subdirs with no permissions
 			_, err = runSSHCommand(fmt.Sprintf("sftpgo-copy %v %v", vdirPath1, "newdir"), user, usePubKey)
 			assert.Error(t, err)
 			err = os.Chmod(subPath, os.ModePerm)

sftpd/ssh_cmd.go

@@ -589,10 +589,28 @@ func (c *sshCommand) hasCopyPermissions(sshSourcePath, sshDestPath string, srcIn
 }
 
 // fsSourcePath must be a directory
-func (c *sshCommand) checkRecursiveCopyPermissions(fsSrc vfs.Fs, fsDst vfs.Fs, fsSourcePath, fsDestPath, sshDestPath string) error {
+func (c *sshCommand) checkRecursiveCopyPermissions(fsSrc vfs.Fs, fsDst vfs.Fs, fsSourcePath, fsDestPath,
+	sshSourcePath, sshDestPath string,
+) error {
 	if !c.connection.User.HasPerm(dataprovider.PermCreateDirs, path.Dir(sshDestPath)) {
 		return common.ErrPermissionDenied
 	}
+	if !c.connection.User.HasPermissionsInside(sshSourcePath) &&
+		!c.connection.User.HasPermissionsInside(sshDestPath) {
+		// if there are no subdirs with defined permissions we can just check source and destination paths
+		dstPerms := []string{
+			dataprovider.PermCreateDirs,
+			dataprovider.PermCreateSymlinks,
+			dataprovider.PermUpload,
+		}
+		if c.connection.User.HasPerm(dataprovider.PermListItems, sshSourcePath) &&
+			c.connection.User.HasPerms(dstPerms, sshDestPath) {
+			return nil
+		}
+		// we don't return an error here because we checked all the required permissions above
+		// for example the directory could not have symlinks inside, so we have to walk to check
+		// permissions for each item
+	}
 
 	return fsSrc.Walk(fsSourcePath, func(walkedPath string, info os.FileInfo, err error) error {
 		if err != nil {
@@ -608,9 +626,11 @@ func (c *sshCommand) checkRecursiveCopyPermissions(fsSrc vfs.Fs, fsDst vfs.Fs, f
 	})
 }
 
-func (c *sshCommand) checkCopyPermissions(fsSrc vfs.Fs, fsDst vfs.Fs, fsSourcePath, fsDestPath, sshSourcePath, sshDestPath string, info os.FileInfo) error {
+func (c *sshCommand) checkCopyPermissions(fsSrc vfs.Fs, fsDst vfs.Fs, fsSourcePath, fsDestPath, sshSourcePath,
+	sshDestPath string, info os.FileInfo,
+) error {
 	if info.IsDir() {
-		return c.checkRecursiveCopyPermissions(fsSrc, fsDst, fsSourcePath, fsDestPath, sshDestPath)
+		return c.checkRecursiveCopyPermissions(fsSrc, fsDst, fsSourcePath, fsDestPath, sshSourcePath, sshDestPath)
 	}
 	if !c.hasCopyPermissions(sshSourcePath, sshDestPath, info) {
 		return c.connection.GetPermissionDeniedError()

vfs/httpfs.go

@@ -31,6 +31,10 @@ const (
 	httpFsName = "httpfs"
 )
 
+var (
+	supportedEndpointSchema = []string{"http://", "https://"}
+)
+
 // HTTPFsConfig defines the configuration for HTTP based filesystem
 type HTTPFsConfig struct {
 	sdk.BaseHTTPFsConfig
@@ -95,6 +99,9 @@ func (c *HTTPFsConfig) validate() error {
 	if err != nil {
 		return fmt.Errorf("httpfs: invalid endpoint: %w", err)
 	}
+	if !util.IsStringPrefixInSlice(c.Endpoint, supportedEndpointSchema) {
+		return errors.New("httpfs: invalid endpoint schema: http and https are supported")
+	}
 	if c.Password.IsEncrypted() && !c.Password.IsValid() {
 		return errors.New("httpfs: invalid encrypted password")
 	}
@@ -150,9 +157,7 @@ func NewHTTPFs(connectionID, localTempDir, mountPath string, config HTTPFsConfig
 			localTempDir = filepath.Clean(os.TempDir())
 		}
 	}
-	if err := config.validate(); err != nil {
-		return nil, err
-	}
+	config.setEmptyCredentialsIfNil()
 	if !config.Password.IsEmpty() {
 		if err := config.Password.TryDecrypt(); err != nil {
 			return nil, err
@@ -343,16 +348,6 @@ func (*HTTPFs) Readlink(name string) (string, error) {
 
 // Chown changes the numeric uid and gid of the named file.
 func (fs *HTTPFs) Chown(name string, uid int, gid int) error {
-	/*ctx, cancelFn := context.WithDeadline(context.Background(), time.Now().Add(fs.ctxTimeout))
-	defer cancelFn()
-
-	queryString := fmt.Sprintf("?uid=%d&gid=%d", uid, gid)
-	resp, err := fs.sendHTTPRequest(ctx, http.MethodPatch, "chown", name, queryString, "", nil)
-	if err != nil {
-		return err
-	}
-	defer resp.Body.Close()
-	return nil*/
 	return ErrVfsUnsupported
 }