Skip to content

Conversation

sr-rossanderson
Copy link

@sr-rossanderson sr-rossanderson commented Jul 25, 2025

This PR fixes a bug in _resolve_username_dn() where attributes=[self.lookup_dn_user_dn_attribute] could result in [None] being passed to conn.search(), which raises a TypeError.

Before:

attributes=[self.lookup_dn_user_dn_attribute]  # if None → [None]

After:

attributes=[self.lookup_dn_user_dn_attribute] if self.lookup_dn_user_dn_attribute else ldap3.ALL_ATTRIBUTES

Fixes:

TypeError: argument of type 'NoneType' is not iterable when lookup_dn_user_dn_attribute is unset.

Copy link
Collaborator

@yuvipanda yuvipanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR, @sr-rossanderson!

It looks like resolve_username is only called if lookup_dn is true. I think the issue is that if lookup_dn is true, then lookup_dn_user_dn_attribute must be set. We currently don't validate this, causing this issue.

So there are two pathways here:

  1. When lookup_dn_user_dn_attribute is not specified, get all the attributes.
  2. Add a valiation step with validate to lookup_dn_user_dn_attribute, so it exits with a clear error message if it's not set but lookup_dn is set.

I would prefer we do (2) as that is a non-breaking change, and preserves current behavior while giving admins a well defined error message if they don't get it right. Do you think you'll be able to shift the implementation to do validation instead? It's fairly straightforward with traitlets, and you can see an example in ldapauthenticator here:

@validate("bind_dn_template")

Thank you so much for your contribution, it's highly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

2 participants