Skip to content

Commit

Permalink
cdimascio#627 Remove readOnly properties from request instead of thro…
Browse files Browse the repository at this point in the history
…wing Bad request errors
  • Loading branch information
pilerou committed Aug 23, 2022
1 parent 6448f45 commit 120a98e
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 37 deletions.
17 changes: 4 additions & 13 deletions src/framework/ajv/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,23 +104,14 @@ function createAjv(
ajv.removeKeyword('readOnly');
ajv.addKeyword({
keyword: 'readOnly',
modifying: true,
errors: true,
compile: (sch, p, it) => {
if (sch) {
const validate: DataValidateFunction = (data, ctx) => {
const isValid = data == null;
if (!isValid) {
validate.errors = [
{
keyword: 'readOnly',
instancePath: ctx.instancePath,
schemaPath: it.schemaPath.str,
message: `is read-only`,
params: { writeOnly: ctx.parentDataProperty },
},
];
}
return false;
// Remove readonly properties in request
delete ctx.parentData[ctx.parentDataProperty];
return true;
};
return validate;
}
Expand Down
33 changes: 16 additions & 17 deletions test/read.only.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe(packageJson.name, () => {
app.server.close();
});

it('should not allow read only properties in requests', async () =>
it('should remove read only properties in requests', async () =>
request(app)
.post(`${app.basePath}/products`)
.set('content-type', 'application/json')
Expand All @@ -72,11 +72,11 @@ describe(packageJson.name, () => {
price: 10.99,
created_at: new Date().toISOString(),
})
.expect(400)
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not be allowed in the request
expect(body.message).to.contain('id');
// id is a readonly property and should be allowed in the request but should be deleted before entering in route
expect(body.id).to.be.undefined;
}));

it('should allow read only properties in responses', async () =>
Expand All @@ -87,7 +87,7 @@ describe(packageJson.name, () => {
expect(r.body).to.be.an('array').with.length(1);
}));

it('should not allow read only inlined properties in requests', async () =>
it('should remove read only inlined properties in requests', async () =>
request(app)
.post(`${app.basePath}/products/inlined`)
.set('content-type', 'application/json')
Expand All @@ -97,14 +97,13 @@ describe(packageJson.name, () => {
price: 10.99,
created_at: new Date().toUTCString(),
})
.expect(400)
.then((r) => {
const body = r.body;
// id is a readonly property and should not be allowed in the request
expect(body.message).to.contain('id');
expect(body.id).to.be.undefined;
}));

it('should not allow read only properties in requests (nested schema $refs)', async () =>
it('should remove read only properties in requests (nested schema $refs)', async () =>
request(app)
.post(`${app.basePath}/products/nested`)
.set('content-type', 'application/json')
Expand All @@ -113,19 +112,19 @@ describe(packageJson.name, () => {
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
reviews: {
id: 'review_id',
reviews: [{
id: 2,
rating: 5,
},
}],
})
.expect(400)
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not be allowed in the request
expect(body.message).to.contain('id');
// id is a readonly property and should be removed from the request
expect(body.id).to.be.equal('test');
}));

it('should not allow read only properties in requests (deep nested schema $refs)', async () =>
it('should remove read only properties in requests (deep nested schema $refs)', async () =>
request(app)
.post(`${app.basePath}/products/nested`)
.set('content-type', 'application/json')
Expand All @@ -139,11 +138,11 @@ describe(packageJson.name, () => {
},
],
})
.expect(400)
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not be allowed in the request
expect(body.message).to.contain('request/body/reviews/0/id');
expect(body.reviews[0].id).to.be.undefined;
}));

it('should pass validation if required read only properties to be missing from request ($ref)', async () =>
Expand Down
15 changes: 8 additions & 7 deletions test/write.only.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe(packageJson.name, () => {
app.server.close();
});

it('should not allow ready only inlined properties in requests', async () =>
it('should remove read only inlined properties in requests', async () =>
request(app)
.post(`${app.basePath}/products/inlined`)
.set('content-type', 'application/json')
Expand All @@ -51,11 +51,12 @@ describe(packageJson.name, () => {
price: 10.99,
created_at: new Date().toUTCString(),
})
.expect(400)
.expect(200)
.then(r => {
const body = r.body;
// id is a readonly property and should not be allowed in the request
expect(body.message).to.contain('created_at');
// created_at is a readonly property and should be removed form the request
expect(body.created_at).to.be.undefined;
expect(body.name).to.be.equal('some name');
}));

it('should not allow write only inlined properties in responses', async () =>
Expand Down Expand Up @@ -110,7 +111,7 @@ describe(packageJson.name, () => {
expect(body.errors[0].message).to.contain('write-only');
}));

it('should not allow read only properties in requests (deep nested schema $refs)', async () =>
it('should remove read only properties in requests (deep nested schema $refs)', async () =>
request(app)
.post(`${app.basePath}/products/nested`)
.query({
Expand All @@ -128,9 +129,9 @@ describe(packageJson.name, () => {
},
],
})
.expect(400)
.expect(200)
.then(r => {
const body = r.body;
expect(body.message).to.contain('request/body/reviews/0/id');
expect(body.reviews[0].id).to.be.undefined;
}));
});

0 comments on commit 120a98e

Please sign in to comment.