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

after renaming resource as sharer the receiver cannot access it by WebDAV using the new name #8242

Closed
individual-it opened this issue Jan 18, 2024 · 16 comments · Fixed by #8246
Assignees
Milestone

Comments

@individual-it
Copy link
Member

Describe the bug

After the sharer changed the name of a shared resource, the receiver can see the new name, but cannot access it by using the new name.
d:href in the WebDAV response stays the old one, and that is how the resource can be accessed

Steps to reproduce

  1. create a folder
  2. share the folder
  3. as sharer rename the folder
  4. as receiver do a propfind on the share-jail curl -XPROPFIND -uuu1:uu1 https://host.d'ocker.internal:9200/dav/spaces/a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668' -k | xmllint --format -

Expected behavior

d:href element and oc:name should correlate

Actual behavior

d:href shows the old name oc:name the current one

response (shortened)

 <d:response>
    <d:href>/dav/spaces/a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668/folder1/</d:href>
    <d:propstat>
      <d:prop>
        <oc:name>folder-renamed</oc:name>
        <d:displayname>folder-renamed</d:displayname>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>

Setup

Please describe how you started the server and provide a list of relevant environment variables or configuration files.

PROXY_ENABLE_BASIC_AUTH=true OCIS_INSECURE=true PROXY_HTTP_ADDR=0.0.0.0:9200 OCIS_URL=https://host.docker.internal:9200 PROXY_TRANSPORT_TLS_KEY=../ocis.pem PROXY_TRANSPORT_TLS_CERT=../ocis.crt ./bin/ocis server

@saw-jan
Copy link
Member

saw-jan commented Jan 18, 2024

Similar findings with search also #7262

@ScharfViktor
Copy link
Contributor

do we use anywhere propfind on the share-jail? (I didn't find)

web uses propfind on the mounpoint of shared folder which gives <d:href>/dav/spaces/mounpoitUUID</d:href> - works correct

We raised the same issue here and after a long time we closed it as is: #6376

@individual-it
Copy link
Member Author

The moodle plugin is doing that using the PHP-SDK.
What happens there is

  1. get all drives and finds the drive-id of the share-drive
  2. gets the drive meta-data via /v1.0/drives/{drive-id}
  3. makes a PROPFIND on that drive using its webDavUrl property
  4. when selecting a sub-folder it will again make a PROPFIND doing webDavUrl of the drive + selected folder name (taken from the last PROPFIND response

@micbar wrote in #6376

Once the share was accepted by the recipient, it will not be affected by a rename on the owner side. This prevents "naming battles" because people tend to be very obsessed of naming and like to have it under their control.

and

We should not rename the file on the recipient side (actual behavior), so no notification is needed.

But in this issue here, the IS a rename happening on the recipient side.
oc:name and d:displayname show the new name, but that cannot be used for webDAV requests.

So as I understand #6376 d:href is currently correct but the rename of oc:name and d:displayname should not happen.
If that would be the case, the moodle plugin would work fine.

@ScharfViktor
Copy link
Contributor

  1. get all drives and finds the drive-id of the share-drive
  2. gets the drive meta-data via /v1.0/drives/{drive-id}

Then I suggest you workaround (I hope it can help)

Screenshot 2024-01-18 at 12 31 19

@2403905 2403905 mentioned this issue Jan 18, 2024
9 tasks
@2403905
Copy link
Contributor

2403905 commented Jan 18, 2024

@individual-it I made the fix, that makes d:href and oc:name consistent. But a lot of tests failed.

@2403905
Copy link
Contributor

2403905 commented Jan 18, 2024

CI is pass

@2403905 2403905 self-assigned this Jan 18, 2024
@saw-jan
Copy link
Member

saw-jan commented Jan 19, 2024

@individual-it I made the fix, that makes d:href and oc:name consistent. But a lot of tests failed.

So now, the rename won't be populated to the receiver.

With fix (#8246):

<d:href>/dav/spaces/{virtual-share-drive-id}/afolder</d:href>
<d:propstat>
  <d:prop>
    <oc:name>afolder</oc:name>
    <d:displayname>afolder</d:displayname>
  </d:prop>
</d:propstat>

Before:

<d:href>/dav/spaces/{virtual-share-drive-id}/afolder</d:href>
<d:propstat>
  <d:prop>
    <oc:name>afolder-renamed</oc:name>
    <d:displayname>afolder-renamed</d:displayname>
  </d:prop>
</d:propstat>

@individual-it
Copy link
Member Author

individual-it commented Jan 19, 2024

Great!

@micbar
Copy link
Contributor

micbar commented Jan 19, 2024

Needs clarification.

@butonic

@2403905
Copy link
Contributor

2403905 commented Jan 22, 2024

Needs clarification.

@butonic
I followed on
#6376 (comment)

@phil-davis
Copy link
Contributor

yes, when a sharer shares a resource to a sharee, the sharee sees the name of the resource at that time. The sharee keeps seeing that name in the future, even if the sharer renames or moves the resource. That is the "traditional" oC10 behavior that is preserved in ocis.

@micbar
Copy link
Contributor

micbar commented Jan 22, 2024

@phil-davis Thanks for posting the expected behavior.

I can confirm that for ocis.

@micbar
Copy link
Contributor

micbar commented Jan 22, 2024

This feature is there to avoid naming collisions. The WebDAV listing of the Share Jail should NEVER have any naming collisions.

@micbar micbar added this to the Release 5.0.0 milestone Jan 22, 2024
@saw-jan
Copy link
Member

saw-jan commented Jan 26, 2024

For old/new webdav paths, the behavior is still the same.

https://localhost:9200/remote.php/webdav/Shares
https://localhost:9200/remote.php/dav/files/<username>/Shares
<d:href>/remote.php/dav/files/<username>/Shares/afolder</d:href>
<d:propstat>
  <d:prop>
    <oc:name>afolder-renamed</oc:name>
    <d:displayname>afolder-renamed</d:displayname>
  </d:prop>
</d:propstat>

CC @2403905 . Should we re-open the issue?

@2403905
Copy link
Contributor

2403905 commented Feb 6, 2024

@saw-jan merge the PR
Please validate.

@saw-jan
Copy link
Member

saw-jan commented Feb 7, 2024

@saw-jan merge the PR Please validate.

Fix cofirmed. Fixed for all webdav endpoints old/new/spaces
API test coverage added in #8295

I am clsoing this issue here.

CC @2403905

@saw-jan saw-jan closed this as completed Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants