Add high level LDAPObject.set_tls_options()#350
Add high level LDAPObject.set_tls_options()#350tiran wants to merge 1 commit intopython-ldap:mainfrom
Conversation
tiran
force-pushed
the
tlsoptions
branch
7 times, most recently
from
7039cbf to
2155f13
Compare
Codecov Report
@@ Coverage Diff @@
## master #350 +/- ##
==========================================
- Coverage 71.30% 71.04% -0.27%
==========================================
Files 50 50
Lines 4796 4855 +59
Branches 802 823 +21
==========================================
+ Hits 3420 3449 +29
- Misses 1045 1063 +18
- Partials 331 343 +12
Continue to review full report at Codecov.
|
tiran
force-pushed
the
tlsoptions
branch
2 times, most recently
from
aad4418 to
4e83107
Compare
| # just any directory | ||
| cacertdir=certdir, | ||
| require_cert=ldap.OPT_X_TLS_DEMAND, | ||
| protocol_min=0x303, |
There was a problem hiding this comment.
It is a very minor nitpick.
I understand that it is common knowledge but can we mention here in a comment that 0x303 is TLS 1.2? For friendliness and explicitness:)
| if "ldapi://" in self._uri: | ||
| raise ValueError("IPC (ldapi) does not support TLS.") | ||
| if self._ldap_call(self._l.tls_inplace): | ||
| raise ValueError("TLS connection already established") |
There was a problem hiding this comment.
If I understand correctly the exception types... I think this one will fit a bit better - EnvironmentError (instead of ValueError).
| self.assertEqual(conn.get_option(option), value) | ||
|
|
||
| @requires_tls() | ||
| def test_set_tls_options_ldap(self): |
There was a problem hiding this comment.
Looks like the coverage report has failed.
I think it's important to test all the parts before merging...
If you haven't started on the test suite expansion yet, I can take a look later and add a few tests so it will cover the rest of the code. :)
The new high level function ``set_tls_options`` deals with most common quirks and issues when setting TLS/SSL related options. Signed-off-by: Christian Heimes <cheimes@redhat.com>
|
Speaking of friendliness, the added docs use a lot of acronyms that are very hard to search for if you don't already know what they mean. |
3 participants
The new high level function
set_tls_optionsdeals with most commonquirks and issues when setting TLS/SSL related options.
Signed-off-by: Christian Heimes cheimes@redhat.com