Merge branch 'MDL-67818-check-api-fixes' of https://github.com/brendanheywood/moodle

auth/none/classes/check/noauth.php

@@ -39,12 +39,12 @@ use core\check\result;
 class noauth extends \core\check\check {
 
     /**
-     * Constructor
+     * A link to a place to action this
+     *
+     * @return action_link
      */
-    public function __construct() {
+    public function get_action_link(): ?\action_link {
-        $this->id = 'noauth';
+        return new \action_link(
-        $this->name = get_string('check_noauth_name', 'auth_none');
-        $this->actionlink = new \action_link(
             new \moodle_url('/admin/settings.php?section=manageauths'),
             get_string('authsettings', 'admin'));
     }
@@ -54,15 +54,14 @@ class noauth extends \core\check\check {
      * @return result
      */
     public function get_result(): result {
-
         if (is_enabled_auth('none')) {
             $status = result::ERROR;
-            $summary = get_string('check_noauth_error', 'auth_none');
+            $summary = get_string('checknoautherror', 'auth_none');
         } else {
             $status = result::OK;
-            $summary = get_string('check_noauth_ok', 'auth_none');
+            $summary = get_string('checknoauthok', 'auth_none');
         }
-        $details = get_string('check_noauth_details', 'auth_none');
+        $details = get_string('checknoauthdetails', 'auth_none');
 
         return new result($status, $summary, $details);
     }

auth/none/lang/en/auth_none.php

@@ -25,7 +25,7 @@
 $string['auth_nonedescription'] = 'Users can sign in and create valid accounts immediately, with no authentication against an external server and no confirmation via email.  Be careful using this option - think of the security and administration problems this could cause.';
 $string['pluginname'] = 'No authentication';
 $string['privacy:metadata'] = 'The No authentication plugin does not store any personal data.';
-$string['check_noauth_details'] = '<p>The <em>No authentication</em> plugin is not intended for production sites. Please disable it unless this is a development test site.</p>';
+$string['checknoauthdetails'] = '<p>The <em>No authentication</em> plugin is not intended for production sites. Please disable it unless this is a development test site.</p>';
-$string['check_noauth_error'] = 'The No authentication plugin cannot be used on production sites.';
+$string['checknoautherror'] = 'The No authentication plugin cannot be used on production sites.';
-$string['check_noauth_name'] = 'No authentication';
+$string['checknoauth'] = 'No authentication';
-$string['check_noauth_ok'] = 'The no authentication plugin is disabled.';
+$string['checknoauthok'] = 'The no authentication plugin is disabled.';

lib/classes/check/access/defaultuserrole.php

@@ -41,13 +41,22 @@ use core\check\result;
 class defaultuserrole extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_defaultuserrole_name', 'report_security');
+    }
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
         global $CFG;
-        $this->id = 'defaultuserrole';
+        return new \action_link(
-        $this->name = get_string('check_defaultuserrole_name', 'report_security');
-        $this->actionlink = new \action_link(
             new \moodle_url('/admin/roles/define.php?action=view&roleid=' . $CFG->defaultuserroleid),
             get_string('userpolicies', 'admin'));
     }

lib/classes/check/access/frontpagerole.php

@@ -41,12 +41,21 @@ use core\check\result;
 class frontpagerole extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        $this->id = 'frontpagerole';
+        return get_string('check_frontpagerole_name', 'report_security');
-        $this->name = get_string('check_frontpagerole_name', 'report_security');
+    }
-        $this->actionlink = new \action_link(
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=frontpagesettings#admin-defaultfrontpageroleid'),
             get_string('frontpagesettings', 'admin'));
     }

lib/classes/check/access/guestrole.php

@@ -41,13 +41,21 @@ use core\check\result;
 class guestrole extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_guestrole_name', 'report_security');
+    }
 
-        $this->id = 'guestrole';
+    /**
-        $this->name = get_string('check_guestrole_name', 'report_security');
+     * A link to a place to action this
-        $this->actionlink = new \action_link(
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=userpolicies'),
             get_string('userpolicies', 'admin'));
     }

lib/classes/check/access/riskadmin.php

@@ -41,12 +41,21 @@ use core\check\result;
 class riskadmin extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        $this->id = 'riskadmin';
+        return get_string('check_riskadmin_name', 'report_security');
-        $this->name = get_string('check_riskadmin_name', 'report_security');
+    }
-        $this->actionlink = new \action_link(
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/roles/admins.php'),
             get_string('siteadministrators', 'role'));
     }

lib/classes/check/access/riskbackup.php

@@ -39,13 +39,21 @@ use core\check\result;
 class riskbackup extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_riskbackup_name', 'report_security');
+    }
 
-        $this->id = 'riskbackup';
+    /**
-        $this->name = get_string('check_riskbackup_name', 'report_security');
+     * A link to a place to action this
-        $this->actionlink = new \action_link(
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/roles/manage.php'),
             get_string('manageroles', 'role'));
     }

lib/classes/check/access/riskxss.php

@@ -46,13 +46,21 @@ use core\check\result;
 class riskxss extends \core\check\check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_riskxss_name', 'report_security');
+    }
 
-        $this->id = 'riskxss';
+    /**
-        $this->name = get_string('check_riskxss_name', 'report_security');
+     * A link to a place to action this
-        $this->actionlink = new \action_link(
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/roles/manage.php'),
             get_string('manageroles', 'role'));
     }

lib/classes/check/check.php

@@ -41,21 +41,6 @@ abstract class check {
      */
     protected $component = 'core';
 
