passport-jwt
passport-jwt copied to clipboard
mikenicholson
Error: No auth token - is not set as Error
If there is no Authentication header, the passport.authenticate()-callback gets set the info parameter to Error: No auth token. But the 1st error parameter is undef.
Shouldn't the error parameter be an error if an error occurs?
Handling missing token with fail() rather than error() is consistent with the way passport-local works. I assume since this is written by the author of passport it's the canonical example.
See the documentation for Strategy.error() and Strategy.fail().
fail() is meant for a failed authentication attempt - I would consider expired credentials or missing credentials to be a failure to authenticate.
error() is meant for application errors like a failure to connect to the database.
Based on my interpretation of the docs and other strategies I disagree with this request and don't currently expect to merge the corresponding pull request.
I'm open to discussion on this if you can provide documentation or examples suggesting I'm misreading the passportjs documentation.
In that case, can a callback to passed to _jwtFromRequest() that acts similar to passport's done method? In particular, can the callback's form be (error, token, info)? That way, we can use fail() correctly to indicated a failed authentication attempt while also allowing _jwtFromRequest() to include more information in the info. I'm trying to give my users better information on why a jwt token could not be extracted from the request.
The work around has been to throw something in my jwtFromRequest(), but is less elegant.
edit: spelling
@jerroydmoore I really like the idea of switching to an extractor function that accepts a done callback rather than simply returning the JWT.
I’ll probably have to wait until I get some time around the holidays to get around this but PR’s are welcome if they maintain the existing level of test coverage.
@mikenicholson
See the documentation for Strategy.error() and Strategy.fail().
That documenation states that fail() should be called with arguments of types String, Number, and error() with argument of type Error. You, however, pass an Error object to fail().
Perhaps, instead of
return self.fail(new Error("No auth token"));
you should be doing
return self.fail("No auth token", 401);
?
