jaredhanson / passport-local

Username and password authentication strategy for Passport and Node.js.
https://www.passportjs.org/packages/passport-local/?utm_source=github&utm_medium=referral&utm_campaign=passport-local&utm_content=about
MIT License
2.73k stars 498 forks source link

revised strategy to allow the credentials to be located anywhere in t… #189

Closed tqwhite closed 3 years ago

tqwhite commented 3 years ago

The module's lookup() function actually accepts a dotted path as input. In the current implementation, this is applied to either the request body or query properties. It was useful to pass the credentials in the header but, since the lookup() function was applied specifically to those two properties, this strategy would not support that case.

The code was revised to, if the credentials were not found in the backward compatible case, try to get them as a reference to the req object itself, eg, lookup(req, 'headers.password');

jaredhanson commented 3 years ago

What is the rationale for passing username and password in non-standard header fields?

tqwhite commented 3 years ago

Backward compatibility for an API.

This allows credentials to be sent along with a request. The process flow will be unchanged except adding another exception category (401).

BTW, I was impressed with the brevity of your lookup() function. Quite nice.

tqii

TQ White II • 708-763-0100 Website • blog.genericwhite.com

On Nov 1, 2021, at 5:32 PM, Jared Hanson @.***> wrote:

What is the rationale for passing username and password in non-standard header fields?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jaredhanson/passport-local/pull/189#issuecomment-956767692, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAG6MGSGBNISUY6QNVCANLUJ4IQXANCNFSM5HFBSFNA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jaredhanson commented 3 years ago

This strategy is aimed at supporting form-based authentication, where the username and password are submitted in an HTML form. Thus the reason why the strategy searches req.body (for <form action="post">) and req.query (for form action="get"). Given that, there's little reason for searching for credentials outside of those locations.

For header-based authentication using username and password, I would strongly encourage use of HTTP Basic, which is implemented by passport-http. If you have an existing API using non-standard headers, I'd recommend creating a separate strategy which implements that mechanism.

I don't have a strong desire to support non-standard or uncommon ways of encoding credentials in this strategy, and this doesn't seem like a good fit for this strategy in particular.

Thoughts or concerns?

tqwhite commented 3 years ago

passport-http is infinitely more complicated than I need. I chose passport-local because it is straightforward, understandable and clean.

My programming goal is maximum flexibility and minimum complexity. My pull request does that. It fully supports the use cases you like but does not force that decision on the user.

For the price of two if statements, you open it to, basically, any other use. If a person can get the credentials into the request object, they can use your module. I don't know any reason for us to care if they are standard or not.

When I am writing code for others to use, I just want to help. I think that this change will help more people. Would have helped me.

tqii

TQ White II • 708-763-0100 Website • blog.genericwhite.com

On Nov 1, 2021, at 5:50 PM, Jared Hanson @.***> wrote:

This strategy is aimed at supporting form-based authentication, where the username and password are submitted in an HTML form. Thus the reason why the strategy searches req.body (for

) and req.query (for form action="get"). Given that, there's little reason for searching for credentials outside of those locations.

For header-based authentication using username and password, I would strongly encourage use of HTTP Basic, which is implemented by passport-http https://github.com/jaredhanson/passport-http. If you have an existing API using non-standard headers, I'd recommend creating a separate strategy which implements that mechanism.

I don't have a strong desire to support non-standard or uncommon ways of encoding credentials in this strategy, and this doesn't seem like a good fit for this strategy in particular.

Thoughts or concerns?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jaredhanson/passport-local/pull/189#issuecomment-956801536, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAG6MCASWX5PTCKJ5KHVY3UJ4KT5ANCNFSM5HFBSFNA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

tqwhite commented 3 years ago

I take it back, though. The lookup() routine doesn't always work.

Never mind.