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

Enable LDAP module #1204

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enable LDAP module #1204

wants to merge 1 commit into from

Conversation

damluji
Copy link

@damluji damluji commented Nov 26, 2018

Enable LDAP(S) and install prerequisites.
fix logging,
try to improve authentication in a better way for use cases that cannot authenticate with a '@' symbol.

search_conn = self._ldap_search_conn
else:
search_conn = self._ldap_conn

self._logger.debug('username %s formatted _ldap_search username %s' % (self.username, self._ldap_search.format(ldap.dn.escape_dn_chars(self.username))))

Choose a reason for hiding this comment

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

line too long (164 > 79 characters)

@@ -387,11 +395,13 @@ def authenticate(self):
try:
# Create separate connection for search?
if self._ldap_search_conn is not None:
self._ldap_search_conn.simple_bind(self._ldap_search_binddn, self._ldap_search_password)
self._ldap_search_conn.simple_bind_s(self._ldap_search_binddn, self._ldap_search_password)

Choose a reason for hiding this comment

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

line too long (106 > 79 characters)

@@ -368,13 +373,16 @@ def authenticate(self):
return self._authenticated

try:
self._logger.debug('username %s formatted _ldap_binddn_fmt username %s' % (self.username, self._ldap_binddn_fmt.format(ldap.dn.escape_dn_chars(self.username))))

Choose a reason for hiding this comment

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

line too long (172 > 79 characters)

@garberg
Copy link
Member

garberg commented Mar 12, 2019

Thanks for contributing!

To understand the proposed changes, what problem did you want to address by adding the secondary_backend? Wouldn't you achieve the same thing by simply making ldap1 the default backend (which would be used for usernames without "@" in them), and using the username "localadmin@local" when you want to authenticate using the local auth backend? Hard-coding usernames in the auth library is something I'd really like to avoid.

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

Successfully merging this pull request may close these issues.

3 participants