Skip to content

Commit

Permalink
Preliminary work on fixing search result handling
Browse files Browse the repository at this point in the history
  • Loading branch information
consideRatio committed Oct 31, 2024
1 parent 0af43b8 commit 2caa725
Showing 1 changed file with 29 additions and 1 deletion.
30 changes: 29 additions & 1 deletion ldapauthenticator/ldapauthenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,25 @@ def _observe_escape_userdn(self, change):
""",
)

def _filter_search_response(self, connection_response):
"""
A ldap3 search response is a list of dictionaries of various types,
where the data we currently handle is in dictionaries with
type=searchResEntry.
A response with type=searchResRef may indicate that we actually need to
do another search, like a paginated search, in order to get the full
response. For now though, we are just debug logging such entries and
discarding them if they are part of the response.
References:
- discussion: https://github.com/jupyterhub/ldapauthenticator/issues/292
- ldap3 docs: https://ldap3.readthedocs.io/en/latest/searches.html#response
- ldap3 code: https://github.com/cannatag/ldap3/blob/v2.9.1/ldap3/core/connection.py#L760
- LDAP docs: https://datatracker.ietf.org/doc/html/rfc4511#section-4.5.2
"""
return [e for e in connection_response if e["type"] == "searchResEntry"]

def resolve_username(self, username_supplied_by_user):
"""
Resolves a username (that could be used to construct a DN through a
Expand Down Expand Up @@ -455,7 +474,9 @@ def resolve_username(self, username_supplied_by_user):
search_filter=search_filter,
attributes=[self.lookup_dn_user_dn_attribute],
)
response = conn.response
# FIXME: maybe what we want is to look at conn.entries instead of
# conn.response...
response = self._filter_search_response(conn.response)
if len(response) == 0:
self.log.warning(
f"Failed to lookup a DN for username '{username_supplied_by_user}'"
Expand Down Expand Up @@ -547,6 +568,8 @@ def get_user_attributes(self, conn, userdn):
search_filter="(objectClass=*)",
attributes=self.auth_state_attributes,
)
# FIXME: Is this logic sound? Can we assume conn.entries[0]? This
# hasn't caused issues historically though, so maybe?
if found:
attrs = conn.entries[0].entry_attributes_as_dict
return attrs
Expand Down Expand Up @@ -632,6 +655,8 @@ async def authenticate(self, handler, data):
),
attributes=self.attributes,
)
# FIXME: conn.response likely incorrect to use here, maybe
# conn.entries works?
n_users = len(conn.response)
if n_users == 0:
self.log.warning(
Expand Down Expand Up @@ -667,6 +692,9 @@ async def authenticate(self, handler, data):
),
attributes=self.group_attributes,
)
# FIXME: is this logic sound? Is found correctly implying that
# we have a search result entry, and not just a reference
# to search elsewhere?
if found:
ldap_groups.append(group)
# Returned in auth_state, so fetch the full list
Expand Down

0 comments on commit 2caa725

Please sign in to comment.