Merge pull request #51 from pbojinov/fintechstudios-fix/x-forwarded-for

Fintechstudios fix/x forwarded for

.gitignore

@@ -6,3 +6,5 @@ node_modules
 coverage
 package-lock.json
 yarn.lock
+.nyc_output
+.idea

README.md

@@ -53,7 +53,7 @@ It looks for specific headers in the request and falls back to some defaults if
 The user ip is determined by the following order:
 
 1. `X-Client-IP`  
-2. `X-Forwarded-For` (Header may return multiple IP addresses in the format: "client IP, proxy 1 IP, proxy 2 IP", so we take the first one.)
+2. `X-Forwarded-For` (Header may return multiple IP addresses in the format: "proxy 1 IP, proxy 2 IP, client IP, ", so we take the the last one.)
 3. `CF-Connecting-IP` (Cloudflare)
 4. `Fastly-Client-Ip` (Fastly CDN and Firebase hosting header when forwared to a cloud function)
 5. `True-Client-Ip` (Akamai and Cloudflare)

src/index.js

@@ -16,7 +16,7 @@ function getClientIpFromXForwardedFor(value) {
     }
 
     // x-forwarded-for may return multiple IP addresses in the format:
-    // "client IP, proxy 1 IP, proxy 2 IP"
+    // "proxy 1 IP, proxy 2 IP, client IP"
     // Therefore, the right-most IP address is the IP address of the most recent proxy
     // and the left-most IP address is the IP address of the originating client.
     // source: http://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html
@@ -34,9 +34,16 @@ function getClientIpFromXForwardedFor(value) {
     });
 
     // Sometimes IP addresses in this header can be 'unknown' (http://stackoverflow.com/a/11285650).
-    // Therefore taking the left-most IP address that is not unknown
+    // Therefore taking the right-most IP address that is not unknown
     // A Squid configuration directive can also set the value to "unknown" (http://www.squid-cache.org/Doc/config/forwarded_for/)
-    return forwardedIps.find(is.ip);
+    for (let i = forwardedIps.length - 1; i >= 0; i -= 1) {
+        if (is.ip(forwardedIps[i])) {
+            return forwardedIps[i];
+        }
+    }
+
+    // If no value in the split list is an ip, return null
+    return null;
 }
 
 /**

test/index.js

@@ -45,8 +45,8 @@ test('req.headers is undefined', (t) => {
 
 test('getClientIpFromXForwardedFor', (t) => {
     t.plan(3);
-    t.equal(requestIp.getClientIpFromXForwardedFor('107.77.213.113, 172.31.41.116'), '107.77.213.113');
-    t.equal(requestIp.getClientIpFromXForwardedFor('unknown, unknown'), undefined);
+    t.equal(requestIp.getClientIpFromXForwardedFor('107.77.213.113, 172.31.41.116'), '172.31.41.116');
+    t.equal(requestIp.getClientIpFromXForwardedFor('unknown, unknown'), null);
     t.throws(() => requestIp.getClientIpFromXForwardedFor({}), TypeError);
 });
 
@@ -121,8 +121,8 @@ test('x-forwarded-for', (t) => {
         request(options, (error, response, found) => {
             if (!error && response.statusCode === 200) {
                 // make sure response ip is the same as the one we passed in
-                const firstIp = options.headers['x-forwarded-for'].split(',')[0].trim();
-                t.equal(firstIp, found);
+                const lastIp = options.headers['x-forwarded-for'].split(',')[2].trim();
+                t.equal(lastIp, found);
                 server.close();
             }
         });
@@ -496,7 +496,7 @@ test('android request to AWS EBS app (x-forwarded-for)', (t) => {
         headers: {
             host: '[redacted]',
             'x-real-ip': '172.31.41.116',
-            'x-forwarded-for': '107.77.213.113, 172.31.41.116',
+            'x-forwarded-for': `172.31.41.116, ${wanted}`,
             'accept-encoding': 'gzip',
             'user-agent': 'okhttp/3.4.1',
             'x-forwarded-port': '443',