pimutils / vdirsyncer

📇 Synchronize calendars and contacts.
https://vdirsyncer.pimutils.org/
Other
1.57k stars 163 forks source link

vdirsyncer 0.19: digest auth broken #1015

Closed 0-wiz-0 closed 1 month ago

0-wiz-0 commented 1 year ago

When I updated from 0.18 to 0.19, 'vdirsyncer sync' broke.

Syncing remote1/default                                                                                                                                                                                       
error: Unknown error occurred for remote1/default: BasicAuth() tuple is required instead                                                                                                                      
error: Use `-vdebug` to see the full traceback.

with -vdebug:

debug:   File "/usr/pkg/lib/python3.11/site-packages/vdirsyncer/cli/tasks.py", line 72, in sync_collection
debug:     await sync.sync(                                                                                      
debug:   File "/usr/pkg/lib/python3.11/site-packages/vdirsyncer/sync/__init__.py", line 144, in sync                                                                                                                               
debug:     a_nonempty = await a_info.prepare_new_status()                
debug:                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                        
debug:   File "/usr/pkg/lib/python3.11/site-packages/vdirsyncer/sync/__init__.py", line 48, in prepare_new_status 
debug:     async for href, etag in self.storage.list():                                                          
debug:   File "/usr/pkg/lib/python3.11/site-packages/vdirsyncer/storage/dav.py", line 859, in list                                                                                                                                 
debug:     async for href, etag in super().list():                                                               
debug:   File "/usr/pkg/lib/python3.11/site-packages/vdirsyncer/storage/dav.py", line 676, in list                                                                                                                                 
debug:     response = await self.session.request(                                                                                                                                                                                  
debug:                ^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                
debug:   File "/usr/pkg/lib/python3.11/site-packages/vdirsyncer/storage/dav.py", line 416, in request            
debug:     return await http.request(method, url, session=session, **more)                                       
debug:            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                
debug:   File "/usr/pkg/lib/python3.11/site-packages/vdirsyncer/http.py", line 132, in request                                                                                                                                     
debug:     response = await session.request(method, url, **kwargs)                                               
debug:                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                                                                 
debug:   File "/usr/pkg/lib/python3.11/site-packages/aiohttp/client.py", line 508, in _request                   
debug:     req = self._request_class(                                                                            
debug:           ^^^^^^^^^^^^^^^^^^^^                                                                            
debug:   File "/usr/pkg/lib/python3.11/site-packages/aiohttp/client_reqrep.py", line 310, in __init__            
debug:     self.update_auth(auth)                                                                                
debug:   File "/usr/pkg/lib/python3.11/site-packages/aiohttp/client_reqrep.py", line 495, in update_auth
debug:     raise TypeError("BasicAuth() tuple is required instead")

In case it matters, this is with Python 3.11.0 on NetBSD, aiohttp 3.8.3, server is running baikal 0.9.2

The config for the server looks like this:

type = "caldav"
url = "https://caldav.hostname.at/cal.php/principals/wiz/"
username = "wiz"
password = "something"
auth = "digest"

Removing auth or setting it to basic doesn't work.

WhyNotHugo commented 1 year ago

Just acknowledging that I saw this issue last week. This is indeed broken, I'll try to address it soon.

WhyNotHugo commented 1 year ago

Relevant: https://github.com/aio-libs/aiohttp/issues/4939

WhyNotHugo commented 1 year ago

Probably gonna have to copy chunks from https://github.com/aio-libs/aiohttp/pull/2213

WhyNotHugo commented 1 year ago

Pinning this issue since it's high priority to get this through.

hnrd commented 1 year ago

I have the same issue on a http remote:

[storage example_remote]
type = "http"
url = "https://mail.example.com/home/user@example.com/Calendar.ics"
auth = "guess"
username = "user"
password = "hunter2"
read_only = "true"

results in:

error: Unknown error occurred for example: BasicAuth() tuple is required instead
error: Use `-vdebug` to see the full traceback.
debug:   File "/usr/lib/python3.10/site-packages/vdirsyncer/cli/tasks.py", line 72, in sync_collection
debug:     await sync.sync(
debug:   File "/usr/lib/python3.10/site-packages/vdirsyncer/sync/__init__.py", line 145, in sync
debug:     b_nonempty = await b_info.prepare_new_status()
debug:   File "/usr/lib/python3.10/site-packages/vdirsyncer/sync/__init__.py", line 48, in prepare_new_status
debug:     async for href, etag in self.storage.list():
debug:   File "/usr/lib/python3.10/site-packages/vdirsyncer/storage/http.py", line 73, in list
debug:     r = await request(
debug:   File "/usr/lib/python3.10/site-packages/vdirsyncer/http.py", line 132, in request
debug:     response = await session.request(method, url, **kwargs)
debug:   File "/usr/lib/python3.10/site-packages/aiohttp/client.py", line 508, in _request
debug:     req = self._request_class(
debug:   File "/usr/lib/python3.10/site-packages/aiohttp/client_reqrep.py", line 310, in __init__
debug:     self.update_auth(auth)
debug:   File "/usr/lib/python3.10/site-packages/aiohttp/client_reqrep.py", line 495, in update_auth
debug:     raise TypeError("BasicAuth() tuple is required instead")
WhyNotHugo commented 1 year ago

A lot of the networking bits were re-written (in great part due to some dependencies being unmaintained themselves and not working on recent python versions).

During this rewrite, I omitted Digest Auth. It needs to be implemented (and I'll get around to it as soon as I get a chance), but there's no workaround in the meantime.

hnrd commented 1 year ago

digest was a good keyword for me, it works when I change auth to basic (remote is a Zimbra server) :+1:

gador commented 1 year ago

It seems this bug prevents me from using vdirsyncer, too with baikal. Baikal doesn't support basic-auth and digest seems to be broken with vdirsyncer. Is there any workaround?

clombt commented 1 year ago

It seems this bug prevents me from using vdirsyncer, too with baikal. Baikal doesn't support basic-auth and digest seems to be broken with vdirsyncer. Is there any workaround?

Just want to say that Baikal does support basic authentication (at least v0.9.3). The method can be set in the admin web interface under system settings. It works with vdirsyncer for me.

gador commented 1 year ago

@clombt ah thanks, I haven't seen this. Thanks for the pointer!

emuhr commented 1 year ago

Hi @WhyNotHugo do you have an update on this? I'm using vdirsyncer version 0.19.2_2 from my distribution and I'm facing the same issue.

dorchain commented 9 months ago

As there is no real progress on the underlying library, maybe it is worth looking for another solution, e.g. httpx

WhyNotHugo commented 9 months ago

Yeah, I should have used httpx rather than aiohttp.

Switching now would require rewriting a lot of the codebase, and I'm already focused on a whole rewrite. Adding digest auth to the new version will be easier.

If someone wants to port the current version to httpx, I'd have no objection. But I don't think it's good timing to do so now; once the rewrite is a bit more stable, adding digest auth support to it won't be too hard.

duckunix commented 8 months ago

For what it is worth, I am getting this trying to sync with Google. However, it is on some entries which I have not been able to track down. Otherwise, syncing works well.

malmeloo commented 2 months ago

Depending on how far that rewrite is coming along, may I suggest implementing a temporary solution that makes vdirsyncer handle digest auth? It could use requests's HTTPDigestAuth.build_digest_header to find the value of the Authorization header. That would resolve this issue without too much effort, at least until the rewrite is done.

If there are no plans to do something like this, I would at least suggest reporting a friendlier error message instead of the error that is shown now.

WhyNotHugo commented 2 months ago

We're not using requests, so I'm not entirely sure thatbuild_digest_header` would work.

I agree that a better error message should be shown here.

WhyNotHugo commented 2 months ago

@duckunix Google does not use Digest Auth. Can you open a separate issue with your configuration and the whole error? It is likely a different issue.

malmeloo commented 2 months ago

We're not using requests, so I'm not entirely sure thatbuild_digest_header` would work.

I agree that a better error message should be shown here.

I meant that as in, use that method to generate the content of the Authorization header, then set it in aiohttp when making the request. I may (emphasis) be able to cobble something together once I have some time, which would be in about a week or so.

malmeloo commented 1 month ago

I finally got around to it and implemented digest auth in #1137. It's not the prettiest solution since it tries hard to retrofit into the existing code base but it appears to work perfectly.

WhyNotHugo commented 1 month ago

Fixed in https://github.com/pimutils/vdirsyncer/commit/35f299679ff9740f8c26661dde8458d4e67efcca

0-wiz-0 commented 1 month ago

I tried 0.19.3 and it works for me. Thank you, @malmeloo !