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

vdirsyncer 0.19: digest auth broken #1015

Closed
0-wiz-0 opened this issue Dec 8, 2022 · 21 comments
Closed

vdirsyncer 0.19: digest auth broken #1015

0-wiz-0 opened this issue Dec 8, 2022 · 21 comments

Comments

@0-wiz-0
Copy link
Contributor

0-wiz-0 commented Dec 8, 2022

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.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 11, 2022
@WhyNotHugo
Copy link
Member

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

@WhyNotHugo
Copy link
Member

Relevant: aio-libs/aiohttp#4939

@WhyNotHugo
Copy link
Member

Probably gonna have to copy chunks from aio-libs/aiohttp#2213

@WhyNotHugo
Copy link
Member

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

@hnrd
Copy link

hnrd commented Jan 24, 2023

I have the same issue on a http remote:

[storage example_remote]
type = "http"
url = "https://mail.example.com/home/[email protected]/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
Copy link
Member

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
Copy link

hnrd commented Jan 24, 2023

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

@gador
Copy link

gador commented May 30, 2023

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
Copy link

clombt commented Jun 8, 2023

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
Copy link

gador commented Jun 12, 2023

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

@emuhr
Copy link

emuhr commented Oct 17, 2023

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
Copy link

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

@WhyNotHugo
Copy link
Member

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
Copy link

duckunix commented Feb 17, 2024

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
Copy link
Contributor

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
Copy link
Member

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

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

@WhyNotHugo
Copy link
Member

@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
Copy link
Contributor

We're not using requests, so I'm not entirely sure that build_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
Copy link
Contributor

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
Copy link
Member

Fixed in 35f2996

@0-wiz-0
Copy link
Contributor Author

0-wiz-0 commented Sep 12, 2024

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

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

No branches or pull requests

9 participants