From c84216c6f8bcfa93e932ff970f22a86ed210baf2 Mon Sep 17 00:00:00 2001 From: Dmitrii Metelkin Date: Fri, 19 Aug 2016 16:01:08 +1000 Subject: [PATCH] Fix ip restriction functionality --- auth.php | 42 +++++++++++++++++++++----- tests/auth_plugin_test.php | 62 ++++++++++++++++++++++++++++++++++++-- tests/externallib_test.php | 49 +++++++++++++++++++++++++++--- 3 files changed, 139 insertions(+), 14 deletions(-) diff --git a/auth.php b/auth.php index 5d8ac83..09fcca0 100644 --- a/auth.php +++ b/auth.php @@ -252,6 +252,19 @@ class auth_plugin_userkey extends auth_plugin_base { return false; } + /** + * Check if restriction by IP is enabled. + * + * @return bool + */ + protected function is_iprestriction_enabled() { + if (isset($this->config->iprestriction) && $this->config->iprestriction == true) { + return true; + } + + return false; + } + /** * Create a new user. */ @@ -265,24 +278,28 @@ class auth_plugin_userkey extends auth_plugin_base { /** * Return login URL. * - * @param array|stdClass $user User data. + * @param array|stdClass $data User data. * * @return string Login URL. * * @throws \invalid_parameter_exception */ - public function get_login_url($user) { + public function get_login_url($data) { global $CFG, $DB; - $user = (array)$user; + $data = (array)$data; $mappingfield = $this->get_mapping_field(); - if (!isset($user[$mappingfield]) || empty($user[$mappingfield])) { + if (!isset($data[$mappingfield]) || empty($data[$mappingfield])) { throw new invalid_parameter_exception('Required field "' . $mappingfield . '" is not set or empty.'); } + if ($this->is_iprestriction_enabled() && !isset($data['ip'])) { + throw new invalid_parameter_exception('Required parameter "ip" is not set.'); + } + $params = array( - $mappingfield => $user[$mappingfield], + $mappingfield => $data[$mappingfield], 'mnethostid' => $CFG->mnet_localhost_id, ); @@ -297,7 +314,8 @@ class auth_plugin_userkey extends auth_plugin_base { } if (!isset($this->userkeymanager)) { - $userkeymanager = new core_userkey_manager($user->id, $this->config); + $ips = isset($data['ip']) ? $data['ip'] : null; + $userkeymanager = new core_userkey_manager($user->id, $this->config, $ips); $this->set_userkey_manager($userkeymanager); } @@ -369,8 +387,18 @@ class auth_plugin_userkey extends auth_plugin_base { * @return array */ protected function get_user_fields_parameters() { + $parameters = array(); + + if ($this->is_iprestriction_enabled()) { + $parameters['ip'] = new external_value( + PARAM_HOST, + 'User IP address' + ); + } + // TODO: add more fields here when we implement user creation. - return array(); + + return $parameters; } /** diff --git a/tests/auth_plugin_test.php b/tests/auth_plugin_test.php index 5016d5f..6335066 100644 --- a/tests/auth_plugin_test.php +++ b/tests/auth_plugin_test.php @@ -197,6 +197,22 @@ class auth_plugin_userkey_testcase extends advanced_testcase { $actual = $this->auth->get_login_url($user); } + /** + * Test that auth plugin throws correct exception if we trying to request user, + * but ip field is not set and iprestriction is enabled. + * + * @expectedException \invalid_parameter_exception + * @expectedExceptionMessage Invalid parameter value detected (Required parameter "ip" is not set.) + */ + public function test_throwing_exception_if_iprestriction_is_enabled_but_ip_is_missing_in_data() { + $user = array(); + $user['email'] = 'exists@test.com'; + set_config('iprestriction', true, 'auth_userkey'); + $this->auth = new auth_plugin_userkey(); + + $actual = $this->auth->get_login_url($user); + } + /** * Test that we can request a user provided user data as an array. */ @@ -239,6 +255,28 @@ class auth_plugin_userkey_testcase extends advanced_testcase { $this->assertEquals($expected, $actual); } + /** + * Test that we can request a user provided user data as an object. + */ + public function test_return_correct_login_url_if_iprestriction_is_enabled_and_data_is_correct() { + global $CFG; + + $user = new stdClass(); + $user->username = 'username'; + $user->email = 'exists@test.com'; + $user->ip = '192.168.1.1'; + + self::getDataGenerator()->create_user($user); + + $userkeymanager = new \auth_userkey\fake_userkey_manager(); + $this->auth->set_userkey_manager($userkeymanager); + + $expected = $CFG->wwwroot . '/auth/userkey/login.php?key=FaKeKeyFoRtEsTiNg'; + $actual = $this->auth->get_login_url($user); + + $this->assertEquals($expected, $actual); + } + /** * Test that we can get login url if we do not use fake keymanager. */ @@ -336,6 +374,26 @@ class auth_plugin_userkey_testcase extends advanced_testcase { $actual = $this->auth->get_request_login_url_user_parameters(); $this->assertEquals($expected, $actual); + + // Check IP if iprestriction disabled. + set_config('iprestriction', false, 'auth_userkey'); + $this->auth = new auth_plugin_userkey(); + $expected = array(); + $actual = $this->auth->get_request_login_url_user_parameters(); + $this->assertEquals($expected, $actual); + + // Check IP if iprestriction enabled. + set_config('iprestriction', true, 'auth_userkey'); + $this->auth = new auth_plugin_userkey(); + $expected = array( + 'ip' => new external_value( + PARAM_HOST, + 'User IP address' + ), + ); + $actual = $this->auth->get_request_login_url_user_parameters(); + $this->assertEquals($expected, $actual); + } /** @@ -524,12 +582,12 @@ class auth_plugin_userkey_testcase extends advanced_testcase { } /** - * Test that IP address mismatch exception gets thrown if incorrect IP. + * Test that IP address mismatch exception gets thrown if user id is incorrect. * * @expectedException moodle_exception * @expectedExceptionMessage Invalid user id */ - public function test_invalid_user_exception_thrown_if_ip_is_incorrect() { + public function test_invalid_user_exception_thrown_if_user_is_invalid() { global $DB; $key = new stdClass(); diff --git a/tests/externallib_test.php b/tests/externallib_test.php index e996639..4d0342f 100644 --- a/tests/externallib_test.php +++ b/tests/externallib_test.php @@ -126,15 +126,34 @@ class auth_userkey_externallib_testcase extends advanced_testcase { $this->assertTrue(is_array($result)); $this->assertTrue(key_exists('loginurl', $result)); $this->assertEquals($expectedurl, $result['loginurl']); + + // IP restriction. + set_config('iprestriction', true, 'auth_userkey'); + set_config('mappingfield', 'idnumber', 'auth_userkey'); + $params = array( + 'idnumber' => 'idnumber', + 'ip' => '192.168.1.1', + ); + + // Simulate the web service server. + $result = auth_userkey_external::request_login_url($params); + $result = external_api::clean_returnvalue(auth_userkey_external::request_login_url_returns(), $result); + + $actualkey = $DB->get_record('user_private_key', array('userid' => $this->user->id)); + $expectedurl = $CFG->wwwroot . '/auth/userkey/login.php?key=' . $actualkey->value; + + $this->assertTrue(is_array($result)); + $this->assertTrue(key_exists('loginurl', $result)); + $this->assertEquals($expectedurl, $result['loginurl']); } /** - * Test call with incorrect required parameter. + * Test call with missing email required parameter. * * @expectedException invalid_parameter_exception * @expectedExceptionMessage Invalid parameter value detected (Required field "email" is not set or empty.) */ - public function test_request_incorrect_parameters() { + public function test_exception_thrown_if_required_parameter_email_is_not_seе() { global $CFG; $this->setAdminUser(); @@ -143,9 +162,29 @@ class auth_userkey_externallib_testcase extends advanced_testcase { $params = array( 'bla' => 'exists@test.com', ); - // Simulate the web service server. - $result = auth_userkey_external::request_login_url($params); - $result = external_api::clean_returnvalue(auth_userkey_external::request_login_url_returns(), $result); + + auth_userkey_external::request_login_url($params); + } + + /** + * Test call with missing ip required parameter. + * + * @expectedException invalid_parameter_exception + * @expectedExceptionMessage Invalid parameter value detected (Required parameter "ip" is not set.) + */ + public function test_exception_thrown_if_required_parameter_ip_is_not_seе() { + global $CFG; + + $this->setAdminUser(); + $CFG->auth = "userkey"; + + set_config('iprestriction', true, 'auth_userkey'); + + $params = array( + 'email' => 'exists@test.com', + ); + + auth_userkey_external::request_login_url($params); } /**