1
0
mirror of https://github.com/owncloud/ocis.git synced 2026-10-23 23:29:02 +03:00

fix: [OCISDEV-388] renaming a resource to its current name is a no-op (#12784)

A MOVE whose source and destination resolve to the same resource is now
answered with 204 No Content server side instead of a confusing 403 or
404, and instead of 409 Conflict on the id based dav route, so every
client behaves the same way. The no-op still requires the move
permission.

The server-side guard lives in reva (owncloud/reva#708 and the follow-up
owncloud/reva#713, which moves the guard above the recursion checks so it
is also reached when the request addresses the resource by its id).
Pulled in here by bumping reva to v2.0.0-20260813121157-d386075a0421.
vendor/ was regenerated with `go mod vendor`; no hand edits.

On the test side the @issue-1976 tags are dropped from the five MOVE
feature files and the corresponding entries are removed from both
expected-failures lists. Scenario Outlines that mix roles with and
without the move permission parameterize the expected status in an extra
http-status-code Examples column rather than being split into duplicate
scenarios. The gherlint newline_before_scenario rule is configured as
["error", 2, true], counting tag lines toward the total, so the eight
scenarios that lost their tag get back the blank line it occupied.

Drop the "viewer" row from the same-name Examples table in
moveByFileId.feature: "viewer" is not a valid space role for a
permissionsRole invitation, so the step errored with
Role 'viewer' not found, and the "Space Viewer" row already covers
the 403.
This commit is contained in:
Gaurav Soni
2026-08-14 10:50:35 +02:00
committed by GitHub
parent 2063b84b8f
commit ef41e736d1
12 changed files with 130 additions and 119 deletions

View File

@@ -0,0 +1,15 @@
Bugfix: Renaming a resource to its current name is a silent no-op
Renaming a file or folder to the name it already has failed with a
confusing `403 Forbidden` or `404 Not Found`, and `409 Conflict` when the
request addressed the resource by its id. Every client had to work around
it on its own. A `MOVE` whose source and destination resolve to the same
resource is now detected server side and answered with `204 No Content`
without touching the resource, so the behaviour is consistent for all
clients and for both dav routes. The no-op still requires the move
permission: a user who may not rename the resource keeps getting
`403 Forbidden`.
https://github.com/owncloud/ocis/pull/12784
https://github.com/owncloud/reva/pull/708
https://github.com/owncloud/reva/pull/713

2
go.mod
View File

@@ -67,7 +67,7 @@ require (
github.com/open-policy-agent/opa v1.19.0
github.com/orcaman/concurrent-map v1.0.0
github.com/owncloud/libre-graph-api-go v1.0.5-0.20260216101009-eeac018af245
github.com/owncloud/reva/v2 v2.0.0-20260812122437-aa0c3881c0af
github.com/owncloud/reva/v2 v2.0.0-20260813121157-d386075a0421
github.com/pkg/errors v0.9.1
github.com/pkg/xattr v0.4.12
github.com/prometheus/client_golang v1.24.1

4
go.sum
View File

@@ -736,8 +736,8 @@ github.com/orcaman/concurrent-map v1.0.0 h1:I/2A2XPCb4IuQWcQhBhSwGfiuybl/J0ev9HD
github.com/orcaman/concurrent-map v1.0.0/go.mod h1:Lu3tH6HLW3feq74c2GC+jIMS/K2CFcDWnWD9XkenwhI=
github.com/owncloud/libre-graph-api-go v1.0.5-0.20260216101009-eeac018af245 h1:JRidLTAKhnvyLMRtVtSF4lhBa0NSAOs6fof+d6JnKII=
github.com/owncloud/libre-graph-api-go v1.0.5-0.20260216101009-eeac018af245/go.mod h1:z61VMGAJRtR1nbgXWiNoCkxUXP1B3Je9rMuJbnGd+Og=
github.com/owncloud/reva/v2 v2.0.0-20260812122437-aa0c3881c0af h1:we/bQco5/RxOHczr6SDZPpZq9pUk+dhppHS5N9j63Ag=
github.com/owncloud/reva/v2 v2.0.0-20260812122437-aa0c3881c0af/go.mod h1:XBnHPPDOGYq+Uf/PD1wGXne3xTvoghBxXPUULcSZjlc=
github.com/owncloud/reva/v2 v2.0.0-20260813121157-d386075a0421 h1:orjzG6JMpulTJvkeg//KwgeqdrOMGXAO0K6GaH4XCxI=
github.com/owncloud/reva/v2 v2.0.0-20260813121157-d386075a0421/go.mod h1:f4ymc4PUG6a3QSOHXq7Q9Ck5udiSNU6Lv2Im2Q0jvRc=
github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c h1:rp5dCmg/yLR3mgFuSOe4oEnDDmGLROTvMragMUXpTQw=
github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c/go.mod h1:X07ZCGwUbLaax7L0S3Tw4hpejzu63ZrrQiUe6W0hcy0=
github.com/pablodz/inotifywaitgo v0.0.12 h1:DVJJTFMDlSAdO46yisPF2vzozs/r+4k6ih8MVghq78M=

View File

@@ -155,18 +155,6 @@ _ocdav: api compatibility, return correct status code_
- [coreApiTrashbin/trashbinDelete.feature:92](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiTrashbin/trashbinDelete.feature#L92)
#### [MOVE a file into same folder with same name returns 404 instead of 403](https://github.com/owncloud/ocis/issues/1976)
- [coreApiWebdavMove2/moveFile.feature:100](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove2/moveFile.feature#L100)
- [coreApiWebdavMove2/moveFile.feature:101](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove2/moveFile.feature#L101)
- [coreApiWebdavMove2/moveFile.feature:102](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove2/moveFile.feature#L102)
- [coreApiWebdavMove1/moveFolder.feature:217](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove1/moveFolder.feature#L217)
- [coreApiWebdavMove1/moveFolder.feature:218](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove1/moveFolder.feature#L218)
- [coreApiWebdavMove1/moveFolder.feature:219](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove1/moveFolder.feature#L219)
- [coreApiWebdavMove2/moveShareOnOcis.feature:334](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove2/moveShareOnOcis.feature#L334)
- [coreApiWebdavMove2/moveShareOnOcis.feature:337](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove2/moveShareOnOcis.feature#L337)
- [coreApiWebdavMove2/moveShareOnOcis.feature:340](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiWebdavMove2/moveShareOnOcis.feature#L340)
#### [COPY file/folder to same name is possible (but 500 code error for folder with spaces path)](https://github.com/owncloud/ocis/issues/8711)
- [coreApiSharePublicLink2/copyFromPublicLink.feature:198](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/coreApiSharePublicLink2/copyFromPublicLink.feature#L198)

View File

@@ -203,19 +203,8 @@ The expected failures in this file are from features in the owncloud/ocis repo.
#### [sharee (editor role) MOVE a file by file-id into shared sub-folder returns 502](https://github.com/owncloud/ocis/issues/7617)
- [apiSpacesDavOperation/moveByFileId.feature:368](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L368)
- [apiSpacesDavOperation/moveByFileId.feature:591](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L591)
#### [MOVE a file into same folder with same name returns 404 instead of 403](https://github.com/owncloud/ocis/issues/1976)
- [apiSpacesShares/moveSpaces.feature:69](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesShares/moveSpaces.feature#L69)
- [apiSpacesShares/moveSpaces.feature:70](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesShares/moveSpaces.feature#L70)
- [apiSpacesShares/moveSpaces.feature:416](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesShares/moveSpaces.feature#L416)
- [apiSpacesDavOperation/moveByFileId.feature:61](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L61)
- [apiSpacesDavOperation/moveByFileId.feature:174](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L174)
- [apiSpacesDavOperation/moveByFileId.feature:175](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L175)
- [apiSpacesDavOperation/moveByFileId.feature:176](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L176)
- [apiSpacesDavOperation/moveByFileId.feature:393](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L393)
- [apiSpacesDavOperation/moveByFileId.feature:367](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L367)
- [apiSpacesDavOperation/moveByFileId.feature:590](https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiSpacesDavOperation/moveByFileId.feature#L590)
#### [OCM. admin cannot get federated users if he hasn't connection with them ](https://github.com/owncloud/ocis/issues/9829)

View File

@@ -57,12 +57,12 @@ Feature: moving/renaming file using file id
But for user "Alice" folder "folder/sub-folder" of the space "Personal" should not contain these files:
| textfile.txt |
@issue-1976
Scenario: try to move a file into same folder with same name
Scenario: move a file into same folder with same name
And user "Alice" has uploaded file with content "some data" to "textfile.txt"
And we save it into "FILEID"
When user "Alice" moves file with id "<<FILEID>>" as "textfile.txt" into folder "/" inside space "Personal"
Then the HTTP status code should be "403"
Then the HTTP status code should be "204"
And as "Alice" file "textfile.txt" should not exist in the trashbin of the space "Personal"
And for user "Alice" the content of the file "textfile.txt" of the space "Personal" should be "some data"
@@ -153,8 +153,8 @@ Feature: moving/renaming file using file id
| Manager |
| Space Editor |
@issue-1976
Scenario Outline: try to move a file within a project space into a folder with same name
Scenario Outline: move a file within a project space into a folder with same name
Given the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API
And user "Brian" has been created with default attributes
And user "Alice" has created a space "project-space" with the default quota using the Graph API
@@ -166,14 +166,13 @@ Feature: moving/renaming file using file id
| shareType | user |
| permissionsRole | <space-role> |
When user "Brian" moves file with id "<<FILEID>>" as "textfile.txt" into folder "/" inside space "project-space"
Then the HTTP status code should be "403"
Then the HTTP status code should be "<http-status-code>"
And as "Alice" file "textfile.txt" should not exist in the trashbin of the space "project-space"
And for user "Brian" the content of the file "textfile.txt" of the space "project-space" should be "some data"
Examples:
| space-role |
| Manager |
| Space Viewer |
| viewer |
| space-role | http-status-code |
| Manager | 204 |
| Space Viewer | 403 |
Scenario: try to move a file into a folder inside project space (viewer)
@@ -389,8 +388,8 @@ Feature: moving/renaming file using file id
And for user "Alice" folder "folder" of the space "Personal" should not contain these files:
| test.txt |
@issue-1976
Scenario: sharee tries to move a file into same shared folder with same name
Scenario: sharee moves a file into same shared folder with same name
Given user "Brian" has been created with default attributes
And user "Alice" has created folder "folder"
And user "Alice" has uploaded file with content "some data" to "folder/test.txt"
@@ -403,7 +402,7 @@ Feature: moving/renaming file using file id
| permissionsRole | Editor |
And user "Brian" has a share "/folder" synced
When user "Brian" moves file with id "<<FILEID>>" as "test.txt" into folder "folder" inside space "Shares"
Then the HTTP status code should be "403"
Then the HTTP status code should be "204"
And as "Alice" file "test.txt" should not exist in the trashbin of the space "Personal"
And for user "Brian" the content of the file "folder/test.txt" of the space "Shares" should be "some data"
And for user "Alice" the content of the file "folder/test.txt" of the space "Personal" should be "some data"

View File

@@ -50,8 +50,8 @@ Feature: move (rename) file
But for user "Alice" the space "Project" should contain these entries:
| insideSpace.txt |
@issue-1976
Scenario Outline: try to move a file within a project space into a folder with same name
Scenario Outline: move a file within a project space into a folder with same name
Given the administrator has assigned the role "Space Admin" to user "Brian" using the Graph API
And user "Brian" has created a space "Project" with the default quota using the Graph API
And user "Brian" has uploaded a file inside space "Project" with content "some content" to "insideSpace.txt"
@@ -61,14 +61,14 @@ Feature: move (rename) file
| shareType | user |
| permissionsRole | <space-role> |
When user "Alice" moves file "insideSpace.txt" from space "Project" to "insideSpace.txt" inside space "Project" using the WebDAV API
Then the HTTP status code should be "403"
Then the HTTP status code should be "<http-status-code>"
And as "Brian" file "insideSpace.txt" should not exist in the trashbin of the space "Project"
And for user "Alice" the content of the file "insideSpace.txt" of the space "Project" should be "some content"
Examples:
| space-role |
| Manager |
| Space Editor |
| Space Viewer |
| space-role | http-status-code |
| Manager | 204 |
| Space Editor | 204 |
| Space Viewer | 403 |
@issue-8116
Scenario Outline: user moves a file from a space project with different a role to a space project with different role
@@ -395,8 +395,8 @@ Feature: move (rename) file
But for user "Alice" folder "testshare" of the space "Shares" should contain these entries:
| testfile.txt |
@issue-1976
Scenario Outline: sharee tries to move a file into same shared folder with same name
Scenario Outline: sharee moves a file into same shared folder with same name
Given user "Brian" has created folder "testshare"
And user "Brian" has uploaded file with content "test file content" to "testshare/testfile.txt"
And user "Brian" has sent the following resource share invitation:
@@ -407,15 +407,15 @@ Feature: move (rename) file
| permissionsRole | <permissions-role> |
And user "Alice" has a share "testshare" synced
When user "Alice" moves file "testshare/testfile.txt" from space "Shares" to "testshare/testfile.txt" inside space "Shares" using the WebDAV API
Then the HTTP status code should be "403"
Then the HTTP status code should be "<http-status-code>"
And as "Brian" file "testfile.txt" should not exist in the trashbin of the space "Personal"
And for user "Alice" the content of the file "testshare/testfile.txt" of the space "Shares" should be "test file content"
And for user "Brian" the content of the file "testshare/testfile.txt" of the space "Personal" should be "test file content"
Examples:
| permissions-role |
| Editor |
| Uploader |
| Viewer |
| permissions-role | http-status-code |
| Editor | 204 |
| Uploader | 403 |
| Viewer | 403 |
Scenario: overwrite a file while moving in project space

View File

@@ -205,12 +205,13 @@ Feature: move (rename) folder
| new |
| spaces |
@issue-1976
Scenario Outline: try to rename folder to same name
Scenario Outline: rename folder to same name
Given using <dav-path-version> DAV path
And user "Alice" has created folder "testFolder"
When user "Alice" moves folder "testFolder" to "testFolder" using the WebDAV API
Then the HTTP status code should be "404"
Then the HTTP status code should be "204"
And as "Alice" folder "testFolder" should exist
And as "Alice" the folder with original path "testFolder" should not exist in the trashbin
Examples:
| dav-path-version |

View File

@@ -87,12 +87,12 @@ Feature: move (rename) file
| new |
| spaces |
@issue-1976
Scenario Outline: try to move a file into same folder with same name
Scenario Outline: move a file into same folder with same name
Given using <dav-path-version> DAV path
And user "Alice" has uploaded file with content "ownCloud test text file" to "testfile.txt"
When user "Alice" moves file "testfile.txt" to "testfile.txt" using the WebDAV API
Then the HTTP status code should be "403"
Then the HTTP status code should be "204"
And as "Alice" the file with original path "testfile.txt" should not exist in the trashbin
And the content of file "testfile.txt" for user "Alice" should be "ownCloud test text file"
Examples:

View File

@@ -309,8 +309,8 @@ Feature: move (rename) file
| new |
| spaces |
@issue-1976
Scenario Outline: sharee tries to move a file into same shared folder with same name
Scenario Outline: sharee moves a file into same shared folder with same name
Given using <dav-path-version> DAV path
And user "Brian" has been created with default attributes
And user "Brian" has created folder "testshare"
@@ -323,18 +323,18 @@ Feature: move (rename) file
| permissionsRole | <permissions-role> |
And user "Alice" has a share "testshare" synced
When user "Alice" moves folder "Shares/testshare/testfile.txt" to "Shares/testshare/testfile.txt" using the WebDAV API
Then the HTTP status code should be "403"
Then the HTTP status code should be "<http-status-code>"
And as "Brian" the file with original path "testshare/testfile.txt" should not exist in the trashbin
And the content of file "Shares/testshare/testfile.txt" for user "Alice" should be "test data"
And the content of file "testshare/testfile.txt" for user "Brian" should be "test data"
Examples:
| dav-path-version | permissions-role |
| old | Viewer |
| old | Uploader |
| old | Editor |
| new | Viewer |
| new | Uploader |
| new | Editor |
| spaces | Viewer |
| spaces | Uploader |
| spaces | Editor |
| dav-path-version | permissions-role | http-status-code |
| old | Viewer | 403 |
| old | Uploader | 403 |
| old | Editor | 204 |
| new | Viewer | 403 |
| new | Uploader | 403 |
| new | Editor | 204 |
| spaces | Viewer | 403 |
| spaces | Uploader | 403 |
| spaces | Editor | 204 |

View File

@@ -148,6 +148,69 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req
errors.HandleWebdavError(&log, w, b, err)
return
}
client, err := s.gatewaySelector.Next()
if err != nil {
log.Error().Err(err).Msg("error selecting next client")
w.WriteHeader(http.StatusInternalServerError)
return
}
// check src exists
srcStatReq := &provider.StatRequest{Ref: src}
srcStatRes, err := client.Stat(ctx, srcStatReq)
if err != nil {
log.Error().Err(err).Msg("error sending grpc stat request")
w.WriteHeader(http.StatusInternalServerError)
return
}
if srcStatRes.Status.Code != rpc.Code_CODE_OK {
if srcStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND {
w.WriteHeader(http.StatusNotFound)
m := fmt.Sprintf("Resource %v not found", srcStatReq.Ref.Path)
b, err := errors.Marshal(http.StatusNotFound, m, "", "")
errors.HandleWebdavError(&log, w, b, err)
}
errors.HandleErrorStatus(&log, w, srcStatRes.Status)
return
}
if utils.IsSpaceRoot(srcStatRes.GetInfo()) {
log.Error().Msg("the source is disallowed")
w.WriteHeader(http.StatusBadRequest)
return
}
// check dst exists
dstStatReq := &provider.StatRequest{Ref: dst}
dstStatRes, err := client.Stat(ctx, dstStatReq)
if err != nil {
log.Error().Err(err).Msg("error sending grpc stat request")
w.WriteHeader(http.StatusInternalServerError)
return
}
if dstStatRes.Status.Code != rpc.Code_CODE_OK && dstStatRes.Status.Code != rpc.Code_CODE_NOT_FOUND {
errors.HandleErrorStatus(&log, w, dstStatRes.Status)
return
}
if dstStatRes.Status.Code == rpc.Code_CODE_OK && srcStatRes.GetInfo().GetId() != nil &&
utils.ResourceIDEqual(srcStatRes.GetInfo().GetId(), dstStatRes.GetInfo().GetId()) {
// a user without move permission must not be told the rename succeeded
if !srcStatRes.GetInfo().GetPermissionSet().GetMove() {
w.WriteHeader(http.StatusForbidden)
b, err := errors.Marshal(http.StatusForbidden, "permission denied", "", "")
errors.HandleWebdavError(&log, w, b, err)
return
}
log.Debug().Msg("move: source and destination are the same resource, nothing to do")
info := dstStatRes.GetInfo()
w.Header().Set(net.HeaderContentType, info.GetMimeType())
w.Header().Set(net.HeaderETag, info.GetEtag())
w.Header().Set(net.HeaderOCFileID, storagespace.FormatResourceID(info.GetId()))
w.Header().Set(net.HeaderOCETag, info.GetEtag())
w.WriteHeader(http.StatusNoContent)
return
}
isChild, err := s.referenceIsChildOf(ctx, s.gatewaySelector, dst, src)
if err != nil {
switch err.(type) {
@@ -201,50 +264,6 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req
return
}
client, err := s.gatewaySelector.Next()
if err != nil {
log.Error().Err(err).Msg("error selecting next client")
w.WriteHeader(http.StatusInternalServerError)
return
}
// check src exists
srcStatReq := &provider.StatRequest{Ref: src}
srcStatRes, err := client.Stat(ctx, srcStatReq)
if err != nil {
log.Error().Err(err).Msg("error sending grpc stat request")
w.WriteHeader(http.StatusInternalServerError)
return
}
if srcStatRes.Status.Code != rpc.Code_CODE_OK {
if srcStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND {
w.WriteHeader(http.StatusNotFound)
m := fmt.Sprintf("Resource %v not found", srcStatReq.Ref.Path)
b, err := errors.Marshal(http.StatusNotFound, m, "", "")
errors.HandleWebdavError(&log, w, b, err)
}
errors.HandleErrorStatus(&log, w, srcStatRes.Status)
return
}
if utils.IsSpaceRoot(srcStatRes.GetInfo()) {
log.Error().Msg("the source is disallowed")
w.WriteHeader(http.StatusBadRequest)
return
}
// check dst exists
dstStatReq := &provider.StatRequest{Ref: dst}
dstStatRes, err := client.Stat(ctx, dstStatReq)
if err != nil {
log.Error().Err(err).Msg("error sending grpc stat request")
w.WriteHeader(http.StatusInternalServerError)
return
}
if dstStatRes.Status.Code != rpc.Code_CODE_OK && dstStatRes.Status.Code != rpc.Code_CODE_NOT_FOUND {
errors.HandleErrorStatus(&log, w, dstStatRes.Status)
return
}
successCode := http.StatusCreated // 201 if new resource was created, see https://tools.ietf.org/html/rfc4918#section-9.9.4
if dstStatRes.Status.Code == rpc.Code_CODE_OK {
successCode = http.StatusNoContent // 204 if target already existed, see https://tools.ietf.org/html/rfc4918#section-9.9.4

2
vendor/modules.txt vendored
View File

@@ -1323,7 +1323,7 @@ github.com/orcaman/concurrent-map
# github.com/owncloud/libre-graph-api-go v1.0.5-0.20260216101009-eeac018af245
## explicit; go 1.18
github.com/owncloud/libre-graph-api-go
# github.com/owncloud/reva/v2 v2.0.0-20260812122437-aa0c3881c0af
# github.com/owncloud/reva/v2 v2.0.0-20260813121157-d386075a0421
## explicit; go 1.26
github.com/owncloud/reva/v2/cmd/revad/internal/grace
github.com/owncloud/reva/v2/cmd/revad/runtime