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

Public dav endpoint doesn't allow GET of files #19700

Closed
skjnldsv opened this issue Feb 28, 2020 · 3 comments · Fixed by #32400
Closed

Public dav endpoint doesn't allow GET of files #19700

skjnldsv opened this issue Feb 28, 2020 · 3 comments · Fixed by #32400

Comments

@skjnldsv
Copy link
Member

skjnldsv commented Feb 28, 2020

Because the token is passed as basic auth, you cannot do a simple get without this parameter, which is not what a basic auth is made for anyway.

We should use the cookie for authentication, and let us direct access any file a PROPFIND returns as this is most likely not compliant to any dav endpoint :)

/**
* Validates a username and password
*
* This method should return true or false depending on if login
* succeeded.
*
* @param string $username
* @param string $password
*
* @return bool
* @throws \Sabre\DAV\Exception\NotAuthenticated
*/
protected function validateUserPass($username, $password) {
try {
$share = $this->shareManager->getShareByToken($username);
} catch (ShareNotFound $e) {
return false;
}

userName: token,

@skjnldsv skjnldsv added enhancement 0. Needs triage Pending check for reproducibility or if it fits our roadmap 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Feb 28, 2020
@skjnldsv skjnldsv modified the milestones: Nextcloud 18.0.3, Nextcloud 19 Feb 28, 2020
@juliusknorr
Copy link
Member

The token as username is not only for public share pages but also used by federated sharing this way. Also just storing in a cookie won't work as you could have multiple share links open at the same time, but we probably could store a list of all share tokens there.

cc @rullzer

@skjnldsv
Copy link
Member Author

skjnldsv commented Mar 4, 2020

The token as username is not only for public share pages but also used by federated sharing this way. Also just storing in a cookie won't work as you could have multiple share links open at the same time, but we probably could store a list of all share tokens there.

Argh, this complicates :(
We still should avoid this. What about a special dav endpoint like public.php/dav/TOKEN instead?

@ShaunCurrier
Copy link

This re-purposing of the basic auth header for sending the public share token also breaks my reverse proxy configuration. I know this isn't an officially supported configuration, but I use basic auth on the reverse proxy to keep the big bad internet away from the Nextcloud application. I send the (really long) basic auth credentials to anyone I want to share a public link with. Problem is, the page only partially loads because it breaks when it gets to the public.php/webdav request which seems to require the basic auth header when hitting the backend server. The reverse proxy is stripping that off, breaking the page. I wish the header wouldn't be abused in this way so that standard things could work in standard ways.

Just my two cents. I still love Nextcloud!

@blizzz blizzz removed this from the Nextcloud 23 milestone Nov 30, 2021
@blizzz blizzz added this to the Nextcloud 24 milestone Nov 30, 2021
@blizzz blizzz modified the milestones: Nextcloud 24, Nextcloud 25 Apr 21, 2022
@skjnldsv skjnldsv mentioned this issue May 15, 2022
3 tasks
@skjnldsv skjnldsv added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels May 15, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Oct 19, 2022
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants