CODEWITHSUNDEEP
javascriptnode.jsexpress
Craig Walsh
Craig Walsh

Reputation: 117

Express JS authorization middleware

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);
    }
};

Upvotes: 0

Views: 354

Answers (2)

Maroshii
Maroshii

Reputation: 4017

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();
};

Upvotes: 2

rink.attendant.6
rink.attendant.6

Reputation: 46218

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.

Upvotes: 0

Related Questions