Fixed issues from code review

This commit is contained in:
Andrew Hancox 2017-10-03 18:03:02 +01:00
parent 5171723d0e
commit 833a25daf3
3 changed files with 9 additions and 36 deletions

View file

@ -30,7 +30,6 @@ use auth_userkey\userkey_manager_interface;
require_once($CFG->libdir . "/externallib.php");
require_once($CFG->libdir.'/authlib.php');
require_once($CFG->dirroot . '/user/lib.php');
require_once($CFG->dirroot . '/lib/classes/user.php');
/**
* User key authentication plugin.
@ -605,14 +604,14 @@ class auth_plugin_userkey extends auth_plugin_base {
$mappingfield = $this->get_mapping_field();
if ($this->should_create_user() || $this->should_update_user()) {
$parameters['firstname'] = new external_value(core_user::get_property_type('firstname'), 'The first name(s) of the user', VALUE_OPTIONAL);
$parameters['lastname'] = new external_value(core_user::get_property_type('lastname'), 'The family name of the user', VALUE_OPTIONAL);
$parameters['firstname'] = new external_value(PARAM_NOTAGS, 'The first name(s) of the user', VALUE_OPTIONAL);
$parameters['lastname'] = new external_value(PARAM_NOTAGS, 'The family name of the user', VALUE_OPTIONAL);
if ($mappingfield != 'email') {
$parameters['email'] = new external_value(core_user::get_property_type('email'), 'A valid and unique email address', VALUE_OPTIONAL);
$parameters['email'] = new external_value(PARAM_RAW_TRIMMED, 'A valid and unique email address', VALUE_OPTIONAL);
}
if ($mappingfield != 'username') {
$parameters['username'] = new external_value(core_user::get_property_type('username'), 'A valid and unique username', VALUE_OPTIONAL);
$parameters['username'] = new external_value(PARAM_USERNAME, 'A valid and unique username', VALUE_OPTIONAL);
}
}

View file

@ -125,7 +125,9 @@ class core_userkey_manager implements userkey_manager_interface {
if ($key->iprestriction) {
$remoteaddr = getremoteaddr(null);
$whitelist = get_config('auth_userkey', 'ipwhitelist');
if (!empty($whitelist)) {
if (empty($remoteaddr) ) {
print_error('noip', 'auth_userkey');
} else if (!empty($whitelist)) {
$ips = explode(';', $whitelist);
$whitelisted = false;
foreach ($ips as $ip) {
@ -136,8 +138,6 @@ class core_userkey_manager implements userkey_manager_interface {
if (!$whitelisted) {
print_error('ipmismatch', 'error', '', null, "Remote address: $remoteaddr\nKey IP: $key->iprestriction");
}
} else if (empty($remoteaddr) ) {
print_error('noip', 'auth_userkey');
} else if (!address_in_subnet($remoteaddr, $key->iprestriction)) {
print_error('ipmismatch', 'error', '', null, "Remote address: $remoteaddr\nKey IP: $key->iprestriction");
}

View file

@ -844,7 +844,7 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
}
/**
* Test that IP address mismatch exception gets thrown if incorrect IP.
* Test that IP address mismatch is ingored if IP is whitelisted.
*
* @expectedException moodle_exception
* @expectedExceptionMessage Unsupported redirect to http://www.example.com/moodle detected, execution terminated.
@ -870,33 +870,7 @@ class auth_plugin_userkey_testcase extends advanced_testcase {
}
/**
* Test that IP address mismatch exception gets thrown if incorrect IP.
*
* @expectedException moodle_exception
* @expectedExceptionMessage Unsupported redirect to http://www.example.com/moodle detected, execution terminated.
*/
public function test_ipmismatch_exception_notthrown_if_ip_is_whitelisted_temp() {
global $DB;
set_config('ipwhitelist', '192.168.1.2', 'auth_userkey');
$key = new stdClass();
$key->value = 'IpmismatchKey';
$key->script = 'auth/userkey';
$key->userid = $this->user->id;
$key->instance = $this->user->id;
$key->iprestriction = '192.168.1.1';
$key->validuntil = time() + 300;
$key->timecreated = time();
$DB->insert_record('user_private_key', $key);
$_POST['key'] = 'IpmismatchKey';
$_SERVER['HTTP_CLIENT_IP'] = '192.168.1.2';
@$this->auth->user_login_userkey();
}
/**
* Test that IP address mismatch exception gets thrown if incorrect IP.
* Test that IP address mismatch exception gets thrown if incorrect IP and outside whitelist.
*
* @expectedException moodle_exception
* @expectedExceptionMessage Client IP address mismatch