From 271c64043e19a9abab466063a83a524d877f4ca6 Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Tue, 28 Nov 2023 14:16:21 +1100 Subject: [PATCH] test: add unit tests for grace mode redirects handling --- factor/grace/classes/factor.php | 3 +- factor/grace/tests/factor_test.php | 70 ++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/factor/grace/classes/factor.php b/factor/grace/classes/factor.php index 48ab128c..5d97b4a3 100644 --- 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 (isset($keys[$factor->name]) && $factor->passed() diff --git a/factor/grace/tests/factor_test.php b/factor/grace/tests/factor_test.php index f5d8b86a..2ea89ca6 100644 --- 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.'); + } }