MDL-78509 tool_mfa: Fixes based on the report and other issues

In this commit, there are couple of fixes based on the report:
1. Removed legacy polyfill from provider.php
2. Fixed phpunit warning detected by CodeChecker
3. Removed unused files
4. Fixed the PHPunit failures by removing "securityquestions"
   from the data_provider due to it has not been included as one of the factors
5. Added PHPunit test to the factors that can be unittested
6. Removed !important rule from tool_mfa/styles.css
7. Added (int) type to sleep method within sleep_timer method due to a php deprecation
in too_mfa/classes/manager.php
8. Changed last param form bool to string in not_enough_factors() when initiating
a new single_button object in tool_mfa/renderer.php
9. Add explanation text to login page
10. Fixed "Access to an undefined property .." from PHPStan
12. Fixed all the "Variable $.. might not be defined" from PHPStan
13. Fixed the issue from catalyst/moodle-tool_mfa#379

admin/tool/mfa/classes/event/user_deleted_factor.php

@@ -16,6 +16,8 @@
 
 namespace tool_mfa\event;
 
+use stdClass;
+
 /**
  * Event for when user factor is deleted.
  *
@@ -37,11 +39,11 @@ class user_deleted_factor extends \core\event\base {
      * @param stdClass $deleteuser the user who performed the factor delete.
      * @param string $factorname deleted factor
      *
-     * @return user_factor_deleted the user_factor_deleted event
+     * @return \core\event\base the user_factor_deleted event
      *
      * @throws \coding_exception
      */
-    public static function user_deleted_factor_event($user, $deleteuser, $factorname) {
+    public static function user_deleted_factor_event(stdClass $user, $deleteuser, $factorname): \core\event\base {
 
         $data = [
             'relateduserid' => $user->id,
@@ -61,7 +63,7 @@ public static function user_deleted_factor_event($user, $deleteuser, $factorname
      *
      * @return void
      */
-    protected function init() {
+    protected function init(): void {
         $this->data['crud'] = 'd';
         $this->data['edulevel'] = self::LEVEL_OTHER;
     }
@@ -71,7 +73,7 @@ protected function init() {
      *
      * @return string
      */
-    public function get_description() {
+    public function get_description(): string {
         // The log message changed from logging the deleter user object to the ID. This must be kept for backwards compat
         // With old log events.
         if (is_object($this->other['delete'])) {
@@ -89,7 +91,7 @@ public function get_description() {
      * @return string
      * @throws \coding_exception
      */
-    public static function get_name() {
+    public static function get_name(): string {
         return get_string('event:userdeletedfactor', 'tool_mfa');
     }
 }

admin/tool/mfa/classes/event/user_failed_mfa.php

@@ -16,6 +16,8 @@
 
 namespace tool_mfa\event;
 
+use stdClass;
+
 /**
  * Event for when user successfully passed all MFA factor checks.
  *
@@ -33,13 +35,13 @@ class user_failed_mfa extends \core\event\base {
     /**
      * Create instance of event.
      *
-     * @param int $user the User object of the User who failed MFA authentication.
+     * @param stdClass $user the User object of the User who failed MFA authentication.
      *
      * @return user_failed_mfa the user_passed_mfa event
      *
      * @throws \coding_exception
      */
-    public static function user_failed_mfa_event($user) {
+    public static function user_failed_mfa_event(stdClass $user): user_failed_mfa {
         // Build debug info string.
         $factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
         $debug = '';
@@ -71,7 +73,7 @@ public static function user_failed_mfa_event($user) {
      *
      * @return void
      */
-    protected function init() {
+    protected function init(): void {
         $this->data['crud'] = 'r';
         $this->data['edulevel'] = self::LEVEL_OTHER;
     }
@@ -81,7 +83,7 @@ protected function init() {
      *
      * @return string
      */
-    public function get_description() {
+    public function get_description(): string {
         return "The user with id '{$this->other['userid']}' failed authenticating with MFA.
             <br> Information: {$this->other['failurereason']}{$this->other['debug']}";
     }
@@ -92,7 +94,7 @@ public function get_description() {
      * @return string
      * @throws \coding_exception
      */
-    public static function get_name() {
+    public static function get_name(): string {
         return get_string('event:userfailedmfa', 'tool_mfa');
     }
 }

admin/tool/mfa/classes/event/user_passed_mfa.php

@@ -16,6 +16,8 @@
 
 namespace tool_mfa\event;
 
+use stdClass;
+
 /**
  * Event for when user successfully passed all MFA factor checks.
  *
@@ -33,13 +35,13 @@ class user_passed_mfa extends \core\event\base {
     /**
      * Create instance of event.
      *
-     * @param int $user the User object of the User who passed all MFA factor checks.
+     * @param stdClass $user the User object of the User who passed all MFA factor checks.
      *
      * @return user_passed_mfa the user_passed_mfa event
      *
      * @throws \coding_exception
      */
-    public static function user_passed_mfa_event($user) {
+    public static function user_passed_mfa_event(stdClass $user): user_passed_mfa {
 
         // Build debug info string.
         $factors = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
@@ -65,7 +67,7 @@ public static function user_passed_mfa_event($user) {
      *
      * @return void
      */
-    protected function init() {
+    protected function init(): void {
         $this->data['crud'] = 'r';
         $this->data['edulevel'] = self::LEVEL_OTHER;
     }
@@ -75,7 +77,7 @@ protected function init() {
      *
      * @return string
      */
-    public function get_description() {
+    public function get_description(): string {
         return "The user with id '{$this->other['userid']}' successfully passed MFA. <br> Information: {$this->other['debug']}";
     }
 
@@ -85,7 +87,7 @@ public function get_description() {
      * @return string
      * @throws \coding_exception
      */
-    public static function get_name() {
+    public static function get_name(): string {
         return get_string('event:userpassedmfa', 'tool_mfa');
     }
 }

admin/tool/mfa/classes/event/user_revoked_factor.php

@@ -16,6 +16,8 @@
 
 namespace tool_mfa\event;
 
+use stdClass;
+
 /**
  * Event for when user successfully revoked MFA Factor.
  *
@@ -33,14 +35,14 @@ class user_revoked_factor extends \core\event\base {
     /**
      * Create instance of event.
      *
-     * @param int $user the User object of the User who has revoked new factor
+     * @param stdClass $user the User object of the User who has revoked new factor
      * @param string $factorname revoked factor
      *
-     * @return user_passed_mfa the user_passed_mfa event
+     * @return self the related event
      *
      * @throws \coding_exception
      */
-    public static function user_revoked_factor_event($user, $factorname) {
+    public static function user_revoked_factor_event(stdClass $user, $factorname): self {
 
         $data = [
             'relateduserid' => null,
@@ -59,7 +61,7 @@ public static function user_revoked_factor_event($user, $factorname) {
      *
      * @return void
      */
-    protected function init() {
+    protected function init(): void {
         $this->data['crud'] = 'd';
         $this->data['edulevel'] = self::LEVEL_OTHER;
     }
@@ -69,7 +71,7 @@ protected function init() {
      *
      * @return string
      */
-    public function get_description() {
+    public function get_description(): string {
         return "The user with id '{$this->other['userid']}' successfully revoked {$this->other['factorname']}";
     }
 
@@ -79,7 +81,7 @@ public function get_description() {
      * @return string
      * @throws \coding_exception
      */
-    public static function get_name() {
+    public static function get_name(): string {
         return get_string('event:userrevokedfactor', 'tool_mfa');
     }
 }

admin/tool/mfa/classes/event/user_setup_factor.php

@@ -16,6 +16,8 @@
 
 namespace tool_mfa\event;
 
+use stdClass;
+
 /**
  * Event for when user successfully setup new MFA Factor.
  *
@@ -33,14 +35,14 @@ class user_setup_factor extends \core\event\base {
     /**
      * Create instance of event.
      *
-     * @param object $user the User object of the User who has setup new factor
+     * @param stdClass $user the User object of the User who has setup new factor
      * @param string $factorname setup factor
      *
-     * @return user_passed_mfa the user_passed_mfa event
+     * @return self the related event
      *
      * @throws \coding_exception
      */
-    public static function user_setup_factor_event($user, $factorname) {
+    public static function user_setup_factor_event(stdClass $user, $factorname): self {
 
         $data = [
             'relateduserid' => null,
@@ -59,7 +61,7 @@ public static function user_setup_factor_event($user, $factorname) {
      *
      * @return void
      */
-    protected function init() {
+    protected function init(): void {
         $this->data['crud'] = 'c';
         $this->data['edulevel'] = self::LEVEL_OTHER;
     }
@@ -69,7 +71,7 @@ protected function init() {
      *
      * @return string
      */
-    public function get_description() {
+    public function get_description(): string {
         return "The user with id '{$this->other['userid']}' successfully setup {$this->other['factorname']}";
     }
 
@@ -79,7 +81,7 @@ public function get_description() {
      * @return string
      * @throws \coding_exception
      */
-    public static function get_name() {
+    public static function get_name(): string {
         return get_string('event:usersetupfactor', 'tool_mfa');
     }
 }

admin/tool/mfa/classes/local/admin_setting_managemfa.php

@@ -43,9 +43,9 @@ public function __construct() {
     /**
      * Always returns true
      *
-     * @return true
+     * @return bool
      */
-    public function get_setting() {
+    public function get_setting(): bool {
         return true;
     }
 
@@ -55,7 +55,7 @@ public function get_setting() {
      * @param mixed $data
      * @return string Always returns ''
      */
-    public function write_setting($data) {
+    public function write_setting($data): string {
         return '';
     }
 
@@ -69,7 +69,7 @@ public function write_setting($data) {
      * @throws \coding_exception
      * @throws \moodle_exception
      */
-    public function output_html($data, $query='') {
+    public function output_html($data, $query=''): string {
         global $OUTPUT;
 
         $return = $OUTPUT->box_start('generalbox');

admin/tool/mfa/classes/local/factor/fallback.php

@@ -36,32 +36,32 @@ public function __construct() {
     /**
      * {@inheritDoc}
      */
-    public function get_display_name() {
+    public function get_display_name(): string {
         return get_string('fallback', 'tool_mfa');
     }
 
     /**
      * {@inheritDoc}
      */
-    public function get_info() {
+    public function get_info(): string {
         return get_string('fallback_info', 'tool_mfa');
     }
 
     /**
      * {@inheritDoc}
      */
-    public function get_state() {
+    public function get_state(): string {
         return \tool_mfa\plugininfo\factor::STATE_FAIL;
     }
 
     /**
      * Sets the state of the factor check into the session.
      * Returns whether storing the var was successful.
      *
-     * @param mixed $state
+     * @param string $state
      * @return bool
      */
-    public function set_state($state) {
+    public function set_state(string $state): bool {
         return false;
     }
 }