From 6d8185c33e7863ac65096cf884e0f0e0fa617f66 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 23 Aug 2017 11:52:02 -0600 Subject: [PATCH] modified middleware to change AND -> OR, added mod to com pages --- middleware/authorization.js | 27 +++++++--- routes/api/users/index.js | 12 ++--- test/server/middleware/authorization.js | 47 ++++++++++++++++ test/server/routes/api/account/index.js | 72 ++++++++++--------------- 4 files changed, 101 insertions(+), 57 deletions(-) create mode 100644 test/server/middleware/authorization.js diff --git a/middleware/authorization.js b/middleware/authorization.js index d93913109..742b6983a 100644 --- a/middleware/authorization.js +++ b/middleware/authorization.js @@ -10,18 +10,29 @@ const debug = require('debug')('talk:middleware:authorization'); const ErrNotAuthorized = require('../errors').ErrNotAuthorized; /** - * has returns true if the user has all the roles specified, otherwise it will - * return false. + * has returns true if the user has at least one of the roles specified, + * otherwise it will return false. * @param {Object} user the user to check for roles - * @param {Array} roles all the roles that a user must have - * @return {Boolean} true if the user has all the roles required, false + * @param {Array} roles roles to check if the user has + * @return {Boolean} true if the user has some the roles required, false * otherwise */ -authorization.has = (user, ...roles) => roles.every((role) => { +authorization.has = (user, ...roles) => { - // TODO: remove toUpperCase once we've migrated over the roles. - return user.roles.indexOf(role.toUpperCase()) >= 0; -}); + // If no user is specified, then they can't have the roles you want! + if (!user || !user.roles) { + return false; + } + + // If no roles are specified, then any user has the roles you want! + if (!roles || roles.length === 0) { + return true; + } + + // If there's a user, and roles, then check to see that the user has at least + // one of those roles. + return roles.some((role) => user.roles.includes(role)); +}; /** * needed is a connect middleware layer that ensures that all requests coming diff --git a/routes/api/users/index.js b/routes/api/users/index.js index 60c568253..41b2771ca 100644 --- a/routes/api/users/index.js +++ b/routes/api/users/index.js @@ -10,7 +10,7 @@ const { } = require('../../../config'); // get a list of users. -router.get('/', authorization.needed('ADMIN'), async (req, res, next) => { +router.get('/', authorization.needed('ADMIN', 'MODERATOR'), async (req, res, next) => { const { value = '', field = 'created_at', @@ -44,7 +44,7 @@ router.get('/', authorization.needed('ADMIN'), async (req, res, next) => { }); -router.post('/:user_id/role', authorization.needed('ADMIN'), async (req, res, next) => { +router.post('/:user_id/role', authorization.needed('ADMIN', 'MODERATOR'), async (req, res, next) => { try { await UsersService.addRoleToUser(req.params.user_id, req.body.role); res.status(204).end(); @@ -54,7 +54,7 @@ router.post('/:user_id/role', authorization.needed('ADMIN'), async (req, res, ne }); // update the status of a user -router.post('/:user_id/status', authorization.needed('ADMIN'), async (req, res, next) => { +router.post('/:user_id/status', authorization.needed('ADMIN', 'MODERATOR'), async (req, res, next) => { let {status} = req.body; try { @@ -74,7 +74,7 @@ router.post('/:user_id/status', authorization.needed('ADMIN'), async (req, res, } }); -router.post('/:user_id/username-enable', authorization.needed('ADMIN'), async (req, res, next) => { +router.post('/:user_id/username-enable', authorization.needed('ADMIN', 'MODERATOR'), async (req, res, next) => { try { await UsersService.toggleNameEdit(req.params.user_id, true); res.status(204).end(); @@ -83,7 +83,7 @@ router.post('/:user_id/username-enable', authorization.needed('ADMIN'), async (r } }); -router.post('/:user_id/email', authorization.needed('ADMIN'), async (req, res, next) => { +router.post('/:user_id/email', authorization.needed('ADMIN', 'MODERATOR'), async (req, res, next) => { try { let user = await UsersService.findById(req.params.user_id); @@ -189,7 +189,7 @@ router.post('/resend-verify', async (req, res, next) => { }); // trigger an email confirmation re-send from the admin panel -router.post('/:user_id/email/confirm', authorization.needed('ADMIN'), async (req, res, next) => { +router.post('/:user_id/email/confirm', authorization.needed('ADMIN', 'MODERATOR'), async (req, res, next) => { const { user_id } = req.params; diff --git a/test/server/middleware/authorization.js b/test/server/middleware/authorization.js new file mode 100644 index 000000000..7966cffa9 --- /dev/null +++ b/test/server/middleware/authorization.js @@ -0,0 +1,47 @@ +const chai = require('chai'); +const expect = chai.expect; + +const authz = require('../../../middleware/authorization'); + +describe('middleware.authorization', () => { + describe('#has', () => { + it('allows if no roles are specified', () => { + expect(authz.has({roles: []})).to.be.true; + }); + it('allows if the correct roles are met', () => { + expect(authz.has({roles: ['ADMIN']}, 'ADMIN', 'MODERATOR')).to.be.true; + }); + it('disallows if the role required is missing', () => { + expect(authz.has({roles: []}, 'ADMIN', 'MODERATOR')).to.be.false; + }); + }); + + describe('#needed', () => { + let needed = (...roles) => { + let middleware = authz.needed(...roles); + + return middleware[middleware.length - 1]; + }; + + it('allows if no roles are specified', () => { + needed()({user: {roles: []}}, {}, (err) => { + expect(err).to.be.undefined; + }); + }); + it('allows if the correct roles are met', () => { + needed()({user: {roles: ['ADMIN']}}, {}, (err) => { + expect(err).to.be.undefined; + }); + }); + it('disallows if the role required is missing', () => { + needed('ADMIN', 'MODERATOR')({user: {roles: []}}, {}, (err) => { + expect(err).to.not.be.undefined; + }); + }); + it('disallows if there is no user on the request', () => { + needed('ADMIN', 'MODERATOR')({}, {}, (err) => { + expect(err).to.not.be.undefined; + }); + }); + }); +}); diff --git a/test/server/routes/api/account/index.js b/test/server/routes/api/account/index.js index 8d4e0057a..3195db0e5 100644 --- a/test/server/routes/api/account/index.js +++ b/test/server/routes/api/account/index.js @@ -1,71 +1,57 @@ const passport = require('../../../passport'); const app = require('../../../../../app'); + const chai = require('chai'); +chai.use(require('chai-as-promised')); +chai.use(require('chai-http')); const expect = chai.expect; +const UsersService = require('../../../../../services/users'); const SettingsService = require('../../../../../services/settings'); const settings = {id: '1', moderation: 'PRE', wordlist: {banned: ['bad words'], suspect: ['suspect words']}}; -// Setup chai. -chai.should(); -chai.use(require('chai-http')); - -const UsersService = require('../../../../../services/users'); - describe('/api/v1/account/username', () => { let mockUser; - - beforeEach(() => SettingsService.init(settings).then(() => { - return UsersService.createLocalUser('ana@gmail.com', '123321123', 'Ana'); - }) - .then((user) => { - mockUser = user; - })); + beforeEach(async () => { + await SettingsService.init(settings); + mockUser = await UsersService.createLocalUser('ana@gmail.com', '123321123', 'Ana'); + }); describe('#put', () => { - it('it should enable a user to edit their username if canEditName is enabled', () => { - return chai.request(app) + it('it should enable a user to edit their username if canEditName is enabled', async () => { + await chai.request(app) .post(`/api/v1/users/${mockUser.id}/username-enable`) - .set(passport.inject({id: '456', roles: ['ADMIN']})) - .then(() => chai.request(app) + .set(passport.inject({id: '456', roles: ['ADMIN']})); + + const res = await chai.request(app) .put('/api/v1/account/username') .set(passport.inject({id: mockUser.id, roles: []})) - .send({username: 'MojoJojo'})) - .then((res) => { - expect(res).to.have.status(204); - }); + .send({username: 'MojoJojo'}); + + expect(res).to.have.status(204); }); - it('it should return an error if the wrong user tries to edit a username', (done) => { - chai.request(app) + it('it should return an error if the wrong user tries to edit a username', async () => { + await chai.request(app) .post(`/api/v1/users/${mockUser.id}/username-enable`) - .set(passport.inject({id: '456', roles: ['ADMIN']})) - .then(() => chai.request(app) + .set(passport.inject({id: '456', roles: ['ADMIN']})); + + let res = chai.request(app) .put('/api/v1/account/username') .set(passport.inject({id: 'wrongid', roles: []})) - .send({username: 'MojoJojo'})) - .then(() => { - done(new Error('Expected Error')); - }) - .catch((err) => { - expect(err).to.be.ok; - done(); - }); + .send({username: 'MojoJojo'}); + + return expect(res).to.eventually.be.rejected; }); - it('it should return an error when the user tries to edit their username if canEditName is disabled', (done) => { - chai.request(app) + it('it should return an error when the user tries to edit their username if canEditName is disabled', () => { + let res = chai.request(app) .put('/api/v1/account/username') .set(passport.inject({id: mockUser.id, roles: []})) - .send({username: 'MojoJojo'}) - .then(() => { - done(new Error('Expected Error')); - }) - .catch((err) => { - expect(err).to.be.ok; - done(); - }); + .send({username: 'MojoJojo'}); + + return expect(res).to.eventually.be.rejected; }); }); });