test: add unit tests for grace mode redirects handling

catalyst · Nov 28, 2023 · 1d89121 · 1d89121
1 parent 1756328
commit 1d89121
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 1 deletion.
diff --git a/factor/grace/classes/factor.php b/factor/grace/classes/factor.php
@@ -110,6 +110,7 @@ public function get_state($redirectable = true) {
 
             if (!empty($duration)) {
                 if (time() > $starttime + $duration) {
+
                     // If gracemode would have given points, but now doesnt,
                     // Jump out of the loop and force a factor setup.
                     // We will return once there is a setup, or the user tries to leave.
@@ -316,7 +317,6 @@ public function get_affecting_factors(): array {
      */
     private function should_skip_force_setup(): bool {
         // Checking if there are contributing factors, that should not cause a redirect.
-        $active = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
         $noredirectlist = get_config('factor_grace', 'noredirectlist');
         if (!$noredirectlist) {
             return false;
@@ -325,6 +325,7 @@ private function should_skip_force_setup(): bool {
         $values = explode(',', $noredirectlist);
         $keys = array_flip($values);
 
+        $active = \tool_mfa\plugininfo\factor::get_active_user_factor_types();
         foreach ($active as $factor) {
             if ($factor->passed()
                 && isset($keys[$factor->name])

diff --git a/factor/grace/tests/factor_test.php b/factor/grace/tests/factor_test.php
@@ -55,4 +55,74 @@ public function test_affecting_factors() {
         $affecting = $grace->get_affecting_factors();
         $this->assertEquals(0, count($affecting));
     }
+
+    /**
+     * Test factors leading to a redirect.
+     */
+    public function test_redirect_factors() {
+        $this->resetAfterTest(true);
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
+
+        $grace = \tool_mfa\plugininfo\factor::get_factor('grace');
+
+        set_config('enabled', 1, 'factor_totp');
+        set_config('enabled', 1, 'factor_grace');
+        set_config('forcesetup', 1, 'factor_grace');
+        set_config('graceperiod', -1, 'factor_grace'); // Grace period expired.
+
+        // Set up exemption factor for a person.
+        set_config('duration', 100, 'factor_exemption');
+        \factor_exemption\factor::add_exemption($user);
+
+        $redirected = false;
+        try {
+            $grace->get_state(true);
+        } catch (\Throwable $e) {
+            $expected = get_string('redirecterrordetected', 'error');
+            if ($expected === $e->getMessage()) {
+                $redirected = true;
+            }
+        }
+
+        $this->assertTrue($redirected, 'No redirect detected, but was expected.');
+    }
+
+    /**
+     * Test factors leading to a redirect, but avoiding it
+     */
+    public function test_gracemode_expires_noredirect() {
+        $this->resetAfterTest(true);
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
+
+        $grace = \tool_mfa\plugininfo\factor::get_factor('grace');
+        $totp = \tool_mfa\plugininfo\factor::get_factor('totp');
+        $exemption = \tool_mfa\plugininfo\factor::get_factor('exemption');
+
+        set_config('enabled', 1, 'factor_totp');
+        set_config('enabled', 1, 'factor_exemption');
+        set_config('enabled', 1, 'factor_grace');
+        set_config('forcesetup', 1, 'factor_grace');
+        set_config('graceperiod', -1, 'factor_grace'); // Grace period expired.
+
+        // Set up exemption factor for a person.
+        set_config('duration', 100, 'factor_exemption');
+        \factor_exemption\factor::add_exemption($user);
+
+        // Set exemption as a factor that should prevent redirects.
+        set_config('noredirectlist', 'exemption', 'factor_grace');
+
+        $redirected = false;
+        try {
+            $grace->get_state(true);
+        } catch (\Throwable $e) {
+            $expected = get_string('redirecterrordetected', 'error');
+            if ($expected === $e->getMessage()) {
+                $redirected = true;
+            }
+        }
+
+        $this->assertFalse($redirected, 'The function cause a redirect, where none was expected.');
+    }
 }