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

Remove early auth error when no user or pass #114

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BanovMiroslav
Copy link

When not adding a user and pass, there is a specific error being sent. But this case is already handled by the verify callback, and it is easier and more natural to send a prettier error response to the end-user if there is no special handling of the case where user and password are not provided.

@Fonger
Copy link

Fonger commented Apr 7, 2016

@jaredhanson

+1 for this. This is obviously a bug!
jaredhanson/passport#466

However I think you should simply use this.error instead of removing it in this line:
https://github.com/jaredhanson/passport-local/blob/master/lib/strategy.js#L75

@vuaru
Copy link

vuaru commented Apr 7, 2016

@Fonger
Then you're assuming that not having a username or password is an error, which might not always be the case. Determining this is the job of the verify callback, imo.

@rwky
Copy link

rwky commented Jul 7, 2018

@BanovMiroslav if you make a PR against https://github.com/passport-next/passport-local this will get looked into.

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.

None yet

4 participants