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

add webdav path resolver composite #52

Merged
merged 4 commits into from
Oct 3, 2021

Conversation

thestomprock
Copy link

Hi,
I added an WebDavPath resolver. This is because, my nextcloud instance ( 20.0.4) does not have the same default paths
as the implementation on the master branch.
I added an interface where you can composite the path calculation.
The default behavior still remains the same.

Mark

@a-schild
Copy link
Owner

@thestomprock Thanks for your updated patch.
It indeed looks like the webdav url has changed from server/remote.php/webdav to server/remote.php/dav/files/USERNAME/ somewhere arround nextcloud v12-14.

nextcloud/server#8795

But as far as I can tell, the "old" url is still working as of nextcloud v20.0.4.
I wonder if we should implement an autodetection of the correct webdav url for the user with the given credentials...

@thestomprock
Copy link
Author

@a-schild added auto detection of the correct path/ncVersion
Additionally to FILES I added VCARD and CALDAV paths.
Everything defaults to the values as it was.

@tobiasKaminsky
Copy link

But as far as I can tell, the "old" url is still working as of nextcloud v20.0.4.

Not sure how old NC server you want to support, but all suported NC version have new dav already in place, same is since NC12, so I would not go with auto detect, but simply use newest endpoint.

@a-schild
Copy link
Owner

a-schild commented Sep 9, 2021

There is a bug in your code, in the test cases the resulting remote path is incorrect.
It constructs this:

https://aarboard.oncloud7.ch:443/%5Cremote.php%5Cdav%5Cfiles%5Ca.schild%5Ctest6.txt/

but correct would be:

https://aarboard.oncloud7.ch:443/remote.php/dav/files/a.schild/test6.txt

Some / are encoded as hex values and there are too many / at the start+end of the path

@a-schild a-schild merged commit b11799e into a-schild:master Oct 3, 2021
@a-schild
Copy link
Owner

@thestomprock There is a problem with your pull request.
Currently you build the username part of the URL from the user login name, but this name is not always the same as the one used in the URL.
Most notably when using a LDAP backend, chances are that the username is not the one from the login credentials.
See issue #68

I think to solve the problem, we must login to NC and then retrieve the current user ID and then use this as the username part in the url.
Also, when using sso with bearer authentification, no username is passed to the server in the request, so we will also need to detect the user name for URL generation

@a-schild
Copy link
Owner

https://docs.nextcloud.com/server/latest/developer_manual/client_apis/LoginFlow/index.html
Section about obtaining credentials mentions the potential difference between login name and user name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants