Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions packages/app/obojobo-express/__tests__/express_validators.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
],
"canViewAsStudent": ["Learner", "urn:lti:role:ims/lis/Learner"],
"canViewStatsPage": [],
"canViewSystemStats": []
"canViewSystemStats": [],
"canViewAdminPage": []
},
"test": {
"canDoThing": ["roleName"]
Expand Down
3 changes: 3 additions & 0 deletions packages/app/obojobo-express/server/express_validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
3 changes: 2 additions & 1 deletion packages/app/obojobo-express/server/obo_express_dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/app/obojobo-repository/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
1 change: 1 addition & 0 deletions packages/app/obojobo-repository/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
96 changes: 96 additions & 0 deletions packages/app/obojobo-repository/server/models/admin_interface.js
Original file line number Diff line number Diff line change
@@ -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
183 changes: 183 additions & 0 deletions packages/app/obojobo-repository/server/models/admin_interface.test.js
Original file line number Diff line number Diff line change
@@ -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')
})
})
})
26 changes: 26 additions & 0 deletions packages/app/obojobo-repository/server/routes/admin.js
Original file line number Diff line number Diff line change
@@ -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
Loading