From 1656e8ad4045a6f51899a7d8cc42d6edece73ddd Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Mon, 27 Nov 2023 16:46:55 +1100 Subject: [PATCH] feat: add a way to prevent gracemode redirects if user has passed certain factors --- classes/local/factor/object_factor_base.php | 9 ++++++ classes/manager.php | 8 ++--- factor/grace/classes/factor.php | 35 ++++++++++++++++++++- factor/grace/lang/en/factor_grace.php | 2 ++ factor/grace/settings.php | 9 ++++++ 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/classes/local/factor/object_factor_base.php b/classes/local/factor/object_factor_base.php index 6152ca21..b9296dc4 100644 --- a/classes/local/factor/object_factor_base.php +++ b/classes/local/factor/object_factor_base.php @@ -635,4 +635,13 @@ public function global_validation($data, $files): array { public function global_submit($data) { return; } + + /** + * Returns whether or not the factor has passed + * + * @return bool + */ + public function passed(): bool { + return $this->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS; + } } diff --git a/classes/manager.php b/classes/manager.php index b61d6afd..937cf32f 100644 --- a/classes/manager.php +++ b/classes/manager.php @@ -81,7 +81,7 @@ public static function display_debug_notification() { // Stop adding weight if 100 achieved. if (!$weighttoggle) { - $achieved = $factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS ? $factor->get_weight() : 0; + $achieved = $factor->passed() ? $factor->get_weight() : 0; $achieved = '+'.$achieved; $runningtotal += $achieved; } else { @@ -147,7 +147,7 @@ public static function get_total_weight() { $factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types(); foreach ($factors as $factor) { - if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS) { + if ($factor->passed()) { $totalweight += $factor->get_weight(); } } @@ -818,7 +818,7 @@ public static function get_cumulative_weight() { $factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types(); $totalweight = 0; foreach ($factors as $factor) { - if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS) { + if ($factor->passed()) { $totalweight += $factor->get_weight(); // If over 100, break. Dont care about >100. if ($totalweight >= 100) { @@ -851,7 +851,7 @@ public static function check_factor_pending($factorname) { continue; } - if ($factor->get_state() == \tool_mfa\plugininfo\factor::STATE_PASS) { + if ($factor->passed()) { $totalweight += $factor->get_weight(); if ($totalweight >= 100) { $weighttoggle = true; diff --git a/factor/grace/classes/factor.php b/factor/grace/classes/factor.php index 97724e24..48ab128c 100644 --- a/factor/grace/classes/factor.php +++ b/factor/grace/classes/factor.php @@ -113,7 +113,7 @@ public function get_state($redirectable = true) { // If gracemode would have given points, but now doesnt, // Jump out of the loop and force a factor setup. // We will return once there is a setup, or the user tries to leave. - if (get_config('factor_grace', 'forcesetup') && $redirectable) { + if ($redirectable && get_config('factor_grace', 'forcesetup')) { if (empty($SESSION->mfa_gracemode_recursive)) { // Set a gracemode lock so any further recursive gets fall past any recursive calls. $SESSION->mfa_gracemode_recursive = true; @@ -124,9 +124,15 @@ public function get_state($redirectable = true) { foreach ($factorurls as $factorurl) { if ($factorurl->compare($cleanurl)) { $redirectable = false; + break; } } + // Checking if there are contributing factors, that should not cause a redirect. + if ($redirectable && $this->should_skip_force_setup()) { + $redirectable = false; + } + // We should never redirect if we have already passed. if ($redirectable && \tool_mfa\manager::get_cumulative_weight() >= 100) { $redirectable = false; @@ -302,4 +308,31 @@ public function get_affecting_factors(): array { }); return $factors; } + + /** + * Returns whether or not the force setup of MFA is skippable + * + * @return bool + */ + private function should_skip_force_setup(): bool { + // Checking if there are contributing factors, that should not cause a redirect. + $active = \tool_mfa\plugininfo\factor::get_active_user_factor_types(); + $noredirectlist = get_config('factor_grace', 'noredirectlist'); + if (!$noredirectlist) { + return false; + } + + $values = explode(',', $noredirectlist); + $keys = array_flip($values); + + foreach ($active as $factor) { + if (isset($keys[$factor->name]) + && $factor->passed() + && $factor->get_weight() > 0) { + return true; + } + } + + return false; + } } diff --git a/factor/grace/lang/en/factor_grace.php b/factor/grace/lang/en/factor_grace.php index c2f2f7d0..568995b9 100644 --- a/factor/grace/lang/en/factor_grace.php +++ b/factor/grace/lang/en/factor_grace.php @@ -33,6 +33,8 @@ $string['settings:customwarning_help'] = 'Add content here to replace the grace warning notification with custom HTML contents. Adding {timeremaining} in text will replace it with the current grace duration for the user, and {setuplink} will replace with the URL of the setup page for the user.'; $string['settings:forcesetup'] = 'Force factor setup'; $string['settings:forcesetup_help'] = 'Forces a user to the preferences page to setup MFA when the gracemode period expires. If set to off, users will be unable to authenticate when the grace period expires.'; +$string['settings:noredirectlist'] = 'Force factor skip list'; +$string['settings:noredirectlist_help'] = 'In cases where certain users DO NOT need to set up MFA. If any selected factors contribute points, grace mode will NOT force factor setup.'; $string['settings:graceperiod'] = 'Grace period'; $string['settings:graceperiod_help'] = 'Period of time when users can access Moodle without configured and enabled factors'; $string['settings:ignorelist'] = 'Ignored factors'; diff --git a/factor/grace/settings.php b/factor/grace/settings.php index eb1448bd..6e41c1a8 100644 --- a/factor/grace/settings.php +++ b/factor/grace/settings.php @@ -41,6 +41,15 @@ new lang_string('settings:forcesetup', 'factor_grace'), new lang_string('settings:forcesetup_help', 'factor_grace'), 0)); +$allfactors = \tool_mfa\plugininfo\factor::get_factors(); +$noredirectlistfactors = []; +foreach ($allfactors as $factor) { + $noredirectlistfactors[$factor->name] = $factor->get_display_name(); +} +$settings->add(new admin_setting_configmultiselect('factor_grace/noredirectlist', + new lang_string('settings:noredirectlist', 'factor_grace'), + new lang_string('settings:noredirectlist_help', 'factor_grace'), [], $noredirectlistfactors)); + $settings->add(new admin_setting_configduration('factor_grace/graceperiod', new lang_string('settings:graceperiod', 'factor_grace'), new lang_string('settings:graceperiod_help', 'factor_grace'), '604800'));