refactor: remove usage of middlewares

Specifically, middleware.isAdmin|exposePrivilegeSet|exposePrivileges
NodeBB · Oct 29, 2020 · 266d758 · 266d758
1 parent bff53de
commit 266d758
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 19 deletions.
diff --git a/src/controllers/write/admin.js b/src/controllers/write/admin.js
@@ -1,13 +1,16 @@
 'use strict';
 
 const meta = require('../../meta');
+const privileges = require('../../privileges');
 
 const helpers = require('../helpers');
 
 const Admin = module.exports;
 
 Admin.updateSetting = async (req, res) => {
-	if (!res.locals.privileges['admin:settings']) {
+	const ok = await privileges.admin.can('admin:settings', req.uid);
+
+	if (!ok) {
 		return helpers.formatApiResponse(403, res);
 	}
 

diff --git a/src/controllers/write/categories.js b/src/controllers/write/categories.js
@@ -1,18 +1,30 @@
 'use strict';
 
+const privileges = require('../../privileges');
 const categories = require('../../categories');
 const api = require('../../api');
 
 const helpers = require('../helpers');
 
 const Categories = module.exports;
 
+const hasAdminPrivilege = async (uid) => {
+	const ok = await privileges.admin.can(`admin:categories`, uid);
+	if (!ok) {
+		throw new Error('[[error:no-privileges]]');
+	}
+};
+
 Categories.create = async (req, res) => {
+	await hasAdminPrivilege(req.uid);
+
 	const response = await api.categories.create(req, req.body);
 	helpers.formatApiResponse(200, res, response);
 };
 
 Categories.update = async (req, res) => {
+	await hasAdminPrivilege(req.uid);
+
 	const payload = {};
 	payload[req.params.cid] = req.body;
 	await api.categories.update(req, payload);
@@ -21,6 +33,8 @@ Categories.update = async (req, res) => {
 };
 
 Categories.delete = async (req, res) => {
+	await hasAdminPrivilege(req.uid);
+
 	await api.categories.delete(req, { cid: req.params.cid });
 	helpers.formatApiResponse(200, res);
 };
diff --git a/src/controllers/write/users.js b/src/controllers/write/users.js
@@ -2,13 +2,22 @@
 
 const api = require('../../api');
 const meta = require('../../meta');
+const privileges = require('../../privileges');
 const utils = require('../../utils');
 
 const helpers = require('../helpers');
 
 const Users = module.exports;
 
+const hasAdminPrivilege = async (uid, privilege) => {
+	const ok = await privileges.admin.can(`admin:${privilege}`, uid);
+	if (!ok) {
+		throw new Error('[[error:no-privileges]]');
+	}
+};
+
 Users.create = async (req, res) => {
+	await hasAdminPrivilege(req.uid, 'users');
 	const userObj = await api.users.create(req, req.body);
 	helpers.formatApiResponse(200, res, userObj);
 };
@@ -24,6 +33,7 @@ Users.delete = async (req, res) => {
 };
 
 Users.deleteMany = async (req, res) => {
+	await hasAdminPrivilege(req.uid, 'users');
 	await api.users.deleteMany(req, req.body);
 	helpers.formatApiResponse(200, res);
 };
@@ -49,19 +59,20 @@ Users.unfollow = async (req, res) => {
 };
 
 Users.ban = async (req, res) => {
+	await hasAdminPrivilege(req.uid, 'users');
 	await api.users.ban(req, { ...req.body, uid: req.params.uid });
 	helpers.formatApiResponse(200, res);
 };
 
 Users.unban = async (req, res) => {
+	await hasAdminPrivilege(req.uid, 'users');
 	await api.users.unban(req, { ...req.body, uid: req.params.uid });
 	helpers.formatApiResponse(200, res);
 };
 
 Users.generateToken = async (req, res) => {
-	if (!res.locals.privileges['admin:settings']) {
-		return helpers.formatApiResponse(403, res, new Error('[[error:no-privileges]]'));
-	} else if (parseInt(req.params.uid, 10) !== parseInt(req.user.uid, 10)) {
+	await hasAdminPrivilege(req.uid, 'settings');
+	if (parseInt(req.params.uid, 10) !== parseInt(req.user.uid, 10)) {
 		return helpers.formatApiResponse(401, res);
 	}
 
@@ -80,9 +91,8 @@ Users.generateToken = async (req, res) => {
 };
 
 Users.deleteToken = async (req, res) => {
-	if (!res.locals.privileges['admin:settings']) {
-		return helpers.formatApiResponse(403, res, new Error('[[error:no-privileges]]'));
-	} else if (parseInt(req.params.uid, 10) !== parseInt(req.user.uid, 10)) {
+	await hasAdminPrivilege(req.uid, 'settings');
+	if (parseInt(req.params.uid, 10) !== parseInt(req.user.uid, 10)) {
 		return helpers.formatApiResponse(401, res);
 	}
 

diff --git a/src/middleware/user.js b/src/middleware/user.js
@@ -201,6 +201,7 @@ module.exports = function (middleware) {
 
 	middleware.isAdmin = helpers.try(async function isAdmin(req, res, next) {
 		const isAdmin = await user.isAdministrator(req.uid);
+
 		if (!isAdmin) {
 			return controllers.helpers.notAllowed(req, res);
 		}

diff --git a/src/privileges/admin.js b/src/privileges/admin.js
@@ -158,8 +158,11 @@ module.exports = function (privileges) {
 	};
 
 	privileges.admin.can = async function (privilege, uid) {
-		const isUserAllowedTo = await helpers.isUserAllowedTo(privilege, uid, [0]);
-		return isUserAllowedTo[0];
+		const [isUserAllowedTo, isAdministrator] = await Promise.all([
+			helpers.isUserAllowedTo(privilege, uid, [0]),
+			user.isAdministrator(uid),
+		]);
+		return isAdministrator || isUserAllowedTo[0];
 	};
 
 	// privileges.admin.canGroup = async function (privilege, groupName) {

diff --git a/src/routes/write/admin.js b/src/routes/write/admin.js
@@ -10,7 +10,7 @@ const setupApiRoute = routeHelpers.setupApiRoute;
 module.exports = function () {
 	const middlewares = [middleware.authenticate];
 
-	setupApiRoute(router, 'put', '/settings/:setting', [...middlewares, middleware.checkRequired.bind(null, ['value']), middleware.exposePrivilegeSet], controllers.write.admin.updateSetting);
+	setupApiRoute(router, 'put', '/settings/:setting', [...middlewares, middleware.checkRequired.bind(null, ['value'])], controllers.write.admin.updateSetting);
 
 	return router;
 };
diff --git a/src/routes/write/categories.js b/src/routes/write/categories.js
@@ -10,9 +10,9 @@ const setupApiRoute = routeHelpers.setupApiRoute;
 module.exports = function () {
 	const middlewares = [middleware.authenticate];
 
-	setupApiRoute(router, 'post', '/', [...middlewares, middleware.checkRequired.bind(null, ['name']), middleware.isAdmin], controllers.write.categories.create);
-	setupApiRoute(router, 'put', '/:cid', [...middlewares, middleware.isAdmin], controllers.write.categories.update);
-	setupApiRoute(router, 'delete', '/:cid', [...middlewares, middleware.isAdmin], controllers.write.categories.delete);
+	setupApiRoute(router, 'post', '/', [...middlewares, middleware.checkRequired.bind(null, ['name'])], controllers.write.categories.create);
+	setupApiRoute(router, 'put', '/:cid', [...middlewares], controllers.write.categories.update);
+	setupApiRoute(router, 'delete', '/:cid', [...middlewares], controllers.write.categories.delete);
 
 	return router;
 };
diff --git a/src/routes/write/users.js b/src/routes/write/users.js
@@ -15,8 +15,8 @@ function guestRoutes() {
 function authenticatedRoutes() {
 	const middlewares = [middleware.authenticate];
 
-	setupApiRoute(router, 'post', '/', [...middlewares, middleware.checkRequired.bind(null, ['username']), middleware.isAdmin], controllers.write.users.create);
-	setupApiRoute(router, 'delete', '/', [...middlewares, middleware.checkRequired.bind(null, ['uids']), middleware.isAdmin, middleware.exposePrivileges], controllers.write.users.deleteMany);
+	setupApiRoute(router, 'post', '/', [...middlewares, middleware.checkRequired.bind(null, ['username'])], controllers.write.users.create);
+	setupApiRoute(router, 'delete', '/', [...middlewares, middleware.checkRequired.bind(null, ['uids'])], controllers.write.users.deleteMany);
 
 	setupApiRoute(router, 'put', '/:uid', [...middlewares, middleware.assert.user], controllers.write.users.update);
 	setupApiRoute(router, 'delete', '/:uid', [...middlewares, middleware.assert.user, middleware.exposePrivileges], controllers.write.users.delete);
@@ -28,11 +28,11 @@ function authenticatedRoutes() {
 	setupApiRoute(router, 'put', '/:uid/follow', [...middlewares, middleware.assert.user], controllers.write.users.follow);
 	setupApiRoute(router, 'delete', '/:uid/follow', [...middlewares, middleware.assert.user], controllers.write.users.unfollow);
 
-	setupApiRoute(router, 'put', '/:uid/ban', [...middlewares, middleware.assert.user, middleware.exposePrivileges], controllers.write.users.ban);
-	setupApiRoute(router, 'delete', '/:uid/ban', [...middlewares, middleware.assert.user, middleware.exposePrivileges], controllers.write.users.unban);
+	setupApiRoute(router, 'put', '/:uid/ban', [...middlewares, middleware.assert.user], controllers.write.users.ban);
+	setupApiRoute(router, 'delete', '/:uid/ban', [...middlewares, middleware.assert.user], controllers.write.users.unban);
 
-	setupApiRoute(router, 'post', '/:uid/tokens', [...middlewares, middleware.assert.user, middleware.exposePrivilegeSet], controllers.write.users.generateToken);
-	setupApiRoute(router, 'delete', '/:uid/tokens/:token', [...middlewares, middleware.assert.user, middleware.exposePrivilegeSet], controllers.write.users.deleteToken);
+	setupApiRoute(router, 'post', '/:uid/tokens', [...middlewares, middleware.assert.user], controllers.write.users.generateToken);
+	setupApiRoute(router, 'delete', '/:uid/tokens/:token', [...middlewares, middleware.assert.user], controllers.write.users.deleteToken);
 }
 
 module.exports = function () {