moodle/moodle
3
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-08-11 20:06:46 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

MDL-78534 Authentication: MFA refactoring and clean up

Browse source 
General refactoring and cleanup of MFA code, to allow for easier
addition of UX improvements.
This commit is contained in:
Matt Porritt 2023-08-16 13:39:47 +10:00
parent f140a0dff9
commit 27ca1dee46
5 changed files with 124 additions and 88 deletions
Download patch file Download diff file Expand all files Collapse all files

9
admin/tool/mfa/classes/local/form/verification_field.php
View file

@ -51,7 +51,8 @@ class verification_field extends \MoodleQuickForm_text {
$attributes['autocomplete'] = 'one-time-code';
$attributes['inputmode'] = 'numeric';
$attributes['pattern'] = '[0-9]*';
$attributes['class'] = 'tool-mfa-verification-code';
$attributes['class'] = 'tool-mfa-verification-code font-weight-bold';
$attributes['maxlength'] = 6;
// If we aren't on the auth page, this might be part of a larger form such as for setup.
// We shouldn't autofocus here, as it probably isn't the only element, or main target.
@ -62,11 +63,7 @@ class verification_field extends \MoodleQuickForm_text {
// If we are on the auth page, load JS for element.
$this->appendjs = false;
if ($auth) {
if ($PAGE->pagelayout === 'secure') {
$this->appendjs = true;
} else {
$PAGE->requires->js_call_amd('tool_mfa/autosubmit_verification_code', 'init', []);
}
$PAGE->requires->js_call_amd('tool_mfa/autosubmit_verification_code', 'init', []);
}
// Force element name to match JS.

76
admin/tool/mfa/classes/manager.php
View file

@ -17,6 +17,7 @@
namespace tool_mfa;
use dml_exception;
use tool_mfa\plugininfo\factor;
/**
* MFA management class.
@ -69,8 +70,8 @@ class manager {
'text-right',
'text-center',
];
$factors = \tool_mfa\plugininfo\factor::get_enabled_factors();
$userfactors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
$factors = factor::get_enabled_factors();
$userfactors = factor::get_active_user_factor_types();
$runningtotal = 0;
$weighttoggle = false;
@ -79,13 +80,13 @@ class manager {
$name = get_string('pluginname', $namespace);
// If factor is unknown, pending from here.
if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_UNKNOWN) {
if ($factor->get_state() == factor::STATE_UNKNOWN) {
$weighttoggle = true;
}
// Stop adding weight if 100 achieved.
if (!$weighttoggle) {
$achieved = $factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS ? $factor->get_weight() : 0;
$achieved = $factor->get_state() == factor::STATE_PASS ? $factor->get_weight() : 0;
$achieved = '+'.$achieved;
$runningtotal += $achieved;
} else {
@ -148,10 +149,10 @@ class manager {
*/
public static function get_total_weight(): int {
$totalweight = 0;
$factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
$factors = factor::get_active_user_factor_types();
foreach ($factors as $factor) {
if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS) {
if ($factor->get_state() == factor::STATE_PASS) {
$totalweight += $factor->get_weight();
}
}
@ -227,12 +228,12 @@ class manager {
global $SESSION;
// Check for any instant fail states.
$factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
$factors = factor::get_active_user_factor_types();
foreach ($factors as $factor) {
$factor->load_locked_state();
if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_FAIL) {
return \tool_mfa\plugininfo\factor::STATE_FAIL;
if ($factor->get_state() == factor::STATE_FAIL) {
return factor::STATE_FAIL;
}
}
@ -240,25 +241,24 @@ class manager {
self::passed_enough_factors());
// Check next factor for instant fail (fallback).
if (\tool_mfa\plugininfo\factor::get_next_user_factor()->get_state() ==
\tool_mfa\plugininfo\factor::STATE_FAIL) {
if (factor::get_next_user_login_factor()->get_state() == factor::STATE_FAIL) {
// We need to handle a special case here, where someone reached the fallback,
// If they were able to modify their state on the error page, such as passing iprange,
// We must return pass.
if ($passcondition) {
return \tool_mfa\plugininfo\factor::STATE_PASS;
return factor::STATE_PASS;
}
return \tool_mfa\plugininfo\factor::STATE_FAIL;
return factor::STATE_FAIL;
}
// Now check for general passing state. If found, ensure that session var is set.
if ($passcondition) {
return \tool_mfa\plugininfo\factor::STATE_PASS;
return factor::STATE_PASS;
}
// Else return neutral state.
return \tool_mfa\plugininfo\factor::STATE_NEUTRAL;
return factor::STATE_NEUTRAL;
}
/**
@ -272,7 +272,7 @@ class manager {
global $SESSION;
$state = self::get_status();
if ($state == \tool_mfa\plugininfo\factor::STATE_PASS) {
if ($state == factor::STATE_PASS) {
self::set_pass_state();
// Check if user even had to reach auth page.
if (isset($SESSION->tool_mfa_has_been_redirected)) {
@ -287,7 +287,7 @@ class manager {
// Don't touch anything, let user be on their way.
return;
}
} else if ($state == \tool_mfa\plugininfo\factor::STATE_FAIL) {
} else if ($state == factor::STATE_FAIL) {
self::cannot_login();
} else if ($shouldreload) {
// Set a session variable to track whether user is where they want to be.
@ -305,9 +305,9 @@ class manager {
public static function passed_enough_factors(): bool {
// Check for any instant fail states.
$factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
$factors = factor::get_active_user_factor_types();
foreach ($factors as $factor) {
if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_FAIL) {
if ($factor->get_state() == factor::STATE_FAIL) {
self::mfa_logout();
}
}
@ -353,17 +353,17 @@ class manager {
// @codingStandardsIgnoreEnd
// Fire post pass state factor actions.
$factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
$factors = factor::get_active_user_factor_types();
foreach ($factors as $factor) {
$factor->post_pass_state();
// Also set the states for this session to neutral if they were locked.
if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_LOCKED) {
$factor->set_state(\tool_mfa\plugininfo\factor::STATE_NEUTRAL);
if ($factor->get_state() == factor::STATE_LOCKED) {
$factor->set_state(factor::STATE_NEUTRAL);
}
}
// Output notifications if any factors were reset for this user.
$enabledfactors = \tool_mfa\plugininfo\factor::get_enabled_factors();
$enabledfactors = factor::get_enabled_factors();
foreach ($enabledfactors as $factor) {
$pref = 'tool_mfa_reset_' . $factor->name;
$factorpref = get_user_preferences($pref, false);
@ -377,7 +377,7 @@ class manager {
}
// Also check for a global reset.
// TODO: Delete this in a ferw months, the reset all preference is no longer set.
// TODO: Delete this in a few months, the reset all preference is no longer set.
$allfactor = get_user_preferences('tool_mfa_reset_all', false);
if ($allfactor) {
$url = new \moodle_url('/admin/tool/mfa/user_preferences.php');
@ -563,7 +563,7 @@ class manager {
* @return array
*/
public static function get_no_redirect_urls(): array {
$factors = \tool_mfa\plugininfo\factor::get_factors();
$factors = factor::get_factors();
$urls = [
new \moodle_url('/login/logout.php'),
new \moodle_url('/admin/tool/mfa/guide.php'),
@ -723,7 +723,7 @@ class manager {
return false;
}
$enabledfactors = \tool_mfa\plugininfo\factor::get_enabled_factors();
$enabledfactors = factor::get_enabled_factors();
if (count($enabledfactors) == 0) {
return false;
}
@ -791,13 +791,13 @@ class manager {
global $USER;
// Get all active factors.
$factors = \tool_mfa\plugininfo\factor::get_enabled_factors();
$factors = factor::get_enabled_factors();
// Check if there are enough factors that a user can ONLY pass, if so, don't display the menu.
$weight = 0;
foreach ($factors as $factor) {
$states = $factor->possible_states($USER);
if (count($states) == 1 && reset($states) == \tool_mfa\plugininfo\factor::STATE_PASS) {
if (count($states) == 1 && reset($states) == factor::STATE_PASS) {
$weight += $factor->get_weight();
if ($weight >= 100) {
return false;
@ -808,7 +808,7 @@ class manager {
// Now if there is a factor that can be setup, that may return a pass state for the user, display menu.
foreach ($factors as $factor) {
if ($factor->has_setup()) {
if (in_array(\tool_mfa\plugininfo\factor::STATE_PASS, $factor->possible_states($USER))) {
if (in_array(factor::STATE_PASS, $factor->possible_states($USER))) {
return true;
}
}
@ -820,19 +820,21 @@ class manager {
/**
* Gets current user weight, up until first unknown factor.
*
* @return int
* @return int $totalweight Total weight of all factors.
*/
public static function get_cumulative_weight(): int {
$factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
$factors = factor::get_active_user_factor_types();
// Factor order is important here, so sort the factors by state.
$sortedfactors = factor::sort_factors_by_state($factors, factor::STATE_PASS);
$totalweight = 0;
foreach ($factors as $factor) {
if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS) {
foreach ($sortedfactors as $factor) {
if ($factor->get_state() == factor::STATE_PASS) {
$totalweight += $factor->get_weight();
// If over 100, break. Dont care about >100.
// If over 100, break. Don't care about >100.
if ($totalweight >= 100) {
break;
}
} else if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_UNKNOWN) {
} else if ($factor->get_state() == factor::STATE_UNKNOWN) {
break;
}
}
@ -846,7 +848,7 @@ class manager {
* @return bool true if factor is pending.
*/
public static function check_factor_pending(string $factorname): bool {
$factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
$factors = factor::get_active_user_factor_types();
// Setup vars.
$pending = [];
$totalweight = 0;
@ -859,7 +861,7 @@ class manager {
continue;
}
if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS) {
if ($factor->get_state() == factor::STATE_PASS) {
$totalweight += $factor->get_weight();
if ($totalweight >= 100) {
$weighttoggle = true;

47
admin/tool/mfa/renderer.php → admin/tool/mfa/classes/output/renderer.php
View file

@ -14,6 +14,13 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
namespace tool_mfa\output;
use tool_mfa\local\factor\object_factor;
use tool_mfa\local\form\login_form;
use \html_writer;
use tool_mfa\plugininfo\factor;
/**
* MFA renderer.
*
@ -22,10 +29,10 @@
* @copyright Catalyst IT
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class tool_mfa_renderer extends plugin_renderer_base {
class renderer extends \plugin_renderer_base {
/**
* Returns the state of the factor as a badge
* Returns the state of the factor as a badge.
*
* @param string $state
* @return string
@ -33,36 +40,36 @@ class tool_mfa_renderer extends plugin_renderer_base {
public function get_state_badge(string $state): string {
switch ($state) {
case \tool_mfa\plugininfo\factor::STATE_PASS:
return \html_writer::tag('span', get_string('state:pass', 'tool_mfa'), ['class' => 'badge badge-success']);
case factor::STATE_PASS:
return html_writer::tag('span', get_string('state:pass', 'tool_mfa'), ['class' => 'badge badge-success']);
case \tool_mfa\plugininfo\factor::STATE_FAIL:
return \html_writer::tag('span', get_string('state:fail', 'tool_mfa'), ['class' => 'badge badge-danger']);
case factor::STATE_FAIL:
return html_writer::tag('span', get_string('state:fail', 'tool_mfa'), ['class' => 'badge badge-danger']);
case \tool_mfa\plugininfo\factor::STATE_NEUTRAL:
return \html_writer::tag('span', get_string('state:neutral', 'tool_mfa'), ['class' => 'badge badge-warning']);
case factor::STATE_NEUTRAL:
return html_writer::tag('span', get_string('state:neutral', 'tool_mfa'), ['class' => 'badge badge-warning']);
case \tool_mfa\plugininfo\factor::STATE_UNKNOWN:
return \html_writer::tag('span', get_string('state:unknown', 'tool_mfa'),
case factor::STATE_UNKNOWN:
return html_writer::tag('span', get_string('state:unknown', 'tool_mfa'),
['class' => 'badge badge-secondary']);
case \tool_mfa\plugininfo\factor::STATE_LOCKED:
return \html_writer::tag('span', get_string('state:locked', 'tool_mfa'), ['class' => 'badge badge-error']);
case factor::STATE_LOCKED:
return html_writer::tag('span', get_string('state:locked', 'tool_mfa'), ['class' => 'badge badge-error']);
default:
return \html_writer::tag('span', get_string('pending', 'tool_mfa'), ['class' => 'badge badge-secondary']);
return html_writer::tag('span', get_string('pending', 'tool_mfa'), ['class' => 'badge badge-secondary']);
}
}
/**
* Returns a list of factors which a user can add
* Returns a list of factors which a user can add.
*
* @return string
*/
public function available_factors(): string {
$html = $this->output->heading(get_string('preferences:availablefactors', 'tool_mfa'), 2);
$factors = \tool_mfa\plugininfo\factor::get_enabled_factors();
$factors = factor::get_enabled_factors();
foreach ($factors as $factor) {
// TODO is_configured / is_ready.
if (!$factor->has_setup() || !$factor->show_setup_buttons()) {
@ -144,7 +151,7 @@ class tool_mfa_renderer extends plugin_renderer_base {
];
$table->data = [];
$factors = \tool_mfa\plugininfo\factor::get_enabled_factors();
$factors = factor::get_enabled_factors();
foreach ($factors as $factor) {
$userfactors = $factor->get_active_user_factors($USER);
@ -231,7 +238,7 @@ class tool_mfa_renderer extends plugin_renderer_base {
// Logout button.
$url = new \moodle_url('/admin/tool/mfa/auth.php', ['logout' => 1]);
$btn = new \single_button($url, get_string('logout'), 'post', \single_button::BUTTON_PRIMARY);
$btn = new \single_button($url, get_string('logout'), 'post', true);
$return .= $this->render($btn);
$return .= $this->guide_link();
@ -248,7 +255,7 @@ class tool_mfa_renderer extends plugin_renderer_base {
public function factors_in_use_table(int $lookback): string {
global $DB;
$factors = \tool_mfa\plugininfo\factor::get_factors();
$factors = factor::get_factors();
// Setup 2 arrays, one with internal names, one pretty.
$columns = [''];
@ -383,7 +390,7 @@ class tool_mfa_renderer extends plugin_renderer_base {
public function factors_locked_table(): string {
global $DB;
$factors = \tool_mfa\plugininfo\factor::get_factors();
$factors = factor::get_factors();
$table = new \html_table();
@ -525,7 +532,7 @@ class tool_mfa_renderer extends plugin_renderer_base {
*
* In certain situations, includes a script element which adds autosubmission behaviour.
*
* @param HTML_QuickForm_element $element element
* @param \HTML_QuickForm_element $element element
* @param bool $required if input is required field
* @param bool $advanced if input is an advanced field
* @param string|null $error error message to display

72
admin/tool/mfa/classes/plugininfo/factor.php
View file

@ -127,32 +127,18 @@ class factor extends \core\plugininfo\base {
}
/**
* Finds active factors for current user.
* Finds active factors for a user.
* If user is not specified, current user is used.
*
* @param mixed $user user object or null.
* @return array of factor objects.
*/
public static function get_active_user_factor_types(): array {
public static function get_active_user_factor_types(mixed $user = null): array {
global $USER;
$return = [];
$factors = self::get_enabled_factors();
foreach ($factors as $factor) {
$userfactors = $factor->get_active_user_factors($USER);
if (count($userfactors) > 0) {
$return[] = $factor;
}
if (is_null($user)) {
$user = $USER;
}
return $return;
}
/**
* Finds active factors for given user.
*
* @param stdClass $user the user to get types for.
* @return array of factor objects.
*/
public static function get_active_other_user_factor_types(stdClass $user): array {
$return = [];
$factors = self::get_enabled_factors();
@ -168,10 +154,11 @@ class factor extends \core\plugininfo\base {
/**
* Returns next factor to authenticate user.
* Only returns factors that require user input.
*
* @return mixed factor object the next factor to be authenticated or false.
*/
public static function get_next_user_factor(): object {
public static function get_next_user_login_factor(): mixed {
$factors = self::get_active_user_factor_types();
foreach ($factors as $factor) {
@ -187,6 +174,23 @@ class factor extends \core\plugininfo\base {
return new \tool_mfa\local\factor\fallback();
}
/**
* Returns all factors that require user input.
*
* @return array of factor objects.
*/
public static function get_all_user_login_factors(): array {
$factors = self::get_active_user_factor_types();
$loginfactors = [];
foreach ($factors as $factor) {
if ($factor->has_input()) {
$loginfactors[] = $factor;
}
}
return $loginfactors;
}
/**
* Returns the list of available actions with factor.
*
@ -336,4 +340,30 @@ class factor extends \core\plugininfo\base {
parent::uninstall_cleanup();
}
/**
* Sorts factors by state.
*
* @param array $factors The factors to sort.
* @param string $state The state to sort by.
* @return array $factors The sorted factors.
*/
public static function sort_factors_by_state(array $factors, string $state): array {
usort($factors, function ($a, $b) use ($state) {
$statea = $a->get_state();
$stateb = $b->get_state();
if ($statea === $state && $stateb !== $state) {
return -1; // A comes before B.
}
if ($stateb === $state && $statea !== $state) {
return 1; // B comes before A.
}
return 0; // They are the same, keep current order.
});
return $factors;
}
}

8
admin/tool/mfa/tests/plugininfo_factor_test.php
View file

@ -45,7 +45,7 @@ class plugininfo_factor_test extends \advanced_testcase {
$this->setUser($user);
// Test that with no enabled factors, fallback is returned.
$this->assertEquals('fallback', \tool_mfa\plugininfo\factor::get_next_user_factor()->name);
$this->assertEquals('fallback', \tool_mfa\plugininfo\factor::get_next_user_login_factor()->name);
// Setup enabled totp factor for user.
set_config('enabled', 1, 'factor_totp');
@ -57,11 +57,11 @@ class plugininfo_factor_test extends \advanced_testcase {
$this->assertNotEmpty($totpfactor->setup_user_factor((object) $totpdata));
// Test that factor now appears (from STATE_UNKNOWN).
$this->assertEquals('totp', \tool_mfa\plugininfo\factor::get_next_user_factor()->name);
$this->assertEquals('totp', \tool_mfa\plugininfo\factor::get_next_user_login_factor()->name);
// Now pass this factor, check for fallback.
$totpfactor->set_state(\tool_mfa\plugininfo\factor::STATE_PASS);
$this->assertEquals('fallback', \tool_mfa\plugininfo\factor::get_next_user_factor()->name);
$this->assertEquals('fallback', \tool_mfa\plugininfo\factor::get_next_user_login_factor()->name);
// Add in a no-input factor.
set_config('enabled', 1, 'factor_auth');
@ -73,6 +73,6 @@ class plugininfo_factor_test extends \advanced_testcase {
// Check that the next factor is still the fallback factor.
$this->assertEquals(2, count(\tool_mfa\plugininfo\factor::get_active_user_factor_types()));
$this->assertEquals('fallback', \tool_mfa\plugininfo\factor::get_next_user_factor()->name);
$this->assertEquals('fallback', \tool_mfa\plugininfo\factor::get_next_user_login_factor()->name);
}
}