connector/ldap: display login error#1530
Conversation
| if err != nil { | ||
| s.logger.Errorf("Failed to login user: %v", err) | ||
| s.renderError(w, http.StatusInternalServerError, "Login error.") | ||
| s.renderError(w, http.StatusInternalServerError, fmt.Sprintf("Login error: %v", err)) |
There was a problem hiding this comment.
Which errors could this be? Is there anything potentially sensible about them? I.e. we don't want let someone guess usernames this way... 💭
There was a problem hiding this comment.
I'm not sure, after checking the two backends LDAP and Keystone from Dex code point of view (all the PasswordConnectors) there is no error message which would reveal something like that. Of course, if the upstream provider/lib would return something like that (we wrap those errors) that could be dangerous, but in that case, there is a security concern in that software already.
Currently, Dex returns Internal Server Error - Login error. even in the case, the user mistyped the password or the LDAP/Keystone server is inaccessible.
srenatus
left a comment
There was a problem hiding this comment.
Thanks for looking into the potential errors, too. :)
|
Welcome :) |
connector/ldap: display login error
CallbackConnectorsdisplay the error message to the user, which is quite nice for debugging purposes however,PasswordConnectorshaven't returned it, this fixes that.