From 5b5d84836b3079c404f0eaf3e28b9dc5be8ed7a1 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 05c8d0f4cd..d7f5673035 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 07ed0f6981..5212c1c1df 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 () {