Bug 25950: Remove <client> in X-Forwarded-For from proxy tests

This patch removes the <client> ip address in the X-Forwarded-For
header from being tested against koha_trusted_proxies.

Without this patch, REMOTE_ADDR will be set to null, if the <client>
ip address matches against koha_trusted_proxies.

To Test:
1. Run the unit test t/Koha/Middleware/RealIP.t

Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/Koha/Middleware/RealIP.pm b/Koha/Middleware/RealIP.pm
index 88a843d..fc9f280 100644 (file)
--- a/Koha/Middleware/RealIP.pm
+++ b/Koha/Middleware/RealIP.pm
@@ -81,9 +81,10 @@ sub get_real_ip {
 
     my $trusted_proxies = get_trusted_proxies();
 
+    #X-Forwarded-For: <client>, <proxy1>, <proxy2>
+    my $real_ip = shift @forwarded_for;
     my @unconfirmed = ( @forwarded_for, $remote_addr );
 
-    my $real_ip;
     while (my $addr = pop @unconfirmed) {
         my $has_matched = 0;
         foreach my $netmask (@$trusted_proxies) {

diff --git a/t/Koha/Middleware/RealIP.t b/t/Koha/Middleware/RealIP.t
index 34bc02a..c60994b 100644 (file)
--- a/t/Koha/Middleware/RealIP.t
+++ b/t/Koha/Middleware/RealIP.t
@@ -20,7 +20,7 @@
 
 use strict;
 use warnings;
-use Test::More tests => 13;
+use Test::More tests => 11;
 use Test::Warn;
 
 use t::lib::Mocks;
@@ -28,62 +28,106 @@ use_ok("Koha::Middleware::RealIP");
 
 my ($remote_address,$x_forwarded_for_header,$address);
 
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "";
-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-is($address,'1.1.1.1',"There is no X-Forwarded-For header, so just use the remote address");
-
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2";
-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-is($address,'1.1.1.1',"Don't trust 1.1.1.1 as a proxy, so use it as the remote address");
-
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2";
-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.1');
-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-is($address,'2.2.2.2',"Trust proxy (1.1.1.1), so use the X-Forwarded-For header for the remote address");
-
-
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2,3.3.3.3";
-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.1 3.3.3.3');
-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-is($address,'2.2.2.2',"Trust multiple proxies (1.1.1.1 and 3.3.3.3), so use the X-Forwaded-For <client> portion for the remote address");
-
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2,3.3.3.3";
-t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad configuration');
-warnings_are {
+subtest "No X-Forwarded-For header" => sub {
+    plan tests => 1;
+    $remote_address = "1.1.1.1";
+    $x_forwarded_for_header = "";
     $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-} ["could not parse bad","could not parse configuration"],"Warn on misconfigured koha_trusted_proxies";
-is($address,'1.1.1.1',"koha_trusted_proxies is misconfigured so ignore the X-Forwarded-For header");
+    is($address,'1.1.1.1',"There is no X-Forwarded-For header, so just use the remote address");
+};
+
+subtest "No configuration" => sub {
+    plan tests => 1;
+    $remote_address = "2.2.2.2";
+    $x_forwarded_for_header = "1.1.1.1";
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'2.2.2.2',"No trusted proxies available, so don't trust 2.2.2.2 as a proxy, instead use it as the remote address");
+};
+
+subtest "Bad configuration" => sub {
+    plan tests => 4;
+    $remote_address = "1.1.1.1";
+    $x_forwarded_for_header = "2.2.2.2,3.3.3.3";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad configuration');
+    warnings_are {
+        $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    } ["could not parse bad","could not parse configuration"],"Warn on misconfigured koha_trusted_proxies";
+    is($address,'1.1.1.1',"koha_trusted_proxies is misconfigured so ignore the X-Forwarded-For header");
+
+    $remote_address = "1.1.1.1";
+    $x_forwarded_for_header = "2.2.2.2";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad 1.1.1.1');
+    warning_is {
+        $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    } "could not parse bad","Warn on partially misconfigured koha_trusted_proxies";
+    is($address,'2.2.2.2',"koha_trusted_proxies contains an invalid value but still includes one correct value, which is relevant, so use X-Forwarded-For header");
+};
+
+subtest "Fail proxy isn't trusted" => sub {
+    plan tests => 1;
+    $remote_address = "2.2.2.2";
+    $x_forwarded_for_header = "1.1.1.1";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '3.3.3.3');
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'2.2.2.2',"The 2.2.2.2 proxy isn't in our trusted list, so use it as the remote address");
+};
+
+subtest "The most recent proxy is trusted but the proxy before it is not trusted" => sub {
+    plan tests => 1;
+    $remote_address = "3.3.3.3";
+    $x_forwarded_for_header = "1.1.1.1, 2.2.2.2";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '3.3.3.3');
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'2.2.2.2',"We trust 3.3.3.3, but we don't trust 2.2.2.2, so use 2.2.2.2 as the remote address");
+};
+
+subtest "Success one proxy" => sub {
+    plan tests => 1;
+    $remote_address = "2.2.2.2";
+    $x_forwarded_for_header = "1.1.1.1";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.2');
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'1.1.1.1',"Trust proxy (2.2.2.2), so use the client IP from the X-Forwarded-For header for the remote address");
+};
+
+subtest "Success multiple proxies" => sub {
+    plan tests => 1;
+    $remote_address = "2.2.2.2";
+    $x_forwarded_for_header = "1.1.1.1,3.3.3.3";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.2 3.3.3.3');
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'1.1.1.1',"Trust multiple proxies (2.2.2.2 and 3.3.3.3), so use the X-Forwaded-For <client> portion for the remote address");
+};
+
+subtest "Test alternative configuration styles" => sub {
+    plan tests => 3;
+    $remote_address = "2.2.2.2";
+    $x_forwarded_for_header = "1.1.1.1";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.0/24');
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'1.1.1.1',"Trust proxy (2.2.2.2) using CIDR notation, so use the X-Forwarded-For header for the remote address");
 
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2";
-t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad 1.1.1.1');
-warning_is {
+    $remote_address = "2.2.2.2";
+    $x_forwarded_for_header = "1.1.1.1";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2');
     $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-} "could not parse bad","Warn on partially misconfigured koha_trusted_proxies";
-is($address,'2.2.2.2',"koha_trusted_proxies contains an invalid value but still includes one correct value, which is relevant, so use X-Forwarded-For header");
-
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2";
-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.0/24');
-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-is($address,'2.2.2.2',"Trust proxy (1.1.1.1) using CIDR notation, so use the X-Forwarded-For header for the remote address");
-
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2";
-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1');
-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-is($address,'2.2.2.2',"Trust proxy (1.1.1.1) using abbreviated notation, so use the X-Forwarded-For header for the remote address");
-
-$remote_address = "1.1.1.1";
-$x_forwarded_for_header = "2.2.2.2";
-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.0:255.255.255.0');
-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
-is($address,'2.2.2.2',"Trust proxy (1.1.1.1) using an IP address and netmask separated by a colon, so use the X-Forwarded-For header for the remote address");
+    is($address,'1.1.1.1',"Trust proxy (2.2.2.2) using abbreviated notation, so use the X-Forwarded-For header for the remote address");
+
+    $remote_address = "2.2.2.2";
+    $x_forwarded_for_header = "1.1.1.1";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.0:255.255.255.0');
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'1.1.1.1',"Trust proxy (2.2.2.2) using an IP address and netmask separated by a colon, so use the X-Forwarded-For header for the remote address");
+};
+
+subtest "Client IP is properly processed even if it is in koha_trusted_proxies" => sub {
+    $remote_address = "1.1.1.2";
+    $x_forwarded_for_header = "1.1.1.1";
+    t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.0/24');
+    $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header );
+    is($address,'1.1.1.1',"The X-Forwarded-For value matches the koha_trusted_proxies, but there is no proxy specified");
+    done_testing(1);
+};
 
 subtest "IPv6 support" => sub {
     my ($remote_address,$x_forwarded_for_header,$address);