Implemented connecting with an external TlsConnector.

inejge / ldap3

A pure-Rust LDAP library using the Tokio stack

Apache License 2.0

220 stars 38 forks source link

Implemented connecting with an external TlsConnector. #11

Closed golddranks closed 7 years ago

golddranks commented 7 years ago

This is a backwards-compatible change. I didn't change any public interfaces.

I added two methods: with_connector on LdapConnBuilder and connect_ssl_with_connector on Ldap. (The latter mirrors the already existing method method connect_ssl, which I couldn't change)

It's now possible to provide a TlsConnector of native-tls crate, that builds the connection. This allows using self-signed certificates, because you can use them providing the certifate with add_root_certificate method.

Fixes https://github.com/inejge/ldap3/issues/10

inejge commented 7 years ago

Thanks for the PR! I plan to merge it, with the following changes:

connect_ssl_with_connector() will not be publicly available in the Ldap struct, only through LdapConnBuilder;
with_connector() will be renamed to with_tls_connector();
with_tls_connector() will be #[doc(hidden)], to mark it as semi-private and liable to breaking change.

Before introducing with_tls_connector() as stable, I would like to see whether it's possible to have other frequently necessary cert-related misfeatures, such as expired certificates, available through the same mechanism, as well as possible interaction with StartTLS.

I'll try to merge the changes in the next two days, but I can't promise anything -- I'm quite busy for the foreseeable future.

golddranks commented 7 years ago

Thanks for the comment, I made the modifications you suggested. I agree that being conservative with API changes here is the way to go, at least for now.

Btw. I forgot to say – thanks for maintaining this crate! I think it's a blessing to be able to use Rust in my work, and it's largely due to the efforts of dozens of maintainers of various crates like this. Thank you!

inejge commented 7 years ago

Oh dear, I've already pulled your original PR locally, and did some additional refactoring (mostly hiding the connector behind a #cfg, since native-tls shouldn't be included in the build if the "tls" feature is not defined), so your new update is unmergeable. I'll close this PR, since the refactored original is now in master.

Please test the new master, and thanks again for the PR!