Merge branch 'w/8.1/bugfix/ARSN-453-fix-ipchecks' into tmp/octopus/w/…

…8.2/bugfix/ARSN-453-fix-ipchecks

lib/policyEvaluator/requestUtils.ts

@@ -8,9 +8,6 @@ export interface S3Config {
     }
 }
 
-// TODO
-//   I'm not sure about this behavior.
-//   Should it returns string | string[] | undefined or string ?
 /**
  * getClientIp - Gets the client IP from the request
  * @param request - http request object
@@ -20,8 +17,7 @@ export interface S3Config {
 export function getClientIp(request: IncomingMessage, s3config?: S3Config): string {
     const requestConfig = s3config?.requests;
     const remoteAddress = request.socket.remoteAddress;
-    // TODO What to do if clientIp === undefined ?
-    const clientIp = (requestConfig ? remoteAddress : request.headers['x-forwarded-for'] || remoteAddress)?.toString() ?? '';
+    const clientIp = remoteAddress?.toString() ?? '';
     if (requestConfig) {
         const { trustedProxyCIDRs, extractClientIPFromHeader } = requestConfig;
         /**
@@ -30,11 +26,14 @@ export function getClientIp(request: IncomingMessage, s3config?: S3Config): stri
          * which header to be used to extract client IP
          */
         if (ipCheck.ipMatchCidrList(trustedProxyCIDRs, clientIp)) {
-            const ipFromHeader = request.headers[extractClientIPFromHeader]?.toString();
+            // Request headers in nodejs are lower-cased, so we should not
+            // be case-sentive when looking for the header, as http headers
+            // are case-insensitive.
+            const ipFromHeader = request.headers[extractClientIPFromHeader.toLowerCase()]?.toString();
             if (ipFromHeader && ipFromHeader.trim().length) {
                 return ipFromHeader.split(',')[0].trim();
             }
         }
     }
-    return clientIp?.toString() ?? '';
+    return clientIp;
 }

package.json

@@ -3,7 +3,7 @@
   "engines": {
     "node": ">=16"
   },
-  "version": "8.1.143",
+  "version": "8.1.144",
   "description": "Common utilities for the S3 project components",
   "main": "build/index.js",
   "repository": {

tests/unit/policyEvaluator/requestUtils.spec.js

@@ -27,6 +27,27 @@ describe('requestUtils.getClientIp', () => {
         assert.strictEqual(result, testClientIp1);
     });
 
+    it('should return client Ip address in the proxy case when the header has uppercases', () => {
+        const request = new DummyRequest({
+            headers: {
+                'x-forwarded-for': [testClientIp1, testProxyIp].join(','),
+            },
+            url: '/',
+            parsedHost: 'localhost',
+            socket: {
+                remoteAddress: testProxyIp,
+            },
+        });
+        const result = requestUtils.getClientIp(request, {
+            requests: {
+                viaProxy: true,
+                trustedProxyCIDRs: ['192.168.100.0/22'],
+                extractClientIPFromHeader: 'X-Forwarded-For',
+            },
+        });
+        assert.strictEqual(result, testClientIp1);
+    });
+
     it('should return client Ip address from socket info if the request is not forwarded from proxies', () => {
         const request = new DummyRequest({
             headers: {},
@@ -56,8 +77,8 @@ describe('requestUtils.getClientIp', () => {
         assert.strictEqual(result, testClientIp2);
     });
 
-    it('should return client Ip address from header if the request comes via proxies and ' +
-        'no request config is available', () => {
+    it('should not return client Ip address from header if the request comes via proxies and ' +
+        'no request config is available as the proxy is not trusted', () => {
         const request = new DummyRequest({
             headers: {
                 'x-forwarded-for': testClientIp1,
@@ -69,7 +90,7 @@ describe('requestUtils.getClientIp', () => {
             },
         });
         const result = requestUtils.getClientIp(request, configWithoutProxy);
-        assert.strictEqual(result, testClientIp1);
+        assert.strictEqual(result, testProxyIp);
     });
 
     it('should return client Ip address from socket info if the request comes via proxies and ' +