Removed res.isAdmin usages from v2 express app (#9884)

refs #9866

- Removed `res.isAdmin` flag in v2 express app
- Did not touch v0.1 express app
- Separated url redirect middleware for admin and content API

core/server/web/api/v2/admin/app.js

@@ -21,13 +21,6 @@ module.exports = function setupApiApp() {
     debug('Admin API v2 setup start');
     const apiApp = express();
 
-    // @TODO finish refactoring this away.
-    apiApp.use(function setIsAdmin(req, res, next) {
-        // api === isAdmin
-        res.isAdmin = true;
-        next();
-    });
-
     // API middleware
 
     // Body parsing

core/server/web/api/v2/admin/middleware.js

@@ -1,6 +1,6 @@
 const prettyURLs = require('../../../middleware/pretty-urls'),
     cors = require('../../../middleware/api/cors'),
-    urlRedirects = require('../../../middleware/url-redirects'),
+    {adminRedirect} = require('../../../middleware/url-redirects'),
     auth = require('../../../../services/auth');
 
 /**
@@ -11,7 +11,7 @@ module.exports.authenticatePrivate = [
     auth.authenticate.authenticateUser,
     auth.authorize.requiresAuthorizedUser,
     cors,
-    urlRedirects,
+    adminRedirect,
     prettyURLs
 ];
 
@@ -24,7 +24,7 @@ module.exports.authenticateClient = function authenticateClient(client) {
         auth.authenticate.authenticateUser,
         auth.authorize.requiresAuthorizedClient(client),
         cors,
-        urlRedirects,
+        adminRedirect,
         prettyURLs
     ];
 };

core/server/web/api/v2/content/app.js

@@ -17,13 +17,6 @@ module.exports = function setupApiApp() {
     debug('Content API v2 setup start');
     const apiApp = express();
 
-    // @TODO finish refactoring this away.
-    apiApp.use(function setIsAdmin(req, res, next) {
-        // api === isAdmin
-        res.isAdmin = true;
-        next();
-    });
-
     // API middleware
 
     // Query parsing

core/server/web/api/v2/content/middleware.js

@@ -1,6 +1,6 @@
 const prettyURLs = require('../../../middleware/pretty-urls'),
     cors = require('../../../middleware/api/cors'),
-    urlRedirects = require('../../../middleware/url-redirects'),
+    {adminRedirect} = require('../../../middleware/url-redirects'),
     auth = require('../../../../services/auth');
 
 /**
@@ -21,6 +21,6 @@ module.exports.authenticatePublic = [
     // This is a labs-enabled middleware
     auth.authorize.requiresAuthorizedUserPublicAPI,
     cors,
-    urlRedirects,
+    adminRedirect,
     prettyURLs
 ];

core/server/web/middleware/url-redirects.js

@@ -91,15 +91,15 @@ _private.getBlogRedirectUrl = function getBlogRedirectUrl(options) {
  *
  * 1. required SSL redirects
  * 2. redirect to the correct admin url
+ *
  */
-const urlRedirects = function urlRedirects(req, res, next) {
-    const redirectFn = res.isAdmin ? _private.getAdminRedirectUrl : _private.getBlogRedirectUrl,
-        redirectUrl = redirectFn({
-            requestedHost: req.get('host'),
-            requestedUrl: url.parse(req.originalUrl || req.url).pathname,
-            queryParameters: req.query,
-            secure: req.secure
-        });
+_private.redirect = (req, res, next, redirectFn) => {
+    const redirectUrl = redirectFn({
+        requestedHost: req.get('host'),
+        requestedUrl: url.parse(req.originalUrl || req.url).pathname,
+        queryParameters: req.query,
+        secure: req.secure
+    });
 
     if (redirectUrl) {
         debug('url redirect to: ' + redirectUrl);
@@ -110,4 +110,17 @@ const urlRedirects = function urlRedirects(req, res, next) {
     next();
 };
 
+/*
+ * @deprecated: in favor of adminRedirect (extract public getBlogRedirectUrl method when needed)
+ */
+const urlRedirects = (req, res, next) => {
+    const redirectFn = res.isAdmin ? _private.getAdminRedirectUrl : _private.getBlogRedirectUrl;
+    _private.redirect(req, res, next, redirectFn);
+};
+
+const adminRedirect = (req, res, next) => {
+    _private.redirect(req, res, next, _private.getAdminRedirectUrl);
+};
+
 module.exports = urlRedirects;
+module.exports.adminRedirect = adminRedirect;

core/test/unit/web/middleware/url-redirects_spec.js

@@ -1,8 +1,12 @@
 var should = require('should'),
     sinon = require('sinon'),
+    rewire = require('rewire'),
     configUtils = require('../../../utils/configUtils'),
-    urlRedirects = require('../../../../server/web/middleware/url-redirects'),
-
+    urlRedirects = rewire('../../../../server/web/middleware/url-redirects'),
+    {adminRedirect} = urlRedirects,
+    getAdminRedirectUrl = urlRedirects.__get__('_private.getAdminRedirectUrl'),
+    getBlogRedirectUrl = urlRedirects.__get__('_private.getBlogRedirectUrl'),
+    redirect = urlRedirects.__get__('_private.redirect'),
     sandbox = sinon.sandbox.create();
 
 describe('UNIT: url redirects', function () {
@@ -28,6 +32,46 @@ describe('UNIT: url redirects', function () {
         host = null;
     });
 
+    describe('calls to _private.redirect()', function () {
+        let redirectSpy;
+
+        beforeEach(function () {
+            redirectSpy = sandbox.spy();
+            urlRedirects.__set__('_private.redirect', redirectSpy);
+        });
+
+        it('urlRedirects passes getAdminRedirectUrl method when iAdmin flag is not set', function () {
+            configUtils.set({
+                url: 'https://default.com:2368/'
+            });
+
+            urlRedirects(req, res, next);
+
+            redirectSpy.calledWith(req, res, next, getBlogRedirectUrl).should.eql(true);
+        });
+
+        it('urlRedirects passes getAdminRedirectUrl method when iAdmin flag present', function () {
+            res.isAdmin = true;
+            configUtils.set({
+                url: 'https://default.com:2368/'
+            });
+
+            urlRedirects(req, res, next);
+
+            redirectSpy.calledWith(req, res, next, getAdminRedirectUrl).should.eql(true);
+        });
+
+        it('adminRedirect passes getAdminRedirectUrl', function () {
+            configUtils.set({
+                url: 'https://default.com:2368/'
+            });
+
+            adminRedirect(req, res, next);
+
+            redirectSpy.calledWith(req, res, next, getAdminRedirectUrl).should.eql(true);
+        });
+    });
+
     describe('expect redirect', function () {
         it('blog is https, request is http', function (done) {
             configUtils.set({
@@ -37,7 +81,7 @@ describe('UNIT: url redirects', function () {
             host = 'default.com:2368';
 
             req.originalUrl = '/';
-            urlRedirects(req, res, next);
+            redirect(req, res, next, getBlogRedirectUrl);
             next.called.should.be.false();
             res.redirect.called.should.be.true();
             res.redirect.calledWith(301, 'https://default.com:2368/').should.be.true();
@@ -53,7 +97,7 @@ describe('UNIT: url redirects', function () {
             host = 'localhost:2368';
 
             req.originalUrl = '/';
-            urlRedirects(req, res, next);
+            redirect(req, res, next, getBlogRedirectUrl);
             next.called.should.be.false();
             res.redirect.called.should.be.true();
             res.redirect.calledWith(301, 'https://localhost:2368/').should.be.true();
@@ -61,141 +105,137 @@ describe('UNIT: url redirects', function () {
             done();
         });
 
-        it('url and admin url are equal, but protocol is different, request is http', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368',
-                admin: {
-                    url: 'https://default.com:2368'
-                }
+        describe(`admin redirects`, function () {
+            it('url and admin url are equal, but protocol is different, request is http', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368',
+                    admin: {
+                        url: 'https://default.com:2368'
+                    }
+                });
+
+                host = 'default.com:2368';
+
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.false();
+                res.redirect.calledWith(301, 'https://default.com:2368/ghost/').should.be.true();
+                res.set.called.should.be.true();
+                done();
             });
 
-            host = 'default.com:2368';
-            res.isAdmin = true;
+            it('url and admin url are different, request is http', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368',
+                    admin: {
+                        url: 'https://admin.default.com:2368'
+                    }
+                });
 
-            req.originalUrl = '/ghost';
-            urlRedirects(req, res, next);
-            next.called.should.be.false();
-            res.redirect.calledWith(301, 'https://default.com:2368/ghost/').should.be.true();
-            res.set.called.should.be.true();
-            done();
-        });
+                host = 'default.com:2368';
 
-        it('url and admin url are different, request is http', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368',
-                admin: {
-                    url: 'https://admin.default.com:2368'
-                }
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.false();
+                res.redirect.calledWith(301, 'https://admin.default.com:2368/ghost/').should.be.true();
+                res.set.called.should.be.true();
+                done();
             });
 
-            host = 'default.com:2368';
-            res.isAdmin = true;
+            it('subdirectory', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368/blog',
+                    admin: {
+                        url: 'https://admin.default.com:2368'
+                    }
+                });
 
-            req.originalUrl = '/ghost';
-            urlRedirects(req, res, next);
-            next.called.should.be.false();
-            res.redirect.calledWith(301, 'https://admin.default.com:2368/ghost/').should.be.true();
-            res.set.called.should.be.true();
-            done();
-        });
+                host = 'default.com:2368';
 
-        it('subdirectory', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368/blog',
-                admin: {
-                    url: 'https://admin.default.com:2368'
-                }
+                req.originalUrl = '/blog/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.false();
+                res.redirect.calledWith(301, 'https://admin.default.com:2368/blog/ghost/').should.be.true();
+                res.set.called.should.be.true();
+
+                req.secure = true;
+                host = 'admin.default.com:2368';
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.true();
+                res.redirect.calledOnce.should.be.true();
+                res.set.calledOnce.should.be.true();
+                done();
             });
 
-            host = 'default.com:2368';
-            res.isAdmin = true;
+            it('keeps query', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368',
+                    admin: {
+                        url: 'https://admin.default.com:2368'
+                    }
+                });
 
-            req.originalUrl = '/blog/ghost';
-            urlRedirects(req, res, next);
-            next.called.should.be.false();
-            res.redirect.calledWith(301, 'https://admin.default.com:2368/blog/ghost/').should.be.true();
-            res.set.called.should.be.true();
+                host = 'default.com:2368';
 
-            req.secure = true;
-            host = 'admin.default.com:2368';
-            urlRedirects(req, res, next);
-            next.called.should.be.true();
-            res.redirect.calledOnce.should.be.true();
-            res.set.calledOnce.should.be.true();
-            done();
-        });
+                req.originalUrl = '/ghost';
+                req.query = {
+                    test: true
+                };
 
-        it('keeps query', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368',
-                admin: {
-                    url: 'https://admin.default.com:2368'
-                }
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.false();
+                res.redirect.calledWith(301, 'https://admin.default.com:2368/ghost/?test=true').should.be.true();
+                res.set.called.should.be.true();
+                done();
             });
 
-            host = 'default.com:2368';
-            res.isAdmin = true;
+            it('original url has search params', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368',
+                    admin: {
+                        url: 'https://admin.default.com:2368'
+                    }
+                });
 
-            req.originalUrl = '/ghost';
-            req.query = {
-                test: true
-            };
+                host = 'default.com:2368';
 
-            urlRedirects(req, res, next);
-            next.called.should.be.false();
-            res.redirect.calledWith(301, 'https://admin.default.com:2368/ghost/?test=true').should.be.true();
-            res.set.called.should.be.true();
-            done();
-        });
+                req.originalUrl = '/ghost/something?a=b';
+                req.query = {
+                    a: 'b'
+                };
 
-        it('original url has search params', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368',
-                admin: {
-                    url: 'https://admin.default.com:2368'
-                }
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.false();
+                res.redirect.calledWith(301, 'https://admin.default.com:2368/ghost/something/?a=b').should.be.true();
+                res.set.called.should.be.true();
+                done();
             });
 
-            host = 'default.com:2368';
-            res.isAdmin = true;
+            it('ensure redirect loop won\'t happen', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368',
+                    admin: {
+                        url: 'https://default.com:2368'
+                    }
+                });
 
-            req.originalUrl = '/ghost/something?a=b';
-            req.query = {
-                a: 'b'
-            };
+                host = 'default.com:2368';
 
-            urlRedirects(req, res, next);
-            next.called.should.be.false();
-            res.redirect.calledWith(301, 'https://admin.default.com:2368/ghost/something/?a=b').should.be.true();
-            res.set.called.should.be.true();
-            done();
-        });
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.false();
+                res.redirect.calledWith(301, 'https://default.com:2368/ghost/').should.be.true();
+                res.set.called.should.be.true();
 
-        it('ensure redirect loop won\'t happen', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368',
-                admin: {
-                    url: 'https://default.com:2368'
-                }
+                res.redirect.reset();
+
+                req.secure = true;
+                redirect(req, res, next, getAdminRedirectUrl);
+                res.redirect.called.should.be.false();
+                res.set.calledOnce.should.be.true();
+                next.called.should.be.true();
+                done();
             });
-
-            host = 'default.com:2368';
-            res.isAdmin = true;
-
-            req.originalUrl = '/ghost';
-            urlRedirects(req, res, next);
-            next.called.should.be.false();
-            res.redirect.calledWith(301, 'https://default.com:2368/ghost/').should.be.true();
-            res.set.called.should.be.true();
-
-            res.redirect.reset();
-
-            req.secure = true;
-            urlRedirects(req, res, next);
-            res.redirect.called.should.be.false();
-            res.set.calledOnce.should.be.true();
-            next.called.should.be.true();
-            done();
         });
     });
 
