2

I am reading a code that has two files like below:

first file that uses the currentuser middleware:

const router = express.Router();

router.get("/api/users/currentuser", currentUser, (req, res) => {
  res.send({ currentUser: req.currentUser || null });
});

export { router as currentUserRouter };

Second file that defines the middleware:

interface UserPayload {
  id: string;
  email: string;
}

declare global {
  namespace Express {
    interface Request {
      currentUser?: UserPayload;
    }
  }
}

export const currentUser = (
  req: Request,
  res: Response,
  next: NextFunction
) => {
  if (!req.session?.jwt) {
    return next();
  }

  try {
    const payload = jwt.verify(
      req.session.jwt,
      process.env.JWT_KEY!
    ) as UserPayload;
    req.currentUser = payload;
  } catch (err) {}

  next();
};

I understand that if there is a verified jwt token, the middleware will take the the payload out of it and add it to the req object. But what if it fails and it can't add the payload/current user to the req? What would happen for the following request and what will the res object look like?

router.get("/api/users/currentuser", currentUser, (req, res) => {
  res.send({ currentUser: req.currentUser || null });
});

Could you edit this get request to show how can I catch the probable error if I am not the writer of the middleware?

GoodMan
  • 542
  • 6
  • 19

2 Answers2

3

If you had a catchall exception handler, and your middleware threw an exception, you would determine the response.

If your middleware threw an exception and you did not catch it, the system might just exit the process.

If your middleware did not throw an exception, and did not call next(), and did not respond, the request would hang.

If your middleware returned a response, and did not call next(), your send function would never get invoked.

The bottom line is that you need to dump the response on your server and see exactly how your middleware handles this.

In most of my auth middleware, I choose to not call next(), and return a 403 error. But there are some benefits by throwing an exception, then returning a 403 from a catchall handler.

Steven Spungin
  • 27,002
  • 5
  • 88
  • 78
  • Could you edit this get request `router.get("/api/users/currentuser", currentUser, (req, res) => { res.send({ currentUser: req.currentUser || null }); });` to show how can I catch the probable error if I am not the writer of the middleware? – GoodMan Nov 03 '22 at 00:24
  • 1
    Did you determine if your middleware was throwing, calling send(), and/or calling next()? BTW, your error handler will be another entry, not in your get() function. `router.use(errorHandler)` – Steven Spungin Nov 03 '22 at 04:33
  • So if the middleware throws an error, or there is an error happened by network connection, etc. I must catch all of them after my `get` request, using `router.use(errorHandler)` approach? – GoodMan Nov 03 '22 at 18:22
  • Can't I chain those catsh errors to my requests like `router.get(/, middleware, callback()).catshError()`? – GoodMan Nov 03 '22 at 18:23
  • Error handlers may not reach your middleware. – Steven Spungin Nov 04 '22 at 15:40
2

You need to respond with an error HTTP status code, and an error message in the body. The exact status and message depends on the type of the exception and its parameters, so you need to catch it and check it.

The current express middleware does not handle errors, it just does not set the req.currentUser = payload;, so you won't know about the user. I don't think this is a proper solution for an authentication error.

In the documentation you can see how error are handled: https://expressjs.com/en/guide/using-middleware.html

app.use((err, req, res, next) => {
  console.error(err.stack)
  res.status(500).send('Something broke!')
})

So I would rewrite the code and if the JWT verification fails, then I return for example 401 unauthorized. https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/401

I guess you are using this JWT library: https://github.com/auth0/node-jsonwebtoken According to the docs and the code there are 3 types of errors: TokenExpiredError, JsonWebTokenError, NotBeforeError for verify. Here you can check when they are thrown: https://github.com/auth0/node-jsonwebtoken/blob/master/verify.js , here are their definitions: https://github.com/auth0/node-jsonwebtoken/tree/master/lib

So in the catch block you just check the type of the error with instanceof e.g. if (err instanceof jwt.JsonWebTokenError) ... and send the message accordingly with the res.status(401) and put the next() to the end of the try block, because it should be called only if the verification does not fail.

inf3rno
  • 24,976
  • 11
  • 115
  • 197
  • So you say the current code will throw an unknown error? – GoodMan Nov 03 '22 at 00:10
  • @GoodMan If I understand your concern, then the `catch (err) {}` will catch the exception and it will fail silently. There won't be any response. You need to add the response with the error message to that part of the code or remove the try-catch block and handle the exception on a higher level. – inf3rno Nov 03 '22 at 00:12
  • @GoodMan Here you can see how proper error handling goes in an express middleware: https://expressjs.com/en/guide/using-middleware.html – inf3rno Nov 03 '22 at 00:13
  • Could you edit my get request to show how can I catch the error, if I am not the writer of the middleware? – GoodMan Nov 03 '22 at 00:16
  • You would add error handling middleware. :) https://expressjs.com/en/guide/error-handling.html – Steven Spungin Nov 03 '22 at 00:19