From 01e2263c012dc0e61d78424655c72c30a637345a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Fri, 12 May 2017 17:53:23 -0400 Subject: [PATCH] more tests --- src/controllers/helpers.js | 17 ++-- src/groups/membership.js | 8 +- src/middleware/admin.js | 22 ++--- src/middleware/index.js | 67 +------------- src/middleware/user.js | 181 +++++++++++++++++++++++-------------- test/controllers.js | 93 +++++++++++++++++-- test/helpers/index.js | 3 +- 7 files changed, 225 insertions(+), 166 deletions(-) diff --git a/src/controllers/helpers.js b/src/controllers/helpers.js index 5571bfbf62..9ee0ebc0d9 100644 --- a/src/controllers/helpers.js +++ b/src/controllers/helpers.js @@ -10,8 +10,9 @@ var privileges = require('../privileges'); var categories = require('../categories'); var plugins = require('../plugins'); var meta = require('../meta'); +var middleware = require('../middleware'); -var helpers = {}; +var helpers = module.exports; helpers.notAllowed = function (req, res, error) { plugins.fireHook('filter:helpers.notAllowed', { @@ -31,11 +32,13 @@ helpers.notAllowed = function (req, res, error) { title: '[[global:403.title]]', }); } else { - res.status(403).render('403', { - path: req.path, - loggedIn: !!req.uid, - error: error, - title: '[[global:403.title]]', + middleware.buildHeader(req, res, function () { + res.status(403).render('403', { + path: req.path, + loggedIn: !!req.uid, + error: error, + title: '[[global:403.title]]', + }); }); } } else if (res.locals.isAPI) { @@ -175,5 +178,3 @@ function recursive(category, categoriesData, level) { recursive(child, categoriesData, '    ' + level); }); } - -module.exports = helpers; diff --git a/src/groups/membership.js b/src/groups/membership.js index 6337db1d8a..2660d716e1 100644 --- a/src/groups/membership.js +++ b/src/groups/membership.js @@ -323,12 +323,12 @@ module.exports = function (Groups) { Groups.isMember = function (uid, groupName, callback) { if (!uid || parseInt(uid, 10) <= 0) { - return callback(null, false); + return setImmediate(callback, null, false); } var cacheKey = uid + ':' + groupName; if (cache.has(cacheKey)) { - return process.nextTick(callback, null, cache.get(cacheKey)); + return setImmediate(callback, null, cache.get(cacheKey)); } async.waterfall([ @@ -344,7 +344,7 @@ module.exports = function (Groups) { Groups.isMembers = function (uids, groupName, callback) { function getFromCache(next) { - process.nextTick(next, null, uids.map(function (uid) { + setImmediate(next, null, uids.map(function (uid) { return cache.get(uid + ':' + groupName); })); } @@ -377,7 +377,7 @@ module.exports = function (Groups) { Groups.isMemberOfGroups = function (uid, groups, callback) { function getFromCache(next) { - process.nextTick(next, null, groups.map(function (groupName) { + setImmediate(next, null, groups.map(function (groupName) { return cache.get(uid + ':' + groupName); })); } diff --git a/src/middleware/admin.js b/src/middleware/admin.js index 3e36b74db9..f40e273317 100644 --- a/src/middleware/admin.js +++ b/src/middleware/admin.js @@ -16,17 +16,17 @@ module.exports = function (middleware) { middleware.admin.isAdmin = function (req, res, next) { winston.warn('[middleware.admin.isAdmin] deprecation warning, no need to use this from plugins!'); - if (!req.user) { - return controllers.helpers.notAllowed(req, res); - } - - user.isAdministrator(req.user.uid, function (err, isAdmin) { - if (err || isAdmin) { - return next(err); - } - - controllers.helpers.notAllowed(req, res); - }); + async.waterfall([ + function (next) { + user.isAdministrator(req.uid, next); + }, + function (isAdmin, next) { + if (!isAdmin) { + return controllers.helpers.notAllowed(req, res); + } + next(); + }, + ], next); }; middleware.admin.buildHeader = function (req, res, next) { diff --git a/src/middleware/index.js b/src/middleware/index.js index d4cdb0bfd2..06b336d97f 100644 --- a/src/middleware/index.js +++ b/src/middleware/index.js @@ -21,7 +21,7 @@ var controllers = { helpers: require('../controllers/helpers'), }; -var middleware = {}; +var middleware = module.exports; middleware.applyCSRF = csrf(); @@ -34,68 +34,6 @@ require('./maintenance')(middleware); require('./user')(middleware); require('./headers')(middleware); -middleware.authenticate = function (req, res, next) { - if (req.user) { - return next(); - } else if (plugins.hasListeners('action:middleware.authenticate')) { - return plugins.fireHook('action:middleware.authenticate', { - req: req, - res: res, - next: next, - }); - } - - controllers.helpers.notAllowed(req, res); -}; - -middleware.ensureSelfOrGlobalPrivilege = function (req, res, next) { - /* - The "self" part of this middleware hinges on you having used - middleware.exposeUid prior to invoking this middleware. - */ - async.waterfall([ - function (next) { - if (!req.uid) { - return setImmediate(next, null, false); - } - - if (req.uid === parseInt(res.locals.uid, 10)) { - return setImmediate(next, null, true); - } - user.isAdminOrGlobalMod(req.uid, next); - }, - function (isAdminOrGlobalMod, next) { - if (!isAdminOrGlobalMod) { - return controllers.helpers.notAllowed(req, res); - } - next(); - }, - ], next); -}; - -middleware.ensureSelfOrPrivileged = function (req, res, next) { - /* - The "self" part of this middleware hinges on you having used - middleware.exposeUid prior to invoking this middleware. - */ - if (req.user) { - if (parseInt(req.user.uid, 10) === parseInt(res.locals.uid, 10)) { - return next(); - } - - user.isPrivileged(req.uid, function (err, ok) { - if (err) { - return next(err); - } else if (ok) { - return next(); - } - controllers.helpers.notAllowed(req, res); - }); - } else { - controllers.helpers.notAllowed(req, res); - } -}; - middleware.pageView = function (req, res, next) { analytics.pageView({ ip: req.ip, @@ -226,6 +164,3 @@ middleware.processTimeagoLocales = function (req, res) { }); } }; - - -module.exports = middleware; diff --git a/src/middleware/user.js b/src/middleware/user.js index 85b5d6e808..6b0abaf5b0 100644 --- a/src/middleware/user.js +++ b/src/middleware/user.js @@ -6,14 +6,65 @@ var nconf = require('nconf'); var meta = require('../meta'); var user = require('../user'); var privileges = require('../privileges'); +var plugins = require('../plugins'); var controllers = { helpers: require('../controllers/helpers'), }; module.exports = function (middleware) { + middleware.authenticate = function (req, res, next) { + if (req.uid) { + return next(); + } + + if (plugins.hasListeners('action:middleware.authenticate')) { + return plugins.fireHook('action:middleware.authenticate', { + req: req, + res: res, + next: next, + }); + } + + controllers.helpers.notAllowed(req, res); + }; + + middleware.ensureSelfOrGlobalPrivilege = function (req, res, next) { + ensureSelfOrMethod(user.isAdminOrGlobalMod, req, res, next); + }; + + middleware.ensureSelfOrPrivileged = function (req, res, next) { + ensureSelfOrMethod(user.isPrivileged, req, res, next); + }; + + function ensureSelfOrMethod(method, req, res, next) { + /* + The "self" part of this middleware hinges on you having used + middleware.exposeUid prior to invoking this middleware. + */ + async.waterfall([ + function (next) { + if (!req.uid) { + return setImmediate(next, null, false); + } + + if (req.uid === parseInt(res.locals.uid, 10)) { + return setImmediate(next, null, true); + } + + method(req.uid, next); + }, + function (allowed, next) { + if (!allowed) { + return controllers.helpers.notAllowed(req, res); + } + next(); + }, + ], next); + } + middleware.checkGlobalPrivacySettings = function (req, res, next) { - if (!req.user && !!parseInt(meta.config.privateUserInfo, 10)) { + if (!req.uid && !!parseInt(meta.config.privateUserInfo, 10)) { return middleware.authenticate(req, res, next); } @@ -44,28 +95,28 @@ module.exports = function (middleware) { next(null, false); } }, - ], function (err, allowed) { - if (err || allowed) { - return next(err); - } - controllers.helpers.notAllowed(req, res); - }); + function (allowed) { + if (allowed) { + return next(); + } + controllers.helpers.notAllowed(req, res); + }, + ], next); }; middleware.redirectToAccountIfLoggedIn = function (req, res, next) { - if (req.session.forceLogin) { + if (req.session.forceLogin || !req.uid) { return next(); } - if (!req.user) { - return next(); - } - user.getUserField(req.user.uid, 'userslug', function (err, userslug) { - if (err) { - return next(err); - } - controllers.helpers.redirect(res, '/user/' + userslug); - }); + async.waterfall([ + function (next) { + user.getUserField(req.uid, 'userslug', next); + }, + function (userslug) { + controllers.helpers.redirect(res, '/user/' + userslug); + }, + ], next); }; middleware.redirectUidToUserslug = function (req, res, next) { @@ -73,71 +124,61 @@ module.exports = function (middleware) { if (!uid) { return next(); } - user.getUserField(uid, 'userslug', function (err, userslug) { - if (err || !userslug) { - return next(err); - } - - var path = req.path.replace(/^\/api/, '') + async.waterfall([ + function (next) { + user.getUserField(uid, 'userslug', next); + }, + function (userslug) { + if (!userslug) { + return next(); + } + var path = req.path.replace(/^\/api/, '') .replace('uid', 'user') .replace(uid, function () { return userslug; }); - controllers.helpers.redirect(res, path); - }); + controllers.helpers.redirect(res, path); + }, + ], next); }; middleware.isAdmin = function (req, res, next) { - if (!req.uid) { - return controllers.helpers.notAllowed(req, res); - } - - user.isAdministrator(req.uid, function (err, isAdmin) { - if (err) { - return next(err); - } - - if (isAdmin) { - user.hasPassword(req.uid, function (err, hasPassword) { - if (err) { - return next(err); - } - - if (!hasPassword) { - return next(); - } - - var loginTime = req.session.meta ? req.session.meta.datetime : 0; - if (loginTime && parseInt(loginTime, 10) > Date.now() - 3600000) { - var timeLeft = parseInt(loginTime, 10) - (Date.now() - 3600000); - if (timeLeft < 300000) { - req.session.meta.datetime += 300000; - } - - return next(); - } + async.waterfall([ + function (next) { + user.isAdministrator(req.uid, next); + }, + function (isAdmin, next) { + if (!isAdmin) { + return controllers.helpers.notAllowed(req, res); + } + user.hasPassword(req.uid, next); + }, + function (hasPassword, next) { + if (!hasPassword) { + return next(); + } - req.session.returnTo = req.path.replace(/^\/api/, ''); - req.session.forceLogin = 1; - if (res.locals.isAPI) { - res.status(401).json({}); - } else { - res.redirect(nconf.get('relative_path') + '/login'); + var loginTime = req.session.meta ? req.session.meta.datetime : 0; + if (loginTime && parseInt(loginTime, 10) > Date.now() - 3600000) { + var timeLeft = parseInt(loginTime, 10) - (Date.now() - 3600000); + if (timeLeft < 300000) { + req.session.meta.datetime += 300000; } - }); - return; - } - if (res.locals.isAPI) { - return controllers.helpers.notAllowed(req, res); - } + return next(); + } - middleware.buildHeader(req, res, function () { - controllers.helpers.notAllowed(req, res); - }); - }); + req.session.returnTo = req.path.replace(/^\/api/, ''); + req.session.forceLogin = 1; + if (res.locals.isAPI) { + res.status(401).json({}); + } else { + res.redirect(nconf.get('relative_path') + '/login'); + } + }, + ], next); }; middleware.requireUser = function (req, res, next) { - if (req.user) { + if (req.uid) { return next(); } diff --git a/test/controllers.js b/test/controllers.js index 956e5a8daf..c353effbcb 100644 --- a/test/controllers.js +++ b/test/controllers.js @@ -9,6 +9,7 @@ var db = require('./mocks/databasemock'); var categories = require('../src/categories'); var topics = require('../src/topics'); var user = require('../src/user'); +var groups = require('../src/groups'); var meta = require('../src/meta'); var translator = require('../src/translator'); @@ -19,6 +20,7 @@ describe('Controllers', function () { var fooUid; before(function (done) { + groups.resetCache(); async.series({ category: function (next) { categories.create({ @@ -355,7 +357,7 @@ describe('Controllers', function () { }); it('should load stylesheet.css', function (done) { - request(nconf.get('url') + '/stylesheet.css', function (err, res, body) { + request(nconf.get('url') + '/assets/stylesheet.css', function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 200); assert(body); @@ -364,7 +366,7 @@ describe('Controllers', function () { }); it('should load admin.css', function (done) { - request(nconf.get('url') + '/admin.css', function (err, res, body) { + request(nconf.get('url') + '/assets/admin.css', function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 200); assert(body); @@ -374,7 +376,7 @@ describe('Controllers', function () { it('should load nodebb.min.js', function (done) { - request(nconf.get('url') + '/nodebb.min.js', function (err, res, body) { + request(nconf.get('url') + '/assets/nodebb.min.js', function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 200); assert(body); @@ -383,7 +385,7 @@ describe('Controllers', function () { }); it('should load acp.min.js', function (done) { - request(nconf.get('url') + '/acp.min.js', function (err, res, body) { + request(nconf.get('url') + '/assets/acp.min.js', function (err, res, body) { assert.ifError(err); assert.equal(res.statusCode, 200); assert(body); @@ -491,7 +493,6 @@ describe('Controllers', function () { }); it('should load group details page', function (done) { - var groups = require('../src/groups'); groups.create({ name: 'group-details', description: 'Foobar!', @@ -800,6 +801,57 @@ describe('Controllers', function () { }); }); + it('should redirect to account page with logged in user', function (done) { + request(nconf.get('url') + '/api/login', { jar: jar, json: true }, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 308); + assert.equal(body, '/user/foo'); + done(); + }); + }); + + it('should 404 if uid is not a number', function (done) { + request(nconf.get('url') + '/api/uid/test', { json: true }, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 404); + done(); + }); + }); + + it('should redirect to userslug', function (done) { + request(nconf.get('url') + '/api/uid/' + fooUid, { json: true }, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 308); + assert.equal(body, '/user/foo'); + done(); + }); + }); + + it('should 404 if user does not exist', function (done) { + request(nconf.get('url') + '/api/uid/123123', { json: true }, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 404); + done(); + }); + }); + + it('should 401 if user is not logged in', function (done) { + request(nconf.get('url') + '/api/admin', { json: true }, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 401); + done(); + }); + }); + + it('should 403 if user is not admin', function (done) { + request(nconf.get('url') + '/api/admin', { jar: jar, json: true }, function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 403); + done(); + }); + }); + + it('should load /user/foo/posts', function (done) { request(nconf.get('url') + '/api/user/foo/posts', function (err, res, body) { assert.ifError(err); @@ -947,6 +999,35 @@ describe('Controllers', function () { done(); }); }); + + it('should return 401 if privateUserInfo is turned on', function (done) { + meta.config.privateUserInfo = 1; + request(nconf.get('url') + '/api/user/foo', { json: true }, function (err, res, body) { + meta.config.privateUserInfo = 0; + assert.ifError(err); + assert.equal(res.statusCode, 401); + assert.equal(body, 'not-authorized'); + done(); + }); + }); + + it('should return false if user can not edit user', function (done) { + user.create({ username: 'regularJoe', password: 'barbar' }, function (err) { + assert.ifError(err); + helpers.loginUser('regularJoe', 'barbar', function (err, jar) { + assert.ifError(err); + request(nconf.get('url') + '/api/user/foo/info', { jar: jar, json: true }, function (err, res) { + assert.ifError(err); + assert.equal(res.statusCode, 403); + request(nconf.get('url') + '/api/user/foo/edit', { jar: jar, json: true }, function (err, res) { + assert.ifError(err); + assert.equal(res.statusCode, 403); + done(); + }); + }); + }); + }); + }); }); describe('account follow page', function () { @@ -1120,7 +1201,7 @@ describe('Controllers', function () { }, }); - request(nconf.get('url') + '/users', { }, function (err, res, body) { + request(nconf.get('url') + '/users', { }, function (err, res) { plugins.loadedHooks['filter:router.page'] = []; assert.ifError(err); assert.equal(res.statusCode, 403); diff --git a/test/helpers/index.js b/test/helpers/index.js index 2b999246f7..e850ba23e2 100644 --- a/test/helpers/index.js +++ b/test/helpers/index.js @@ -3,6 +3,7 @@ var request = require('request'); var nconf = require('nconf'); var fs = require('fs'); +var winston = require('winston'); var myXhr = require('../mocks/newXhr'); var utils = require('../../public/src/utils'); @@ -105,7 +106,7 @@ helpers.uploadFile = function (uploadEndPoint, filePath, body, jar, csrf_token, return callback(err); } if (res.statusCode !== 200) { - console.log(body); + winston.error(body); } callback(null, res, body); });