Compare commits

...

2 Commits

Author SHA1 Message Date
Naren cd972d1e07 bf: ARSN-57 improve requestUtil
check request header 'x-forwarded-for' if there is no request
configuration.
2022-01-28 15:35:39 -08:00
Naren 96b4de23f0 bf: ARSN-57 log correct client ip 2022-01-27 19:07:17 -08:00
3 changed files with 50 additions and 15 deletions

View File

@ -7,8 +7,9 @@ const ipCheck = require('../ipCheck');
* @return {string} - returns client IP from the request * @return {string} - returns client IP from the request
*/ */
function getClientIp(request, s3config) { function getClientIp(request, s3config) {
const clientIp = request.socket.remoteAddress;
const requestConfig = s3config ? s3config.requests : {}; const requestConfig = s3config ? s3config.requests : {};
const remoteAddress = request.socket.remoteAddress;
const clientIp = requestConfig ? remoteAddress : request.headers['x-forwarded-for'] || remoteAddress;
if (requestConfig) { if (requestConfig) {
const { trustedProxyCIDRs, extractClientIPFromHeader } = requestConfig; const { trustedProxyCIDRs, extractClientIPFromHeader } = requestConfig;
/** /**

View File

@ -10,6 +10,8 @@ const routeOPTIONS = require('./routes/routeOPTIONS');
const routesUtils = require('./routesUtils'); const routesUtils = require('./routesUtils');
const routeWebsite = require('./routes/routeWebsite'); const routeWebsite = require('./routes/routeWebsite');
const requestUtils = require('../../lib/policyEvaluator/requestUtils');
const routeMap = { const routeMap = {
GET: routeGET, GET: routeGET,
PUT: routePUT, PUT: routePUT,
@ -67,7 +69,8 @@ function checkBucketAndKey(bucketName, objectKey, method, reqQuery,
return undefined; return undefined;
} }
function checkTypes(req, res, params, logger) { // TODO: ARSN-59 remove assertions or restrict it to dev environment only.
function checkTypes(req, res, params, logger, s3config) {
assert.strictEqual(typeof req, 'object', assert.strictEqual(typeof req, 'object',
'bad routes param: req must be an object'); 'bad routes param: req must be an object');
assert.strictEqual(typeof res, 'object', assert.strictEqual(typeof res, 'object',
@ -114,6 +117,9 @@ function checkTypes(req, res, params, logger) {
}); });
assert.strictEqual(typeof params.dataRetrievalFn, 'function', assert.strictEqual(typeof params.dataRetrievalFn, 'function',
'bad routes param: dataRetrievalFn must be a defined function'); 'bad routes param: dataRetrievalFn must be a defined function');
if (s3config) {
assert.strictEqual(typeof s3config, 'object', 'bad routes param: s3config must be an object');
}
} }
/** routes - route request to appropriate method /** routes - route request to appropriate method
@ -134,9 +140,10 @@ function checkTypes(req, res, params, logger) {
* values for whether queries are supported * values for whether queries are supported
* @param {function} params.dataRetrievalFn - function to retrieve data * @param {function} params.dataRetrievalFn - function to retrieve data
* @param {RequestLogger} logger - werelogs logger instance * @param {RequestLogger} logger - werelogs logger instance
* @param {String} [s3config] - s3 configuration
* @returns {undefined} * @returns {undefined}
*/ */
function routes(req, res, params, logger) { function routes(req, res, params, logger, s3config) {
checkTypes(req, res, params, logger); checkTypes(req, res, params, logger);
const { const {
@ -150,7 +157,7 @@ function routes(req, res, params, logger) {
} = params; } = params;
const clientInfo = { const clientInfo = {
clientIP: req.socket.remoteAddress, clientIP: requestUtils.getClientIp(req, s3config),
clientPort: req.socket.remotePort, clientPort: req.socket.remotePort,
httpMethod: req.method, httpMethod: req.method,
httpURL: req.url, httpURL: req.url,

View File

@ -12,8 +12,7 @@ describe('requestUtils.getClientIp', () => {
const testClientIp2 = '192.168.104.0'; const testClientIp2 = '192.168.104.0';
const testProxyIp = '192.168.100.2'; const testProxyIp = '192.168.100.2';
it('should return client Ip address from header ' + it('should return client Ip address from header if the request comes via proxies', () => {
'if the request comes via proxies', () => {
const request = new DummyRequest({ const request = new DummyRequest({
headers: { headers: {
'x-forwarded-for': [testClientIp1, testProxyIp].join(','), 'x-forwarded-for': [testClientIp1, testProxyIp].join(','),
@ -28,13 +27,9 @@ describe('requestUtils.getClientIp', () => {
assert.strictEqual(result, testClientIp1); assert.strictEqual(result, testClientIp1);
}); });
it('should not return client Ip address from header ' + it('should return client Ip address from socket info if the request is not forwarded from proxies', () => {
'if the request is not forwarded from proxies or ' +
'fails ip check', () => {
const request = new DummyRequest({ const request = new DummyRequest({
headers: { headers: {},
'x-forwarded-for': [testClientIp1, testProxyIp].join(','),
},
url: '/', url: '/',
parsedHost: 'localhost', parsedHost: 'localhost',
socket: { socket: {
@ -45,12 +40,11 @@ describe('requestUtils.getClientIp', () => {
assert.strictEqual(result, testClientIp2); assert.strictEqual(result, testClientIp2);
}); });
it('should not return client Ip address from header ' + it('should not return client Ip address from header if the request is forwarded from proxies, but the request ' +
'if the request is forwarded from proxies, but the request ' +
'has no expected header or the header value is empty', () => { 'has no expected header or the header value is empty', () => {
const request = new DummyRequest({ const request = new DummyRequest({
headers: { headers: {
'x-forwarded-for': ' ', 'x-forwarded-for': '',
}, },
url: '/', url: '/',
parsedHost: 'localhost', parsedHost: 'localhost',
@ -61,4 +55,37 @@ describe('requestUtils.getClientIp', () => {
const result = requestUtils.getClientIp(request, configWithProxy); const result = requestUtils.getClientIp(request, configWithProxy);
assert.strictEqual(result, testClientIp2); assert.strictEqual(result, testClientIp2);
}); });
it('should return client Ip address from header if the request comes via proxies and ' +
'no request config is available', () => {
const request = new DummyRequest({
headers: {
'x-forwarded-for': testClientIp1,
},
url: '/',
parsedHost: 'localhost',
socket: {
remoteAddress: testProxyIp,
},
});
const result = requestUtils.getClientIp(request, configWithoutProxy);
assert.strictEqual(result, testClientIp1);
});
it('should return client Ip address from socket info if the request comes via proxies and ' +
'request config is available and ip check fails', () => {
const dummyRemoteIP = '221.10.221.10';
const request = new DummyRequest({
headers: {
'x-forwarded-for': testClientIp1,
},
url: '/',
parsedHost: 'localhost',
socket: {
remoteAddress: dummyRemoteIP,
},
});
const result = requestUtils.getClientIp(request, configWithProxy);
assert.strictEqual(result, dummyRemoteIP);
});
}); });