From 466f2e88b3b02efa73ee1ea6a347640d28d0f657 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 15 Apr 2023 14:16:26 +0200 Subject: [PATCH] WebClient: fix rename Signed-off-by: Nicola Murino --- go.mod | 2 +- go.sum | 3 +- internal/common/connection.go | 7 ++-- internal/common/protocol_test.go | 5 +++ internal/httpd/api_http_user.go | 22 ++++++++--- internal/httpd/httpd_test.go | 67 ++++++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index 5e0ae5f7..a4507d18 100644 --- a/go.mod +++ b/go.mod @@ -104,7 +104,7 @@ require ( github.com/coreos/go-systemd/v22 v22.5.0 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 // indirect + github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect github.com/fatih/color v1.15.0 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-jose/go-jose/v3 v3.0.0 // indirect diff --git a/go.sum b/go.sum index e26e2466..3201f517 100644 --- a/go.sum +++ b/go.sum @@ -839,8 +839,9 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/crypto/blake256 v1.0.0/go.mod h1:sQl2p6Y26YV+ZOcSTP6thNdn47hh8kt6rqSlvmrXFAc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 h1:HbphB4TFFXpv7MNrT52FGrrgVXF1owhMVTHFZIlnvd4= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0/go.mod h1:DZGJHZMqrU4JJqFAWUS2UO1+lbSKsdiOoYi9Zzey7Fc= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 h1:8UrgZ3GkP4i/CLijOJx79Yu+etlyjdBU4sfcs2WYQMs= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0= github.com/dennwc/varint v1.0.0/go.mod h1:hnItb35rvZvJrbTALZtY/iQfDs48JKRG1RPpgziApxA= github.com/denverdino/aliyungo v0.0.0-20190125010748-a747050bb1ba/go.mod h1:dV8lFg6daOBZbT6/BDGIz6Y3WFGn8juu6G+CQ6LHtl0= github.com/devigned/tab v0.1.1/go.mod h1:XG9mPq0dFghrYvoBF3xdRrJzSTX1b7IQrvaL9mzjeJY= diff --git a/internal/common/connection.go b/internal/common/connection.go index 07406976..ad005879 100644 --- a/internal/common/connection.go +++ b/internal/common/connection.go @@ -587,7 +587,7 @@ func (c *BaseConnection) copyFile(virtualSourcePath, virtualTargetPath string, s if ok, _ := c.User.IsFileAllowed(virtualTargetPath); !ok { return fmt.Errorf("file %q is not allowed: %w", virtualTargetPath, c.GetPermissionDeniedError()) } - if c.isSameResource(virtualSourcePath, virtualTargetPath) { + if c.IsSameResource(virtualSourcePath, virtualTargetPath) { fs, fsTargetPath, err := c.GetFsAndResolvedPath(virtualTargetPath) if err != nil { return err @@ -1156,7 +1156,7 @@ func (c *BaseConnection) checkFolderRename(fsSrc, fsDst vfs.Fs, fsSourcePath, fs func (c *BaseConnection) isRenamePermitted(fsSrc, fsDst vfs.Fs, fsSourcePath, fsTargetPath, virtualSourcePath, virtualTargetPath string, fi os.FileInfo, ) bool { - if !c.isSameResource(virtualSourcePath, virtualTargetPath) { + if !c.IsSameResource(virtualSourcePath, virtualTargetPath) { c.Log(logger.LevelInfo, "rename %#q->%q is not allowed: the paths must be on the same resource", virtualSourcePath, virtualTargetPath) return false @@ -1407,7 +1407,8 @@ func (c *BaseConnection) HasSpace(checkFiles, getUsage bool, requestPath string) return result, transferQuota } -func (c *BaseConnection) isSameResource(virtualSourcePath, virtualTargetPath string) bool { +// IsSameResource returns true if source and target paths are on the same resource +func (c *BaseConnection) IsSameResource(virtualSourcePath, virtualTargetPath string) bool { sourceFolder, errSrc := c.User.GetVirtualFolderForPath(virtualSourcePath) dstFolder, errDst := c.User.GetVirtualFolderForPath(virtualTargetPath) if errSrc != nil && errDst != nil { diff --git a/internal/common/protocol_test.go b/internal/common/protocol_test.go index 58ccb127..de52e8da 100644 --- a/internal/common/protocol_test.go +++ b/internal/common/protocol_test.go @@ -3854,8 +3854,13 @@ func TestEventRule(t *testing.T) { err = os.Remove(uploadLogFilePath) assert.NoError(t, err) + lastReceivedEmail.reset() + assert.Eventually(t, func() bool { + return lastReceivedEmail.get().From != "" + }, 3000*time.Millisecond, 100*time.Millisecond) } + lastReceivedEmail.reset() _, err = httpdtest.RemoveEventRule(rule1, http.StatusOK) assert.NoError(t, err) _, err = httpdtest.RemoveEventRule(rule2, http.StatusOK) diff --git a/internal/httpd/api_http_user.go b/internal/httpd/api_http_user.go index 45d9e73b..53db4352 100644 --- a/internal/httpd/api_http_user.go +++ b/internal/httpd/api_http_user.go @@ -134,11 +134,23 @@ func renameUserFsEntry(w http.ResponseWriter, r *http.Request) { oldName := connection.User.GetCleanedPath(r.URL.Query().Get("path")) newName := connection.User.GetCleanedPath(r.URL.Query().Get("target")) - err = connection.Rename(oldName, newName) - if err != nil { - sendAPIResponse(w, r, err, fmt.Sprintf("Unable to rename %q -> %q", oldName, newName), - getMappedStatusCode(err)) - return + if !connection.IsSameResource(oldName, newName) { + if err := connection.Copy(oldName, newName); err != nil { + sendAPIResponse(w, r, err, fmt.Sprintf("Cannot perform copy step to rename %q -> %q", oldName, newName), + getMappedStatusCode(err)) + return + } + if err := connection.RemoveAll(oldName); err != nil { + sendAPIResponse(w, r, err, fmt.Sprintf("Cannot perform remove step to rename %q -> %q", oldName, newName), + getMappedStatusCode(err)) + return + } + } else { + if err := connection.Rename(oldName, newName); err != nil { + sendAPIResponse(w, r, err, fmt.Sprintf("Unable to rename %q -> %q", oldName, newName), + getMappedStatusCode(err)) + return + } } sendAPIResponse(w, r, nil, fmt.Sprintf("%q renamed to %q", oldName, newName), http.StatusOK) } diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 566c9082..69cad376 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -15235,6 +15235,73 @@ func TestWebGetFiles(t *testing.T) { assert.NoError(t, err) } +func TestRenameDifferentResource(t *testing.T) { + folderName := "foldercryptfs" + u := getTestUser() + u.VirtualFolders = append(u.VirtualFolders, vfs.VirtualFolder{ + BaseVirtualFolder: vfs.BaseVirtualFolder{ + Name: folderName, + MappedPath: filepath.Join(os.TempDir(), "folderName"), + FsConfig: vfs.Filesystem{ + Provider: sdk.CryptedFilesystemProvider, + CryptConfig: vfs.CryptFsConfig{ + Passphrase: kms.NewPlainSecret("super secret"), + }, + }, + }, + VirtualPath: "/folderPath", + }) + user, _, err := httpdtest.AddUser(u, http.StatusCreated) + assert.NoError(t, err) + + testFileName := "file.txt" + + webAPIToken, err := getJWTAPIUserTokenFromTestServer(defaultUsername, defaultPassword) + assert.NoError(t, err) + + req, err := http.NewRequest(http.MethodPost, userFileActionsPath+"/move?path="+testFileName+"&target="+url.QueryEscape(path.Join("/", "folderPath", testFileName)), nil) + assert.NoError(t, err) + setBearerForReq(req, webAPIToken) + rr := executeRequest(req) + checkResponseCode(t, http.StatusNotFound, rr) + assert.Contains(t, rr.Body.String(), "Cannot perform copy step") + + err = os.WriteFile(filepath.Join(user.GetHomeDir(), testFileName), []byte("just a test"), os.ModePerm) + assert.NoError(t, err) + + req, err = http.NewRequest(http.MethodPost, userFileActionsPath+"/move?path="+testFileName+"&target="+url.QueryEscape(path.Join("/", "folderPath", testFileName)), nil) + assert.NoError(t, err) + setBearerForReq(req, webAPIToken) + rr = executeRequest(req) + checkResponseCode(t, http.StatusOK, rr) + + // recreate the file and remove the delete permission + err = os.WriteFile(filepath.Join(user.GetHomeDir(), testFileName), []byte("just another test"), os.ModePerm) + assert.NoError(t, err) + + u.Permissions = map[string][]string{ + "/": {dataprovider.PermUpload, dataprovider.PermListItems, dataprovider.PermCreateDirs, + dataprovider.PermDownload, dataprovider.PermOverwrite}, + } + _, resp, err := httpdtest.UpdateUser(u, http.StatusOK, "") + assert.NoError(t, err, string(resp)) + webAPIToken, err = getJWTAPIUserTokenFromTestServer(defaultUsername, defaultPassword) + assert.NoError(t, err) + req, err = http.NewRequest(http.MethodPost, userFileActionsPath+"/move?path="+testFileName+"&target="+url.QueryEscape(path.Join("/", "folderPath", testFileName)), nil) + assert.NoError(t, err) + setBearerForReq(req, webAPIToken) + rr = executeRequest(req) + checkResponseCode(t, http.StatusForbidden, rr) + assert.Contains(t, rr.Body.String(), "Cannot perform remove step") + + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) + err = os.RemoveAll(user.GetHomeDir()) + assert.NoError(t, err) + _, err = httpdtest.RemoveFolder(vfs.BaseVirtualFolder{Name: folderName}, http.StatusOK) + assert.NoError(t, err) +} + func TestWebDirsAPI(t *testing.T) { user, _, err := httpdtest.AddUser(getTestUser(), http.StatusCreated) assert.NoError(t, err)