diff --git a/packages/app/obojobo-express/__tests__/express_validators.test.js b/packages/app/obojobo-express/__tests__/express_validators.test.js index 1bed32b95a..23afbe2fc2 100644 --- a/packages/app/obojobo-express/__tests__/express_validators.test.js +++ b/packages/app/obojobo-express/__tests__/express_validators.test.js @@ -519,6 +519,28 @@ describe('current user middleware', () => { }) }) + // requireCanViewAdminPage + + test('requireCanViewAdminPage calls next and has no validation errors', () => { + mockUser.hasPermission = perm => perm === 'canViewAdminPage' + mockReq.requireCurrentUser = jest.fn().mockResolvedValue(mockUser) + return Validators.requireCanViewAdminPage(mockReq, mockRes, mockNext).then(() => { + expect(mockNext).toHaveBeenCalledTimes(1) + expect(mockRes.notAuthorized).toHaveBeenCalledTimes(0) + expect(mockReq._validationErrors).toBeUndefined() + }) + }) + + test('requireCanViewAdminPage doesnt call next and has errors', () => { + mockUser.hasPermission = () => false + mockReq.requireCurrentUser = jest.fn().mockResolvedValue(mockUser) + return Validators.requireCanViewAdminPage(mockReq, mockRes, mockNext).then(() => { + expect(mockNext).toHaveBeenCalledTimes(0) + expect(mockRes.notAuthorized).toHaveBeenCalledTimes(1) + expect(mockReq._validationErrors).toBeUndefined() + }) + }) + // checkValidationRules test('checkValidationRules calls next with no errors', () => { diff --git a/packages/app/obojobo-express/server/config/permission_groups.json b/packages/app/obojobo-express/server/config/permission_groups.json index 4ecb9e960c..23242fb2e4 100644 --- a/packages/app/obojobo-express/server/config/permission_groups.json +++ b/packages/app/obojobo-express/server/config/permission_groups.json @@ -39,7 +39,8 @@ ], "canViewAsStudent": ["Learner", "urn:lti:role:ims/lis/Learner"], "canViewStatsPage": [], - "canViewSystemStats": [] + "canViewSystemStats": [], + "canViewAdminPage": [] }, "test": { "canDoThing": ["roleName"] diff --git a/packages/app/obojobo-express/server/express_validators.js b/packages/app/obojobo-express/server/express_validators.js index 9b84e8c613..bc40fac153 100644 --- a/packages/app/obojobo-express/server/express_validators.js +++ b/packages/app/obojobo-express/server/express_validators.js @@ -120,6 +120,9 @@ exports.requireCanViewStatsPage = (req, res, next) => exports.requireCanViewSystemStats = (req, res, next) => requireCurrentUser(req, res, next, 'canViewSystemStats') +exports.requireCanViewAdminPage = (req, res, next) => + requireCurrentUser(req, res, next, 'canViewAdminPage') + exports.checkValidationRules = (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { diff --git a/packages/app/obojobo-express/server/obo_express_dev.js b/packages/app/obojobo-express/server/obo_express_dev.js index c608054d65..4698df6825 100644 --- a/packages/app/obojobo-express/server/obo_express_dev.js +++ b/packages/app/obojobo-express/server/obo_express_dev.js @@ -16,7 +16,8 @@ const POSSIBLE_PERMS = [ 'canDeleteDrafts', 'canPreviewDrafts', 'canViewStatsPage', - 'canViewSystemStats' + 'canViewSystemStats', + 'canViewAdminPage' ] // Normally the query running in User.saveOrCreate would auto-fill the new user's id, diff --git a/packages/app/obojobo-repository/index.js b/packages/app/obojobo-repository/index.js index c63c9b5808..d635722e81 100644 --- a/packages/app/obojobo-repository/index.js +++ b/packages/app/obojobo-repository/index.js @@ -6,6 +6,7 @@ module.exports = { repository: 'shared/components/pages/page-library.jsx', dashboard: 'shared/components/pages/page-dashboard-client.jsx', stats: 'shared/components/pages/page-stats-client.jsx', + admin: 'shared/components/pages/page-admin-client.jsx', homepage: 'shared/components/pages/page-homepage.jsx', 'page-module': 'shared/components/pages/page-module-client.jsx', 'page-library': 'shared/components/pages/page-library-client.jsx' diff --git a/packages/app/obojobo-repository/server/index.js b/packages/app/obojobo-repository/server/index.js index 95f15a494a..1556e965ec 100644 --- a/packages/app/obojobo-repository/server/index.js +++ b/packages/app/obojobo-repository/server/index.js @@ -22,6 +22,7 @@ app.on('mount', app => { app.use('/', require('./routes/dashboard')) app.use('/', require('./routes/library')) app.use('/', require('./routes/stats')) + app.use('/', require('./routes/admin')) // register the event listeners require('./events') diff --git a/packages/app/obojobo-repository/server/models/admin_interface.js b/packages/app/obojobo-repository/server/models/admin_interface.js new file mode 100644 index 0000000000..ba1c7677df --- /dev/null +++ b/packages/app/obojobo-repository/server/models/admin_interface.js @@ -0,0 +1,96 @@ +const db = require('obojobo-express/server/db') +const logger = require('obojobo-express/server/logger') + +const User = require('obojobo-express/server/models/user') + +const { PERMS_PER_ROLE } = require('../../shared/util/implicit-perms') + +class AdminInterface { + static addPermission(userId, permission) { + return new Promise((resolve, reject) => { + User.fetchById(userId) + .then(u => { + let perms = u.perms + if (perms.includes(permission)) return resolve(u) + + perms = [...u.perms, permission] + + const allRolePerms = new Set() + + u.roles.forEach(role => { + PERMS_PER_ROLE[role].forEach(perm => allRolePerms.add(perm)) + }) + + perms = perms.filter(p => !allRolePerms.has(p)) + + const dedupedPerms = new Set([...u.perms, ...perms]) + u.perms = [...dedupedPerms] + + // Then add new permission (if it is new) + this._updateUserPerms(userId, perms) + .then(() => resolve(u)) + .catch(() => { + reject(logger.logError(`AdminInterface error adding permission`)) + }) + }) + .catch(() => { + reject(logger.logError(`AdminInterface error finding user with id ${userId}`)) + }) + }) + } + + static removePermission(userId, permission) { + return new Promise((resolve, reject) => { + User.fetchById(userId) + .then(u => { + const allRolePerms = new Set() + + u.roles.forEach(role => { + PERMS_PER_ROLE[role].forEach(perm => allRolePerms.add(perm)) + }) + + const perms = u.perms.filter(p => !allRolePerms.has(p)) + + const ix = perms.indexOf(permission) + if (ix === -1) return resolve(u) + perms.splice(ix, 1) + + // add any remaining perms to the 'allRolePerms' set so we can use it to set the user's new combined perms + perms.forEach(p => allRolePerms.add(p)) + + u.perms = [...allRolePerms] + + // Then remove permission (if present) + this._updateUserPerms(userId, perms) + .then(() => resolve(u)) + .catch(() => { + reject(logger.logError(`AdminInterface error removing permission`)) + }) + }) + .catch(() => { + reject(logger.logError(`AdminInterface error finding user with id ${userId}`)) + }) + }) + } + + static _updateUserPerms(userId, perms) { + return new Promise((resolve, reject) => { + db.oneOrNone( + ` + INSERT INTO user_perms (user_id, perms) + VALUES ($[userId], $[perms]) + ON CONFLICT (user_id) + DO UPDATE SET perms = $[perms] + WHERE user_perms.user_id = $[userId] + `, + { userId, perms } + ) + .then(() => resolve(userId)) + .catch(error => { + reject(logger.logError('AdminInterface _updateUserPerms error', error)) + }) + }) + } +} + +module.exports = AdminInterface diff --git a/packages/app/obojobo-repository/server/models/admin_interface.test.js b/packages/app/obojobo-repository/server/models/admin_interface.test.js new file mode 100644 index 0000000000..01341aa48e --- /dev/null +++ b/packages/app/obojobo-repository/server/models/admin_interface.test.js @@ -0,0 +1,183 @@ +jest.mock('obojobo-express/server/db') +jest.mock('obojobo-express/server/logger') +jest.mock('obojobo-express/server/models/user') +jest.mock('../../shared/util/implicit-perms', () => ({ + PERMS_PER_ROLE: { + mockRole: ['mockExistingPermission'] + } +})) + +const db = require('obojobo-express/server/db') +const logger = require('obojobo-express/server/logger') + +const User = require('obojobo-express/server/models/user') + +const AdminInterface = require('./admin_interface') + +describe('AdminInterface Model', () => { + let expectedResponseUser + + beforeEach(() => { + jest.resetModules() + jest.resetAllMocks() + + expectedResponseUser = { + perms: [], + roles: [] + } + + // provide this by defualt, override in individual tests if necessary + // if every test ends up overriding this, just remove this one + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + }) + + test('addPermission does nothing if trying to add a permission the given user already has', () => { + expect.assertions(2) + + // set the user's permissions such that they already have the one we're trying to give them + // ideally we could check implicit and explicit perms separately, but they're added to a single + // array inside the User model so our only option is to check the one location + expectedResponseUser.perms = ['someExistingPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.addPermission(5, 'someExistingPermission').then(u => { + expect(u).toEqual(expectedResponseUser) + expect(db.oneOrNone).not.toHaveBeenCalled() + }) + }) + + test('addPermission only saves explicitly granted permissions', () => { + expect.assertions(3) + + db.oneOrNone.mockResolvedValueOnce(5) + + // 'mockRole' will account for the 'mockExistingPermission' perm below + expectedResponseUser.roles = ['mockRole'] + expectedResponseUser.perms = ['someExistingPermission', 'mockExistingPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.addPermission(5, 'someNewPermission').then(u => { + expect(u).toEqual({ + ...expectedResponseUser, + perms: ['someExistingPermission', 'mockExistingPermission', 'someNewPermission'] + }) + expect(db.oneOrNone).toHaveBeenCalledTimes(1) + // first argument to db function is the query string, no need to check that + expect(db.oneOrNone.mock.calls[0][1]).toEqual({ + userId: 5, + // since 'mockExistingPermission' is a perm-based/implicit perm, + // it should not have been saved explicitly + perms: ['someExistingPermission', 'someNewPermission'] + }) + }) + }) + + test('addPermission catches error when fetching user with invalid id', () => { + User.fetchById = jest.fn().mockRejectedValueOnce('mock-error') + + expect.hasAssertions() + + return AdminInterface.addPermission(123456, 'someNewPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface error finding user with id 123456' + ) + }) + }) + + test('addPermission catches error when updating user perms', () => { + expect.hasAssertions() + + db.oneOrNone.mockRejectedValueOnce('mock-error') + + return AdminInterface.addPermission(5, 'someNewPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledTimes(2) + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface _updateUserPerms error', + 'mock-error' + ) + expect(logger.logError).toHaveBeenCalledWith('AdminInterface error adding permission') + }) + }) + + test('removePermission does nothing if trying to remove a permission the given user does not have', () => { + expect.assertions(2) + + // set the user's permissions such that they already have the one we're trying to give them + // ideally we could check implicit and explicit perms separately, but they're added to a single + // array inside the User model so our only option is to check the one location + expectedResponseUser.perms = ['someOtherPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.removePermission(5, 'someExistingPermission').then(u => { + expect(u).toEqual(expectedResponseUser) + expect(db.oneOrNone).not.toHaveBeenCalled() + }) + }) + + test('removePermission does nothing if trying to remove a permission the given user has implicitly', () => { + expect.assertions(2) + + expectedResponseUser.roles = ['mockRole'] + expectedResponseUser.perms = ['someExistingPermission', 'mockExistingPermission'] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.removePermission(5, 'mockExistingPermission').then(u => { + expect(u).toEqual(expectedResponseUser) + expect(db.oneOrNone).not.toHaveBeenCalled() + }) + }) + + test('removePermission saves explicit permissions after removing one from the given user', () => { + db.oneOrNone.mockResolvedValueOnce(5) + + // 'mockRole' will account for the 'mockExistingPermission' perm below + expectedResponseUser.roles = ['mockRole'] + expectedResponseUser.perms = [ + 'someExistingPermission', + 'someOtherExistingPermission', + 'mockExistingPermission' + ] + User.fetchById = jest.fn().mockResolvedValueOnce(expectedResponseUser) + + return AdminInterface.removePermission(5, 'someOtherExistingPermission').then(u => { + expect(u).toEqual({ + ...expectedResponseUser, + perms: ['mockExistingPermission', 'someExistingPermission'] + }) + expect(db.oneOrNone).toHaveBeenCalledTimes(1) + expect(db.oneOrNone.mock.calls[0][1]).toEqual({ + userId: 5, + perms: ['someExistingPermission'] + }) + }) + }) + + test('removePermission catches error when fetching user with invalid id', () => { + User.fetchById = jest.fn().mockRejectedValueOnce('mock-error') + + expect.hasAssertions() + + return AdminInterface.removePermission(123456, 'someExistingPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface error finding user with id 123456' + ) + }) + }) + + test('removePermission catches error when updating user perms', () => { + expect.hasAssertions() + + db.oneOrNone.mockRejectedValueOnce('mock-error') + + expectedResponseUser.perms = ['someExistingPermission'] + + return AdminInterface.removePermission(5, 'someExistingPermission').catch(() => { + expect(logger.logError).toHaveBeenCalledTimes(2) + expect(logger.logError).toHaveBeenCalledWith( + 'AdminInterface _updateUserPerms error', + 'mock-error' + ) + expect(logger.logError).toHaveBeenCalledWith('AdminInterface error removing permission') + }) + }) +}) diff --git a/packages/app/obojobo-repository/server/routes/admin.js b/packages/app/obojobo-repository/server/routes/admin.js new file mode 100644 index 0000000000..82966a4de2 --- /dev/null +++ b/packages/app/obojobo-repository/server/routes/admin.js @@ -0,0 +1,26 @@ +const express = require('express') +const router = express.Router() +const { webpackAssetPath } = require('obojobo-express/server/asset_resolver') +const { + requireCurrentUser, + requireCanViewAdminPage +} = require('obojobo-express/server/express_validators') + +// Admin page +// mounted as /admin +// NOTE: is an isomorphic react page +router + .route('/admin') + .get([requireCurrentUser, requireCanViewAdminPage]) + .get((req, res) => { + const props = { + title: 'Admin', + currentUser: req.currentUser, + // must use webpackAssetPath for all webpack assets to work in dev and production! + appCSSUrl: webpackAssetPath('admin.css'), + appJsUrl: webpackAssetPath('admin.js') + } + res.render('pages/page-admin-server.jsx', props) + }) + +module.exports = router diff --git a/packages/app/obojobo-repository/server/routes/admin.test.js b/packages/app/obojobo-repository/server/routes/admin.test.js new file mode 100644 index 0000000000..9dd86a606f --- /dev/null +++ b/packages/app/obojobo-repository/server/routes/admin.test.js @@ -0,0 +1,115 @@ +jest.unmock('fs') // need fs working for view rendering +jest.unmock('express') // we'll use supertest + express for this +jest.mock( + 'obojobo-express/server/asset_resolver', + () => ({ + assetForEnv: path => path, + webpackAssetPath: path => path + }), + { virtual: true } +) + +jest.setTimeout(10000) // extend test timeout? + +// override requireCurrentUser for tests to provide our own user +let mockCurrentUser + +jest.mock('obojobo-express/server/express_current_user', () => (req, res, next) => { + req.requireCurrentUser = () => { + req.currentUser = mockCurrentUser + return Promise.resolve(mockCurrentUser) + } + req.getCurrentUser = () => { + req.currentUser = mockCurrentUser + return Promise.resolve(mockCurrentUser) + } + req.requireCanViewAdminPage = () => { + req.currentUser = mockCurrentUser + return Promise.resolve(mockCurrentUser) + } + next() +}) + +let mockAdminComponent +let mockAdminComponentConstructor +jest.mock('obojobo-repository/shared/components/pages/page-admin-server') + +const componentPropsDesiredProperties = ['title', 'currentUser', 'allModules'] + +// setup express server +const path = require('path') +const request = require('supertest') +const express = require('express') +const bodyParser = require('body-parser') +const app = express() + +// register express-react-views template engine if not already registered +app.engine('jsx', require('express-react-views-custom').createEngine()) + +app.set('view engine', 'jsx') + +let viewPaths = app.get('views') +if (!Array.isArray(viewPaths)) viewPaths = [viewPaths] +viewPaths.push(path.resolve(`${__dirname}/../../shared/components`)) // add the components dir so babel can transpile the jsx +app.set('views', viewPaths) + +app.use(bodyParser.json()) +app.use(bodyParser.urlencoded({ extended: true })) + +app.use(require('obojobo-express/server/express_current_user')) + +app.use('/', require('obojobo-express/server/express_response_decorator')) +app.use('/', require('obojobo-repository/server/routes/admin')) + +describe('repository admin route', () => { + beforeEach(() => { + jest.resetAllMocks() + mockCurrentUser = { + id: 99, + // return true when the perm being asked about is 'canViewAdminPage + hasPermission: perm => perm === 'canViewAdminPage' + } + + //there's extra express garbage attached to the props we care about + //this roundabout solution exists to only pull out the ones we want + mockAdminComponentConstructor = jest.fn() + mockAdminComponent = require('obojobo-repository/shared/components/pages/page-admin-server') + mockAdminComponent.mockImplementation(props => { + const desiredProps = {} + componentPropsDesiredProperties.forEach(prop => { + desiredProps[prop] = props[prop] + }) + mockAdminComponentConstructor(desiredProps) + return '' + }) + }) + + test('get /admin returns a "not authorized" if the viewer does not have canViewAdminPage', () => { + // always return false - a.k.a. the user does not have the right perms to use this + mockCurrentUser.hasPermission = () => false + + expect.hasAssertions() + + return request(app) + .get('/admin') + .then(response => { + expect(mockAdminComponent).toHaveBeenCalledTimes(0) + expect(response.statusCode).toBe(401) + }) + }) + + test('get /admin sends the correct props to the Admin component when the user has canViewAdminPage', () => { + expect.hasAssertions() + + return request(app) + .get('/admin') + .then(response => { + expect(mockAdminComponent).toHaveBeenCalledTimes(1) + expect(mockAdminComponentConstructor).toHaveBeenCalledWith({ + title: 'Admin', + currentUser: mockCurrentUser + }) + expect(response.statusCode).toBe(200) + }) + }) +}) diff --git a/packages/app/obojobo-repository/server/routes/api.js b/packages/app/obojobo-repository/server/routes/api.js index f6f55da1e6..b912aa8688 100644 --- a/packages/app/obojobo-repository/server/routes/api.js +++ b/packages/app/obojobo-repository/server/routes/api.js @@ -6,6 +6,7 @@ const Draft = require('obojobo-express/server/models/draft') const DraftSummary = require('../models/draft_summary') const DraftPermissions = require('../models/draft_permissions') const DraftsMetadata = require('../models/drafts_metadata') +const AdminInterface = require('../models/admin_interface') const { requireCanPreviewDrafts, requireCurrentUser, @@ -14,7 +15,8 @@ const { requireCanCreateDrafts, requireCanDeleteDrafts, check, - requireCanViewStatsPage + requireCanViewStatsPage, + requireCanViewAdminPage } = require('obojobo-express/server/express_validators') const UserModel = require('obojobo-express/server/models/user') const { searchForUserByString } = require('../services/search') @@ -169,6 +171,36 @@ router } }) +router + .route('/permissions/add') + .post([requireCanViewAdminPage]) + .post(async (req, res) => { + const userId = req.body.userId + const perm = req.body.perm + + try { + const modifiedUser = await AdminInterface.addPermission(userId, perm) + res.success(modifiedUser) + } catch (error) { + res.unexpected(error) + } + }) + +router + .route('/permissions/remove') + .post([requireCanViewAdminPage]) + .post(async (req, res) => { + const userId = req.body.userId + const perm = req.body.perm + + try { + const modifiedUser = await AdminInterface.removePermission(userId, perm) + res.success(modifiedUser) + } catch (error) { + res.unexpected(error) + } + }) + // Copy a draft to the current user // mounted as /api/drafts/:draftId/copy router diff --git a/packages/app/obojobo-repository/server/routes/api.test.js b/packages/app/obojobo-repository/server/routes/api.test.js index db541d935e..538d94debd 100644 --- a/packages/app/obojobo-repository/server/routes/api.test.js +++ b/packages/app/obojobo-repository/server/routes/api.test.js @@ -10,6 +10,7 @@ jest.mock('../services/collections') jest.mock('../services/count') jest.mock('obojobo-express/server/models/user') jest.mock('obojobo-express/server/insert_event') +jest.mock('../models/admin_interface') jest.unmock('fs') // need fs working for view rendering jest.unmock('express') // we'll use supertest + express for this @@ -24,6 +25,7 @@ let CountServices let UserModel let insertEvent let DraftPermissions +let AdminInterface // override requireCurrentUser for tests to provide our own user let mockCurrentUser @@ -98,6 +100,7 @@ describe('repository api route', () => { CountServices = require('../services/count') DraftPermissions = require('../models/draft_permissions') UserModel = require('obojobo-express/server/models/user') + AdminInterface = require('../models/admin_interface') insertEvent = require('obojobo-express/server/insert_event') }) @@ -494,6 +497,64 @@ describe('repository api route', () => { }) }) + test('post /permissions/add returns the expected response', () => { + expect.hasAssertions() + + // this should be a user object but for testing purposes it doesn't matter + AdminInterface.addPermission.mockResolvedValueOnce(true) + + return request(app) + .post('/permissions/add') + .send({ userId: 5, perm: 'someNewPerm' }) + .then(response => { + expect(AdminInterface.addPermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.addPermission).toHaveBeenCalledWith(5, 'someNewPerm') + expect(response.statusCode).toBe(200) + }) + }) + test('post /permissions/add handles thrown errors', () => { + expect.hasAssertions() + + AdminInterface.addPermission.mockRejectedValueOnce('database error') + + return request(app) + .post('/permissions/add') + .send({ userId: 5, perm: 'someNewPerm' }) + .then(response => { + expect(AdminInterface.addPermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.addPermission).toHaveBeenCalledWith(5, 'someNewPerm') + expect(response.statusCode).toBe(500) + }) + }) + test('post /permissions/remove returns the expected response', () => { + expect.hasAssertions() + + AdminInterface.removePermission.mockResolvedValueOnce(true) + + return request(app) + .post('/permissions/remove') + .send({ userId: 5, perm: 'someExistingPerm' }) + .then(response => { + expect(AdminInterface.removePermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.removePermission).toHaveBeenCalledWith(5, 'someExistingPerm') + expect(response.statusCode).toBe(200) + }) + }) + test('post /permissions/remove handles thrown errors', () => { + expect.hasAssertions() + + AdminInterface.removePermission.mockRejectedValueOnce('database error') + + return request(app) + .post('/permissions/remove') + .send({ userId: 5, perm: 'someExistingPerm' }) + .then(response => { + expect(AdminInterface.removePermission).toHaveBeenCalledTimes(1) + expect(AdminInterface.removePermission).toHaveBeenCalledWith(5, 'someExistingPerm') + expect(response.statusCode).toBe(500) + }) + }) + test('post /drafts/:draftId/copy returns the expected response when user can copy draft', () => { expect.hasAssertions() diff --git a/packages/app/obojobo-repository/server/services/search.js b/packages/app/obojobo-repository/server/services/search.js index 19dd0c5080..1ba978780a 100644 --- a/packages/app/obojobo-repository/server/services/search.js +++ b/packages/app/obojobo-repository/server/services/search.js @@ -6,18 +6,21 @@ const searchForUserByString = async searchString => { return db .manyOrNone( `SELECT - id, - first_name AS "firstName", - last_name AS "lastName", - email, - username, - created_at AS "createdAt", - roles + users.id, + users.first_name AS "firstName", + users.last_name AS "lastName", + users.email, + users.username, + users.created_at AS "createdAt", + users.roles, + user_perms.perms FROM users - WHERE obo_immutable_concat_ws(' ', first_name, last_name) ILIKE $[search] - OR email ILIKE $[search] - OR username ILIKE $[search] - ORDER BY first_name, last_name + LEFT JOIN user_perms + ON users.id = user_perms.user_id + WHERE obo_immutable_concat_ws(' ', users.first_name, users.last_name) ILIKE $[search] + OR users.email ILIKE $[search] + OR users.username ILIKE $[search] + ORDER BY users.first_name, users.last_name LIMIT 25`, { search: `%${searchString}%` } ) diff --git a/packages/app/obojobo-repository/server/services/search.test.js b/packages/app/obojobo-repository/server/services/search.test.js index 789f406285..61c748db44 100644 --- a/packages/app/obojobo-repository/server/services/search.test.js +++ b/packages/app/obojobo-repository/server/services/search.test.js @@ -36,18 +36,21 @@ describe('Search Services', () => { expect.hasAssertions() const manyOrNoneQueryString = `SELECT - id, - first_name AS "firstName", - last_name AS "lastName", - email, - username, - created_at AS "createdAt", - roles + users.id, + users.first_name AS "firstName", + users.last_name AS "lastName", + users.email, + users.username, + users.created_at AS "createdAt", + users.roles, + user_perms.perms FROM users - WHERE obo_immutable_concat_ws(' ', first_name, last_name) ILIKE $[search] - OR email ILIKE $[search] - OR username ILIKE $[search] - ORDER BY first_name, last_name + LEFT JOIN user_perms + ON users.id = user_perms.user_id + WHERE obo_immutable_concat_ws(' ', users.first_name, users.last_name) ILIKE $[search] + OR users.email ILIKE $[search] + OR users.username ILIKE $[search] + ORDER BY users.first_name, users.last_name LIMIT 25` return SearchServices.searchForUserByString('searchString').then(response => { diff --git a/packages/app/obojobo-repository/shared/actions/admin-actions.js b/packages/app/obojobo-repository/shared/actions/admin-actions.js new file mode 100644 index 0000000000..ab594a955b --- /dev/null +++ b/packages/app/obojobo-repository/shared/actions/admin-actions.js @@ -0,0 +1,117 @@ +const debouncePromise = require('debounce-promise') + +const JSON_MIME_TYPE = 'application/json' +const defaultOptions = () => ({ + method: 'GET', + credentials: 'include', + headers: { + Accept: JSON_MIME_TYPE, + 'Content-Type': JSON_MIME_TYPE + } +}) + +const throwIfNotOk = res => { + if (!res.ok) throw Error(`Error requesting ${res.url}, status code: ${res.status}`) + return res +} + +const apiSearchForUser = searchString => { + return fetch(`/api/users/search?q=${searchString}`, defaultOptions()) + .then(throwIfNotOk) + .then(res => res.json()) +} + +const apiSearchForUserDebounced = debouncePromise(apiSearchForUser, 300) + +// =================== API ======================= + +// not using this yet +// const apiGetModules = () => { +// const JSON_MIME_TYPE = 'application/json' +// const fetchOptions = { +// method: 'GET', +// credentials: 'include', +// headers: { +// Accept: JSON_MIME_TYPE, +// 'Content-Type': JSON_MIME_TYPE +// } +// } +// return fetch('/api/drafts', fetchOptions).then(res => res.json()) +// } + +const apiAddUserPermission = (userId, perm) => { + const JSON_MIME_TYPE = 'application/json' + const fetchOptions = { + method: 'POST', + credentials: 'include', + headers: { + Accept: JSON_MIME_TYPE, + 'Content-Type': JSON_MIME_TYPE + }, + body: JSON.stringify({ userId, perm }) + } + + return fetch('/api/permissions/add', fetchOptions).then(res => res.json()) +} + +const apiRemoveUserPermission = (userId, perm) => { + const JSON_MIME_TYPE = 'application/json' + const fetchOptions = { + method: 'POST', + credentials: 'include', + headers: { + Accept: JSON_MIME_TYPE, + 'Content-Type': JSON_MIME_TYPE + }, + body: JSON.stringify({ userId, perm }) + } + + return fetch('/api/permissions/remove', fetchOptions).then(res => res.json()) +} + +// ================== ACTIONS =================== + +// const LOAD_ALL_MODULES = 'LOAD_ALL_MODULES' +// const loadModuleList = () => ({ +// type: LOAD_ALL_MODULES, +// promise: apiGetModules() +// }) + +const LOAD_USER_SEARCH = 'LOAD_USER_SEARCH' +const searchForUser = searchString => ({ + type: LOAD_USER_SEARCH, + meta: { searchString }, + promise: apiSearchForUserDebounced(searchString) +}) + +const ADD_USER_PERMISSION = 'ADD_USER_PERMISSION' +const addUserPermission = (userId, perm) => ({ + type: ADD_USER_PERMISSION, + promise: apiAddUserPermission(userId, perm) +}) + +const REMOVE_USER_PERMISSION = 'REMOVE_USER_PERMISSION' +const removeUserPermission = (userId, perm) => ({ + type: REMOVE_USER_PERMISSION, + promise: apiRemoveUserPermission(userId, perm) +}) + +const CLEAR_PEOPLE_SEARCH_RESULTS = 'CLEAR_PEOPLE_SEARCH_RESULTS' +const clearPeopleSearchResults = () => ({ type: CLEAR_PEOPLE_SEARCH_RESULTS }) + +module.exports = { + // LOAD_ALL_MODULES, + // loadModuleList, + + ADD_USER_PERMISSION, + addUserPermission, + + REMOVE_USER_PERMISSION, + removeUserPermission, + + LOAD_USER_SEARCH, + searchForUser, + + CLEAR_PEOPLE_SEARCH_RESULTS, + clearPeopleSearchResults +} diff --git a/packages/app/obojobo-repository/shared/actions/admin-actions.test.js b/packages/app/obojobo-repository/shared/actions/admin-actions.test.js new file mode 100644 index 0000000000..07b0fc4491 --- /dev/null +++ b/packages/app/obojobo-repository/shared/actions/admin-actions.test.js @@ -0,0 +1,163 @@ +const dayjs = require('dayjs') +const advancedFormat = require('dayjs/plugin/advancedFormat') + +jest.mock('./shared-api-methods', () => ({ + apiGetAssessmentDetailsForDraft: () => Promise.resolve() +})) + +dayjs.extend(advancedFormat) + +describe('Admin Actions', () => { + let standardFetchResponse + + let AdminActions + + // this is lifted straight out of admin-actions, for ease of comparison + // barring any better ways of using it + const defaultFetchOptions = { + method: 'GET', + credentials: 'include', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + } + } + + const originalFetch = global.fetch + + beforeAll(() => { + global.fetch = jest.fn() + }) + + beforeEach(() => { + jest.resetModules() + jest.resetAllMocks() + jest.useFakeTimers() + + global.alert = jest.fn() + + delete window.location + window.location = { + reload: jest.fn() + } + + standardFetchResponse = { + ok: true, + json: jest.fn(() => ({ value: 'mockVal' })) + } + + AdminActions = require('./admin-actions') + }) + + afterAll(() => { + global.fetch = originalFetch + }) + + test('searchForUser returns the expected output and calls other functions correctly - server ok', () => { + global.fetch.mockResolvedValueOnce({ ...standardFetchResponse, ok: true }) + + const actionReply = AdminActions.searchForUser('mockSearchString') + jest.runAllTimers() + + expect(global.fetch).toHaveBeenCalledWith( + '/api/users/search?q=mockSearchString', + defaultFetchOptions + ) + + expect(actionReply).toEqual({ + type: AdminActions.LOAD_USER_SEARCH, + meta: { + searchString: 'mockSearchString' + }, + promise: expect.any(Object) + }) + + return actionReply.promise.then(() => { + expect(standardFetchResponse.json).toHaveBeenCalled() + }) + }) + test('searchForUser returns the expected output and calls other functions correctly - server error', () => { + const mockFetchUrl = '/api/users/search?q=mockSearchString' + global.fetch.mockResolvedValueOnce({ + ok: false, + url: mockFetchUrl, + status: 500 + }) + + const actionReply = AdminActions.searchForUser('mockSearchString') + + expect(actionReply).toEqual({ + type: AdminActions.LOAD_USER_SEARCH, + meta: { + searchString: 'mockSearchString' + }, + promise: expect.any(Object) + }) + + jest.runAllTimers() + return actionReply.promise.catch(error => { + expect(error).toBeInstanceOf(Error) + expect(error.message).toBe( + 'Error requesting /api/users/search?q=mockSearchString, status code: 500' + ) + + expect(global.fetch).toHaveBeenCalledWith( + '/api/users/search?q=mockSearchString', + defaultFetchOptions + ) + }) + }) + + test('addUserPermission returns the expected output and calls other functions', () => { + global.fetch.mockResolvedValueOnce({ ...standardFetchResponse, ok: true }) + + const actionReply = AdminActions.addUserPermission(5, 'someNewPermission') + jest.runAllTimers() + + expect(global.fetch).toHaveBeenCalledWith('/api/permissions/add', { + ...defaultFetchOptions, + method: 'POST', + body: JSON.stringify({ userId: 5, perm: 'someNewPermission' }) + }) + + expect(actionReply).toEqual({ + type: AdminActions.ADD_USER_PERMISSION, + promise: expect.any(Object) + }) + + return actionReply.promise.then(() => { + expect(standardFetchResponse.json).toHaveBeenCalled() + }) + }) + + test('removeUserPermission returns the expected output and calls other functions', () => { + global.fetch.mockResolvedValueOnce({ ...standardFetchResponse, ok: true }) + + const actionReply = AdminActions.removeUserPermission(5, 'someExistingPermission') + jest.runAllTimers() + + expect(global.fetch).toHaveBeenCalledWith('/api/permissions/remove', { + ...defaultFetchOptions, + method: 'POST', + body: JSON.stringify({ userId: 5, perm: 'someExistingPermission' }) + }) + + expect(actionReply).toEqual({ + type: AdminActions.REMOVE_USER_PERMISSION, + promise: expect.any(Object) + }) + + return actionReply.promise.then(() => { + expect(standardFetchResponse.json).toHaveBeenCalled() + }) + }) + + test('clearPeopleSearchResults returns the expected output', () => { + const actionReply = AdminActions.clearPeopleSearchResults() + + expect(global.fetch).not.toHaveBeenCalled() + expect(actionReply).toEqual({ + type: AdminActions.CLEAR_PEOPLE_SEARCH_RESULTS + }) + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap b/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap new file mode 100644 index 0000000000..485364a0c2 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/__snapshots__/admin.test.js.snap @@ -0,0 +1,123 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Admin renders default view correctly 1`] = ` + +
+ +
+
+
+
+

