Repository: cdimascio/express-openapi-validator (ts)

Issue:
multipartMiddleware preceeds securityMiddleware so files begin uploading before security check
**Describe the bug**

When `OpenApiValidator.middleware()` is configured to apply both upload and security middleware...
```js
OpenApiValidator.middleware({
  ...
  fileUploader: {
    storage: myDiskStorageEngine,
  },
  validateSecurity: {
    handlers: {
      securityCheck: mySecurityCheck,
    },
  },
})
```
... the upload middleware (`multipartMiddleware`) runs before the security middleware (`securityMiddleware `). As a result, file uploads begin even for users that fail the security check.

The order of the middlewares is defined here:
https://github.com/cdimascio/express-openapi-validator/blob/bb8d6b80c4bc42be9213d03c46b5d3f0195d1040/src/openapi.validator.ts#L152-L179

**To Reproduce**
1. Configure `OpenApiValidator.middleware` with both a `fileUploader.storage` storage engine and a `validateSecurity` handler.
1. Attempt to upload a file using multipart/form-data with a request that does not satisfy `validateSecurity` handler.

**Actual behavior** 
File upload will be processed. For example, if the multer storage engine is a disk storage engine, the file is written to disk.

**Expected behavior**
The security check should abort the request pipeline before the file upload is processed.

**Examples and context**
I've tested reversing the order of the above mentioned middleware locally by editing `node_modules/express-openapi-validator/dist/openapi.validator.js` and the security check works as expected (file upload does not begin). I am willing to create a PR for this change, but am unsure whether there's a reason for the current order (maybe the security check for some users depends on form data?). If so, perhaps the order could be configurable?

Pull request:
Reorder upload and security middlewares
- Move multipart middleware after security middleware so that security handlers can abort request pipeline before uploads are processed.

Fixes #865

