From 715a8f96b29ee7f1c55b57123e50f694c83f1ed1 Mon Sep 17 00:00:00 2001 From: Robert Boloc Date: Thu, 23 Jul 2015 20:50:06 +0100 Subject: [PATCH 1/2] Added support for CIDRs --- auth.php | 64 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/auth.php b/auth.php index 15bd1e5..dd6577c 100755 --- a/auth.php +++ b/auth.php @@ -38,12 +38,9 @@ require_once($CFG->dirroot.'/auth/manual/auth.php'); */ class auth_plugin_ip extends auth_plugin_manual { - /** - * Constructor - */ function __construct() { $this->authtype = 'ip'; - $this->config = get_config('auth_ip'); + $this->config = get_config('auth_ip'); } /** @@ -57,19 +54,66 @@ class auth_plugin_ip extends auth_plugin_manual { function user_login($username, $password) { global $DB, $CFG; if (($user = $DB->get_record('user', array('username'=>$username, 'mnethostid'=>$CFG->mnet_localhost_id)))) { - $valid_ips = explode(',', $this->config->valid_ips); - //check if IP is one of the restricted ones. - $remote_addr = filter_input(INPUT_SERVER, 'REMOTE_ADDR'); - if (isset($remote_addr) && in_array($remote_addr, $valid_ips)) { + // Check if IP is one of the restricted ones. + $userIp = filter_input(INPUT_SERVER, 'REMOTE_ADDR'); + + if (isset($userIp) && $this->is_ip_valid($userIp)) { return validate_internal_user_password($user, $password); } else { return false; } } - // if no valid username, we do not allow to create a new user using this auth type. + // If no valid username, we do not allow to create a new user using this auth type. return false; } + /** + * Determine if the $ip is in the allowed list of IP or CIDR. + * + * @see https://secure.php.net/manual/en/ref.network.php#74656 + * @param $ip + * @return bool + */ + function is_ip_valid($ip) { + // List of allowed IP addresses or CIDR ranges + $valid_ips_or_cidrs = explode(',', str_replace(' ', '', $this->config->valid_ips)); + + // Check all the allowed IP or CIDR for matches + foreach ($valid_ips_or_cidrs as $valid_ip_or_cidr) { + // If CIDR check if in range + if ($this->is_cidr($valid_ip_or_cidr)) { + list ($net, $mask) = explode('/', $valid_ip_or_cidr); + + $ip_net = ip2long($net); + $ip_mask = ~((1 << (32 - $mask)) - 1); + + $ip_ip = ip2long($ip); + + $ip_ip_net = $ip_ip & $ip_mask; + + if ($ip_ip_net === $ip_net) { + return true; + } + // Simple IP compare with equality + } elseif ($valid_ip_or_cidr === $ip) { + return true; + } + } + + // No match found mark as not allowed + return false; + } + + /** + * Check if a string is a CIDR. + * + * @param string $ip_or_cidr + * @return bool + */ + function is_cidr($ip_or_cidr) { + return strpos($ip_or_cidr, '/') > 0; + } + /** * Returns true if this authentication plugin is 'internal'. * @@ -90,7 +134,7 @@ class auth_plugin_ip extends auth_plugin_manual { * @param array $user_fields * @return void */ - function config_form($config, $err, $user_fields) { + function config_form($config, $error, $user_fields) { include 'config.html'; } From 0cb18b4573147adf0f1c8d1e50ca1cf298b6120d Mon Sep 17 00:00:00 2001 From: Robert Boloc Date: Thu, 23 Jul 2015 20:50:17 +0100 Subject: [PATCH 2/2] Added unit tests --- tests/ip_test.php | 96 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 tests/ip_test.php diff --git a/tests/ip_test.php b/tests/ip_test.php new file mode 100644 index 0000000..58b4344 --- /dev/null +++ b/tests/ip_test.php @@ -0,0 +1,96 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot.'/auth/ip/auth.php'); + +class auth_ip_testcase extends advanced_testcase { + protected $authplugin; + + protected function setUp() { + $this->authplugin = new auth_plugin_ip(); + } + + /** + * Test that valid IPs or IPs in range are detected as valid. + * + * @dataProvider is_ip_valid_data_provider + * + * @param array $valid_ips + * @param string $ip + * @param boolean $is_valid + */ + public function test_is_ip_valid_detects_valid_ips($valid_ips, $ip, $is_valid) { + $this->authplugin->config->valid_ips = $valid_ips; + + $this->assertEquals( + $is_valid, + $this->authplugin->is_ip_valid($ip) + ); + } + + /** + * @return array + */ + public static function is_ip_valid_data_provider() { + return array( + array('192.168.1.1,192.168.1.2', '192.168.1.1', true), + array('192.168.1.1,192.168.1.2', '192.168.1.3', false), + array('192.168.0.0/24', '192.168.0.200', true), + array('111.112.0.0/12,96.0.0.0/6', '192.168.0.200', false), + array('111.112.0.0/12,96.0.0.0/6', '99.255.255.254', true), + array('111.112.0.0/12,10.40.22.0/24,96.0.0.0/6', '10.40.22.50', true), + array('111.112.0.0/12, 10.40.22.0/24, 96.0.0.0/6', '10.40.22.50', true), + array(' 111.112.0.0 ', '111.112.0.0', true), // Extra spaces for testing + ); + } + + /** + * Test range detection + * + * @dataProvider is_cidr_data_provider + * + * @param string $ip_or_cidr + * @param boolean $is_cidr + */ + public function test_is_cidr_detects_cidrs($ip_or_cidr, $is_cidr) { + $this->assertEquals( + $this->authplugin->is_cidr($ip_or_cidr), + $is_cidr + ); + } + + /** + * @return array + */ + public static function is_cidr_data_provider() { + return array( + array('192.168.1.1', false), + array('192.168.1.1/24', true), + array('10.15.1.1', false), + array('10.15.1.1/2', true), + ); + } + + /** + * Test the plugin is not marked as internal. + */ + public function test_is_not_internal() { + $this->assertFalse($this->authplugin->is_internal()); + } +}