diff --git a/src/middleware/admin.js b/src/middleware/admin.js index 67408905ca..bfce0c2d83 100644 --- a/src/middleware/admin.js +++ b/src/middleware/admin.js @@ -117,7 +117,7 @@ middleware.checkPrivileges = helpers.try(async (req, res, next) => { } // Otherwise, check for privilege based on page (if not in mapping, deny access) - const path = req.path.replace(/^(\/api)?\/admin\/?/g, ''); + const path = req.path.replace(/^(\/api)?(\/v3)?\/admin\/?/g, ''); if (path) { const privilege = privileges.admin.resolve(path); if (!await privileges.admin.can(privilege, req.uid)) { diff --git a/src/privileges/admin.js b/src/privileges/admin.js index 141944ad02..e6139f2203 100644 --- a/src/privileges/admin.js +++ b/src/privileges/admin.js @@ -61,13 +61,13 @@ privsAdmin.routeMap = { 'extend/widgets': 'admin:settings', 'extend/rewards': 'admin:settings', }; -privsAdmin.routeRegexpMap = { - '^manage/categories/\\d+': 'admin:categories', - '^manage/privileges/(\\d+|admin)': 'admin:privileges', - '^manage/groups/.+$': 'admin:groups', - '^settings/[\\w\\-]+$': 'admin:settings', - '^appearance/[\\w]+$': 'admin:settings', - '^plugins/[\\w\\-]+$': 'admin:settings', +privsAdmin.routePrefixMap = { + 'manage/categories/': 'admin:categories', + 'manage/privileges/': 'admin:privileges', + 'manage/groups/': 'admin:groups', + 'settings/': 'admin:settings', + 'appearance/': 'admin:settings', + 'plugins/': 'admin:settings', }; // Mapping for socket call methods to a privilege @@ -120,16 +120,8 @@ privsAdmin.resolve = (path) => { return privsAdmin.routeMap[path]; } - let privilege; - Object.keys(privsAdmin.routeRegexpMap).forEach((regexp) => { - if (!privilege) { - if (new RegExp(regexp).test(path)) { - privilege = privsAdmin.routeRegexpMap[regexp]; - } - } - }); - - return privilege; + const found = Object.entries(privsAdmin.routePrefixMap).find(entry => path.startsWith(entry[0])); + return found ? found[1] : undefined; }; privsAdmin.list = async function (uid) { diff --git a/test/controllers-admin.js b/test/controllers-admin.js index cee00076c7..bcee1e80ab 100644 --- a/test/controllers-admin.js +++ b/test/controllers-admin.js @@ -811,41 +811,71 @@ describe('Admin Controllers', () => { userJar = (await helpers.loginUser('regularjoe', 'barbar')).jar; }); - it('should allow normal user access to admin pages', async function () { - this.timeout(50000); - function makeRequest(url) { - return new Promise((resolve, reject) => { - request(url, { jar: userJar, json: true }, (err, res, body) => { - if (err) reject(err); - else resolve(res); + describe('routeMap parsing', () => { + it('should allow normal user access to admin pages', async function () { + this.timeout(50000); + function makeRequest(url) { + return new Promise((resolve, reject) => { + request(url, { jar: userJar, json: true }, (err, res, body) => { + if (err) reject(err); + else resolve(res); + }); }); - }); - } - for (const route of Object.keys(privileges.admin.routeMap)) { - /* eslint-disable no-await-in-loop */ - await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); - let res = await makeRequest(`${nconf.get('url')}/api/admin/${route}`); - assert.strictEqual(res.statusCode, 403); - - await privileges.admin.give([privileges.admin.routeMap[route]], uid); - res = await makeRequest(`${nconf.get('url')}/api/admin/${route}`); - assert.strictEqual(res.statusCode, 200); - - await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); - } - - for (const route of Object.keys(privileges.admin.routeMap)) { - /* eslint-disable no-await-in-loop */ - await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); - let res = await makeRequest(`${nconf.get('url')}/api/admin`); - assert.strictEqual(res.statusCode, 403); - - await privileges.admin.give([privileges.admin.routeMap[route]], uid); - res = await makeRequest(`${nconf.get('url')}/api/admin`); - assert.strictEqual(res.statusCode, 200); + } + for (const route of Object.keys(privileges.admin.routeMap)) { + /* eslint-disable no-await-in-loop */ + await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); + let res = await makeRequest(`${nconf.get('url')}/api/admin/${route}`); + assert.strictEqual(res.statusCode, 403); + + await privileges.admin.give([privileges.admin.routeMap[route]], uid); + res = await makeRequest(`${nconf.get('url')}/api/admin/${route}`); + assert.strictEqual(res.statusCode, 200); + + await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); + } + + for (const route of Object.keys(privileges.admin.routeMap)) { + /* eslint-disable no-await-in-loop */ + await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); + let res = await makeRequest(`${nconf.get('url')}/api/admin`); + assert.strictEqual(res.statusCode, 403); + + await privileges.admin.give([privileges.admin.routeMap[route]], uid); + res = await makeRequest(`${nconf.get('url')}/api/admin`); + assert.strictEqual(res.statusCode, 200); + + await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); + } + }); + }); - await privileges.admin.rescind([privileges.admin.routeMap[route]], uid); - } + describe('routePrefixMap parsing', () => { + it('should allow normal user access to admin pages', async () => { + // this.timeout(50000); + function makeRequest(url) { + return new Promise((resolve, reject) => { + process.stdout.write(`calling ${url} `); + request(url, { jar: userJar, json: true }, (err, res, body) => { + process.stdout.write(`got ${res.statusCode}\n`); + if (err) reject(err); + else resolve(res); + }); + }); + } + for (const route of Object.keys(privileges.admin.routePrefixMap)) { + /* eslint-disable no-await-in-loop */ + await privileges.admin.rescind([privileges.admin.routePrefixMap[route]], uid); + let res = await makeRequest(`${nconf.get('url')}/api/admin/${route}foobar/derp`); + assert.strictEqual(res.statusCode, 403); + + await privileges.admin.give([privileges.admin.routePrefixMap[route]], uid); + res = await makeRequest(`${nconf.get('url')}/api/admin/${route}foobar/derp`); + assert.strictEqual(res.statusCode, 404); + + await privileges.admin.rescind([privileges.admin.routePrefixMap[route]], uid); + } + }); }); }); }); diff --git a/test/middleware.js b/test/middleware.js index 7ac48616a0..e6ee7818db 100644 --- a/test/middleware.js +++ b/test/middleware.js @@ -8,10 +8,12 @@ const groups = require('../src/groups'); describe('Middlewares', () => { let adminUid; + before(async () => { adminUid = await user.create({ username: 'admin', password: '123456' }); await groups.join('administrators', adminUid); }); + describe('expose', () => { it('should expose res.locals.isAdmin = false', (done) => { const resMock = { locals: {} };