lib/policyEvaluator/requestUtils.js

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

lib/s3routes/routes.js

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

tests/unit/policyEvaluator/requestUtils.js

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