Skip to content

Update sabre dav to 3.2#26115

Merged
DeepDiver1975 merged 6 commits into
masterfrom
update-sabre-dab
Oct 13, 2016
Merged

Update sabre dav to 3.2#26115
DeepDiver1975 merged 6 commits into
masterfrom
update-sabre-dab

Conversation

@DeepDiver1975

Copy link
Copy Markdown
Member

Update sabre/dav

@butonic

butonic commented Sep 26, 2016

Copy link
Copy Markdown
Contributor

tested up and downloads, rename and delete in chrome and vie sync client. everything working as expectedt and no errors in the log. sync to / from smb / wnd working fine.

👍

@butonic

butonic commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

@VicDeo a second review for updating sabre/dav?

@butonic

butonic commented Sep 27, 2016

Copy link
Copy Markdown
Contributor

@tomneedham a second review for updating sabre/dav?

@butonic butonic changed the title Update sabre dab Update sabre dav Sep 29, 2016
@PVince81

PVince81 commented Oct 5, 2016

Copy link
Copy Markdown
Contributor

Conflicts ahead

@DeepDiver1975

Copy link
Copy Markdown
Member Author

Will take care ....

@DeepDiver1975

Copy link
Copy Markdown
Member Author

fuxx

bildschirmfoto von 2016-10-06 13-42-11

@PVince81 PVince81 changed the title Update sabre dav Update sabre dav to 3.2 Oct 10, 2016
@PVince81 PVince81 mentioned this pull request Oct 10, 2016
@PVince81

Copy link
Copy Markdown
Contributor

Fixes #22724

@PVince81

Copy link
Copy Markdown
Contributor

Hmm, weird. Something seems wrong with the authentication. When debugging, the PROPFIND reaches the server but the user session has no user.

@PVince81

Copy link
Copy Markdown
Contributor

When debugging into the server, the $_SERVER variable of the PROPFIND request contains neither username/password pair no Authorization header. The calling code from the integration test does specify credentials to be passed to curl.

@PVince81

Copy link
Copy Markdown
Contributor

Tried updating the sabre version used by integration tests, no success.

@DeepDiver1975 I guess we also want to upgrade the sabre dav version for the integration tests ? This is mostly using the sabre client stuff. (I can do it)

@PVince81

PVince81 commented Oct 12, 2016

Copy link
Copy Markdown
Contributor

on an unrelated note, the basic auth plugin is registered twice:

  $this->backends                = (array[4]);
    $this->backends[0]           = (object|OCA\DAV\Connector\Sabre\Auth[7])+;
    $this->backends[1]           = (object|OCA\DAV\DAV\PublicAuth[1])+;
    $this->backends[2]           = (object|OCA\Federation\DAV\FedAuth[3])+;
    $this->backends[3]           = (object|OCA\DAV\Connector\Sabre\Auth[7])+;

something to look into separately, adding to "suspicious stuff to investigate to do list"...

raised #26351

@PVince81

Copy link
Copy Markdown
Contributor

I also debugged into the PROPPATCH test row which appears green, that one also internally fails because of missing auth headers. Weird, I don't see how updating the server dav version would affect the headers sent by the integration tests...

@PVince81

Copy link
Copy Markdown
Contributor

