Skip to content

Reformatted specs and reworked way to propagate InvalidCredentials error to application #23

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

Merged
merged 6 commits into from
Jan 23, 2013

Conversation

DimaD
Copy link
Contributor

@DimaD DimaD commented Oct 20, 2012

I reformatted specs so they have unified style now and they are a bit easier to support.
Also I added check which enforces non-empty passwords which is already mentioned in tickets many times. For example #15, #18
Also instead of raising custom extension OmniAuth::Strategies:: LDAP::MissingCredentialsError I used method fail!(:missing_credentials) which is recommended way to propagate errors from OmniAuth to application. Otherwise non-Rails rack based app can not handle exceptions raised from middleware

Dmitriy Dzema added 6 commits September 27, 2012 17:31
When exception is raised from middleware host application can not
handle it. This is why OmniAuth uses #failure! which sends message to
application so it can do something about it
AD allows bind with with correct username and empty password
so application will think user successfully authenticated which
is a serious mistake
pyu10055 pushed a commit that referenced this pull request Jan 23, 2013
Reformatted specs and reworked way to propagate InvalidCredentials error to application
@pyu10055 pyu10055 merged commit ef89fe7 into omniauth:master Jan 23, 2013
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