jupyterhub / ldapauthenticator

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

Fix parsing of search response #294

Closed consideRatio closed 2 weeks ago

consideRatio commented 3 weeks ago

Fixes #292 and a bug that maybe never surfaced where things could work before 2.0.0 when the conn.response list had multiple elements thanks to being ordered in a way where the first element was the one of relevance, even though LDAP specification sais any ordering is allowed. Maybe the ldap3 client orders them, but who knows.

Also fixes #295

Investigation

References

manics commented 3 weeks ago

Assuming this works can you add a unit test for _filter_search_response using the responses in https://github.com/jupyterhub/ldapauthenticator/issues/292#issuecomment-2449576305 as test data?

consideRatio commented 3 weeks ago

@manics I'll make sure to include that if the function is retained, but I think maybe use of conn.entries can make it irrelevant. The documentation isn't so clear, so I look to investigate further.

consideRatio commented 2 weeks ago

@manics @franciscaestecker this is ready for review now. I think the bug resided in this projet's use of conn.response while we expected the data from conn.entries that doesn't include anything but the searchResEntry type of responses.

consideRatio commented 2 weeks ago

I iterated a bit on the log messages as well to make errors have some pointer on what may be configured wrong or similar. I'm hands off now!

consideRatio commented 2 weeks ago

Good point @manics, I've made the resolve_username function not draw conclusions on whats happening outside the function and instead focused on logging things in scope for the function to log based on its purpose only.

consideRatio commented 2 weeks ago

@manics I'll proceed with a release