Web UI setting as favorite with old and new endpoint (#25494) do work against this PR.

@PVince81

Copy link
Copy Markdown
Contributor

Okay, I'm onto something:

  • on master, the requests are sent twice: first fails with 401, second goes through with auth headers.
  • on this PR: the request is sent, fails with 401 but no "www-authenticate" header is set when running integration test (from CLI curl it is set)

@PVince81

Copy link
Copy Markdown
Contributor

When called from CLI curl I can see that the $response contains the "www-authenticate" header but it does not when coming from the integration test. Now need to examine the differences in code path or HTTP request.

@PVince81

Copy link
Copy Markdown
Contributor

Okay, here we go, likely a plugin order issue:

  • master returns 401 when no credentials given:
  % curl -D - -X PROPFIND http://localhost/owncloud/remote.php/dav/files/user0/FOLDER
HTTP/1.1 401 Unauthorized
Date: Wed, 12 Oct 2016 09:36:08 GMT
Server: Apache
X-Powered-By: PHP/5.6.26
Set-Cookie: oc2d13a6a068=ihh4c69mtmaidedosah7m9r3abd3mbl341nomi0u0h3q7dd61ri0; path=/owncloud; HttpOnly
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache
Set-Cookie: oc_sessionPassphrase=T9p5DdLAyuXmvrR97RJHWLqfLhdxJpUayJLzY7SAVOET1SnqVH2jiQGyeAlfmujxj4et77L6Q9%2FweOKlG3pxCUPEwef9hs%2F4wS062X09A0y68xLlMMxMDZPp9JOYPMxq; path=/owncloud; httponly
Content-Security-Policy: default-src 'none';
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Frame-Options: Sameorigin
X-Robots-Tag: none
X-Download-Options: noopen
X-Permitted-Cross-Domain-Policies: none
WWW-Authenticate: Basic realm="ownCloud"
WWW-Authenticate: Basic realm="ownCloud"
WWW-Authenticate: Basic realm="ownCloud"
Content-Length: 558
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\NotAuthenticated</s:exception>
  <s:message>No 'Authorization: Basic' header found. Either the client didn't send one, or the server is mis-configured, No public access to this resource., No 'Authorization: Basic' header found. Either the client didn't send one, or the server is mis-configured, No 'Authorization: Basic' header found. Either the client didn't send one, or the server is mis-configured</s:message>
</d:error>
  • on this PR, 404 is returned instead of 401:
  % curl -D - -X PROPFIND http://localhost/owncloud/remote.php/dav/files/user0/FOLDER
HTTP/1.1 404 Not Found
Date: Wed, 12 Oct 2016 09:39:15 GMT
Server: Apache
X-Powered-By: PHP/5.6.26
Set-Cookie: oc2d13a6a068=8qnuq6jevg35s1s9h0km7uv4g2rqd4rb53pveeefuoe5qnea2m80; path=/owncloud; HttpOnly
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache
Set-Cookie: oc_sessionPassphrase=vr7D8H%2B5CUtm77H%2B9uliJr6WDsquKku%2B0sEMIigxEaRM9yzhYNd%2BMA9gHBO8fsRDtn%2FtEtHj9CmcCh0aomNPZZrLhQ2g5rmHcdGlbOFoNm%2FKpBiGTfSlTRYCf84zzYes; path=/owncloud; httponly
Content-Security-Policy: default-src 'none';
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Frame-Options: Sameorigin
X-Robots-Tag: none
X-Download-Options: noopen
X-Permitted-Cross-Domain-Policies: none
Content-Length: 227
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>Sabre\DAV\Exception\NotFound</s:exception>
  <s:message>Principal with name user0 not found</s:message>
</d:error>

@PVince81

Copy link
Copy Markdown
Contributor

Okayyyy, getting closer... In the old Sabre, if at least one of the auth backends would fail, it would directly throw a NotAuthenticatedException which would bypass any additional plugins.

On the new Sabre, somehow it seems to happily continue processing other plugins and going into the Propfind method. Maybe this is by design for better plugin flexibility...

@PVince81

Copy link
Copy Markdown
Contributor

and here it is "Support for unauthenticated access": http://sabre.io/dav/upgrade/3.1-to-3.2/

At the start of the request, see if authentication headers were set. If they were, attempt to authenticate the user but don't deny the user if they aren't set or incorrect.

@PVince81

Copy link
Copy Markdown
Contributor

So, according to the docs we'd eventually need to better integrate with the ACL plugin so that any of the regular non-auth plugins are able to fail if a user's ACL does not fit.

I think this is something for later. I'll use the workaround for now and set "$allowUnauthenticatedAccess" to false.

This is needed to make Sabre 3.2 behave like we did before.
Eventually we should integrate better with the ACL plugin which itself
should implement an auth failure when appropriate.
@PVince81

Copy link
Copy Markdown
Contributor

@DeepDiver1975 tests should pass now: 459bde5

@DeepDiver1975

Copy link
Copy Markdown
Member Author

NICE!

@DeepDiver1975 DeepDiver1975 merged commit 6b8e8ed into master Oct 13, 2016
@DeepDiver1975 DeepDiver1975 deleted the update-sabre-dab branch October 13, 2016 10:15
@rullzer rullzer mentioned this pull request Oct 18, 2016
1 task
elie195 pushed a commit to elie195/core that referenced this pull request Mar 23, 2017
* Update sabre/dav to 3.2.0

* Adjust code to work with sabre/dav 3.2.0 and it's dependencies

* Adding own CalDAV plugin to fix calendar home property

* Test if there is a user logged in when listing files home

* Update sabre version used by integration tests

* Disable unauthenticated DAV access

This is needed to make Sabre 3.2 behave like we did before.
Eventually we should integrate better with the ACL plugin which itself
should implement an auth failure when appropriate.
@lock

lock Bot commented Aug 4, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants