diff --git a/admin/tool/mfa/classes/local/form/verification_field.php b/admin/tool/mfa/classes/local/form/verification_field.php index f92ee21972c..7f0b4144b84 100644 --- a/admin/tool/mfa/classes/local/form/verification_field.php +++ b/admin/tool/mfa/classes/local/form/verification_field.php @@ -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. diff --git a/admin/tool/mfa/classes/manager.php b/admin/tool/mfa/classes/manager.php index e32ed935ae9..3d1aabcd711 100644 --- a/admin/tool/mfa/classes/manager.php +++ b/admin/tool/mfa/classes/manager.php @@ -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; diff --git a/admin/tool/mfa/renderer.php b/admin/tool/mfa/classes/output/renderer.php similarity index 92% rename from admin/tool/mfa/renderer.php rename to admin/tool/mfa/classes/output/renderer.php index ea65a92dbd8..e32ad7069f7 100644 --- a/admin/tool/mfa/renderer.php +++ b/admin/tool/mfa/classes/output/renderer.php @@ -14,6 +14,13 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . +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 diff --git a/admin/tool/mfa/classes/plugininfo/factor.php b/admin/tool/mfa/classes/plugininfo/factor.php index bbc653762c1..0509ce8a1c3 100644 --- a/admin/tool/mfa/classes/plugininfo/factor.php +++ b/admin/tool/mfa/classes/plugininfo/factor.php @@ -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; + } } diff --git a/admin/tool/mfa/tests/plugininfo_factor_test.php b/admin/tool/mfa/tests/plugininfo_factor_test.php index 61e57345ef8..10e24332ed9 100644 --- a/admin/tool/mfa/tests/plugininfo_factor_test.php +++ b/admin/tool/mfa/tests/plugininfo_factor_test.php @@ -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); } }