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

extract_auth_types - shortening strings #289

Closed
jdrozdnovak opened this issue Feb 25, 2023 · 9 comments · Fixed by home-assistant/core#88791
Closed

extract_auth_types - shortening strings #289

jdrozdnovak opened this issue Feb 25, 2023 · 9 comments · Fixed by home-assistant/core#88791

Comments

@jdrozdnovak
Copy link

jdrozdnovak commented Feb 25, 2023

Hello
This lambda function https://github.com/python-caldav/caldav/blob/master/caldav/davclient.py#L553 not works ideally when having simple headers like www-authenticate: Basic
I don't know enough about other caldav environments to do a change myself. I have only 2 available for test.

what the extract_auth_types returns in my case is ['basi'] which fails the future "basic" in auth_type condition

Thanks for reviewing.

@tobixen
Copy link
Member

tobixen commented Feb 25, 2023

Weird - I would have expected some of my test servers to fail, and people from the homeassistant community to be whining a lot if basic auth was broken.

This code section seems to be missing unit tests, so I'll try to write up some and possibly see if I can reproduce the problem.

@tobixen
Copy link
Member

tobixen commented Feb 25, 2023

I can reproduce it.

@tobixen
Copy link
Member

tobixen commented Feb 25, 2023

This bug has been there since 1.0 actually, it wasn't introduced in 1.1. I suppose almost all servers with basic auth have a realm in the header.

@tobixen
Copy link
Member

tobixen commented Feb 25, 2023

@jdrozdnovak ...

It would be nice if you could test and see if the master branch solves the problem for you - or, if it's possible to make me a testing account on your caldav server allowing me to test it myself it would be great.

Since this is a considered to be a critical bugfix, I will roll out a new minor-release soon - I should just run the test code first towards a handful of servers. Anyway, I'm pretty sure the fix does not break anything.

@jdrozdnovak
Copy link
Author

I can tomorrow test it with master and let you know.
I am using monicahq, where it fails.

docker run --name some-monica -d -p 8080:80 monica

@tobixen
Copy link
Member

tobixen commented Feb 25, 2023

Ok, thanks

@jdrozdnovak
Copy link
Author

I tested it with the code in master branch and that works fine.

@tobixen
Copy link
Member

tobixen commented Feb 26, 2023

I'm running tests and will release 1.1.4 during the day probably

@jdrozdnovak
Copy link
Author

thank you very much! Have great sunday!

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

Successfully merging a pull request may close this issue.

2 participants