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.

Sign up for GitHub

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

Jump to bottom

Rewire dav files endpoint to home #1120

Closed
wants to merge 2 commits into from
Closed

Rewire dav files endpoint to home #1120

wants to merge 2 commits into from

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Aug 25, 2020 •
edited
Loading

If the user specified in the dav files URL matches the current one,
rewire it to use the webDavHandler which is wired to the home storage.

This fixes path mapping issues.

  • add changelog
  • EOS
    • test regular file operations on dav files
    • test upload on dav files => 💥
    • test file ops on a received share
    • test upload on a received share
  • OC storage
    • test regular file operations on dav files
    • test upload on dav files
    • test file ops on a received share
    • test upload on a received share
  • BUG: litmus issue with OPTIONS on new dav:
 2. options............... FAIL (OPTIONS on base collection `/remote.php/dav/files/4c510ada-c86b-4815-8820-42cdf82c3d51/litmus/': Could not read status line: connection was closed by server)
  • fix base path for PROPFIND response on new dav

@butonic FYI

@update-docs
Copy link

update-docs bot commented Aug 25, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@PVince81 PVince81 mentioned this pull request Aug 25, 2020
Closed
@PVince81
Copy link
Contributor Author

the rewiring logic itself is broken, needs debugging

butonic
butonic reviewed Aug 25, 2020
View reviewed changes
internal/http/services/owncloud/ocdav/ocdav.go
@@ -175,9 +175,32 @@ func (s *svc) Handler() http.Handler {
// oc uses /dav/files/$user -> /$home/$user/...
// for oc we need to prepend the path to user homes
// or we take the path starting at /dav and allow rewriting it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR finally addresses the above comments. The /webdav and dav/files/ endpoints need to show the same tree for a user. /dav/files/ can also use the userid for other users, which is currently forbidden by oc10 but for ocis access is in theory possible using the eos driver because access is based on acls.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@labkode how dous this fit with the way cern exposes projects at /webdav? Afaict this is better solved by the spaces concept we plan to introduce: an endpoint that iterates the storages a user has access to, as well as the quota, usage and root etag information.

@PVince81
Copy link
Contributor Author

the path parts are not split in the expected way, need some trimming

Rewire dav files endpoint to home
9573b60 
If the user specified in the dav files URL matches the current one,
rewire it to use the webDavHandler which is wired to the home storage.

This fixes path mapping issues.
@PVince81
Copy link
Contributor Author

PVince81 commented Aug 25, 2020 •
edited
Loading

I've updated the code.

At least a curl request goes through, like : curl -D - -X PROPFIND -u einstein:relativity -k https://localhost:9200/remote.php/dav/files/4c510ada-c86b-4815-8820-42cdf82c3d51 or curl -D - -X PROPFIND -u einstein:relativity -k https://localhost:9200/remote.php/dav/files/4c510ada-c86b-4815-8820-42cdf82c3d51/Shares

Upload is broken: the initial POST works, but the subsequent PATCH requests return a 404:

ocis          | 2020-08-25T17:57:12Z INF access token is already provided pkg=rhttp service=reva traceid=8b301035c954af5c60a3181424b5b37b
ocis          | 2020-08-25T17:57:12Z DBG sending request to internal data server pkg=rhttp service=reva target=http://localhost:9156/data/e8364ebd-19fa-4982-9ef6-57c464129e8e traceid=8b301035c954af5c60a3181424b5b37b
ocis          | 2020-08-25T17:57:12Z INF access token is already provided pkg=rhttp service=reva traceid=8b301035c954af5c60a3181424b5b37b
ocis          | 2020-08-25T17:57:12Z INF tusd routing: path=/e8364ebd-19fa-4982-9ef6-57c464129e8e pkg=rhttp service=reva traceid=8b301035c954af5c60a3181424b5b37b
ocis          | 2020-08-25T17:57:12Z WRN http end="25/Aug/2020:17:57:12 +0000" host=127.0.0.1 method=PATCH pkg=rhttp proto=HTTP/1.1 service=reva size=17 start="25/Aug/2020:17:57:12 +0000" status=404 time_ns=763138 traceid=8b301035c954af5c60a3181424b5b37b uri=/data/e8364ebd-19fa-4982-9ef6-57c464129e8e url=/data/e8364ebd-19fa-4982-9ef6-57c464129e8e
ocis          | 2020-08-25T17:57:12Z WRN http end="25/Aug/2020:17:57:12 +0000" host=127.0.0.1 method=PATCH pkg=rhttp proto=HTTP/1.1 service=reva size=0 start="25/Aug/2020:17:57:12 +0000" status=404 time_ns=3211395 traceid=8b301035c954af5c60a3181424b5b37b uri=/data/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZXZhIiwiZXhwIjoxNTk4NDY0NjMyLCJpYXQiOjE1OTgzNzgyMzIsInRhcmdldCI6Imh0dHA6Ly9sb2NhbGhvc3Q6OTE1Ni9kYXRhL2U4MzY0ZWJkLTE5ZmEtNDk4Mi05ZWY2LTU3YzQ2NDEyOWU4ZSJ9.HfKM0jlzSGtsMZACtUk-71_6ZUrwtJqhu2-IKghxMMI url=/data/eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZXZhIiwiZXhwIjoxNTk4NDY0NjMyLCJpYXQiOjE1OTgzNzgyMzIsInRhcmdldCI6Imh0dHA6Ly9sb2NhbGhvc3Q6OTE1Ni9kYXRhL2U4MzY0ZWJkLTE5ZmEtNDk4Mi05ZWY2LTU3YzQ2NDEyOWU4ZSJ9.HfKM0jlzSGtsMZACtUk-71_6ZUrwtJqhu2-IKghxMMI

it says "upload not found".
I wonder if there's some storage id thing in there.

Note: this was testing with EOS with OCIS and the settings from owncloud/ocis#484

@PVince81
Copy link
Contributor Author

Uploading to OC storage works fine, so the problem is specific to EOS and/or the current setup.

@butonic
Copy link
Contributor

butonic commented Aug 26, 2020

  • move interception from dav to files handler

@PVince81
Copy link
Contributor Author

650+ failures in the API tests. With upload broken a lot of failures are to be expected.

@butonic
Copy link
Contributor

butonic commented Aug 26, 2020

also needs #1124

@PVince81 PVince81 mentioned this pull request Aug 26, 2020
Merged
7 tasks
@PVince81
Copy link
Contributor Author

better approach here: #1125

@PVince81 PVince81 closed this Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@butonic butonic butonic left review comments

@glpatcern glpatcern Awaiting requested review from glpatcern glpatcern will be requested when the pull request is marked ready for review glpatcern is a code owner

@diocas diocas Awaiting requested review from diocas diocas will be requested when the pull request is marked ready for review diocas is a code owner

@jessegeens jessegeens Awaiting requested review from jessegeens jessegeens will be requested when the pull request is marked ready for review jessegeens is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@PVince81 @butonic