node-oauth2-server icon indicating copy to clipboard operation
node-oauth2-server copied to clipboard

Faulty login throws server error

Open Dronar opened this issue 6 years ago • 3 comments

Hi! I'm implementing an authorization code grant flow in my app. During this process I learned that I need to implement my own authentication handler which I've done. The documentation says that this function should return a falsy value if no user could be found. Which I interpret as no user matching the provided username and password. I'm hooking into my model's getUser which fulfills this requirement.

This is where the problem occurs. This code throws a server error whenever no user is supplied. Which would be anytime someone types the wrong username or password. https://github.com/oauthjs/node-oauth2-server/blob/e1f741fdad191ee47e7764b80a8403c1ea2804d4/lib/handlers/authorize-handler.js#L241

Is this as intended? If so, what is the suggested solution? I can't really assume all server errors are faulty logins :)

Dronar avatar Jan 15 '18 22:01 Dronar

What does your authentication handler look like? How are you invoking it as part of your authorization flow?

mjsalinger avatar Feb 13 '18 12:02 mjsalinger

I guess this comes down to how the instructions are read.

If there is no associated user (i.e. the user is not logged in) a falsy value should be returned.

I assumed passing a falsy value would return something useful or put the flow into a state which I would have to handle by sending a 400 or sending the user back to the login page. I did not expect to the thrown a ServerError that I can't really use. I shouldn't be requested to send a falsy value when it doesn't help me.

I rewrote my code following some examples I found. In this new flow I would never send a falsy value (jinx!) making this problem rather small.

Dronar avatar Feb 18 '18 22:02 Dronar

@Dronar You don't have to apologise as it wasn't the wrong assumption. The docs here are clearly rubbish as my understanding of it was exactly the same as yours and I wasted hours trying to make sense of it.

The handle function should be the place where you either return a user object or null, as there is no other Model function that validates a retrieved user. Had there been such a Model function (validateUser() say) that is called before the call to saveAuthorizationCode(), it would make sense to always return some object which could then be validated within validateUser().

However handle does not like null being returned and there is no such Model function, leaving us to write logic within POST:/authorize itself to validate and redirect with error upon failure - @Dronar I'm guess you did something like this? Kind of defeats the purpose of such libraries, which exist to abstract away details of the OAuth flow.

Also, no further responses from the contributors since Feb 2018!?

FunkyLambda avatar Jun 26 '21 02:06 FunkyLambda