Skip to content

Commit

Permalink
chore: Don't set development permissions if they already exist
Browse files Browse the repository at this point in the history
This prevents the permissions being overridden on every request.

It also means that they can be updated via another method like session
injection and won't be overridden.
  • Loading branch information
teneightfive committed Nov 24, 2020
1 parent 6386a96 commit 5b5d848
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 9 deletions.
5 changes: 3 additions & 2 deletions common/middleware/development.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ function bypassAuth(bypass) {

function setUserPermissions(permissions) {
return (req, res, next) => {
if (permissions) {
req.session.user = req.session.user || {}
req.session.user = req.session.user || {}

if (permissions && !req.session.user.permissions) {
req.session.user.permissions = permissions.split(',')
}

Expand Down
27 changes: 20 additions & 7 deletions common/middleware/development.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,7 @@ describe('Development specific middleware', function () {

beforeEach(function () {
req = {
session: {
user: {
permissions: [],
},
},
session: {},
}
})

Expand All @@ -90,8 +86,8 @@ describe('Development specific middleware', function () {
middleware.setUserPermissions()(req, {}, nextSpy)
})

it('should not update permissions', function () {
expect(req.session.user.permissions).to.deep.equal([])
it('should not set permissions', function () {
expect(req.session.user.permissions).to.be.undefined
})

it('should call next', function () {
Expand Down Expand Up @@ -121,6 +117,23 @@ describe('Development specific middleware', function () {
expect(nextSpy).to.be.calledOnceWithExactly()
})
})

context('when permissions exist', function () {
beforeEach(function () {
req.session.user = {
permissions: ['foo', 'bar'],
}
middleware.setUserPermissions('one,two,three')(req, {}, nextSpy)
})

it('should not update permissions', function () {
expect(req.session.user.permissions).to.deep.equal(['foo', 'bar'])
})

it('should call next', function () {
expect(nextSpy).to.be.calledOnceWithExactly()
})
})
})

describe('#setUserLocations()', function () {
Expand Down

0 comments on commit 5b5d848

Please sign in to comment.