+

+
+
+
+
+

+ Find Users to Manage +

+ +
+
+
    +
+
+
+ +`; diff --git a/packages/app/obojobo-repository/shared/components/__snapshots__/repository-nav.test.js.snap b/packages/app/obojobo-repository/shared/components/__snapshots__/repository-nav.test.js.snap index 72da9e742d..6957fcd3e4 100644 --- a/packages/app/obojobo-repository/shared/components/__snapshots__/repository-nav.test.js.snap +++ b/packages/app/obojobo-repository/shared/components/__snapshots__/repository-nav.test.js.snap @@ -78,6 +78,93 @@ exports[`RepositoryNav does not render stats section with just canViewSystemStat
`; +exports[`RepositoryNav renders admin section with canViewAdminPage 1`] = ` +
+ +
+`; + exports[`RepositoryNav renders correctly with standard expected props 1`] = `
state +const mapActionsToProps = { + addUserPermission, + removeUserPermission, + searchForUser, + clearPeopleSearchResults +} +module.exports = connect( + mapStoreStateToProps, + mapActionsToProps +)(Admin) diff --git a/packages/app/obojobo-repository/shared/components/admin-hoc.test.js b/packages/app/obojobo-repository/shared/components/admin-hoc.test.js new file mode 100644 index 0000000000..50103f49b5 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin-hoc.test.js @@ -0,0 +1,36 @@ +jest.mock('react-redux') +jest.mock('../actions/admin-actions') +jest.mock('./admin', () => ({})) + +import AdminActions from '../actions/admin-actions' +import Admin from './admin' + +describe('admin HOC', () => { + test('redux collect is called with the correct arguments', () => { + const ReactRedux = require('react-redux') + + const mockReduxConnectReturn = jest.fn() + ReactRedux.connect = jest.fn(mapStoreStateToProps => { + // The first argument to 'connect' is a private (untestable) method in the HOC + // that method will never be 'covered' unless we call it like this + // since we also know that it should just return what it's given, we can test that too + const mapStoreStateToPropsArgs = { testKey: 'testProp' } + const mapStoreStateToPropsReturn = mapStoreStateToProps(mapStoreStateToPropsArgs) + expect(mapStoreStateToPropsReturn).toStrictEqual(mapStoreStateToPropsArgs) + return mockReduxConnectReturn + }) + + require('./admin-hoc') + + expect(ReactRedux.connect).toHaveBeenCalledTimes(1) + expect(ReactRedux.connect).toHaveBeenCalledWith(expect.any(Function), { + addUserPermission: AdminActions.addUserPermission, + clearPeopleSearchResults: AdminActions.clearPeopleSearchResults, + removeUserPermission: AdminActions.removeUserPermission, + searchForUser: AdminActions.searchForUser + }) + + expect(mockReduxConnectReturn).toHaveBeenCalledTimes(1) + expect(mockReduxConnectReturn).toHaveBeenCalledWith(Admin) + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/admin.jsx b/packages/app/obojobo-repository/shared/components/admin.jsx new file mode 100644 index 0000000000..301378dd7f --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin.jsx @@ -0,0 +1,178 @@ +require('./modal.scss') +require('./admin.scss') + +const React = require('react') +const { useState, useEffect } = require('react') +const RepositoryNav = require('./repository-nav') +const RepositoryBanner = require('./repository-banner') +const Search = require('./search') +const Button = require('./button') +const PeopleListItem = require('./people-list-item') + +const { POSSIBLE_PERMS, PERMS_PER_ROLE } = require('../util/implicit-perms') + +const NO_SELECTION_PERMISSION = 'no-select-permission' + +const Admin = props => { + // storing selected user in state by choosing one from the list fetched by the search may not work + // perms attached to a user via the search didn't previously include explicitly added perms, just role perms + // that was changed but may slow down the initial fetch due to a join in the query + // may be faster to instead run a new action to fetch the single user explicitly, which will get all perms + // but that would put the selected user in props, as opposed to doing this + const [selectedUser, setSelectedUser] = useState(null) + const [selectedPermission, setSelectedPermission] = useState(NO_SELECTION_PERMISSION) + + // make sure user/module search results are zero'd out on first render + useEffect(() => { + props.clearPeopleSearchResults() + }, []) + + const onAddUserPermission = async () => { + if (selectedPermission === NO_SELECTION_PERMISSION) return + + const action = await props.addUserPermission(selectedUser.id, selectedPermission) + if (action.payload.status === 'ok') setSelectedUser(action.payload.value) + } + + const onRemoveUserPermission = async () => { + if (selectedPermission === NO_SELECTION_PERMISSION) return + + const action = await props.removeUserPermission(selectedUser.id, selectedPermission) + if (action.payload.status === 'ok') setSelectedUser(action.payload.value) + } + + const renderPermissionSelectList = () => { + return ( + + ) + } + + const renderListItem = u => { + if (u.id !== props.currentUser.id) { + return ( + + + + ) + } + } + + const renderContent = () => { + if (selectedUser) { + const allRolePerms = new Set() + const rolePermsRender = selectedUser.roles.map(role => { + const roles = PERMS_PER_ROLE[role].map(perm => { + allRolePerms.add(perm) + + return
  • {perm}
    • + }) + return ( +
    + +
      {roles}
    +
    + ) + }) + + const nonRolePerms = selectedUser.perms + .filter(p => !allRolePerms.has(p)) + .map(p =>
  • {p}
    • ) + + return ( + +
    +

    Managing:

    + +
    +
    +

    Manage User Permissions

    +
    + {selectedUser.roles.length > 0 ? ( + + + Current implicit permissions from role{selectedUser.roles.length > 1 ? 's' : ''} + : + + {rolePermsRender} + + ) : ( + 'None' + )} +
    +
    + Current explicit permissions: + {nonRolePerms.length < 1 ? 'None' :
      {nonRolePerms}
    } +
    +
    +

    Select permission:

    + {renderPermissionSelectList()} +
    +
    + + +
    +
    +     + ) + } + return ( + +
    +

    Find Users to Manage

    + +
    +
    +
      + {props.searchUsers && props.searchUsers.items + ? props.searchUsers.items.map(renderListItem) + : null} +
    +
    +     + ) + } + + return ( + + + +
    +
    {renderContent()}
    +
    +     + ) +} + +module.exports = Admin diff --git a/packages/app/obojobo-repository/shared/components/admin.scss b/packages/app/obojobo-repository/shared/components/admin.scss new file mode 100644 index 0000000000..03367097db --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin.scss @@ -0,0 +1,52 @@ +@import '../../client/css/defaults'; + +#admin-root { + .repository--section-wrapper { + max-width: inherit; + } + + .repository--data-grid-drafts-container { + max-height: 30em; + overflow: scroll; + margin-bottom: 1em; + } + + .repository--assessment-stats { + height: 50em; + } + + .repository--drafts-search { + padding: 0.25em; + width: 14em; + margin-bottom: 1em; + } + + .title { + background-color: $color-banner-bg; + color: $color-action-minor; + border-radius: 0.25em; + padding: 1em; + margin: 0; + } + + select { + padding: 0.8em; + border-radius: 0.25em; + min-width: 30%; + } + + .row { + display: flex; + align-items: center; + justify-content: space-between; + } + + .row.buttons { + justify-content: left; + gap: 1em; + } + + .tool-button { + width: 6em; + } +} diff --git a/packages/app/obojobo-repository/shared/components/admin.test.js b/packages/app/obojobo-repository/shared/components/admin.test.js new file mode 100644 index 0000000000..819dae0806 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/admin.test.js @@ -0,0 +1,464 @@ +import React from 'react' +import { create, act } from 'react-test-renderer' +// import Button from './button' + +jest.mock('./people-list-item', () => props => { + return {props.children} +}) +import PeopleListItem from './people-list-item' + +jest.mock('./search', () => props => { + return {props.children} +}) +import Search from './search' + +jest.mock('../util/implicit-perms', () => ({ + POSSIBLE_PERMS: ['perm1', 'perm2', 'perm3', 'perm4', 'mockExtraPerm'], + PERMS_PER_ROLE: { + role1: ['perm1', 'perm2', 'perm3'], + role2: ['perm4'] + } +})) + +import Admin from './admin' + +describe('Admin', () => { + beforeEach(() => { + jest.resetAllMocks() + }) + + const defaultProps = { + currentUser: { + id: 99, + avatarUrl: '/path/to/avatar', + firstName: 'firstName', + lastName: 'lastName', + perms: [ + 'canViewEditor', + 'canEditDrafts', + 'canDeleteDrafts', + 'canCreateDrafts', + 'canPreviewDrafts' + ] + }, + addUserPermission: jest.fn(), + removeUserPermission: jest.fn(), + searchForUser: jest.fn(), + clearPeopleSearchResults: jest.fn() + } + + test('renders default view correctly', () => { + const component = create() + + // should only be a search input visible by default, no search results + expect(component.root.findAllByType(PeopleListItem).length).toBe(0) + expect(component.root.findByProps({ className: 'user-list' }).children.length).toBe(0) + + const tree = component.toJSON() + expect(tree).toMatchSnapshot() + }) + + test('renders user search results correctly', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { id: 1, firstName: 'Mock', lastName: 'User1' }, + { id: 2, firstName: 'Mock', lastName: 'User2' }, + { id: 3, firstName: 'Mock', lastName: 'User3' }, + // just to make sure the current user doesn't show up in the list + { ...defaultProps.currentUser } + ] + } + + const component = create() + const searchResults = component.root.findAllByType(PeopleListItem) + expect(searchResults.length).toBe(3) + // bit inelegant but does the job + searchResults.forEach(i => { + expect(i.props.isMe).toBe(false) + expect(i.props.firstName).not.toBe(defaultProps.currentUser.firstName) + expect(i.props.lastName).not.toBe(defaultProps.currentUser.lastName) + }) + }) + + test('calls functions appropriately when search value changes', () => { + const component = create() + act(() => { + // ordinarily there'd be targets and values to parse, but the Search component does that for us + component.root.findByType(Search).props.onChange('mock-new-search-value') + }) + + expect(defaultProps.searchForUser).toHaveBeenCalledTimes(1) + expect(defaultProps.searchForUser).toHaveBeenCalledWith('mock-new-search-value') + }) + + test('sets selected user correctly - no roles or explicit perms', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: [], + perms: [] + } + ] + } + + const component = create() + + // pre-selection state - make sure the search tools are visible + expect(component.root.findAllByType('mock-Search').length).toBe(1) + expect(component.root.findAllByProps({ className: 'tool' }).length).toBe(0) + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // post-selection state - make sure the search tools are no longer available + expect(component.root.findAllByType('mock-Search').length).toBe(0) + expect(component.root.findAllByProps({ className: 'tool' }).length).toBe(1) + + const implicitContainer = component.root.findByProps({ className: 'implicit-perms-container' }) + expect(implicitContainer.findAllByType('ul').length).toBe(0) + expect(implicitContainer.children[implicitContainer.children.length - 1]).toBe('None') + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findAllByType('ul').length).toBe(0) + expect(explicitContainer.children[explicitContainer.children.length - 1]).toBe('None') + + // for good measure, make sure the permissions list is populated correctly and the add/remove buttons appear + // this is a bit brute force-y, but at least we know for sure + // also a bit magical since we happen to know what the mocked values are + const expectedSelectOptions = [ + 'no-select-permission', // this will always be there + 'perm1', + 'perm2', + 'perm3', + 'perm4', + 'mockExtraPerm' + ] + const permSelect = component.root.findByType('select') + expectedSelectOptions.forEach((v, i) => { + expect(permSelect.findAllByType('option')[i].props.value).toBe(v) + }) + + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + expect(buttons.length).toBe(2) + expect(buttons[0].children[0].children[0]).toBe('Add') + expect(buttons[1].children[0].children[0]).toBe('Remove') + }) + + test('sets selected user correctly - one role and explicit perms', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3', 'mockExtraPerm'] + } + ] + } + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + const implicitContainer = component.root.findByProps({ className: 'implicit-perms-container' }) + // the 'single' rendered string is actually three strings, since the singular/plural part is interpolated + // checking to make sure 'role' isn't 'roles' since any number greater than one requires the extra 's' + expect(implicitContainer.findByType('span').children).toEqual([ + 'Current implicit permissions from role', + '', + ':' + ]) + expect(implicitContainer.findAllByType('ul').length).toBe(1) + // we happen to know what this should be based on our mock of the role-based perms at the top of this file + // ...but ideally this shouldn't be a magical number + expect(implicitContainer.findByType('ul').children.length).toBe(3) + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('mockExtraPerm') + }) + + test('sets selected user correctly - multiple roles', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1', 'role2'], + perms: ['perm1', 'perm2', 'perm3', 'perm4', 'mockExtraPerm'] + } + ] + } + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + const implicitContainer = component.root.findByProps({ className: 'implicit-perms-container' }) + + expect(implicitContainer.findByType('span').children).toEqual([ + 'Current implicit permissions from role', + 's', + ':' + ]) + expect(implicitContainer.findAllByType('ul').length).toBe(2) + // same situation - we know what order these will be in because we mocked them to show up this way, but... magical + expect(implicitContainer.findAllByType('ul')[0].children.length).toBe(3) + expect(implicitContainer.findAllByType('ul')[0].children[0].children[0]).toBe('perm1') + expect(implicitContainer.findAllByType('ul')[0].children[1].children[0]).toBe('perm2') + expect(implicitContainer.findAllByType('ul')[0].children[2].children[0]).toBe('perm3') + + expect(implicitContainer.findAllByType('ul')[1].children.length).toBe(1) + expect(implicitContainer.findAllByType('ul')[1].children[0].children[0]).toBe('perm4') + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('mockExtraPerm') + }) + + test('does nothing when trying to add or remove before selecting a valid permission', () => { + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [ + { + id: 1, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3'] + } + ] + } + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // without selecting a new permission, try adding and removing to make sure they don't do anything + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + + // clicking the 'Add' button should do nothing + act(() => { + buttons[0].props.onClick() + }) + expect(newProps.addUserPermission).not.toHaveBeenCalled() + + // clicking the 'Remove' button should do nothing + act(() => { + buttons[1].props.onClick() + }) + expect(newProps.removeUserPermission).not.toHaveBeenCalled() + }) + + test('sends the correct permission to the API utility when adding - success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.addUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { + status: 'ok', + value: { ...mockUser, perms: ['perm1', 'perm2', 'perm3', 'perm4'] } + } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // make sure there are no explicit perms before we add one + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findAllByType('ul').length).toBe(0) + + // as a bonus we're also making sure the permission selection update works properly + // since there's no real way of checking that independently + // make sure clicking the 'Add' button calls functions appropriately + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + // ordinarily this value would come from the selected option attached to the change event + // we can just simulate the change event's target's value here + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[0].props.onClick() + }) + + expect(newProps.addUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.addUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // on success, the implicit/explicit perms areas should update with any changes + // we added an explicit change this time so we can check that + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + }) + test('sends the correct permission to the API utility when adding - non-success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.addUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { status: 'not-ok' } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // make sure there are no explicit perms before we try adding one + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findAllByType('ul').length).toBe(0) + + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[0].props.onClick() + }) + + expect(newProps.addUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.addUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // response wasn't successful, make sure nothing changed + expect(explicitContainer.findAllByType('ul').length).toBe(0) + }) + + test('sends the correct permission to the API utility when removing - success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3', 'perm4'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.removeUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { + status: 'ok', + value: { ...mockUser, perms: ['perm1', 'perm2', 'perm3'] } + } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + // make sure the single explicit perm is shown before we remove it + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + + // make sure clicking the 'Remove' button calls functions appropriately + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[1].props.onClick() + }) + + expect(newProps.removeUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.removeUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // we removed the only explicit perm, make sure that's reflected properly + expect(explicitContainer.findAllByType('ul').length).toBe(0) + }) + test('sends the correct permission to the API utility when removing - success response', async () => { + const mockUser = { + id: 43, + firstName: 'Mock', + lastName: 'User1', + roles: ['role1'], + perms: ['perm1', 'perm2', 'perm3', 'perm4'] + } + + const newProps = { ...defaultProps } + newProps.searchUsers = { + items: [{ ...mockUser }] + } + + newProps.removeUserPermission = jest.fn().mockResolvedValueOnce({ + payload: { + status: 'not-ok' + } + }) + + const component = create() + + act(() => { + component.root.findByType(PeopleListItem).children[0].children[0].props.onClick() + }) + + const explicitContainer = component.root.findByProps({ className: 'explicit-perms-container' }) + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + + const permSelect = component.root.findByType('select') + const buttons = component.root.findAllByProps({ className: 'tool-button' }) + act(() => { + const mockChangeEvent = { target: { value: 'perm4' } } + permSelect.props.onChange(mockChangeEvent) + }) + await act(async () => { + await buttons[1].props.onClick() + }) + + expect(newProps.removeUserPermission).toHaveBeenCalledTimes(1) + expect(newProps.removeUserPermission).toHaveBeenCalledWith(mockUser.id, 'perm4') + + // response wasn't successful, make sure nothing changed + expect(explicitContainer.findByType('ul').children.length).toBe(1) + expect(explicitContainer.findByType('ul').children[0].children[0]).toBe('perm4') + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/pages/page-admin-client.jsx b/packages/app/obojobo-repository/shared/components/pages/page-admin-client.jsx new file mode 100644 index 0000000000..d05e363127 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/pages/page-admin-client.jsx @@ -0,0 +1,8 @@ +import { hydrateEl } from '../../react-utils' +import Admin from '../admin-hoc' +import AdminReducer from '../../reducers/admin-reducer' + +// include LayoutDefault so client side react has a copy of it +import LayoutDefault from '../layouts/default' //eslint-disable-line no-unused-vars + +hydrateEl(Admin, AdminReducer, '#react-hydrate-root') diff --git a/packages/app/obojobo-repository/shared/components/pages/page-admin-server.jsx b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.jsx new file mode 100644 index 0000000000..390a086d68 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.jsx @@ -0,0 +1,24 @@ +const React = require('react') +const LayoutDefault = require('../layouts/default') +const Admin = require('../admin-hoc') +const { propsToStore, createCommonReactApp, convertPropsToString } = require('../../react-utils') +const AdminReducer = require('../../reducers/admin-reducer') + +const PageAdminServer = props => { + return ( + + + {createCommonReactApp(Admin, propsToStore(AdminReducer, props))} + + + ) +} + +PageAdminServer.defaultProps = {} + +module.exports = PageAdminServer diff --git a/packages/app/obojobo-repository/shared/components/pages/page-admin-server.test.js b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.test.js new file mode 100644 index 0000000000..74c4e06c60 --- /dev/null +++ b/packages/app/obojobo-repository/shared/components/pages/page-admin-server.test.js @@ -0,0 +1,35 @@ +import React from 'react' +import PageAdminServer from './page-admin-server' +import { shallow } from 'enzyme' + +jest.mock('../../react-utils') +jest.mock('../admin-hoc') +jest.mock('../../reducers/admin-reducer') + +const ReactUtils = require('../../react-utils') +const Admin = require('../admin-hoc') +const AdminReducer = require('../../reducers/admin-reducer') +const LayoutDefault = require('../layouts/default') + +describe('Server-side Admin Page', () => { + test('renders and runs functions correctly with default props', () => { + const mockStore = { + mockStoreKey: 'mockStoreVal' + } + + ReactUtils.propsToStore.mockReturnValueOnce(mockStore) + + const component = shallow() + + expect(ReactUtils.convertPropsToString).toHaveBeenCalledTimes(1) + expect(ReactUtils.convertPropsToString).toHaveBeenCalledWith(PageAdminServer.defaultProps) + + expect(ReactUtils.propsToStore).toHaveBeenCalledTimes(1) + expect(ReactUtils.propsToStore).toHaveBeenCalledWith(AdminReducer, PageAdminServer.defaultProps) + + expect(ReactUtils.createCommonReactApp).toHaveBeenCalledTimes(1) + expect(ReactUtils.createCommonReactApp).toHaveBeenCalledWith(Admin, mockStore) + + expect(component.find(LayoutDefault).length).toBe(1) + }) +}) diff --git a/packages/app/obojobo-repository/shared/components/repository-nav.jsx b/packages/app/obojobo-repository/shared/components/repository-nav.jsx index 361dfd0fd1..f95c072c74 100644 --- a/packages/app/obojobo-repository/shared/components/repository-nav.jsx +++ b/packages/app/obojobo-repository/shared/components/repository-nav.jsx @@ -46,6 +46,11 @@ const RepositoryNav = props => { Stats
    ) : null} + {props.userPerms.indexOf('canViewAdminPage') > -1 ? ( +
    + Admin +
    + ) : null} {props.userId !== 0 ? (