Skip to content

Commit

Permalink
MDL-78509 tool_mfa: Fix from upstream and internal
Browse files Browse the repository at this point in the history
1. Add missing authenticator name lang string
2. Check nosetup factor correctly to display menu
3. Align email page controls with manager
4. Add explanation text to login page
5. Fixed "Access to an undefined property .." from PHPStan
6. Fixed all the "Variable $.. might not be defined" from PHPStan
7. Fixed the issue from catalyst/moodle-tool_mfa#379

Co-authored-by: Peter Burnett <[email protected]>
Co-authored-by: Alex Morris <[email protected]>
  • Loading branch information
3 people authored and stevandoMoodle committed Jul 31, 2023
1 parent 8f9ad54 commit d42593d
Show file tree
Hide file tree
Showing 40 changed files with 108 additions and 372 deletions.
15 changes: 0 additions & 15 deletions admin/tool/mfa/.github/workflows/ci.yml

This file was deleted.

10 changes: 0 additions & 10 deletions admin/tool/mfa/README.md

This file was deleted.

4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/admin/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2019102400; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_admin';
$plugin->release = 'v0.1';
$plugin->maturity = MATURITY_STABLE;
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/auth/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2021020500; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_auth';
$plugin->release = 2021020500;
$plugin->maturity = MATURITY_STABLE;
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/capability/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2020071400; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_capability';
$plugin->release = 'v0.1';
$plugin->maturity = MATURITY_STABLE;
Expand Down
6 changes: 2 additions & 4 deletions admin/tool/mfa/factor/cohort/tests/factor_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace factor_cohort\tests;

defined('MOODLE_INTERNAL') || die();
namespace factor_cohort;

