From 1e80400d09d6042df4f7aeb8f1a9439905a6f418 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 17 Oct 2024 18:40:53 +0200 Subject: [PATCH] [ticket/17414] Extend unit test coverage for base class PHPBB-17414 --- phpBB/phpbb/captcha/factory.php | 2 +- phpBB/phpbb/captcha/plugins/base.php | 5 +- .../phpbb/captcha/plugins/legacy_wrapper.php | 4 +- .../captcha/plugins/plugin_interface.php | 4 +- tests/captcha/qa_test.php | 4 - tests/captcha/turnstile_test.php | 125 +++++++++++++++++- 6 files changed, 130 insertions(+), 14 deletions(-) diff --git a/phpBB/phpbb/captcha/factory.php b/phpBB/phpbb/captcha/factory.php index d9259d8da9..fc974569bb 100644 --- a/phpBB/phpbb/captcha/factory.php +++ b/phpBB/phpbb/captcha/factory.php @@ -65,7 +65,7 @@ class factory function garbage_collect($name) { $captcha = $this->get_instance($name); - $captcha->garbage_collect(0); + $captcha->garbage_collect(); } /** diff --git a/phpBB/phpbb/captcha/plugins/base.php b/phpBB/phpbb/captcha/plugins/base.php index 25355bcb53..43d24f7c8d 100644 --- a/phpBB/phpbb/captcha/plugins/base.php +++ b/phpBB/phpbb/captcha/plugins/base.php @@ -151,6 +151,7 @@ abstract class base implements plugin_interface { $this->code = gen_rand_string_friendly(CAPTCHA_MAX_CHARS); $this->confirm_id = md5(unique_id()); + $this->attempts = 0; $sql = 'INSERT INTO ' . CONFIRM_TABLE . ' ' . $this->db->sql_build_array('INSERT', array( 'confirm_id' => $this->confirm_id, @@ -207,13 +208,13 @@ abstract class base implements plugin_interface /** * @inheritDoc */ - public function garbage_collect(int $confirm_type = 0): void + public function garbage_collect(confirm_type $confirm_type = confirm_type::UNDEFINED): void { $sql = 'SELECT DISTINCT c.session_id FROM ' . CONFIRM_TABLE . ' c LEFT JOIN ' . SESSIONS_TABLE . ' s ON (c.session_id = s.session_id) WHERE s.session_id IS NULL' . - ((empty($type)) ? '' : ' AND c.confirm_type = ' . (int) $type); + ((empty($confirm_type)) ? '' : ' AND c.confirm_type = ' . $confirm_type->value); $result = $this->db->sql_query($sql); if ($row = $this->db->sql_fetchrow($result)) diff --git a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php index 48575733cc..d728a05aba 100644 --- a/phpBB/phpbb/captcha/plugins/legacy_wrapper.php +++ b/phpBB/phpbb/captcha/plugins/legacy_wrapper.php @@ -198,11 +198,11 @@ class legacy_wrapper implements plugin_interface /** * {@inheritDoc} */ - public function garbage_collect(int $confirm_type = 0): void + public function garbage_collect(confirm_type $confirm_type = confirm_type::UNDEFINED): void { if (method_exists($this->legacy_captcha, 'garbage_collect')) { - $this->legacy_captcha->garbage_collect($confirm_type); + $this->legacy_captcha->garbage_collect($confirm_type->value); } } diff --git a/phpBB/phpbb/captcha/plugins/plugin_interface.php b/phpBB/phpbb/captcha/plugins/plugin_interface.php index c6e4e6c6de..f9dd9c6402 100644 --- a/phpBB/phpbb/captcha/plugins/plugin_interface.php +++ b/phpBB/phpbb/captcha/plugins/plugin_interface.php @@ -110,10 +110,10 @@ interface plugin_interface /** * Garbage collect captcha plugin * - * @param int $confirm_type Confirm type to garbage collect, defaults to all (0) + * @param confirm_type $confirm_type Confirm type to garbage collect, defaults to all (0) * @return void */ - public function garbage_collect(int $confirm_type = 0): void; + public function garbage_collect(confirm_type $confirm_type = confirm_type::UNDEFINED): void; /** * Display acp page diff --git a/tests/captcha/qa_test.php b/tests/captcha/qa_test.php index d429336104..1c61a7d10c 100644 --- a/tests/captcha/qa_test.php +++ b/tests/captcha/qa_test.php @@ -41,10 +41,6 @@ class phpbb_captcha_qa_test extends \phpbb_database_test_case public function test_is_installed() { - $this->assertFalse($this->qa->is_installed()); - - $this->qa->install(); - $this->assertTrue($this->qa->is_installed()); } diff --git a/tests/captcha/turnstile_test.php b/tests/captcha/turnstile_test.php index dd9b1347ed..f78c4d3177 100644 --- a/tests/captcha/turnstile_test.php +++ b/tests/captcha/turnstile_test.php @@ -62,7 +62,7 @@ class phpbb_captcha_turnstile_test extends \phpbb_database_test_case { // Mock the dependencies $this->config = $this->createMock(config::class); - $this->db = $this->createMock(driver_interface::class); + $this->db = $this->new_dbal(); $this->language = $this->createMock(language::class); $this->log = $this->createMock(log_interface::class); $this->request = $this->createMock(request::class); @@ -83,7 +83,7 @@ class phpbb_captcha_turnstile_test extends \phpbb_database_test_case ); } - public function testIsAvailable(): void + public function test_is_available(): void { // Test when both sitekey and secret are present $this->config->method('offsetGet')->willReturnMap([ @@ -97,9 +97,128 @@ class phpbb_captcha_turnstile_test extends \phpbb_database_test_case ]); $this->assertTrue($this->turnstile->is_available()); + + $this->assertEquals(0, $this->turnstile->get_attempt_count()); } - public function test_not_vailable(): void + public function test_attempt_count_increase(): void + { + // Test when both sitekey and secret are present + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', 'sitekey_value'], + ['captcha_turnstile_secret', 'secret_value'], + ]); + + $this->request->method('variable')->willReturnMap([ + ['confirm_id', '', false, request_interface::REQUEST, 'confirm_id'], + ['confirm_code', '', false, request_interface::REQUEST, 'confirm_code'] + ]); + + $this->turnstile->init(confirm_type::REGISTRATION); + $this->assertFalse($this->turnstile->validate()); + + $confirm_id_reflection = new \ReflectionProperty($this->turnstile, 'confirm_id'); + $confirm_id = $confirm_id_reflection->getValue($this->turnstile); + + $this->request = $this->createMock(request::class); + $this->request->method('variable')->willReturnMap([ + ['confirm_id', '', false, request_interface::REQUEST, $confirm_id], + ['confirm_code', '', false, request_interface::REQUEST, 'confirm_code'] + ]); + + $this->turnstile = new turnstile( + $this->config, + $this->db, + $this->language, + $this->log, + $this->request, + $this->template, + $this->user + ); + + $this->turnstile->init(confirm_type::REGISTRATION); + $this->assertEquals(1, $this->turnstile->get_attempt_count()); + + // Run some garbage collection + $this->turnstile->garbage_collect(confirm_type::REGISTRATION); + + // Start again at 0 after garbage collection + $this->turnstile->init(confirm_type::REGISTRATION); + $this->assertEquals(0, $this->turnstile->get_attempt_count()); + } + + public function test_reset(): void + { + // Test when both sitekey and secret are present + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', 'sitekey_value'], + ['captcha_turnstile_secret', 'secret_value'], + ]); + + $this->request->method('variable')->willReturnMap([ + ['confirm_id', '', false, request_interface::REQUEST, 'confirm_id'], + ['confirm_code', '', false, request_interface::REQUEST, 'confirm_code'] + ]); + + $this->turnstile->init(confirm_type::REGISTRATION); + $this->assertFalse($this->turnstile->validate()); + $this->turnstile->reset(); + + $confirm_id_reflection = new \ReflectionProperty($this->turnstile, 'confirm_id'); + $confirm_id = $confirm_id_reflection->getValue($this->turnstile); + + $this->request = $this->createMock(request::class); + $this->request->method('variable')->willReturnMap([ + ['confirm_id', '', false, request_interface::REQUEST, $confirm_id], + ['confirm_code', '', false, request_interface::REQUEST, 'confirm_code'] + ]); + + $this->turnstile = new turnstile( + $this->config, + $this->db, + $this->language, + $this->log, + $this->request, + $this->template, + $this->user + ); + + $this->turnstile->init(confirm_type::REGISTRATION); + // Should be zero attempts since we reset the captcha + $this->assertEquals(0, $this->turnstile->get_attempt_count()); + } + + public function test_get_hidden_fields(): void + { + // Test when both sitekey and secret are present + $this->config->method('offsetGet')->willReturnMap([ + ['captcha_turnstile_sitekey', 'sitekey_value'], + ['captcha_turnstile_secret', 'secret_value'], + ]); + + $this->request->method('variable')->willReturnMap([ + ['confirm_id', '', false, request_interface::REQUEST, 'confirm_id'], + ['confirm_code', '', false, request_interface::REQUEST, 'confirm_code'] + ]); + + $this->turnstile->init(confirm_type::REGISTRATION); + $this->assertFalse($this->turnstile->validate()); + $this->turnstile->reset(); + + $confirm_id_reflection = new \ReflectionProperty($this->turnstile, 'confirm_id'); + $confirm_id = $confirm_id_reflection->getValue($this->turnstile); + + $this->assertEquals( + [ + 'confirm_id' => $confirm_id, + 'confirm_code' => '', + ], + $this->turnstile->get_hidden_fields(), + ); + $this->assertEquals('CONFIRM_CODE_WRONG', $this->turnstile->get_error()); + } + + public function test_not_available(): void { $this->request->method('variable')->willReturnMap([ ['confirm_id', '', false, request_interface::REQUEST, 'confirm_id'],