Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate parameter values against their schema #1573

Closed
bajtos opened this issue Jul 30, 2018 · 11 comments
Closed

Validate parameter values against their schema #1573

bajtos opened this issue Jul 30, 2018 · 11 comments
Assignees
Labels
feature good first issue REST Issues related to @loopback/rest package and REST transport in general Validation

Comments

@bajtos
Copy link
Member

bajtos commented Jul 30, 2018

OpenAPI spec allows developers to provide additional restrictions on input parameters via parameter schema.

For example, a pageSize parameter can be defined as follows:

parameters:
 - name: pageSize
   in: query
   description: Number of records to return.
   required: false
   schema:
     type: integer
     format: int32      
     minimum: 0
     maximum: 100

At the moment, @loopback/rest's sequence action parseParams validates parameter values against their schema for in: body arguments only.

We should modify parseParamsto validate all parameter values against their schema. Leverage AJV to perform the actual validation.

@bajtos bajtos added LB4 GA Validation REST Issues related to @loopback/rest package and REST transport in general labels Jul 30, 2018
@bajtos bajtos changed the title Validate parameter schema Validate parameter values against their schema Jul 30, 2018
@bajtos bajtos removed the LB4 GA label Aug 13, 2018
@hbakhtiyor
Copy link

@bajtos, need to create another packages/rest/src/validation/params.validator.ts ?

@bajtos
Copy link
Member Author

bajtos commented Nov 23, 2018

need to create another packages/rest/src/validation/params.validator.ts ?

Possibly. I think we should refactor the code in request-body.validator.ts and extract pieces that will be shared with parameter validator, e.g. convertToJsonSchema, validateValueAgainstSchema and createValidator, so there may be more new files to add in the end.

@YaelGit
Copy link

YaelGit commented Nov 27, 2018

@bajtos is there more information you can share in regards to how we can reproduce the issue described above.
If issue can be reproduced using the example of todo-list, or other ways
Please share if available,

@jannyHou
Copy link
Contributor

I think PR #2059 is relevant.

@YaelGit
Copy link

YaelGit commented Dec 6, 2018

Hello @bajtos, please note i would like to start handling this issue #1573. According to my understanding the needed is to add similar functions of validation as there are for validation of parameters of the body (according to the schema), for the parameters of the query.

@bajtos
Copy link
Member Author

bajtos commented Dec 6, 2018

@YaelGit

i would like to start handling this issue

Awesome! I am looking forward to see your contribution. Let me know if you need any help. I think it will be best if you can open a pull request early on, even while the changes are not complete & finished yet.

According to my understanding the needed is to add similar functions of validation as there are for validation of parameters of the body (according to the schema), for the parameters of the query.

Yes please.

Re-posting #1573 (comment):

I think we should refactor the code in request-body.validator.ts and extract pieces that will be shared with parameter validator, e.g. convertToJsonSchema, validateValueAgainstSchema and createValidator, so there may be more new files to add in the end.

@YaelGit
Copy link

YaelGit commented Dec 11, 2018

Hello @bajtos, according to my understanding this is rather a feature and not a minor bug therefore will need a solution design. Do you think this is a good stage to perform the pull request ? or should i do that only after the deign is complete?

@bajtos bajtos added the feature label Dec 11, 2018
@bajtos
Copy link
Member Author

bajtos commented Dec 11, 2018

according to my understanding this is rather a feature and not a minor bug

Makes sense, I updated the issue labels accordingly 👍

therefore will need a solution design. Do you think this is a good stage to perform the pull request ? or should i do that only after the deign is complete?

I think the most important the design work has been already done while implementing request-body validation.

@YaelGit Please go ahead and open a pull request. Feel free to open it early, even before the implementation and tests are complete. That way we can provide you with early feedback and you can save time by keeping possible reworks smaller in scope.

@YaelGit
Copy link

YaelGit commented Dec 13, 2018

Hi Bajtos,

After going through the validate request body implementation I would like to verify we are on same page before actually performing my first Pull Request.
As described in issue, there's a need to implement a request query schema validation. The option, currently doesn't exist in LoopBack 4, only body schema validation is available.
I will be following the request body validation implementation guidelines for the query validation.
Schema validation for query parameters feature description:
The input of the requested validation feature is the request query, i.e: the parameters arriving in the URL, and the query schema.
Each request can have a list of query parameters. The validation should be performed on all of them.
Each parameter has a schema which is a [key: value] list of attributes.
For each attribute type there shall be a specific validator using the AJV package.
For example for pageSize parameter, there will be a verification that parameter value is not under minimum (from schema minimum value) and not over the maximum.
The output of the requested validation feature will be, when false: a List of errors.

Moreover, I think we can be able to transform the available body validation method into a generic validation method – which will receive one more parameter – type (to represent body/query type)
so that the validation performed shall be similar for both cases, only that for body type validation will have an extra step – transforming open API schema into specific entity schema.
Appreciate any comment from you,
Thanks for the issue assignment,
Yael

@bajtos
Copy link
Member Author

bajtos commented Dec 13, 2018

@YaelGit I find it difficult to discuss the changes this way, without seeing any code. Please try to make some changes showing the direction you would like to take and open a pull request. It's ok to send a pull request that's not complete.

YaelGit added a commit to YaelGit/loopback-next that referenced this issue Jan 30, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
YaelGit added a commit to YaelGit/loopback-next that referenced this issue Jan 30, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
YaelGit added a commit to YaelGit/loopback-next that referenced this issue Feb 12, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
YaelGit added a commit to YaelGit/loopback-next that referenced this issue Feb 12, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
YaelGit added a commit to YaelGit/loopback-next that referenced this issue Feb 12, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
YaelGit added a commit to YaelGit/loopback-next that referenced this issue Feb 12, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
YaelGit added a commit to YaelGit/loopback-next that referenced this issue Feb 12, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
YaelGit added a commit to YaelGit/loopback-next that referenced this issue Feb 12, 2019
openapi spec allows developers to provide additional restrictions on input parameters via parameter
schema. For example, pageSize can include schema restrictions

fix loopbackio#1573
@achrinza
Copy link
Member

Closing as done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature good first issue REST Issues related to @loopback/rest package and REST transport in general Validation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants