-
Notifications
You must be signed in to change notification settings - Fork 53
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
avoid duplicating ldaptor code in LDAPServerWithUPNBind #161
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #161 +/- ##
==========================================
- Coverage 93.35% 93.35% -0.01%
==========================================
Files 72 72
Lines 9915 9914 -1
Branches 973 972 -1
==========================================
- Hits 9256 9255 -1
Misses 537 537
Partials 122 122
Continue to review full report at Codecov.
|
Thanks for the cleanup. |
@adiroiban I think it's ready to go |
def _request(): | ||
if not (b"@" in request.dn and b"," not in request.dn): | ||
defer.returnValue(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this
def _request(): | |
if not (b"@" in request.dn and b"," not in request.dn): | |
defer.returnValue(request) | |
def _resolveUPNBindDN(): | |
""" | |
See if we have an MS UPN style BIND DN request and resolve it to the | |
actual LDAP BindDN. | |
UPN bind is in the form of User.Name@ad.example.tld | |
""" | |
if b"@" not in request.dn or b"," in request.dn: | |
# This is not an UPN bind request. | |
defer.returnValue(request) |
def _request(): | ||
if not (b"@" in request.dn and b"," not in request.dn): | ||
defer.returnValue(request) | ||
root = interfaces.IConnectedLDAPEntry(self.factory) | ||
# This might be an UPN request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# This might be an UPN request. | |
# The the LDAP DN associated with the UPN request. |
if len(results) != 1: | ||
defer.returnValue(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if len(results) != 1: | |
defer.returnValue(request) | |
if len(results) != 1: | |
# We could not find an exact UPN match so we go with the | |
# requested Bind DN. | |
defer.returnValue(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the cleanup.
Please see my comments.
Even thou I wrote this code a few months ago, I feel that it needs more documentation.
With the current version, at the first read it was hard to understand what is going on there and why we have all that logic.
Contributor Checklist:
docs/source/NEWS.rst
CONTRIBUTING.rst
.