Merge pull request #112 from comino/fix-#111

fix-#111 endpoint query parameters overwrite security query parameters
cdimascio · Nov 9, 2019 · 87affd0 · 87affd0
2 parents ec5863e + 82c4bed
commit 87affd0
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 17 deletions.
diff --git a/src/middlewares/openapi.request.validator.ts b/src/middlewares/openapi.request.validator.ts
@@ -64,6 +64,11 @@ export class RequestValidator {
 
   private buildMiddleware(path, pathSchema, contentType) {
     const parameters = this.parametersToSchema(path, pathSchema.parameters);
+    const securityQueryParameter = this.getSecurityQueryParams(
+      pathSchema,
+      this._apiDocs.components.securitySchemes,
+    );
+
     let requestBody = pathSchema.requestBody;
 
     if (requestBody && requestBody.hasOwnProperty('$ref')) {
@@ -85,7 +90,11 @@ export class RequestValidator {
 
     const validator = this.ajv.compile(schema);
     return (req, res, next) => {
-      this.rejectUnknownQueryParams(req.query, schema.properties.query);
+      this.rejectUnknownQueryParams(
+        req.query,
+        schema.properties.query,
+        securityQueryParameter,
+      );
 
       const shouldUpdatePathParams =
         Object.keys(req.openapi.pathParams).length > 0;
@@ -155,9 +164,10 @@ export class RequestValidator {
     };
   }
 
-  private rejectUnknownQueryParams(query, schema) {
+  private rejectUnknownQueryParams(query, schema, whiteList = []) {
     if (!schema.properties) return;
     const knownQueryParams = new Set(Object.keys(schema.properties));
+    whiteList.forEach(item => knownQueryParams.add(item));
     const queryParams = Object.keys(query);
     for (const q of queryParams) {
       if (!knownQueryParams.has(q)) {
@@ -185,6 +195,19 @@ export class RequestValidator {
     return {};
   }
 
+  private getSecurityQueryParams(pathSchema, securitySchema) {
+    return pathSchema.security && securitySchema
+      ? pathSchema.security
+          .filter(obj => Object.entries(obj).length !== 0)
+          .map(sec => {
+            const securityKey = Object.keys(sec)[0];
+            return securitySchema[securityKey];
+          })
+          .filter(sec => sec && sec.in && sec.in === 'query')
+          .map(sec => sec.name)
+      : [];
+  }
+
   private parametersToSchema(path, parameters = []) {
     const schema = { query: {}, headers: {}, params: {}, cookies: {} };
     const reqFields = {

diff --git a/test/resources/security.top.level.yaml b/test/resources/security.top.level.yaml
@@ -39,6 +39,21 @@ paths:
         '401':
           description: unauthorized
 
+  /api_query_keys:
+    get:
+      security:
+        - ApiKeyQueryAuth: []
+      parameters: 
+        - name: param1
+          in: query
+          schema:
+            type: string
+      responses:
+        '200':
+          description: OK
+        '401':
+          description: unauthorized
+
   /bearer:
     get:
       security:

diff --git a/test/routes.spec.ts b/test/routes.spec.ts
@@ -355,5 +355,6 @@ describe(packageJson.name, () => {
           });
       });
     });
+
   });
 });
diff --git a/test/security.top.level.spec.ts b/test/security.top.level.spec.ts
@@ -26,6 +26,7 @@ describe(packageJson.name, () => {
         .Router()
         .get(`/api_key`, (req, res) => res.json({ logged_in: true }))
         .get(`/api_query_key`, (req, res) => res.json({ logged_in: true }))
+        .get(`/api_query_keys`, (req, res) => res.json({ logged_in: true }))
         .get(`/api_key_or_anonymous`, (req, res) =>
           res.json({ logged_in: true }),
         )
@@ -47,18 +48,14 @@ describe(packageJson.name, () => {
         const body = r.body;
         expect(body.errors).to.be.an('array');
         expect(body.errors).to.have.length(1);
-        expect(body.errors[0].message).to.equals(
-          "'X-API-Key' header required",
-        );
+        expect(body.errors[0].message).to.equals("'X-API-Key' header required");
       }));
 
-
   it('should return 200 if apikey exists', async () =>
     request(app)
       .get(`${basePath}/api_key`)
       .set('X-API-Key', 'test')
-      .expect(200)
-      );
+      .expect(200));
 
   it('should return 404 if apikey exist, but path doesnt exist', async () =>
     request(app)
@@ -69,9 +66,7 @@ describe(packageJson.name, () => {
         const body = r.body;
         expect(body.errors).to.be.an('array');
         expect(body.errors).to.have.length(1);
-        expect(body.errors[0].message).to.equals(
-          'not found',
-        );
+        expect(body.errors[0].message).to.equals('not found');
       }));
 
   it('should return 405 if apikey exist, but invalid method used', async () =>
@@ -83,18 +78,27 @@ describe(packageJson.name, () => {
         const body = r.body;
         expect(body.errors).to.be.an('array');
         expect(body.errors).to.have.length(1);
-        expect(body.errors[0].message).to.equals(
-          'POST method not allowed',
-        );
+        expect(body.errors[0].message).to.equals('POST method not allowed');
       }));
 
   it('should return 200 if apikey exist as query param', async () =>
     request(app)
       .get(`${basePath}/api_query_key`)
-      .query({ "APIKey": 'test' })
-      .expect(200)
-  );
+      .query({ APIKey: 'test' })
+      .expect(200));
 
+  it('should return 200 if apikey exist as query param with another query parmeter in the request', async () =>
+    request(app)
+      .get(`${basePath}/api_query_keys`)
+      .query({ APIKey: 'test' })
+      .query({ param1: 'anotherTest' })
+      .expect(200));
+
+  it('should return 200 if apikey exist as query param with no query parmeter in the request but in the spec', async () =>
+    request(app)
+      .get(`${basePath}/api_query_keys`)
+      .query({ APIKey: 'test' })
+      .expect(200));
   it('should return 200 if apikey or anonymous', async () =>
     request(app)
       .get(`${basePath}/api_key_or_anonymous`)
-Original file line number
+Diff line change
@@ Expand Up / @@ -355,5 +355,6 @@ describe(packageJson.name, () => { @@
               });
           });
         });
       });
     });