From 35bec6b695a5a39d1ebdcc858c968301cbdf7af8 Mon Sep 17 00:00:00 2001 From: Dmitrii Metelkin Date: Mon, 26 Sep 2016 12:05:23 +1000 Subject: [PATCH] Use redirect function in user_login_userkey --- auth.php | 10 ++--- login.php | 3 +- tests/auth_plugin_test.php | 78 ++++++++++++++++++++++++++------------ 3 files changed, 60 insertions(+), 31 deletions(-) diff --git a/auth.php b/auth.php index ed4d228..c9ddf11 100644 --- a/auth.php +++ b/auth.php @@ -129,9 +129,7 @@ class auth_plugin_userkey extends auth_plugin_base { } /** - * Login user using userkey and return URL to redirect after. - * - * @return string URL to redirect. + * Logs a user in using userkey and redirects after. * * @throws \moodle_exception If something went wrong. */ @@ -151,10 +149,12 @@ class auth_plugin_userkey extends auth_plugin_base { $SESSION->userkey = true; if (!empty($wantsurl)) { - return $wantsurl; + $redirecturl = $wantsurl; } else { - return $CFG->wwwroot; + $redirecturl = $CFG->wwwroot; } + + $this->redirect($redirecturl); } /** diff --git a/login.php b/login.php index e4b6bef..7956ff4 100644 --- a/login.php +++ b/login.php @@ -28,5 +28,4 @@ if (!is_enabled_auth('userkey')) { print_error(get_string('pluginisdisabled', 'auth_userkey')); } -$redirect = get_auth_plugin('userkey')->user_login_userkey(); -redirect($redirect); \ No newline at end of file +get_auth_plugin('userkey')->user_login_userkey(); \ No newline at end of file diff --git a/tests/auth_plugin_test.php b/tests/auth_plugin_test.php index a5dcd91..b9c8159 100644 --- a/tests/auth_plugin_test.php +++ b/tests/auth_plugin_test.php @@ -656,18 +656,43 @@ class auth_plugin_userkey_testcase extends advanced_testcase { $_POST['key'] = 'RemoveKey'; $_SERVER['HTTP_CLIENT_IP'] = '192.168.1.1'; - // Using @ is the only way to test this. Thanks moodle! - @$this->auth->user_login_userkey(); - - $keyexists = $DB->record_exists('user_private_key', array('value' => 'RemoveKey')); - $this->assertFalse($keyexists); + try { + // Using @ is the only way to test this. Thanks moodle! + @$this->auth->user_login_userkey(); + } catch (moodle_exception $e) { + $keyexists = $DB->record_exists('user_private_key', array('value' => 'RemoveKey')); + $this->assertFalse($keyexists); + } } /** - * Test that a user loggs in correctly. + * Test that a user logs in and gets redirected correctly. + * + * @expectedException moodle_exception + * @expectedExceptionMessage Unsupported redirect to http://www.example.com/moodle detected, execution terminated. */ - public function test_that_user_logged_in() { - global $DB, $USER, $SESSION, $CFG; + public function test_that_user_logged_in_and_redirected() { + global $DB; + + $key = new stdClass(); + $key->value = 'UserLogin'; + $key->script = 'auth/userkey'; + $key->userid = $this->user->id; + $key->instance = $this->user->id; + $key->iprestriction = null; + $key->validuntil = time() + 300; + $key->timecreated = time(); + $DB->insert_record('user_private_key', $key); + + $_POST['key'] = 'UserLogin'; + @$this->auth->user_login_userkey(); + } + + /** + * Test that a user logs in correctly. + */ + public function test_that_user_logged_in_correctly() { + global $DB, $USER, $SESSION; $key = new stdClass(); $key->value = 'UserLogin'; @@ -681,18 +706,23 @@ class auth_plugin_userkey_testcase extends advanced_testcase { $_POST['key'] = 'UserLogin'; - // Using @ is the only way to test this. Thanks moodle! - $redirect = @$this->auth->user_login_userkey(); - $this->assertEquals($CFG->wwwroot, $redirect); - $this->assertEquals($this->user->id, $USER->id); - $this->assertSame(sesskey(), $USER->sesskey); - $this->assertObjectHasAttribute('userkey', $SESSION); + try { + // Using @ is the only way to test this. Thanks moodle! + @$this->auth->user_login_userkey(); + } catch (moodle_exception $e) { + $this->assertEquals($this->user->id, $USER->id); + $this->assertSame(sesskey(), $USER->sesskey); + $this->assertObjectHasAttribute('userkey', $SESSION); + } } /** - * Test that wantsurl URL gets returned after user logged in if wantsurl's set. + * Test that a user gets redirected to internal wantsurl URL successful log in. + * + * @expectedException moodle_exception + * @expectedExceptionMessage Unsupported redirect to /course/index.php?id=12&key=134 detected, execution terminated. */ - public function test_that_return_wantsurl() { + public function test_that_user_gets_redirected_to_internal_wantsurl() { global $DB; $key = new stdClass(); @@ -709,15 +739,17 @@ class auth_plugin_userkey_testcase extends advanced_testcase { $_POST['wantsurl'] = '/course/index.php?id=12&key=134'; // Using @ is the only way to test this. Thanks moodle! - $redirect = @$this->auth->user_login_userkey(); - - $this->assertEquals('/course/index.php?id=12&key=134', $redirect); + @$this->auth->user_login_userkey(); } /** - * Test that wantsurl URL gets returned after user logged in if wantsurl's set to external URL. + * Test that a user gets redirected to external wantsurl URL successful log in. + * + * @expectedException moodle_exception + * @expectedExceptionMessage Unsupported redirect to http://test.com/course/index.php?id=12&key=134 detected, + * execution terminated. */ - public function test_that_return_wantsurl_if_it_is_external_url() { + public function test_that_user_gets_redirected_to_external_wantsurl() { global $DB; $key = new stdClass(); @@ -734,9 +766,7 @@ class auth_plugin_userkey_testcase extends advanced_testcase { $_POST['wantsurl'] = 'http://test.com/course/index.php?id=12&key=134'; // Using @ is the only way to test this. Thanks moodle! - $redirect = @$this->auth->user_login_userkey(); - - $this->assertEquals('http://test.com/course/index.php?id=12&key=134', $redirect); + @$this->auth->user_login_userkey(); } /**