Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

500 Error on blank LDAP login #1077

Closed
c1fe opened this Issue Jul 11, 2012 · 2 comments

Comments

Projects
None yet
2 participants

c1fe commented Jul 11, 2012

If no credentials are entered when logging in using LDAP, the 500 Error page is shown which isn't terribly descriptive or useful for debugging.

Contributor

patthoyts commented Jul 16, 2012

This causes the omniauth ldap code to raise a OmniAuth::Strategies::LDAP::MissingCredentialsError. In theory we should be able to trap this with (I think) a rescue_from somewhere but adding this to the application controller or the omniauth controller code doesn't seem to do anything. From some articles it seems this ldap strategy should be using fail! to throw this error and that raising an exception like this is passing it into Rack where we cannot handle it. (See http://stackoverflow.com/a/11362892/291641 ). If so, then the best strategy would likely be to add something to the login form to only enable the Submit button once both the username and password fields are non-empty.

Contributor

patthoyts commented Jul 20, 2012

This issue can be resolved by two fixes - one to the omniauth-ldap gem (gitlabhq/omniauth-ldap#1) to convert this exception into a omniauth "fail!" which can be trapped in our omniauth_controller failure method. The second is to test for and use an exception's message function in the failure handler.

@patthoyts patthoyts added a commit to patthoyts/gitlabhq that referenced this issue Jul 21, 2012

@patthoyts patthoyts Handle LDAP missing credentials error with a flash message.
If a user fails to provide a username or password to the LDAP login
form then a 500 error is returned due to an exception being raised
in omniauth-ldap. This gem has been amended to use the omniauth
error propagation function (fail!) to pass this exception message to
the registered omniauth failure handler so that the Rails application
can handle it approriately.

The failure function now knows about standard exceptions and no longer
requires a specific check for the OmniAuth::Error exception added by
commit f322975.

This resolves issue #1077.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
a2d244e

@c1fe c1fe closed this Jul 21, 2012

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