mikenicholson / passport-jwt

Passport authentication using JSON Web Tokens
MIT License
1.96k stars 213 forks source link

how to customize http response if failed authentication? #157

Closed zhaoyi0113 closed 5 years ago

zhaoyi0113 commented 6 years ago

I have below code to use passport-jwt to verify user authentication. But it returns 500 internal error to the client if it failed authentication. How can I customize the error response with a different http status code with a different message?

router.post(
            "/user",
            passport.authenticate("admin", { session: false }),
            (req, res) => {
      ...

below is my passport setup:

const adminStrategy = new JwtStrategy(jwtOptions, (jwtPayload, next) => {
        l.info("admin payload received", jwtPayload);
        userDb.hgetall(jwtPayload.id, (err, obj) => {
            if (!err && obj) {
                if (obj.role !== "admin") {
                    next(new Error("not admin user"), false);
                } else {
                    next(null, jwtPayload);
                }
            } else {
                next(null, false);
            }
        });
    });
passport.use("admin", adminStrategy);
joshdenz commented 6 years ago

Check out the passport docs on Custom Callbacks.

http://www.passportjs.org/docs/authenticate

You will be able to pass a custom callback that has access to req and res and you can customize your response from there.

Sorry Im on mobile or I would try to whip up an example. There is an example on the page I linked though.

kittrCZ commented 6 years ago

@joshdenz isn't there a more generic example? The Custom Callback section shows only example for one endpoint. I have currently around 25 endpoints from which I would like to return a custom error message. Is there a better solution?

I have the generic nodejs error handler:

app.use((err, req, res, next) => {
    // set locals, only providing error in development
    res.locals.message = err.message;
    res.locals.error = req.app.get('env') === 'development' ? err : {};
    if (req.app.get('env') === 'development') { console.log(err); }
    const response = {};
    response.alerts = errors.customMessage500(err.message);
    return res.status(err.status || 500).json(response);
});

But I don't think it ever gets trigger when there is passportjs middleware.

Ideas?

joshdenz commented 6 years ago

@kittrCZ No error will be passed to your express error handler if it the "error" is just an authentication failure as the express error handler is only concerned with a routing error.

Could you not use passport.authenticate in your middleware stack, and then use req.url or something to determine which error message to send? Then you could have an object with error messages keyed to specific routes and apply them inside your custom callback.

Something like

router.use('/', (req, res, next) => {
passport.authenticate('jwt', {session: false}, (err, user, info) => {
    // Handle your authentication here.
    })(req, res, next);
})

Just an idea, would something like that work for your use case?

Edit: Worth noting the following from the passport.js source regarding Custom Callbacks https://github.com/jaredhanson/passport/blob/master/lib/middleware/authenticate.js:


 * Note that if a callback is supplied, it becomes the application's
 * responsibility to log-in the user, establish a session, and otherwise perform
 * the desired operations.
mikenicholson commented 5 years ago

Closing as @joshdenz answered. Custom callbacks are the recommended approach.

adamreisnz commented 5 years ago

Because it's annoying that you have to define your callback dynamically (as it's not passed req/res/next), we are using a helper to generate callback functions, which is passed the request control variables.

E.g.

/**
 * Passport authentication callback generator
 */
module.exports = function passportCb(req, res, next) {
  return function(error, user) {

    //Wrap errors in not authenticated error
    if (error) {
      return next(new NotAuthenticatedError(error));
    }

    //No user found?
    if (!user) {
      return next(new NotAuthenticatedError());
    }

    //User pending approval?
    if (user.isPending) {
      return next(new UserPendingError());
    }

    //User archived?
    if (user.isArchived) {
      return next(new UserArchivedError());
    }

    //Set user in request
    req.user = user;
    next();
  };
};

And to use it in middleware:

authenticate(req, res, next) {
  const callback = passportCb(req, res, next);
  return passport.authenticate('bearer', callback)(req, res, next);
}

This at least reduces boilerplate code to a minimum, and allows you to have a generic callback handler with custom error handling etc.

Hope it's of use to someone.

kuldeepdhaka commented 5 years ago

Anyone looking for a quick and minimal way to take control over authentication failure responce:

// old way: no control over the authentication failure response 
router.get('/list', passport.authenticate('jwt', { session: false }),
    function(req, res, next) {
        res.send({"protected": "resource"})
    }
);

// new way: reduced to one function with json response for authentication failure

// print json response instead of string
// using Custom Callback
//http://www.passportjs.org/docs/authenticate/
function passport_authenticate_jwt(callback) {
    function hack(req, res, next) {
        passport.authenticate('jwt', function(err, user, info) {
            if (err) return next(err)
            if (!user) return res.status(401).send({
                "error": {
                    "code": "INVALID_AUTHORIZATION_CODE",
                    "message": "Invalid authorization code"
                }
            });

            req.user = user
            return callback(req, res, next);
        })(req, res, next);
    }

    return hack
}

router.get('/list', passport_authenticate_jwt(function(req, res, next) {
    req.send({"protected": "resource"})
}));
tomislav13 commented 5 years ago

Similar to kuldeepdhaka's answer, maybe use it as a middleware function:

function authenticateJwt(req, res, next) {
  passport.authenticate('jwt', function(err, user, info) {
    if (err) return next(err);
    if (!user) throw new AuthError('401', 'User is not authenticated.');
    req.user = user;
    next();
  })(req, res, next);
}

router.get('/list', authenticateJwt, your_other_middleware_function1, your_other_function2, ... );
gucompi commented 4 years ago

Similar to kuldeepdhaka's answer, maybe use it as a middleware function:

function authenticateJwt(req, res, next) {
  passport.authenticate('jwt', function(err, user, info) {
    if (err) return next(err);
    if (!user) throw new AuthError('401', 'User is not authenticated.');
    req.user = user;
    next();
  })(req, res, next);
}

router.get('/list', authenticateJwt, your_other_middleware_function1, your_other_function2, ... );

This works for me. Thanks!

chan-dev commented 4 years ago

Any updates on this one. I've tried the middleware solution from @gucompi as well as the solution from @kuldeepdhaka but i'm currently having like infinite loop. I have no clue why that is happening.