-    /**
-     * @var string $id - Should be unique identifier within a component.
-     */
-    protected $id = '';
-
-    /**
-     * @var string $name - Name for the check, should be the same regardless of state.
-     */
-    protected $name = '';
-
-    /**
-     * @var action_link - an optional link to a place to address the check.
-     */
-    protected $actionlink = null;
-
     /**
      * Get the frankenstyle component name
      *
@@ -77,10 +62,16 @@ abstract class check {
     /**
      * Get the check's id
      *
-     * @return string must be unique for it's component
+     * This defaults to the base name of the class which is ok in the most
+     * cases but if you have a check which can have multiple instances then
+     * you should override this to be unique.
+     *
+     * @return string must be unique within a component
      */
     public function get_id(): string {
-        return $this->id;
+        $class = get_class($this);
+        $id = explode("\\", $class);
+        return end($id);
     }
 
     /**
@@ -103,7 +94,8 @@ abstract class check {
      * @return string
      */
     public function get_name(): string {
-        return $this->name;
+        $id = $this->get_id();
+        return get_string("check{$id}", $this->get_component());
     }
 
     /**
@@ -112,7 +104,7 @@ abstract class check {
      * @return action_link|null
      */
     public function get_action_link(): ?\action_link {
-        return $this->actionlink;
+        return null;
     }
 
     /**

lib/classes/check/environment/configrw.php

@@ -41,12 +41,12 @@ use core\check\result;
 class configrw extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        global $CFG;
+        return get_string('check_configrw_name', 'report_security');
-        $this->id = 'configrw';
-        $this->name = get_string('check_configrw_name', 'report_security');
     }
 
     /**

lib/classes/check/environment/displayerrors.php

@@ -47,12 +47,12 @@ use core\check\check;
 class displayerrors extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-
+        return get_string('check_displayerrors_name', 'report_security');
-        $this->id = 'displayerrors';
-        $this->name = get_string('check_displayerrors_name', 'report_security');
     }
 
     /**

lib/classes/check/environment/nodemodules.php

@@ -41,12 +41,12 @@ use core\check\result;
 class nodemodules extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        global $CFG;
+        return get_string('check_nodemodules_name', 'report_security');
-        $this->id = 'nodemodules';
-        $this->name = get_string('check_nodemodules_name', 'report_security');
     }
 
     /**

lib/classes/check/environment/preventexecpath.php

@@ -41,13 +41,12 @@ use core\check\result;
 class preventexecpath extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-
+        return get_string('check_preventexecpath_name', 'report_security');
-        global $CFG;
-        $this->id = 'preventexecpath';
-        $this->name = get_string('check_preventexecpath_name', 'report_security');
     }
 
     /**

lib/classes/check/environment/unsecuredataroot.php

@@ -40,14 +40,12 @@ use core\check\result;
 class unsecuredataroot extends \core\check\check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-
+        return get_string('check_unsecuredataroot_name', 'report_security');
-        global $CFG;
-
-        $this->id = 'unsecuredataroot';
-        $this->name = get_string('check_unsecuredataroot_name', 'report_security');
     }
 
     /**

lib/classes/check/environment/vendordir.php

@@ -41,12 +41,12 @@ use core\check\result;
 class vendordir extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        global $CFG;
+        return get_string('check_vendordir_name', 'report_security');
-        $this->id = 'vendordir';
-        $this->name = get_string('check_vendordir_name', 'report_security');
     }
 
     /**

lib/classes/check/http/cookiesecure.php

@@ -45,13 +45,21 @@ use core\check\result;
 class cookiesecure extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_cookiesecure_name', 'report_security');
+    }
 
-        $this->id = 'cookiesecure';
+    /**
-        $this->name = get_string('check_cookiesecure_name', 'report_security');
+     * A link to a place to action this
-        $this->actionlink = new \action_link(
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=httpsecurity#admin-cookiesecure'),
             get_string('httpsecurity', 'admin'));
     }

lib/classes/check/security/crawlers.php

@@ -47,13 +47,21 @@ use core\check\result;
 class crawlers extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_crawlers_name', 'report_security');
+    }
 
-        $this->id = 'crawlers';
+    /**
-        $this->name = get_string('check_crawlers_name', 'report_security');
+     * A link to a place to action this
-        $this->actionlink = new \action_link(
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=sitepolicies#admin-opentowebcrawlers'),
             get_string('sitepolicies', 'admin'));
     }

lib/classes/check/security/emailchangeconfirmation.php

@@ -41,12 +41,21 @@ use core\check\result;
 class emailchangeconfirmation extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        $this->id = 'emailchangeconfirmation';
+        return get_string('check_emailchangeconfirmation_name', 'report_security');
-        $this->name = get_string('check_emailchangeconfirmation_name', 'report_security');
+    }
-        $this->actionlink = new \action_link(
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=sitepolicies#admin-emailchangeconfirmation'),
             get_string('sitepolicies', 'admin'));
     }

lib/classes/check/security/embed.php

@@ -41,12 +41,21 @@ use core\check\result;
 class embed extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        $this->id = 'embed';
+        return get_string('check_embed_name', 'report_security');
-        $this->name = get_string('check_embed_name', 'report_security');
+    }
-        $this->actionlink = new \action_link(
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=sitepolicies#admin-allowobjectembed'),
             get_string('sitepolicies', 'admin'));
     }

lib/classes/check/security/mediafilterswf.php

@@ -41,13 +41,21 @@ use core\check\result;
 class mediafilterswf extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        global $CFG;
+        return get_string('check_mediafilterswf_name', 'report_security');
-        $this->id = 'mediafilterswf';
+    }
-        $this->name = get_string('check_mediafilterswf_name', 'report_security');
+
-        $this->actionlink = new \action_link(
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=managemediaplayers'),
             get_string('managemediaplayers', 'media'));
     }

lib/classes/check/security/openprofiles.php

@@ -41,13 +41,21 @@ use core\check\result;
 class openprofiles extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
+        return get_string('check_openprofiles_name', 'report_security');
+    }
 
-        $this->id = 'openprofiles';
+    /**
-        $this->name = get_string('check_openprofiles_name', 'report_security');
+     * A link to a place to action this
-        $this->actionlink = new \action_link(
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=sitepolicies#admin-forcelogin'),
             get_string('sitepolicies', 'admin'));
     }

lib/classes/check/security/passwordpolicy.php

@@ -41,12 +41,21 @@ use core\check\result;
 class passwordpolicy extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        $this->id = 'passwordpolicy';
+        return get_string('check_passwordpolicy_name', 'report_security');
-        $this->name = get_string('check_passwordpolicy_name', 'report_security');
+    }
-        $this->actionlink = new \action_link(
+
+    /**
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
             new \moodle_url('/admin/settings.php?section=sitepolicies#admin-passwordpolicy'),
             get_string('sitepolicies', 'admin'));
     }

lib/classes/check/security/webcron.php

@@ -41,14 +41,23 @@ use core\check\result;
 class webcron extends check {
 
     /**
-     * Constructor
+     * Get the short check name
+     *
+     * @return string
      */
