Bump qs and express

package.json

@@ -35,7 +35,7 @@
         "cors": "^2.8.5",
         "crypto-random-string": "^3.3.1",
         "dotenv": "^5.0.1",
-        "express": "^4.22.2",
+        "express": "^5.2.1",
         "express-jsonschema": "^1.1.6",
         "express-rate-limit": "^6.5.2",
         "express-validator": "^6.14.2",

src/controller/cve-id.controller/cve-id.middleware.js

@@ -4,7 +4,7 @@ const error = new errors.CveIdControllerError()
 const utils = require('../../utils/utils')
 
 function parseGetParams (req, res, next) {
-  utils.reqCtxMapping(req, 'query', ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt'])
+  utils.reqCtxValidatedQueryMapping(req, ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt'])
   utils.reqCtxMapping(req, 'params', ['id'])
   next()
 }

src/controller/cve.controller/cve.middleware.js

@@ -21,7 +21,7 @@ function parsePostParams (req, res, next) {
 }
 
 function parseGetParams (req, res, next) {
-  utils.reqCtxMapping(req, 'query', ['page', 'time_modified.lt', 'time_modified.gt', 'time_created.lt', 'time_created.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit'])
+  utils.reqCtxValidatedQueryMapping(req, ['page', 'time_modified.lt', 'time_modified.gt', 'time_created.lt', 'time_created.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit'])
   utils.reqCtxMapping(req, 'params', ['id'])
   next()
 }

src/controller/org.controller/org.middleware.js

@@ -149,7 +149,7 @@ function parsePutParams (req, res, next) {
     ...QUERY_PARAMETERS.registryOnly,
     ...QUERY_PARAMETERS.userParams
   ]
-  utils.reqCtxMapping(req, 'query', allQueryParams)
+  utils.reqCtxValidatedQueryMapping(req, allQueryParams)
   utils.reqCtxMapping(req, 'params', ['shortname', 'username', 'identifier'])
   next()
 }

src/index.js

@@ -3,6 +3,9 @@ const cors = require('cors')
 const config = require('config')
 const express = require('express')
 const app = express()
+// Express 5 defaults to the simple query parser. Keep the qs-based parser used
+// by Express 4 so existing bracket/nested query parameters retain their API.
+app.set('query parser', 'extended')
 const helmet = require('helmet')
 const mongoose = require('mongoose')
 const morgan = require('morgan')

src/utils/utils.js

@@ -8,6 +8,7 @@ const getConstants = require('../constants').getConstants
 const _ = require('lodash')
 const { DateTime } = require('luxon')
 const BaseOrgRepository = require('../repositories/baseOrgRepository')
+const { matchedData } = require('express-validator')
 
 async function getOrgUUID (shortName, useRegistry = false, options = {}) {
   const ModelToQuery = useRegistry ? BaseOrg : Org
@@ -177,6 +178,31 @@ function reqCtxMapping (req, keyType, keys) {
   }
 }
 
+// Express 5 exposes req.query as a getter which reparses the URL on every
+// access. express-validator sanitizers therefore cannot persist changes on
+// req.query. Copy the validator context instead, while retaining the flat
+// dotted keys expected by the legacy controllers.
+function reqCtxValidatedQueryMapping (req, keys) {
+  if (!('query' in req.ctx)) {
+    req.ctx.query = {}
+  }
+
+  const validatedQuery = matchedData(req, {
+    locations: ['query'],
+    includeOptionals: true
+  })
+
+  keys.forEach(key => {
+    // Direct controller callers (including unit tests) do not always run the
+    // route validation chains. Retain their allowed raw query value when no
+    // validator context exists for the key.
+    const value = _.get(validatedQuery, key) ?? req.query[key]
+    if (value !== undefined) {
+      req.ctx.query[key] = value
+    }
+  })
+}
+
 // Return true if boolean is 0, true, or yes, with any mix of casing
 // Please note that this function does NOT evaluate "undefined" as false. - A tired developer who lost way too much time to this.
 function booleanIsTrue (val) {
@@ -334,6 +360,7 @@ module.exports = {
   getUserUUID,
   getUserFullName,
   reqCtxMapping,
+  reqCtxValidatedQueryMapping,
   booleanIsTrue,
   toDate,
   convertDatesToISO

test/integration-tests/middleware/validatedQueryContextTest.js

@@ -0,0 +1,80 @@
+const chai = require('chai')
+chai.use(require('chai-http'))
+const expect = chai.expect
+const express = require('express')
+const { query } = require('express-validator')
+
+const utils = require('../../../src/utils/utils')
+const toDate = require('../../../src/utils/utils').toDate
+
+describe('Validated query request context', () => {
+  it('retains a sanitized date when Express 5 reparses req.query', async () => {
+    const app = express()
+    app.set('query parser', 'extended')
+    app.use((req, res, next) => {
+      req.ctx = {}
+      next()
+    })
+    app.get('/date',
+      query('time_modified.gt').customSanitizer(toDate),
+      (req, res) => {
+        utils.reqCtxValidatedQueryMapping(req, ['time_modified.gt'])
+        return res.status(200).json({
+          isDate: req.ctx.query['time_modified.gt'] instanceof Date,
+          value: req.ctx.query['time_modified.gt'].toISOString()
+        })
+      })
+
+    const response = await chai.request(app)
+      .get('/date?time_modified.gt=2022-01-01T00:00:00Z')
+
+    expect(response).to.have.status(200)
+    expect(response.body).to.deep.equal({
+      isDate: true,
+      value: '2022-01-01T00:00:00.000Z'
+    })
+  })
+
+  it('retains validator-produced role arrays for organization updates', async () => {
+    const app = express()
+    app.set('query parser', 'extended')
+    app.use((req, res, next) => {
+      req.ctx = {}
+      next()
+    })
+    app.put('/roles',
+      query('active_roles.add').toArray(),
+      query('active_roles.remove').toArray(),
+      (req, res) => {
+        utils.reqCtxValidatedQueryMapping(req, ['active_roles.add', 'active_roles.remove'])
+        return res.status(200).json(req.ctx.query)
+      })
+
+    const response = await chai.request(app)
+      .put('/roles?active_roles.add=ADMIN&active_roles.remove=CNA')
+
+    expect(response).to.have.status(200)
+    expect(response.body).to.deep.equal({
+      'active_roles.add': ['ADMIN'],
+      'active_roles.remove': ['CNA']
+    })
+  })
+
+  it('retains allowed raw query values when no route validators ran', async () => {
+    const app = express()
+    app.use((req, res, next) => {
+      req.ctx = {}
+      next()
+    })
+    app.put('/direct-controller', (req, res) => {
+      utils.reqCtxValidatedQueryMapping(req, ['new_short_name'])
+      return res.status(200).json(req.ctx.query)
+    })
+
+    const response = await chai.request(app)
+      .put('/direct-controller?new_short_name=renamed-org')
+
+    expect(response).to.have.status(200)
+    expect(response.body).to.deep.equal({ new_short_name: 'renamed-org' })
+  })
+})