Security definition in query overwritten with additional query parameters

[comino]

Using a APIKey as quere parameter and having more parameters causes the apiKey query parameter beeing rejected "Unknown query parameter apiKey"

Example:

  /api_query_keys:
    get:
      security:
        - ApiKeyQueryAuth: []
      parameters: 
        - name: param1
          in: query
          schema:
            type: string
      responses:
        '200':
          description: OK
        '401':
          description: unauthorized

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

Both tests fail with 400. This was recently introduced and working before.
Investigating

[comino]

Now had tome to start digging, it turns out that unknown query parameters are only rejected if any query parameters are defined for that path.

  private rejectUnknownQueryParams(query, schema) {
    if (!schema.properties) return;
    const knownQueryParams = new Set(Object.keys(schema.properties));
    const queryParams = Object.keys(query);
    for (const q of queryParams) {
      if (!knownQueryParams.has(q)) {
        throw validationError(
          400,
          `.query.${q}`,
          `Unknown query parameter ${q}`,
        );
      }
    }
  }

Since security query parameters dont appear in the schema.query they are rejected in the middleware.

Offtopic#1: If we reject the request on an unknown query parameter we should always reject even if no query parameters were defined at all - so I would remove

  if (!schema.properties) return;

I would rather skip the validation if there arent any query parameters:

  if (!query) return;

Offtopic#2: How about making the rejectUnknownQueryParams optional?

Regarding this issue:
WIP for a fix

[comino]

I came up with two options to fix this:

Option A:
We add the security definitions to the validation Schema:

# openapi.request.validator.ts
const schema = {
      required: ['query', 'headers', 'params'].concat(requiredAdds),
      properties: {
        body,
        ...parameters.schema, // include security parameters here ..
      },
    };

This would also 'validate' all security parameters (not only query), which is already done in the security middleware. Furthermore, the security parameters would appear in req.schema.

Option B:
We 'whitelist' security query parameters in:

  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)) {
        throw validationError(
          400,
          `.query.${q}`,
          `Unknown query parameter ${q}`,
        );
      }
    }
  }

and fetch them in the middleware like:

      // take key from pathSchema.security 
      // resolve key in query viathis._apiDocs.components.securitySchemas
      securityQueryParam = [] 
      this.rejectUnknownQueryParams(req.query, schema.properties.query, securityQueryParam);

I will implement the whitelist option and send a PR, feel free to implement a better solution

[cdimascio]

@comino, your fix is in 2.14.3. thanks again!