Create Task
Maniphest T5562

kdav jobs don't throw an error if the authentication failed.
Open, NormalPublic
Actions

Description

If you try to fetch collections with wrong credentials (doe@example.org instead of john.doe@example.org), the the collectionfetchjob will happily complete and not report an error but not retrieve any collections. Which will delete all collections locally.
A potential response is:

BBE}@@PrJ#@i]
'm&HTTP/1.1 207 Multi-Status
Date: Thu, 09 Mar 2017 17:23:46 GMT
Server: Apache/2.4.6 (CentOS) PHP/5.4.16
X-Powered-By: PHP/5.4.16
X-Sabre-Version: 2.1.11
Vary: Brief,Prefer
DAV: 1, 3, extended-mkcol, access-control, calendarserver-principal-property-search, calendar-access, calendar-proxy, calendar-schedule, calendar-auto-schedule, addressbook, 2
Content-Length: 947
Keep-Alive: timeout=5, max=93
Connection: Keep-Alive
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:cal="urn:ietf:params:xml:ns:caldav" xmlns:cs="http://calendarserver.org/ns/" xmlns:card="urn:ietf:params:xml:ns:carddav"><d:response><d:href>/iRony/addressbooks/doe@example.org/</d:href><d:propstat><d:prop><d:displayname/><d:resourcetype/><cs:getctag/></d:prop><d:status>HTTP/1.1 403 Forbidden</d:status></d:propstat></d:response><d:response><d:href>/iRony/addressbooks/doe@example.org/cd2ee6cd-b5f9-4639-9373-01784e367a49/</d:href><d:propstat><d:prop><d:displayname/><d:resourcetype/><cs:getctag/></d:prop><d:status>HTTP/1.1 403 Forbidden</d:status></d:propstat></d:response><d:response><d:href>/iRony/addressbooks/doe@example.org/f3283c17-8213-4d78-9dd1-7d24fbc250f2/</d:href><d:propstat><d:prop><d:displayname/><d:resourcetype/><cs:getctag/></d:prop><d:status>HTTP/1.1 403 Forbidden</d:status></d:propstat></d:response></d:multistatus>

The request:

BBE@@,rP#gJeY-
&&<?xml version="1.0" encoding="utf-8"?>
<propfind xmlns="DAV:">
 <prop xmlns="DAV:">
  <displayname xmlns="DAV:"/>
  <resourcetype xmlns="DAV:"/>
  <getctag xmlns="http://calendarserver.org/ns/"/>
 </prop>
</propfind>

Another potential response:

BBE	@@>PVVTZ
KHKHTTP/1.1 401 Unauthorized
Date: Thu, 09 Mar 2017 16:53:20 GMT
Server: Apache/2.4.6 (CentOS) PHP/5.4.16
X-Powered-By: PHP/5.4.16
WWW-Authenticate: Basic realm="KolabDAV"
Content-Length: 292
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
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:sabredav-version>2.1.11</s:sabredav-version>
  <s:exception>Sabre\DAV\Exception\NotAuthenticated</s:exception>
  <s:message>No basic authentication headers were found</s:message>
</d:error>

Not sure what triggers one or the other response, but both are not recognized as errors.

knauss added a subscriber: knauss.Mar 9 2017, 8:51 PM

Just a little bit curious: is "BBE}@@PrJ#@i]\n'm&" etc are sent frome the server? If yes the server is comply broken, because the first bytes must be something like "HTTP/..."

I think you are talking about collectionsfetchjob ( with an s), aren't you?

On thing you need to make sure, if you are in the DavPrincipalHomeSetsFetchJob case ( collectionsfetchjob.cpp l.45), this triggers before the collectionsfetch a different job.

But nevertheless the first error looks like an error inside collectionsfetchjob. So far I expect the problem is l235 that makes the job not fail, if one collection is not accessible. But the check in l247 do not fail if no object is accessible.

For the other problem I don't see the source of the problem dirrectly.

