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

Remove read only and write only fields #895

Merged
merged 2 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 38 additions & 24 deletions src/framework/ajv/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,19 +117,26 @@ function createAjv(
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 },
},
];
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
// Remove readonly properties in request
delete ctx.parentData[ctx.parentDataProperty];
return true;
}
else {
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;
}
return false;
};
return validate;
}
Expand Down Expand Up @@ -178,19 +185,26 @@ function createAjv(
compile: (sch, p, it) => {
if (sch) {
const validate: DataValidateFunction = (data, ctx) => {
const isValid = data == null;
if (!isValid) {
validate.errors = [
{
keyword: 'writeOnly',
instancePath: ctx.instancePath,
schemaPath: it.schemaPath.str,
message: `is write-only`,
params: { writeOnly: ctx.parentDataProperty },
},
];
if (options.removeAdditional == true || options.removeAdditional == "all" || options.removeAdditional == "failing") {
// Remove readonly properties in request
delete ctx.parentData[ctx.parentDataProperty];
return true;
}
else {
const isValid = data == null;
if (!isValid) {
validate.errors = [
{
keyword: 'writeOnly',
instancePath: ctx.instancePath,
schemaPath: it.schemaPath.str,
message: `is write-only`,
params: {writeOnly: ctx.parentDataProperty},
},
];
}
return false;
}
return false;
};
return validate;
}
Expand Down
214 changes: 214 additions & 0 deletions test/read.only.removeadditional.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
import * as path from 'path';
import { expect } from 'chai';
import * as request from 'supertest';
import { createApp } from './common/app';
import * as packageJson from '../package.json';

describe(packageJson.name, () => {
let app = null;

before(async () => {
// Set up the express app
const apiSpec = path.join('test', 'resources', 'read.only.yaml');
app = await createApp({ apiSpec, validateRequests: {removeAdditional:true}, validateResponses: true }, 3005, (app) =>
app
.post(`${app.basePath}/products`, (req, res) => res.json(req.body))
.get(`${app.basePath}/products`, (req, res) =>
res.json([
{
id: 'id_1',
name: 'name_1',
price: 9.99,
created_at: new Date().toISOString(),
},
]),
)
.post(`${app.basePath}/products/inlined`, (req, res) =>
res.json(req.body),
)
.post(`${app.basePath}/user`, (req, res) =>
res.json({
...req.body,
...(req.query.include_id ? { id: 'test_id' } : {}),
}),
)
.post(`${app.basePath}/user_inlined`, (req, res) =>
res.json({
...req.body,
...(req.query.include_id ? { id: 'test_id' } : {}),
}),
)
.post(`${app.basePath}/products/nested`, (req, res) => {
const body = req.body;
body.id = 'test';
body.created_at = new Date().toISOString();
body.reviews = body.reviews.map((r) => ({
id: 99,
rating: r.rating ?? 2,
}));
res.json(body);
})
.post(`${app.basePath}/readonly_required_allof`, (req, res) => {
const json = {
name: 'My Name',
...(req.query.include_id ? { id: 'test_id' } : {}),
};
res.json(json);
}),
);
});

after(() => {
app.server.close();
});

it('should remove read only properties in requests thanks to removeAdditional', async () =>
request(app)
.post(`${app.basePath}/products`)
.set('content-type', 'application/json')
.send({
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
})
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not be allowed in the request
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
expect(body.id).to.be.undefined;
}));


it('should allow read only properties in responses', async () =>
request(app)
.get(`${app.basePath}/products`)
.expect(200)
.then((r) => {
expect(r.body).to.be.an('array').with.length(1);
}));

it('should remove read only inlined properties in requests thanks to removeAdditional', async () =>
await request(app)
.post(`${app.basePath}/products/inlined`)
.set('content-type', 'application/json')
.send({
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
})
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not not be allowed in the request
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
expect(body.id).to.be.undefined;
}));


it('should remove read only properties in requests (nested and deep nested schema $refs) thanks to removeAdditional', async () =>
request(app)
.post(`${app.basePath}/products/nested`)
.set('content-type', 'application/json')
.send({
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
reviews: [{
id: 10,
rating: 5,
}],
})
.expect(200)
.then((r) => {
const body = r.body;
// id is a readonly property and should not not be allowed in the request
// but, as removeAdditional is true for requests, it should be deleted before entering in the route
expect(body.id).to.be.equal('test');
expect(body.reviews[0].id).to.be.equal(99);
}));

it('should pass validation if required read only properties to be missing from request ($ref)', async () =>
request(app)
.post(`${app.basePath}/user`)
.set('content-type', 'application/json')
.query({
include_id: true,
})
.send({
username: 'test',
})
.expect(200)
.then((r) => {
expect(r.body).to.be.an('object').with.property('id');
expect(r.body).to.have.property('username');
}));

it('should pass validation if required read only properties to be missing from request (inlined)', async () =>
request(app)
.post(`${app.basePath}/user_inlined`)
.set('content-type', 'application/json')
.query({
include_id: true,
})
.send({
username: 'test',
})
.expect(200)
.then((r) => {
expect(r.body).to.be.an('object').with.property('id');
expect(r.body).to.have.property('username');
}));

it('should pass validation if required read only properties to be missing from request (with charset)', async () =>
request(app)
.post(`${app.basePath}/user_inlined`)
.set('content-type', 'application/json; charset=utf-8')
.query({
include_id: true,
})
.send({
username: 'test',
})
.expect(200)
.then((r) => {
expect(r.body).to.be.an('object').with.property('id');
expect(r.body).to.have.property('username');
}));

it('should fail validation if required read only properties is missing from the response', async () =>
request(app)
.post(`${app.basePath}/user`)
.set('content-type', 'application/json')
.send({
username: 'test',
})
.expect(500)
.then((r) => {
expect(r.body.errors[0])
.to.have.property('message')
.equals("must have required property 'id'");
}));

it('should require readonly required property in response', async () =>
request(app)
.post(`${app.basePath}/readonly_required_allof`)
.query({ include_id: true })
.send({ optional: 'test' })
.set('content-type', 'application/json')
.expect(200));

it('should return 500 if readonly required property is missing from response', async () =>
request(app)
.post(`${app.basePath}/readonly_required_allof`)
.query({ include_id: false })
.send({ optional: 'test' })
.set('content-type', 'application/json')
.expect(500)
.then((r) => {
expect(r.body.message).includes("must have required property 'id'");
}));
});
6 changes: 3 additions & 3 deletions test/read.only.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe(packageJson.name, () => {
id: 'id_1',
name: 'some name',
price: 10.99,
created_at: new Date().toUTCString(),
created_at: new Date().toISOString(),
})
.expect(400)
.then((r) => {
Expand All @@ -113,10 +113,10 @@ describe(packageJson.name, () => {
name: 'some name',
price: 10.99,
created_at: new Date().toISOString(),
reviews: {
reviews: [{
id: 'review_id',
rating: 5,
},
}],
})
.expect(400)
.then((r) => {
Expand Down