From 7423a1fdb283c9143ba4d25feaf645db64a99537 Mon Sep 17 00:00:00 2001
From: Dom Smith <dom@teneightfive.com>
Date: Tue, 24 Nov 2020 13:01:23 +0000
Subject: [PATCH] chore: Don't set development permissions if they already
 exist

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.
---
 common/middleware/development.js      |  5 +++--
 common/middleware/development.test.js | 27 ++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/common/middleware/development.js b/common/middleware/development.js
index 05c8d0f4c..d7f567303 100644
--- a/common/middleware/development.js
+++ b/common/middleware/development.js
@@ -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(',')
     }
 
diff --git a/common/middleware/development.test.js b/common/middleware/development.test.js
index 07ed0f698..5212c1c1d 100644
--- a/common/middleware/development.test.js
+++ b/common/middleware/development.test.js
@@ -59,11 +59,7 @@ describe('Development specific middleware', function () {
 
     beforeEach(function () {
       req = {
-        session: {
-          user: {
-            permissions: [],
-          },
-        },
+        session: {},
       }
     })
 
@@ -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 () {
@@ -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 () {