Unit Testing Express Middleware That Verifies Auth Tokens

Question

I am writing an Express middleware component that'll run on Node 4, which has limited ES2015 support.

The fundamental purpose of this component is to verify JSON web tokens, using the jsonwebtoken module. Here is said component:

const jwt = require("jsonwebtoken");

module.exports = {
  ensureAuthenticated: function(req, res) {
    return new Promise(function(resolve, reject) {
      const token = req.query.token;
      if (!token) {
        return reject({
          status: 401
        });
      } else {
        jwt
          .verify(token, "secret", function(err, decoded) {
            if (err) {
              return reject({
                status: 401
              });
            } else {
              req.user = decoded;
              resolve();
            }
          });
      }
    });
  }
};

I also wrote some unit tests, using Mocha and Chai:

const chai = require("chai");
const proxyquire = require("proxyquire");
const httpMocks = require("node-mocks-http");

const expect = chai.expect;

suite("ensureAuthenticated", function() {

  test("should return 401 when token is \"undefined\"", function() {
    const sut = require("../../middleware");
    const req = httpMocks.createRequest();

    return sut
      .ensureAuthenticated(req)
      .catch(function(outcome) {
        expect(outcome.status).to.equal(401);
      });
  });

  test("should return 401 when token is invalid", function() {
    const jsonwebtoken = { };
    jsonwebtoken.verify = function(token, secret, cb) {
      cb({});
    };
    const req = httpMocks.createRequest({
      query: {
        token: "bar"
      }
    });
    const sut = proxyquire("../../middleware", {
      jsonwebtoken: jsonwebtoken
    });

    return sut
      .ensureAuthenticated(req)
      .catch(function(outcome) {
        expect(outcome.status).to.equal(401);
      });
  });

  test("should add decoded data to req object when token is valid", function() {
    const decoded = {
      username: "dummy username"
    };
    const jsonwebtoken = { };
    jsonwebtoken.verify = function(token, secret, cb) {
      cb(undefined, decoded);
    };
    const req = httpMocks.createRequest({
      query: {
        token: "dummy token"
      }
    });
    const sut = proxyquire("../../middleware", {
      jsonwebtoken: jsonwebtoken
    });

    return sut
      .ensureAuthenticated(req)
      .then(function() {
        expect(req.user).to.equal(decoded);
      });
  });
});

As you can see, the component returns a Promise, which makes it unit-testable. I got the idea from this talk.

I would please like to hear any thoughts you have about the quality of my tests. I am also interested in any general criticism.

Answer 1

I only looked at the tests briefly, but they look good and I like your approach

The following changes will help the readability of your main code:

• Prefer early returning guard clauses over if/else. It's easier to follow, uses less code, and avoids a level of indentation.
• Name concepts clearly and move them to helper functions, so that your main body expresses its intent better: see notAuthorized()
• Arguably a personal preference, but consider it: For short, one-line if statements, don't use brackets and put them on a single line.

Rewrite (note this hasn't been tested, so it's possible there are trivial errors):

module.exports = {
  ensureAuthenticated: function(req, res) {
    return new Promise(function(resolve, reject) {

      // Use guard clause rather if / else
      const token = req.query.token;
      if (!token) return notAuthorized();

      jwt.verify(token, "secret", function(err, decoded) {

        if (err) return notAuthorized();
        req.user = decoded;
        resolve();
      });

    });
    function notAuthorized() { return reject({ status: 401}) }
  }
};