catalyst · waleedhassan5 · Jun 30, 2026
diff --git a/amd/src/connectivity_test.js b/amd/src/connectivity_test.js
@@ -22,6 +22,12 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
+/**
+ * Initialise connectivity check: fetches checkTarget and redirects to redirURL on success.
+ *
+ * @param {string} checkTarget URL to HEAD-request as the connectivity probe
+ * @param {string} redirURL URL to redirect to when the probe succeeds
+ */
 export const init = (checkTarget, redirURL) => {
     // We need to use no-cors to ignore cors, however,
     // this means we are returned an opaque response.

diff --git a/certificatelock.php b/certificatelock.php
@@ -39,25 +39,23 @@
 $settingspage = new moodle_url('/admin/settings.php?section=authsettingsaml2');
 
 if ($data = $form->get_data()) {
-    if ($form->is_submitted()) {
-        $certfiles = [$saml2auth->certpem, $saml2auth->certcrt];
-        if (isset($data->unlockcertsbutton)) {
-            // Change the permissions in order to regenerate if unlocked.
-            foreach ($certfiles as $certfile) {
-                chmod($certfile, $CFG->filepermissions);
-            }
-            // Store the unlocked state in config.
-            set_config('certs_locked', '0', 'auth_saml2');
-        } else {
-            foreach ($certfiles as $certfile) {
-                chmod($certfile, $CFG->filepermissions & 0440);
-            }
-            // Store the locked state in config.
-            set_config('certs_locked', '1', 'auth_saml2');
+    $certfiles = [$saml2auth->certpem, $saml2auth->certcrt];
+    if (isset($data->unlockcertsbutton)) {
+        // Change the permissions in order to regenerate if unlocked.
+        foreach ($certfiles as $certfile) {
+            chmod($certfile, $CFG->filepermissions);
         }
-
-        redirect($settingspage);
+        // Store the unlocked state in config.
+        set_config('certs_locked', '0', 'auth_saml2');
+    } else {
+        foreach ($certfiles as $certfile) {
+            chmod($certfile, $CFG->filepermissions & 0440);
+        }
+        // Store the locked state in config.
+        set_config('certs_locked', '1', 'auth_saml2');
     }
+
+    redirect($settingspage);
 } else if ($form->is_cancelled()) {
     redirect($settingspage);
 }

diff --git a/classes/admin/setting_button.php b/classes/admin/setting_button.php
@@ -66,25 +66,14 @@ public function __construct($name, $visiblename, $description, $label, $href) {
      * @return string Returns an HTML string
      */
     public function output_html($data, $query = '') {
-        if (moodle_major_version() < '3.3') {
-            $params = [
-                'type'    => 'button',
-                'value'   => $this->label,
-                'onclick' => 'location.href="' . $this->href . '"',
-            ];
+        global $OUTPUT;
+        $context = (object)[
+            'label'    => $this->label,
+            'href'     => $this->href,
+            'forceltr' => $this->get_force_ltr(),
+        ];
 
-            $content = html_writer::empty_tag('input', $params);
-            $element = html_writer::div($content, 'form-text defaultsnext');
-        } else {
-            global $OUTPUT;
-            $context = (object)[
-                'label'    => $this->label,
-                'href'     => $this->href,
-                'forceltr' => $this->get_force_ltr(),
-            ];
-
-            $element = $OUTPUT->render_from_template('auth_saml2/setting_configbutton', $context);
-        }
+        $element = $OUTPUT->render_from_template('auth_saml2/setting_configbutton', $context);
 
         return format_admin_setting($this, $this->visiblename, $element, $this->description);
     }

diff --git a/classes/admin/setting_idpmetadata.php b/classes/admin/setting_idpmetadata.php
@@ -148,9 +148,9 @@ private function process_idp_xml(
         $names = $xpath->query('.//mdui:DisplayName', $idpelements);
         $idpname = null;
         if ($names && $names->length > 0) {
-            $idpname = $names->item(0)->textContent;
+            $idpname = clean_param($names->item(0)->textContent, PARAM_TEXT);
         } else if (!empty($idp->idpname)) {
-            $idpname = $idp->idpname;
+            $idpname = clean_param($idp->idpname, PARAM_TEXT);
         } else {
             $idpname = get_string('idpnamedefault', 'auth_saml2');
         }
@@ -159,7 +159,7 @@ private function process_idp_xml(
         $logos = $xpath->query('.//mdui:Logo', $idpelements);
         $logo = null;
         if ($logos && $logos->length > 0) {
-            $logo = $logos->item(0)->textContent;
+            $logo = clean_param($logos->item(0)->textContent, PARAM_URL);
         }
 
         if (isset($oldidps[$idp->idpurl][$entityid])) {
@@ -197,11 +197,17 @@ private function process_idp_xml(
     private function remove_old_idps($oldidps) {
         global $DB;
 
+        $ids = [];
         foreach ($oldidps as $metadataidps) {
             foreach ($metadataidps as $oldidp) {
-                $DB->delete_records('auth_saml2_idps', ['id' => $oldidp->id]);
+                $ids[] = $oldidp->id;
             }
         }
+
+        if (!empty($ids)) {
+            [$insql, $inparams] = $DB->get_in_or_equal($ids);
+            $DB->delete_records_select('auth_saml2_idps', "id $insql", $inparams);
+        }
     }
 
     /**
@@ -254,7 +260,7 @@ private function get_idp_xml_path(idp_data $idp) {
             $lines = explode("\n", $rawxml);
             $msg = '';
             foreach ($errors as $error) {
-                $msg .= "<br>Error ({$error->code}) line $error->line char  $error->column: $error->message";
+                $msg .= '<br>Error (' . s($error->code) . ') line ' . s($error->line) . ' char ' . s($error->column) . ': ' . s($error->message);
             }
 
             throw new setting_idpmetadata_exception(get_string('idpmetadata_invalid', 'auth_saml2') . $msg);

diff --git a/classes/api.php b/classes/api.php
@@ -46,7 +46,7 @@ public static function after_logout_from_sp($state): void {
         global $saml2config;
 
         $cookiename = $saml2config['session.cookie.name'];
-        $sessid = $_COOKIE[$cookiename];
+        $sessid = clean_param($_COOKIE[$cookiename] ?? '', PARAM_ALPHANUMEXT);
 
         // In SSP should do this for us but remove stored SP session data.
         $storeclass = $saml2config['store.type'];

diff --git a/classes/auth.php b/classes/auth.php
@@ -647,7 +647,7 @@ public function saml_login() {
             }
         }
 
-        if (isset($_GET['rememberidp']) && $_GET['rememberidp'] == 1) {
+        if (optional_param('rememberidp', 0, PARAM_BOOL) == 1) {
             $this->set_idp_cookie($SESSION->saml2idp);
         }
 
@@ -794,7 +794,7 @@ public function saml_login_complete($attributes) {
                     'reason' => AUTH_LOGIN_NOUSER]]);
                 $event->trigger();
                 $this->log(__FUNCTION__ . " user '$uid' is not in moodle so error");
-                $this->error_page(get_string('nouser', 'auth_saml2', $uid));
+                $this->error_page(get_string('nouser', 'auth_saml2', s($uid)));
             }
         } else {
             // Prevent access to users who are suspended.
@@ -808,7 +808,7 @@ public function saml_login_complete($attributes) {
                 ]);
                 $event->trigger();
 
-                $this->error_page(get_string('suspendeduser', 'auth_saml2', $uid));
+                $this->error_page(get_string('suspendeduser', 'auth_saml2', s($uid)));
             }
 
             $this->log(__FUNCTION__ . ' found user ' . $user->username);
@@ -825,7 +825,7 @@ public function saml_login_complete($attributes) {
             $event->trigger();
 
             $this->log(__FUNCTION__ . " user $uid is auth type: $user->auth");
-            $this->error_page(get_string('wrongauth', 'auth_saml2', $uid));
+            $this->error_page(get_string('wrongauth', 'auth_saml2', s($uid)));
         }
 
         if ($this->config->anyauth && !is_enabled_auth($user->auth)) {
@@ -840,7 +840,7 @@ public function saml_login_complete($attributes) {
 
             $this->log(__FUNCTION__ . " user $uid's auth type: $user->auth is not enabled");
             $this->error_page(get_string('anyauthotherdisabled', 'auth_saml2', [
-                'username' => $uid, 'auth' => $user->auth,
+                'username' => s($uid), 'auth' => s($user->auth),
             ]));
         }
 
@@ -860,8 +860,6 @@ public function saml_login_complete($attributes) {
         // Make sure all user data is fetched.
         $user = get_complete_user_data('username', $user->username, null, false);
         complete_user_login($user);
-        $USER->loggedin = true;
-        $USER->site = $CFG->wwwroot;
         set_moodle_cookie($USER->username);
 
         $wantsurl = core_login_get_return_url();

diff --git a/classes/check/certificateexpiry.php b/classes/check/certificateexpiry.php
@@ -47,7 +47,6 @@ class certificateexpiry extends check {
      * Constructor
      */
     public function __construct() {
-        global $CFG;
         $this->id = 'certificateexpiry';
         $this->name = get_string('checkcertificateexpiry', 'auth_saml2');
     }
@@ -69,7 +68,7 @@ public function get_action_link(): ?\action_link {
      * @return result
      */
     public function get_result(): result {
-        global $CFG, $saml2auth;
+        global $saml2auth;
 
         $path = $saml2auth->certcrt;
         $data = openssl_x509_parse(file_get_contents($path));

diff --git a/classes/event/cert_regenerated.php b/classes/event/cert_regenerated.php
@@ -26,7 +26,6 @@
 namespace auth_saml2\event;
 
 use core\event\base;
-use moodle_url;
 
 /**
  * cert_regenerated class.

diff --git a/classes/form/availableidps.php b/classes/form/availableidps.php
@@ -28,7 +28,6 @@
 defined('MOODLE_INTERNAL') || die();
 
 use moodleform;
-use core\output\notification;
 
 require_once("$CFG->libdir/formslib.php");
 
@@ -45,7 +44,6 @@ class availableidps extends moodleform {
      * Definition
      */
     public function definition() {
-        global $OUTPUT;
         $mform = $this->_form;
 
         $metadataentities = $this->_customdata['metadataentities'];
@@ -61,7 +59,7 @@ public function definition() {
 
                 // List the source.
                 $mform->addElement('html', \html_writer::div(
-                    get_string('source', 'auth_saml2', $idpentity['entityid']),
+                    get_string('source', 'auth_saml2', s($idpentity['entityid'])),
                     'alert p-2 bg-gray bg-gray020'
                 ));
 
@@ -87,6 +85,7 @@ public function definition() {
                     [],
                     [false, true]
                 );
+                $mform->setType($fieldkey . '[activeidp]', PARAM_BOOL);
 
                 // Add the defaultidp checkbox.
                 $mform->addElement(
@@ -97,6 +96,7 @@ public function definition() {
                     [],
                     [false, true]
                 );
+                $mform->setType($fieldkey . '[defaultidp]', PARAM_BOOL);
 
                 // Add the adminidp checkbox.
                 $mform->addElement(
@@ -107,6 +107,7 @@ public function definition() {
                     [],
                     [false, true]
                 );
+                $mform->setType($fieldkey . '[adminidp]', PARAM_BOOL);
                 $mform->addHelpButton($fieldkey . '[adminidp]', 'multiidp:label:admin', 'auth_saml2');
 
                 // Add whitelisted IP for redirection to this IdP.

diff --git a/classes/form/lockcertificate.php b/classes/form/lockcertificate.php
@@ -35,8 +35,9 @@
 /**
  * Form for locking the certificates.
  *
- * @copyright Catalyst IT
- * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @package    auth_saml2
+ * @copyright  Catalyst IT
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class lockcertificate extends moodleform {
     /**

diff --git a/classes/form/regenerate.php b/classes/form/regenerate.php
@@ -33,6 +33,7 @@
 /**
  * Regenerate the Private Key and Certificate files
  *
+ * @package    auth_saml2
  * @copyright  Matt Porritt <mattp@catalyst-au.net>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */

diff --git a/classes/form/selectidp_buttons.php b/classes/form/selectidp_buttons.php
@@ -53,6 +53,7 @@ public function definition() {
         $mform->addElement('hidden', 'wants', $wants);
         $mform->setType('wants', PARAM_URL);
         $mform->addElement('checkbox', 'rememberidp', '', get_string('rememberidp', 'auth_saml2'));
+        $mform->setType('rememberidp', PARAM_BOOL);
 
         foreach ($metadataentities as $idpentities) {
             if (isset($idpentities[$storedchoiceidp])) {
@@ -82,12 +83,14 @@ public function definition() {
      * @return string
      */
     private function get_idpbutton($idpentityid, $idpname, $logourl, $rememberedidp = false) {
-        $logo = !is_null($logourl) ? "<img src=\"{$logourl}\"> " : "";
+        $safeentityid = s($idpentityid);
+        $safename = s($idpname);
+        $logo = !is_null($logourl) ? '<img src="' . s($logourl) . '"> ' : '';
         $extraclasses = $rememberedidp ? "rememberedidp" : "";
         return <<<EOD
 <div class="fitem fitem_actionbuttons fitem_fsubmit ">
-    <button value="{$idpentityid}" class="btn idpbtn {$extraclasses}" type="submit" name="idp">
-        {$logo}{$idpname}
+    <button value="{$safeentityid}" class="btn idpbtn {$extraclasses}" type="submit" name="idp">
+        {$logo}{$safename}
     </button>
 </div>
 EOD;

diff --git a/classes/form/selectidp_dropdown.php b/classes/form/selectidp_dropdown.php
@@ -55,7 +55,9 @@ public function definition() {
         $mform->addElement('hidden', 'wants', $wants);
         $mform->setType('wants', PARAM_URL);
         $mform->addElement('select', 'idp', '', $idpentityids);
+        $mform->setType('idp', PARAM_TEXT);
         $mform->addElement('checkbox', 'rememberidp', '', get_string('rememberidp', 'auth_saml2'));
+        $mform->setType('rememberidp', PARAM_BOOL);
 
         $mform->addElement('submit', 'login', $idpname, ['style' => 'margin-left:0px']);
     }

diff --git a/classes/form/testidpselect.php b/classes/form/testidpselect.php
@@ -53,18 +53,21 @@ public function definition() {
         }
 
         $mform->addElement('select', 'idp', get_string('test_auth_button_login', 'auth_saml2'), $selectvalues);
+        $mform->setType('idp', PARAM_TEXT);
 
         $radioarray = [];
         $radioarray[] = $mform->createElement('radio', 'testtype', '', get_string('test_auth_str', 'auth_saml2'), 'login');
         $radioarray[] = $mform->createElement('radio', 'testtype', '', get_string('test_passive_str', 'auth_saml2'), 'passive');
 
         $mform->setDefault('testtype', 'login');
+        $mform->setType('testtype', PARAM_TEXT);
         $mform->addGroup($radioarray, 'radioar', '', ['<br/>'], false);
         $mform->addElement('submit', 'login', get_string('test_auth_button_login', 'auth_saml2'));
 
         $mform->addElement('html', '<br /><br />');
 
         $mform->addElement('select', 'idplogout', get_string('test_auth_button_logout', 'auth_saml2'), $selectvalues);
+        $mform->setType('idplogout', PARAM_TEXT);
         $mform->addElement('submit', 'logout', get_string('test_auth_button_logout', 'auth_saml2'));
     }
 }
diff --git a/classes/idp_data.php b/classes/idp_data.php
@@ -64,6 +64,7 @@ public function get_rawxml() {
      * Set raw xml.
      *
      * @param string $rawxml
+     * @return void
      */
     public function set_rawxml($rawxml) {
         $this->rawxml = $rawxml;