From 91c4be7df42b2bc946e54897d05a00f65fa441f0 Mon Sep 17 00:00:00 2001
From: Andrew Hancox <andrewdchancox@googlemail.com>
Date: Sat, 18 Nov 2017 14:10:45 +0000
Subject: [PATCH] Fix issue matching IPs when whitelisting is enabled

---
 classes/core_userkey_manager.php    | 61 ++++++++++++++++-------------
 tests/core_userkey_manager_test.php | 19 +++++++++
 2 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/classes/core_userkey_manager.php b/classes/core_userkey_manager.php
index c302cb9..2288670 100644
--- a/classes/core_userkey_manager.php
+++ b/classes/core_userkey_manager.php
@@ -122,38 +122,45 @@ class core_userkey_manager implements userkey_manager_interface {
             print_error('expiredkey');
         }
 
-        if ($key->iprestriction) {
-            $remoteaddr = getremoteaddr(null);
-
-            if (isset($this->config->ipwhitelist)) {
-                $whitelist = $this->config->ipwhitelist;
-            } else {
-                $whitelist = false;
-            }
-
-            if (empty($remoteaddr) ) {
-                print_error('noip', 'auth_userkey');
-            } else if (!empty($whitelist)) {
-                $ips = explode(';', $whitelist);
-                $whitelisted = false;
-                foreach ($ips as $ip) {
-                    if (address_in_subnet($remoteaddr, $ip)) {
-                        $whitelisted = true;
-                    }
-                }
-                if (!$whitelisted) {
-                    print_error('ipmismatch', 'error', '', null, "Remote address: $remoteaddr\nKey IP: $key->iprestriction");
-                }
-            } else if (!address_in_subnet($remoteaddr, $key->iprestriction)) {
-                print_error('ipmismatch', 'error', '', null, "Remote address: $remoteaddr\nKey IP: $key->iprestriction");
-            }
-        }
-
         if (!$user = $DB->get_record('user', array('id' => $key->userid))) {
             print_error('invaliduserid');
         }
 
+        $this->validate_ip_address($key);
         return $key;
     }
 
+    /**
+     * Validates key IP address and returns true if valid.
+     *
+     * @param object $key Key object including userid property.
+     *
+     * @throws \moodle_exception If provided key is not valid.
+     */
+    protected function validate_ip_address($key) {
+        if (!$key->iprestriction) {
+            return true;
+        }
+
+        $remoteaddr = getremoteaddr(null);
+
+        if (empty($remoteaddr)) {
+            print_error('noip', 'auth_userkey');
+        }
+
+        if (address_in_subnet($remoteaddr, $key->iprestriction)) {
+            return true;
+        }
+
+        if (isset($this->config->ipwhitelist)) {
+            $ips = explode(';', $this->config->ipwhitelist);
+            foreach ($ips as $ip) {
+                if (address_in_subnet($remoteaddr, $ip)) {
+                    return true;
+                }
+            }
+        }
+
+        print_error('ipmismatch', 'error', '', null, "Remote address: $remoteaddr\nKey IP: $key->iprestriction");
+    }
 }
diff --git a/tests/core_userkey_manager_test.php b/tests/core_userkey_manager_test.php
index 14fb4bc..a6945d3 100644
--- a/tests/core_userkey_manager_test.php
+++ b/tests/core_userkey_manager_test.php
@@ -169,6 +169,25 @@ class core_userkey_manager_testcase extends advanced_testcase {
         $manager->validate_key($value);
     }
 
+    /**
+     * Test that IP address mismatch exception gets thrown if incorrect IP and outside whitelist.
+     */
+    public function test_create_correct_key_if_ip_correct_not_whitelisted_and_whitelist_set() {
+        global $DB;
+
+        $this->config->iprestriction = true;
+
+        $this->config->ipwhitelist = '10.0.0.0/8;172.16.0.0/12;192.168.0.0/16';
+
+        $manager = new core_userkey_manager($this->config);
+        $value = $manager->create_key($this->user->id, '193.168.1.1');
+
+        $_SERVER['HTTP_CLIENT_IP'] = '193.168.1.1';
+
+        $key = $manager->validate_key($value);
+        $this->assertEquals($this->user->id, $key->userid);
+    }
+
     /**
      * Test that key is accepted if incorrect IP and within whitelist.
      */