diff --git a/ldapauthenticator/ldapauthenticator.py b/ldapauthenticator/ldapauthenticator.py index 3d26b4d..bf115b4 100644 --- a/ldapauthenticator/ldapauthenticator.py +++ b/ldapauthenticator/ldapauthenticator.py @@ -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 @@ -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}'" @@ -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 @@ -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( @@ -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