From 488c0ab6e642d1516c13aa5a768c61a0409a9e2e Mon Sep 17 00:00:00 2001
From: Andrew Rumble <andrew.rumble@overleaf.com>
Date: Wed, 30 Jul 2025 13:37:49 +0100
Subject: [PATCH] Try checking admin access before getting capabilities

GitOrigin-RevId: 75ff99aa6b6e94c1b84a99d22d65b327f4d40126
---
 .../Helpers/AdminAuthorizationHelper.js       | 12 ++++-----
 .../AdminAuthorizationHelperTests.js          | 25 ++++++++++++++++++-
 2 files changed, 29 insertions(+), 8 deletions(-)

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()