search

python-caldav / caldav

Apache License 2.0
317 stars 94 forks source link

Bearer authentication support #257

Closed azmeuk closed 1 year ago

azmeuk commented 1 year ago

This allows bearer token authentication when the caldav server announce supporting it in its www-authenticate header and a password is given. This fixes #119. In the end I did not added a token argument as I suggested there, and just reused password to store the token.

This is just a draft to check with you that I take the right direction. What do you think?

tobixen commented 1 year ago

Looks good. While you're at it, I think it makes sense to change the ordering of the authentication methods - as several authentication methods can be offered by the server, it makes sense to choose the most secure first.

One "complaint" that I've mentioned several times before, I strongly believe that this logic belongs in the requests library and not in the caldav library. In my opinion, it should merely be needed to pass username and password to the requests library and then it should fix the proper authentication methods. Having a new look at the requests web page, the first thing I see is exactly this - username and password passed to the requests library without specifying the authentication method. Perhaps it's a new feature?

azmeuk commented 1 year ago

While you're at it, I think it makes sense to change the ordering of the authentication methods - as several authentication methods can be offered by the server, it makes sense to choose the most secure first.

Do you mean Bearer then Digest then Basic?

I strongly believe that this logic belongs in the requests library and not in the caldav library. In my opinion, it should merely be needed to pass username and password to the requests library and then it should fix the proper authentication methods.

Indeed you could remove the username and password arguments, and the guess which authentication method to use part, and let users customize their authentication method by setting auth themselves. That sounds good to me too, and indeed let the library focus on the DAV part.

Having a new look at the requests web page, the first thing I see is exactly this - username and password passed to the requests library without specifying the authentication method. Perhaps it's a new feature?

The requests doc seems to indicate that this is Basic by default.

tobixen commented 1 year ago

Do you mean Bearer then Digest then Basic?

Probably :-) I don't know what is best of bearer and digest, but I believe basic is deprecated.

Having a new look at the requests web page, the first thing I see is exactly this - username and password passed to the requests library without specifying the authentication method. Perhaps it's a new feature?

The requests doc seems to indicate that this is Basic by default.

Ok ... and I also don't want to change too much on the auth code right before releasing 1.0. I've already had a lot of problems working around bugs in the https-supposrt on misc cladav servers :-)

Just change the ordering (since you're already working on this part of the code) and I'll approve it and run the tests towards my list of caldav servers and release 1.0.