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

Add explicit notes around password-less simple auth #292

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

Conversation

captainpete
Copy link

Hi there,

The simple auth bind example does not show how unauthenticated bind success responses can happen. Many A/D servers are configured to have successful bind when no password is supplied if the username is correct. RFC4513 5.1.2 recommends only assuming authenticated bind when password sent was present (and to improve the A/D server configuration).

At the risk of complicating the example I've created this pull request, please let me know how I can improve it!

Thanks,
Pete

@@ -823,7 +829,11 @@ def search(args = {})
# ldap.port = 389
# ldap.auth your_user_name, your_user_password
# if ldap.bind
# # authentication succeeded
# if your_user_password.size > 0
Copy link
Author

Choose a reason for hiding this comment

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

I think this complicates the example, should auth, authenticate or bind check for presence of the password internally when the authentication method is simple?.

@aurcioli-handy
Copy link

FWIW This to me is a major security oversight of the library and a big gotcha! I just stumbled upon this after realizing a blank password was accepted for an admin login on one of our internal services utilizing this library.

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.

2 participants