@@ -208,7 +248,7 @@ describe('UNIT: url redirects', function () {
             host = 'default.com:2368';
 
             req.originalUrl = '/';
-            urlRedirects(req, res, next);
+            redirect(req, res, next, getBlogRedirectUrl);
             next.called.should.be.true();
             res.redirect.called.should.be.false();
             res.set.called.should.be.false();
@@ -225,7 +265,7 @@ describe('UNIT: url redirects', function () {
 
             req.originalUrl = '/';
             req.secure = true;
-            urlRedirects(req, res, next);
+            redirect(req, res, next, getBlogRedirectUrl);
             next.called.should.be.true();
             res.redirect.called.should.be.false();
             res.set.called.should.be.false();
@@ -241,7 +281,7 @@ describe('UNIT: url redirects', function () {
 
             req.originalUrl = '/';
             req.secure = true;
-            urlRedirects(req, res, next);
+            redirect(req, res, next, getBlogRedirectUrl);
             next.called.should.be.true();
             res.redirect.called.should.be.false();
             res.set.called.should.be.false();
@@ -257,7 +297,7 @@ describe('UNIT: url redirects', function () {
 
             req.originalUrl = '/';
             req.secure = true;
-            urlRedirects(req, res, next);
+            redirect(req, res, next, getBlogRedirectUrl);
             next.called.should.be.true();
             res.redirect.called.should.be.false();
             res.set.called.should.be.false();
@@ -274,84 +314,82 @@ describe('UNIT: url redirects', function () {
 
             req.originalUrl = '/';
             req.secure = true;
-            urlRedirects(req, res, next);
+            redirect(req, res, next, getBlogRedirectUrl);
             next.called.should.be.true();
             res.redirect.called.should.be.false();
             res.set.called.should.be.false();
             done();
         });
 
-        it('admin is blog url and http, requester is http', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368'
+        describe(`admin redirects`, function () {
+            it('admin is blog url and http, requester is http', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368'
+                });
+
+                host = 'default.com:2368';
+
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.true();
+                res.redirect.called.should.be.false();
+                res.set.called.should.be.false();
+                done();
             });
 
-            host = 'default.com:2368';
-            res.isAdmin = true;
+            it('admin request, no custom admin.url configured', function (done) {
+                configUtils.set({
+                    url: 'http://default.com:2368'
+                });
 
-            req.originalUrl = '/ghost';
-            urlRedirects(req, res, next);
-            next.called.should.be.true();
-            res.redirect.called.should.be.false();
-            res.set.called.should.be.false();
-            done();
-        });
+                host = 'localhost:2368';
 
-        it('admin request, no custom admin.url configured', function (done) {
-            configUtils.set({
-                url: 'http://default.com:2368'
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+                next.called.should.be.true();
+                res.redirect.called.should.be.false();
+                res.set.called.should.be.false();
+                done();
             });
 
-            host = 'localhost:2368';
-            res.isAdmin = true;
+            it('url and admin url are different, protocol is different, request is not secure', function (done) {
+                configUtils.set({
+                    url: 'http://blog.ghost.org',
+                    admin: {
+                        url: 'http://something.com'
+                    }
+                });
 
-            req.originalUrl = '/ghost';
-            urlRedirects(req, res, next);
-            next.called.should.be.true();
-            res.redirect.called.should.be.false();
-            res.set.called.should.be.false();
-            done();
-        });
+                host = 'something.com';
+                req.secure = false;
 
-        it('url and admin url are different, protocol is different, request is not secure', function (done) {
-            configUtils.set({
-                url: 'http://blog.ghost.org',
-                admin: {
-                    url: 'http://something.com'
-                }
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+                res.redirect.called.should.be.false();
+                res.set.called.should.be.false();
+                next.called.should.be.true();
+                done();
             });
 
-            host = 'something.com';
-            res.isAdmin = true;
-            req.secure = false;
+            it('url and admin url are different, protocol is different, request is secure', function (done) {
+                configUtils.set({
+                    url: 'http://blog.ghost.org',
+                    admin: {
+                        url: 'http://something.com'
+                    }
+                });
 
-            req.originalUrl = '/ghost';
-            urlRedirects(req, res, next);
-            res.redirect.called.should.be.false();
-            res.set.called.should.be.false();
-            next.called.should.be.true();
-            done();
-        });
+                host = 'something.com';
+                req.secure = true;
 
-        it('url and admin url are different, protocol is different, request is secure', function (done) {
-            configUtils.set({
-                url: 'http://blog.ghost.org',
-                admin: {
-                    url: 'http://something.com'
-                }
+                req.originalUrl = '/ghost';
+                redirect(req, res, next, getAdminRedirectUrl);
+
+                res.redirect.called.should.be.false();
+                res.set.called.should.be.false();
+                next.called.should.be.true();
+                done();
             });
-
-            host = 'something.com';
-            res.isAdmin = true;
-            req.secure = true;
-
-            req.originalUrl = '/ghost';
-            urlRedirects(req, res, next);
-
-            res.redirect.called.should.be.false();
-            res.set.called.should.be.false();
-            next.called.should.be.true();
-            done();
         });
     });
 });