Patch:
diff --git a/src/framework/openapi.context.ts b/src/framework/openapi.context.ts
index 254ed37..2df421f 100644
--- a/src/framework/openapi.context.ts
+++ b/src/framework/openapi.context.ts
@@ -11,16 +11,23 @@ export class OpenApiContext {
   public readonly openApiRouteMap = {};
   public readonly routes: RouteMetadata[] = [];
   public readonly ignoreUndocumented: boolean;
+  public readonly useRequestUrl: boolean;
   private readonly basePaths: string[];
   private readonly ignorePaths: RegExp | Function;
 
-  constructor(spec: Spec, ignorePaths: RegExp | Function, ignoreUndocumented: boolean = false) {
+  constructor(
+    spec: Spec,
+    ignorePaths: RegExp | Function,
+    ignoreUndocumented: boolean = false,
+    useRequestUrl = false,
+  ) {
     this.apiDoc = spec.apiDoc;
     this.basePaths = spec.basePaths;
     this.routes = spec.routes;
     this.ignorePaths = ignorePaths;
     this.ignoreUndocumented = ignoreUndocumented;
     this.buildRouteMaps(spec.routes);
+    this.useRequestUrl = useRequestUrl;
   }
 
   public isManagedRoute(path: string): boolean {
diff --git a/src/framework/types.ts b/src/framework/types.ts
index 6f7c58a..201e6fb 100644
--- a/src/framework/types.ts
+++ b/src/framework/types.ts
@@ -118,6 +118,7 @@ export interface OpenApiValidatorOpts {
   ignoreUndocumented?: boolean;
   securityHandlers?: SecurityHandlers;
   coerceTypes?: boolean | 'array';
+  useRequestUrl?: boolean;
   /**
    * @deprecated
    * Use `formats` + `validateFormats` to ignore specified formats
diff --git a/src/middlewares/openapi.metadata.ts b/src/middlewares/openapi.metadata.ts
index 85f2e9c..2dae08a 100644
--- a/src/middlewares/openapi.metadata.ts
+++ b/src/middlewares/openapi.metadata.ts
@@ -25,12 +25,12 @@ export function applyOpenApiMetadata(
     if (openApiContext.shouldIgnoreRoute(path)) {
       return next();
     }
-    const matched = lookupRoute(req);
+    const matched = lookupRoute(req, openApiContext.useRequestUrl);
     if (matched) {
       const { expressRoute, openApiRoute, pathParams, schema } = matched;
       if (!schema) {
         // Prevents validation for routes which match on path but mismatch on method
-        if(openApiContext.ignoreUndocumented) {
+        if (openApiContext.ignoreUndocumented) {
           return next();
         }
         throw new MethodNotAllowed({
@@ -54,7 +54,10 @@ export function applyOpenApiMetadata(
         // add the response schema if validating responses
         (<any>req.openapi)._responseSchema = (<any>matched)._responseSchema;
       }
-    } else if (openApiContext.isManagedRoute(path) && !openApiContext.ignoreUndocumented) {
+    } else if (
+      openApiContext.isManagedRoute(path) &&
+      !openApiContext.ignoreUndocumented
+    ) {
       throw new NotFound({
         path: req.path,
         message: 'not found',
@@ -63,8 +66,11 @@ export function applyOpenApiMetadata(
     next();
   };
 
-  function lookupRoute(req: OpenApiRequest): OpenApiRequestMetadata {
-    const path = req.originalUrl.split('?')[0];
+  function lookupRoute(
+    req: OpenApiRequest,
+    useRequestUrl: boolean,
+  ): OpenApiRequestMetadata {
+    const path = useRequestUrl ? req.url : req.originalUrl.split('?')[0];
     const method = req.method;
     const routeEntries = Object.entries(openApiContext.expressRouteMap);
     for (const [expressRoute, methods] of routeEntries) {
diff --git a/src/openapi.validator.ts b/src/openapi.validator.ts
index a578ef5..dd12fed 100644
--- a/src/openapi.validator.ts
+++ b/src/openapi.validator.ts
@@ -49,6 +49,7 @@ export class OpenApiValidator {
     if (options.$refParser == null) options.$refParser = { mode: 'bundle' };
     if (options.validateFormats == null) options.validateFormats = true;
     if (options.formats == null) options.formats = {};
+    if (options.useRequestUrl == null) options.useRequestUrl = false;
 
     if (typeof options.operationHandlers === 'string') {
       /**
@@ -103,7 +104,12 @@ export class OpenApiValidator {
           resOpts,
         ).preProcess();
         return {
-          context: new OpenApiContext(spec, this.options.ignorePaths, this.options.ignoreUndocumented),
+          context: new OpenApiContext(
+            spec,
+            this.options.ignorePaths,
+            this.options.ignoreUndocumented,
+            this.options.useRequestUrl,
+          ),
           responseApiDoc: sp.apiDocRes,
           error: null,
         };
@@ -149,19 +155,6 @@ export class OpenApiValidator {
         .catch(next);
     });
 
-    if (this.options.fileUploader) {
-      // multipart middleware
-      let fumw;
-      middlewares.push(function multipartMiddleware(req, res, next) {
-        return pContext
-          .then(({ context: { apiDoc } }) => {
-            fumw = fumw || self.multipartMiddleware(apiDoc);
-            return fumw(req, res, next);
-          })
-          .catch(next);
-      });
-    }
-
     // security middlware
     let scmw;
     middlewares.push(function securityMiddleware(req, res, next) {
@@ -178,6 +171,19 @@ export class OpenApiValidator {
         .catch(next);
     });
 
+    if (this.options.fileUploader) {
+      // multipart middleware
+      let fumw;
+      middlewares.push(function multipartMiddleware(req, res, next) {
+        return pContext
+          .then(({ context: { apiDoc } }) => {
+            fumw = fumw || self.multipartMiddleware(apiDoc);
+            return fumw(req, res, next);
+          })
+          .catch(next);
+      });
+    }
+
     // request middlweare
     if (this.options.validateRequests) {
       let reqmw;
@@ -201,7 +207,7 @@ export class OpenApiValidator {
             return resmw(req, res, next);
           })
           .catch(next);
-      })
+      });
     }
 
     // op handler middleware


Test patch:
diff --git a/test/user-request-url.router.spec.ts b/test/user-request-url.router.spec.ts
new file mode 100644
index 0000000..4bba5de
--- /dev/null
+++ b/test/user-request-url.router.spec.ts
@@ -0,0 +1,272 @@
+import { expect } from 'chai';
+import type {
+  Express,
+  IRouter,
+  Response,
+  NextFunction,
+  Request,
+} from 'express';
+import * as express from 'express';
+import { OpenAPIV3 } from '../src/framework/types';
+import * as request from 'supertest';
+import { createApp } from './common/app';
+
+import * as OpenApiValidator from '../src';
+import { Server } from 'http';
+
+interface HTTPError extends Error {
+  status: number;
+  text: string;
+  method: string;
+  path: string;
+}
+
+describe('when useRequestUrl is set to "true" on the child router', async () => {
+  let app: Express & { server?: Server };
+
+  before(async () => {
+    const router = makeRouter({ useRequestUrl: true });
+    app = await makeMainApp();
+    app.use(router);
+  });
+
+  after(() => app?.server?.close());
+
+  it('should apply parent app schema to requests', async () => {
+    const result = await request(app).get('/api/pets/1');
+    const error = result.error as HTTPError;
+    expect(result.statusCode).to.equal(400);
+    expect(error.path).to.equal('/api/pets/1');
+    expect(error.text).to.contain(
+      'Bad Request: request/params/petId must NOT have fewer than 3 characters',
+    );
+  });
+
+  it('should apply child router schema to requests', async () => {
+    const result = await request(app).get('/api/pets/not-uuid');
+    const error = result.error as HTTPError;
+    expect(result.statusCode).to.equal(400);
+    expect(error.path).to.equal('/api/pets/not-uuid');
+    expect(error.text).to.contain(
+      'Bad Request: request/params/petId must match format &quot;uuid&quot',
+    );
+  });
+
+  it('should return a reponse if request is valid', async () => {
+    const validId = 'f627f309-cae3-46d2-84f7-d03856c84b02';
+    const result = await request(app).get(`/api/pets/${validId}`);
+    expect(result.statusCode).to.equal(200);
+    expect(result.body).to.deep.equal({
+      id: 'f627f309-cae3-46d2-84f7-d03856c84b02',
+      name: 'Mr Sparky',
+      tag: "Ain't nobody tags me",
+    });
+  });
+});
+
+describe('when useRequestUrl is set to "false" on the child router', async () => {
+  let app: Express & { server?: Server };
+
+  before(async () => {
+    const router = makeRouter({ useRequestUrl: false });
+    app = await makeMainApp();
+    app.use(router);
+  });
+
+  after(() => app?.server?.close());
+
+  it('should throw not found', async () => {
+    const result = await request(app).get('/api/pets/valid-pet-id');
+    const error = result.error as HTTPError;
+    expect(result.statusCode).to.equal(404);
+    expect(error.path).to.equal('/api/pets/valid-pet-id');
+    expect(error.text).to.contain('Not Found');
+  });
+});
+
+function defaultResponse(): OpenAPIV3.ResponseObject {
+  return {
+    description: 'unexpected error',
+    content: {
+      'application/json': {
+        schema: {
+          type: 'object',
+          required: ['code', 'message'],
+          properties: {
+            code: {
+              type: 'integer',
+              format: 'int32',
+            },
+            message: {
+              type: 'string',
+            },
+          },
+        },
+      },
+    },
+  };
+}
+
+/* 
+   represents spec of the "public" entrypoint to our application e.g gateway. The
+   type of id in path and id in the response here defined as simple string
+   with minLength
+ */
+const gatewaySpec: OpenAPIV3.Document = {
+  openapi: '3.0.0',
+  info: { version: '1.0.0', title: 'test bug OpenApiValidator' },
+  servers: [{ url: 'http://localhost:3000/api' }],
+  paths: {
+    '/pets/{petId}': {
+      get: {
+        summary: 'Info for a specific pet',
+        operationId: 'showPetById',
+        tags: ['pets'],
+        parameters: [
+          {
+            name: 'petId',
+            in: 'path',
+            required: true,
+            description: 'The id of the pet to retrieve',
+            schema: {
+              type: 'string',
+              minLength: 3,
+            },
+          },
+        ],
+        responses: {
+          '200': {
+            description: 'Expected response to a valid request',
+            content: {
+              'application/json': {
+                schema: {
+                  type: 'object',
+                  required: ['id', 'name'],
+                  properties: {
+                    id: {
+                      type: 'string',
+                    },
+                    name: {
+                      type: 'string',
+                    },
+                    tag: {
+                      type: 'string',
+                    },
+                  },
+                },
+              },
+            },
+          },
+          default: defaultResponse(),
+        },
+      },
+    },
+  },
+};
+
+/*
+ represents spec of the child router. We route request from main app (gateway) to this router.
+ This router has its own schema, routes and validation formats. In particular, we force id in the path and id in the response to be uuid.
+ */
+const childRouterSpec: OpenAPIV3.Document = {
+  openapi: '3.0.0',
+  info: { version: '1.0.0', title: 'test bug OpenApiValidator' },
+  servers: [{ url: 'http://localhost:3000/' }],
+  paths: {
+    '/internal/api/pets/{petId}': {
+      get: {
+        summary: 'Info for a specific pet',
+        operationId: 'showPetById',
+        tags: ['pets'],
+        parameters: [
+          {
+            name: 'petId',
+            in: 'path',
+            required: true,
+            description: 'The id of the pet to retrieve',
+            schema: {
+              type: 'string',
+              format: 'uuid',
+            },
+          },
+        ],
+        responses: {
+          '200': {
+            description: 'Expected response to a valid request',
+            content: {
+              'application/json': {
+                schema: {
+                  type: 'object',
+                  required: ['id', 'name'],
+                  properties: {
+                    id: {
+                      type: 'string',
+                      format: 'uuid',
+                    },
+                    name: {
+                      type: 'string',
+                    },
+                    tag: {
+                      type: 'string',
+                    },
+                  },
+                },
+              },
+            },
+          },
+        },
+      },
+    },
+  },
+};
+
+function redirectToInternalService(
+  req: Request,
+  _res: Response,
+  next: NextFunction,
+): void {
+  req.url = `/internal${req.originalUrl}`;
+  next();
+}
+
+function makeMainApp(): ReturnType<typeof createApp> {
+  return createApp(
+    {
+      apiSpec: gatewaySpec,
+      validateResponses: true,
+      validateRequests: true,
+    },
+    3000,
+    (app) => {
+      app
+        .get(
+          '/api/pets/:petId',
+          function (_req: Request, _res: Response, next: NextFunction) {
+            next();
+          },
+        )
+        .use(redirectToInternalService);
+    },
+    false,
+  );
+}
+
+function makeRouter({ useRequestUrl }: { useRequestUrl: boolean }): IRouter {
+  return express
+    .Router()
+    .use(
+      OpenApiValidator.middleware({
+        apiSpec: childRouterSpec,
+        validateRequests: true,
+        validateResponses: true,
+        useRequestUrl,
+      }),
+    )
+    .get('/internal/api/pets/:petId', function (req, res) {
+      res.json({
+        id: req.params.petId,
+        name: 'Mr Sparky',
+        tag: "Ain't nobody tags me",
+      });
+    });
+}