jaredhanson / passport-http

HTTP Basic and Digest authentication strategies for Passport and Node.js.
https://www.passportjs.org/packages/passport-http/?utm_source=github&utm_medium=referral&utm_campaign=passport-http&utm_content=about
MIT License
268 stars 110 forks source link

Added passwordRequired to options, defaults to true #31

Open jackdent opened 10 years ago

paulmelnikow commented 9 years ago

@jaredhanson Can this be merged? This would be helpful for my use case as well.

jaredhanson commented 9 years ago

Is this any different than doing: passport.authenticate(['basic', 'anonymous'])?

paulmelnikow commented 9 years ago

Yeah. These requests aren't anonymous. They're authenticated using a single API key, instead of a username + password combination. The client sends that key as the username, and a blank password.

jackdent commented 9 years ago

@jaredhanson For example, the Stripe API uses the username as the authentication key, and leaves the password blank

jaredhanson commented 9 years ago

Wouldn't that be the point of passport-http-bearer?

paulmelnikow commented 9 years ago

Sure, seems like that would be one way to design the API. But basic auth is widely implemented in clients. The spec for http basic doesn't seem to require that the password is non-zero length, so using it in this manner seems reasonable (as well as common).

jackdent commented 9 years ago

+1

jaredhanson commented 9 years ago

Cool. Can't we just always accept empty passwords and pass them back to the verify callback then? Doesn't seem like we need the option to the constructor.

jackdent commented 9 years ago

I had backwards compatibility in mind, since some people might be relying on the failure mode for empty passwords. I don't think the change adds too much complexity, but you're right: it's nicer to just accept blank strings.

paulmelnikow commented 9 years ago

Wanted to bump this up. @jaredhanson, what do you think?

ianstormtaylor commented 8 years ago

+1 Also need this to implement a Stripe-like Basic Auth convenience. @jaredhanson can we merge this?

Backwards compatibility and the ability to opt-out of having to spend the resources to lookup a user and verify their hashed password might be good reasons to have the optional flag.

itaysabato commented 7 years ago

There are at least two issues and two pull requests in total on this exact issue that have been outstanding for over two years now (See #28 #30 #37). The fix seems pretty straight forward which ever way you go.

It makes me wonder: is this library actually maintained?

No offense.

ianstormtaylor commented 7 years ago

@jaredhanson ?...

noahajohn commented 7 years ago

I also echo the above comments. I think allowing for the password to be optional, which would allow us to mimic a Stripe-style API, would be very helpful.

alexchuvand commented 7 years ago

When is that going to be implemented ? It doesn't seem to be in the version 0.3.0