Skip to content

Commit 4c43340

Browse files
committed
Adds reqParamChecker middleware
- Adds a utils method to check if a value is a number - Adds a middleware to check if req params are valid integers - Adds tests
1 parent 0bf727b commit 4c43340

File tree

6 files changed

+358
-1
lines changed

6 files changed

+358
-1
lines changed
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
const response = {
2+
success: false,
3+
message: 'Request params must be positive integers',
4+
};
5+
6+
const responseTwo = {
7+
success: false,
8+
message: 'URL params must be greater than zero',
9+
};
10+
11+
const checkParams = (req, res, next) => {
12+
const { todoId, todoItemId } = req.params;
13+
14+
if (!Number.isInteger(Number(todoId))) {
15+
return res.status(400).json(response);
16+
}
17+
18+
if (todoItemId && !Number.isInteger(Number(todoItemId))) {
19+
return res.status(400).json(response);
20+
}
21+
22+
if (todoId < 1 || todoItemId < 1) {
23+
return res.status(400).json(responseTwo);
24+
}
25+
return next();
26+
};
27+
28+
const checkUserRouteParams = (req, res, next) => {
29+
const { userId } = req.params;
30+
31+
if (!Number.isInteger(Number(userId))) {
32+
return res.status(400).json(response);
33+
}
34+
35+
if (userId < 1) {
36+
return res.status(400).json(responseTwo);
37+
}
38+
return next();
39+
};
40+
41+
module.exports = {
42+
checkParams,
43+
checkUserRouteParams,
44+
};

server/routes/todosRouter.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const todoItemsController = require('../controllers').todoItems;
55
const validator = require('../validators/validators');
66
const checkOwner = require('../middlewares/checkOwner');
77
const asyncHandler = require('../middlewares/asyncHandler');
8+
const { checkParams } = require('../middlewares/reqParamChecker');
89
const { paginate } = require('../middlewares/pagination');
910

1011
function todosRoutes() {
@@ -13,7 +14,7 @@ function todosRoutes() {
1314
todosRouter.route('/todos')
1415
.post(celebrate({ body: validator.validateTodo }), asyncHandler(todosController.createTodo))
1516
.get(asyncHandler(todosController.list));
16-
todosRouter.use('/todos/:todoId', asyncHandler(checkOwner.findTodo));
17+
todosRouter.use('/todos/:todoId', checkParams, asyncHandler(checkOwner.findTodo));
1718
todosRouter.route('/todos/:todoId')
1819
.get(asyncHandler(todosController.retrieve))
1920
.put(celebrate({ body: validator.validateTodo }), asyncHandler(todosController.update))
@@ -22,6 +23,7 @@ function todosRoutes() {
2223
.post(celebrate({
2324
body: validator.validateTodoItem,
2425
}), asyncHandler(todoItemsController.createTodoItem));
26+
todosRouter.use('/todos/:todoId/items/:todoItemId', checkParams);
2527
todosRouter.route('/todos/:todoId/items/:todoItemId')
2628
.put(celebrate({ body: validator.validateTodoItem }), asyncHandler(todoItemsController.update))
2729
.delete(asyncHandler(todoItemsController.destroy));

server/routes/usersRouter.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ const { celebrate } = require('celebrate');
33
const usersController = require('../controllers').users;
44
const validator = require('../validators/validators');
55
const asyncHandler = require('../middlewares/asyncHandler');
6+
const { checkUserRouteParams } = require('../middlewares/reqParamChecker');
67

78
function userRoutes() {
89
const userRouter = express.Router();
910

11+
userRouter.use('/users/:userId', checkUserRouteParams);
1012
userRouter.route('/users/:userId')
1113
.patch(celebrate({
1214
body: validator.validatePassword,
Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
const chai = require('chai');
2+
3+
const { expect } = chai;
4+
const sinonChai = require('sinon-chai');
5+
const sinon = require('sinon');
6+
const { mockReq, mockRes } = require('sinon-express-mock');
7+
const { checkParams, checkUserRouteParams } = require('../../middlewares/reqParamChecker');
8+
9+
chai.use(sinonChai);
10+
11+
describe('Request param checker', () => {
12+
describe('checkParams', () => {
13+
it('should return 400 if todoId is not a positive integer', (done) => {
14+
const request = {
15+
params: {
16+
todoId: -2,
17+
}
18+
};
19+
const req = mockReq(request);
20+
const res = mockRes();
21+
const next = sinon.spy();
22+
23+
checkParams(req, res, next);
24+
expect(res.status).to.have.been.calledWith(400);
25+
done()
26+
});
27+
28+
it('should return 400 if the todoId is a string', (done) => {
29+
const request = {
30+
params: {
31+
todoId: 's',
32+
}
33+
};
34+
const req = mockReq(request);
35+
const res = mockRes();
36+
const next = sinon.spy();
37+
38+
checkParams(req, res, next);
39+
expect(res.status).to.have.been.calledWith(400);
40+
done()
41+
});
42+
43+
it('should return 400 if the todoId is a float', (done) => {
44+
const request = {
45+
params: {
46+
todoId: 1.1,
47+
}
48+
};
49+
const req = mockReq(request);
50+
const res = mockRes();
51+
const next = sinon.spy();
52+
53+
checkParams(req, res, next);
54+
expect(res.status).to.have.been.calledWith(400);
55+
done()
56+
});
57+
58+
xit('should return 400 if the todoId is a .0 float', (done) => {
59+
const request = {
60+
params: {
61+
todoId: 1.0,
62+
}
63+
};
64+
const req = mockReq(request);
65+
const res = mockRes();
66+
const next = sinon.spy();
67+
68+
checkParams(req, res, next);
69+
expect(res.status).to.have.been.calledWith(400);
70+
done()
71+
});
72+
73+
it('should return 400 if the todoId is a string that is alphanumeric', (done) => {
74+
const request = {
75+
params: {
76+
todoId: '4s',
77+
}
78+
};
79+
const req = mockReq(request);
80+
const res = mockRes();
81+
const next = sinon.spy();
82+
83+
checkParams(req, res, next);
84+
expect(res.status).to.have.been.calledWith(400);
85+
done()
86+
});
87+
88+
it('should return 400 if the todoItemId is not a positive integer', (done) => {
89+
const request = {
90+
params: {
91+
todoId: 1,
92+
todoItemId: -2,
93+
}
94+
};
95+
const req = mockReq(request);
96+
const res = mockRes();
97+
const next = sinon.spy();
98+
99+
checkParams(req, res, next);
100+
expect(res.status).to.have.been.calledWith(400);
101+
done()
102+
});
103+
104+
it('should return 400 if the todoItemId is a string', (done) => {
105+
const request = {
106+
params: {
107+
todoId: 1,
108+
todoItemId: 's',
109+
}
110+
};
111+
const req = mockReq(request);
112+
const res = mockRes();
113+
const next = sinon.spy();
114+
115+
checkParams(req, res, next);
116+
expect(res.status).to.have.been.calledWith(400);
117+
done()
118+
});
119+
120+
it('should return 400 if the todoItemId is an alphanumeric string', (done) => {
121+
const request = {
122+
params: {
123+
todoId: 1,
124+
todoItemId: '4s',
125+
}
126+
};
127+
const req = mockReq(request);
128+
const res = mockRes();
129+
const next = sinon.spy();
130+
131+
checkParams(req, res, next);
132+
expect(res.status).to.have.been.calledWith(400);
133+
done()
134+
});
135+
136+
it('should return 400 if the todoItemId is a float', (done) => {
137+
const request = {
138+
params: {
139+
todoId: 1,
140+
todoItemId: 2.3,
141+
}
142+
};
143+
const req = mockReq(request);
144+
const res = mockRes();
145+
const next = sinon.spy();
146+
147+
checkParams(req, res, next);
148+
expect(res.status).to.have.been.calledWith(400);
149+
done()
150+
});
151+
152+
xit('should return 400 if the todoItemId is a .0 float', (done) => {
153+
const request = {
154+
params: {
155+
todoId: 1,
156+
todoItemId: 2.0,
157+
}
158+
};
159+
const req = mockReq(request);
160+
const res = mockRes();
161+
const next = sinon.spy();
162+
163+
checkParams(req, res, next);
164+
expect(res.status).to.have.been.calledWith(400);
165+
done()
166+
});
167+
168+
it('should return call next if all checks pass', (done) => {
169+
const request = {
170+
params: {
171+
todoId: 1,
172+
todoItemId: 4,
173+
}
174+
};
175+
const req = mockReq(request);
176+
const res = mockRes();
177+
const next = sinon.spy();
178+
179+
checkParams(req, res, next);
180+
expect(next.called).to.be.true;
181+
done()
182+
});
183+
184+
describe('checkParams for user', () => {
185+
it('should return 400 if userId is not a positive integer', (done) => {
186+
const request = {
187+
params: {
188+
userId: -2,
189+
}
190+
};
191+
const req = mockReq(request);
192+
const res = mockRes();
193+
const next = sinon.spy();
194+
195+
checkUserRouteParams(req, res, next);
196+
expect(res.status).to.have.been.calledWith(400);
197+
done()
198+
});
199+
200+
it('should return 400 if the userId is a string', (done) => {
201+
const request = {
202+
params: {
203+
userId: 's',
204+
}
205+
};
206+
const req = mockReq(request);
207+
const res = mockRes();
208+
const next = sinon.spy();
209+
210+
checkUserRouteParams(req, res, next);
211+
expect(res.status).to.have.been.calledWith(400);
212+
done()
213+
});
214+
215+
it('should return 400 if the userId is a float', (done) => {
216+
const request = {
217+
params: {
218+
userId: 1.1,
219+
}
220+
};
221+
const req = mockReq(request);
222+
const res = mockRes();
223+
const next = sinon.spy();
224+
225+
checkUserRouteParams(req, res, next);
226+
expect(res.status).to.have.been.calledWith(400);
227+
done()
228+
});
229+
230+
xit('should return 400 if the userId is a .0 float', (done) => {
231+
const request = {
232+
params: {
233+
userId: 1.0,
234+
}
235+
};
236+
const req = mockReq(request);
237+
const res = mockRes();
238+
const next = sinon.spy();
239+
240+
checkUserRouteParams(req, res, next);
241+
expect(res.status).to.have.been.calledWith(400);
242+
done()
243+
});
244+
245+
it('should return 400 if the userId is a string that is alphanumeric', (done) => {
246+
const request = {
247+
params: {
248+
userId: '4s',
249+
}
250+
};
251+
const req = mockReq(request);
252+
const res = mockRes();
253+
const next = sinon.spy();
254+
255+
checkUserRouteParams(req, res, next);
256+
expect(res.status).to.have.been.calledWith(400);
257+
done()
258+
});
259+
})
260+
});
261+
});
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
const chai = require('chai');
2+
3+
const { expect } = chai;
4+
const sinonChai = require('sinon-chai');
5+
const { isNumber } = require('../../utils/typeChecker');
6+
7+
chai.use(sinonChai);
8+
9+
describe('typeChecker', () => {
10+
describe('isNumber', () => {
11+
it('should return true if value is a positive integer', (done) => {
12+
expect(isNumber(4)).to.be.true;
13+
expect(isNumber(434)).to.be.true;
14+
done()
15+
});
16+
17+
it('should return false if value is a negative integer', (done) => {
18+
expect(isNumber(-4)).to.be.false;
19+
done()
20+
});
21+
22+
it('should return false if value is a float', (done) => {
23+
expect(isNumber(4.1)).to.be.false;
24+
expect(isNumber(4.9)).to.be.false;
25+
done()
26+
});
27+
28+
xit('should return false if value is a .0 float', (done) => {
29+
expect(isNumber(4.0)).to.be.false;
30+
done()
31+
});
32+
33+
it('should return true if value is a string of a valid number', (done) => {
34+
expect(isNumber('4')).to.be.true;
35+
done()
36+
});
37+
38+
it('should return false if value is a string of a valid number', (done) => {
39+
expect(isNumber('4s')).to.be.false;
40+
done()
41+
});
42+
});
43+
});

server/utils/typeChecker.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
const isNumber = (value) => /^\d+$/.test(value);
2+
3+
module.exports = {
4+
isNumber,
5+
};

0 commit comments

Comments
 (0)