Move hasAdminCapability middleware into helpers

This is so that we can test it more easily than embedding it would allow

GitOrigin-RevId: be23d945bc7c816d32b18b4990ecd9e0a6592eb5

services/web/app/src/Features/Helpers/AdminAuthorizationHelper.js

@@ -1,10 +1,14 @@
 const Settings = require('@overleaf/settings')
 const Modules = require('../../infrastructure/Modules')
+const { expressify } = require('@overleaf/promise-utils')
+const SessionManager = require('../Authentication/SessionManager')
+const logger = require('@overleaf/logger')
 
 module.exports = {
   hasAdminAccess,
   canRedirectToAdminDomain,
   getAdminCapabilities,
+  addHasAdminCapabilityToLocals: expressify(addHasAdminCapabilityToLocals),
 }
 
 function hasAdminAccess(user) {
@@ -25,6 +29,32 @@ async function getAdminCapabilities(user) {
   }
 }
 
+async function addHasAdminCapabilityToLocals(req, res, next) {
+  const user = SessionManager.getSessionUser(req.session)
+  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
+      }
+      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')
+    }
+    // A module probably threw so adminCapabilitiesAvailable should be true if we are here so deny to be safe
+    res.locals.hasAdminCapability = () => false
+  }
+  next()
+}
+
 function canRedirectToAdminDomain(user) {
   if (Settings.adminPrivilegeAvailable) return false
   if (!Settings.adminUrl) return false

services/web/app/src/infrastructure/ExpressLocals.js

@@ -13,15 +13,14 @@ const PackageVersions = require('./PackageVersions')
 const Modules = require('./Modules')
 const Errors = require('../Features/Errors/Errors')
 const {
+  addHasAdminCapabilityToLocals,
   canRedirectToAdminDomain,
-  getAdminCapabilities,
   hasAdminAccess,
 } = require('../Features/Helpers/AdminAuthorizationHelper')
 const {
   addOptionalCleanupHandlerAfterDrainingConnections,
 } = require('./GracefulShutdown')
 const { sanitizeSessionUserForFrontEnd } = require('./FrontEndUser')
-const { expressify } = require('@overleaf/promise-utils')
 
 const IEEE_BRAND_ID = Settings.ieeeBrandId
 
@@ -314,34 +313,7 @@ module.exports = function (webRouter, privateApiRouter, publicApiRouter) {
     next()
   })
 
-  webRouter.use(
-    expressify(async function (req, res, next) {
-      const user = SessionManager.getSessionUser(req.session)
-      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
-          }
-          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')
-        }
-        // adminCapabilitiesAvailable should be true if we are here so deny to be safe
-        res.locals.hasAdminCapability = () => false
-      }
-
-      next()
-    })
-  )
+  webRouter.use(addHasAdminCapabilityToLocals)
 
   webRouter.use(function (req, res, next) {
     // Clone the nav settings so they can be modified for each request

services/web/test/unit/src/HelperFiles/AdminAuthorizationHelperTests.js

@@ -1,6 +1,8 @@
 const { expect } = require('chai')
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
+const MockRequest = require('../helpers/MockRequest')
+const MockResponse = require('../helpers/MockResponse')
 
 const modulePath =
   '../../../../app/src/Features/Helpers/AdminAuthorizationHelper'
@@ -57,4 +59,169 @@ describe('AdminAuthorizationHelper', function () {
       })
     })
   })
+  describe('addHasAdminCapabilityToLocals', function () {
+    describe('when getting capabilities from modules throws an error', function () {
+      beforeEach(async function () {
+        this.fireHook.rejects(new Error('Module error'))
+
+        this.req = new MockRequest()
+        this.res = new MockResponse()
+        this.next = sinon.stub()
+
+        this.user = {
+          isAdmin: false,
+        }
+
+        this.req.logger = {
+          warn: sinon.stub(),
+        }
+
+        this.req.session = {
+          user: this.user,
+        }
+
+        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
+      })
+      it('logs a warning', function () {
+        expect(this.logger.warn).to.have.been.calledOnce
+        expect(this.logger.warn.firstCall.args[0]).to.have.property('error')
+        expect(this.logger.warn.firstCall.args[0].error.message).to.equal(
+          'Module error'
+        )
+      })
+    })
+    describe('when admin capabilities are not available', function () {
+      describe('user is not an admin', function () {
+        beforeEach(async function () {
+          this.req = new MockRequest()
+          this.res = new MockResponse()
+          this.next = sinon.stub()
+
+          this.user = {
+            isAdmin: false,
+          }
+
+          this.req.session = {
+            user: this.user,
+          }
+
+          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 an admin', function () {
+        beforeEach(async function () {
+          this.req = new MockRequest()
+          this.res = new MockResponse()
+          this.next = sinon.stub()
+
+          this.user = {
+            isAdmin: true,
+          }
+
+          this.req.session = {
+            user: this.user,
+          }
+
+          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 true when called with any capability', function () {
+          expect(this.res.locals.hasAdminCapability('capability1')).to.be.true
+        })
+      })
+    })
+    describe('when admin capabilities are available', function () {
+      beforeEach(function () {
+        this.fireHook.resolves(['capability1', 'capability2'])
+      })
+      describe('user is not an admin', function () {
+        beforeEach(async function () {
+          this.req = new MockRequest()
+          this.res = new MockResponse()
+          this.next = sinon.stub()
+
+          this.user = {
+            isAdmin: false,
+          }
+
+          this.req.session = {
+            user: this.user,
+          }
+
+          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 a capability the user has', function () {
+          expect(this.res.locals.hasAdminCapability('capability1')).to.be.false
+        })
+        it('returns false when called with a capability the user does not have', function () {
+          expect(this.res.locals.hasAdminCapability('capability3')).to.be.false
+        })
+      })
+      describe('user is an admin', function () {
+        beforeEach(async function () {
+          this.req = new MockRequest()
+          this.res = new MockResponse()
+          this.next = sinon.stub()
+
+          this.user = {
+            isAdmin: true,
+          }
+
+          this.req.session = {
+            user: this.user,
+          }
+
+          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 true when called with a capability the user has', function () {
+          expect(this.res.locals.hasAdminCapability('capability2')).to.be.true
+        })
+        it('returns false when called with a capability the user does not have', function () {
+          expect(this.res.locals.hasAdminCapability('capability3')).to.be.false
+        })
+      })
+    })
+  })
 })