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

Unknown field validation #1204

Merged
merged 7 commits into from
Jan 20, 2023
Merged

Unknown field validation #1204

merged 7 commits into from
Jan 20, 2023

Conversation

gustavohenke
Copy link
Member

@gustavohenke gustavohenke commented Jan 17, 2023

Description

This PR implements long awaited exact matching of requests, a.k.a unknown field validations.

It works by

  1. grabbing all contexts and extracting the fields and locations that they validate; then
  2. creating a recursive tree out of the known fields paths; for example, foo.bar becomes { foo: { bar: {} }; then
  3. recursively iterating over each field under each validated request location, seeing if they match a known field or not.

I made the conscious decision of not including headers and cookies in the default validated locations.
The reason is a bit obvious: browsers will add stuff you weren't expecting.

The API is very flexible too. All of the below are valid:

// Only a single id param can exist
app.get('user/:id', checkExact(param('id').isInt()), getUserRoute);

// Only the email and password body properties can exist
app.post('login', checkExact([
	body('email').isEmail(),
	body('password').notEmpty()
], loginRoute);

// Same as above, but with a schema
app.post('login', checkExact(checkSchema({
	email: { isEmail: true },
	password: { notEmpty: true }
})), loginRoute);

// Same as above, but as separate middlewares
app.post('login', [
	body('email').isEmail(),
	body('password').notEmpty(),
	checkExact()
], loginRoute);

It's important to notice in the above that checkExact is always the last middleware, or one that wraps others.
That's because it will report fields from chains after it as unknown.

Note: performance wise, this might be slow, depending on what the request and the validation rules look like.

I'm not adding docs in this PR as I wish to revamp them before v7 is released.

To-do list

  • I have added tests for what I changed.
  • This pull request is ready to merge.

@coveralls
Copy link

Coverage Status

Coverage: 100.0%. Remained the same when pulling c8a4642 on v7/check-exact into 327af8b on v7-features.

@@ -55,9 +55,10 @@ export interface FieldInstance {
originalPath: string;
location: Location;
value: any;
originalValue: any;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never really used

Comment on lines +266 to +272
// This one seems controversial.
// The nested property wouldn't pass validation - unless it's optional, in which case it's fair to
// argue that 'foo' should be selected?
it('does not select parent when only nested field is known and not an object', () => {
const req = { body: { foo: 'bla' } };
const instances = selectUnknownFields(req, ['foo.bar'], ['body']);
expect(instances).toHaveLength(0);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noteworthy. Let's see what happens after releasing this

@gustavohenke gustavohenke merged commit 4357ca2 into v7-features Jan 20, 2023
@gustavohenke gustavohenke deleted the v7/check-exact branch January 20, 2023 22:12
gustavohenke added a commit that referenced this pull request Jan 20, 2023
Also extracting field path reconstruction function, from #1204

Closes #1205
gustavohenke added a commit that referenced this pull request Jan 22, 2023
gustavohenke added a commit that referenced this pull request Mar 11, 2023
gustavohenke added a commit that referenced this pull request Mar 11, 2023
@gustavohenke gustavohenke added this to the v7.0.0 milestone Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants