From 6120d0a80f4db2698d776b3704afab64d29e1b8a Mon Sep 17 00:00:00 2001 From: Sebastian Gierlinger Date: Fri, 21 Aug 2015 23:46:42 +0200 Subject: [PATCH] Improve middleware coverage refs #5286 - changed auth-strategies to be testable - added tests --- core/server/middleware/auth-strategies.js | 40 +++-- core/server/middleware/index.js | 7 +- .../unit/middleware/auth-strategies_spec.js | 140 ++++++++++++++++++ core/test/utils/index.js | 11 +- 4 files changed, 172 insertions(+), 26 deletions(-) create mode 100644 core/test/unit/middleware/auth-strategies_spec.js diff --git a/core/server/middleware/auth-strategies.js b/core/server/middleware/auth-strategies.js index 8982f1af07..962736e4a0 100644 --- a/core/server/middleware/auth-strategies.js +++ b/core/server/middleware/auth-strategies.js @@ -1,6 +1,4 @@ -var BearerStrategy = require('passport-http-bearer').Strategy, - ClientPasswordStrategy = require('passport-oauth2-client-password').Strategy, - models = require('../models'), +var models = require('../models'), strategies; strategies = { @@ -14,9 +12,8 @@ strategies = { * HTTP Basic scheme to authenticate (not implemented yet). * Use of the client password strategy is implemented to support ember-simple-auth. */ - clientPasswordStrategy: new ClientPasswordStrategy( - function strategy(clientId, clientSecret, done) { - models.Client.forge({slug: clientId}) + clientPasswordStrategy: function clientPasswordStrategy(clientId, clientSecret, done) { + return models.Client.forge({slug: clientId}) .fetch() .then(function then(model) { if (model) { @@ -27,8 +24,7 @@ strategies = { } return done(null, false); }); - } - ), + }, /** * BearerStrategy @@ -38,24 +34,23 @@ strategies = { * application, which is issued an access token to make requests on behalf of * the authorizing user. */ - bearerStrategy: new BearerStrategy( - function strategy(accessToken, done) { - models.Accesstoken.forge({token: accessToken}) + bearerStrategy: function bearerStrategy(accessToken, done) { + return models.Accesstoken.forge({token: accessToken}) .fetch() .then(function then(model) { if (model) { var token = model.toJSON(); if (token.expires > Date.now()) { - models.User.forge({id: token.user_id}) - .fetch() - .then(function then(model) { - if (model) { - var user = model.toJSON(), - info = {scope: '*'}; - return done(null, {id: user.id}, info); - } - return done(null, false); - }); + return models.User.forge({id: token.user_id}) + .fetch() + .then(function then(model) { + if (model) { + var user = model.toJSON(), + info = {scope: '*'}; + return done(null, {id: user.id}, info); + } + return done(null, false); + }); } else { return done(null, false); } @@ -63,8 +58,7 @@ strategies = { return done(null, false); } }); - } - ) + } }; module.exports = strategies; diff --git a/core/server/middleware/index.js b/core/server/middleware/index.js index e6e62f838c..b98d935980 100644 --- a/core/server/middleware/index.js +++ b/core/server/middleware/index.js @@ -31,6 +31,9 @@ var bodyParser = require('body-parser'), themeHandler = require('./theme-handler'), privateBlogging = require('./private-blogging'), + ClientPasswordStrategy = require('passport-oauth2-client-password').Strategy, + BearerStrategy = require('passport-http-bearer').Strategy, + blogApp, middleware, setupMiddleware; @@ -55,8 +58,8 @@ setupMiddleware = function setupMiddleware(blogAppInstance, adminApp) { oauthServer = oauth2orize.createServer(); // silence JSHint without disabling unused check for the whole file - passport.use(authStrategies.clientPasswordStrategy); - passport.use(authStrategies.bearerStrategy); + passport.use(new ClientPasswordStrategy(authStrategies.clientPasswordStrategy)); + passport.use(new BearerStrategy(authStrategies.bearerStrategy)); // Cache express server instance blogApp = blogAppInstance; diff --git a/core/test/unit/middleware/auth-strategies_spec.js b/core/test/unit/middleware/auth-strategies_spec.js new file mode 100644 index 0000000000..f9ad47a4b4 --- /dev/null +++ b/core/test/unit/middleware/auth-strategies_spec.js @@ -0,0 +1,140 @@ +/*globals describe, before, beforeEach, afterEach, it*/ +/*jshint expr:true*/ +var should = require('should'), + sinon = require('sinon'), + Promise = require('bluebird'), + testUtils = require('../../utils'), + authStrategies = require('../../../server/middleware/auth-strategies'), + models = require('../../../server/models'), + globalUtils = require('../../../server/utils'); + +// To stop jshint complaining +should.equal(true, true); + +describe('Auth Strategies', function () { + var next, sandbox; + + before(testUtils.teardown); + + beforeEach(function () { + sandbox = sinon.sandbox.create(); + next = sandbox.spy(); + }); + + afterEach(function () { + sandbox.restore(); + }); + afterEach(testUtils.teardown); + + describe('Client Password Strategy', function () { + beforeEach(testUtils.setup('clients')); + + it('should find client', function (done) { + var clientId = 'ghost-admin', + clientSecret = 'not_available'; + + authStrategies.clientPasswordStrategy(clientId, clientSecret, function () { + arguments.length.should.eql(2); + should.equal(arguments[0], null); + arguments[1].slug.should.eql('ghost-admin'); + done(); + }); + }); + + it('shouldn\'t find client with invalid id', function (done) { + var clientId = 'invalid_id', + clientSecret = 'not_available'; + authStrategies.clientPasswordStrategy(clientId, clientSecret, next).then(function () { + next.called.should.be.true; + next.calledWith(null, false).should.be.true; + done(); + }); + }); + + it('shouldn\'t find client with invalid secret', function (done) { + var clientId = 'ghost-admin', + clientSecret = 'invalid_secret'; + authStrategies.clientPasswordStrategy(clientId, clientSecret, next).then(function () { + next.called.should.be.true; + next.calledWith(null, false).should.be.true; + done(); + }); + }); + }); + + describe('Bearer Strategy', function () { + beforeEach(testUtils.setup('users:roles', 'users', 'clients')); + + it('should find user with valid token', function (done) { + var accessToken = 'valid-token'; + + testUtils.fixtures.insertAccessToken({ + user_id: 3, + token: accessToken, + client_id: 1, + expires: Date.now() + globalUtils.ONE_DAY_MS + }).then(function () { + authStrategies.bearerStrategy(accessToken, function () { + should.equal(arguments[0], null); + arguments[1].id.should.eql(3); + arguments[2].scope.should.eql('*'); + done(); + }); + }); + }); + + it('shouldn\'t find user with invalid token', function (done) { + var accessToken = 'invalid_token'; + + authStrategies.bearerStrategy(accessToken, next).then(function () { + next.called.should.be.true; + next.calledWith(null, false).should.be.true; + done(); + }); + }); + + it('should find user that doesn\'t exist', function (done) { + var accessToken = 'valid-token'; + + // stub needed for mysql, pg + // this case could only happen in sqlite + sandbox.stub(models.User, 'forge', function () { + return { + fetch: function () { + return Promise.resolve(); + } + }; + }); + + testUtils.fixtures.insertAccessToken({ + user_id: 3, + token: accessToken, + client_id: 1, + expires: Date.now() + globalUtils.ONE_DAY_MS + }).then(function () { + return authStrategies.bearerStrategy(accessToken, next); + }).then(function () { + next.called.should.be.true; + next.calledWith(null, false).should.be.true; + done(); + }); + }); + + it('should find user with expired token', function (done) { + var accessToken = 'expired-token'; + + testUtils.fixtures.insertAccessToken({ + user_id: 3, + token: accessToken, + client_id: 1, + expires: Date.now() - globalUtils.ONE_DAY_MS + }).then(function () { + return authStrategies.bearerStrategy(accessToken, next); + }).then(function () { + next.called.should.be.true; + next.calledWith(null, false).should.be.true; + done(); + }); + }); + }); +}); diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 7d0167d6af..26acea53fb 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -362,6 +362,14 @@ fixtures = { return knex('permissions').insert(permsToInsert).then(function () { return knex('permissions_roles').insert(permissionsRoles); }); + }, + insertClients: function insertClients() { + var knex = config.database.knex; + return knex('clients').insert(DataGenerator.forKnex.clients); + }, + insertAccessToken: function insertAccessToken(override) { + var knex = config.database.knex; + return knex('accesstokens').insert(DataGenerator.forKnex.createToken(override)); } }; @@ -410,7 +418,8 @@ toDoList = { 'perms:init': function initPermissions() { return permissions.init(); }, perms: function permissionsFor(obj) { return function permissionsForObj() { return fixtures.permissionsFor(obj); }; - } + }, + clients: function insertClients() { return fixtures.insertClients(); } }; /**