/**
* Tests for cohort factor.
Expand All @@ -43,7 +41,7 @@ public function test_get_summary_condition() {

// Create a cohort.
$cohortid = $DB->insert_record('cohort', [
'idnumber' => NULL,
'idnumber' => null,
'name' => 'test',
'contextid' => \context_system::instance()->id,
'description' => '',
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/cohort/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2022101100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_cohort';
$plugin->release = 'v0.2';
$plugin->maturity = MATURITY_STABLE;
Expand Down
12 changes: 10 additions & 2 deletions admin/tool/mfa/factor/email/email.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,21 @@
$PAGE->set_title(get_string('unauthemail', 'factor_email'));
$PAGE->set_cacheable(false);
$instance = $DB->get_record('tool_mfa', ['id' => $instanceid]);
$factor = \tool_mfa\plugininfo\factor::get_factor('email');

// If pass is set, require login to force $SESSION and user, and pass for that session.
if (!empty($instance) && $pass != 0 && $secret != 0) {
require_login();
if ($factor->get_state() === \tool_mfa\plugininfo\factor::STATE_LOCKED) {
// Redirect through to auth, this will bounce them to the next factor.
redirect(new moodle_url('/admin/tool/mfa/auth.php'));
}
// Check the code with the same measures on the page entry.
if ($instance->secret != $secret) {
\tool_mfa\manager::sleep_timer();
$factor->increment_lock_counter();
throw new moodle_exception('error:parameters', 'factor_email');
}
require_login();
$factor = \tool_mfa\plugininfo\factor::get_factor('email');
$factor->set_state(\tool_mfa\plugininfo\factor::STATE_PASS);
// If wantsurl is already set in session, go to it.
Expand Down Expand Up @@ -80,7 +88,7 @@

// Suspend user account.
if (get_config('factor_email', 'suspend')) {
$DB->set_field('user', 'suspended', 1, ['id' => $userid]);
$DB->set_field('user', 'suspended', 1, ['id' => $user->id]);
}

$message = get_string('email:revokesuccess', 'factor_email', fullname($user));
Expand Down
4 changes: 1 addition & 3 deletions admin/tool/mfa/factor/email/tests/factor_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace factor_email\tests;

defined('MOODLE_INTERNAL') || die();
namespace factor_email;

/**
* Tests for email factor.
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/email/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2019102400; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_email';
$plugin->release = 'v0.1';
$plugin->maturity = MATURITY_STABLE;
Expand Down
8 changes: 7 additions & 1 deletion admin/tool/mfa/factor/grace/tests/factor_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace factor_grace\tests;
namespace factor_grace;

/**
* Tests for grace factor.
Expand All @@ -26,6 +26,12 @@
*/
class factor_test extends \advanced_testcase {

/**
* Test affecting factors
*
* @covers ::get_affecting_factors
* @return void
*/
public function test_affecting_factors() {
$this->resetAfterTest(true);
$user = $this->getDataGenerator()->create_user();
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/grace/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2022020401; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_grace';
$plugin->release = 'v0.1';
$plugin->maturity = MATURITY_STABLE;
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/iprange/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2019102400; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_iprange';
$plugin->release = 'v0.1';
$plugin->maturity = MATURITY_STABLE;
Expand Down
12 changes: 3 additions & 9 deletions admin/tool/mfa/factor/nosetup/classes/factor.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,9 @@ public function get_state() {
* @return void
*/
public function possible_states($user) {
// Check if user has any other input or setup factors active.
$factors = \tool_mfa\plugininfo\factor::get_active_other_user_factor_types($user);
foreach ($factors as $factor) {
if ($factor->has_input() || $factor->has_setup()) {
return [\tool_mfa\plugininfo\factor::STATE_NEUTRAL];
}
}

return [\tool_mfa\plugininfo\factor::STATE_PASS];
// We return Neutral here because to support optional rollouts
// it needs to report neutral or the menu to setup will not display.
return [\tool_mfa\plugininfo\factor::STATE_NEUTRAL];
}

/**
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/nosetup/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2020042302; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_nosetup';
$plugin->release = 'v0.1';
$plugin->maturity = MATURITY_STABLE;
Expand Down
2 changes: 1 addition & 1 deletion admin/tool/mfa/factor/role/classes/factor.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public function get_roles(string $selectedroles) : string {
$roles[] = get_string('administrator');
} else {
$record = $DB->get_record('role', ['id' => $role]);
$roles[] = role_get_name($record);
$roles[] = $record ? role_get_name($record) : null;
}
}
return implode(', ', $roles);
Expand Down
4 changes: 1 addition & 3 deletions admin/tool/mfa/factor/role/tests/factor_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace factor_role\tests;

defined('MOODLE_INTERNAL') || die();
namespace factor_role;

/**
* Tests for role factor.
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/role/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2020072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_role';
$plugin->release = 'v0.1';
$plugin->maturity = MATURITY_STABLE;
Expand Down
19 changes: 18 additions & 1 deletion admin/tool/mfa/factor/token/tests/factor_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace factor_token\tests;
namespace factor_token;

/**
* Tests for MFA manager class.
Expand All @@ -27,11 +27,24 @@
*/
class factor_test extends \advanced_testcase {

/**
* Holds specific requested factor, which is token factor.
*
* @var \factor_token\factor $factor
*/
public \factor_token\factor $factor;

public function setUp(): void {
$this->resetAfterTest();
$this->factor = new \factor_token\factor('token');
}

/**
* Test calculating expiry time in general
*
* @covers ::calculate_expiry_time
* @return void
*/
public function test_calculate_expiry_time_in_general() {
$timestamp = 1642213800; // 1230 UTC.

Expand Down Expand Up @@ -75,6 +88,7 @@ public function test_calculate_expiry_time_in_general() {
* value, provided it never goes past raw value expiry time, and when it
* needs to be 2am, it's 2am on the following morning.
*
* @covers ::calculate_expiry_time
* @param int $timestamp
* @dataProvider timestamp_provider
*/
Expand Down Expand Up @@ -123,6 +137,7 @@ public function test_calculate_expiry_time_for_overnight_expiry_with_one_day_exp
* value, provided it never goes past raw value expiry time, and when it
* needs to be 2am, it's 2am on the morning after tomorrow.
*
* @covers ::calculate_expiry_time
* @param int $timestamp
* @dataProvider timestamp_provider
*/
Expand Down Expand Up @@ -173,6 +188,7 @@ public function test_calculate_expiry_time_for_overnight_expiry_with_two_day_exp
/**
* This should check if the 3am expiry is pushed back to 2am as expected, but everything else appears as expected
*
* @covers ::calculate_expiry_time
* @param int $timestamp
* @dataProvider timestamp_provider
*/
Expand Down Expand Up @@ -217,6 +233,7 @@ public function test_calculate_expiry_time_for_overnight_expiry_with_three_hour_
/**
* Only relevant based on the hour padding used, which is currently set to 2 hours (2am).
*
* @covers ::calculate_expiry_time
* @param int $timestamp
* @dataProvider timestamp_provider
*/
Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/token/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2022011700; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_token';
$plugin->release = 2022011700;
$plugin->maturity = MATURITY_STABLE;
Expand Down
2 changes: 1 addition & 1 deletion admin/tool/mfa/factor/totp/tests/factor_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

namespace factor_totp\tests;
namespace factor_totp;

defined('MOODLE_INTERNAL') || die();

Expand Down
4 changes: 2 additions & 2 deletions admin/tool/mfa/factor/totp/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@

defined('MOODLE_INTERNAL') || die();

$plugin->version = 2021021700; // The current plugin version (Date: YYYYMMDDXX).
$plugin->version = 2023072100; // The current plugin version (Date: YYYYMMDDXX).
$plugin->release = 2021021700;
$plugin->requires = 2017051500.00; // Support back to 3.3.
$plugin->requires = 2023042400.00; // Supports from 4.2.
$plugin->component = 'factor_totp';
$plugin->maturity = MATURITY_STABLE;
$plugin->dependencies = ['tool_mfa' => 2019102400];
2 changes: 1 addition & 1 deletion admin/tool/mfa/factor/webauthn/amd/build/utils.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit d42593d

Please sign in to comment.