But nevertheless the complete code has not very much testcases, so I can't tell you why this detection is there etc. But you can now easily create a new testcase now. See autotests/davitemslistjobtest.cpp, you can use fakeserver and a textfile in the autotests/data dir to add these testcases.
C- is the client data that is sent (header) , clientdata is not implemented yet
S- server header
D-server data
(the Content-Length header is calculated automatically for the data lines to have ever the correct sent length of the data, that's actually why S and D is seperated.)

cmollekopf added a comment.Mar 9 2017, 10:35 PM

The garbage output is just wireshark not copying the packet properly.

I guess the question is whether there is "normal" case where we get a "403 Forbidden" response. If we're supposed to ignore that silently it becomes difficult to distinguish what's expected and what isn't...

It's actually two requests being sent like this:

PROPFIND /iRony/addressbooks/doe%40example.org HTTP/1.1
Host: kolab
Connection: keep-alive
User-Agent: Mozilla/5.0 (X11; Linux x86_64) KHTML/5.27.0 (like Gecko) Konqueror/5.27
Pragma: no-cache
Cache-control: no-cache
Accept: text/html, text/*;q=0.9, image/jpeg;q=0.9, image/png;q=0.9, image/*;q=0.9, */*;q=0.8
Accept-Encoding: gzip, deflate, x-gzip, x-deflate
Accept-Charset: utf-8,*;q=0.5
Accept-Language: en-US,en;q=0.9
Content-Type: text/xml
Depth: 0
Content-Length: 243

BBE'G@@FPeSTPYG
<?xml version="1.0" encoding="utf-8"?>
<propfind xmlns="DAV:">
 <prop xmlns="DAV:">
  <addressbook-home-set xmlns="urn:ietf:params:xml:ns:carddav"/>
  <current-user-principal xmlns="DAV:"/>
  <principal-URL xmlns="DAV:"/>
 </prop>
</propfind>
BBE@@YPFTPeTZ
HTTP/1.1 401 Unauthorized
Date: Thu, 09 Mar 2017 22:23:03 GMT
Server: Apache/2.4.6 (CentOS) PHP/5.4.16
X-Powered-By: PHP/5.4.16
WWW-Authenticate: Basic realm="KolabDAV"
Content-Length: 292
Keep-Alive: timeout=5, max=100
Connection: Keep-Alive
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:sabredav-version>2.1.11</s:sabredav-version>
  <s:exception>Sabre\DAV\Exception\NotAuthenticated</s:exception>
  <s:message>No basic authentication headers were found</s:message>
</d:error>
PROPFIND /iRony/addressbooks/doe%40example.org HTTP/1.1
Host: kolab
Connection: keep-alive
User-Agent: Mozilla/5.0 (X11; Linux x86_64) KHTML/5.27.0 (like Gecko) Konqueror/5.27
Pragma: no-cache
Cache-control: no-cache
Accept: text/html, text/*;q=0.9, image/jpeg;q=0.9, image/png;q=0.9, image/*;q=0.9, */*;q=0.8
Accept-Encoding: gzip, deflate, x-gzip, x-deflate
Accept-Charset: utf-8,*;q=0.5
Accept-Language: en-US,en;q=0.9
Content-Type: text/xml
Depth: 0
Authorization: Basic ZG9lQGV4YW1wbGUub3JnOldlbGNvbWUyS29sYWJTeXN0ZW1z
Content-Length: 243


BBE'G@@FPeVTYG
<?xml version="1.0" encoding="utf-8"?>
<propfind xmlns="DAV:">
 <prop xmlns="DAV:">
  <addressbook-home-set xmlns="urn:ietf:params:xml:ns:carddav"/>
  <current-user-principal xmlns="DAV:"/>
  <principal-URL xmlns="DAV:"/>
 </prop>
</propfind>
BBE@@PFTeW\
HTTP/1.1 207 Multi-Status
Date: Thu, 09 Mar 2017 22:23:03 GMT
Server: Apache/2.4.6 (CentOS) PHP/5.4.16
X-Powered-By: PHP/5.4.16
X-Sabre-Version: 2.1.11
Vary: Brief,Prefer
DAV: 1, 3, extended-mkcol, access-control, calendarserver-principal-property-search, calendar-access, calendar-proxy, calendar-schedule, calendar-auto-schedule, addressbook, 2
Content-Length: 482
Keep-Alive: timeout=5, max=99
Connection: Keep-Alive
Content-Type: application/xml; charset=utf-8

<?xml version="1.0" encoding="utf-8"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:cal="urn:ietf:params:xml:ns:caldav" xmlns:cs="http://calendarserver.org/ns/" xmlns:card="urn:ietf:params:xml:ns:carddav"><d:response><d:href>/iRony/addressbooks/doe@example.org/</d:href><d:propstat><d:prop><card:addressbook-home-set/><d:current-user-principal/><d:principal-URL/></d:prop><d:status>HTTP/1.1 403 Forbidden</d:status></d:propstat></d:response></d:multistatus>

This is now in proper chronological order.

knauss added a comment.Mar 10 2017, 7:58 PM

Ah okay, now getting a more cleaner view :) The first roundtrip request of the addressbook is without auth info (no Authorization header). This is the common behaviour, i think the idea behind is to not send auth data to server that do not require it. And because we get an 401 a second request is executed with the Authentification header added. So far so good, this is all the expected behaviour.

But now the server response with a 207 - this is 200er reponse, so the request is okay and we get a valid XML back. The problem I see, that we can get 40X repones together with good reponses. So how should the job behave? I think not fail if we get an 20X response is a valid behavour. On the other side the application executing the job must have a possibility to detect a error with one of the collections. But I think you need to read more about the DAV Protocol, what are the possibile reponses from the server. Than we can think about, how we match the server reponses to job results.

cmollekopf moved this task from Backlog to 0.6 on the Kube board.Feb 3 2018, 1:01 PM
cmollekopf edited projects, added Kube (0.6); removed Kube.
cmollekopf moved this task from Backlog to In Progress on the Kube (0.6) board.Feb 28 2018, 2:03 PM
cmollekopf edited projects, added Sink; removed Kube (0.6).Feb 28 2018, 2:18 PM