Initial support for Radicale 3 #13
Conversation
|
This is still a bit of a work in progress and I would like to first receive some feedback. |
|
You should be able to do "python test/integration_tests.py" judging by the code. |
|
For the rest: python -m unittest test/basic_tests.py |
| This is an authentication plugin for Radicale 2. It adds an LDAP authentication backend which can be used for authenticating users against an LDAP server. | ||
| This is an authentication plugin for Radicale 3. It adds an LDAP authentication backend which can be used for authenticating users against an LDAP server. | ||
|
|
||
| Use the `2-final` git tag for Radicale 2 support. |
There was a problem hiding this comment.
Maybe we should reference the 2 branch as there has been an unexpected additional contribution to the Radicale 2 plugin since the creation of the 2-final tag :) Maybe there are more to come
| conn = ldap3.Connection(server=server, user=self.ldap_binddn, | ||
| password=self.ldap_password, check_names=True, | ||
| lazy=False, raise_exceptions=False) | ||
| conn.open() |
There was a problem hiding this comment.
Is this actually needed? The old code didn't open the connection and worked just fine. I just had a quick look at ldap3's connection.py and it seems like open is called automatically whenever the connection isn't open when it should be. I don't know a lot about ldap and ldap3 though, so I could be wrong! Could you check whether we need this?
| conn = ldap3.Connection(server=server, user=final_user_dn, | ||
| password=password, check_names=True, | ||
| lazy=False, raise_exceptions=False) | ||
| conn.open() |
| logger.error(conn.result) | ||
| return "" | ||
|
|
||
| final_search_filter = self.ldap_filter.replace("%username", login) |
There was a problem hiding this comment.
As I understand it, login is user-specified, right (I don't know a lot about radicale 3's plugin interface either 😆 )? In case my assumption is right, this looks very dangerous. I'm not so sure about escaping ldap-queries but I'm almost sure submitting user-controlled content directly inside the query string is a bad idea. Could you have a look at #4 ? According to ldap3's documentation it seems like escape_filter_chars() from ldap3.utils.conv would be the right tool for this.
| version="0.1", | ||
| description="LDAP Authentication Plugin for Radicale 2", | ||
| version="0.2", | ||
| description="LDAP Authentication Plugin for Radicale 3", | ||
| author="Raoul Thill", |
There was a problem hiding this comment.
In case you wrote this all by yourself feel free to take the credits ;-)
|
Regarding the tests, I'm afraid they were never really useful anyway. You would probably either have to rewrite them to support the new API (which shouldn't be a lot of work) or remove them altogether. The only useful test is the integration test and this needs a running ldap server and valid credentials to work (see |
|
Ooops, anonymous bind is not supported when using this. |
|
I will add that then. Sorry I had no time yet to further work on this. I hope this coming week works. |
Fixes #11