Merge pull request #65 from microsoft/MOODLE_501_STABLE

Release 5.1.1 of Microsoft plugins for Moodle 5.1

Author: weilai-irl

auth.php

@@ -53,9 +53,9 @@ public function __construct($forceloginflow = null) {
 
         if (
             isset($SESSION->stateadditionaldata) && !empty($SESSION->stateadditionaldata) &&
-            isset($SESSION->stateadditoinaldata['forceflow'])
+            isset($SESSION->stateadditionaldata['forceflow'])
         ) {
-            $loginflow = $SESSION->stateadditoinaldata['forceflow'];
+            $loginflow = $SESSION->stateadditionaldata['forceflow'];
         } else {
             if (!empty($forceloginflow) && is_string($forceloginflow)) {
                 $loginflow = $forceloginflow;

classes/event/user_created.php

@@ -44,7 +44,7 @@ public static function get_name() {
      * @return string
      */
     public function get_description() {
-        return "A user (user id '{$this->userid}') was creatd using the OpenID Connect authentication plugin.";
+        return "A user (user id '{$this->userid}') was created using the OpenID Connect authentication plugin.";
     }
 
     /**

classes/form/application.php

@@ -79,10 +79,39 @@ protected function definition() {
         $mform->setDefault('clientauthmethod', AUTH_OIDC_AUTH_METHOD_SECRET);
         $mform->addElement('static', 'clientauthmethod_help', '', get_string('clientauthmethod_help', 'auth_oidc'));
 
-        // Secret.
-        $mform->addElement('text', 'clientsecret', auth_oidc_config_name_in_form('clientsecret'), ['size' => 60]);
+        // Secret - Check if there's an existing secret to determine if we should add a "change" checkbox.
+        $hasexistingsecret = isset($this->_customdata['oidcconfig']->clientsecret) &&
+            !empty($this->_customdata['oidcconfig']->clientsecret);
+
+        if ($hasexistingsecret) {
+            $mform->addElement(
+                'advcheckbox',
+                'changesecret',
+                get_string('change_client_secret', 'auth_oidc'),
+                get_string('change_client_secret_desc', 'auth_oidc')
+            );
+            $mform->setType('changesecret', PARAM_BOOL);
+            $mform->disabledIf('changesecret', 'clientauthmethod', 'eq', AUTH_OIDC_AUTH_METHOD_CERTIFICATE);
+
+            // Store the original masked value in a hidden field AND as a data attribute for JavaScript to use.
+            $maskedsecret = auth_oidc_mask_secret($this->_customdata['oidcconfig']->clientsecret);
+            $mform->addElement('hidden', 'originalsecretmasked', $maskedsecret);
+            $mform->setType('originalsecretmasked', PARAM_TEXT);
+        }
+
+        $attributes = ['size' => 60, 'autocomplete' => 'off', 'class' => 'secret-field'];
+        if ($hasexistingsecret) {
+            $maskedsecret = auth_oidc_mask_secret($this->_customdata['oidcconfig']->clientsecret);
+            $attributes['data-original-masked'] = $maskedsecret;
+        }
+        $mform->addElement('text', 'clientsecret', auth_oidc_config_name_in_form('clientsecret'), $attributes);
         $mform->setType('clientsecret', PARAM_TEXT);
         $mform->disabledIf('clientsecret', 'clientauthmethod', 'neq', AUTH_OIDC_AUTH_METHOD_SECRET);
+
+        if ($hasexistingsecret) {
+            $mform->disabledIf('clientsecret', 'changesecret', 'notchecked');
+        }
+
         $mform->addElement('static', 'clientsecret_help', '', get_string('clientsecret_help', 'auth_oidc'));
 
         // Certificate source.
@@ -132,10 +161,38 @@ protected function definition() {
         $mform->disabledIf('clientcertfile', 'clientcertsource', 'neq', AUTH_OIDC_AUTH_CERT_SOURCE_FILE);
         $mform->addElement('static', 'clientcertfile_help', '', get_string('clientcertfile_help', 'auth_oidc'));
 
-        // Certificate file passphrase.
-        $mform->addElement('text', 'clientcertpassphrase', auth_oidc_config_name_in_form('clientcertpassphrase'), ['size' => 60]);
+        // Certificate file passphrase - Check if there's an existing passphrase.
+        $hasexistingpassphrase = isset($this->_customdata['oidcconfig']->clientcertpassphrase) &&
+            !empty($this->_customdata['oidcconfig']->clientcertpassphrase);
+
+        if ($hasexistingpassphrase) {
+            $mform->addElement(
+                'advcheckbox',
+                'changecertpassphrase',
+                get_string('change_cert_passphrase', 'auth_oidc'),
+                get_string('change_cert_passphrase_desc', 'auth_oidc')
+            );
+            $mform->setType('changecertpassphrase', PARAM_BOOL);
+
+            // Store the original masked value in a hidden field AND as a data attribute for JavaScript to use.
+            $maskedpassphrase = auth_oidc_mask_secret($this->_customdata['oidcconfig']->clientcertpassphrase);
+            $mform->addElement('hidden', 'originalpassphrasemasked', $maskedpassphrase);
+            $mform->setType('originalpassphrasemasked', PARAM_TEXT);
+        }
+
+        $attributes = ['size' => 60, 'autocomplete' => 'off', 'class' => 'secret-field'];
+        if ($hasexistingpassphrase) {
+            $maskedpassphrase = auth_oidc_mask_secret($this->_customdata['oidcconfig']->clientcertpassphrase);
+            $attributes['data-original-masked'] = $maskedpassphrase;
+        }
+        $mform->addElement('text', 'clientcertpassphrase', auth_oidc_config_name_in_form('clientcertpassphrase'), $attributes);
         $mform->setType('clientcertpassphrase', PARAM_TEXT);
         $mform->disabledIf('clientcertpassphrase', 'clientauthmethod', 'neq', AUTH_OIDC_AUTH_METHOD_CERTIFICATE);
+
+        if ($hasexistingpassphrase) {
+            $mform->disabledIf('clientcertpassphrase', 'changecertpassphrase', 'notchecked');
+        }
+
         $mform->addElement('static', 'clientcertpassphrase_help', '', get_string('clientcertpassphrase_help', 'auth_oidc'));
 
         // Endpoints header.
@@ -172,6 +229,11 @@ protected function definition() {
         $mform->setDefault('oidcscope', 'openid profile email');
         $mform->addElement('static', 'oidcscope_help', '', get_string('oidcscope_help', 'auth_oidc'));
 
+        // Custom Claim.
+        $mform->addElement('text', 'customclaims', auth_oidc_config_name_in_form('customclaims'), ['size' => 120]);
+        $mform->setType('customclaims', PARAM_TEXT);
+        $mform->setDefault('customclaims', '');
+        $mform->addElement('static', 'customclaims_help', '', get_string('customclaims_help', 'auth_oidc'));
         // Secret expiry notifications recipients.
         if (auth_oidc_is_local_365_installed()) {
             $mform->addElement(
@@ -230,7 +292,21 @@ public function validation($data, $files) {
         // Validate authentication variables.
         switch ($data['clientauthmethod']) {
             case AUTH_OIDC_AUTH_METHOD_SECRET:
-                if (empty(trim($data['clientsecret']))) {
+                // Check if user is attempting to change the secret.
+                $changesecret = isset($data['changesecret']) ? $data['changesecret'] : true;
+                $existingsecret = get_config('auth_oidc', 'clientsecret');
+
+                if ($changesecret) {
+                    // User wants to change the secret, validate the new value.
+                    if (empty(trim($data['clientsecret']))) {
+                        $errors['clientsecret'] = get_string('error_empty_client_secret', 'auth_oidc');
+                    } else if (auth_oidc_is_masked_secret($data['clientsecret'])) {
+                        // User checked "change secret" but didn't enter a new value.
+                        $errors['clientsecret'] = get_string('error_masked_secret_not_changed', 'auth_oidc');
+                    }
+                } else if (empty($existingsecret) && empty(trim($data['clientsecret']))) {
+                    // No existing secret and field is empty - this is invalid.
+                    // This handles edge cases where checkbox logic fails.
                     $errors['clientsecret'] = get_string('error_empty_client_secret', 'auth_oidc');
                 }
                 break;
@@ -253,6 +329,16 @@ public function validation($data, $files) {
                         }
                         break;
                 }
+
+                // Validate certificate passphrase if user is attempting to change it.
+                $changecertpassphrase = isset($data['changecertpassphrase']) ? $data['changecertpassphrase'] : true;
+
+                if ($changecertpassphrase && !empty($data['clientcertpassphrase'])) {
+                    if (auth_oidc_is_masked_secret($data['clientcertpassphrase'])) {
+                        // User checked "change passphrase" but didn't enter a new value.
+                        $errors['clientcertpassphrase'] = get_string('error_masked_secret_not_changed', 'auth_oidc');
+                    }
+                }
                 break;
         }
 
@@ -299,6 +385,70 @@ public function validation($data, $files) {
             }
         }
 
+        // Validate custom claims.
+        if (!empty($data['customclaims'])) {
+            $claims = explode(' ', $data['customclaims']);
+            foreach ($claims as $claim) {
+                $claim = trim($claim);
+                if (!empty($claim) && !preg_match('/^[a-zA-Z0-9_-]+$/', $claim)) {
+                    $errors['customclaims'] = get_string('error_invalid_custom_claim', 'auth_oidc');
+                    break;
+                }
+            }
+        }
+
         return $errors;
     }
+
+    /**
+     * Process data after form definition and data loading.
+     * This is called after set_data() and after validation errors, allowing us to override submitted values.
+     *
+     * @return void
+     */
+    public function definition_after_data() {
+        parent::definition_after_data();
+
+        $mform =& $this->_form;
+
+        // Get the current checkbox states using getSubmitValue (works for submitted data).
+        $changesecret = $mform->getSubmitValue('changesecret');
+        $changecertpassphrase = $mform->getSubmitValue('changecertpassphrase');
+
+        // If the "change secret" checkbox is NOT checked and there's an existing secret,
+        // ensure the field shows the masked value (especially important after validation errors).
+        if (isset($this->_customdata['oidcconfig']->clientsecret) && !empty($this->_customdata['oidcconfig']->clientsecret)) {
+            $currentvalue = $mform->getSubmitValue('clientsecret');
+
+            // If checkbox is not checked and field is empty or doesn't match masked value, restore it.
+            if (empty($changesecret) && (empty($currentvalue) || !auth_oidc_is_masked_secret($currentvalue))) {
+                $maskedsecret = auth_oidc_mask_secret($this->_customdata['oidcconfig']->clientsecret);
+                // Force the element to use the masked value.
+                $element = $mform->getElement('clientsecret');
+                $element->setValue($maskedsecret);
+
+                // Also update the data attribute for JavaScript reliability.
+                $element->updateAttributes(['data-original-masked' => $maskedsecret]);
+            }
+        }
+
+        // Same logic for certificate passphrase.
+        if (
+            isset($this->_customdata['oidcconfig']->clientcertpassphrase) &&
+            !empty($this->_customdata['oidcconfig']->clientcertpassphrase)
+        ) {
+            $currentvalue = $mform->getSubmitValue('clientcertpassphrase');
+
+            // If checkbox is not checked and field is empty or doesn't match masked value, restore it.
+            if (empty($changecertpassphrase) && (empty($currentvalue) || !auth_oidc_is_masked_secret($currentvalue))) {
+                $maskedpassphrase = auth_oidc_mask_secret($this->_customdata['oidcconfig']->clientcertpassphrase);
+                // Force the element to use the masked value.
+                $element = $mform->getElement('clientcertpassphrase');
+                $element->setValue($maskedpassphrase);
+
+                // Also update the data attribute for JavaScript reliability.
+                $element->updateAttributes(['data-original-masked' => $maskedpassphrase]);
+            }
+        }
+    }
 }

classes/loginflow/authcode.php

@@ -27,6 +27,7 @@
 namespace auth_oidc\loginflow;
 
 use auth_oidc\event\user_authed;
+use auth_oidc\event\user_created;
 use auth_oidc\event\user_rename_attempt;
 use auth_oidc\jwt;
 use auth_oidc\utils;
@@ -801,6 +802,15 @@ protected function handlelogin(string $oidcuniqid, array $authparams, array $tok
                         }
                     }
                     $user = create_user_record($username, '', 'oidc');
+
+                    // Trigger user_created event.
+                    $eventdata = [
+                        'objectid' => $user->id,
+                        'userid' => $user->id,
+                        'relateduserid' => $user->id,
+                    ];
+                    $event = user_created::create($eventdata);
+                    $event->trigger();
                 } else {
                     // Trigger login failed event.
                     $failurereason = AUTH_LOGIN_NOUSER;

classes/loginflow/base.php

@@ -674,6 +674,9 @@ protected function createtoken(
         // Cleanup old invalid token with the same oidcusername.
         $DB->delete_records('auth_oidc_token', ['oidcusername' => $oidcusername]);
 
+        // Cleanup old token with the same Moodle username to prevent duplicates.
+        $DB->delete_records('auth_oidc_token', ['username' => $username]);
+
         // Handle "The existing token for this user does not contain a valid user ID" error.
         if ($userid == 0) {
             $userrec = $DB->get_record('user', ['username' => $username]);
@@ -752,7 +755,7 @@ protected function get_oidc_username_from_token_claim(jwt $idtoken, string $bind
 
         switch ($bindingusernameclaim) {
             case 'custom':
-                $bindingusernameclaim = get_config('auth_oidc', 'custombindingclaim');
+                $bindingusernameclaim = get_config('auth_oidc', 'customclaimname');
                 // No break.
             case 'preferred_username':
             case 'email':

classes/loginflow/rocreds.php

@@ -25,6 +25,7 @@
 
 namespace auth_oidc\loginflow;
 
+use auth_oidc\event\user_created;
 use auth_oidc\utils;
 
 defined('MOODLE_INTERNAL') || die();
@@ -137,6 +138,16 @@ public function loginpage_hook(&$frm, &$user) {
         }
 
         $user = create_user_record($username, $password, $auth);
+
+        // Trigger user_created event.
+        $eventdata = [
+            'objectid' => $user->id,
+            'userid' => $user->id,
+            'relateduserid' => $user->id,
+        ];
+        $event = user_created::create($eventdata);
+        $event->trigger();
+
         return true;
     }
 

classes/observers.php

@@ -45,7 +45,6 @@ class observers {
     public static function handle_user_deleted(user_deleted $event) {
         global $DB;
         $userid = $event->objectid;
-        $DB->delete_records('auth_oidc_token', ['userid' => $userid]);
-        return true;
+        return $DB->delete_records('auth_oidc_token', ['userid' => $userid]);
     }
 }

db/install.xml

@@ -58,6 +58,7 @@
         <INDEX NAME="oidcuniqid" UNIQUE="false" FIELDS="oidcuniqid"/>
         <INDEX NAME="userid" UNIQUE="false" FIELDS="userid"/>
         <INDEX NAME="username" UNIQUE="false" FIELDS="username"/>
+        <INDEX NAME="oidcusername" UNIQUE="false" FIELDS="oidcusername"/>
       </INDEXES>
     </TABLE>
     <TABLE NAME="auth_oidc_sid" COMMENT="Stores sid from IdP, to be used to find connected Moodle users when SLO is triggered from IdP.">

db/upgrade.php

@@ -567,5 +567,19 @@ function xmldb_auth_oidc_upgrade($oldversion) {
         upgrade_plugin_savepoint(true, 2024100702, 'auth', 'oidc');
     }
 
+    if ($oldversion < 2025100600.01) {
+        // Define index to be added to auth_oidc_token.
+        $table = new xmldb_table('auth_oidc_token');
+        $index = new xmldb_index('oidcusername', XMLDB_INDEX_NOTUNIQUE, ['oidcusername']);
+
+        // Conditionally launch add index oidcusername.
+        if (!$dbman->index_exists($table, $index)) {
+            $dbman->add_index($table, $index);
+        }
+
+        // Oidc savepoint reached.
+        upgrade_plugin_savepoint(true, 2025100600.01, 'auth', 'oidc');
+    }
+
     return true;
 }

index.php

@@ -29,4 +29,38 @@
 
 $auth = new \auth_plugin_oidc('authcode');
 $auth->set_httpclient(new \auth_oidc\httpclient());
-$auth->handleredirect();
+
+try {
+    $auth->handleredirect();
+} catch (moodle_exception $e) {
+    // If debugging is off, re-throw to let Moodle handle it with generic message.
+    if (empty($CFG->debug) || $CFG->debug < DEBUG_MINIMAL) {
+        throw $e;
+    }
+
+    // Only display detailed debug information if debug display is enabled.
+    // This prevents leaking sensitive internal details to unauthenticated users.
+    $showdetails = !empty($CFG->debugdisplay);
+
+    if ($showdetails) {
+        // Display error details when debug display is enabled.
+        $errormessage = $e->getMessage();
+        if (!empty($e->debuginfo)) {
+            $errormessage .= ' (' . $e->debuginfo . ')';
+        }
+    } else {
+        // Show generic error message to prevent information disclosure.
+        $errormessage = get_string('errorauthgeneral', 'auth_oidc');
+    }
+
+    $PAGE->set_url('/auth/oidc/');
+    $PAGE->set_context(context_system::instance());
+    $PAGE->set_pagelayout('login');
+    $PAGE->set_title(get_string('error'));
+
+    echo $OUTPUT->header();
+    echo $OUTPUT->notification($errormessage, 'error');
+    echo $OUTPUT->single_button(new moodle_url('/login/index.php'), get_string('login'), 'get');
+    echo $OUTPUT->footer();
+    exit;
+}