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

[Feature Request] Allow looking up the userdn rather than having to specify a template #88

Closed
wants to merge 2 commits into from

Conversation

ermakovpetr
Copy link

@ermakovpetr ermakovpetr commented Jun 22, 2018

Without bind_dn_template, any password is authenticated successfully! (after update from 1.1 to 1.2.2)
#44
If don't set bind_dn_template - bind_dn_template=[]
and isBound don't set True
https://github.com/jupyterhub/ldapauthenticator/blob/master/ldapauthenticator/ldapauthenticator.py#L315-L335

        for dn in bind_dn_template:
            userdn = dn.format(username=resolved_username)
            msg = 'Status of user bind {username} with {userdn} : {isBound}'
            try:
                conn = getConnection(userdn, username, password)
            except ldap3.core.exceptions.LDAPBindError as exc:
                isBound = False
                msg += '\n{exc_type}: {exc_msg}'.format(
                    exc_type=exc.__class__.__name__,
                    exc_msg=exc.args[0] if exc.args else ''
                )
            else:
                isBound = conn.bind()
            msg = msg.format(
                username=username,
                userdn=userdn,
                isBound=isBound
            )
            self.log.debug(msg)
            if isBound:
                break

But we can take userdn from connection after search user conn.response[0]['dn']
this is my temporarily dirty fix
If you ok about the idea, I try to make pull-request correctly and beautifully

@dhirschfeld
Copy link
Collaborator

Hi @ermakovpetr! Thanks for looking into this, but I can't reproduce and the logic looks fine to me as mentioned in #44.

To be clear, what I'm seeing is that if you don't specify bind_dn_template in your config then it will always fail authentication with the below error message:
image
...which is the intended behaviour.

If you can provide further information to enable me to reproduce issue #44 I'm happy to take a look...

@ermakovpetr
Copy link
Author

ermakovpetr commented Jul 19, 2018

@dhirschfeld

what I'm seeing is that if you don't specify bind_dn_template in your config then it will always fail authentication with the below error message

Yes!

But why is this intended behavior?

  1. in my opinion correctly to issue the corresponding error (if not in the interface, then at least in the logs)
  2. This information (userdn) can be taken from LDAP already after the first request (as I did in the pull-requester) and it works.

@dhirschfeld
Copy link
Collaborator

Looking up the userdn is a separate issue from that in #44 so I'll change this issue title to more clearly reflect that

@dhirschfeld dhirschfeld changed the title dirty fix (temporarily) [Feature Request] Allow looking up the userdn rather than having to specify a template Jul 19, 2018
@ermakovpetr
Copy link
Author

OK. I inherit from #95 and try to make pull-request correctly and beautifully

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

Successfully merging this pull request may close these issues.

2 participants