-    public function __construct() {
+    public function get_name(): string {
-        $this->id = 'webcron';
+        return get_string('check_webcron_name', 'report_security');
-        $this->name = get_string('check_webcron_name', 'report_security');
+    }
-        $this->actionlink = new \action_link(
+
-            new \moodle_url('/admin/settings.php?section=sitepolicies#admin-cronclionly'),
+    /**
-            get_string('sitepolicies', 'admin'));
+     * A link to a place to action this
+     *
+     * @return action_link|null
+     */
+    public function get_action_link(): ?\action_link {
+        return new \action_link(
+           new \moodle_url('/admin/settings.php?section=sitepolicies#admin-cronclionly'),
+           get_string('sitepolicies', 'admin'));
     }
 
     /**

lib/outputrenderers.php

@@ -1687,7 +1687,7 @@ class core_renderer extends renderer_base {
      * @param result $result
      * @return string HTML fragment
      */
-    protected function render_result(core\check\result $result) {
+    protected function render_check_result(core\check\result $result) {
         return $this->render_from_template($result->get_template_name(), $result->export_for_template($this));
     }
 
@@ -1697,8 +1697,8 @@ class core_renderer extends renderer_base {
      * @param result $result
      * @return string HTML fragment
      */
-    public function result(core\check\result $result) {
+    public function check_result(core\check\result $result) {
-        return $this->render_result($result);
+        return $this->render_check_result($result);
     }
 
     /**

report/security/index.php

@@ -87,7 +87,7 @@ foreach ($checks as $check) {
     $link = new \moodle_url('/report/security/index.php', ['detail' => $ref]);
 
     $row = [];
-    $row[] = $OUTPUT->result($result);
+    $row[] = $OUTPUT->check_result($result);
     $row[] = $OUTPUT->action_link($link, $check->get_name());
 
     $row[] = $result->get_summary();