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

Providing custom message when failed to extract token #84

Closed
rkt2spc opened this issue Nov 12, 2016 · 11 comments
Closed

Providing custom message when failed to extract token #84

rkt2spc opened this issue Nov 12, 2016 · 11 comments

Comments

@rkt2spc
Copy link

rkt2spc commented Nov 12, 2016

Are there any support for this ?

passport.use('jwt', new JwtStrategy({
        secretOrKey: authConfig.secret,
        jwtFromRequest: ExtractJwt.fromAuthHeader(), //If return null, finishes with empty message
        algorithms: authConfig.algorithms
    },
    function(jwt_payload, done) {
        //Never had the chance to call done with custom message
        Account.findById(jwt_payload.userId, function(err, account) {
            if (err) return done(err);
            if (!account) return done(null, false, 'invalid Token');
            return done(null, account, 'valid Token');
        });
    })
);
@mikenicholson
Copy link
Owner

I think you're looking for the Custom Callback functionality of the Passport module.

@rkt2spc
Copy link
Author

rkt2spc commented Nov 12, 2016

Sure, I can go and check if the authorization header exists before triggering the passport middleware. But why don't we put it into the strategy extraction method itself? Something like:
jwtFromRequest: ExtractJwt.fromAuthHeader({failmessage: 'missing token'})

@mikenicholson
Copy link
Owner

This is not the point of the extractor function and a violation of single responsibility pattern. The extractor function is only responsible for extracting the JWT from the request if it exists.

There is no need to check the request for the authorization header before triggering passport. The decision of what to do if the JWT is missing takes places in the strategy's authenticate() method.. Right now it returns a generic error which you can then catch and handle in the custom callback register via passports Custom Callback functionality.

That is where I recommend providing the custom error response.

The potential improvements I see in this area are:

  1. Return a custom error class to make it easier to identify when the JWT was missing.
  2. Do something like passport-local and provide a custom error message via the Strategy constructors generic options argument.

@mikenicholson
Copy link
Owner

Let me know if this covers your use case. If you have a suggestion I am open to pull requests or we can create a feature request to address the need.

Thanks for using the module and I appreciate any feedback!.

@mikenicholson
Copy link
Owner

Haven't heard anything in a few days. Closing for now.

@GeekEdem
Copy link

Hello. Cannot understand how to set custom error message for different errors and how can I embed JWT black list to this strategy? Thanks for answer

@mikenicholson
Copy link
Owner

@GeekEdem Sorry to sound like a broken record but handling custom error messages is explained here: http://passportjs.org/docs#custom-callback

and if you want to blacklist JWTs you can put that logic into the verify callback when constructing the strategy

passport.use(new JwtStrategy(opts, function(jwt_payload, done) {
       if ( is_jwt_blacklisted(jwt_payload) ) {
            return done(err, false);
        }
        else  { 
            // jWT is not blacklisted, look up a user or
           // whatever else you were planning on doing with a
          // valid JWT
        } 
    });

note that is_jwt_blacklisted is just a funciton I made up. You'll have to supply that function as the strategy doesn't maintain a blackisted jwt list on its own.

@GeekEdem
Copy link

GeekEdem commented Nov 21, 2016

@themikenicholson Here is my solution, if someone need)

function requireAuth (req, res, next){
    passport.authenticate('jwt', jwtSession, function (error, decryptToken, jwtError) {
        if(typeof (jwtError) === 'object'){
            return general.response(res, {
                field: 'Authorization',
                location: 'header',
                messages: [
                    jwtError.message
                ]
            });
        } else if (!error) {
            let token = req.header('Authorization').slice(4);
            TokenModel.findOne({token: token}).lean().exec( (err, result) => {
                if(!err && !result) {
                    req.user = decryptToken;
                    return next();
                } else if (!err && result) general.response(res, {
                    field: 'Authorization',
                    location: 'header',
                    messages: [
                        'token is in black list'
                    ]
                });
                else general.response(res, err);
            });
        }
    })(req, res, next);
}

this method I use as middleware in express.
general.response - this is method, that make response message and return them.
TokenModel - this is mongoose model, to check is token in blacklist collection in MongoDB.

@rkt2spc
Copy link
Author

rkt2spc commented Jan 2, 2017

@themikenicholson
Following the passport custom-callback

router.get('/jwt', (req, res, next) => {

    passport.authenticate('jwt', (err, user, info) => {

        if (err) return next(err); // It is null
        if (!user) return res.status(403).json(info);
        res.status(200).json(user);

    })(req, res, next);
});

If token extraction failed, the custom error is placed in the "info" parameter. Is this by design? Shouldn't it be at the "err" parameter?

Update: I will continue with Issue #75

@azharuniverse
Copy link

You can check for Error string in info to catch and provide custom messages.

passport.authenticate('jwt', (error, user, info) => {
    sails.log.info('isAuthenticated policy: ', error, user, info.name);

    if (info.name === 'TokenExpiredError') info.status = 401;
    if (info.name === 'JsonWebTokenError') info.status = 401;
    if (info.name === 'Error') info.status = 401;
    if (error || !user) return res.negotiate(error || info);

    req.user = user;
    next();

  })(req, res);

@mrtnzagustin
Copy link

Manteiners should add this "info" param and examples to the http://www.passportjs.org/packages/passport-jwt/ docs. I could't find the error until i read this issue. Thanks

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

No branches or pull requests

5 participants