jupyterhub / ldapauthenticator

LDAP Authenticator Plugin for Jupyter
BSD 3-Clause "New" or "Revised" License
206 stars 178 forks source link

Review user attributes fetched for auth_state - could end up not being the users without an error being raised? #295

Closed consideRatio closed 2 weeks ago

consideRatio commented 2 weeks ago

Extracted from https://github.com/jupyterhub/ldapauthenticator/pull/294#discussion_r1825748693

Regarding this code:

    def get_user_attributes(self, conn, userdn):
        attrs = {}
        if self.auth_state_attributes:
            found = conn.search(
                search_base=userdn,
                search_scope=ldap3.SUBTREE,
                search_filter="(objectClass=*)",
                attributes=self.auth_state_attributes,
            )
            # FIXME: Handle situations with multiple entries below or comment
            #        why its not important to do.
            #
            if found:
                attrs = conn.entries[0].entry_attributes_as_dict
        return attrs

@manics wrote:

Maybe we should throw an error, same as in resolve_username? If there's a possibility of the entries corresponding to different Identities this implies a change in the LDAP server could lead to a different ordering of responses, resulting in a user gaining access to another user's account.

If it's two entries for the same user we still need to understand what the difference is, in case some attributes are different which could lead to inconsistent configuration of the singleuser server.