Skip to content

Commit

Permalink
feat: Specify allow unknown params on a per-operation level (#477)
Browse files Browse the repository at this point in the history
* feat: Specify allow unknown params on a per-operation level

* chore: Additional Prettier fixes

* fix: do not modify vendor typing, use `!allow` over `disallow`
  • Loading branch information
JacobLey committed Dec 3, 2020
1 parent a7e7812 commit 4e737b8
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 20 deletions.
20 changes: 20 additions & 0 deletions README.md
Expand Up @@ -543,6 +543,26 @@ Determines whether the validator should validate requests.
}
```

`allowUnknownQueryParameters` is set for the entire validator. It can be overwritten per-operation using
a custom property `x-allow-unknown-query-parameters`.

For example to allow unknown query parameters on ONLY a single endpoint:

```yaml
paths:
/allow_unknown:
get:
x-allow-unknown-query-parameters: true
parameters:
- name: value
in: query
schema:
type: string
responses:
200:
description: success
```

### 鈻笍 validateResponses (optional)

Determines whether the validator should validate responses. Also accepts response validation options.
Expand Down
24 changes: 13 additions & 11 deletions src/middlewares/openapi.request.validator.ts
Expand Up @@ -107,6 +107,11 @@ export class RequestValidator {
body: this.ajvBody,
});

const allowUnknownQueryParameters = !!(
reqSchema['x-allow-unknown-query-parameters'] ??
this.requestOpts.allowUnknownQueryParameters
);

return (req: OpenApiRequest, res: Response, next: NextFunction): void => {
const openapi = <OpenApiRequestMetadata>req.openapi;
const hasPathParams = Object.keys(openapi.pathParams).length > 0;
Expand All @@ -125,7 +130,7 @@ export class RequestValidator {

mutator.modifyRequest(req);

if (!this.requestOpts.allowUnknownQueryParameters) {
if (!allowUnknownQueryParameters) {
this.processQueryParam(
req.query,
schemaPoperties.query,
Expand Down Expand Up @@ -184,13 +189,13 @@ export class RequestValidator {
if (discriminator) {
const { options, property, validators } = discriminator;
const discriminatorValue = req.body[property]; // TODO may not alwasy be in this position
if (options.find((o) => o.option === discriminatorValue)) {
if (options.find(o => o.option === discriminatorValue)) {
return validators[discriminatorValue];
} else {
throw new BadRequest({
path: req.path,
message: `'${property}' should be equal to one of the allowed values: ${options
.map((o) => o.option)
.map(o => o.option)
.join(', ')}.`,
});
}
Expand All @@ -208,14 +213,11 @@ export class RequestValidator {
keys.push(key);
}
const knownQueryParams = new Set(keys);
whiteList.forEach((item) => knownQueryParams.add(item));
whiteList.forEach(item => knownQueryParams.add(item));
const queryParams = Object.keys(query);
const allowedEmpty = schema.allowEmptyValue;
for (const q of queryParams) {
if (
!this.requestOpts.allowUnknownQueryParameters &&
!knownQueryParams.has(q)
) {
if (!knownQueryParams.has(q)) {
throw new BadRequest({
path: `.query.${q}`,
message: `Unknown query parameter '${q}'`,
Expand Down Expand Up @@ -322,12 +324,12 @@ class Security {
): string[] {
return usedSecuritySchema && securitySchema
? usedSecuritySchema
.filter((obj) => Object.entries(obj).length !== 0)
.map((sec) => {
.filter(obj => Object.entries(obj).length !== 0)
.map(sec => {
const securityKey = Object.keys(sec)[0];
return <SecuritySchemeObject>securitySchema[securityKey];
})
.filter((sec) => sec?.type === 'apiKey' && sec?.in == 'query')
.filter(sec => sec?.type === 'apiKey' && sec?.in == 'query')
.map((sec: ApiKeySecurityScheme) => sec.name)
: [];
}
Expand Down
13 changes: 13 additions & 0 deletions test/query.params.allow.unknown.spec.ts
@@ -1,3 +1,4 @@
import { expect } from 'chai';
import * as path from 'path';
import * as express from 'express';
import * as request from 'supertest';
Expand Down Expand Up @@ -52,4 +53,16 @@ describe(packageJson.name, () => {
unknown_prop: 'test',
})
.expect(200));

it('should fail if operation overrides x-allow-unknown-query-parameters=false', async () =>
request(app)
.get(`${app.basePath}/unknown_query_params/disallow`)
.query({
value: 'foobar',
unknown_prop: 'test',
})
.expect(400)
.then(r => {
expect(r.body.errors).to.be.an('array');
}));
});
30 changes: 22 additions & 8 deletions test/query.params.spec.ts
Expand Up @@ -11,15 +11,18 @@ describe(packageJson.name, () => {
before(async () => {
// Set up the express app
const apiSpec = path.join('test', 'resources', 'query.params.yaml');
app = await createApp({ apiSpec }, 3005, (app) =>
app = await createApp({ apiSpec }, 3005, app =>
app.use(
`${app.basePath}`,
express
.Router()
.post(`/pets/nullable`, (req, res) => res.json(req.body))
.get(`/no_reserved`, (req, res) => res.json(req.body))
.get(`/no_query_params`, (req, res) => res.json({ complete: true }))
.get(`/allow_reserved`, (req, res) => res.json(req.body)),
.get(`/allow_reserved`, (req, res) => res.json(req.body))
.get(`/unknown_query_params/allow`, (req, res) =>
res.json({ complete: true }),
),
),
);
});
Expand Down Expand Up @@ -47,15 +50,15 @@ describe(packageJson.name, () => {
name: 'max',
})
.expect(400)
.then((r) => {
.then(r => {
expect(r.body.errors).to.be.an('array');
}));

it('should return 200 if no query params are supplied', async () =>
request(app)
.get(`${app.basePath}/no_query_params`)
.expect(200)
.then((r) => {
.then(r => {
expect(r.body.complete).to.equal(true);
}));

Expand All @@ -71,10 +74,19 @@ describe(packageJson.name, () => {
unknown_prop: 'test',
})
.expect(400)
.then((r) => {
.then(r => {
expect(r.body.errors).to.be.an('array');
}));

it('should return 200 if operation overrides x-allow-unknown-query-parameters=true', async () =>
request(app)
.get(`${app.basePath}/unknown_query_params/allow`)
.query({
value: 'foobar',
unknown_prop: 'test',
})
.expect(200));

it('should not allow empty query param value', async () =>
request(app)
.get(`${app.basePath}/pets`)
Expand All @@ -86,11 +98,13 @@ describe(packageJson.name, () => {
owner_name: 'carmine',
})
.expect(400)
.then((r) => {
.then(r => {
expect(r.body)
.to.have.property('message')
.that.equals("Empty value found for query parameter 'breed'");
expect(r.body.errors).to.be.an('array').with.length(1);
expect(r.body.errors)
.to.be.an('array')
.with.length(1);
expect(r.body.errors[0].path).to.equal('.query.breed');
}));

Expand All @@ -117,7 +131,7 @@ describe(packageJson.name, () => {
request(app)
.get(`${app.basePath}/no_reserved?value=ThisHas$ReservedChars!`)
.expect(400)
.then((r) => {
.then(r => {
const body = r.body;
expect(body.message).equals(
"Parameter 'value' must be url encoded. It's value may not contain reserved characters.",
Expand Down
26 changes: 25 additions & 1 deletion test/resources/query.params.yaml
Expand Up @@ -69,7 +69,7 @@ paths:
responses:
'200':
description: success
/no_query_params:
/no_query_params:
get:
description: test no query parameters
responses:
Expand Down Expand Up @@ -118,6 +118,30 @@ paths:
type: array
items:
$ref: '#/components/schemas/Pet'
/unknown_query_params/allow:
get:
x-allow-unknown-query-parameters: true
parameters:
- name: value
in: query
required: true
schema:
type: string
responses:
200:
description: success
/unknown_query_params/disallow:
get:
x-allow-unknown-query-parameters: false
parameters:
- name: value
in: query
required: true
schema:
type: string
responses:
200:
description: success
components:
parameters:
name:
Expand Down

0 comments on commit 4e737b8

Please sign in to comment.