diff --git a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js index afccb6a01b..743e191c6e 100644 --- a/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js +++ b/services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js @@ -31,13 +31,14 @@ async function getAdminCapabilities(user) { async function addHasAdminCapabilityToLocals(req, res, next) { const user = SessionManager.getSessionUser(req.session) + if (!hasAdminAccess(user)) { + res.locals.hasAdminCapability = () => false + return next() + } try { const { adminCapabilities, adminCapabilitiesAvailable } = await getAdminCapabilities(user) res.locals.hasAdminCapability = capability => { - if (!hasAdminAccess(user)) { - return false - } if (!adminCapabilitiesAvailable) { // If admin capabilities are not available, then all admins have all capabilities return true @@ -45,10 +46,7 @@ async function addHasAdminCapabilityToLocals(req, res, next) { return adminCapabilities.includes(capability) } } catch (error) { - if (user) { - // This is unexpected, it probably means that the session user does not exist. - logger.warn({ error, req, user }, 'Failed to get admin capabilities') - } + logger.warn({ error, req, user }, 'Failed to get admin capabilities') // A module probably threw so adminCapabilitiesAvailable should be true if we are here so deny to be safe res.locals.hasAdminCapability = () => false } diff --git a/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js b/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js index a25bf985c1..6ef8c57111 100644 --- a/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js +++ b/services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js @@ -69,7 +69,7 @@ describe('AdminAuthorizationHelper', function () { this.next = sinon.stub() this.user = { - isAdmin: false, + isAdmin: true, } this.req.logger = { @@ -101,6 +101,29 @@ describe('AdminAuthorizationHelper', function () { }) }) describe('when admin capabilities are not available', function () { + describe('user is null', function () { + beforeEach(async function () { + this.req = new MockRequest() + this.res = new MockResponse() + this.next = sinon.stub() + + this.req.session = { + user: null, + } + + await this.AdminAuthorizationHelper.addHasAdminCapabilityToLocals( + this.req, + this.res, + this.next + ) + }) + it('defines hasAdminCapability on res.locals', function () { + expect(this.res.locals).to.have.property('hasAdminCapability') + }) + it('returns false when called with any capability', function () { + expect(this.res.locals.hasAdminCapability('capability1')).to.be.false + }) + }) describe('user is not an admin', function () { beforeEach(async function () { this.req = new MockRequest()