From 3ec133e062d985c8d46c5842c028072479afe7eb Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Tue, 3 Mar 2026 13:06:16 +0000
Subject: [PATCH] MDL-88076 tool_mfa: improved parsing of redirect exclusions
 config.

Ensure all permutations of EOL markers are covered/trimmed.
---
 public/admin/tool/mfa/classes/manager.php    |  5 ++-
 public/admin/tool/mfa/tests/manager_test.php | 47 +++++++++++---------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/public/admin/tool/mfa/classes/manager.php b/public/admin/tool/mfa/classes/manager.php
index 74e9ca7216a..dc95d28e1dc 100644
--- a/public/admin/tool/mfa/classes/manager.php
+++ b/public/admin/tool/mfa/classes/manager.php
@@ -605,8 +605,9 @@ class manager {
 
         // Allow forced redirection exclusions.
         if ($exclusions = get_config('tool_mfa', 'redir_exclusions')) {
-            foreach (explode("\n", $exclusions) as $exclusion) {
-                $urls[] = new \moodle_url($exclusion);
+            $exclusions = preg_split('/\n|\r/', $exclusions, -1, PREG_SPLIT_NO_EMPTY);
+            foreach ($exclusions as $exclusion) {
+                $urls[] = new \moodle_url(trim($exclusion));
             }
         }
 
diff --git a/public/admin/tool/mfa/tests/manager_test.php b/public/admin/tool/mfa/tests/manager_test.php
index 16e97565198..944e814ab22 100644
--- a/public/admin/tool/mfa/tests/manager_test.php
+++ b/public/admin/tool/mfa/tests/manager_test.php
@@ -20,6 +20,7 @@ namespace tool_mfa;
  * Tests for MFA manager class.
  *
  * @package     tool_mfa
+ * @covers      \tool_mfa\manager
  * @author      Peter Burnett <peterburnett@catalyst-au.net>
  * @copyright   Catalyst IT
  * @license     http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
@@ -30,9 +31,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests getting the factor total weight
-     *
-     * @covers ::get_total_weight
-     * @covers ::setup_user_factor
      */
     public function test_get_total_weight(): void {
         $this->resetAfterTest(true);
@@ -76,8 +74,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests getting the factor status
-     *
-     * @covers ::get_status
      */
     public function test_get_status(): void {
         $this->resetAfterTest(true);
@@ -117,8 +113,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests checking if passed enough factors
-     *
-     * @covers ::passed_enough_factors
      */
     public function test_passed_enough_factors(): void {
         $this->resetAfterTest(true);
@@ -151,6 +145,30 @@ final class manager_test extends \advanced_testcase {
         $this->assertEquals(true, \tool_mfa\manager::passed_enough_factors());
     }
 
+    /**
+     * Test reading no redirect URLs configuration
+     */
+    public function test_get_no_redirect_urls(): void {
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        // Ensure all EOL markers are covered.
+        $config = "/user/profile.php\r" .
+            "/course/index.php\n" .
+            "/admin/settings.php\r\n" .
+            "/mod/bar/view.php ";
+
+        set_config('redir_exclusions', $config, 'tool_mfa');
+
+        // Cast URLs to string for simpler comparison.
+        $noredirecturls = array_map(fn(\moodle_url $url) => (string) $url, manager::get_no_redirect_urls());
+
+        $this->assertContains((string) (new \moodle_url('/user/profile.php')), $noredirecturls);
+        $this->assertContains((string) (new \moodle_url('/course/index.php')), $noredirecturls);
+        $this->assertContains((string) (new \moodle_url('/admin/settings.php')), $noredirecturls);
+        $this->assertContains((string) (new \moodle_url('/mod/bar/view.php')), $noredirecturls);
+    }
+
     /**
      * The data provider for whether urls should be redirected or not
      *
@@ -178,7 +196,6 @@ final class manager_test extends \advanced_testcase {
     /**
      * Tests whether it should require mfa
      *
-     * @covers ::should_require_mfa
      * @param string $urlstring
      * @param string $webroot
      * @param bool $status
@@ -197,8 +214,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests whether it should require the mfa checks
-     *
-     * @covers ::should_require_mfa
      */
     public function test_should_require_mfa_checks(): void {
         // Setup test and user.
@@ -285,8 +300,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests should require the mfa redirection loop
-     *
-     * @covers ::should_require_mfa
      */
     public function test_should_require_mfa_redirection_loop(): void {
         // Setup test and user.
@@ -341,9 +354,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests checking for possible setup factor
-     *
-     * @covers ::possible_factor_setup
-     * @covers ::setup_user_factor
      */
     public function test_possible_factor_setup(): void {
         // Setup test and user.
@@ -377,8 +387,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests checking if a factor is ready
-     *
-     * @covers ::is_ready
      */
     public function test_is_ready(): void {
         // Setup test and user.
@@ -419,9 +427,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests core hooks
-     *
-     * @covers ::mfa_config_hook_test
-     * @covers ::mfa_login_hook_test
      */
     public function test_core_hooks(): void {
         // Setup test and user.
@@ -439,8 +444,6 @@ final class manager_test extends \advanced_testcase {
 
     /**
      * Tests circular redirect auth
-     *
-     * @covers ::should_require_mfa
      */
     public function test_circular_redirect_auth(): void {
         // Setup test and user.