diff --git a/api-server/server/middlewares/jwt-authorizaion.test.js b/api-server/server/middlewares/jwt-authorizaion.test.js deleted file mode 100644 index 8374198fc5..0000000000 --- a/api-server/server/middlewares/jwt-authorizaion.test.js +++ /dev/null @@ -1,30 +0,0 @@ -/* global describe xdescribe it expect */ -import { isWhiteListedPath } from './jwt-authorization'; - -describe('jwt-authorization', () => { - describe('isWhiteListedPath', () => { - const whiteList = [/^\/is-ok\//, /^\/this-is\/also\/ok\//]; - - it('returns a boolean', () => { - const result = isWhiteListedPath(); - - expect(typeof result).toBe('boolean'); - }); - - it('returns true for a white listed path', () => { - expect.assertions(2); - - const resultA = isWhiteListedPath('/is-ok/should-be/good', whiteList); - const resultB = isWhiteListedPath('/this-is/also/ok/surely', whiteList); - expect(resultA).toBe(true); - expect(resultB).toBe(true); - }); - - it('returns false for a non-white-listed path', () => { - const result = isWhiteListedPath('/hax0r-42/no-go', whiteList); - expect(result).toBe(false); - }); - }); - - xdescribe('authorizeByJWT'); -}); diff --git a/api-server/server/middlewares/request-authorizaion.test.js b/api-server/server/middlewares/request-authorizaion.test.js new file mode 100644 index 0000000000..9648d57151 --- /dev/null +++ b/api-server/server/middlewares/request-authorizaion.test.js @@ -0,0 +1,139 @@ +/* global describe it expect */ +import sinon from 'sinon'; +import { mockReq, mockRes } from 'sinon-express-mock'; +import jwt from 'jsonwebtoken'; + +import createRequestAuthorization, { + isWhiteListedPath +} from './request-authorization'; + +const validJWTSecret = 'this is a super secret string'; +const invalidJWTSecret = 'This is not correct secret'; +const now = new Date(Date.now()); +const theBeginningOfTime = new Date(0); +const accessToken = { + id: '123abc', + userId: '456def', + ttl: 60000, + created: now +}; +const users = { + '456def': { + username: 'camperbot', + progressTimestamps: [1, 2, 3, 4] + } +}; +const mockGetUserById = id => + id in users ? Promise.resolve(users[id]) : Promise.reject('No user found'); + +describe('request-authorization', () => { + describe('isWhiteListedPath', () => { + const whiteList = [/^\/is-ok\//, /^\/this-is\/also\/ok\//]; + + it('returns a boolean', () => { + const result = isWhiteListedPath(); + + expect(typeof result).toBe('boolean'); + }); + + it('returns true for a white listed path', () => { + expect.assertions(2); + + const resultA = isWhiteListedPath('/is-ok/should-be/good', whiteList); + const resultB = isWhiteListedPath('/this-is/also/ok/surely', whiteList); + expect(resultA).toBe(true); + expect(resultB).toBe(true); + }); + + it('returns false for a non-white-listed path', () => { + const result = isWhiteListedPath('/hax0r-42/no-go', whiteList); + expect(result).toBe(false); + }); + }); + + describe('createRequestAuthorization', () => { + const requestAuthorization = createRequestAuthorization({ + _jwtSecret: validJWTSecret, + getUserById: mockGetUserById + }); + + it('is a function', () => { + expect(typeof requestAuthorization).toEqual('function'); + }); + + it('throws when no access token is present', () => { + expect.assertions(2); + const req = mockReq({ path: '/internal/some-path/that-needs/auth' }); + const res = mockRes(); + const next = sinon.spy(); + expect(() => requestAuthorization(req, res, next)).toThrowError( + 'Access token is required for this request' + ); + expect(next.called).toBe(false); + }); + + it('throws when the access token is invalid', () => { + expect.assertions(2); + const invalidJWT = jwt.sign({ accessToken }, invalidJWTSecret); + const req = mockReq({ + path: '/internal/some-path/that-needs/auth', + // eslint-disable-next-line camelcase + cookie: { jwt_access_token: invalidJWT } + }); + const res = mockRes(); + const next = sinon.spy(); + + expect(() => requestAuthorization(req, res, next)).toThrowError( + 'invalid signature' + ); + expect(next.called).toBe(false); + }); + + it('throws when the access token has expired', () => { + expect.assertions(2); + const invalidJWT = jwt.sign( + { accessToken: { ...accessToken, created: theBeginningOfTime } }, + validJWTSecret + ); + const req = mockReq({ + path: '/internal/some-path/that-needs/auth', + // eslint-disable-next-line camelcase + cookie: { jwt_access_token: invalidJWT } + }); + const res = mockRes(); + const next = sinon.spy(); + + expect(() => requestAuthorization(req, res, next)).toThrowError( + 'Access token is no longer vaild' + ); + expect(next.called).toBe(false); + }); + + it('adds the user to the request object', async done => { + expect.assertions(5); + const validJWT = jwt.sign({ accessToken }, validJWTSecret); + const req = mockReq({ + path: '/internal/some-path/that-needs/auth', + // eslint-disable-next-line camelcase + cookie: { jwt_access_token: validJWT } + }); + const res = mockRes(); + const next = sinon.spy(); + await requestAuthorization(req, res, next); + expect(next.called).toBe(true); + expect(req).toHaveProperty('user'); + expect(req.user).toEqual(users['456def']); + expect(req.user).toHaveProperty('points'); + expect(req.user.points).toEqual(4); + return done(); + }); + + it('calls next if request does not require authorization', async () => { + const req = mockReq({ path: '/unauthenticated/another/route' }); + const res = mockRes(); + const next = sinon.spy(); + await requestAuthorization(req, res, next); + expect(next.called).toBe(true); + }); + }); +}); diff --git a/api-server/server/middlewares/jwt-authorization.js b/api-server/server/middlewares/request-authorization.js similarity index 77% rename from api-server/server/middlewares/jwt-authorization.js rename to api-server/server/middlewares/request-authorization.js index 06f88fed05..ef408882f0 100644 --- a/api-server/server/middlewares/jwt-authorization.js +++ b/api-server/server/middlewares/request-authorization.js @@ -1,6 +1,7 @@ import loopback from 'loopback'; import jwt from 'jsonwebtoken'; import { isBefore } from 'date-fns'; +import { isEmpty } from 'lodash'; import { homeLocation } from '../../../config/env'; @@ -18,8 +19,11 @@ export function isWhiteListedPath(path, whiteListREs = _whiteListREs) { return whiteListREs.some(re => re.test(path)); } -export default () => - function authorizeByJWT(req, res, next) { +export default ({ + _jwtSecret = process.env.JWT_SECRET, + getUserById = _getUserById +} = {}) => + function requestAuthorisation(req, res, next) { const { path } = req; if (apiProxyRE.test(path) && !isWhiteListedPath(path)) { const cookie = @@ -39,7 +43,7 @@ export default () => } let token; try { - token = jwt.verify(cookie, process.env.JWT_SECRET); + token = jwt.verify(cookie, _jwtSecret); } catch (err) { throw wrapHandledError(new Error(err.message), { type: 'info', @@ -60,9 +64,9 @@ export default () => status: 403 }); } - if (!req.user) { - const User = loopback.getModelByType('User'); - return User.findById(userId) + + if (isEmpty(req.user)) { + return getUserById(userId) .then(user => { if (user) { user.points = user.progressTimestamps.length; @@ -73,8 +77,20 @@ export default () => .then(next) .catch(next); } else { - return next(); + return Promise.resolve(next()); } } - return next(); + return Promise.resolve(next()); }; + +export function _getUserById(id) { + const User = loopback.getModelByType('User'); + return new Promise((resolve, reject) => + User.findById(id, (err, instance) => { + if (err) { + return reject(err); + } + return resolve(instance); + }) + ); +}