Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MOVE between Shares responds with 502 #8063

Closed
michaelstingl opened this issue Dec 22, 2023 · 14 comments · Fixed by #8102
Closed

MOVE between Shares responds with 502 #8063

michaelstingl opened this issue Dec 22, 2023 · 14 comments · Fixed by #8102
Assignees
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Milestone

Comments

@michaelstingl
Copy link
Contributor

michaelstingl commented Dec 22, 2023

Describe the bug

oCIS no longer allows MOVE between Shares (cs3org/reva#4318), but the the HTTP response is 502

Steps to reproduce

curl 'https://ocis.ocis-traefik.latest.owncloud.works/remote.php/dav/spaces/a0ca6a90-a365-4782-871e-d44447bbc668!eb249b94-9051-4743-9e36-577b08b851ea%3Aa8024631-d2a3-4b7d-81bf-78a2b1c80f96%3A92fa8dc2-0845-49e4-a135-284750891d97/Newfile.txt' \
-X 'MOVE' \
-H 'Authorization: Bearer eyJhbGciOiJQUzI1NiIsImtpZCI6InByaXZhdGUta2V5IiwidHlwIjoiSldUIn0.eyJhdWQiOiJ3ZWIiLCJleHAiOjE3MDMyNjAzNDUsImlhdCI6MTcwMzI2MDA0NSwiaXNzIjoiaHR0cHM6Ly9vY2lzLm9jaXMtdHJhZWZpay5sYXRlc3Qub3duY2xvdWQud29ya3MiLCJqdGkiOiJrTXEzT2R1TnlYWjc3OEdZY0UzMlBsTmxITTY1Q0VvdSIsImxnLmkiOnsiZG4iOiJBbGJlcnQgRWluc3RlaW4iLCJpZCI6Im93bkNsb3VkVVVJRD00YzUxMGFkYS1jODZiLTQ4MTUtODgyMC00MmNkZjgyYzNkNTEiLCJ1biI6ImVpbnN0ZWluIn0sImxnLnAiOiJpZGVudGlmaWVyLWxkYXAiLCJsZy50IjoiMSIsInNjcCI6Im9wZW5pZCBwcm9maWxlIGVtYWlsIiwic3ViIjoiVnM5c2FaLWszUkFrTmJwZ0BIOEN3a2xBSlFjS1lWT3NWTk5Ld0k2MUpLeW1xOS00bnlPX3ZibS1mV1kwUzB0V09NVEdpMVdYOGstdnN0S2xlTVVmOG13In0.bPBIK4k8s9xfpJi7IDnTjmQKzm8iAJhtdyVescQQ3fnuD-OWbGve_fehiI6VJhxrgBjAvx61zCQ4xsmFxfYKyopekzxxOylNwIkiFZl8_iacIgvFbrzbGSru4N4_6365GR0e4c3pOo0V40MuUNsqsv-Xn83t1qS64QhTW3DEeBFCRFfEjkvh8KjHlTeGHeDAtRojA537Vc4yhjAnuOYoZf9V6UYMll6y2I3PG80LsV_eSI79HnLOB0wpWwlntsQGX6lnxHiufAyEDiNl4QHZXDsG1DEwJAWGspiU4mcmyP1OLIIlGCv5fXd8YmuIKRVWSWJKr-erm2WOykpUy9HPQUitmJQMiRoRw0dtEuXUj5c-Y9l1Nxy0nXwvzNlkTzhrySIZe0ItuVsi67dbcBX5pkU0x8yIc_S1_PKNKDBSri2Y_r0ZwdGOWwCXXaePibJPaxAhwrSDmvaevSCa18LZcwIPJP-CJ4IuOc84jTFUhqF9gHi90IKAWNKLiLBknPstdoCzG48a8YpZaRJ2O6HbEX3SJ_XnWXXFdH7pOMiwavwVpI_ZS6xeKG2VVQR-7lcdZvZzOwQP5O8onOhJhb_tdL5aHiuphR5qcXuJpUfr_F-fSE-z3KylFSTYpprwG7rHxZsm_PmiDaKpjL9cIG7aYWDYLuzFms35v8YGUhsnxu0' \
-H 'Destination: https://ocis.ocis-traefik.latest.owncloud.works/remote.php/dav/spaces/a0ca6a90-a365-4782-871e-d44447bbc668!eb249b94-9051-4743-9e36-577b08b851ea%3Aa8024631-d2a3-4b7d-81bf-78a2b1c80f96%3Adffbb75a-56cb-4705-a165-0f75b85c0acd/Newfile.txt' \
-i

HTTP/2 502 
access-control-allow-origin: *
content-security-policy: default-src 'none';
content-type: text/xml; charset=utf-8
date: Fri, 22 Dec 2023 15:49:22 GMT
vary: Origin
x-content-type-options: nosniff
x-download-options: noopen
x-frame-options: SAMEORIGIN
x-permitted-cross-domain-policies: none
x-request-id: e457e174ae18/ckIpIxC007-001350
x-robots-tag: none
x-xss-protection: 1; mode=block
content-length: 205

<?xml version="1.0" encoding="UTF-8"?>
<d:error xmlns:d="DAV" xmlns:s="http://sabredav.org/ns">
	<s:exception/>
	<s:message>sharesstorageprovider: can not move between shares</s:message>
</d:error>

Expected behavior

I'd expect a 403, similar to oC10.

 curl 'https://demo.owncloud.com/remote.php/dav/files/test/share/New%20text%20file2.txt' \
-X 'MOVE' \
-H 'Destination: https://demo.owncloud.com/remote.php/dav/files/test/share_readonly/New%20text%20file2.txt' \
-u "test:test" \
-i

HTTP/2 403 
cache-control: no-store, no-cache, must-revalidate
content-security-policy: default-src 'none';
content-type: application/xml; charset=utf-8
date: Fri, 22 Dec 2023 15:21:47 GMT
expires: Thu, 19 Nov 1981 08:52:00 GMT
pragma: no-cache
referrer-policy: strict-origin-when-cross-origin
server: Apache
set-cookie: ocnn761yjb3e=scpfk8sa5sp47agjvcmno2gkin; path=/; secure; HttpOnly; SameSite=Lax
set-cookie: oc_sessionPassphrase=UmTYkVWcKNMvVRBLNVv5XCRxGB55QpiSckoIphwV4bHWRIZwuW7ajFYO6nPJ2CORyF3gRtAEiLcXCA0EBL%2BeIonxXPFsvdS8J%2BPTO5rVglCnQAFMZ9zJvkjGY3uYlN%2Fk; expires=Fri, 22-Dec-2023 15:41:47 GMT; Max-Age=1200; path=/; secure; HttpOnly; SameSite=Lax
set-cookie: ocnn761yjb3e=j48edh390nd6p18fctj01fhtfm; path=/; secure; HttpOnly; SameSite=Lax
set-cookie: cookie_test=test; expires=Fri, 22-Dec-2023 16:21:47 GMT; Max-Age=3600
strict-transport-security: max-age=315360000; preload
x-content-type-options: nosniff
x-download-options: noopen
x-frame-options: SAMEORIGIN
x-permitted-cross-domain-policies: none
x-robots-tag: none
x-xss-protection: 0
content-length: 230

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\Forbidden</s:exception>
  <s:message>Destination directory is not writable</s:message>
</d:error>

Setup

[Log]  ownCloud Web UI 8.0.0-rc.1  (index.html-MZztZoFM.mjs, line 1)
[Log]  Infinite Scale 5.0.0-beta.2+5362f82ef Community  (index.html-MZztZoFM.mjs, line 1)
@saw-jan
Copy link
Member

saw-jan commented Dec 22, 2023

There are similar bug reports: #4087, #7617 and #7618
And there was this (#4087 (comment)) discussion about 502 status code. I hope these are related to this bug report

@micbar
Copy link
Contributor

micbar commented Dec 27, 2023

Fix in cs3org/reva#4439

@micbar micbar self-assigned this Dec 27, 2023
@micbar micbar added Priority:p2-high Escalation, on top of current planning, release blocker and removed p2-high labels Dec 27, 2023
@saw-jan
Copy link
Member

saw-jan commented Dec 29, 2023

I tried to do MOVE with some combination like in #4087 (comment) and here are some questionable statuscode.
(Tested including fix cs3org/reva#4439)

Using file path

from from (role) to to (role) result
project manager/editor project manager/editor 502 ❓
project manager/editor project viewer 403 ✔️
project viewer project manager/editor/viewer 403 ✔️
project manager/editor/viewer personal   409 ❓
project manager/editor/viewer share jail editor/viewer 502 ❓
personal   project manager/editor 502 ❓
personal   project viewer 403 ✔️
personal   share jail editor/viewer 502 ❓
share jail editor/viewer personal   409 ❓
share jail editor/viewer project manager/editor/viewer 502 ❓
share jail editor/viewer share jail editor/viewer 403 ✔️
share jail editor share jail(same) 201 ✔️
share jail viewer share jail(same) 403 ✔️

Using file-id (some differences)

from from (role) to to (role) result
share jail editor project manager/editor 502 ❓
share jail editor project viewer 403 ✔️
share jail viewer project manager/editor/viewer 403 ✔️
share jail editor/viewer share jail editor/viewer 502 ❓
share jail editor/viewer share jail(same) 502 ❓

I think 502 and 409 are not expected ones.

CC @micbar @ScharfViktor

@micbar
Copy link
Contributor

micbar commented Jan 2, 2024

@saw-jan I created a follow up PR cs3org/reva#4443

This fixes the 502 status codes.

Please check the 409 again. This is a different error IMHO. 409 is returned when an intermediate path segment is not existing.

@saw-jan
Copy link
Member

saw-jan commented Jan 2, 2024

I created a follow up PR cs3org/reva#4443

Cool! ✨ I will retest the matrix

@saw-jan
Copy link
Member

saw-jan commented Jan 2, 2024

Updated martrix from the API test scenarios (Need to test manually)

Using file path

from from (role) to to (role) result
project manager/editor project manager/editor 502 ❓
project manager/editor project viewer 403 ✔️
project viewer project manager/editor/viewer 403 ✔️
project manager/editor personal   502 ❓
project viewer personal   403 ✔️
project manager/editor/viewer share jail editor/viewer 403 ✔️
personal   project manager/editor 502 ❓
personal   project viewer 403 ✔️
personal   share jail editor/viewer 403 ✔️
share jail editor/viewer personal   403 ✔️
share jail editor/viewer project manager/editor/viewer 403 ✔️
share jail editor/viewer share jail editor/viewer 403 ✔️
share jail editor share jail(same) 201 ✔️
share jail viewer share jail(same) 403 ✔️

Using file-id (some differences)

from from (role) to to (role) result
share jail editor project manager/editor 502 ❓
share jail editor project viewer 403 ✔️
share jail viewer project manager/editor/viewer 403 ✔️
share jail editor/viewer share jail editor/viewer 403 👍 (tested vscharf)
share jail editor/viewer share jail(same) 403 👍 (tested vscharf)
share jail editor personal   502 ❓ Updated vscharf - can confirm that 502

@micbar
Copy link
Contributor

micbar commented Jan 2, 2024

@saw-jan I don' get it.

According to your findings, things got worse?

@saw-jan
Copy link
Member

saw-jan commented Jan 2, 2024

@saw-jan I don' get it.

According to your findings, things got worse?

No no, I meant to show that some got fixed but there're still other cases where there is 502

That 409 was probably my mistake. We have 502 expectation in the API test scenarios.

@ScharfViktor
Copy link
Contributor

502 when moving from shares jail(edit permission) to personal:

curl --location --request MOVE 'https://localhost:9200/remote.php/dav/spaces/ed9b7703-0abc-4562-b9bd-b06574db4bed$534bb038-6f9d-4093-946f-133be61fa4e7!c7460ad8-b43f-4a17-b275-e08ba4efdfe9' \
--header 'Overwrite: F' \
--header 'Destination: https://localhost:9200/remote.php/dav/spaces/ed9b7703-0abc-4562-b9bd-b06574db4bed%244c510ada-c86b-4815-8820-42cdf82c3d51/subCat' \
--header 'Authorization: Basic ZWluc3RlaW46cmVsYXRpdml0eQ=='

@saw-jan
Copy link
Member

saw-jan commented Jan 4, 2024

we can leave this issue closed here. I have created #8121 to make a track of MOVE related test cases, related bugs and their coverage

@micbar
Copy link
Contributor

micbar commented Jan 9, 2024

Re opening the issue.

@michaelstingl @TheOneRing @dragotin

On behalf of https://datatracker.ietf.org/doc/html/rfc4918#section-9.9.4 I am re-opening this issue.

Proposal

We should treat a "Cross-Storage" MOVE in ocis like a cross-server move in the RFC like ...when the destination is on another sub-section of the same server namespace. This has the implication that we need to roll back the changes and respond with a 502 Status code like described in the RFC:

In addition to the general status codes possible, the following
   status codes have specific applicability to MOVE:

   201 (Created) - The source resource was successfully moved, and a new
   URL mapping was created at the destination.

   204 (No Content) - The source resource was successfully moved to a
   URL that was already mapped.

   207 (Multi-Status) - Multiple resources were to be affected by the
   MOVE, but errors on some of them prevented the operation from taking
   place.  Specific error messages, together with the most appropriate
   of the source and destination URLs, appear in the body of the multi-
   status response.  For example, if a source resource was locked and
   could not be moved, then the source resource URL appears with the 423
   (Locked) status.

   403 (Forbidden) - Among many possible reasons for forbidding a MOVE
   operation, this status code is recommended for use when the source
   and destination resources are the same.

   409 (Conflict) - A resource cannot be created at the destination
   until one or more intermediate collections have been created.  The
   server MUST NOT create those intermediate collections automatically.
   Or, the server was unable to preserve the behavior of the live
   properties and still move the resource to the destination (see
   'preserved-live-properties' postcondition).

   412 (Precondition Failed) - A condition header failed.  Specific to
   MOVE, this could mean that the Overwrite header is "F" and the
   destination URL is already mapped to a resource.

   423 (Locked) - The source or the destination resource, the source or
   destination resource parent, or some resource within the source or
   destination collection, was locked.  This response SHOULD contain the
   'lock-token-submitted' precondition element.

   502 (Bad Gateway) - This may occur when the destination is on another
   server and the destination server refuses to accept the resource.
   This could also occur when the destination is on another sub-section
   of the same server namespace.

@micbar micbar reopened this Jan 9, 2024
@michaelstingl
Copy link
Contributor Author

We should treat a "Cross-Storage" MOVE in ocis like a cross-server move in the RFC like ...when the destination is on another sub-section of the same server namespace.

This definition of storage in our WebDAV implementation should be added here:
https://owncloud.dev/apis/http/webdav/

@micbar
Copy link
Contributor

micbar commented Jan 26, 2024

cs3org/reva#4480

@micbar
Copy link
Contributor

micbar commented Jan 26, 2024

Fixed by #8287 due to reva update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:p2-high Escalation, on top of current planning, release blocker Type:Bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants