Skip to content

Commit

Permalink
Added stronger redirect logic for login hook
Browse files Browse the repository at this point in the history
  • Loading branch information
@Peterburnett
Peterburnett committed Dec 9, 2019
1 parent 27cf683 commit 3469cd2
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 12 deletions.
109 changes: 108 additions & 1 deletion classes/manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@

class manager {

const REDIRECT = 1;
const NO_REDIRECT = 0;
const REDIRECT_EXCEPTION = -1;

/**
* Displays a debug table with current factor information.
*/
Expand Down Expand Up @@ -257,12 +261,115 @@ public static function set_pass_state() {
$event = \tool_mfa\event\user_passed_mfa::user_passed_mfa_event($USER);
$event->trigger();

// Unset session vars during mfa auth.
unset($SESSION->mfa_redir_referer);
unset($SESSION->mfa_redir_count);

// Fire post pass state factor actions.
$factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
foreach ($factors as $factor) {
$factor->post_pass_state();
}
}
}
}

/**
* Checks whether the user should be redirected from the provided url.
*
* @return int
*/
public static function should_require_mfa($url, $preventredirect) {
global $CFG, $USER, $SESSION;
// Remove all params before comparison.
$url->remove_all_params();

// Soft maintenance mode.
if (!empty($CFG->maintenance_enabled)) {
return self::NO_REDIRECT;
}

// Admin not setup.
if (!empty($CFG->adminsetuppending)) {
return self::NO_REDIRECT;
}

// Initial installation.
// We get this for free from get_plugins_with_function.

// Upgrade check.
// We get this for free from get_plugins_with_function.

// Honor prevent_redirect.
if ($preventredirect) {
return 0;
}

// User not properly setup.
if (user_not_fully_set_up($USER)) {
return self::NO_REDIRECT;
}

// Enrolment.
$enrol = new \moodle_url('/enrol/index.php');
if ($enrol->compare($url)) {
return self::NO_REDIRECT;
}

// Guest access.
if (isguestuser()) {
return self::NO_REDIRECT;
}

// Forced password changes.
if (get_user_preferences('auth_forcepasswordchange')) {
return self::NO_REDIRECT;
}

// Login as.
if (\core\session\manager::is_loggedinas()) {
return self::NO_REDIRECT;
}

// Site policy.
if (isset($USER->policyagreed) && !$USER->policyagreed
&& defined('NO_SITEPOLICY_CHECK') && !NO_SITEPOLICY_CHECK) {
$manager = new \core_privacy\local\sitepolicy\manager();
$policyurl = $manager->get_redirect_url(false);
if (!empty($policyurl)) {
return self::NO_REDIRECT;
}
}

// WS/AJAX check.
if (WS_SERVER || AJAX_SCRIPT) {
return self::REDIRECT_EXCEPTION;
}

// Circular checks.
if (isset($SESSION->mfa_redir_referer) &&
$SESSION->mfa_redir_referer != 'admin/tool/mfa/auth.php') {
if ($SESSION->mfa_redir_referer == get_local_referer(true)) {
// Possible redirect loop.
if (!isset($SESSION->mfa_redir_count)) {
$SESSION->mfa_redir_count = 1;
} else {
$SESSION->mfa_redir_count++;
}
if ($SESSION->mfa_redir_count > 5) {
return self::REDIRECT_EXCEPTION;
}
} else {
// If not a match, reset counter.
$SESSION->mfa_redir_count = 0;
}
}
// Set referer after checks.
$SESSION->mfa_redir_referer = get_local_referer(true);

$safe = new \moodle_url('/admin/tool/mfa/auth.php');
if ($safe->compare($url)) {
return self::NO_REDIRECT;
}
return self::REDIRECT;
}
}
25 changes: 15 additions & 10 deletions lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,28 @@
* @return void
* @throws \moodle_exception
*/
function tool_mfa_after_require_login() {
global $SESSION, $ME, $CFG;
function tool_mfa_after_require_login($courseorid = null, $autologinguest = null, $cm = null,
$setwantsurltome = null, $preventredirect = null) {
global $SESSION, $ME;

if (!tool_mfa_ready()) {
return;
}

if (empty($SESSION->tool_mfa_authenticated) || !$SESSION->tool_mfa_authenticated) {
if (empty($SESSION->wantsurl)) {
$SESSION->wantsurl = qualified_me();
$SESSION->tool_mfa_setwantsurl = true;
}

$clearurl = str_replace($CFG->wwwroot, '', $ME);

if (strpos($clearurl, '/admin/tool/mfa/auth.php') !== 0) {
$cleanurl = new moodle_url($ME);
$redir = \tool_mfa\manager::should_require_mfa($cleanurl, $preventredirect);
if ($redir == \tool_mfa\manager::REDIRECT) {
if (empty($SESSION->wantsurl)) {
!empty($setwantsurltome)
? $SESSION->wantsurl = $setwantsurltome
: $SESSION->wantsurl = qualified_me();

$SESSION->tool_mfa_setwantsurl = true;
}
redirect(new moodle_url('/admin/tool/mfa/auth.php'));
} else if ($redir == \tool_mfa\manager::REDIRECT_EXCEPTION) {
throw new moodle_exception('redirecterrordetected', 'error');
}
}
}
Expand Down
155 changes: 154 additions & 1 deletion tests/manager_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,158 @@ public function test_passed_enough_factors() {
$factoremail->set_state(\tool_mfa\plugininfo\factor::STATE_PASS);
$this->assertEquals(\tool_mfa\manager::passed_enough_factors(), true);
}
}

public static function should_redirect_urls_provider() {
$badurl1 = new \moodle_url('/');
$badparam1 = $badurl1->out();
$badurl2 = new \moodle_url('admin/tool/mfa/auth.php');
$badparam2 = $badurl2->out();
return [
['/', 'http://test.server', true],
['/admin/tool/mfa/action.php', 'http://test.server', true],
['/admin/tool/mfa/factor/totp/settings.php', 'http://test.server', true],
['/', 'http://test.server', true, array('url' => $badparam1)],
['/', 'http://test.server', true, array('url' => $badparam2)],
['/admin/tool/mfa/auth.php', 'http://test.server', false],
['/admin/tool/mfa/auth.php', 'http://test.server/parent/directory', false],
['/admin/tool/mfa/action.php', 'http://test.server/parent/directory', true],
['/', 'http://test.server/parent/directory', true, array('url' => $badparam1)],
['/', 'http://test.server/parent/directory', true, array('url' => $badparam2)],
];
}

/**
* @dataProvider should_redirect_urls_provider
*/
public function test_should_require_mfa_urls($urlstring, $webroot, $status, $params = null) {
$this->resetAfterTest(true);
global $CFG;
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);
$CFG->wwwroot = $webroot;
$url = new \moodle_url($urlstring, $params);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), $status);
}

public function test_should_require_mfa_checks() {
// Setup test and user.
global $CFG;
$this->resetAfterTest(true);
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

$badurl = new \moodle_url('/');

// Maintenance mode.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$CFG->maintenance_enabled = 1;
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
$CFG->maintenance_enabled = 0;

// Admin not setup.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$CFG->adminsetuppending = 1;
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
$CFG->adminsetuppending = 0;

// Initial installation.
$roles = $CFG->rolesactive;
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
unset($CFG->rolesactive);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
$CFG->rolesactive = $roles;

// Upgrade check.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$CFG->upgraderunning = 1;
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
$CFG->upgraderunning = 0;

// Check prevent_redirect.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, true), \tool_mfa\manager::NO_REDIRECT);

// User not setup properly.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$this->setUser(null);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
$this->setUser($user);

// Enrolment.
$enrolurl = new \moodle_url('/enrol/index.php');
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($enrolurl, false), \tool_mfa\manager::NO_REDIRECT);

// Guest User.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$this->setGuestUser();
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
$this->setUser($user);

// Forced password changes.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
set_user_preference('auth_forcepasswordchange', true);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
set_user_preference('auth_forcepasswordchange', false);

// Login as check.
$user2 = $this->getDataGenerator()->create_user();
$syscontext = \context_system::instance();
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::REDIRECT);
$this->setAdminUser();
\core\session\manager::loginas($user2->id, $syscontext, false);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($badurl, false), \tool_mfa\manager::NO_REDIRECT);
$this->setUser($user);
}

public function test_should_require_mfa_redirection_loop() {
// Setup test and user.
global $CFG, $SESSION;
$CFG->wwwroot = 'http://phpunit.test';
$this->resetAfterTest(true);
$user = $this->getDataGenerator()->create_user();
$this->setUser($user);

// Set first referer url.
$_SERVER['HTTP_REFERER'] = 'http://phpunit.test';
$url = new \moodle_url('/');

// Test you get three redirs then exception.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
// Set count to threshold
$SESSION->mfa_redir_count = 5;
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT_EXCEPTION);
// Reset session vars.
unset($SESSION->mfa_redir_referer);
unset($SESSION->mfa_redir_count);

// Test 4 different redir urls.
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$_SERVER['HTTP_REFERER'] = 'http://phpunit.test/2';
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$_SERVER['HTTP_REFERER'] = 'http://phpunit3.test/3';
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$_SERVER['HTTP_REFERER'] = 'http://phpunit4.test/4';
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
// Reset session vars.
unset($SESSION->mfa_redir_referer);
unset($SESSION->mfa_redir_count);

// Test 6 then jump to new referer (5 + 1 to set the first time).
$_SERVER['HTTP_REFERER'] = 'http://phpunit.test';
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);

$_SERVER['HTTP_REFERER'] = 'http://phpunit.test/2';
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
// Now test that going back to original URL doesnt cause exception.
$_SERVER['HTTP_REFERER'] = 'http://phpunit.test';
$this->assertEquals(\tool_mfa\manager::should_require_mfa($url, false), \tool_mfa\manager::REDIRECT);
}
}

0 comments on commit 3469cd2

Please sign in to comment.