Merge pull request #5353 from morficus/issue-5286

splitting client authentication-related middleware in to its own file

core/server/middleware/client-auth.js

@@ -0,0 +1,35 @@
+var passport    = require('passport'),
+    _           = require('lodash'),
+    oauthServer,
+
+    clientAuth;
+
+function cacheOauthServer(server) {
+    oauthServer = server;
+}
+
+clientAuth = {
+    // work around to handle missing client_secret
+    // oauth2orize needs it, but untrusted clients don't have it
+    addClientSecret: function addClientSecret(req, res, next) {
+        if (_.isEmpty(req.body.client_secret)) {
+            req.body.client_secret = 'not_available';
+        }
+        next();
+    },
+
+    // ### Authenticate Client Middleware
+    // authenticate client that is asking for an access token
+    authenticateClient: function authenticateClient(req, res, next) {
+        return passport.authenticate(['oauth2-client-password'], {session: false})(req, res, next);
+    },
+
+    // ### Generate access token Middleware
+    // register the oauth2orize middleware for password and refresh token grants
+    generateAccessToken: function generateAccessToken(req, res, next) {
+        return oauthServer.token()(req, res, next);
+    }
+};
+
+module.exports = clientAuth;
+module.exports.cacheOauthServer = cacheOauthServer;

core/server/middleware/middleware.js

@@ -19,10 +19,10 @@ var _           = require('lodash'),
     busboy       = require('./ghost-busboy'),
     cacheControl = require('./cache-control'),
     spamPrevention  = require('./spam-prevention'),
+    clientAuth  = require('./client-auth'),
 
     middleware,
-    blogApp,
-    oauthServer;
+    blogApp;
 
 function isBlackListedFileType(file) {
     var blackListedFileTypes = ['.hbs', '.md', '.json'],
@@ -34,10 +34,6 @@ function cacheBlogApp(app) {
     blogApp = app;
 }
 
-function cacheOauthServer(server) {
-    oauthServer = server;
-}
-
 function isSSLrequired(isAdmin, configUrl, forceAdminSSL) {
     var forceSSL = url.parse(configUrl).protocol === 'https:' ? true : false;
     if (forceSSL || (isAdmin && forceAdminSSL)) {
@@ -173,27 +169,6 @@ middleware = {
         });
     },
 
-    // work around to handle missing client_secret
-    // oauth2orize needs it, but untrusted clients don't have it
-    addClientSecret: function (req, res, next) {
-        if (!req.body.client_secret) {
-            req.body.client_secret = 'not_available';
-        }
-        next();
-    },
-
-    // ### Authenticate Client Middleware
-    // authenticate client that is asking for an access token
-    authenticateClient: function (req, res, next) {
-        return passport.authenticate(['oauth2-client-password'], {session: false})(req, res, next);
-    },
-
-    // ### Generate access token Middleware
-    // register the oauth2orize middleware for password and refresh token grants
-    generateAccessToken: function (req, res, next) {
-        return oauthServer.token()(req, res, next);
-    },
-
     // Check to see if we should use SSL
     // and redirect if needed
     checkSSL: function (req, res, next) {
@@ -330,7 +305,11 @@ middleware = {
 
 module.exports = middleware;
 module.exports.cacheBlogApp = cacheBlogApp;
-module.exports.cacheOauthServer = cacheOauthServer;
+
+module.exports.addClientSecret = clientAuth.addClientSecret;
+module.exports.cacheOauthServer = clientAuth.cacheOauthServer;
+module.exports.authenticateClient = clientAuth.authenticateClient;
+module.exports.generateAccessToken = clientAuth.generateAccessToken;
 
 // SSL helper functions are exported primarily for unity testing.
 module.exports.isSSLrequired = isSSLrequired;

core/test/unit/middleware/clientAuth_spec.js

@@ -0,0 +1,44 @@
+/*globals describe, beforeEach, it*/
+/*jshint expr:true*/
+var should          = require('should'),
+    sinon           = require('sinon'),
+
+    middleware      = require('../../../server/middleware').middleware;
+
+describe('Middleware: Client Auth', function () {
+    var req, res, next;
+
+    beforeEach(function () {
+        req = {};
+        res = {};
+        next = sinon.spy();
+    });
+
+    describe('addClientSecret', function () {
+        it('sets a `client_secret` if not part of body', function () {
+            var requestBody = {};
+
+            req.body = requestBody;
+
+            middleware.addClientSecret(req, res, next);
+
+            next.called.should.be.true;
+            should(req.body).have.property('client_secret');
+            req.body.client_secret.should.not.be.empty;
+        });
+
+        it('does not tamper with `client_secret` if already present', function () {
+            var requestBody = {
+                client_secret: 'keep-it-safe-keep-it-secret'
+            };
+
+            req.body = requestBody;
+
+            middleware.addClientSecret(req, res, next);
+
+            next.called.should.be.true;
+            should(req.body).have.property('client_secret');
+            req.body.client_secret.should.equal('keep-it-safe-keep-it-secret');
+        });
+    });
+});