From 463668d4d92ab90ef5affb03474bc7b26c7f794c Mon Sep 17 00:00:00 2001 From: raortegar Date: Tue, 11 Mar 2025 14:16:50 +0100 Subject: [PATCH] MDL-83692 factor_sms: MFA set default values when gateway list is empty --- admin/tool/mfa/factor/sms/classes/factor.php | 5 +- admin/tool/mfa/factor/sms/settings.php | 93 +++++++++---------- ..._mfa_setup_and_manage_user_factors.feature | 9 ++ 3 files changed, 59 insertions(+), 48 deletions(-) diff --git a/admin/tool/mfa/factor/sms/classes/factor.php b/admin/tool/mfa/factor/sms/classes/factor.php index e51603b99a2..4526471a948 100644 --- a/admin/tool/mfa/factor/sms/classes/factor.php +++ b/admin/tool/mfa/factor/sms/classes/factor.php @@ -351,7 +351,10 @@ class factor extends object_factor_base { * @return bool */ public function show_setup_buttons(): bool { - return true; + if (get_config('factor_sms', 'smsgateway') > 0) { + return true; + } + return false; } /** diff --git a/admin/tool/mfa/factor/sms/settings.php b/admin/tool/mfa/factor/sms/settings.php index b0e28f77472..001b4e40d9b 100644 --- a/admin/tool/mfa/factor/sms/settings.php +++ b/admin/tool/mfa/factor/sms/settings.php @@ -52,59 +52,14 @@ if ($ADMIN->fulltree) { ); $settings->add(new admin_setting_heading('factor_sms/settings', new lang_string('settings', 'moodle'), '')); + // Get available gateways, or link to gateway creation. + $gateways = [0 => new lang_string('none')]; if (count($gatewayrecords) > 0) { - $gateways = [0 => new lang_string('none')]; foreach ($gatewayrecords as $record) { $values = explode('\\', $record->gateway); $gatewayname = new lang_string('pluginname', $values[0]); $gateways[$record->id] = $record->name . ' (' . $gatewayname . ')'; } - - $settings->add( - new admin_setting_configselect( - 'factor_sms/smsgateway', - new lang_string('settings:smsgateway', 'factor_sms'), - new lang_string('settings:smsgateway_help', 'factor_sms', $smsconfigureurl), - 0, - $gateways, - ), - ); - - $enabled = new admin_setting_configcheckbox( - 'factor_sms/enabled', - new lang_string('settings:enablefactor', 'tool_mfa'), - new lang_string('settings:enablefactor_help', 'tool_mfa'), - 0, - ); - $enabled->set_updatedcallback(function () { - \tool_mfa\manager::do_factor_action( - 'sms', - get_config('factor_sms', 'enabled') ? 'enable' : 'disable', - ); - }); - $settings->add($enabled); - - $settings->add( - new admin_setting_configtext( - 'factor_sms/weight', - new lang_string('settings:weight', 'tool_mfa'), - new lang_string('settings:weight_help', 'tool_mfa'), - 100, - PARAM_INT, - ), - ); - $settings->hide_if('factor_sms/weight', 'factor_sms/enabled'); - - $settings->add( - new admin_setting_configduration( - 'factor_sms/duration', - new lang_string('settings:duration', 'tool_mfa'), - new lang_string('settings:duration_help', 'tool_mfa'), - 30 * MINSECS, - MINSECS, - ), - ); - $settings->hide_if('factor_sms/duration', 'factor_sms/enabled'); } else { $notify = new \core\output\notification( get_string('settings:setupdesc', 'factor_sms', $smsconfigureurl), @@ -112,4 +67,48 @@ if ($ADMIN->fulltree) { ); $settings->add(new admin_setting_heading('factor_sms/setupdesc', '', $OUTPUT->render($notify))); } + + $settings->add( + new admin_setting_configselect( + 'factor_sms/smsgateway', + new lang_string('settings:smsgateway', 'factor_sms'), + new lang_string('settings:smsgateway_help', 'factor_sms', $smsconfigureurl), + 0, + $gateways, + ), + ); + + $enabled = new admin_setting_configcheckbox( + 'factor_sms/enabled', + new lang_string('settings:enablefactor', 'tool_mfa'), + new lang_string('settings:enablefactor_help', 'tool_mfa'), + 0, + ); + $enabled->set_updatedcallback(function () { + \tool_mfa\manager::do_factor_action( + 'sms', + get_config('factor_sms', 'enabled') ? 'enable' : 'disable', + ); + }); + $settings->add($enabled); + + $settings->add( + new admin_setting_configtext( + 'factor_sms/weight', + new lang_string('settings:weight', 'tool_mfa'), + new lang_string('settings:weight_help', 'tool_mfa'), + 100, + PARAM_INT, + ), + ); + + $settings->add( + new admin_setting_configduration( + 'factor_sms/duration', + new lang_string('settings:duration', 'tool_mfa'), + new lang_string('settings:duration_help', 'tool_mfa'), + 30 * MINSECS, + MINSECS, + ), + ); } diff --git a/admin/tool/mfa/tests/behat/tool_mfa_setup_and_manage_user_factors.feature b/admin/tool/mfa/tests/behat/tool_mfa_setup_and_manage_user_factors.feature index ecc7d81baeb..0349bbce621 100644 --- a/admin/tool/mfa/tests/behat/tool_mfa_setup_and_manage_user_factors.feature +++ b/admin/tool/mfa/tests/behat/tool_mfa_setup_and_manage_user_factors.feature @@ -37,8 +37,17 @@ Feature: Set up and manage user factors Scenario: I can revoke a factor only when there is more than one active factor Given the following config values are set as admin: | enabled | 1 | factor_webauthn | + And I navigate to "Plugins > SMS > Manage SMS gateways" in site administration + And I follow "Create new SMS gateway" + And I set the following fields to these values: + | SMS gateway provider | AWS | + | Gateway name | Dummy gateway | + | Access key | key123 | + | Secret access key | secret456 | + And I press "Save changes" And the following config values are set as admin: | enabled | 1 | factor_sms | + | smsgateway | Dummy gateway (AWS) | factor_sms | And the following config values are set as admin: | enabled | 0 | factor_email | And the following "tool_mfa > User factors" exist: