HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Unit Testing Express Middleware That Verifies Auth Tokens

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
middlewareauthexpressverifiesthattestingtokensunit

Problem

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 us

Solution

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

Code Snippets

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

Context

StackExchange Code Review Q#113159, answer score: 3

Revisions (0)

No revisions yet.