jupyterhub / ldapauthenticator

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

`escape_userdn` doesn't seem right - is it? #266

Closed consideRatio closed 2 months ago

consideRatio commented 2 months ago

The escape_userdn config was introduced with a discussion in https://github.com/jupyterhub/ldapauthenticator/pull/32#pullrequestreview-26048588. For some it worked, for others it didn't.

I don't understand what went wrong for some and not for others then, but I think things aren't right now, and that the LDAP v3 specification clarifies whats right.

I understand it as whenever a DN is written in string form, the attribute values should be escaped according to https://datatracker.ietf.org/doc/html/rfc4514#section-2.4, for example using the escape_rdn function in the ldap3 package on all attribute values in a DN.

This means that whenever we create a DN, attribute values should be escaped. For example when we insert a username into a bind_dn_template string, the username should be escaped with escape_rdn. This aligns with ldap3 packages documented recommentation as well.

image

Currently though, we aren't using escape_rdn on attribute values in a DN, instead we are escaping the full DN using escape_filter_chars. This isn't something we should do for a dn in all situations, but we should if its used as a value inside a search filter - which some people may have done before.