-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Added an error message bag for connection issues with LDAP #17955
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
base: develop
Are you sure you want to change the base?
Added an error message bag for connection issues with LDAP #17955
Conversation
|
@uberbrady I made some changes around the |
|
@uberbrady gentle poke, do you have any thoughts or issues with this? |
|
I think I would be okay with this if it, say, showed an error (only to a superuser) about how it needed to fail through LDAP to do local login, I think something like that would be just fine - and useful. But showing ldap errors or codes to unauthenticated users could disclose to an attacker that LDAP was being employed - and might even disclose the IP address in an error message - something like "Could not connect to 1.2.3.4 port 636" - that could be very dangerous. Also, the LDAP system has been pretty heavily re-worked recently, so this PR is pretty heavily conflicted. If you want to try and get it back into a mergable state, I will definitely take another look at it, but I'm still generally pretty nervous about this approach for security reasons. |
This adds error handling around LDAP login. if it times out, it does so with an amorphous response: