Skip to content

Commit 19cd3e3

Browse files
author
Arun Patra
authored
Merge pull request #11 from Reloadly/develop
Code coverage improvement
2 parents 4dc640a + aa36d11 commit 19cd3e3

6 files changed

Lines changed: 532 additions & 299 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@prizemates/http-firewall",
3-
"version": "1.0.1",
3+
"version": "1.0.2",
44
"description": "HTTP Firewall based on Spring Security HttpFirewall",
55
"private": false,
66
"main": "./lib/index.js",

‎src/__tests__/strict-http-firewall.tests.ts‎

Lines changed: 254 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,104 @@
1-
import { httpFirewall, HttpFirewallOptions } from '../strict-http-firewall';
1+
import { httpFirewall } from '../strict-http-firewall';
2+
import { HttpFirewallOptions, HttpMethod } from '../types';
23
import express from 'express';
34
import request from 'supertest';
45

56
describe('HttpStrictFirewall test suite', () => {
7+
const unNormalizedPathsThatCanBeHandledBySupertest: string[] = [
8+
'/..',
9+
'/./path/',
10+
'/path/path/.',
11+
'/path/path//.',
12+
13+
'//path',
14+
'//path/path',
15+
'//path//path',
16+
'/path//path',
17+
];
18+
19+
// const unNormalizedPathsTrappedBySanitizer: string[] = [
20+
// "./path/../path//.",
21+
// "./path",
22+
// ".//path",
23+
// ".",
24+
// "/..",
25+
// "/./path/",
26+
// "/path/path/.",
27+
// "/path/path//."
28+
// ];
29+
// describe('URL Normalization Tests: .isNormalized()', () => {
30+
//
31+
// for (const path of unNormalizedPathsTrappedBySanitizer) {
32+
// test(`Should reject un-normalized path: ${path}`, async () => {
33+
// const fw = new StrictHttpFirewall();
34+
// // const fwProto = Object.getPrototypeOf(fw);
35+
//
36+
// const valid = fw.isNormalized(path);
37+
// expect(valid).toBe(false)
38+
// });
39+
// }
40+
// });
41+
642
describe('Integration Tests: .firewall()', () => {
7-
test('Should reject disallowed Http method', async () => {
43+
it('Should block XST attacks using TRACE', async () => {
44+
const app = express();
45+
app.use(httpFirewall());
46+
const res = await request(app).trace('/').set('Content-Type', 'application/json');
47+
expect(res.statusCode).toBe(403);
48+
});
49+
50+
it('When valid methods are used, call should not be rejected', async () => {
51+
const app = express();
52+
app.use(httpFirewall());
53+
setupRoutes(app, ['GET', 'PUT', 'POST', 'HEAD', 'OPTIONS', 'DELETE', 'PATCH']);
54+
55+
let res = await request(app).get('/');
56+
expect(res.statusCode).toBe(200);
57+
58+
res = await request(app).put('/');
59+
expect(res.statusCode).toBe(200);
60+
61+
res = await request(app).post('/');
62+
expect(res.statusCode).toBe(200);
63+
64+
res = await request(app).head('/');
65+
expect(res.statusCode).toBe(200);
66+
67+
res = await request(app).options('/');
68+
expect(res.statusCode).toBe(200);
69+
70+
res = await request(app).delete('/');
71+
expect(res.statusCode).toBe(200);
72+
73+
res = await request(app).patch('/');
74+
expect(res.statusCode).toBe(200);
75+
});
76+
77+
it('When any methods are allowed unsafely, call should not be rejected', async () => {
78+
const app = express();
79+
app.use(httpFirewall({ unsafeAllowAnyHttpMethod: true }));
80+
setupRoutes(app, ['TRACE']);
81+
82+
let res = await request(app).trace('/');
83+
expect(res.statusCode).toBe(200);
84+
});
85+
86+
describe('Un-Normalized URI paths should be rejected', () => {
87+
for (const path of unNormalizedPathsThatCanBeHandledBySupertest) {
88+
it(`Should reject un-normalized path: ${path}`, async () => {
89+
// console.log(`Path = ${path}`)
90+
let app = express();
91+
app.use(httpFirewall());
92+
setupRoutes(app, ['GET']);
93+
let res = await request(app).get(path);
94+
expect(res.statusCode).toBe(403);
95+
});
96+
}
97+
});
98+
99+
it('Should reject disallowed Http method', async () => {
8100
const app = express();
101+
9102
const options: HttpFirewallOptions = {
10103
allowedHttpMethods: ['POST', 'GET'],
11104
};
@@ -14,7 +107,110 @@ describe('HttpStrictFirewall test suite', () => {
14107
expect(res.statusCode).toBe(403);
15108
});
16109

17-
test('Should allow configured Http method', async () => {
110+
it('Should reject path with semicolon', async () => {
111+
const app = express();
112+
113+
const options: HttpFirewallOptions = {
114+
allowedHttpMethods: ['POST', 'GET'],
115+
};
116+
app.use(httpFirewall(options));
117+
const res = await request(app).get('/context;').set('Content-Type', 'application/json');
118+
expect(res.statusCode).toBe(403);
119+
});
120+
121+
it('Should not reject path with semicolon when its allowed', async () => {
122+
const app = express();
123+
124+
const options: HttpFirewallOptions = {
125+
allowSemicolon: true,
126+
};
127+
app.use(httpFirewall(options));
128+
const res = await request(app).get('/;').set('Content-Type', 'application/json');
129+
expect(res.statusCode).toBe(404);
130+
});
131+
132+
it('Should reject encoded percent', async () => {
133+
const app = express();
134+
135+
const options: HttpFirewallOptions = {};
136+
app.use(httpFirewall(options));
137+
const res = await request(app).get('/%25').set('Content-Type', 'application/json');
138+
expect(res.statusCode).toBe(403);
139+
});
140+
141+
it('Should reject encoded period', async () => {
142+
const app = express();
143+
144+
const options: HttpFirewallOptions = {};
145+
app.use(httpFirewall(options));
146+
const res = await request(app).get('/%2e').set('Content-Type', 'application/json');
147+
expect(res.statusCode).toBe(403);
148+
});
149+
150+
it('Should allow encoded period when permitted', async () => {
151+
const app = express();
152+
153+
const options: HttpFirewallOptions = {
154+
allowUrlEncodedPeriod: true,
155+
allowUrlEncodedPercent: true,
156+
};
157+
app.use(httpFirewall(options));
158+
setupRoutes(app, ['GET']);
159+
160+
const res = await request(app).get('/%2e').set('Content-Type', 'application/json');
161+
expect(res.statusCode).toBe(404);
162+
});
163+
164+
// TODO: THis test is not correct
165+
it('Should reject when lower bound hit for ascii chars', async () => {
166+
const app = express();
167+
168+
const options: HttpFirewallOptions = {};
169+
app.use(httpFirewall(options));
170+
setupRoutes(app, ['GET']);
171+
172+
const res = await request(app).get('/test/\\u0019').set('Content-Type', 'application/json');
173+
expect(res.statusCode).toBe(403);
174+
});
175+
176+
it('Should allow Japanese chars', async () => {
177+
const app = express();
178+
179+
const options: HttpFirewallOptions = {
180+
allowBackSlash: true,
181+
allowUrlEncodedSlash: true,
182+
allowUrlEncodedDoubleSlash: true,
183+
};
184+
app.use(httpFirewall(options));
185+
setupRoutes(app, ['GET']);
186+
187+
const res = await request(app).get('/test/\\u3042').set('Content-Type', 'application/json');
188+
expect(res.statusCode).toBe(404);
189+
});
190+
191+
it('Should not reject encoded percent when its permitted', async () => {
192+
const app = express();
193+
194+
const options: HttpFirewallOptions = {
195+
allowUrlEncodedPercent: true,
196+
};
197+
app.use(httpFirewall(options));
198+
const res = await request(app).get('/%25').set('Content-Type', 'application/json');
199+
expect(res.statusCode).toBe(404);
200+
});
201+
202+
it('Should reject encoded semicolon', async () => {
203+
const app = express();
204+
205+
const options: HttpFirewallOptions = {
206+
allowedHttpMethods: ['POST', 'GET'],
207+
};
208+
app.use(httpFirewall(options));
209+
const res = await request(app).get('/context%3B').set('Content-Type', 'application/json');
210+
expect(res.statusCode).toBe(403);
211+
});
212+
213+
it('Should allow configured Http method', async () => {
18214
// Arrange
19215
const app = express();
20216
const options: HttpFirewallOptions = {
@@ -34,3 +230,58 @@ describe('HttpStrictFirewall test suite', () => {
34230
});
35231
});
36232
});
233+
234+
const setupRoutes = (app, methods: HttpMethod[]) => {
235+
for (const method of methods) {
236+
switch (method) {
237+
case 'GET':
238+
app.get('/', (req, res) => {
239+
// You're working with an express req and res now.
240+
res.status(200).send();
241+
});
242+
break;
243+
case 'POST':
244+
app.post('/', (req, res) => {
245+
// You're working with an express req and res now.
246+
res.status(200).send();
247+
});
248+
break;
249+
case 'DELETE':
250+
app.delete('/', (req, res) => {
251+
// You're working with an express req and res now.
252+
res.status(200).send();
253+
});
254+
break;
255+
case 'HEAD':
256+
app.head('/', (req, res) => {
257+
// You're working with an express req and res now.
258+
res.status(200).send();
259+
});
260+
break;
261+
case 'OPTIONS':
262+
app.options('/', (req, res) => {
263+
// You're working with an express req and res now.
264+
res.status(200).send();
265+
});
266+
break;
267+
case 'PATCH':
268+
app.patch('/', (req, res) => {
269+
// You're working with an express req and res now.
270+
res.status(200).send();
271+
});
272+
break;
273+
case 'PUT':
274+
app.put('/', (req, res) => {
275+
// You're working with an express req and res now.
276+
res.status(200).send();
277+
});
278+
break;
279+
case 'TRACE':
280+
app.trace('/', (req, res) => {
281+
// You're working with an express req and res now.
282+
res.status(200).send();
283+
});
284+
break;
285+
}
286+
}
287+
};

‎src/index.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,2 @@
1-
export {
2-
httpFirewall,
3-
HttpFirewallOptions,
4-
HttpMethod,
5-
Predicate
6-
} from './strict-http-firewall';
1+
export { httpFirewall } from './strict-http-firewall';
2+
export { HttpFirewallOptions, HttpMethod, Predicate } from './types/firewall.models';

0 commit comments

Comments
 (0)