Express JS authorization middleware

Question

I'm writing and API with Express JS that uses JSON web tokens for authorization. Is there are more readable way to show the user the correct error message? How would you refactor the following authorization middleware?

module.exports.authorize = function (request, response, next) {
    var apiToken = request.headers['x-api-token'];

    if(apiToken) {
        var decoded = token.verify(apiToken);
        
        if(decoded) {
            if(decoded.exp <= moment().format('x')) {
                next();
            } else {
                var expiredTokenError = new Error('Token has expired');
                expiredTokenError.status = 419;
                return next(expiredTokenError);
            }
        } else {
            var invalidTokenError = new Error('Token is invalid');
            invalidTokenError.status = 401;
            return next(invalidTokenError);
        }
    } else {
        var notFoundError = new Error('Token not found');
        notFoundError.status = 404;
        return next(notFoundError);
    }
};

Maroshii · Accepted Answer · 2015-05-06 22:04:29Z

2

For readability I would suggest to first handle all the errors and call next at the very end if everything went OK. Also you might wanna move the error handling to a separate function to avoid repeating yourself. So in short:

var ERRORS = {
  EXPIRED: {
    message: 'Token has expired',
    status: 419
  },
  NOT_FOUND: {
    message: 'Token not found',
    status: 404
  },
  INVALID: {
    message: 'Token is invalid',
    status: 401
  }
}

var errorHandler = function(err,next) {
  var error = new Error(err.message);
  error.status = err.status;
  next(error);
};

module.exports.authorize = function (request, response, next) {
  var apiToken = request.headers['x-api-token'];

  if(!apiToken){
    return errorHandler(ERRORS.NOT_FOUND,next);
  }

  var decoded = token.verify(apiToken);

  if(!decoded){
    return errorHandler(ERRORS.INVALID,next);
  }

  if(decoded.exp > moment().format('x')){
    return errorHandler(ERRORS.EXPIRED,next);
  }

  next();
};

answered May 6, 2015 at 22:04

Maroshii

4,0274 gold badges26 silver badges29 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Craig Walsh Over a year ago

I like the idea of extracting the error handling code out into a function. Thanks!

Community · Accepted Answer · 2017-04-12 07:32:04Z

I would avoid using else after a return statement:

module.exports.authorize = function (request, response, next) {
    var apiToken = request.headers['x-api-token'];

    if (!apiToken) {
        var notFoundError = new Error('Token not found');
        notFoundError.status = 404;
        return next(notFoundError);
    }

    var decoded = token.verify(apiToken);

    if (!decoded) {
        var invalidTokenError = new Error('Token is invalid');
        invalidTokenError.status = 401;
        return next(invalidTokenError);
    }

    if (decoded.exp > moment().format('x')) {
        var expiredTokenError = new Error('Token has expired');
        expiredTokenError.status = 419;
        return next(expiredTokenError);
    }

    next();
};

I know this is JavaScript, but conceptually this question on Programmers.SE may be of interest to you.

Collectives™ on Stack Overflow

Express JS authorization middleware

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related