Fix: HEAD requests to API routes crash auth middleware (#76)

Author: taylortom

lib/AuthModule.js

@@ -158,10 +158,12 @@ class AuthModule extends AbstractModule {
     } catch (e) {
       initError = e
     }
-    const method = req.method.toLowerCase()
+    // Treat HEAD as GET: Express runs the GET handler chain for HEAD requests,
+    // so auth rules must follow suit.
+    const method = req.method === 'HEAD' ? 'get' : req.method.toLowerCase()
     const route = `${req.baseUrl}${req.route.path}`
     const shortRoute = route.slice(0, route.lastIndexOf('/'))
-    const isUnsecured = this.unsecuredRoutes[method][route] || this.unsecuredRoutes[method][shortRoute]
+    const isUnsecured = this.unsecuredRoutes[method]?.[route] || this.unsecuredRoutes[method]?.[shortRoute]
 
     if (initError && !isUnsecured) {
       this.log('debug', 'BLOCK_REQUEST', req.originalUrl, initError.statusCode, req?.auth?.user?._id)

lib/Permissions.js

@@ -92,7 +92,9 @@ class Permissions {
   async check (req) {
     const route = `${req.baseUrl}${req.path.endsWith('/') ? req.path.slice(0, -1) : req.path}`
     const userScopes = req.auth.scopes || []
-    const neededScopes = this.getScopesForRoute(req.method.toLowerCase(), route)
+    // HEAD reuses the GET handler chain in Express, so it must reuse GET's scopes too.
+    const method = req.method === 'HEAD' ? 'get' : req.method.toLowerCase()
+    const neededScopes = this.getScopesForRoute(method, route)
     if (!neededScopes) {
       log('warn', `blocked access to route with no permissions '${route}'`)
     }

tests/AuthModule.spec.js

@@ -189,5 +189,31 @@ describe('AuthModule', () => {
       await authModule.apiMiddleware(req, res, () => {})
       assert.equal(permissionsChecked, true)
     })
+
+    it('should treat HEAD as GET when checking unsecured routes', async () => {
+      // Express runs the GET handler chain for HEAD requests, so HEAD must
+      // resolve unsecured state against the GET entry rather than crashing on
+      // an absent `head` bucket in the store.
+      const authModule = new AuthModule(createMockApp(), { name: 'test-auth' })
+      authModule.unsecuredRoutes = { get: { '/api/config': true }, post: {}, put: {}, patch: {}, delete: {} }
+      authModule.isEnabled = false
+      authModule.permissions = {
+        check: async () => { assert.fail('permissions.check should not run for a GET-unsecured route') }
+      }
+
+      let nextCalled = false
+      const req = {
+        get: () => undefined,
+        headers: {},
+        method: 'HEAD',
+        baseUrl: '/api',
+        route: { path: '/config/' },
+        originalUrl: '/api/config'
+      }
+      const res = { sendError: () => {} }
+
+      await authModule.apiMiddleware(req, res, () => { nextCalled = true })
+      assert.equal(nextCalled, true)
+    })
   })
 })

tests/Permissions.spec.js

@@ -140,6 +140,21 @@ describe('Permissions', () => {
 
       await assert.doesNotReject(() => permissions.check(req))
     })
+
+    it('should treat HEAD as GET when resolving scopes', async () => {
+      // Express runs the GET handler chain for HEAD requests; the `head`
+      // bucket is absent from the store, so without aliasing this throws.
+      permissions.secureRoute('/api/head-alias', 'get', ['read:head-alias'])
+
+      const req = {
+        baseUrl: '/api',
+        path: '/head-alias',
+        method: 'HEAD',
+        auth: { isSuper: false, scopes: ['read:head-alias'] }
+      }
+
+      await assert.doesNotReject(() => permissions.check(req))
+    })
   })
 
   describe('constructor', () => {