Skip to content

Commit 0d3d3ed

Browse files
pileroupilerou
committed
cdimascio#627 Remove readonly fields in :
- requests if ``validateRequest.removeAdditional`` configuration equals ``true`` or ```'all'`` or ``'failing'`` - responses if ``validateResponse.removeAdditional`` configuration equals ``true`` or ```'all'`` or ``'failing'`` No changes if ``validateRequest = true``, ``validateResponse = true``, ``validateRequest.removeAdditional : false``, ``validateResponse.removeAdditional : false`` Unit tests added to check the behaviour with removeAdditional : true. Fields removed and no error in response.
1 parent 54e6bb3 commit 0d3d3ed

File tree

3 files changed

+350
-24
lines changed

3 files changed

+350
-24
lines changed

‎src/framework/ajv/index.ts

+38-24
Original file line numberDiff line numberDiff line change
@@ -117,19 +117,26 @@ function createAjv(
117117
compile: (sch, p, it) => {
118118
if (sch) {
119119
const validate: DataValidateFunction = (data, ctx) => {
120-
const isValid = data == null;
121-
if (!isValid) {
122-
validate.errors = [
123-
{
124-
keyword: 'readOnly',
125-
instancePath: ctx.instancePath,
126-
schemaPath: it.schemaPath.str,
127-
message: `is read-only`,
128-
params: { writeOnly: ctx.parentDataProperty },
129-
},
130-
];
120+
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
121+
// Remove readonly properties in request
122+
delete ctx.parentData[ctx.parentDataProperty];
123+
return true;
124+
}
125+
else {
126+
const isValid = data == null;
127+
if (!isValid) {
128+
validate.errors = [
129+
{
130+
keyword: 'readOnly',
131+
instancePath: ctx.instancePath,
132+
schemaPath: it.schemaPath.str,
133+
message: `is read-only`,
134+
params: { writeOnly: ctx.parentDataProperty },
135+
},
136+
];
137+
}
138+
return false;
131139
}
132-
return false;
133140
};
134141
return validate;
135142
}
@@ -178,19 +185,26 @@ function createAjv(
178185
compile: (sch, p, it) => {
179186
if (sch) {
180187
const validate: DataValidateFunction = (data, ctx) => {
181-
const isValid = data == null;
182-
if (!isValid) {
183-
validate.errors = [
184-
{
185-
keyword: 'writeOnly',
186-
instancePath: ctx.instancePath,
187-
schemaPath: it.schemaPath.str,
188-
message: `is write-only`,
189-
params: { writeOnly: ctx.parentDataProperty },
190-
},
191-
];
188+
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
189+
// Remove readonly properties in request
190+
delete ctx.parentData[ctx.parentDataProperty];
191+
return true;
192+
}
193+
else {
194+
const isValid = data == null;
195+
if (!isValid) {
196+
validate.errors = [
197+
{
198+
keyword: 'writeOnly',
199+
instancePath: ctx.instancePath,
200+
schemaPath: it.schemaPath.str,
201+
message: `is write-only`,
202+
params: {writeOnly: ctx.parentDataProperty},
203+
},
204+
];
205+
}
206+
return false;
192207
}
193-
return false;
194208
};
195209
return validate;
196210
}

‎test/read.only.removeadditional.spec.ts

+214
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
import * as path from 'path';
2+
import { expect } from 'chai';
3+
import * as request from 'supertest';
4+
import { createApp } from './common/app';
5+
import * as packageJson from '../package.json';
6+
7+
describe(packageJson.name, () => {
8+
let app = null;
9+
10+
before(async () => {
11+
// Set up the express app
12+
const apiSpec = path.join('test', 'resources', 'read.only.yaml');
13+
app = await createApp({ apiSpec, validateRequests: {removeAdditional:true}, validateResponses: true }, 3005, (app) =>
14+
app
15+
.post(`${app.basePath}/products`, (req, res) => res.json(req.body))
16+
.get(`${app.basePath}/products`, (req, res) =>
17+
res.json([
18+
{
19+
id: 'id_1',
20+
name: 'name_1',
21+
price: 9.99,
22+
created_at: new Date().toISOString(),
23+
},
24+
]),
25+
)
26+
.post(`${app.basePath}/products/inlined`, (req, res) =>
27+
res.json(req.body),
28+
)
29+
.post(`${app.basePath}/user`, (req, res) =>
30+
res.json({
31+
...req.body,
32+
...(req.query.include_id ? { id: 'test_id' } : {}),
33+
}),
34+
)
35+
.post(`${app.basePath}/user_inlined`, (req, res) =>
36+
res.json({
37+
...req.body,
38+
...(req.query.include_id ? { id: 'test_id' } : {}),
39+
}),
40+
)
41+
.post(`${app.basePath}/products/nested`, (req, res) => {
42+
const body = req.body;
43+
body.id = 'test';
44+
body.created_at = new Date().toISOString();
45+
body.reviews = body.reviews.map((r) => ({
46+
id: 99,
47+
rating: r.rating ?? 2,
48+
}));
49+
res.json(body);
50+
})
51+
.post(`${app.basePath}/readonly_required_allof`, (req, res) => {
52+
const json = {
53+
name: 'My Name',
54+
...(req.query.include_id ? { id: 'test_id' } : {}),
55+
};
56+
res.json(json);
57+
}),
58+
);
59+
});
60+
61+
after(() => {
62+
app.server.close();
63+
});
64+
65+
it('should remove read only properties in requests thanks to removeAdditional', async () =>
66+
request(app)
67+
.post(`${app.basePath}/products`)
68+
.set('content-type', 'application/json')
69+
.send({
70+
id: 'id_1',
71+
name: 'some name',
72+
price: 10.99,
73+
created_at: new Date().toISOString(),
74+
})
75+
.expect(200)
76+
.then((r) => {
77+
const body = r.body;
78+
// id is a readonly property and should not be allowed in the request
79+
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
80+
expect(body.id).to.be.undefined;
81+
}));
82+
83+
84+
it('should allow read only properties in responses', async () =>
85+
request(app)
86+
.get(`${app.basePath}/products`)
87+
.expect(200)
88+
.then((r) => {
89+
expect(r.body).to.be.an('array').with.length(1);
90+
}));
91+
92+
it('should remove read only inlined properties in requests thanks to removeAdditional', async () =>
93+
await request(app)
94+
.post(`${app.basePath}/products/inlined`)
95+
.set('content-type', 'application/json')
96+
.send({
97+
id: 'id_1',
98+
name: 'some name',
99+
price: 10.99,
100+
created_at: new Date().toISOString(),
101+
})
102+
.expect(200)
103+
.then((r) => {
104+
const body = r.body;
105+
// id is a readonly property and should not not be allowed in the request
106+
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
107+
expect(body.id).to.be.undefined;
108+
}));
109+
110+
111+
it('should remove read only properties in requests (nested and deep nested schema $refs) thanks to removeAdditional', async () =>
112+
request(app)
113+
.post(`${app.basePath}/products/nested`)
114+
.set('content-type', 'application/json')
115+
.send({
116+
id: 'id_1',
117+
name: 'some name',
118+
price: 10.99,
119+
created_at: new Date().toISOString(),
120+
reviews: [{
121+
id: 10,
122+
rating: 5,
123+
}],
124+
})
125+
.expect(200)
126+
.then((r) => {
127+
const body = r.body;
128+
// id is a readonly property and should not not be allowed in the request
129+
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
130+
expect(body.id).to.be.equal('test');
131+
expect(body.reviews[0].id).to.be.equal(99);
132+
}));
133+
134+
it('should pass validation if required read only properties to be missing from request ($ref)', async () =>
135+
request(app)
136+
.post(`${app.basePath}/user`)
137+
.set('content-type', 'application/json')
138+
.query({
139+
include_id: true,
140+
})
141+
.send({
142+
username: 'test',
143+
})
144+
.expect(200)
145+
.then((r) => {
146+
expect(r.body).to.be.an('object').with.property('id');
147+
expect(r.body).to.have.property('username');
148+
}));
149+
150+
it('should pass validation if required read only properties to be missing from request (inlined)', async () =>
151+
request(app)
152+
.post(`${app.basePath}/user_inlined`)
153+
.set('content-type', 'application/json')
154+
.query({
155+
include_id: true,
156+
})
157+
.send({
158+
username: 'test',
159+
})
160+
.expect(200)
161+
.then((r) => {
162+
expect(r.body).to.be.an('object').with.property('id');
163+
expect(r.body).to.have.property('username');
164+
}));
165+
166+
it('should pass validation if required read only properties to be missing from request (with charset)', async () =>
167+
request(app)
168+
.post(`${app.basePath}/user_inlined`)
169+
.set('content-type', 'application/json; charset=utf-8')
170+
.query({
171+
include_id: true,
172+
})
173+
.send({
174+
username: 'test',
175+
})
176+
.expect(200)
177+
.then((r) => {
178+
expect(r.body).to.be.an('object').with.property('id');
179+
expect(r.body).to.have.property('username');
180+
}));
181+
182+
it('should fail validation if required read only properties is missing from the response', async () =>
183+
request(app)
184+
.post(`${app.basePath}/user`)
185+
.set('content-type', 'application/json')
186+
.send({
187+
username: 'test',
188+
})
189+
.expect(500)
190+
.then((r) => {
191+
expect(r.body.errors[0])
192+
.to.have.property('message')
193+
.equals("must have required property 'id'");
194+
}));
195+
196+
it('should require readonly required property in response', async () =>
197+
request(app)
198+
.post(`${app.basePath}/readonly_required_allof`)
199+
.query({ include_id: true })
200+
.send({ optional: 'test' })
201+
.set('content-type', 'application/json')
202+
.expect(200));
203+
204+
it('should return 500 if readonly required property is missing from response', async () =>
205+
request(app)
206+
.post(`${app.basePath}/readonly_required_allof`)
207+
.query({ include_id: false })
208+
.send({ optional: 'test' })
209+
.set('content-type', 'application/json')
210+
.expect(500)
211+
.then((r) => {
212+
expect(r.body.message).includes("must have required property 'id'");
213+
}));
214+
});

‎test/write.only.removeadditional.spec.ts

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import * as path from 'path';
2+
import { expect } from 'chai';
3+
import * as request from 'supertest';
4+
import { createApp } from './common/app';
5+
import * as packageJson from '../package.json';
6+
7+
describe(packageJson.name, () => {
8+
let app = null;
9+
10+
before(async () => {
11+
// Set up the express app
12+
const apiSpec = path.join('test', 'resources', 'write.only.yaml');
13+
app = await createApp({ apiSpec, validateResponses: {removeAdditional : true} }, 3005, app =>
14+
app
15+
.post(`${app.basePath}/products/inlined`, (req, res) => {
16+
const body = req.body;
17+
const excludeWriteOnly = req.query.exclude_write_only;
18+
if (excludeWriteOnly) {
19+
delete body.role;
20+
}
21+
res.json(body);
22+
})
23+
.post(`${app.basePath}/products/nested`, (req, res) => {
24+
const body = req.body;
25+
const excludeWriteOnly = req.query.exclude_write_only;
26+
body.id = 'test';
27+
body.created_at = new Date().toISOString();
28+
body.reviews = body.reviews.map(r => ({
29+
...(excludeWriteOnly ? {} : { role_x: 'admin' }),
30+
rating: r.rating ?? 2,
31+
}));
32+
33+
if (excludeWriteOnly) {
34+
delete body.role;
35+
}
36+
res.json(body);
37+
}),
38+
);
39+
});
40+
41+
after(() => {
42+
app.server.close();
43+
});
44+
45+
it('should remove write only inlined properties in responses thanks to removeAdditional', async () =>
46+
request(app)
47+
.post(`${app.basePath}/products/inlined`)
48+
.set('content-type', 'application/json')
49+
.send({
50+
name: 'some name',
51+
role: 'admin',
52+
price: 10.99,
53+
})
54+
.expect(200)
55+
.then(r => {
56+
const body = r.body;
57+
expect(body.message).to.be.undefined;
58+
expect(body.role).to.be.undefined;
59+
expect(body.price).to.be.equal(10.99);
60+
}));
61+
62+
it('should return 200 if no write-only properties are in the responses', async () =>
63+
request(app)
64+
.post(`${app.basePath}/products/inlined`)
65+
.query({
66+
exclude_write_only: true,
67+
})
68+
.set('content-type', 'application/json')
69+
.send({
70+
name: 'some name',
71+
role: 'admin',
72+
price: 10.99,
73+
})
74+
.expect(200)
75+
);
76+
77+
it('should remove write only properties in responses (nested schema $refs) thanks to removeAdditional', async () =>
78+
request(app)
79+
.post(`${app.basePath}/products/nested`)
80+
.set('content-type', 'application/json')
81+
.send({
82+
name: 'some name',
83+
price: 10.99,
84+
reviews: [
85+
{
86+
rating: 5
87+
},
88+
],
89+
})
90+
.expect(200)
91+
.then(r => {
92+
const body = r.body;
93+
console.log('%j', r.body);
94+
expect(body.reviews[0].role_x).to.be.undefined;
95+
expect(body.reviews[0].rating).to.be.equal(5);
96+
}));
97+
98+
});

0 commit comments

Comments
 (0)