Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ldap auth #423

Open
wants to merge 5 commits into
base: master
from

Conversation

@AugustH
Copy link
Contributor

commented Aug 29, 2019

Improved LDAP Authentication v2

  • extended config
  • allow multiple configs
  • documentation
AugustH added 4 commits Aug 10, 2019
supports advanced ldap login:
- auto_bind: seee https://ldap3.readthedocs.io/connection.html
- different sources fo bind_passwod
@GuillaumeDerval GuillaumeDerval requested a review from anthonygego Sep 8, 2019
# 'Log in' (bind) to the ldap connection
if conn.rebind(user_data['dn'], password=password):
try:
email = user_data['attributes'][mail]

This comment has been minimized.

Copy link
@anthonygego

anthonygego Sep 10, 2019

Member

Here we used to take the first item of the returned array. Are the attributes not returned as an array in your case ?

try:
email = user_data['attributes'][mail]
username = login
realname = user_data['attributes'][cn]

This comment has been minimized.

Copy link
@anthonygego

anthonygego Sep 10, 2019

Member

Same comment here.

This comment has been minimized.

Copy link
@AugustH

AugustH Sep 11, 2019

Author Contributor

i see - but i just rechecked against our ldap (windows server): both cn and userPrincipalName (which is used for mail) are strings, not lists. Sadly i can't check the original code as our ldap server needs TLS_BEFORE_BIND.

Two ways to resolve:

  • add [0], which works even in my case, gives bad realnames and mail
  • additional if isinstance check

Any chance that you/anybody checks the old and new code: are there really lists or is only the first character taken?

This comment has been minimized.

Copy link
@anthonygego

anthonygego Sep 17, 2019

Member

I'll check this soon. AFAIK both the LDAP directories we used returned lists. But there may be differences between openLDAP and Active Directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.