MDL-70424 auth: Avoid random changes to $CFG->auth

moodle · Feb 23, 2021 · 300213e · 300213e
1 parent a93828a
commit 300213e
Show file tree

Hide file tree

Showing 11 changed files with 31 additions and 28 deletions.
diff --git a/admin/tool/mobile/classes/api.php b/admin/tool/mobile/classes/api.php
@@ -221,7 +221,7 @@ public static function get_public_config() {
         }
 
         // Identity providers.
-        $authsequence = get_enabled_auth_plugins(true);
+        $authsequence = get_enabled_auth_plugins();
         $identityproviders = \auth_plugin_base::get_identity_providers($authsequence);
         $identityprovidersdata = \auth_plugin_base::prepare_identity_providers_for_output($identityproviders, $OUTPUT);
         if (!empty($identityprovidersdata)) {

diff --git a/auth/ldap/ntlmsso_attempt.php b/auth/ldap/ntlmsso_attempt.php
@@ -8,7 +8,7 @@
 // Define variables used in page
 $site = get_site();
 
-$authsequence = get_enabled_auth_plugins(true); // auths, in sequence
+$authsequence = get_enabled_auth_plugins(); // Auths, in sequence.
 if (!in_array('ldap', $authsequence, true)) {
     print_error('ldap_isdisabled', 'auth');
 }

diff --git a/auth/ldap/ntlmsso_finish.php b/auth/ldap/ntlmsso_finish.php
@@ -8,7 +8,7 @@
 // Define variables used in page
 $site = get_site();
 
-$authsequence = get_enabled_auth_plugins(true); // auths, in sequence
+$authsequence = get_enabled_auth_plugins(); // Auths, in sequence.
 if (!in_array('ldap', $authsequence, true)) {
     print_error('ldap_isdisabled', 'auth');
 }

diff --git a/auth/ldap/ntlmsso_magic.php b/auth/ldap/ntlmsso_magic.php
@@ -10,7 +10,7 @@
 
 $PAGE->set_context(context_system::instance());
 
-$authsequence = get_enabled_auth_plugins(true); // auths, in sequence
+$authsequence = get_enabled_auth_plugins(); // Auths, in sequence.
 if (!in_array('ldap', $authsequence, true)) {
     print_error('ldap_isdisabled', 'auth');
 }

diff --git a/auth/ldap/tests/plugin_test.php b/auth/ldap/tests/plugin_test.php
@@ -543,7 +543,7 @@ protected function delete_ldap_user($connection, $topdn, $i) {
     }
 
     protected function enable_plugin() {
-        $auths = get_enabled_auth_plugins(true);
+        $auths = get_enabled_auth_plugins();
         if (!in_array('ldap', $auths)) {
             $auths[] = 'ldap';
 

diff --git a/blocks/login/block_login.php b/blocks/login/block_login.php
@@ -101,7 +101,7 @@ class="form-check-input" value="1" '.$checked.'/> ';
                 $this->content->text .= '<div><a href="'.$forgot.'">'.get_string('forgotaccount').'</a></div>';
             }
 
-            $authsequence = get_enabled_auth_plugins(true); // Get all auths, in sequence.
+            $authsequence = get_enabled_auth_plugins(); // Get all auths, in sequence.
             $potentialidps = array();
             foreach ($authsequence as $authname) {
                 $authplugin = get_auth_plugin($authname);

diff --git a/enrol/lti/tests/sync_members_test.php b/enrol/lti/tests/sync_members_test.php
@@ -259,7 +259,7 @@ public function test_sync_unenrol() {
      * Enable auth_lti plugin.
      */
     protected function enable_auth() {
-        $auths = get_enabled_auth_plugins(true);
+        $auths = get_enabled_auth_plugins();
         if (!in_array('lti', $auths)) {
             $auths[] = 'lti';
         }

diff --git a/lib/adminlib.php b/lib/adminlib.php
@@ -6309,7 +6309,7 @@ public function load_choices() {
         $this->choices = array();
         $this->choices[''] = get_string('disable');
 
-        $authsenabled = get_enabled_auth_plugins(true);
+        $authsenabled = get_enabled_auth_plugins();
 
         foreach ($authsenabled as $auth) {
             $authplugin = get_auth_plugin($auth);

diff --git a/lib/classes/session/manager.php b/lib/classes/session/manager.php
@@ -973,12 +973,12 @@ public static function gc() {
             $rs->close();
 
             // Kill sessions of users with disabled plugins.
-            $auth_sequence = get_enabled_auth_plugins(true);
-            $auth_sequence = array_flip($auth_sequence);
-            unset($auth_sequence['nologin']); // No login means user cannot login.
-            $auth_sequence = array_flip($auth_sequence);
+            $authsequence = get_enabled_auth_plugins();
+            $authsequence = array_flip($authsequence);
+            unset($authsequence['nologin']); // No login means user cannot login.
+            $authsequence = array_flip($authsequence);
 
-            list($notplugins, $params) = $DB->get_in_or_equal($auth_sequence, SQL_PARAMS_QM, '', false);
+            list($notplugins, $params) = $DB->get_in_or_equal($authsequence, SQL_PARAMS_QM, '', false);
             $rs = $DB->get_recordset_select('sessions', "userid IN (SELECT id FROM {user} WHERE auth $notplugins)", $params, 'id DESC', 'id, sid');
             foreach ($rs as $session) {
                 self::kill_session($session->sid);
@@ -993,7 +993,7 @@ public static function gc() {
             $params = array('purgebefore' => (time() - $maxlifetime), 'guestid'=>$CFG->siteguest);
 
             $authplugins = array();
-            foreach ($auth_sequence as $authname) {
+            foreach ($authsequence as $authname) {
                 $authplugins[$authname] = get_auth_plugin($authname);
             }
             $rs = $DB->get_recordset_sql($sql, $params);

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
@@ -2725,7 +2725,7 @@ function require_login($courseorid = null, $autologinguest = true, $cm = null, $
             }
 
             // Give auth plugins an opportunity to authenticate or redirect to an external login page
-            $authsequence = get_enabled_auth_plugins(true); // auths, in sequence
+            $authsequence = get_enabled_auth_plugins(); // Auths, in sequence.
             foreach($authsequence as $authname) {
                 $authplugin = get_auth_plugin($authname);
                 $authplugin->pre_loginpage_hook();
@@ -3919,7 +3919,7 @@ function get_auth_plugin($auth) {
 /**
  * Returns array of active auth plugins.
  *
- * @param bool $fix fix $CFG->auth if needed
+ * @param bool $fix fix $CFG->auth if needed. Only set if logged in as admin.
  * @return array
  */
 function get_enabled_auth_plugins($fix=false) {
@@ -3933,18 +3933,21 @@ function get_enabled_auth_plugins($fix=false) {
         $auths = explode(',', $CFG->auth);
     }
 
-    if ($fix) {
-        $auths = array_unique($auths);
-        $oldauthconfig = implode(',', $auths);
-        foreach ($auths as $k => $authname) {
-            $authplugindoesnotexist = !exists_auth_plugin($authname);
-            if ($authplugindoesnotexist || in_array($authname, $default)) {
-                if ($authplugindoesnotexist) {
-                    debugging(get_string('authpluginnotfound', 'debug', $authname));
-                }
-                unset($auths[$k]);
-            }
+    $auths = array_unique($auths);
+    $oldauthconfig = implode(',', $auths);
+    foreach ($auths as $k => $authname) {
+        if (in_array($authname, $default)) {
+            // The manual and nologin plugin never need to be stored.
+            unset($auths[$k]);
+        } else if (!exists_auth_plugin($authname)) {
+            debugging(get_string('authpluginnotfound', 'debug', $authname));
+            unset($auths[$k]);
         }
+    }
+
+    // Ideally only explicit interaction from a human admin should trigger a
+    // change in auth config, see MDL-70424 for details.
+    if ($fix) {
         $newconfig = implode(',', $auths);
         if (!isset($CFG->auth) or $newconfig != $CFG->auth) {
             add_to_config_log('auth', $oldauthconfig, $newconfig, 'core');

diff --git a/login/index.php b/login/index.php
@@ -82,7 +82,7 @@
 $frm  = false;
 $user = false;
 
-$authsequence = get_enabled_auth_plugins(true); // auths, in sequence
+$authsequence = get_enabled_auth_plugins(); // Auths, in sequence.
 foreach($authsequence as $authname) {
     $authplugin = get_auth_plugin($authname);
     // The auth plugin's loginpage_hook() can eventually set $frm and/or $user.